From 51e54e30cbb17bbb9f6d4b522d0f2f2c05d4fb5c Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Tue, 30 Aug 2022 09:47:03 +0000 Subject: dashboard: retest repros on up-to-date branches Branches and repo addresses for our tested kernels sometimes get changed over time, that breaks repro retesting for some of our old repros. Fix that behavior by retesting the repro not on the repo/branch on which it was originally found, but on the ones from the latest successful build for the corresponding syz-manager. Adjust tests. Close #3341. --- dashboard/app/jobs.go | 57 +++++++++++++++++++++++----------------------- dashboard/app/jobs_test.go | 21 ++++++++++++----- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index dce164bfc..db821b4c8 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -302,7 +302,9 @@ func handleRetestForBug(c context.Context, now time.Time, bug *Bug, bugKey *db.K if managerHasJob[crash.Manager] { continue } - build, err := loadBuild(c, bug.Namespace, crash.BuildID) + // Take the last successful build -- the build on which this crash happened + // might contain already obsolete repro and branch values. + build, err := lastManagerBuild(c, bug.Namespace, crash.Manager) if err != nil { return err } @@ -567,42 +569,38 @@ func createJobResp(c context.Context, job *Job, jobKey *db.Key) (*dashapi.JobPol // miss the situation when some actual user sends a patch testing request without // patch. func isRetestReproJob(job *Job, build *Build) bool { - return job.Type == JobTestPatch && + return (job.Type == JobTestPatch || job.Type == JobBisectFix) && job.Patch == 0 && job.KernelRepo == build.KernelRepo && job.KernelBranch == build.KernelBranch } -func handleRetestedRepro(c context.Context, now time.Time, job *Job, jobKey *db.Key, req *dashapi.JobDoneReq) error { +func handleRetestedRepro(c context.Context, now time.Time, job *Job, jobKey *db.Key, + lastBuild *Build, req *dashapi.JobDoneReq) error { bugKey := jobKey.Parent() crashKey := db.NewKey(c, "Crash", "", job.CrashID, bugKey) crash := new(Crash) if err := db.Get(c, crashKey, crash); err != nil { return fmt.Errorf("failed to get crash: %v", crashKey) } - build, err := loadBuild(c, job.Namespace, crash.BuildID) - if err != nil { - return fmt.Errorf("failed to query build: %w", err) - } - if !isRetestReproJob(job, build) { - return nil - } bug := new(Bug) if err := db.Get(c, bugKey, bug); err != nil { return fmt.Errorf("failed to get bug: %v", bugKey) } - if !config.Namespaces[bug.Namespace].RetestRepros { - return nil - } + allTitles := gatherCrashTitles(req) // Update the crash. crash.LastReproRetest = now - allTitles := gatherCrashTitles(req) - if req.Error == nil && len(allTitles) == 0 { - // If there was any crash at all, the repro is still not worth discarding. - // Also, if repro testing itself failed, it might be just a temporary issue. - crash.ReproIsRevoked = true + if req.Error == nil && !crash.ReproIsRevoked { + // If repro testing itself failed, it might be just a temporary issue. + if job.Type == JobTestPatch { + // If there was any crash at all, the repro is still not worth discarding. + crash.ReproIsRevoked = len(allTitles) == 0 + } else if job.Type == JobBisectFix { + // More than one commit is suspected => repro stopped working at some point. + crash.ReproIsRevoked = len(req.Commits) > 0 + } } - crash.UpdateReportingPriority(build, bug) + crash.UpdateReportingPriority(lastBuild, bug) if _, err := db.Put(c, crashKey, crash); err != nil { return fmt.Errorf("failed to put crash: %v", err) } @@ -652,6 +650,16 @@ func doneJob(c context.Context, req *dashapi.JobDoneReq) error { if err != nil { return err } + // Datastore prohibits cross-group queries even inside XG transactions. + // So we have to query last build for the manager before the transaction. + job := new(Job) + if err := db.Get(c, jobKey, job); err != nil { + return fmt.Errorf("job %v: failed to get job: %v", jobID, err) + } + lastBuild, err := lastManagerBuild(c, job.Namespace, job.Manager) + if err != nil { + return err + } now := timeNow(c) tx := func(c context.Context) error { job := new(Job) @@ -661,8 +669,8 @@ func doneJob(c context.Context, req *dashapi.JobDoneReq) error { if !job.Finished.IsZero() { return fmt.Errorf("job %v: already finished", jobID) } - if job.Type == JobTestPatch { - err := handleRetestedRepro(c, now, job, jobKey, req) + if isRetestReproJob(job, lastBuild) { + err := handleRetestedRepro(c, now, job, jobKey, lastBuild, req) if err != nil { return fmt.Errorf("job %v: failed to handle retested repro, %w", jobID, err) } @@ -749,13 +757,6 @@ func updateBugBisection(c context.Context, job *Job, jobKey *db.Key, req *dashap if _, err := db.Put(c, bugKey, bug); err != nil { return fmt.Errorf("failed to put bug: %v", err) } - // The repro is not working on the HEAD commit anymore, update the repro status. - if job.Type == JobBisectFix && req.Error == nil && len(req.Commits) > 0 { - err := handleRetestedRepro(c, now, job, jobKey, req) - if err != nil { - return err - } - } _, bugReporting, _, _, _ := currentReporting(c, bug) // The bug is either already closed or not yet reported in the current reporting, // either way we don't need to report it. If it wasn't reported, it will be reported diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index 31e4eafea..8bb9ec464 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -374,12 +374,12 @@ func TestReproRetestJob(t *testing.T) { c := NewCtx(t) defer c.Close() - build := testBuild(1) - build.KernelRepo = "git://mygit.com/git.git" - build.KernelBranch = "main" - c.client2.UploadBuild(build) + oldBuild := testBuild(1) + oldBuild.KernelRepo = "git://mygit.com/git.git" + oldBuild.KernelBranch = "main" + c.client2.UploadBuild(oldBuild) - crash := testCrash(build, 1) + crash := testCrash(oldBuild, 1) crash.ReproOpts = []byte("repro opts") crash.ReproSyz = []byte("repro syz") c.client2.ReportCrash(crash) @@ -387,11 +387,20 @@ func TestReproRetestJob(t *testing.T) { _, extBugID, err := email.RemoveAddrContext(sender) c.expectOK(err) - crash2 := testCrash(build, 1) + crash2 := testCrash(oldBuild, 1) crash2.ReproOpts = []byte("repro opts") crash2.ReproSyz = []byte("repro syz") crash2.ReproC = []byte("repro C") c.client2.ReportCrash(crash2) + + // Upload a newer build. + c.advanceTime(time.Minute) + build := testBuild(1) + build.ID = "new-build" + build.KernelRepo = "git://mygit.com/new-git.git" + build.KernelBranch = "new-main" + c.client2.UploadBuild(build) + // Wait until the bug is upstreamed. c.advanceTime(15 * 24 * time.Hour) c.pollEmailBug() -- cgit mrf-deployment