From 267e3bb1576b2f9fa97ae49305aaaa80768ba385 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Tue, 4 Oct 2022 10:14:13 +0000 Subject: dashboard: don't deprecate assets of latest builds It's bad to deprecate them if there are no related crashes, because they may appear later. Check if there are newer builds and only deprecate assets if there are. Also, use a grace period not just for HTML reports deletion. Add a test to check the new behavior. --- dashboard/app/asset_storage.go | 36 ++++++++++++++++++----- dashboard/app/asset_storage_test.go | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 8 deletions(-) (limited to 'dashboard') diff --git a/dashboard/app/asset_storage.go b/dashboard/app/asset_storage.go index 379b845cc..5849977ca 100644 --- a/dashboard/app/asset_storage.go +++ b/dashboard/app/asset_storage.go @@ -154,8 +154,9 @@ func deprecateCrashAssets(c context.Context) error { func deprecateNamespaceAssets(c context.Context, ns string) error { ad := buildAssetDeprecator{ - ns: ns, - c: c, + ns: ns, + c: c, + lastBuilds: map[string]*Build{}, } const buildBatchSize = 16 err := ad.batchProcessBuilds(buildBatchSize) @@ -170,10 +171,24 @@ type buildAssetDeprecator struct { c context.Context bugsQueried bool relevantBugs map[string]bool + lastBuilds map[string]*Build } const keepAssetsForClosedBugs = time.Hour * 24 * 30 +func (ad *buildAssetDeprecator) lastBuild(manager string) (*Build, error) { + build, ok := ad.lastBuilds[manager] + if ok { + return build, nil + } + lastBuild, err := lastManagerBuild(ad.c, ad.ns, manager) + if err != nil { + return nil, err + } + ad.lastBuilds[manager] = lastBuild + return lastBuild, err +} + func (ad *buildAssetDeprecator) queryBugs() error { if ad.bugsQueried { return nil @@ -219,11 +234,6 @@ func (ad *buildAssetDeprecator) queryBugs() error { } func (ad *buildAssetDeprecator) buildArchivePolicy(build *Build, asset *Asset) (bool, error) { - // If the asset is reasonably new, we always keep it. - const alwaysKeepPeriod = time.Hour * 24 * 14 - if asset.CreateDate.After(timeNow(ad.c).Add(-alwaysKeepPeriod)) { - return true, nil - } // Query builds to see whether there's a newer same-type asset on the same week. var builds []*Build _, err := db.NewQuery("Build"). @@ -280,10 +290,20 @@ func (ad *buildAssetDeprecator) buildBugStatusPolicy(build *Build) (bool, error) return true, nil } } - return false, nil + // If there are no crashes, but it's the latest build, they may still appear. + lastBuild, err := ad.lastBuild(build.Manager) + if err != nil { + return false, nil + } + return build.ID == lastBuild.ID, nil } func (ad *buildAssetDeprecator) needThisBuildAsset(build *Build, buildAsset *Asset) (bool, error) { + // If the asset is reasonably new, we always keep it. + const alwaysKeepPeriod = time.Hour * 24 * 14 + if buildAsset.CreateDate.After(timeNow(ad.c).Add(-alwaysKeepPeriod)) { + return true, nil + } if buildAsset.Type == dashapi.HTMLCoverageReport { // We want to keep coverage reports forever, not just // while there are any open bugs. But we don't want to diff --git a/dashboard/app/asset_storage_test.go b/dashboard/app/asset_storage_test.go index eb75d1c5d..ad972cf8b 100644 --- a/dashboard/app/asset_storage_test.go +++ b/dashboard/app/asset_storage_test.go @@ -18,6 +18,7 @@ func TestBuildAssetLifetime(t *testing.T) { defer c.Close() build := testBuild(1) + build.Manager = "test_manager" // Embed one of the assets right away. build.Assets = []dashapi.NewAsset{ { @@ -27,6 +28,12 @@ func TestBuildAssetLifetime(t *testing.T) { } c.client2.UploadBuild(build) + // Add one more build, so that the assets of the previous one could be deprecated. + c.advanceTime(time.Minute) + build2 := testBuild(2) + build2.Manager = "test_manager" + c.client2.UploadBuild(build2) + // "Upload" several more assets. c.expectOK(c.client2.AddBuildAssets(&dashapi.AddBuildAssetsReq{ BuildID: build.ID, @@ -258,6 +265,57 @@ func TestCoverReportDeprecation(t *testing.T) { ensureNeeded([]string{weekOneSecond, weekTwoSecond, weekThreeFirst, weekFourFirst}) } +func TestFreshBuildAssets(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + ensureNeeded := func(needed []string) { + _, err := c.GET("/deprecate_assets") + c.expectOK(err) + neededResp, err := c.client.NeededAssetsList() + c.expectOK(err) + sort.Strings(neededResp.DownloadURLs) + sort.Strings(needed) + c.expectEQ(neededResp.DownloadURLs, needed) + } + + build := testBuild(1) + build.Manager = "manager" + build.Assets = []dashapi.NewAsset{ + { + Type: dashapi.KernelObject, + DownloadURL: "http://google.com/vmlinux", + }, + } + c.client.UploadBuild(build) + + // No crashes yet, but it's the latest build, so the assets must be preserved. + ensureNeeded([]string{"http://google.com/vmlinux"}) + + // Upload one more build for the same manager. + c.advanceTime(time.Minute) + build2 := testBuild(2) + build2.Manager = "manager" + build2.Assets = []dashapi.NewAsset{ + { + Type: dashapi.KernelObject, + DownloadURL: "http://google.com/vmlinux2", + }, + } + c.client.UploadBuild(build2) + + // The assets of the previous build are reasonably new, so they must be kept. + ensureNeeded([]string{"http://google.com/vmlinux", "http://google.com/vmlinux2"}) + + // The assets of the first build must be deprecated now. + c.advanceTime(time.Hour * 24 * 14) + ensureNeeded([]string{"http://google.com/vmlinux2"}) + + // But even if a lot of time passes, but there are no new builds, the assets must stay. + c.advanceTime(time.Hour * 24 * 365) + ensureNeeded([]string{"http://google.com/vmlinux2"}) +} + func TestCrashAssetLifetime(t *testing.T) { c := NewCtx(t) defer c.Close() -- cgit mrf-deployment