Merge #147641 #148385

147641: roachtest: add mutator to panic and restart a node r=DarrylWong a=Dev-Kyle

This change adds a new mutator that will enable the ability to randomly panic a node. Currently the node is immediately restarted upon crashing, with the end goal of being able to panic a node and waiting some number of steps before restarting it.

Fixes: None
Epic: None
Release note: None
Team City: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Nightlies_RoachtestNightlyGceBazel?branch=147641&buildTypeTab=overview&mode=builds&hideProblemsFromDependencies=false&hideTestsFromDependencies=false

148385: rpc: add DRPC dial methods r=cthumuluru-crdb a=cthumuluru-crdb

Refactor `Peer` further to make it easy to support both gRPC and DRPC peers. In this change, gRPC peer continues to dial DRPC connections to keep the changes backward compatible. As part of enabling DRPC support across the codebase, future commits will remove the DRPC-specific code and replace it with generic peer.

This commit also adds DRPC dial methods to `rpc.nodedialer` and `rpc.Context`. In the future commits, helper functions for creation RPC clients are update to create gRPC or DRPC clients base on `rpc.experimental_drpc.enabled` cluster setting.

Epic: CRDB-48923 
Fixes: #148383
Release note: none

Co-authored-by: Kyle <[email protected]>
Co-authored-by: Chandra Thumuluru <[email protected]>

Author: 3 people

pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go

@@ -295,6 +295,11 @@ type (
 		// Run implements the actual functionality of the step. This
 		// signature should remain in sync with `stepFunc`.
 		Run(context.Context, *logger.Logger, *rand.Rand, *Helper) error
+		// ConcurrencyDisabled returns true if the step should not be run
+		// concurrently with other steps. This is the case for any steps
+		// that involve restarting a node, as they may attempt to connect
+		// to an unavailable node.
+		ConcurrencyDisabled() bool
 	}
 
 	// singleStep represents steps that implement the pieces on top of

pkg/cmd/roachtest/roachtestutil/mixedversion/mutators.go

@@ -53,7 +53,7 @@ func (m preserveDowngradeOptionRandomizerMutator) Probability() float64 {
 // current version is always mutated. The length of the returned
 // mutations is always even.
 func (m preserveDowngradeOptionRandomizerMutator) Generate(
-	rng *rand.Rand, plan *TestPlan,
+	rng *rand.Rand, plan *TestPlan, planner *testPlanner,
 ) []mutation {
 	var mutations []mutation
 	for _, upgradeSelector := range randomUpgrades(rng, plan) {
@@ -221,7 +221,9 @@ func (m clusterSettingMutator) Probability() float64 {
 // original test plan. Up to `maxChanges` steps will be added to the
 // plan. Changes may be concurrent with user-provided steps and may
 // happen any time after cluster setup.
-func (m clusterSettingMutator) Generate(rng *rand.Rand, plan *TestPlan) []mutation {
+func (m clusterSettingMutator) Generate(
+	rng *rand.Rand, plan *TestPlan, planner *testPlanner,
+) []mutation {
 	var mutations []mutation
 
 	// possiblePointsInTime is the list of steps in the plan that are
@@ -245,13 +247,9 @@ func (m clusterSettingMutator) Generate(rng *rand.Rand, plan *TestPlan) []mutati
 					return false
 				}
 			}
-
-			// We skip restart steps as we might insert the cluster setting
-			// change step concurrently with the selected step.
-			_, isRestartSystem := s.impl.(restartWithNewBinaryStep)
-			_, isRestartTenant := s.impl.(restartVirtualClusterStep)
-			isRestart := isRestartSystem || isRestartTenant
-			return s.context.System.Stage >= OnStartupStage && !isRestart
+			// Cluster setting changes can be inserted concurrently, so we want to avoid
+			// inserting into any steps that cannot run concurrently with other steps.
+			return s.context.System.Stage >= OnStartupStage && !s.impl.ConcurrencyDisabled()
 		})
 
 	for _, changeStep := range m.changeSteps(rng, len(possiblePointsInTime)) {
@@ -383,3 +381,49 @@ func (m clusterSettingMutator) changeSteps(
 
 	return steps
 }
+
+const (
+	// PanicNode is a mutator that will randomly cause a node to crash
+	// during the test, before restarting the node within the same step.
+	PanicNode = "panic_node"
+)
+
+type panicNodeMutator struct {
+}
+
+func (m panicNodeMutator) Name() string {
+	return PanicNode
+}
+
+func (m panicNodeMutator) Probability() float64 {
+	return 0.3
+}
+
+func (m panicNodeMutator) Generate(
+	rng *rand.Rand, plan *TestPlan, planner *testPlanner,
+) []mutation {
+	var mutations []mutation
+	maxPanics := len(plan.upgrades)
+	numPanics := 1 + rng.Intn(maxPanics)
+
+	idx := newStepIndex(plan)
+	possiblePointsInTime := plan.
+		newStepSelector().
+		// We don't want to panic concurrently with other steps, and inserting before a concurrent step
+		// causes the step to run concurrently with that step, so we filter out any concurrent steps.
+		Filter(func(s *singleStep) bool {
+			return s.context.System.Stage >= InitUpgradeStage && !idx.IsConcurrent(s)
+		})
+
+	for range numPanics {
+		nodeList := planner.currentContext.System.Descriptor.Nodes
+		targetNode := nodeList.SeededRandNode(rng)
+		addRandomly := possiblePointsInTime.
+			RandomStep(rng).
+			InsertBefore(panicNodeStep{planner.currentContext.System.Descriptor.Nodes[0], targetNode, planner.rt})
+
+		mutations = append(mutations, addRandomly...)
+	}
+
+	return mutations
+}

pkg/cmd/roachtest/roachtestutil/mixedversion/mutators_test.go

@@ -29,7 +29,7 @@ func TestPreserveDowngradeOptionRandomizerMutator(t *testing.T) {
 	require.NoError(t, err)
 
 	var mut preserveDowngradeOptionRandomizerMutator
-	mutations := mut.Generate(newRand(), plan)
+	mutations := mut.Generate(newRand(), plan, nil)
 	require.NotEmpty(t, mutations)
 	require.True(t, len(mutations)%2 == 0, "should produce even number of mutations") // one removal and one insertion per upgrade
 
@@ -97,7 +97,7 @@ func TestClusterSettingMutator(t *testing.T) {
 
 		const settingName = "test_cluster_setting"
 		mut := newClusterSettingMutator(settingName, possibleValues, options...)
-		mutations := mut.Generate(newRand(), plan)
+		mutations := mut.Generate(newRand(), plan, nil)
 
 		// Number of mutations should be 1 <= n <= maxChanges
 		require.GreaterOrEqual(t, len(mutations), 1, "plan:\n%s", plan.PrettyPrint())

pkg/cmd/roachtest/roachtestutil/mixedversion/planner.go

@@ -127,9 +127,11 @@ type (
 		// this probability for specific mutator implementations as
 		// needed.
 		Probability() float64
-		// Generate takes a test plan and a RNG and returns the list of
-		// mutations that should be applied to the plan.
-		Generate(*rand.Rand, *TestPlan) []mutation
+		// Generate takes a test plan, the testPlanner, and an RNG and returns the list of
+		// mutations that should be applied to the plan. The test plan is the intended output
+		// after applying the mutations. The testPlanner is used to access specific attributes
+		// of the test plan, such as the current context or the services.
+		Generate(*rand.Rand, *TestPlan, *testPlanner) []mutation
 	}
 
 	// mutationOp encodes the type of mutation and controls how the
@@ -211,6 +213,10 @@ const (
 	spanConfigTenantLimit = 50000
 )
 
+var failureInjectionMutators = []mutator{
+	panicNodeMutator{},
+}
+
 // clusterSettingMutators includes a list of all
 // cluster setting mutator implementations.
 var clusterSettingMutators = []mutator{
@@ -250,9 +256,12 @@ var clusterSettingMutators = []mutator{
 // planMutators includes a list of all known `mutator`
 // implementations. A subset of these mutations might be enabled in
 // any mixedversion test plan.
-var planMutators = append([]mutator{
-	preserveDowngradeOptionRandomizerMutator{},
-},
+var planMutators = append(
+	append([]mutator{
+		preserveDowngradeOptionRandomizerMutator{},
+	},
+		failureInjectionMutators...,
+	),
 	clusterSettingMutators...,
 )
 
@@ -418,11 +427,22 @@ func (p *testPlanner) Plan() *TestPlan {
 		isLocal:        p.isLocal,
 	}
 
-	// Probabilistically enable some of of the mutators on the base test
-	// plan generated above.
+	failureInjections := make(map[string]struct{})
+	for _, m := range failureInjectionMutators {
+		failureInjections[m.Name()] = struct{}{}
+	}
+
+	// Probabilistically enable some of the mutators on the base test
+	// plan generated above. We disable any failure injections that
+	// would occur on clusters with less than three nodes as this
+	// can lead to uninteresting failures (e.g. a single node
+	// panic failure).
 	for _, mut := range planMutators {
 		if p.mutatorEnabled(mut) {
-			mutations := mut.Generate(p.prng, testPlan)
+			if _, found := failureInjections[mut.Name()]; found && len(p.currentContext.System.Descriptor.Nodes) < 3 {
+				continue
+			}
+			mutations := mut.Generate(p.prng, testPlan, p)
 			testPlan.applyMutations(p.prng, mutations)
 			testPlan.enabledMutators = append(testPlan.enabledMutators, mut)
 		}
@@ -1396,12 +1416,16 @@ func (ss stepSelector) InsertSequential(rng *rand.Rand, impl singleStepProtocol)
 	})
 }
 
+// InsertBefore inserts the step before the selected step. This is not guaranteed to be a non-concurrent insert, if the
+// selected step is part of a `concurrentRunStep`, the new step will be concurrent with the selected step.
 func (ss stepSelector) InsertBefore(impl singleStepProtocol) []mutation {
 	return ss.insert(impl, func() mutationOp {
 		return mutationInsertBefore
 	})
 }
 
+// InsertAfter inserts the step after the selected step. This is not guaranteed to be a non-concurrent insert, if the
+// selected step is part of a `concurrentRunStep`, the new step will be concurrent with the selected step.
 func (ss stepSelector) InsertAfter(impl singleStepProtocol) []mutation {
 	return ss.insert(impl, func() mutationOp {
 		return mutationInsertAfter

pkg/cmd/roachtest/roachtestutil/mixedversion/planner_test.go

@@ -553,7 +553,7 @@ func Test_stepSelectorFilter(t *testing.T) {
 			name:                   "no filter",
 			predicate:              func(*singleStep) bool { return true },
 			expectedAllSteps:       true,
-			expectedRandomStepType: runHookStep{},
+			expectedRandomStepType: restartWithNewBinaryStep{},
 		},
 		{
 			name: "filter eliminates all steps",
@@ -861,7 +861,9 @@ type concurrentUserHooksMutator struct{}
 func (concurrentUserHooksMutator) Name() string         { return "concurrent_user_hooks_mutator" }
 func (concurrentUserHooksMutator) Probability() float64 { return 0.5 }
 
-func (concurrentUserHooksMutator) Generate(rng *rand.Rand, plan *TestPlan) []mutation {
+func (concurrentUserHooksMutator) Generate(
+	rng *rand.Rand, plan *TestPlan, planner *testPlanner,
+) []mutation {
 	// Insert our `testSingleStep` implementation concurrently with every
 	// user-provided function.
 	return plan.
@@ -880,7 +882,9 @@ type removeUserHooksMutator struct{}
 func (removeUserHooksMutator) Name() string         { return "remove_user_hooks_mutator" }
 func (removeUserHooksMutator) Probability() float64 { return 0.5 }
 
-func (removeUserHooksMutator) Generate(rng *rand.Rand, plan *TestPlan) []mutation {
+func (removeUserHooksMutator) Generate(
+	rng *rand.Rand, plan *TestPlan, planner *testPlanner,
+) []mutation {
 	return plan.
 		newStepSelector().
 		Filter(func(s *singleStep) bool {

pkg/cmd/roachtest/roachtestutil/mixedversion/runner_test.go

@@ -178,6 +178,9 @@ func (*testSingleStep) Background() shouldStop { return nil }
 func (tss *testSingleStep) Run(_ context.Context, _ *logger.Logger, _ *rand.Rand, _ *Helper) error {
 	return tss.runFunc()
 }
+func (s testSingleStep) ConcurrencyDisabled() bool {
+	return false
+}
 
 func newTestStep(f func() error) *singleStep {
 	initialVersion := clusterupgrade.MustParseVersion(predecessorVersion)