Merge pull request #110604 from cockroachdb/blathers/backport-release…

…-23.1-110406

release-23.1: roachtest: fix NPE in test_runner

pkg/cmd/roachtest/cluster.go

@@ -893,6 +893,10 @@ func (f *clusterFactory) newCluster(
 		}
 		return c, nil, nil
 	}
+	// Ensure an allocation is specified.
+	if cfg.alloc == nil {
+		return nil, nil, errors.New("no allocation specified; cfg.alloc must be set")
+	}
 
 	if cfg.localCluster {
 		// Local clusters never expire.
@@ -1097,7 +1101,7 @@ func (c *clusterImpl) StopCockroachGracefullyOnNode(
 func (c *clusterImpl) Save(ctx context.Context, msg string, l *logger.Logger) {
 	l.PrintfCtx(ctx, "saving cluster %s for debugging (--debug specified)", c)
 	// TODO(andrei): should we extend the cluster here? For how long?
-	if c.destroyState.owned { // we won't have an alloc for an unowned cluster
+	if c.destroyState.owned && c.destroyState.alloc != nil { // we won't have an alloc for an unowned cluster
 		c.destroyState.alloc.Freeze()
 	}
 	c.r.markClusterAsSaved(c, msg)

pkg/cmd/roachtest/test_runner.go

@@ -635,10 +635,19 @@ func (r *testRunner) runWorker(
 				// N.B. we do not count reuse attempt error toward clusterCreateErr.
 				// Let's attempt to create a fresh one.
 				testToRun.canReuseCluster = false
-			}
-			// sanity check
-			if c.spec.Cloud != spec.Local && c.spec.Arch != "" && c.arch != c.spec.Arch {
-				return errors.Newf("cluster arch %q does not match specified arch %q on cloud: %q", c.arch, c.spec.Arch, c.spec.Cloud)
+				// Destroy the cluster since we're unable to reuse it.
+				// NB: This is a hack. If we destroy the cluster, the allocation quota will get released back into the pool.
+				// Thus, we can't immediately create a fresh cluster since another worker might grab the quota before us.
+				// Instead, we transfer the allocation quota to the new cluster and pretend the old one didn't have any.
+				testToRun.alloc = c.destroyState.alloc
+				c.destroyState.alloc = nil
+				c.Destroy(context.Background(), closeLogger, l)
+				c = nil
+			} else {
+				// Reuse is possible, let's do a sanity check.
+				if c.spec.Cloud != spec.Local && c.spec.Arch != "" && c.arch != c.spec.Arch {
+					return errors.Newf("cluster arch %q does not match specified arch %q on cloud: %q", c.arch, c.spec.Arch, c.spec.Cloud)
+				}
 			}
 		}
 		arch := testToRun.spec.Cluster.Arch