roachtest/mixedversion: sync workload binary versions with the cluster version

The current workload binary is no longer backwards compatible for certain
workloads. Previously, the workload binary would just use whatever default
binary was staged on the workload node. Currently, there's a quick fix that
installs the current system version's binary on the workload node when a
workload cmd is being executed.

This change bakes that behavior into the test framework itself, so we can remove
the binary staging from the Workload hook. Workload nodes are staged with all
versioned binaries in the upgrade plan so all workload commands can use a
versioned binary that matches the current state of the cluster.

Resolves #147374

Epic: None
Release note: None

Author: williamchoe3

pkg/BUILD.bazel

@@ -1266,6 +1266,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/mock_cluster_generated_test.go renamed to pkg/cmd/roachtest/cluster/mock/mock_cluster_generated.go

@@ -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/BUILD.bazel

@@ -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/clusterstats/exporter_test.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/clusterupgrade/clusterupgrade.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.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/commandbuilder_test.go

@@ -389,6 +389,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/README.md

@@ -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