Merge #155262

155262: roachtest: fix maybeSaveClusterDueToInvariantProblems r=herkolategan a=DarrylWong

This check previously called c.Nodes, which constructs a node list with the passed in options. With nothing passed, this constructs an empty node list which caused this check to always return.

Fixes: none
Epic: none
Release note: none

---------

Was experimenting with taking disk snapshots and struggling to get it to work. Cross referenced with this and realized its been silently skipping the check 😞 

Co-authored-by: DarrylWong <[email protected]>

Author: craig[bot]

pkg/cmd/roachtest/test_runner.go

@@ -1814,7 +1814,7 @@ func gatherFatalNodeLogs(t *testImpl, testLogger *logger.Logger) (string, error)
 func (r *testRunner) maybeSaveClusterDueToInvariantProblems(
 	ctx context.Context, t *testImpl, c *clusterImpl,
 ) {
-	if len(c.Nodes()) == 0 {
+	if len(c.All()) == 0 {
 		return // test only
 	}
 	dets, err := c.RunWithDetails(ctx, t.L(), option.WithNodes(c.All()),
@@ -1834,8 +1834,11 @@ func (r *testRunner) maybeSaveClusterDueToInvariantProblems(
 	for _, det := range dets {
 		if det.Stdout != "" {
 			_ = c.Extend(ctx, 7*24*time.Hour, t.L())
-			timestamp := timeutil.Now().Format("20060102_150405")
-			snapName := fmt.Sprintf("invariant-problem-%s-%s", c.Name(), timestamp)
+			timestamp := timeutil.Now().UnixMilli()
+			// We take the risk that two tests could attempt to create a snapshot
+			// at the same exact millisecond, as we have a 63 character limit on
+			// the name and the cluster name usually exceeds this by itself.
+			snapName := fmt.Sprintf("invariant-problem-%d", timestamp)
 			if _, err := c.CreateSnapshot(ctx, snapName); err != nil {
 				t.L().Printf("failed to create snapshot %q: %s", snapName, err)
 				snapName = "<failed>"

pkg/cmd/roachtest/tests/invariant_check_detection.go

@@ -30,7 +30,7 @@ func registerInvariantCheckDetection(r registry.Registry) {
 			Name:             fmt.Sprintf("invariant-check-detection/failed=%t", failed),
 			Owner:            registry.OwnerTestEng,
 			Suites:           registry.ManualOnly,
-			Cluster:          spec.ClusterSpec{NodeCount: 1, CPUs: 4, ReusePolicy: spec.ReusePolicyNone{}},
+			Cluster:          r.MakeClusterSpec(1, spec.CPU(4), spec.ReuseNone(), spec.VolumeSize(100), spec.GCEVolumeType("pd-ssd")),
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 				runInvariantCheckDetection(ctx, t, c, failed)
 			},

pkg/roachprod/vm/gce/gcloud.go

@@ -874,6 +874,16 @@ func (p *Provider) ListVolumes(l *logger.Logger, v *vm.VM) ([]vm.Volume, error)
 		}
 	}
 
+	// Unfortunately the above responses contain no common identifier to join
+	// on, so we must resort to indexing. This does not work if the number of
+	// attached disks is different from the number of described volumes, i.e.
+	// if the cluster is using local SSDs.
+	if len(attachedDisks) != len(describedVolumes) {
+		err := errors.Newf("expected to find the same number of attached disks (%d) as described volumes (%d)",
+			len(attachedDisks), len(describedVolumes))
+		return nil, errors.WithHint(err, "is the cluster using local SSD?")
+	}
+
 	var volumes []vm.Volume
 	for idx := range attachedDisks {
 		attachedDisk := attachedDisks[idx]