mysqlctl: Remove usage of MYSQL_FLAVOR

go/mysql/endtoend/main_test.go

@@ -66,27 +66,13 @@ func runMysql(t *testing.T, params *mysql.ConnParams, command string) (string, b
 	// In particular, it has the message:
 	// Query OK, 1 row affected (0.00 sec)
 
-	version, getErr := mysqlctl.GetVersionString()
+	version, err := mysqlctl.GetVersionString()
+	if err != nil {
+		failVersionDetection(err)
+	}
 	f, v, err := mysqlctl.ParseVersionString(version)
-
-	if getErr != nil || err != nil {
-		f, v, err = mysqlctl.GetVersionFromEnv()
-		if err != nil {
-			vtenvMysqlRoot, _ := vtenv.VtMysqlRoot()
-			message := fmt.Sprintf(`could not auto-detect MySQL version. You may need to set your PATH so a mysqld binary can be found, or set the environment variable MYSQL_FLAVOR if mysqld is not available locally:
-	PATH: %s
-	VT_MYSQL_ROOT: %s
-	VTROOT: %s
-	vtenv.VtMysqlRoot(): %s
-	MYSQL_FLAVOR: %s
-	`,
-				os.Getenv("PATH"),
-				os.Getenv("VT_MYSQL_ROOT"),
-				os.Getenv("VTROOT"),
-				vtenvMysqlRoot,
-				os.Getenv("MYSQL_FLAVOR"))
-			panic(message)
-		}
+	if err != nil {
+		failVersionDetection(err)
 	}
 
 	t.Logf("Using flavor: %v, version: %v", f, v)
@@ -237,3 +223,20 @@ ssl-key=%v/server-key.pem
 	}()
 	os.Exit(exitCode)
 }
+
+func failVersionDetection(err error) {
+	vtenvMysqlRoot, _ := vtenv.VtMysqlRoot()
+	message := fmt.Sprintf(`could not auto-detect MySQL version: %v
+You may need to set your PATH so a mysqld binary can be found:
+	PATH: %s
+	VT_MYSQL_ROOT: %s
+	VTROOT: %s
+	vtenv.VtMysqlRoot(): %s
+	`,
+		err,
+		os.Getenv("PATH"),
+		os.Getenv("VT_MYSQL_ROOT"),
+		os.Getenv("VTROOT"),
+		vtenvMysqlRoot)
+	panic(message)
+}

go/test/endtoend/mysqlctl/mysqlctl_test.go

@@ -150,10 +150,6 @@ func TestRestart(t *testing.T) {
 func TestAutoDetect(t *testing.T) {
 	defer cluster.PanicHandler(t)
 
-	// Start up tablets with an empty MYSQL_FLAVOR, which means auto-detect
-	sqlFlavor := os.Getenv("MYSQL_FLAVOR")
-	os.Setenv("MYSQL_FLAVOR", "")
-
 	err := clusterInstance.Keyspaces[0].Shards[0].Vttablets[0].VttabletProcess.Setup()
 	require.Nil(t, err, "error should be nil")
 	err = clusterInstance.Keyspaces[0].Shards[0].Vttablets[1].VttabletProcess.Setup()
@@ -162,8 +158,4 @@ func TestAutoDetect(t *testing.T) {
 	// Reparent tablets, which requires flavor detection
 	err = clusterInstance.VtctlclientProcess.InitializeShard(keyspaceName, shardName, cell, primaryTablet.TabletUID)
 	require.Nil(t, err, "error should be nil")
-
-	//Reset flavor
-	os.Setenv("MYSQL_FLAVOR", sqlFlavor)
-
 }

go/test/endtoend/mysqlctld/mysqlctld_test.go

@@ -147,10 +147,6 @@ func TestRestart(t *testing.T) {
 func TestAutoDetect(t *testing.T) {
 	defer cluster.PanicHandler(t)
 
-	// Start up tablets with an empty MYSQL_FLAVOR, which means auto-detect
-	sqlFlavor := os.Getenv("MYSQL_FLAVOR")
-	os.Setenv("MYSQL_FLAVOR", "")
-
 	err := clusterInstance.Keyspaces[0].Shards[0].Vttablets[0].VttabletProcess.Setup()
 	require.Nil(t, err, "error should be nil")
 	err = clusterInstance.Keyspaces[0].Shards[0].Vttablets[1].VttabletProcess.Setup()
@@ -159,8 +155,4 @@ func TestAutoDetect(t *testing.T) {
 	// Reparent tablets, which requires flavor detection
 	err = clusterInstance.VtctlclientProcess.InitializeShard(keyspaceName, shardName, cell, primaryTablet.TabletUID)
 	require.Nil(t, err, "error should be nil")
-
-	//Reset flavor
-	os.Setenv("MYSQL_FLAVOR", sqlFlavor)
-
 }

go/test/endtoend/vreplication/cluster_test.go

@@ -178,37 +178,10 @@ func setVtMySQLRoot(mysqlRoot string) error {
 	return nil
 }
 
-// setDBFlavor sets the MYSQL_FLAVOR OS env var.
-// You should call this after calling setVtMySQLRoot() to ensure that the
-// correct flavor is used by mysqlctl based on the current mysqld version
-// in the path. If you don't do this then mysqlctl will use the incorrect
-// config/mycnf/<flavor>.cnf file and mysqld may fail to start.
-func setDBFlavor() error {
-	versionStr, err := mysqlctl.GetVersionString()
-	if err != nil {
-		return err
-	}
-	f, v, err := mysqlctl.ParseVersionString(versionStr)
-	if err != nil {
-		return err
-	}
-	flavor := fmt.Sprintf("%s%d%d", f, v.Major, v.Minor)
-	err = os.Setenv("MYSQL_FLAVOR", string(flavor))
-	if err != nil {
-		return err
-	}
-	fmt.Printf("MYSQL_FLAVOR is %s\n", string(flavor))
-	return nil
-}
-
 func unsetVtMySQLRoot() {
 	_ = os.Unsetenv("VT_MYSQL_ROOT")
 }
 
-func unsetDBFlavor() {
-	_ = os.Unsetenv("MYSQL_FLAVOR")
-}
-
 // getDBTypeVersionInUse checks the major DB version of the mysqld binary
 // that mysqlctl would currently use, e.g. 5.7 or 8.0 (in semantic versioning
 // this would be major.minor but in MySQL it's effectively the major version).
@@ -781,12 +754,7 @@ func setupDBTypeVersion(t *testing.T, value string) func() {
 	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)
-	}
 	return func() {
-		unsetDBFlavor()
 		unsetVtMySQLRoot()
 	}
 }

go/vt/mysqlctl/mysqld.go

@@ -145,93 +145,39 @@ func NewMysqld(dbcfgs *dbconfigs.DBConfigs) *Mysqld {
 	result.appPool.Open(dbcfgs.AppWithDB())
 
 	/*
-	 Unmanaged tablets are special because the MYSQL_FLAVOR detection
-	 will not be accurate because the mysqld might not be the same
-	 one as the server started.
-
-	 This skips the panic that checks that we can detect a server,
-	 but also relies on none of the flavor detection features being
-	 used at runtime. Currently this assumption is guaranteed true.
+	 If we have an external unmanaged tablet, we can't do the flavor
+	 detection here. We also won't need it, since mysqlctl itself is the only
+	 one that needs capabilities and the flavor.
 	*/
 	if dbconfigs.GlobalDBConfigs.HasGlobalSettings() {
 		log.Info("mysqld is unmanaged or remote. Skipping flavor detection")
 		return result
 	}
 
+	/*
+	 If we have a socketFile here, it means we're not running inside mysqlctl.
+	 This means we don't need the flavor and capability detection, since mysqlctl
+	 itself is the only one that needs this.
+	*/
 	if socketFile != "" {
 		log.Info("mysqld is remote. Skipping flavor detection")
 		return result
 	}
 
-	version, getErr := GetVersionString()
+	version, err := GetVersionString()
+	if err != nil {
+		failVersionDetection(err)
+	}
 	f, v, err := ParseVersionString(version)
-
-	/*
-	 By default Vitess searches in vtenv.VtMysqlRoot() for a mysqld binary.
-	 This is historically the VT_MYSQL_ROOT env, but if it is unset or empty,
-	 Vitess will search the PATH. See go/vt/env/env.go.
-
-	 A number of subdirs inside vtenv.VtMysqlRoot() will be searched, see
-	 func binaryPath() for context. If no mysqld binary is found (possibly
-	 because it is in a container or both VT_MYSQL_ROOT and VTROOT are set
-	 incorrectly), there will be a fallback to using the MYSQL_FLAVOR env
-	 variable.
-
-	 If MYSQL_FLAVOR is not defined, there will be a panic.
-
-	 Note: relying on MySQL_FLAVOR is not recommended, since for historical
-	 purposes "MySQL56" actually means MySQL 5.7, which is a very strange
-	 behavior.
-	*/
-
-	if getErr != nil || err != nil {
-		f, v, err = GetVersionFromEnv()
-		if err != nil {
-			vtenvMysqlRoot, _ := vtenv.VtMysqlRoot()
-			message := fmt.Sprintf(`could not auto-detect MySQL version. You may need to set your PATH so a mysqld binary can be found, or set the environment variable MYSQL_FLAVOR if mysqld is not available locally:
-	PATH: %s
-	VT_MYSQL_ROOT: %s
-	VTROOT: %s
-	vtenv.VtMysqlRoot(): %s
-	MYSQL_FLAVOR: %s
-	`,
-				os.Getenv("PATH"),
-				os.Getenv("VT_MYSQL_ROOT"),
-				os.Getenv("VTROOT"),
-				vtenvMysqlRoot,
-				os.Getenv("MYSQL_FLAVOR"))
-			panic(message)
-		}
+	if err != nil {
+		failVersionDetection(err)
 	}
 
 	log.Infof("Using flavor: %v, version: %v", f, v)
 	result.capabilities = newCapabilitySet(f, v)
 	return result
 }
 
-/*
-GetVersionFromEnv returns the flavor and an assumed version based on the legacy
-MYSQL_FLAVOR environment variable.
-
-The assumed version may not be accurate since the legacy variable only specifies
-broad families of compatible versions. However, the differences between those
-versions should only matter if Vitess is managing the lifecycle of mysqld, in which
-case we should have a local copy of the mysqld binary from which we can fetch
-the accurate version instead of falling back to this function (see GetVersionString).
-*/
-func GetVersionFromEnv() (flavor MySQLFlavor, ver ServerVersion, err error) {
-	env := os.Getenv("MYSQL_FLAVOR")
-	switch env {
-	case "MariaDB":
-		return FlavorMariaDB, ServerVersion{10, 6, 11}, nil
-	case "MySQL80":
-		return FlavorMySQL, ServerVersion{8, 0, 11}, nil
-	case "MySQL56":
-		return FlavorMySQL, ServerVersion{5, 7, 10}, nil
-	}
-	return flavor, ver, fmt.Errorf("could not determine version from MYSQL_FLAVOR: %s", env)
-}
-
 // GetVersionString runs mysqld --version and returns its output as a string
 func GetVersionString() (string, error) {
 	noSocketFile()
@@ -1332,3 +1278,20 @@ func noSocketFile() {
 		panic("Running remotely through mysqlctl, socketFile must not be set")
 	}
 }
+
+func failVersionDetection(err error) {
+	vtenvMysqlRoot, _ := vtenv.VtMysqlRoot()
+	message := fmt.Sprintf(`could not auto-detect MySQL version: %v
+You may need to set your PATH so a mysqld binary can be found:
+	PATH: %s
+	VT_MYSQL_ROOT: %s
+	VTROOT: %s
+	vtenv.VtMysqlRoot(): %s
+	`,
+		err,
+		os.Getenv("PATH"),
+		os.Getenv("VT_MYSQL_ROOT"),
+		os.Getenv("VTROOT"),
+		vtenvMysqlRoot)
+	panic(message)
+}

go/vt/mysqlctl/mysqld_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package mysqlctl
 
 import (
-	"os"
 	"testing"
 )
 
@@ -106,36 +105,3 @@ func TestParseVersionString(t *testing.T) {
 	}
 
 }
-
-func TestAssumeVersionString(t *testing.T) {
-
-	// In these cases, the versionstring is nonsensical or unspecified.
-	// MYSQL_FLAVOR is used instead.
-
-	var testcases = []testcase{
-		{
-			versionString: "MySQL80",
-			version:       ServerVersion{8, 0, 11},
-			flavor:        FlavorMySQL,
-		},
-		{
-			versionString: "MySQL56",
-			version:       ServerVersion{5, 7, 10}, // Yes, this has to lie!
-			flavor:        FlavorMySQL,             // There was no MySQL57 option
-		},
-		{
-			versionString: "MariaDB",
-			version:       ServerVersion{10, 6, 11},
-			flavor:        FlavorMariaDB,
-		},
-	}
-
-	for _, testcase := range testcases {
-		os.Setenv("MYSQL_FLAVOR", testcase.versionString)
-		f, v, err := GetVersionFromEnv()
-		if v != testcase.version || f != testcase.flavor || err != nil {
-			t.Errorf("GetVersionFromEnv() failed for: %#v, Got: %#v, %#v Expected: %#v, %#v", testcase.versionString, v, f, testcase.version, testcase.flavor)
-		}
-	}
-
-}

go/vt/vttest/environment.go

@@ -103,21 +103,16 @@ type LocalTestEnv struct {
 	Env          []string
 }
 
-// DefaultMySQLFlavor is the MySQL flavor used by vttest when MYSQL_FLAVOR is not
-// set in the environment
+// DefaultMySQLFlavor is the MySQL flavor used by vttest when no explicit
+// flavor is given.
 const DefaultMySQLFlavor = "MySQL56"
 
 // GetMySQLOptions returns the default option set for the given MySQL
-// flavor. If flavor is not set, the value from the `MYSQL_FLAVOR` env
-// variable is used, and if this is not set, DefaultMySQLFlavor will
+// flavor. If flavor is not set, DefaultMySQLFlavor will
 // be used.
 // Returns the name of the MySQL flavor being used, the set of MySQL CNF
 // files specific to this flavor, and any errors.
 func GetMySQLOptions(flavor string) (string, []string, error) {
-	if flavor == "" {
-		flavor = os.Getenv("MYSQL_FLAVOR")
-	}
-
 	if flavor == "" {
 		flavor = DefaultMySQLFlavor
 	}
@@ -237,8 +232,7 @@ func randomPort() int {
 // up when closing the Environment.
 // - LogDirectory() is the `logs` subdir inside Directory()
 // - The MySQL flavor is set to `flavor`. If the argument is not set, it will
-// default to the value of MYSQL_FLAVOR, and if this variable is not set, to
-// DefaultMySQLFlavor
+// default DefaultMySQLFlavor
 // - PortForProtocol() will return ports based off the given basePort. If basePort
 // is zero, a random port between 10000 and 20000 will be chosen.
 // - DefaultProtocol() is always "grpc"