From 142c38ee4dae31257923fa929a9e671ae6f28e13 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Wed, 20 Mar 2019 12:00:34 +0100 Subject: dashboard/app: request test/bisect jobs separately Allow separate sets of managers for patch testing and for bisection. This makes things more flexible on syz-ci deployment side. Remove previous hacks for bisection deployment. Update #501 --- dashboard/app/api.go | 4 ++-- dashboard/app/bisect_test.go | 22 ++++++++++------------ dashboard/app/jobs.go | 42 ++++++++++++++++++++++++------------------ dashboard/app/jobs_test.go | 30 +++++++++++++++--------------- dashboard/app/util_test.go | 12 ++++++++++-- dashboard/dashapi/dashapi.go | 6 +++--- syz-ci/jobs.go | 12 +++++++++--- syz-ci/syz-ci.go | 6 ++++-- 8 files changed, 77 insertions(+), 57 deletions(-) diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 1f4fea5dd..88b3471c0 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -356,10 +356,10 @@ func apiJobPoll(c context.Context, r *http.Request, payload []byte) (interface{} if err := json.Unmarshal(payload, req); err != nil { return nil, fmt.Errorf("failed to unmarshal request: %v", err) } - if len(req.Managers) == 0 { + if len(req.PatchTestManagers) == 0 && len(req.BisectManagers) == 0 { return nil, fmt.Errorf("no managers") } - return pollPendingJobs(c, req.Managers) + return pollPendingJobs(c, req.PatchTestManagers, req.BisectManagers) } func apiJobDone(c context.Context, r *http.Request, payload []byte) (interface{}, error) { diff --git a/dashboard/app/bisect_test.go b/dashboard/app/bisect_test.go index 0f1002acd..f8df350c5 100644 --- a/dashboard/app/bisect_test.go +++ b/dashboard/app/bisect_test.go @@ -25,7 +25,7 @@ func TestBisectCause(t *testing.T) { c.client2.pollEmailBug() // No repro - no bisection. - pollResp, _ := c.client2.JobPoll([]string{build.Manager}) + pollResp := c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.ID, "") // Now upload 4 crashes with repros. @@ -55,7 +55,7 @@ func TestBisectCause(t *testing.T) { c.client2.ReportCrash(crash5) c.client2.pollEmailBug() - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectNE(pollResp.ID, "") c.expectEQ(pollResp.Type, dashapi.JobBisectCause) c.expectEQ(pollResp.Manager, build.Manager) @@ -66,7 +66,8 @@ func TestBisectCause(t *testing.T) { c.expectEQ(pollResp.ReproC, []byte("int main() { return 3; }")) // Since we did not reply, we should get the same response. - pollResp2, _ := c.client2.JobPoll([]string{build.Manager}) + c.advanceTime(5 * 24 * time.Hour) + pollResp2 := c.client2.pollJobs(build.Manager) c.expectEQ(pollResp, pollResp2) // Bisection failed with an error. @@ -78,7 +79,7 @@ func TestBisectCause(t *testing.T) { c.expectOK(c.client2.JobDone(done)) // Now we should get bisect for crash 2. - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectNE(pollResp.ID, pollResp2.ID) c.expectEQ(pollResp.ReproOpts, []byte("repro opts 2")) @@ -228,7 +229,7 @@ https://goo.gl/tpsmEJ#testing-patches`, "bugs2@syzkaller.com", "default2@maintainers.com", }) - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) // Bisection succeeded. jobID = pollResp.ID @@ -272,7 +273,7 @@ https://goo.gl/tpsmEJ#testing-patches`, } // No more bisection jobs. - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.ID, "") } @@ -286,8 +287,7 @@ func TestBisectCauseInconclusive(t *testing.T) { c.client2.ReportCrash(crash) msg := c.client2.pollEmailBug() - pollResp, err := c.client2.JobPoll([]string{build.Manager}) - c.expectOK(err) + pollResp := c.client2.pollJobs(build.Manager) jobID := pollResp.ID done := &dashapi.JobDoneReq{ ID: jobID, @@ -405,8 +405,7 @@ func TestBisectCauseAncient(t *testing.T) { c.client2.ReportCrash(crash) msg := c.client2.pollEmailBug() - pollResp, err := c.client2.JobPoll([]string{build.Manager}) - c.expectOK(err) + pollResp := c.client2.pollJobs(build.Manager) jobID := pollResp.ID done := &dashapi.JobDoneReq{ ID: jobID, @@ -511,8 +510,7 @@ func TestBisectCauseExternal(t *testing.T) { c.client.ReportCrash(crash) rep := c.client.pollBug() - pollResp, err := c.client.JobPoll([]string{build.Manager}) - c.expectOK(err) + pollResp := c.client.pollJobs(build.Manager) jobID := pollResp.ID done := &dashapi.JobDoneReq{ ID: jobID, diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index 5ed61839f..1bb67a991 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -188,13 +188,17 @@ func checkTestJob(c context.Context, bug *Bug, bugReporting *BugReporting, crash } // pollPendingJobs returns the next job to execute for the provided list of managers. -func pollPendingJobs(c context.Context, managers []string) (*dashapi.JobPollResp, error) { - mgrs := make(map[string]bool) - for _, mgr := range managers { - mgrs[mgr] = true +func pollPendingJobs(c context.Context, testMgrs, bisectMgrs []string) (*dashapi.JobPollResp, error) { + testManagers := make(map[string]bool) + for _, mgr := range testMgrs { + testManagers[mgr] = true + } + bisectManagers := make(map[string]bool) + for _, mgr := range bisectMgrs { + bisectManagers[mgr] = true } retry: - job, jobKey, err := getNextJob(c, mgrs) + job, jobKey, err := getNextJob(c, testManagers, bisectManagers) if job == nil || err != nil { return nil, err } @@ -208,23 +212,22 @@ retry: return resp, nil } -func getNextJob(c context.Context, managers map[string]bool) (*Job, *datastore.Key, error) { - job, jobKey, err := loadPendingJob(c, managers) +func getNextJob(c context.Context, testManagers, bisectManagers map[string]bool) (*Job, *datastore.Key, error) { + job, jobKey, err := loadPendingJob(c, testManagers, bisectManagers) if job != nil || err != nil { return job, jobKey, err } - // TODO: this is temporal for gradual bisection rollout. - if !appengine.IsDevAppServer() && !managers["ci-gimme-bisect"] { + if len(bisectManagers) == 0 { return nil, nil, nil } // We need both C and syz repros, but the crazy datastore query restrictions // do not allow to use ReproLevel>ReproLevelNone in the query. So we do 2 separate queries. // C repros tend to be of higher reliability so maybe it's not bad. - job, jobKey, err = createBisectJob(c, managers, ReproLevelC) + job, jobKey, err = createBisectJob(c, bisectManagers, ReproLevelC) if job != nil || err != nil { return job, jobKey, err } - return createBisectJob(c, managers, ReproLevelSyz) + return createBisectJob(c, bisectManagers, ReproLevelSyz) } func createBisectJob(c context.Context, managers map[string]bool, reproLevel dashapi.ReproLevel) ( @@ -703,7 +706,7 @@ func jobReported(c context.Context, jobID string) error { return datastore.RunInTransaction(c, tx, nil) } -func loadPendingJob(c context.Context, managers map[string]bool) (*Job, *datastore.Key, error) { +func loadPendingJob(c context.Context, testManagers, bisectManagers map[string]bool) (*Job, *datastore.Key, error) { var jobs []*Job keys, err := datastore.NewQuery("Job"). Filter("Finished=", time.Time{}). @@ -714,12 +717,13 @@ func loadPendingJob(c context.Context, managers map[string]bool) (*Job, *datasto return nil, nil, fmt.Errorf("failed to query jobs: %v", err) } for i, job := range jobs { - if !managers[job.Manager] { - continue - } - if !appengine.IsDevAppServer() && (job.Type == JobBisectCause || job.Type == JobBisectFix) { - // TODO: this is temporal for gradual bisection rollout. - if !managers["ci-gimme-bisect"] { + switch job.Type { + case JobTestPatch: + if !testManagers[job.Manager] { + continue + } + case JobBisectCause, JobBisectFix: + if !bisectManagers[job.Manager] { continue } // Don't retry bisection jobs too often. @@ -730,6 +734,8 @@ func loadPendingJob(c context.Context, managers map[string]bool) (*Job, *datasto timeSince(c, job.Started) < bisectRepeat { continue } + default: + return nil, nil, fmt.Errorf("bad job type %v", job.Type) } return job, keys[i], nil } diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index ae0b7ee23..a5783b63b 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -52,7 +52,7 @@ func TestJob(t *testing.T) { crash.ReproSyz = []byte("repro syz") crash.ReproC = []byte("repro C") c.client2.ReportCrash(crash) - c.client2.pollAndFailBisectJob(build) + c.client2.pollAndFailBisectJob(build.Manager) body = c.pollEmailBug().Body c.expectEQ(strings.Contains(body, "syzbot has found a reproducer"), true) @@ -76,7 +76,7 @@ func TestJob(t *testing.T) { EmailOptFrom("\"foo\" ")) c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) - pollResp, _ := c.client2.JobPoll([]string{build.Manager}) + pollResp := c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.ID, "") // This submits actual test request. @@ -93,9 +93,9 @@ func TestJob(t *testing.T) { c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) - pollResp, _ = c.client2.JobPoll([]string{"foobar"}) + pollResp = c.client2.pollJobs("foobar") c.expectEQ(pollResp.ID, "") - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectNE(pollResp.ID, "") c.expectEQ(pollResp.Type, dashapi.JobTestPatch) c.expectEQ(pollResp.Manager, build.Manager) @@ -108,7 +108,7 @@ func TestJob(t *testing.T) { c.expectEQ(pollResp.ReproSyz, []byte("repro syz")) c.expectEQ(pollResp.ReproC, []byte("repro C")) - pollResp2, _ := c.client2.JobPoll([]string{build.Manager}) + pollResp2 := c.client2.pollJobs(build.Manager) c.expectEQ(pollResp2, pollResp) jobDoneReq := &dashapi.JobDoneReq{ @@ -154,7 +154,7 @@ patch: %[1]v // Testing fails with an error. c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptMessageID(2)) - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.Type, dashapi.JobTestPatch) jobDoneReq = &dashapi.JobDoneReq{ ID: pollResp.ID, @@ -190,7 +190,7 @@ patch: %[1]v // Testing fails with a huge error that can't be inlined in email. c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptMessageID(3)) - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.Type, dashapi.JobTestPatch) jobDoneReq = &dashapi.JobDoneReq{ ID: pollResp.ID, @@ -231,7 +231,7 @@ patch: %[3]v } c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch, EmailOptMessageID(4)) - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.Type, dashapi.JobTestPatch) jobDoneReq = &dashapi.JobDoneReq{ ID: pollResp.ID, @@ -264,7 +264,7 @@ Note: testing is done by a robot and is best-effort only. c.checkURLContents(kernelConfigLink, build.KernelConfig) } - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.ID, "") } @@ -280,13 +280,13 @@ func TestJobWithoutPatch(t *testing.T) { crash.ReproOpts = []byte("repro opts") crash.ReproSyz = []byte("repro syz") c.client2.ReportCrash(crash) - c.client2.pollAndFailBisectJob(build) + c.client2.pollAndFailBisectJob(build.Manager) sender := c.pollEmailBug().Sender _, extBugID, err := email.RemoveAddrContext(sender) c.expectOK(err) c.incomingEmail(sender, "#syz test: git://mygit.com/git.git 5e6a2eea\n", EmailOptMessageID(1)) - pollResp, _ := c.client2.JobPoll([]string{build.Manager}) + pollResp := c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.Type, dashapi.JobTestPatch) testBuild := testBuild(2) testBuild.KernelRepo = "git://mygit.com/git.git" @@ -320,7 +320,7 @@ Note: testing is done by a robot and is best-effort only. c.checkURLContents(kernelConfigLink, testBuild.KernelConfig) } - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.ID, "") } @@ -336,18 +336,18 @@ func TestJobRestrictedManager(t *testing.T) { crash := testCrash(build, 1) crash.ReproSyz = []byte("repro syz") c.client2.ReportCrash(crash) - c.client2.pollAndFailBisectJob(build) + c.client2.pollAndFailBisectJob(build.Manager) sender := c.pollEmailBug().Sender // Testing on a wrong repo must fail and no test jobs passed to manager. c.incomingEmail(sender, "#syz test: git://mygit.com/git.git master\n", EmailOptMessageID(1)) c.expectEQ(strings.Contains((<-c.emailSink).Body, "you should test only on restricted.git"), true) - pollResp, _ := c.client2.JobPoll([]string{build.Manager}) + pollResp := c.client2.pollJobs(build.Manager) c.expectEQ(pollResp.ID, "") // Testing on the right repo must succeed. c.incomingEmail(sender, "#syz test: git://restricted.git/restricted.git master\n", EmailOptMessageID(2)) - pollResp, _ = c.client2.JobPoll([]string{build.Manager}) + pollResp = c.client2.pollJobs(build.Manager) c.expectNE(pollResp.ID, "") c.expectEQ(pollResp.Type, dashapi.JobTestPatch) c.expectEQ(pollResp.Manager, build.Manager) diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index 40ab3f1e1..74540d5b1 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -338,9 +338,17 @@ func (client *apiClient) updateBug(extID string, status dashapi.BugStatus, dup s client.expectTrue(reply.OK) } -func (client *apiClient) pollAndFailBisectJob(build *dashapi.Build) { - resp, err := client.JobPoll([]string{build.Manager}) +func (client *apiClient) pollJobs(manager string) *dashapi.JobPollResp { + resp, err := client.JobPoll(&dashapi.JobPollReq{ + PatchTestManagers: []string{manager}, + BisectManagers: []string{manager}, + }) client.expectOK(err) + return resp +} + +func (client *apiClient) pollAndFailBisectJob(manager string) { + resp := client.pollJobs(manager) client.expectNE(resp.ID, "") client.expectEQ(resp.Type, dashapi.JobBisectCause) done := &dashapi.JobDoneReq{ diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go index 24f2a87a1..42200eb15 100644 --- a/dashboard/dashapi/dashapi.go +++ b/dashboard/dashapi/dashapi.go @@ -121,7 +121,8 @@ func (dash *Dashboard) BuilderPoll(manager string) (*BuilderPollResp, error) { // ID must match JobPollResp.ID. type JobPollReq struct { - Managers []string + PatchTestManagers []string + BisectManagers []string } type JobPollResp struct { @@ -164,8 +165,7 @@ const ( JobBisectFix ) -func (dash *Dashboard) JobPoll(managers []string) (*JobPollResp, error) { - req := &JobPollReq{Managers: managers} +func (dash *Dashboard) JobPoll(req *JobPollReq) (*JobPollResp, error) { resp := new(JobPollResp) err := dash.Query("job_poll", req, resp) return resp, err diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go index 6b1954ba3..f974e5b39 100644 --- a/syz-ci/jobs.go +++ b/syz-ci/jobs.go @@ -192,11 +192,17 @@ func (jp *JobProcessor) getCommitInfo(mgr *Manager, URL, branch string, commits } func (jp *JobProcessor) pollJobs() { - var names []string + var patchTestManagers, bisectManagers []string for _, mgr := range jp.managers { - names = append(names, mgr.name) + patchTestManagers = append(patchTestManagers, mgr.name) + if mgr.mgrcfg.Bisect { + bisectManagers = append(bisectManagers, mgr.name) + } } - req, err := jp.dash.JobPoll(names) + req, err := jp.dash.JobPoll(&dashapi.JobPollReq{ + PatchTestManagers: patchTestManagers, + BisectManagers: bisectManagers, + }) if err != nil { jp.Errorf("failed to poll jobs: %v", err) return diff --git a/syz-ci/syz-ci.go b/syz-ci/syz-ci.go index be9c506eb..e728a7484 100644 --- a/syz-ci/syz-ci.go +++ b/syz-ci/syz-ci.go @@ -112,8 +112,10 @@ type ManagerConfig struct { // File with kernel cmdline values (optional). KernelCmdline string `json:"kernel_cmdline"` // File with sysctl values (e.g. output of sysctl -a, optional). - KernelSysctl string `json:"kernel_sysctl"` - PollCommits bool `json:"poll_commits"` + KernelSysctl string `json:"kernel_sysctl"` + PollCommits bool `json:"poll_commits"` + Bisect bool `json:"bisect"` + ManagerConfig json.RawMessage `json:"manager_config"` managercfg *mgrconfig.Config } -- cgit mrf-deployment