AddShard should use targeted MySQL version (vitessio#11006)

When the host machine had a different major version of MySQL
installed in the standard location than we targeted in the
test, the shards added for the Reshard workflows were using
the default version installed which then differed from the
other shards -- which were using the version downloaded into
/tmp -- and caused schema differences that were surfaced in
CopySchemaShard.

Signed-off-by: Matt Lord <[email protected]>

go/test/endtoend/vreplication/cluster_test.go

@@ -343,39 +343,6 @@ func NewVitessCluster(t *testing.T, name string, cellNames []string, clusterConf
 // AddKeyspace creates a keyspace with specified shard keys and number of replica/read-only tablets.
 // You can pass optional key value pairs (opts) if you want conditional behavior.
 func (vc *VitessCluster) AddKeyspace(t *testing.T, cells []*Cell, ksName string, shards string, vschema string, schema string, numReplicas int, numRdonly int, tabletIDBase int, opts map[string]string) (*Keyspace, error) {
-	if value, exists := opts["DBTypeVersion"]; exists {
-		details := strings.Split(value, "-")
-		if len(details) != 2 {
-			t.Fatalf("Invalid database details: %s", value)
-		}
-		dbType := strings.ToLower(details[0])
-		majorVersion := details[1]
-		dbTypeMajorVersion := fmt.Sprintf("%s-%s", dbType, majorVersion)
-		// Do nothing if this version is already installed
-		dbVersionInUse, err := getDBTypeVersionInUse()
-		if err != nil {
-			t.Fatalf("Could not get details of database to be used for the keyspace: %v", err)
-		}
-		if dbTypeMajorVersion == dbVersionInUse {
-			t.Logf("Requsted database version %s is already installed, doing nothing.", dbTypeMajorVersion)
-		} else {
-			path := fmt.Sprintf("/tmp/%s", dbTypeMajorVersion)
-			// Set the root path and create it if needed
-			if err := setVtMySQLRoot(path); err != nil {
-				t.Fatalf("Could not set VT_MYSQL_ROOT to %s, error: %v", path, err)
-			}
-			defer unsetVtMySQLRoot()
-			// Download and extract the version artifact if needed
-			if err := downloadDBTypeVersion(dbType, majorVersion, path); err != nil {
-				t.Fatalf("Could not download %s, error: %v", majorVersion, err)
-			}
-			// Set the MYSQL_FLAVOR OS ENV var for mysqlctl to use the correct config file
-			if err := setDBFlavor(); err != nil {
-				t.Fatalf("Could not set MYSQL_FLAVOR: %v", err)
-			}
-			defer unsetDBFlavor()
-		}
-	}
 	keyspace := &Keyspace{
 		Name:   ksName,
 		Shards: make(map[string]*Shard),
@@ -392,7 +359,7 @@ func (vc *VitessCluster) AddKeyspace(t *testing.T, cells []*Cell, ksName string,
 		cell.Keyspaces[ksName] = keyspace
 		cellsToWatch = cellsToWatch + cell.Name
 	}
-	require.NoError(t, vc.AddShards(t, cells, keyspace, shards, numReplicas, numRdonly, tabletIDBase))
+	require.NoError(t, vc.AddShards(t, cells, keyspace, shards, numReplicas, numRdonly, tabletIDBase, opts))
 
 	if schema != "" {
 		if err := vc.VtctlClient.ApplySchema(ksName, schema); err != nil {
@@ -468,7 +435,11 @@ func (vc *VitessCluster) AddTablet(t testing.TB, cell *Cell, keyspace *Keyspace,
 }
 
 // AddShards creates shards given list of comma-separated keys with specified tablets in each shard
-func (vc *VitessCluster) AddShards(t testing.TB, cells []*Cell, keyspace *Keyspace, names string, numReplicas int, numRdonly int, tabletIDBase int) error {
+func (vc *VitessCluster) AddShards(t *testing.T, cells []*Cell, keyspace *Keyspace, names string, numReplicas int, numRdonly int, tabletIDBase int, opts map[string]string) error {
+	if value, exists := opts["DBTypeVersion"]; exists {
+		setupDBTypeVersion(t, value)
+	}
+
 	arrNames := strings.Split(names, ",")
 	log.Infof("Addshards got %d shards with %+v", len(arrNames), arrNames)
 	isSharded := len(arrNames) > 1
@@ -717,3 +688,37 @@ func (vc *VitessCluster) startQuery(t *testing.T, query string) (func(t *testing
 	}
 	return commit, rollback
 }
+
+func setupDBTypeVersion(t *testing.T, value string) {
+	details := strings.Split(value, "-")
+	if len(details) != 2 {
+		t.Fatalf("Invalid database details: %s", value)
+	}
+	dbType := strings.ToLower(details[0])
+	majorVersion := details[1]
+	dbTypeMajorVersion := fmt.Sprintf("%s-%s", dbType, majorVersion)
+	// Do nothing if this version is already installed
+	dbVersionInUse, err := getDBTypeVersionInUse()
+	if err != nil {
+		t.Fatalf("Could not get details of database to be used for the keyspace: %v", err)
+	}
+	if dbTypeMajorVersion == dbVersionInUse {
+		t.Logf("Requsted database version %s is already installed, doing nothing.", dbTypeMajorVersion)
+	} else {
+		path := fmt.Sprintf("/tmp/%s", dbTypeMajorVersion)
+		// Set the root path and create it if needed
+		if err := setVtMySQLRoot(path); err != nil {
+			t.Fatalf("Could not set VT_MYSQL_ROOT to %s, error: %v", path, err)
+		}
+		defer unsetVtMySQLRoot()
+		// Download and extract the version artifact if needed
+		if err := downloadDBTypeVersion(dbType, majorVersion, path); err != nil {
+			t.Fatalf("Could not download %s, error: %v", majorVersion, err)
+		}
+		// Set the MYSQL_FLAVOR OS ENV var for mysqlctl to use the correct config file
+		if err := setDBFlavor(); err != nil {
+			t.Fatalf("Could not set MYSQL_FLAVOR: %v", err)
+		}
+		defer unsetDBFlavor()
+	}
+}

go/test/endtoend/vreplication/config_test.go

@@ -21,12 +21,14 @@ package vreplication
 // MySQL 5.7+ and MariaDB 10.2+ to ensure that vreplication still works everywhere and the
 // permissive sql_mode now used in vreplication causes no unwanted side effects.
 // The customer table also tests two important things:
-//   1. Composite or multi-column primary keys
-//   2. PKs that contain an ENUM column
+//  1. Composite or multi-column primary keys
+//  2. PKs that contain an ENUM column
+//
 // The Lead and Lead-1 tables also allows us to test several things:
-//   1. Mixed case identifiers
-//   2. Column and table names with special characters in them, namely a dash
-//   3. Identifiers using reserved words, as lead is a reserved word in MySQL 8.0+ (https://dev.mysql.com/doc/refman/8.0/en/keywords.html)
+//  1. Mixed case identifiers
+//  2. Column and table names with special characters in them, namely a dash
+//  3. Identifiers using reserved words, as lead is a reserved word in MySQL 8.0+ (https://dev.mysql.com/doc/refman/8.0/en/keywords.html)
+//
 // The internal table _vt_PURGE_4f9194b43b2011eb8a0104ed332e05c2_20221210194431 should be ignored by vreplication
 // The db_order_test table is used to ensure vreplication and vdiff work well with complex non-integer PKs, even across DB versions.
 var (

go/test/endtoend/vreplication/resharding_workflows_v2_test.go

@@ -750,7 +750,7 @@ func moveCustomerTableSwitchFlows(t *testing.T, cells []*Cell, sourceCellOrAlias
 func createAdditionalCustomerShards(t *testing.T, shards string) {
 	ksName := "customer"
 	keyspace := vc.Cells[defaultCell.Name].Keyspaces[ksName]
-	require.NoError(t, vc.AddShards(t, []*Cell{defaultCell, vc.Cells["zone2"]}, keyspace, shards, defaultReplicas, defaultRdonly, 400))
+	require.NoError(t, vc.AddShards(t, []*Cell{defaultCell, vc.Cells["zone2"]}, keyspace, shards, defaultReplicas, defaultRdonly, 400, targetKsOpts))
 	arrTargetShardNames := strings.Split(shards, ",")
 
 	for _, shardName := range arrTargetShardNames {

go/test/endtoend/vreplication/vdiff2_test.go

@@ -153,7 +153,7 @@ func testWorkflow(t *testing.T, vc *VitessCluster, tc *testCase, cells []*Cell)
 	arrTargetShards := strings.Split(tc.targetShards, ",")
 	if tc.typ == "Reshard" {
 		tks := vc.Cells[cells[0].Name].Keyspaces[tc.targetKs]
-		require.NoError(t, vc.AddShards(t, cells, tks, tc.targetShards, 0, 0, tc.tabletBaseID))
+		require.NoError(t, vc.AddShards(t, cells, tks, tc.targetShards, 0, 0, tc.tabletBaseID, targetKsOpts))
 		for _, shard := range arrTargetShards {
 			require.NoError(t, vtgate.WaitForStatusOfTabletInShard(fmt.Sprintf("%s.%s.primary", tc.targetKs, shard), 1))
 		}

go/test/endtoend/vreplication/vreplication_test.go

@@ -759,7 +759,7 @@ func reshard(t *testing.T, ksName string, tableName string, workflow string, sou
 		}
 		ksWorkflow := ksName + "." + workflow
 		keyspace := vc.Cells[defaultCell.Name].Keyspaces[ksName]
-		require.NoError(t, vc.AddShards(t, cells, keyspace, targetShards, defaultReplicas, defaultRdonly, tabletIDBase))
+		require.NoError(t, vc.AddShards(t, cells, keyspace, targetShards, defaultReplicas, defaultRdonly, tabletIDBase, targetKsOpts))
 		arrTargetShardNames := strings.Split(targetShards, ",")
 
 		for _, shardName := range arrTargetShardNames {
@@ -1223,6 +1223,7 @@ func dropSources(t *testing.T, ksWorkflow string) {
 // This allows us to confirm two behaviors:
 //  1. MoveTables blocks on starting its first copy phase until we rollback
 //  2. All other workflows continue to work w/o issue with this MVCC history in place (not used yet)
+//
 // Returns a db connection used for the transaction which you can use for follow-up
 // work, such as rolling it back directly or using the releaseInnoDBRowHistory call.
 func generateInnoDBRowHistory(t *testing.T, sourceKS string, neededTrxHistory int) *mysql.Conn {