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

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\" <blAcklisteD@dOmain.COM>"))
 	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
 }