syz-cluster: share the series skip reason

The existing "no suitable commits found" reason is way too ambiguous.

Make CommitSelector return the exact reason why it decides not to
proceed with the particular patch series and display the reason on the
web dashboard.

2 files changed, 38 insertions, 26 deletions

diff --git a/syz-cluster/pkg/triage/commit.go b/syz-cluster/pkg/triage/commit.go
index 3f7fda5a7..eaf8e3eaf 100644
--- a/syz-cluster/pkg/triage/commit.go
+++ b/syz-cluster/pkg/triage/commit.go
@@ -4,9 +4,9 @@
 package triage
 
 import (
-	"log"
 	"time"
 
+	"github.com/google/syzkaller/pkg/debugtracer"
 	"github.com/google/syzkaller/pkg/vcs"
 	"github.com/google/syzkaller/syz-cluster/pkg/api"
 )
@@ -21,26 +21,37 @@ type TreeOps interface {
 }
 
 type CommitSelector struct {
-	ops TreeOps
+	ops    TreeOps
+	tracer debugtracer.DebugTracer
 }
 
-func NewCommitSelector(ops TreeOps) *CommitSelector {
-	return &CommitSelector{ops: ops}
+func NewCommitSelector(ops TreeOps, tracer debugtracer.DebugTracer) *CommitSelector {
+	return &CommitSelector{ops: ops, tracer: tracer}
 }
 
+type SelectResult struct {
+	Commit string
+	Reason string // Set if Commit is empty.
+}
+
+const (
+	reasonSeriesTooOld = "series lags behind the current HEAD too much"
+	reasonNotApplies   = "series does not apply"
+)
+
 // Select returns the best matching commit hash.
-func (cs *CommitSelector) Select(series *api.Series, tree *api.Tree, lastBuild *api.Build) (string, error) {
+func (cs *CommitSelector) Select(series *api.Series, tree *api.Tree, lastBuild *api.Build) (SelectResult, error) {
 	head, err := cs.ops.HeadCommit(tree)
 	if err != nil || head == nil {
-		return "", err
+		return SelectResult{}, err
 	}
-	log.Printf("current HEAD: %q (%v)", head.Hash, head.Date)
+	cs.tracer.Log("current HEAD: %q (%v)", head.Hash, head.Date)
 	// If the series is already too old, it may be incompatible even if it applies cleanly.
 	const seriesLagsBehind = time.Hour * 24 * 7
 	if diff := head.CommitDate.Sub(series.PublishedAt); series.PublishedAt.Before(head.CommitDate) &&
 		diff > seriesLagsBehind {
-		log.Printf("the series is too old: %v before the HEAD", diff)
-		return "", nil
+		cs.tracer.Log("the series is too old: %v before the HEAD", diff)
+		return SelectResult{Reason: reasonSeriesTooOld}, nil
 	}
 
 	// Algorithm:
@@ -53,20 +64,20 @@ func (cs *CommitSelector) Select(series *api.Series, tree *api.Tree, lastBuild *
 	if lastBuild != nil {
 		// Check if the commit is still good enough.
 		if diff := head.CommitDate.Sub(lastBuild.CommitDate); diff > seriesLagsBehind {
-			log.Printf("the last successful build is already too old: %v, skipping", diff)
+			cs.tracer.Log("the last successful build is already too old: %v, skipping", diff)
 		} else {
 			hashes = append(hashes, lastBuild.CommitHash)
 		}
 	}
 	for _, hash := range append(hashes, head.Hash) {
-		log.Printf("considering %q", hash)
+		cs.tracer.Log("considering %q", hash)
 		err := cs.ops.ApplySeries(hash, series.PatchBodies())
 		if err == nil {
-			log.Printf("series can be applied to %q", hash)
-			return hash, nil
+			cs.tracer.Log("series can be applied to %q", hash)
+			return SelectResult{Commit: hash}, nil
 		} else {
-			log.Printf("failed to apply to %q: %v", hash, err)
+			cs.tracer.Log("failed to apply to %q: %v", hash, err)
 		}
 	}
-	return "", nil
+	return SelectResult{Reason: reasonNotApplies}, nil
 }
diff --git a/syz-cluster/pkg/triage/commit_test.go b/syz-cluster/pkg/triage/commit_test.go
index 5a9df1874..d510d5b9b 100644
--- a/syz-cluster/pkg/triage/commit_test.go
+++ b/syz-cluster/pkg/triage/commit_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/syzkaller/pkg/debugtracer"
 	"github.com/google/syzkaller/pkg/vcs"
 	"github.com/google/syzkaller/syz-cluster/pkg/api"
 	"github.com/stretchr/testify/assert"
@@ -20,39 +21,39 @@ func TestCommitSelector(t *testing.T) {
 		ops    TreeOps
 		series *api.Series
 		last   *api.Build
-		commit string
+		result SelectResult
 	}{
 		{
 			name:   "fresh series, no last build",
 			series: &api.Series{PublishedAt: date("2020-Jan-15")},
 			ops:    newTestGitOps(&vcs.Commit{Hash: "head", CommitDate: date("2020-Jan-10")}, allApply),
-			commit: "head",
+			result: SelectResult{Commit: "head"},
 		},
 		{
 			name:   "fresh series with a fresh last build",
 			series: &api.Series{PublishedAt: date("2020-Jan-15")},
 			ops:    newTestGitOps(&vcs.Commit{Hash: "head", CommitDate: date("2020-Jan-10")}, allApply),
 			last:   &api.Build{CommitHash: "build", CommitDate: date("2020-Jan-06")},
-			commit: "build",
+			result: SelectResult{Commit: "build"},
 		},
 		{
 			name:   "fresh series with a too old last build",
 			series: &api.Series{PublishedAt: date("2020-Jan-15")},
 			ops:    newTestGitOps(&vcs.Commit{Hash: "head", CommitDate: date("2020-Jan-10")}, allApply),
 			last:   &api.Build{CommitHash: "build", CommitDate: date("2019-Dec-20")},
-			commit: "head",
+			result: SelectResult{Commit: "head"},
 		},
 		{
 			name:   "slightly old series, no last build",
 			series: &api.Series{PublishedAt: date("2020-Jan-15")},
 			ops:    newTestGitOps(&vcs.Commit{Hash: "head", CommitDate: date("2020-Jan-20")}, allApply),
-			commit: "head",
+			result: SelectResult{Commit: "head"},
 		},
 		{
 			name:   "a too old series",
 			series: &api.Series{PublishedAt: date("2020-Jan-15")},
 			ops:    newTestGitOps(&vcs.Commit{Hash: "head", CommitDate: date("2020-Feb-15")}, allApply),
-			commit: "",
+			result: SelectResult{Reason: reasonSeriesTooOld},
 		},
 		{
 			name:   "doesn't apply to the known build",
@@ -62,7 +63,7 @@ func TestCommitSelector(t *testing.T) {
 				map[string]bool{"head": true, "build": false},
 			),
 			last:   &api.Build{CommitHash: "build", CommitDate: date("2020-Jan-10")},
-			commit: "head",
+			result: SelectResult{Commit: "head"},
 		},
 		{
 			name:   "doesn't apply anywhere",
@@ -72,16 +73,16 @@ func TestCommitSelector(t *testing.T) {
 				nil,
 			),
 			last:   &api.Build{CommitHash: "build", CommitDate: date("2020-Jan-10")},
-			commit: "",
+			result: SelectResult{Reason: reasonNotApplies},
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			selector := NewCommitSelector(test.ops)
-			commit, err := selector.Select(test.series, testTree, test.last)
+			selector := NewCommitSelector(test.ops, &debugtracer.NullTracer{})
+			result, err := selector.Select(test.series, testTree, test.last)
 			assert.NoError(t, err)
-			assert.Equal(t, test.commit, commit)
+			assert.Equal(t, test.result, result)
 		})
 	}
 }