release-25.4: roachtest/mixedversion: sync workload binary versions with the cluste…

pkg/BUILD.bazel

@@ -1271,6 +1271,7 @@ GO_TARGETS = [
     "//pkg/cmd/roachprod/grafana:grafana",
     "//pkg/cmd/roachprod:roachprod",
     "//pkg/cmd/roachprod:roachprod_lib",
+    "//pkg/cmd/roachtest/cluster/mock:mockcluster",
     "//pkg/cmd/roachtest/cluster:cluster",
     "//pkg/cmd/roachtest/clusterstats:clusterstats",
     "//pkg/cmd/roachtest/clusterstats:clusterstats_test",

pkg/cmd/roachtest/cluster/mock/BUILD.bazel

@@ -0,0 +1,40 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "gomock")
+
+gomock(
+    name = "mock_cluster",
+    testonly = True,
+    out = "mock_cluster_generated.go",
+    interfaces = ["Cluster"],
+    library = "//pkg/cmd/roachtest/cluster",
+    package = "mockcluster",
+    self_package = "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster/mock",
+    visibility = [
+        "//pkg/cmd/roachtest/clusterstats:__pkg__",
+        "//pkg/cmd/roachtest/roachtestutil/mixedversion:__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
+)
+
+go_library(
+    name = "mockcluster",
+    srcs = ["mock_cluster_generated.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster/mock",
+    visibility = [
+        "//pkg/cmd/roachtest/clusterstats:__pkg__",
+        "//pkg/cmd/roachtest/roachtestutil/mixedversion:__pkg__",
+        "//pkg/gen:__pkg__",
+    ],
+    deps = [
+        "//pkg/cmd/roachprod/grafana",
+        "//pkg/cmd/roachtest/cluster",
+        "//pkg/cmd/roachtest/option",
+        "//pkg/cmd/roachtest/spec",
+        "//pkg/roachprod",
+        "//pkg/roachprod/failureinjection/failures",
+        "//pkg/roachprod/install",
+        "//pkg/roachprod/logger",
+        "//pkg/roachprod/prometheus",
+        "//pkg/roachprod/vm",
+        "@com_github_golang_mock//gomock",
+    ],
+)

pkg/cmd/roachtest/clusterstats/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "//pkg/cmd/roachprod-microbench/util",
         # Required for generated mocks
         "//pkg/cmd/roachprod/grafana",  #keep
-        "//pkg/cmd/roachtest/cluster",
         "//pkg/cmd/roachtest/option",
         "//pkg/cmd/roachtest/test",
         # Required for generated mocks
@@ -36,6 +35,7 @@ go_library(
         "@com_github_prometheus_client_golang//api/prometheus/v1:prometheus",
         "@com_github_prometheus_common//model",
         "//pkg/cmd/roachtest/roachtestutil",
+        "//pkg/cmd/roachtest/cluster",
     ],
 )
 
@@ -45,15 +45,15 @@ go_test(
         "exporter_test.go",
         "streamer_test.go",
         ":mock_client",  # keep
-        ":mock_cluster",  # keep
         ":mock_test",  # keep
     ],
     embed = [":clusterstats"],
     embedsrcs = ["openmetrics_expected.txt"],
     deps = [
         "//pkg/cmd/roachtest/cluster",
+        "//pkg/cmd/roachtest/cluster/mock:mockcluster",  #keep
         "//pkg/cmd/roachtest/registry",
-        # Required for generated mocks
+        "//pkg/cmd/roachtest/roachtestutil",
         "//pkg/cmd/roachtest/roachtestutil/task",  #keep
         "//pkg/cmd/roachtest/spec",
         "//pkg/cmd/roachtest/test",
@@ -64,7 +64,6 @@ go_test(
         "@com_github_prometheus_client_golang//api/prometheus/v1:prometheus",
         "@com_github_prometheus_common//model",
         "@com_github_stretchr_testify//require",
-        "//pkg/cmd/roachtest/roachtestutil",
     ],
 )
 
@@ -91,15 +90,3 @@ gomock(
         "//pkg/gen:__pkg__",
     ],
 )
-
-gomock(
-    name = "mock_cluster",
-    out = "mock_cluster_generated_test.go",
-    interfaces = ["Cluster"],
-    library = "//pkg/cmd/roachtest/cluster",
-    package = "clusterstats",
-    visibility = [
-        ":__pkg__",
-        "//pkg/gen:__pkg__",
-    ],
-)

pkg/cmd/roachtest/clusterstats/exporter_test.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
+	mockcluster "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster/mock"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
@@ -328,7 +329,7 @@ func TestExport(t *testing.T) {
 			makePromQueryRange(barStat.Query, []string{"1", "2", "3"}, 5, barTS, false),
 		})
 		mockTest := getMockTest(t, ctrl, true, statsFileDest)
-		mockCluster := NewMockCluster(ctrl)
+		mockCluster := mockcluster.NewMockCluster(ctrl)
 		mockTest.EXPECT().Name().Return("mock_name")
 		mockTest.EXPECT().GetRunId().Return("mock_id").AnyTimes()
 		mockCluster.EXPECT().Cloud().Times(1).Return(spec.GCE)
@@ -376,7 +377,7 @@ func TestExport(t *testing.T) {
 			makePromQueryRange(barStat.Query, []string{"1", "2", "3"}, 5, barTS, false),
 		})
 		mockTest := getMockTest(t, ctrl, false, statsFileDest)
-		mockCluster := NewMockCluster(ctrl)
+		mockCluster := mockcluster.NewMockCluster(ctrl)
 		statsWriter = func(ctx context.Context, tt test.Test, c cluster.Cluster, buffer *bytes.Buffer, dest string) error {
 			require.Equal(t, mockTest, tt)
 			require.Equal(t, mockCluster, c)

pkg/cmd/roachtest/roachtestutil/clusterupgrade/clusterupgrade.go

@@ -228,7 +228,6 @@ func UploadWorkload(
 	default:
 		minWorkloadBinaryVersion = MustParseVersion("v22.2.0")
 	}
-
 	// If we are uploading the `current` version, skip version checking,
 	// as the binary used is the one passed via command line flags.
 	if !v.IsCurrent() && !v.AtLeast(minWorkloadBinaryVersion) {
@@ -239,9 +238,15 @@ func UploadWorkload(
 	return path, err == nil, err
 }
 
-// uploadBinaryVersion uploads the specified binary associated with
-// the given version to the given nodes. It returns the path of the
-// uploaded binaries on the nodes.
+// uploadBinaryVersion attempts to upload the specified binary associated with
+// the given version to the given nodes. If the destination binary path already
+// exists, assume the binary has already been uploaded previously. Returns the
+// path of the uploaded binaries on the nodes.
+//
+// If cockroach is the target binary and if --versions-binary-override option
+// is set and if version v is contained in the override map, use that version's
+// value, which is a local binary path as the source binary to upload instead
+// of using roachprod to stage.
 func uploadBinaryVersion(
 	ctx context.Context,
 	t test.Test,
@@ -256,6 +261,8 @@ func uploadBinaryVersion(
 	var isOverridden bool
 	switch binary {
 	case "cockroach":
+		// If the --versions-binary-override option is set and version v is in the
+		// argument map, then use that version's value as the path to the binary
 		defaultBinary, isOverridden = t.VersionsBinaryOverride()[v.String()]
 		if isOverridden {
 			l.Printf("using cockroach binary override for version %s: %s", v, defaultBinary)
@@ -305,7 +312,6 @@ func uploadBinaryVersion(
 			return "", err
 		}
 	}
-
 	return dstBinary, nil
 }
 

pkg/cmd/roachtest/roachtestutil/commandbuilder.go

@@ -19,6 +19,8 @@ type Command struct {
 	Arguments []string
 	Flags     map[string]*string
 	UseEquals bool
+	// EnvVars e.g. COCKROACH_RANDOM_SEED=%d ./workload ...
+	EnvVars map[string]*string
 }
 
 // NewCommand builds a command. The format parameter can take
@@ -35,6 +37,7 @@ func NewCommand(format string, args ...interface{}) *Command {
 		Binary:    parts[0],
 		Arguments: parts[1:],
 		Flags:     make(map[string]*string),
+		EnvVars:   make(map[string]*string),
 	}
 }
 
@@ -94,11 +97,17 @@ func (c *Command) ITEFlag(condition bool, name string, trueVal, falseVal interfa
 	return c.Flag(name, falseVal)
 }
 
+func (c *Command) EnvVar(name string, val interface{}) *Command {
+	c.EnvVars[name] = stringP(fmt.Sprint(val))
+	return c
+}
+
 // String returns a canonical string representation of the command
 // which can be passed to `cluster.Run`.
 func (c *Command) String() string {
 	flags := make([]string, 0, len(c.Flags))
 	names := make([]string, 0, len(c.Flags))
+	envVars := make([]string, 0, len(c.EnvVars))
 	for name := range c.Flags {
 		names = append(names, name)
 	}
@@ -124,10 +133,13 @@ func (c *Command) String() string {
 		flags = append(flags, strings.Join(parts, flagJoinSymbol))
 	}
 
-	cmd := append(
-		[]string{c.Binary},
-		append(c.Arguments, flags...)...,
-	)
+	for name, val := range c.EnvVars {
+		envVars = append(envVars, fmt.Sprintf("%s=%s", name, *val))
+	}
+
+	cmd := append(envVars,
+		append([]string{c.Binary},
+			append(c.Arguments, flags...)...)...)
 
 	return strings.Join(cmd, " ")
 }

pkg/cmd/roachtest/roachtestutil/commandbuilder_test.go

@@ -60,18 +60,27 @@ func TestCommand(t *testing.T) {
 	c.Option("x")
 	require.True(t, c.HasFlag("c"))
 	require.Equal(t, "./cockroach workload run bank {pgurl:1} -c 10 -n 8 -x", c.String())
+
+	c = clone(baseCommand)
+	c.EnvVar("COCKROACH_RANDOM_SEED", 12345)
+	require.Equal(t, "COCKROACH_RANDOM_SEED=12345 ./cockroach workload run bank {pgurl:1}", c.String())
 }
 
 func clone(cmd *Command) *Command {
 	flags := make(map[string]*string)
 	for k, v := range cmd.Flags {
 		flags[k] = v
 	}
+	envVars := make(map[string]*string)
+	for k, v := range cmd.EnvVars {
+		envVars[k] = v
+	}
 
 	return &Command{
 		Binary:    cmd.Binary,
 		Arguments: append([]string{}, cmd.Arguments...),
 		Flags:     flags,
 		UseEquals: cmd.UseEquals,
+		EnvVars:   envVars,
 	}
 }

pkg/cmd/roachtest/roachtestutil/mixedversion/README.md

@@ -359,6 +359,14 @@ mixedversion.DisableMutators(mixedversion.PreserveDowngradeOptionRandomizerMutat
 
 Use this option to disable specific [mutators](#mutators) that are incompatible with the test.
 
+``` go
+mixedversion.WithWorkloadNodes(c.WorkloadNode())
+```
+Certain workloads i.e. bank are no longer backwards compatible as of 25.3. Therefore a new best practice for using
+using workload during your test is to execute a workload command on a binary that matches the cluster version. Using
+this option tells the framework to handle staging all the binaries you need for your test on the workload node(s)
+during test setup.
+
 ### Deployment Modes
 
 By default, each run of a `mixedversion` test happens in one of 3 possible _deployment modes_: `system-only`, `shared-process`, and `separate-process`. In the latter two options, the framework will create a test tenant, and tests should exercise the feature they are testing by invoking it on the tenant.

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

@@ -355,6 +355,14 @@ func (h *Helper) IsSkipVersionUpgrade() bool {
 	return numReleases > 1
 }
 
+// VersionedCockroachPath returns the correct binary path to use when
+// executing workload commands in user-defined hooks that will match the
+// current cluster's version e.g., v25.3.1/cockroach
+func (h *Helper) VersionedCockroachPath(rt test.Test) string {
+	return clusterupgrade.BinaryPathForVersion(
+		rt, h.System.FromVersion, "cockroach")
+}
+
 // logSQL standardizes the logging when a SQL statement or query is
 // run using one of the Helper methods. It includes the node used as
 // gateway, along with the version currently running on it, for ease

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

@@ -350,6 +350,7 @@ type (
 		tag                            string
 		overriddenMutatorProbabilities map[string]float64
 		hooksSupportFailureInjection   bool
+		workloadNodes                  option.NodeListOption
 	}
 
 	CustomOption func(*testOptions)
@@ -578,6 +579,17 @@ func WithTag(tag string) CustomOption {
 	}
 }
 
+// WithWorkloadNodes tells the mixedversion framework that this test's cluster
+// includes workload node(s) so the framework can stage all the cockroach
+// binaries included in the upgrade plan on the workload node so the test can
+// use versioned workload commands without the test itself having to stage
+// those binaries.
+func WithWorkloadNodes(nodes option.NodeListOption) CustomOption {
+	return func(opts *testOptions) {
+		opts.workloadNodes = nodes
+	}
+}
+
 // supportsSkipUpgradeTo returns true if the given version supports skipping the
 // previous major version during upgrade. For example, 24.3 supports upgrade
 // directly from 24.1, but 25.1 only supports upgrade from 24.3.
@@ -836,47 +848,30 @@ func (t *Test) BackgroundCommand(
 // synchronously as a regular startup function. `runCmd` is the
 // command to actually run the command; it is run in the background.
 //
-// If overrideBinary is true, the binary used to run the command(s) will
-// be replaced with the cockroach binary of the current version the
-// cluster is running in.
-// TODO(testeng): Replace with https://github.com/cockroachdb/cockroach/issues/147374
+// By default, the binary used to run the command(s) will be the same as the
+// the current version of the cluster at the time this hook is executed.
+// We do this because we assume the binary is not backwards compatible.
 func (t *Test) Workload(
-	name string,
-	node option.NodeListOption,
-	initCmd, runCmd *roachtestutil.Command,
-	overrideBinary bool,
+	name string, node option.NodeListOption, initCmd, runCmd *roachtestutil.Command,
 ) StopFunc {
 	seed := uint64(t.prng.Int63())
 	addSeed := func(cmd *roachtestutil.Command) {
 		if !cmd.HasFlag("seed") {
 			cmd.Flag("seed", seed)
 		}
 	}
-
 	if initCmd != nil {
 		addSeed(initCmd)
 		t.OnStartup(fmt.Sprintf("initialize %s workload", name), func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *Helper) error {
-			if overrideBinary {
-				binary, err := clusterupgrade.UploadCockroach(ctx, t.rt, t.logger, t.cluster, node, h.System.FromVersion)
-				if err != nil {
-					t.rt.Fatal(err)
-				}
-				initCmd.Binary = binary
-			}
+			initCmd.Binary = h.VersionedCockroachPath(t.rt)
 			l.Printf("running command `%s` on nodes %v", initCmd.String(), node)
 			return t.cluster.RunE(ctx, option.WithNodes(node), initCmd.String())
 		})
 	}
 
 	addSeed(runCmd)
 	return t.BackgroundFunc(fmt.Sprintf("%s workload", name), func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *Helper) error {
-		if overrideBinary {
-			binary, err := clusterupgrade.UploadCockroach(ctx, t.rt, t.logger, t.cluster, node, h.System.FromVersion)
-			if err != nil {
-				t.rt.Fatal(err)
-			}
-			runCmd.Binary = binary
-		}
+		runCmd.Binary = h.VersionedCockroachPath(t.rt)
 		l.Printf("running command `%s` on nodes %v", runCmd.String(), node)
 		return t.cluster.RunE(ctx, option.WithNodes(node), runCmd.String())
 	})