pkg/bisect: abort on infrastructure errors

There's not much sense to continue the bisection if something in the
testing infrastructure got broken. Indicate it with the InfraError
error.

2 files changed, 80 insertions, 20 deletions

diff --git a/pkg/bisect/bisect.go b/pkg/bisect/bisect.go
index 294ba7a2a..b4b211af6 100644
--- a/pkg/bisect/bisect.go
+++ b/pkg/bisect/bisect.go
@@ -111,6 +111,14 @@ type Result struct {
 	IsRelease  bool
 }
 
+type InfraError struct {
+	Title string
+}
+
+func (e InfraError) Error() string {
+	return e.Title
+}
+
 // Run does the bisection and returns either the Result,
 // or, if the crash is not reproduced on the start commit, an error.
 func Run(cfg *Config) (*Result, error) {
@@ -577,33 +585,39 @@ func (env *env) test() (*testResult, error) {
 	results, err := env.inst.Test(numTests, cfg.Repro.Syz, cfg.Repro.Opts, cfg.Repro.C)
 	env.testTime += time.Since(testStart)
 	if err != nil {
-		res.rep = &report.Report{
-			Title: fmt.Sprintf("failed testing reproducer on %v: %v", current.Hash, err),
-		}
-		env.log(res.rep.Title)
-		return res, nil
+		problem := fmt.Sprintf("repro testing failure: %v", err)
+		env.log(problem)
+		return res, &InfraError{Title: problem}
 	}
-	bad, good, rep := env.processResults(current, results)
+	bad, good, infra, rep := env.processResults(current, results)
 	res.rep = rep
 	res.verdict = vcs.BisectSkip
+	if infra > len(results)/2 {
+		// More than 1/2 of runs failed with infrastructure error,
+		// no sense in continuing the bisection at the moment.
+		return res, &InfraError{Title: "more than 50% of runs failed with an infra error"}
+	}
 	if bad != 0 {
 		res.verdict = vcs.BisectBad
 		if !env.flaky && bad < good {
 			env.log("reproducer seems to be flaky")
 			env.flaky = true
 		}
-	} else if len(results)-good-bad > len(results)/3*2 {
-		// More than 2/3 of instances failed with infrastructure error,
-		// can't reliably tell that the commit is good.
-		res.verdict = vcs.BisectSkip
 	} else if good != 0 {
 		res.verdict = vcs.BisectGood
+	} else {
+		res.rep = &report.Report{
+			Title: fmt.Sprintf("failed testing reproducer on %v", current.Hash),
+		}
 	}
+	// If all runs failed with a boot/test error, we just end up with BisectSkip.
+	// TODO: when we start supporting boot/test error bisection, we need to make
+	// processResults treat that verdit as "good".
 	return res, nil
 }
 
 func (env *env) processResults(current *vcs.Commit, results []instance.EnvTestResult) (
-	bad, good int, rep *report.Report) {
+	bad, good, infra int, rep *report.Report) {
 	var verdicts []string
 	for i, res := range results {
 		if res.Error == nil {
@@ -613,7 +627,10 @@ func (env *env) processResults(current *vcs.Commit, results []instance.EnvTestRe
 		}
 		switch err := res.Error.(type) {
 		case *instance.TestError:
-			if err.Boot {
+			if err.Infra {
+				infra++
+				verdicts = append(verdicts, fmt.Sprintf("infra problem: %v", err))
+			} else if err.Boot {
 				verdicts = append(verdicts, fmt.Sprintf("boot failed: %v", err))
 			} else {
 				verdicts = append(verdicts, fmt.Sprintf("basic kernel testing failed: %v", err))
@@ -633,6 +650,7 @@ func (env *env) processResults(current *vcs.Commit, results []instance.EnvTestRe
 			}
 			env.saveDebugFile(current.Hash, i, output)
 		default:
+			infra++
 			verdicts = append(verdicts, fmt.Sprintf("failed: %v", err))
 		}
 	}
diff --git a/pkg/bisect/bisect_test.go b/pkg/bisect/bisect_test.go
index d5c369272..4dbed9e34 100644
--- a/pkg/bisect/bisect_test.go
+++ b/pkg/bisect/bisect_test.go
@@ -4,6 +4,7 @@
 package bisect
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"testing"
@@ -51,7 +52,33 @@ func (env *testEnv) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]inst
 	commit := env.headCommit()
 	if commit >= env.test.brokenStart && commit <= env.test.brokenEnd ||
 		env.config == "baseline-skip" {
-		return nil, fmt.Errorf("broken build")
+		var ret []instance.EnvTestResult
+		for i := 0; i < numVMs; i++ {
+			ret = append(ret, instance.EnvTestResult{
+				Error: &instance.TestError{
+					Boot:  true,
+					Title: "kernel doesn't boot",
+				},
+			})
+		}
+		return ret, nil
+	}
+	if commit >= env.test.infraErrStart && commit <= env.test.infraErrEnd {
+		var ret []instance.EnvTestResult
+		for i := 0; i < numVMs; i++ {
+			var err error
+			// More than 50% failures.
+			if i*2 <= numVMs {
+				err = &instance.TestError{
+					Infra: true,
+					Title: "failed to create a VM",
+				}
+			}
+			ret = append(ret, instance.EnvTestResult{
+				Error: err,
+			})
+		}
+		return ret, nil
 	}
 	var ret []instance.EnvTestResult
 	if (env.config == "baseline-repro" || env.config == "new-minimized-config" || env.config == "original config") &&
@@ -147,6 +174,9 @@ func testBisection(t *testing.T, baseDir string, test BisectionTest) {
 		if test.expectErr != (err != nil) {
 			t.Fatalf("expected error %v, got %v", test.expectErr, err)
 		}
+		if test.expectErrType != nil && !errors.As(err, &test.expectErrType) {
+			t.Fatalf("expected %#v error, got %#v", test.expectErrType, err)
+		}
 		if err != nil {
 			if res != nil {
 				t.Fatalf("got both result and error: '%v' %+v", err, *res)
@@ -194,16 +224,19 @@ func checkBisectionResult(t *testing.T, test BisectionTest, res *Result) {
 
 type BisectionTest struct {
 	// input environment
-	name        string
-	fix         bool
-	startCommit int
-	brokenStart int
-	brokenEnd   int
+	name          string
+	fix           bool
+	startCommit   int
+	brokenStart   int
+	brokenEnd     int
+	infraErrStart int
+	infraErrEnd   int
 	// Range of commits that result in the same kernel binary signature.
 	sameBinaryStart int
 	sameBinaryEnd   int
 	// expected output
-	expectErr bool
+	expectErr     bool
+	expectErrType any
 	// Expect res.Report != nil.
 	expectRep  bool
 	noopChange bool
@@ -492,6 +525,16 @@ var bisectionTests = []BisectionTest{
 		sameBinaryEnd:   650,
 		noopChange:      true,
 	},
+	{
+		name:          "cause-infra-problems",
+		startCommit:   905,
+		expectRep:     false,
+		expectErr:     true,
+		expectErrType: &InfraError{},
+		infraErrStart: 600,
+		infraErrEnd:   800,
+		culprit:       602,
+	},
 }
 
 func TestBisectionResults(t *testing.T) {
@@ -525,7 +568,6 @@ func checkTest(t *testing.T, test BisectionTest) {
 		(test.commitLen != 0 ||
 			test.expectRep ||
 			test.oldestLatest != 0 ||
-			test.culprit != 0 ||
 			test.resultingConfig != "") {
 		t.Fatalf("expecting non-default values on error")
 	}