roachtest: Allow roachtests to opt out of failing in post validation …

…when dead nodes are detected. Epic: none Fixes: cockroachdb#102131 Release note: None Release justification: test only changes
smg260 · Jun 15, 2023 · e3a4669 · e3a4669
1 parent 6119b42
commit e3a4669
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 43 deletions.
diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go
@@ -1526,20 +1526,11 @@ func (c *clusterImpl) assertNoDeadNode(ctx context.Context, t test.Test) error {
 		return err
 	}
 
-	isDead := func(msg string) bool {
-		if msg == "" || msg == "skipped" {
-			return false
-		}
-		// A numeric message is a PID and implies that the node is running.
-		_, err := strconv.Atoi(msg)
-		return err != nil
-	}
-
 	deadNodes := 0
 	for n := range ch {
 		// If there's an error, it means either that the monitor command failed
 		// completely, or that it found a dead node worth complaining about.
-		if n.Err != nil || isDead(n.Msg) {
+		if n.Err != nil || strings.HasPrefix(n.Msg, "dead") {
 			deadNodes++
 		}
 

diff --git a/pkg/cmd/roachtest/registry/test_spec.go b/pkg/cmd/roachtest/registry/test_spec.go
@@ -118,6 +118,8 @@ const (
 	// PostValidationInvalidDescriptors checks if there exists any descriptors in
 	// the crdb_internal.invalid_objects virtual table.
 	PostValidationInvalidDescriptors
+	// PostValidationNoDeadNodes checks if there are any dead nodes in the cluster.
+	PostValidationNoDeadNodes
 )
 
 // MatchType is the type of match a file has to a TestFilter.

diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go
@@ -1147,7 +1147,12 @@ func (r *testRunner) teardownTest(
 		// If this occurs frequently enough, we can look at skipping post validations on a node
 		// failure (or even on any test failure).
 		if err := c.assertNoDeadNode(ctx, t); err != nil {
-			t.Error(err)
+			// Some tests expect dead nodes, so they may opt out of this check.
+			if t.spec.SkipPostValidations&registry.PostValidationNoDeadNodes == 0 {
+				t.Error(err)
+			} else {
+				t.L().Printf("dead node(s) detected but expected")
+			}
 		}
 
 		// We collect all the admin health endpoints in parallel,

diff --git a/pkg/cmd/roachtest/tests/decommissionbench.go b/pkg/cmd/roachtest/tests/decommissionbench.go
@@ -293,9 +293,10 @@ func registerDecommissionBenchSpec(r registry.Registry, benchSpec decommissionBe
 			benchSpec.nodes+addlNodeCount+1,
 			specOptions...,
 		),
-		Timeout:           timeout,
-		NonReleaseBlocker: true,
-		Skip:              benchSpec.skip,
+		SkipPostValidations: registry.PostValidationNoDeadNodes,
+		Timeout:             timeout,
+		NonReleaseBlocker:   true,
+		Skip:                benchSpec.skip,
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			if benchSpec.duration > 0 {
 				runDecommissionBenchLong(ctx, t, c, benchSpec, timeout)

diff --git a/pkg/cmd/roachtest/tests/disk_stall.go b/pkg/cmd/roachtest/tests/disk_stall.go
@@ -54,10 +54,11 @@ func registerDiskStalledDetection(r registry.Registry) {
 	for name, makeStaller := range stallers {
 		name, makeStaller := name, makeStaller
 		r.Add(registry.TestSpec{
-			Name:    fmt.Sprintf("disk-stalled/%s", name),
-			Owner:   registry.OwnerStorage,
-			Cluster: makeSpec(),
-			Timeout: 30 * time.Minute,
+			Name:                fmt.Sprintf("disk-stalled/%s", name),
+			Owner:               registry.OwnerStorage,
+			Cluster:             makeSpec(),
+			Timeout:             30 * time.Minute,
+			SkipPostValidations: registry.PostValidationNoDeadNodes,
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 				runDiskStalledDetection(ctx, t, c, makeStaller(t, c), true /* doStall */)
 			},

diff --git a/pkg/cmd/roachtest/tests/drain.go b/pkg/cmd/roachtest/tests/drain.go
@@ -57,10 +57,11 @@ func registerDrain(r registry.Registry) {
 		})
 
 		r.Add(registry.TestSpec{
-			Name:    "drain/not-at-quorum",
-			Owner:   registry.OwnerSQLFoundations,
-			Cluster: r.MakeClusterSpec(3),
-			Leases:  registry.MetamorphicLeases,
+			Name:                "drain/not-at-quorum",
+			Owner:               registry.OwnerSQLFoundations,
+			Cluster:             r.MakeClusterSpec(3),
+			SkipPostValidations: registry.PostValidationNoDeadNodes,
+			Leases:              registry.MetamorphicLeases,
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 				runClusterNotAtQuorum(ctx, t, c)
 			},

diff --git a/pkg/cmd/roachtest/tests/failover.go b/pkg/cmd/roachtest/tests/failover.go
@@ -86,34 +86,41 @@ func registerFailover(r registry.Registry) {
 				}
 				return s
 			}
+			var postValidation registry.PostValidation = 0
+			if failureMode == failureModeDiskStall {
+				postValidation = registry.PostValidationNoDeadNodes
+			}
 			r.Add(registry.TestSpec{
-				Name:      fmt.Sprintf("failover/non-system/%s%s", failureMode, suffix),
-				Owner:     registry.OwnerKV,
-				Benchmark: true,
-				Timeout:   30 * time.Minute,
-				Cluster:   makeSpec(7 /* nodes */, 4 /* cpus */),
+				Name:                fmt.Sprintf("failover/non-system/%s%s", failureMode, suffix),
+				Owner:               registry.OwnerKV,
+				Benchmark:           true,
+				Timeout:             30 * time.Minute,
+				SkipPostValidations: postValidation,
+				Cluster:             makeSpec(7 /* nodes */, 4 /* cpus */),
 
 				Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 					runFailoverNonSystem(ctx, t, c, failureMode, expirationLeases)
 				},
 			})
 			r.Add(registry.TestSpec{
-				Name:      fmt.Sprintf("failover/liveness/%s%s", failureMode, suffix),
-				Owner:     registry.OwnerKV,
-				Benchmark: true,
-				Timeout:   30 * time.Minute,
-				Cluster:   makeSpec(5 /* nodes */, 4 /* cpus */),
+				Name:                fmt.Sprintf("failover/liveness/%s%s", failureMode, suffix),
+				Owner:               registry.OwnerKV,
+				Benchmark:           true,
+				Timeout:             30 * time.Minute,
+				SkipPostValidations: postValidation,
+				Cluster:             makeSpec(5 /* nodes */, 4 /* cpus */),
 
 				Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 					runFailoverLiveness(ctx, t, c, failureMode, expirationLeases)
 				},
 			})
 			r.Add(registry.TestSpec{
-				Name:      fmt.Sprintf("failover/system-non-liveness/%s%s", failureMode, suffix),
-				Owner:     registry.OwnerKV,
-				Benchmark: true,
-				Timeout:   30 * time.Minute,
-				Cluster:   makeSpec(7 /* nodes */, 4 /* cpus */),
+				Name:                fmt.Sprintf("failover/system-non-liveness/%s%s", failureMode, suffix),
+				Owner:               registry.OwnerKV,
+				Benchmark:           true,
+				Timeout:             30 * time.Minute,
+				SkipPostValidations: postValidation,
+				Cluster:             makeSpec(7 /* nodes */, 4 /* cpus */),
 
 				Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 					runFailoverSystemNonLiveness(ctx, t, c, failureMode, expirationLeases)

diff --git a/pkg/cmd/roachtest/tests/kv.go b/pkg/cmd/roachtest/tests/kv.go
@@ -390,10 +390,11 @@ func registerKVContention(r registry.Registry) {
 
 func registerKVQuiescenceDead(r registry.Registry) {
 	r.Add(registry.TestSpec{
-		Name:    "kv/quiescence/nodes=3",
-		Owner:   registry.OwnerKV,
-		Cluster: r.MakeClusterSpec(4),
-		Leases:  registry.MetamorphicLeases,
+		Name:                "kv/quiescence/nodes=3",
+		Owner:               registry.OwnerKV,
+		Cluster:             r.MakeClusterSpec(4),
+		SkipPostValidations: registry.PostValidationNoDeadNodes,
+		Leases:              registry.MetamorphicLeases,
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			nodes := c.Spec().NodeCount - 1
 			c.Put(ctx, t.Cockroach(), "./cockroach", c.Range(1, nodes))

diff --git a/pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go b/pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go
@@ -75,7 +75,7 @@ func registerLOQRecovery(r registry.Registry) {
 			Tags:                []string{`default`},
 			Cluster:             spec,
 			Leases:              registry.MetamorphicLeases,
-			SkipPostValidations: registry.PostValidationInvalidDescriptors,
+			SkipPostValidations: registry.PostValidationInvalidDescriptors | registry.PostValidationNoDeadNodes,
 			NonReleaseBlocker:   true,
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 				runRecoverLossOfQuorum(ctx, t, c, testSpec)
@@ -88,7 +88,7 @@ func registerLOQRecovery(r registry.Registry) {
 			Tags:                []string{`default`},
 			Cluster:             spec,
 			Leases:              registry.MetamorphicLeases,
-			SkipPostValidations: registry.PostValidationInvalidDescriptors,
+			SkipPostValidations: registry.PostValidationInvalidDescriptors | registry.PostValidationNoDeadNodes,
 			NonReleaseBlocker:   true,
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 				runHalfOnlineRecoverLossOfQuorum(ctx, t, c, testSpec)