Merge #151605

craig[bot] · DarrylWong · craig[bot] · commit 8ebacafe6fbf · 2025-08-12T03:18:46.000Z
151605: failureinjection: add disableStateValidation option to Failers r=shailendra-patel a=DarrylWong

Long running clusters such as DRT perform setup and cleanup in separate CLI calls where state cannot be maintained. This change adds an option to disable state validation.

Fixes: none
Epic: none
Release note: none

Co-authored-by: DarrylWong &lt;darryl@cockroachlabs.com&gt;
diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go
@@ -3341,11 +3341,12 @@ func (c *clusterImpl) GetFailer(
 	l *logger.Logger,
 	nodes option.NodeListOption,
 	failureModeName string,
+	disableStateValidation bool,
 	opts ...failures.ClusterOptionFunc,
 ) (*failures.Failer, error) {
 	fr := failures.GetFailureRegistry()
 	clusterOpts := append(opts, failures.Secure(c.IsSecure()), failures.LocalCertsPath(c.localCertsDir))
-	failer, err := fr.GetFailer(c.MakeNodes(nodes), failureModeName, l, clusterOpts...)
+	failer, err := fr.GetFailer(c.MakeNodes(nodes), failureModeName, l, disableStateValidation, clusterOpts...)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/cmd/roachtest/cluster/cluster_interface.go b/pkg/cmd/roachtest/cluster/cluster_interface.go
@@ -222,5 +222,5 @@ type Cluster interface {
 
 	RegisterClusterHook(hookName string, hookType option.ClusterHookType, timeout time.Duration, hook func(context.Context) error)
 
-	GetFailer(l *logger.Logger, nodes option.NodeListOption, failureModeName string, opts ...failures.ClusterOptionFunc) (*failures.Failer, error)
+	GetFailer(l *logger.Logger, nodes option.NodeListOption, failureModeName string, disableStateValidation bool, opts ...failures.ClusterOptionFunc) (*failures.Failer, error)
 }
diff --git a/pkg/cmd/roachtest/clusterstats/mock_cluster_generated_test.go b/pkg/cmd/roachtest/clusterstats/mock_cluster_generated_test.go
diff --git a/pkg/cmd/roachtest/operations/disk_stall.go b/pkg/cmd/roachtest/operations/disk_stall.go
@@ -49,7 +49,9 @@ func runDiskStall(
 
 	nodes := c.All()
 	nid := nodes[rng.Intn(len(nodes))]
-	ds := roachtestutil.MakeDmsetupDiskStaller(o, c)
+	// Disable state validation since we run dmsetup Setup() during cluster creation and not part
+	// of the operation.
+	ds := roachtestutil.MakeDmsetupDiskStaller(o, c, true /* disableStateValidation */)
 
 	o.Status(fmt.Sprintf("stalling disk on node %d", nid))
 	ds.Stall(ctx, c.Node(nid))
diff --git a/pkg/cmd/roachtest/roachtestutil/disk_stall.go b/pkg/cmd/roachtest/roachtestutil/disk_stall.go
@@ -64,9 +64,9 @@ type cgroupDiskStaller struct {
 var _ DiskStaller = (*cgroupDiskStaller)(nil)
 
 func MakeCgroupDiskStaller(
-	f Fataler, c cluster.Cluster, stallReads bool, stallLogs bool,
+	f Fataler, c cluster.Cluster, stallReads bool, stallLogs bool, disableStateValidation bool,
 ) DiskStaller {
-	diskStaller, err := c.GetFailer(f.L(), c.CRDBNodes(), failures.CgroupsDiskStallName)
+	diskStaller, err := c.GetFailer(f.L(), c.CRDBNodes(), failures.CgroupsDiskStallName, disableStateValidation)
 	if err != nil {
 		f.Fatalf("failed to get failer: %s", err)
 	}
@@ -158,8 +158,8 @@ type dmsetupDiskStaller struct {
 
 var _ DiskStaller = (*dmsetupDiskStaller)(nil)
 
-func MakeDmsetupDiskStaller(f Fataler, c cluster.Cluster) DiskStaller {
-	diskStaller, err := c.GetFailer(f.L(), c.CRDBNodes(), failures.DmsetupDiskStallName)
+func MakeDmsetupDiskStaller(f Fataler, c cluster.Cluster, disableStateValidation bool) DiskStaller {
+	diskStaller, err := c.GetFailer(f.L(), c.CRDBNodes(), failures.DmsetupDiskStallName, disableStateValidation)
 	if err != nil {
 		f.Fatalf("failed to get failer: %s", err)
 	}
diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/mutators.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/mutators.go
@@ -515,7 +515,7 @@ func (m networkPartitionMutator) Generate(
 	nodeList := planner.currentContext.System.Descriptor.Nodes
 
 	failure := failures.GetFailureRegistry()
-	f, err := failure.GetFailer(planner.cluster.Name(), failures.IPTablesNetworkPartitionName, planner.logger)
+	f, err := failure.GetFailer(planner.cluster.Name(), failures.IPTablesNetworkPartitionName, planner.logger, false)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get failer for %s: %w", failures.IPTablesNetworkPartitionName, err)
 	}
diff --git a/pkg/cmd/roachtest/roachtestutil/utils.go b/pkg/cmd/roachtest/roachtestutil/utils.go
@@ -298,7 +298,7 @@ func SimulateMultiRegionCluster(
 	regionToNodeMap failures.RegionToNodes,
 	l *logger.Logger,
 ) (func(), error) {
-	latencyFailer, err := c.GetFailer(l, c.All(), failures.NetworkLatencyName)
+	latencyFailer, err := c.GetFailer(l, c.All(), failures.NetworkLatencyName, false /* disableStateValidation */)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/cmd/roachtest/tests/admission_control_disk_bandwidth_overload.go b/pkg/cmd/roachtest/tests/admission_control_disk_bandwidth_overload.go
@@ -72,7 +72,7 @@ func registerDiskBandwidthOverload(r registry.Registry) {
 			const provisionedBandwidth = 128 << 20 // 128 MiB
 			t.Status(fmt.Sprintf("limiting disk bandwidth to %d bytes/s", provisionedBandwidth))
 			staller := roachtestutil.MakeCgroupDiskStaller(t, c,
-				false /* readsToo */, false /* logsToo */)
+				false /* readsToo */, false /* logsToo */, false /* disableStateValidation */)
 			staller.Setup(ctx)
 			staller.Slow(ctx, c.CRDBNodes(), provisionedBandwidth)
 
diff --git a/pkg/cmd/roachtest/tests/admission_control_elastic_mixed_version.go b/pkg/cmd/roachtest/tests/admission_control_elastic_mixed_version.go
@@ -66,7 +66,7 @@ func registerElasticWorkloadMixedVersion(r registry.Registry) {
 			setDiskBandwidth := func() {
 				t.Status(fmt.Sprintf("limiting disk bandwidth to %d bytes/s", diskBand))
 				staller := roachtestutil.MakeCgroupDiskStaller(t, c,
-					false /* readsToo */, false /* logsToo */)
+					false /* readsToo */, false /* logsToo */, false /* disableStateValidation */)
 				staller.Setup(ctx)
 				staller.Slow(ctx, c.CRDBNodes(), diskBand)
 			}
diff --git a/pkg/cmd/roachtest/tests/admission_control_snapshot_overload_io.go b/pkg/cmd/roachtest/tests/admission_control_snapshot_overload_io.go
@@ -175,7 +175,7 @@ func runAdmissionControlSnapshotOverloadIO(
 		const bandwidthLimit = 128 << 20 // 128 MiB
 		t.Status(fmt.Sprintf("limiting disk bandwidth to %d bytes/s", bandwidthLimit))
 		staller := roachtestutil.MakeCgroupDiskStaller(t, c,
-			false /* readsToo */, false /* logsToo */)
+			false /* readsToo */, false /* logsToo */, false /* disableStateValidation */)
 		staller.Setup(ctx)
 		staller.Slow(ctx, c.CRDBNodes(), bandwidthLimit)
 
diff --git a/pkg/cmd/roachtest/tests/disk_stall.go b/pkg/cmd/roachtest/tests/disk_stall.go
@@ -64,7 +64,7 @@ func runDiskStalledWALFailover(ctx context.Context, t test.Test, c cluster.Clust
 		fmt.Sprintf("COCKROACH_ENGINE_MAX_SYNC_DURATION_DEFAULT=%s", maxSyncDur))
 
 	t.Status("setting up disk staller")
-	s := roachtestutil.MakeDmsetupDiskStaller(t, c)
+	s := roachtestutil.MakeDmsetupDiskStaller(t, c, false)
 	s.Setup(ctx)
 	defer s.Cleanup(ctx)
 
@@ -189,15 +189,17 @@ func runDiskStalledWALFailover(ctx context.Context, t test.Test, c cluster.Clust
 // appropriately.
 func registerDiskStalledDetection(r registry.Registry) {
 	stallers := map[string]func(test.Test, cluster.Cluster) diskStaller{
-		"dmsetup": func(t test.Test, c cluster.Cluster) diskStaller { return roachtestutil.MakeDmsetupDiskStaller(t, c) },
+		"dmsetup": func(t test.Test, c cluster.Cluster) diskStaller {
+			return roachtestutil.MakeDmsetupDiskStaller(t, c, false)
+		},
 		"cgroup/read-write/logs-too=false": func(t test.Test, c cluster.Cluster) diskStaller {
-			return roachtestutil.MakeCgroupDiskStaller(t, c, true, false)
+			return roachtestutil.MakeCgroupDiskStaller(t, c, true, false, false)
 		},
 		"cgroup/read-write/logs-too=true": func(t test.Test, c cluster.Cluster) diskStaller {
-			return roachtestutil.MakeCgroupDiskStaller(t, c, true, true)
+			return roachtestutil.MakeCgroupDiskStaller(t, c, true, true, false)
 		},
 		"cgroup/write-only/logs-too=true": func(t test.Test, c cluster.Cluster) diskStaller {
-			return roachtestutil.MakeCgroupDiskStaller(t, c, false, true)
+			return roachtestutil.MakeCgroupDiskStaller(t, c, false, true, false)
 		},
 	}
 
@@ -466,7 +468,7 @@ func runDiskStalledWALFailoverWithProgress(ctx context.Context, t test.Test, c c
 
 	t.Status("setting up disk staller")
 	// Use CgroupDiskStaller with readsToo=false to only stall writes.
-	s := roachtestutil.MakeCgroupDiskStaller(t, c, false /* readsToo */, false /* logsToo */)
+	s := roachtestutil.MakeCgroupDiskStaller(t, c, false /* readsToo */, false /* logsToo */, false)
 	s.Setup(ctx)
 	// NB: We use a background context in the defer'ed cleanup command,
 	// otherwise on test failure our c.Run calls will be ignored. Leaving
diff --git a/pkg/cmd/roachtest/tests/failover.go b/pkg/cmd/roachtest/tests/failover.go
@@ -1236,7 +1236,7 @@ func makeFailerWithoutLocalNoop(
 			c:             c,
 			m:             m,
 			startSettings: settings,
-			staller:       roachtestutil.MakeDmsetupDiskStaller(t, c),
+			staller:       roachtestutil.MakeDmsetupDiskStaller(t, c, false),
 		}
 	case failureModePause:
 		return &pauseFailer{
diff --git a/pkg/cmd/roachtest/tests/failure_injection.go b/pkg/cmd/roachtest/tests/failure_injection.go
@@ -65,7 +65,7 @@ type failureSmokeTest struct {
 func (t *failureSmokeTest) run(
 	ctx context.Context, l *logger.Logger, c cluster.Cluster,
 ) (err error) {
-	failer, err := c.GetFailer(l, c.CRDBNodes(), t.failureName, failures.ReplicationFactor(3))
+	failer, err := c.GetFailer(l, c.CRDBNodes(), t.failureName, false /* disableStateValidation */, failures.ReplicationFactor(3))
 	if err != nil {
 		return err
 	}
@@ -166,7 +166,7 @@ func (t *failureSmokeTest) run(
 }
 
 func (t *failureSmokeTest) noopRun(ctx context.Context, l *logger.Logger, c cluster.Cluster) error {
-	failer, err := c.GetFailer(l, c.CRDBNodes(), t.failureName)
+	failer, err := c.GetFailer(l, c.CRDBNodes(), t.failureName, false /* disableStateValidation */)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/cmd/roachtest/tests/perturbation/slow_disk.go b/pkg/cmd/roachtest/tests/perturbation/slow_disk.go
@@ -66,7 +66,7 @@ func (s *slowDisk) startTargetNode(ctx context.Context, t test.Test, v variation
 	if v.IsLocal() {
 		s.staller = roachtestutil.NoopDiskStaller{}
 	} else {
-		s.staller = roachtestutil.MakeCgroupDiskStaller(t, v, false /* readsToo */, false /* logsToo */)
+		s.staller = roachtestutil.MakeCgroupDiskStaller(t, v, false /* readsToo */, false /* logsToo */, false)
 		s.staller.Setup(ctx)
 	}
 }
diff --git a/pkg/roachprod/failureinjection/failures/failer.go b/pkg/roachprod/failureinjection/failures/failer.go
@@ -95,11 +95,18 @@ type Failer struct {
 	// on all nodes in the cluster, but then randomly inject the failure on a
 	// subset of nodes at a time.
 	injectArgs FailureArgs
+	// disableStateValidation skips the state validation checks in the Failer.
+	// This is needed for long running clusters (i.e. DRT) where the setup and
+	// cleanup occurs as separate processes and we can't maintain a state.
+	disableStateValidation bool
 }
 
 // checkValidTransition is a helper that returns a generic error message if
 // the current state of the Failer is not in one of the valid states passed.
 func (f *Failer) checkValidTransition(validStates ...failureModeStage) error {
+	if f.disableStateValidation {
+		return nil
+	}
 	validStateMap := make(map[failureModeStage]struct{}, len(validStates))
 	for _, state := range validStates {
 		validStateMap[state] = struct{}{}
diff --git a/pkg/roachprod/failureinjection/failures/failer_test.go b/pkg/roachprod/failureinjection/failures/failer_test.go
@@ -145,7 +145,7 @@ func runTransitionStep(
 
 func Test_FailerLifecycle(t *testing.T) {
 	fr := GetFailureRegistry()
-	f, err := fr.GetFailer("", NoopFailureName, nilLogger(), Secure(false))
+	f, err := fr.GetFailer("", NoopFailureName, nilLogger(), false /* disableValidation */, Secure(false))
 	require.NoError(t, err)
 
 	rng, _ := randutil.NewTestRand()
diff --git a/pkg/roachprod/failureinjection/failures/registry.go b/pkg/roachprod/failureinjection/failures/registry.go
@@ -94,7 +94,10 @@ func (r *FailureRegistry) List(regex string) []string {
 }
 
 func (r *FailureRegistry) GetFailer(
-	clusterName, failureName string, l *logger.Logger, opts ...ClusterOptionFunc,
+	clusterName, failureName string,
+	l *logger.Logger,
+	disableStateValidation bool,
+	opts ...ClusterOptionFunc,
 ) (*Failer, error) {
 	r.Lock()
 	spec, ok := r.failures[failureName]
@@ -113,8 +116,9 @@ func (r *FailureRegistry) GetFailer(
 	}
 
 	failer := &Failer{
-		FailureMode: failureMode,
-		state:       uninitialized,
+		FailureMode:            failureMode,
+		disableStateValidation: disableStateValidation,
+		state:                  uninitialized,
 	}
 	return failer, nil
 }

Original file line number	Diff line number	Diff line change
`@@ -222,5 +222,5 @@ type Cluster interface {`
`222`	`222`
`223`	`223`	`RegisterClusterHook(hookName string, hookType option.ClusterHookType, timeout time.Duration, hook func(context.Context) error)`
`224`	`224`
`225`		`- GetFailer(l logger.Logger, nodes option.NodeListOption, failureModeName string, opts ...failures.ClusterOptionFunc) (failures.Failer, error)`
	`225`	`+ GetFailer(l logger.Logger, nodes option.NodeListOption, failureModeName string, disableStateValidation bool, opts ...failures.ClusterOptionFunc) (failures.Failer, error)`
`226`	`226`	`}`
Original file line number	Diff line number	Diff line change
`@@ -515,7 +515,7 @@ func (m networkPartitionMutator) Generate(`
`515`	`515`	`nodeList := planner.currentContext.System.Descriptor.Nodes`
`516`	`516`
`517`	`517`	`failure := failures.GetFailureRegistry()`
`518`		`- f, err := failure.GetFailer(planner.cluster.Name(), failures.IPTablesNetworkPartitionName, planner.logger)`
	`518`	`+ f, err := failure.GetFailer(planner.cluster.Name(), failures.IPTablesNetworkPartitionName, planner.logger, false)`
`519`	`519`	`if err != nil {`
`520`	`520`	`return nil, fmt.Errorf("failed to get failer for %s: %w", failures.IPTablesNetworkPartitionName, err)`
`521`	`521`	`}`
Original file line number	Diff line number	Diff line change
`@@ -298,7 +298,7 @@ func SimulateMultiRegionCluster(`
`298`	`298`	`regionToNodeMap failures.RegionToNodes,`
`299`	`299`	`l *logger.Logger,`
`300`	`300`	`) (func(), error) {`
`301`		`- latencyFailer, err := c.GetFailer(l, c.All(), failures.NetworkLatencyName)`
	`301`	`+ latencyFailer, err := c.GetFailer(l, c.All(), failures.NetworkLatencyName, false /* disableStateValidation */)`
`302`	`302`	`if err != nil {`
`303`	`303`	`return nil, err`
`304`	`304`	`}`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ func registerElasticWorkloadMixedVersion(r registry.Registry) {`
`66`	`66`	`setDiskBandwidth := func() {`
`67`	`67`	`t.Status(fmt.Sprintf("limiting disk bandwidth to %d bytes/s", diskBand))`
`68`	`68`	`staller := roachtestutil.MakeCgroupDiskStaller(t, c,`
`69`		`- false /* readsToo /, false / logsToo */)`
	`69`	`+ false /* readsToo /, false / logsToo /, false / disableStateValidation */)`
`70`	`70`	`staller.Setup(ctx)`
`71`	`71`	`staller.Slow(ctx, c.CRDBNodes(), diskBand)`
`72`	`72`	`}`