server: rework TestClusterVersionBootstrapStrict

This test... I'm not entirely sure what it was supposed to test to be
honest, but it seemed to be more complicated than it needed to be. It
forced and emphasized MinSupportedVersion being equal to
BinaryServerVersion (which is generally not a thing). I've simplified
it, making it not muck with the versions, while keep (I think) the
things it was testing (to the extent that it was testing anything).

This test was also in my way because it created servers that pretended
to be versions that are not technically supported by the binary, and
this kind of funkiness is making my life hard as I'm trying to rework
the way in which versions are propagated and what knobs servers have,
etc.

Release note: None

pkg/server/version_cluster_test.go

@@ -17,6 +17,7 @@ package server_test
 import (
 	"context"
 	gosql "database/sql"
+	"fmt"
 	"path/filepath"
 	"strconv"
 	"sync/atomic"
@@ -312,44 +313,38 @@ func TestClusterVersionUpgrade(t *testing.T) {
 	}
 }
 
-func TestClusterVersionBootstrapStrict(t *testing.T) {
+// Test that, after cluster bootstrap, the different ways of getting the cluster
+// version all agree.
+func TestAllVersionsAgree(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	ctx := context.Background()
 
-	// Four nodes that are all strictly version X without accepting anything else.
-	for _, versions := range [][][2]string{
-		{{"1.1", "1.1"}, {"1.1", "1.1"}, {"1.1", "1.1"}, {"1.1", "1.1"}},
-		{{"4.7", "4.7"}, {"4.7", "4.7"}, {"4.7", "4.7"}, {"4.7", "4.7"}},
-	} {
-		func() {
-			bootstrapVersion := cluster.ClusterVersion{
-				Version: roachpb.MustParseVersion(versions[0][0]),
-			}
-
-			knobs := base.TestingKnobs{
-				Store: &storage.StoreTestingKnobs{
-					BootstrapVersion: &bootstrapVersion,
-				},
-			}
-			tc := setupMixedCluster(t, knobs, versions, "")
-			defer tc.Stopper().Stop(ctx)
+	tcRaw := testcluster.StartTestCluster(t, 3, base.TestClusterArgs{})
+	defer tcRaw.Stopper().Stop(ctx)
+	tc := testClusterWithHelpers{
+		T:           t,
+		TestCluster: tcRaw,
+	}
 
-			exp := versions[0][0]
+	exp := cluster.BinaryServerVersion.String()
 
-			for i := 0; i < tc.NumServers(); i++ {
-				if version := tc.getVersionFromSetting(i).Version().Version.String(); version != exp {
-					t.Fatalf("%d: incorrect version %s (wanted %s)", i, version, exp)
-				}
-				if version := tc.getVersionFromShow(i); version != exp {
-					t.Fatalf("%d: incorrect version %s (wanted %s)", i, version, exp)
-				}
-
-				if version := tc.getVersionFromSelect(i); version != exp {
-					t.Fatalf("%d: incorrect version %q (wanted %s)", i, version, exp)
-				}
+	// The node bootstrapping the cluster starts at BinaryServerVersion, the
+	// others start at MinimumSupportedVersion and it takes them a gossip update
+	// to get to BinaryServerVersion. Hence, we loop until that gossip comes.
+	testutils.SucceedsSoon(tc, func() error {
+		for i := 0; i < tc.NumServers(); i++ {
+			if version := tc.getVersionFromSetting(i).Version().Version.String(); version != exp {
+				return fmt.Errorf("%d: incorrect version %s (wanted %s)", i, version, exp)
 			}
-		}()
-	}
+			if version := tc.getVersionFromShow(i); version != exp {
+				return fmt.Errorf("%d: incorrect version %s (wanted %s)", i, version, exp)
+			}
+			if version := tc.getVersionFromSelect(i); version != exp {
+				return fmt.Errorf("%d: incorrect version %q (wanted %s)", i, version, exp)
+			}
+		}
+		return nil
+	})
 }
 
 func TestClusterVersionMixedVersionTooOld(t *testing.T) {