roachtest: remove direct CRDB log usages in roachtest

The only usage of the CRDB log in roachtest is now to configure it. Epic: None Release note: None
cockroachdb · Oct 14, 2024 · 2059cac · 2059cac
1 parent 30b31b2
commit 2059cac
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 17 deletions.
diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go
@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"log"
 	"math/rand"
 	"net"
 	"net/url"
@@ -47,7 +48,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm/gce"
 	"github.com/cockroachdb/cockroach/pkg/util/httputil"
-	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
@@ -974,7 +974,7 @@ func (f *clusterFactory) newCluster(
 		logPath := filepath.Join(f.artifactsDir, runnerLogsDir, "cluster-create", genName+retryStr+".log")
 		l, err := logger.RootLogger(logPath, teeOpt)
 		if err != nil {
-			log.Fatalf(ctx, "%v", err)
+			log.Fatalf("%v", err)
 		}
 
 		c := &clusterImpl{

diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go
@@ -39,7 +39,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/util/allstacks"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
-	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
@@ -580,7 +579,7 @@ func (r *testRunner) runWorker(
 ) error {
 	stdout := lopt.stdout
 
-	wStatus := r.addWorker(ctx, name)
+	wStatus := r.addWorker(ctx, l, name)
 	defer func() {
 		r.removeWorker(ctx, name)
 	}()
@@ -649,7 +648,7 @@ func (r *testRunner) runWorker(
 		testToRun := testToRunRes{noWork: true}
 		if c != nil {
 			// Try to reuse cluster.
-			testToRun = work.selectTestForCluster(ctx, c.spec, r.cr, roachtestflags.Cloud)
+			testToRun = work.selectTestForCluster(ctx, l, c.spec, r.cr, roachtestflags.Cloud)
 			if !testToRun.noWork {
 				// We found a test to run on this cluster. Wipe the cluster.
 				if err := c.WipeForReuse(ctx, l, testToRun.spec.Cluster); err != nil {
@@ -1694,12 +1693,12 @@ func (r *testRunner) generateReport() string {
 }
 
 // addWorker updates the bookkeeping for one more worker.
-func (r *testRunner) addWorker(ctx context.Context, name string) *workerStatus {
+func (r *testRunner) addWorker(ctx context.Context, l *logger.Logger, name string) *workerStatus {
 	r.workersMu.Lock()
 	defer r.workersMu.Unlock()
 	w := &workerStatus{name: name}
 	if _, ok := r.workersMu.workers[name]; ok {
-		log.Fatalf(ctx, "worker %q already exists", name)
+		l.FatalfCtx(ctx, "worker %q already exists", name)
 	}
 	r.workersMu.workers[name] = w
 	return w

diff --git a/pkg/cmd/roachtest/work_pool.go b/pkg/cmd/roachtest/work_pool.go
@@ -14,7 +14,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
-	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 )
@@ -83,7 +82,7 @@ func (p *workPool) workRemaining() []testWithCount {
 //
 // cr is used for its information about how many clusters with a given tag currently exist.
 func (p *workPool) selectTestForCluster(
-	ctx context.Context, s spec.ClusterSpec, cr *clusterRegistry, cloud spec.Cloud,
+	ctx context.Context, l *logger.Logger, s spec.ClusterSpec, cr *clusterRegistry, cloud spec.Cloud,
 ) testToRunRes {
 	p.mu.Lock()
 	defer p.mu.Unlock()
@@ -101,14 +100,14 @@ func (p *workPool) selectTestForCluster(
 	candidateScore := 0
 	var candidate testWithCount
 	for _, tc := range testsWithCounts {
-		score := scoreTestAgainstCluster(tc, tag, cr)
+		score := scoreTestAgainstCluster(l, tc, tag, cr)
 		if score > candidateScore {
 			candidateScore = score
 			candidate = tc
 		}
 	}
 
-	p.decTestLocked(ctx, candidate.spec.Name)
+	p.decTestLocked(ctx, l, candidate.spec.Name)
 
 	runNum := p.count - candidate.count + 1
 	return testToRunRes{
@@ -177,7 +176,7 @@ func (p *workPool) selectTest(
 
 		tc := p.mu.tests[candidateIdx]
 		runNum := p.count - tc.count + 1
-		p.decTestLocked(ctx, tc.spec.Name)
+		p.decTestLocked(ctx, l, tc.spec.Name)
 		ttr = testToRunRes{
 			spec:            tc.spec,
 			runCount:        p.count,
@@ -206,13 +205,16 @@ func (p *workPool) selectTest(
 //
 // cr is used for its information about how many clusters with a given tag
 // currently exist.
-func scoreTestAgainstCluster(tc testWithCount, tag string, cr *clusterRegistry) int {
+func scoreTestAgainstCluster(
+	l *logger.Logger, tc testWithCount, tag string, cr *clusterRegistry,
+) int {
 	t := tc.spec
 	testPolicy := t.Cluster.ReusePolicy
 	if tag != "" && testPolicy != (spec.ReusePolicyTagged{Tag: tag}) {
-		log.Fatalf(context.TODO(),
+		l.Fatalf(
 			"incompatible test and cluster. Cluster tag: %s. Test policy: %+v",
-			tag, t.Cluster.ReusePolicy)
+			tag, t.Cluster.ReusePolicy,
+		)
 	}
 	score := 0
 	if _, ok := testPolicy.(spec.ReusePolicyAny); ok {
@@ -253,15 +255,15 @@ func (p *workPool) findCompatibleTestsLocked(
 
 // decTestLocked decrements a test's remaining count and removes it
 // from the workPool if it was exhausted.
-func (p *workPool) decTestLocked(ctx context.Context, name string) {
+func (p *workPool) decTestLocked(ctx context.Context, l *logger.Logger, name string) {
 	idx := -1
 	for idx = range p.mu.tests {
 		if p.mu.tests[idx].spec.Name == name {
 			break
 		}
 	}
 	if idx == -1 {
-		log.Fatalf(ctx, "failed to find test: %s", name)
+		l.FatalfCtx(ctx, "failed to find test: %s", name)
 	}
 	tc := &p.mu.tests[idx]
 	tc.count--