mixedversion: fix panic in refresh{Cluster,Binary}Version

A recent change #155713 fixed the indexing when nodes were
unavailable. However, this change did not account for multi cluster
tests where the roachprod node ID is not the same as the CRDB
node ID, i.e. we can't assume the 4th VM is the same as the
4th node in the cluster.

Fixes: none
Epic: none
Release note: none

Author: DarrylWong

pkg/cmd/roachtest/option/node_list_option.go

@@ -42,6 +42,15 @@ func (n NodeListOption) Equals(o NodeListOption) bool {
 	return true
 }
 
+func (n NodeListOption) Contains(node int) bool {
+	for _, n := range n {
+		if n == node {
+			return true
+		}
+	}
+	return false
+}
+
 // Option implements Option.
 func (n NodeListOption) Option() {}
 

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

@@ -525,16 +525,26 @@ func (tr *testRunner) refreshBinaryVersions(ctx context.Context, service *servic
 	defer cancel()
 
 	group := ctxgroup.WithContext(connectionCtx)
-	for _, node := range tr.getAvailableNodes(service.descriptor) {
+	// We still attempt to refresh binary versions on nodes we expect to be unavailable:
+	// 	1. Some failure injections are overly conservative in marking nodes as
+	//	   unavailable out of caution (e.g. network partitions).
+	//	2. The monitor returns the roachprod node ID, which may be offset compared
+	//		 to what the mixed-version test expects, e.g. a multi cluster test where
+	//		 node 1 may be the 5th VM in the roachprod cluster.
+	availableNodes := tr.getAvailableNodes(service.descriptor)
+	for j, node := range service.descriptor.Nodes {
 		group.GoCtx(func(ctx context.Context) error {
 			bv, err := clusterupgrade.BinaryVersion(ctx, tr.conn(node, service.descriptor.Name))
 			if err != nil {
+				if !availableNodes.Contains(node) {
+					return nil
+				}
 				return fmt.Errorf(
 					"failed to get binary version for node %d (%s): %w",
 					node, service.descriptor.Name, err,
 				)
 			}
-			newBinaryVersions[node-1] = bv
+			newBinaryVersions[j] = bv
 			return nil
 		})
 	}
@@ -556,17 +566,28 @@ func (tr *testRunner) refreshClusterVersions(ctx context.Context, service *servi
 	defer cancel()
 
 	group := ctxgroup.WithContext(connectionCtx)
-	for _, node := range tr.getAvailableNodes(service.descriptor) {
+	// We still attempt to refresh cluster versions on nodes we expect to be unavailable:
+	// 	1. Some failure injections are overly conservative in marking nodes as
+	//	   unavailable out of caution (e.g. network partitions).
+	//	2. The monitor returns the roachprod node ID, which may be offset compared
+	//		 to what the mixed-version test expects, e.g. a multi cluster test where
+	//		 node 1 may be the 5th VM in the roachprod cluster.
+	availableNodes := tr.getAvailableNodes(service.descriptor)
+	for j, node := range service.descriptor.Nodes {
 		group.GoCtx(func(ctx context.Context) error {
 			cv, err := clusterupgrade.ClusterVersion(ctx, tr.conn(node, service.descriptor.Name))
 			if err != nil {
+				if !availableNodes.Contains(node) {
+					return nil
+				}
+
 				return fmt.Errorf(
 					"failed to get cluster version for node %d (%s): %w",
 					node, service.descriptor.Name, err,
 				)
 			}
 
-			newClusterVersions[node-1] = cv
+			newClusterVersions[j] = cv
 			return nil
 		})
 	}