Revert "Make use of environment variables easier to troubleshoot"

Makefile

@@ -62,25 +62,16 @@ endif
 	bash ./build.env
 	go install $(EXTRA_BUILD_FLAGS) $(VT_GO_PARALLEL) -ldflags "$(shell tools/build_version_flags.sh)" -gcflags -'N -l' ./go/...
 
-check-prefix:
-	if [ -z ${PREFIX+x} ]; then echo "set the PREFIX environment with the install target"; false; fi
-
-check-vtroot:
-	if [ -z ${VTROOT+x} ]; then echo "need VTROOT environment variable set"; false; fi
-
-check-version:
-	if [ -z ${VERSION+x} ]; then echo "Set the env var VERSION with the release version"; false; fi
-
 # install copies the files needed to run Vitess into the given directory tree.
 # Usage: make install PREFIX=/path/to/install/root
-install: check-prefix build
+install: build
 	# binaries
 	mkdir -p "$${PREFIX}/bin"
 	cp "$${VTROOT}/bin/"{mysqlctld,vtctld,vtctlclient,vtgate,vttablet,vtworker,vtbackup} "$${PREFIX}/bin/"
 
 # install copies the files needed to run test Vitess using vtcombo into the given directory tree.
 # Usage: make install PREFIX=/path/to/install/root
-install-testing: check-vtroot check-prefix build
+install-testing: build
 	# binaries
 	mkdir -p "$${PREFIX}/bin"
 	cp "$${VTROOT}/bin/"{mysqlctld,mysqlctl,vtcombo,vttestserver} "$${PREFIX}/bin/"
@@ -170,7 +161,7 @@ ifndef NOBANNER
 	echo $$(date): Compiling proto definitions
 endif
 
-$(PROTO_GO_OUTS): check-vtroot install_protoc-gen-go proto/*.proto
+$(PROTO_GO_OUTS): install_protoc-gen-go proto/*.proto
 	for name in $(PROTO_SRC_NAMES); do \
 		$(VTROOT)/bin/protoc --go_out=plugins=grpc:. -Iproto proto/$${name}.proto && \
 		goimports -w vitess.io/vitess/go/vt/proto/$${name}/$${name}.pb.go; \
@@ -309,7 +300,10 @@ docker_unit_test:
 
 # Release a version.
 # This will generate a tar.gz file into the releases folder with the current source
-release: check-version docker_base
+release: docker_base
+	@if [ -z "$VERSION" ]; then \
+	  echo "Set the env var VERSION with the release version"; exit 1;\
+	fi
 	mkdir -p releases
 	docker build -f docker/Dockerfile.release -t vitess/release .
 	docker run -v ${PWD}/releases:/vt/releases --env VERSION=$(VERSION) vitess/release
@@ -318,7 +312,10 @@ release: check-version docker_base
 	echo "git push origin v$(VERSION)"
 	echo "Also, don't forget the upload releases/v$(VERSION).tar.gz file to GitHub releases"
 
-packages: check-version docker_base
+packages: docker_base
+	@if [ -z "$VERSION" ]; then \
+	  echo "Set the env var VERSION with the release version"; exit 1;\
+	fi
 	mkdir -p releases
 	docker build -f docker/packaging/Dockerfile -t vitess/packaging .
 	docker run --rm -v ${PWD}/releases:/vt/releases --env VERSION=$(VERSION) vitess/packaging --package /vt/releases -t deb --deb-no-default-config-files

go/test/endtoend/backup/transform/backup_transform_utils.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path"
 	"testing"
 	"time"
@@ -87,7 +88,7 @@ func TestMainSetup(m *testing.M, useMysqlctld bool) {
 		shard := &localCluster.Keyspaces[0].Shards[0]
 		// changing password for mysql user
 		dbCredentialFile = initialsharding.WriteDbCredentialToTmp(localCluster.TmpDirectory)
-		initDb, _ := ioutil.ReadFile(path.Join(cluster.GetEnvOrPanic("VTROOT"), "/config/init_db.sql"))
+		initDb, _ := ioutil.ReadFile(path.Join(os.Getenv("VTROOT"), "/config/init_db.sql"))
 		sql := string(initDb)
 		newInitDBFile = path.Join(localCluster.TmpDirectory, "init_db_with_passwords.sql")
 		sql = sql + initialsharding.GetPasswordUpdateSQL(localCluster)
@@ -97,7 +98,7 @@ func TestMainSetup(m *testing.M, useMysqlctld bool) {
 		commonTabletArg = append(commonTabletArg, "-db-credentials-file", dbCredentialFile)
 
 		// start mysql process for all replicas and master
-		var mysqlProcs []*cluster.MySQLCmd
+		var mysqlProcs []*exec.Cmd
 		for i := 0; i < 3; i++ {
 			tabletType := "replica"
 			tablet := localCluster.NewVttabletInstance(tabletType, 0, cell)

go/test/endtoend/backup/vtbackup/backup_only_test.go

@@ -182,7 +182,7 @@ func listBackups(shardKsName string) ([]string, error) {
 	backups, err := localCluster.VtctlProcess.ExecuteCommandWithOutput(
 		"-backup_storage_implementation", "file",
 		"-file_backup_storage_root",
-		path.Join(cluster.GetEnvOrPanic("VTDATAROOT"), "tmp", "backupstorage"),
+		path.Join(os.Getenv("VTDATAROOT"), "tmp", "backupstorage"),
 		"ListBackups", shardKsName,
 	)
 	if err != nil {
@@ -206,7 +206,7 @@ func removeBackups(t *testing.T) {
 		_, err := localCluster.VtctlProcess.ExecuteCommandWithOutput(
 			"-backup_storage_implementation", "file",
 			"-file_backup_storage_root",
-			path.Join(cluster.GetEnvOrPanic("VTDATAROOT"), "tmp", "backupstorage"),
+			path.Join(os.Getenv("VTDATAROOT"), "tmp", "backupstorage"),
 			"RemoveBackup", shardKsName, backup,
 		)
 		require.Nil(t, err)

go/test/endtoend/backup/vtbackup/main_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path"
 	"testing"
 
@@ -83,7 +84,7 @@ func TestMain(m *testing.M) {
 		// Create a new init_db.sql file that sets up passwords for all users.
 		// Then we use a db-credentials-file with the passwords.
 		dbCredentialFile = initialsharding.WriteDbCredentialToTmp(localCluster.TmpDirectory)
-		initDb, _ := ioutil.ReadFile(path.Join(cluster.GetEnvOrPanic("VTROOT"), "/config/init_db.sql"))
+		initDb, _ := ioutil.ReadFile(path.Join(os.Getenv("VTROOT"), "/config/init_db.sql"))
 		sql := string(initDb)
 		newInitDBFile = path.Join(localCluster.TmpDirectory, "init_db_with_passwords.sql")
 		sql = sql + initialsharding.GetPasswordUpdateSQL(localCluster)
@@ -101,7 +102,7 @@ func TestMain(m *testing.M) {
 		shard.Vttablets = []*cluster.Vttablet{master, replica1, replica2}
 
 		// Start MySql processes
-		var mysqlProcs []*cluster.MySQLCmd
+		var mysqlProcs []*exec.Cmd
 		for _, tablet := range shard.Vttablets {
 			tablet.VttabletProcess = localCluster.VtprocessInstanceFromVttablet(tablet, shard.Name, keyspaceName)
 			tablet.VttabletProcess.DbPassword = dbPassword
@@ -112,11 +113,12 @@ func TestMain(m *testing.M) {
 			tablet.MysqlctlProcess = *cluster.MysqlCtlProcessInstance(tablet.TabletUID, tablet.MySQLPort, localCluster.TmpDirectory)
 			tablet.MysqlctlProcess.InitDBFile = newInitDBFile
 			tablet.MysqlctlProcess.ExtraArgs = extraArgs
-			proc, err := tablet.MysqlctlProcess.StartProcess()
-			if err != nil {
+			if proc, err := tablet.MysqlctlProcess.StartProcess(); err != nil {
 				return 1, err
+			} else {
+				// ignore golint warning, we need the else block to use proc
+				mysqlProcs = append(mysqlProcs, proc)
 			}
-			mysqlProcs = append(mysqlProcs, proc)
 		}
 		for _, proc := range mysqlProcs {
 			if err := proc.Wait(); err != nil {

go/test/endtoend/backup/vtctlbackup/backup_utils.go

@@ -103,7 +103,7 @@ func LaunchCluster(setupType int, streamMode string, stripes int) (int, error) {
 	shard := &localCluster.Keyspaces[0].Shards[0]
 
 	dbCredentialFile = initialsharding.WriteDbCredentialToTmp(localCluster.TmpDirectory)
-	initDb, _ := ioutil.ReadFile(path.Join(cluster.GetEnvOrPanic("VTROOT"), "/config/init_db.sql"))
+	initDb, _ := ioutil.ReadFile(path.Join(os.Getenv("VTROOT"), "/config/init_db.sql"))
 	sql := string(initDb)
 	newInitDBFile = path.Join(localCluster.TmpDirectory, "init_db_with_passwords.sql")
 	sql = sql + initialsharding.GetPasswordUpdateSQL(localCluster)
@@ -135,7 +135,7 @@ func LaunchCluster(setupType int, streamMode string, stripes int) (int, error) {
 		commonTabletArg = append(commonTabletArg, xtrabackupArgs...)
 	}
 
-	var mysqlProcs []*cluster.MySQLCmd
+	var mysqlProcs []*exec.Cmd
 	for i := 0; i < 3; i++ {
 		tabletType := "replica"
 		if i == 0 {
@@ -437,7 +437,7 @@ func restartMasterReplica(t *testing.T) {
 	// remove all backups
 	localCluster.RemoveAllBackups(t, shardKsName)
 	// start all tablet and mysql instances
-	var mysqlProcs []*cluster.MySQLCmd
+	var mysqlProcs []*exec.Cmd
 	for _, tablet := range []*cluster.Vttablet{master, replica1, replica2} {
 		if tablet.MysqlctldProcess.TabletUID > 0 {
 			err := tablet.MysqlctldProcess.Start()

go/test/endtoend/cellalias/cell_alias_test.go

@@ -26,6 +26,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"os/exec"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -125,7 +126,7 @@ func TestMain(m *testing.M) {
 		shard2Replica = localCluster.NewVttabletInstance("replica", 0, cell2)
 		shard2Rdonly = localCluster.NewVttabletInstance("rdonly", 0, cell2)
 
-		var mysqlProcs []*cluster.MySQLCmd
+		var mysqlProcs []*exec.Cmd
 		for _, tablet := range []*cluster.Vttablet{shard1Master, shard1Replica, shard1Rdonly, shard2Master, shard2Replica, shard2Rdonly} {
 			tablet.MysqlctlProcess = *cluster.MysqlCtlProcessInstance(tablet.TabletUID, tablet.MySQLPort, localCluster.TmpDirectory)
 			tablet.VttabletProcess = cluster.VttabletProcessInstance(tablet.HTTPPort,

go/test/endtoend/cluster/cluster_process.go

@@ -169,7 +169,7 @@ func (cluster *LocalProcessCluster) StartTopo() (err error) {
 		cluster.Cell = DefaultCell
 	}
 	cluster.TopoPort = cluster.GetAndReservePort()
-	cluster.TmpDirectory = path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/tmp_%d", cluster.GetAndReservePort()))
+	cluster.TmpDirectory = path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/tmp_%d", cluster.GetAndReservePort()))
 	cluster.TopoProcess = *TopoProcessInstance(cluster.TopoPort, cluster.GetAndReservePort(), cluster.Hostname, *topoFlavor, "global")
 
 	log.Infof("Starting topo server %v on port: %d", *topoFlavor, cluster.TopoPort)
@@ -210,15 +210,6 @@ func (cluster *LocalProcessCluster) StartTopo() (err error) {
 	return
 }
 
-//GetEnvOrPanic panics is an environment variable is missing
-func GetEnvOrPanic(name string) string {
-	getenv, found := os.LookupEnv(name)
-	if !found {
-		panic(fmt.Sprintf("need %s to be set", name))
-	}
-	return getenv
-}
-
 // StartUnshardedKeyspace starts unshared keyspace with shard name as "0"
 func (cluster *LocalProcessCluster) StartUnshardedKeyspace(keyspace Keyspace, replicaCount int, rdonly bool) error {
 	return cluster.StartKeyspace(keyspace, []string{"0"}, replicaCount, rdonly)
@@ -239,12 +230,13 @@ func (cluster *LocalProcessCluster) StartKeyspace(keyspace Keyspace, shardNames
 
 	log.Infof("Starting keyspace: %v", keyspace.Name)
 	_ = cluster.VtctlProcess.CreateKeyspace(keyspace.Name)
-	var mysqlctlProcessList []*MySQLCmd
+	var mysqlctlProcessList []*exec.Cmd
 	for _, shardName := range shardNames {
 		shard := &Shard{
 			Name: shardName,
 		}
 		log.Infof("Starting shard: %v", shardName)
+		mysqlctlProcessList = []*exec.Cmd{}
 		for i := 0; i < totalTabletsRequired; i++ {
 			// instantiate vttablet object with reserved ports
 			tabletUID := cluster.GetAndReserveTabletUID()
@@ -446,8 +438,8 @@ func (cluster *LocalProcessCluster) NewVtgateInstance() *VtgateProcess {
 func NewCluster(cell string, hostname string) *LocalProcessCluster {
 	cluster := &LocalProcessCluster{Cell: cell, Hostname: hostname, mx: new(sync.Mutex)}
 	go cluster.CtrlCHandler()
-	cluster.OriginalVTDATAROOT = GetEnvOrPanic("VTDATAROOT")
-	cluster.CurrentVTDATAROOT = path.Join(cluster.OriginalVTDATAROOT, fmt.Sprintf("vtroot_%d", cluster.GetAndReservePort()))
+	cluster.OriginalVTDATAROOT = os.Getenv("VTDATAROOT")
+	cluster.CurrentVTDATAROOT = path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("vtroot_%d", cluster.GetAndReservePort()))
 	_ = createDirectory(cluster.CurrentVTDATAROOT, 0700)
 	_ = os.Setenv("VTDATAROOT", cluster.CurrentVTDATAROOT)
 	rand.Seed(time.Now().UTC().UnixNano())

go/test/endtoend/cluster/mysqlctl_process.go

@@ -66,62 +66,28 @@ func (mysqlctl *MysqlctlProcess) Start() (err error) {
 	return tmpProcess.Wait()
 }
 
-// MySQLCmd wraps a exec.Cmd and restarts it with verbose settings if it would fail
-type MySQLCmd struct {
-	mysqlctl *MysqlctlProcess
-	cmd      *exec.Cmd
-}
-
-//Wait waits for the underlying process to finish. If it fails, it's restarted with more log output
-func (m *MySQLCmd) Wait() error {
-	err := m.cmd.Wait()
-	if err != nil {
-		err = m.startProcess(true)
-		if err != nil {
-			return err
-		}
-		return m.cmd.Wait()
-	}
-	return nil
-}
-
-func (m *MySQLCmd) startProcess(verbose bool) error {
-	proc := exec.Command(
-		m.mysqlctl.Binary,
-		"-log_dir", m.mysqlctl.LogDirectory,
-		"-tablet_uid", fmt.Sprintf("%d", m.mysqlctl.TabletUID),
-		"-mysql_port", fmt.Sprintf("%d", m.mysqlctl.MySQLPort),
+// StartProcess starts the mysqlctl and returns the process reference
+func (mysqlctl *MysqlctlProcess) StartProcess() (*exec.Cmd, error) {
+	tmpProcess := exec.Command(
+		mysqlctl.Binary,
+		"-log_dir", mysqlctl.LogDirectory,
+		"-tablet_uid", fmt.Sprintf("%d", mysqlctl.TabletUID),
+		"-mysql_port", fmt.Sprintf("%d", mysqlctl.MySQLPort),
 	)
 	if *isCoverage {
-		proc.Args = append(proc.Args, []string{"-test.coverprofile=" + getCoveragePath("mysql-start.out")}...)
+		tmpProcess.Args = append(tmpProcess.Args, []string{"-test.coverprofile=" + getCoveragePath("mysql-start.out")}...)
 	}
 
-	if len(m.mysqlctl.ExtraArgs) > 0 {
-		proc.Args = append(proc.Args, m.mysqlctl.ExtraArgs...)
-	}
-	if m.mysqlctl.InitMysql {
-		proc.Args = append(proc.Args, "init",
-			"-init_db_sql_file", m.mysqlctl.InitDBFile)
+	if len(mysqlctl.ExtraArgs) > 0 {
+		tmpProcess.Args = append(tmpProcess.Args, mysqlctl.ExtraArgs...)
 	}
-	proc.Args = append(proc.Args, "start")
-	log.Infof("Starting mysqlctl with command: %v", proc.Args)
-	if verbose {
-		proc.Stdout = os.Stdout
-		proc.Stderr = os.Stderr
+	if mysqlctl.InitMysql {
+		tmpProcess.Args = append(tmpProcess.Args, "init",
+			"-init_db_sql_file", mysqlctl.InitDBFile)
 	}
-
-	m.cmd = proc
-
-	return proc.Start()
-}
-
-// StartProcess starts the mysqlctl and returns the process reference
-func (mysqlctl *MysqlctlProcess) StartProcess() (*MySQLCmd, error) {
-	cmd := &MySQLCmd{
-		mysqlctl: mysqlctl,
-	}
-
-	return cmd, cmd.startProcess(false)
+	tmpProcess.Args = append(tmpProcess.Args, "start")
+	log.Infof("Starting mysqlctl with command: %v", tmpProcess.Args)
+	return tmpProcess, tmpProcess.Start()
 }
 
 // Stop executes mysqlctl command to stop mysql instance
@@ -153,11 +119,11 @@ func (mysqlctl *MysqlctlProcess) StopProcess() (*exec.Cmd, error) {
 
 // CleanupFiles clean the mysql files to make sure we can start the same process again
 func (mysqlctl *MysqlctlProcess) CleanupFiles(tabletUID int) {
-	os.RemoveAll(path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d/data", tabletUID)))
-	os.RemoveAll(path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d/relay-logs", tabletUID)))
-	os.RemoveAll(path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d/tmp", tabletUID)))
-	os.RemoveAll(path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d/bin-logs", tabletUID)))
-	os.RemoveAll(path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d/innodb", tabletUID)))
+	os.RemoveAll(path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d/data", tabletUID)))
+	os.RemoveAll(path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d/relay-logs", tabletUID)))
+	os.RemoveAll(path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d/tmp", tabletUID)))
+	os.RemoveAll(path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d/bin-logs", tabletUID)))
+	os.RemoveAll(path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d/innodb", tabletUID)))
 }
 
 // MysqlCtlProcessInstance returns a Mysqlctl handle for mysqlctl process
@@ -167,7 +133,7 @@ func MysqlCtlProcessInstance(tabletUID int, mySQLPort int, tmpDirectory string)
 		Name:         "mysqlctl",
 		Binary:       "mysqlctl",
 		LogDirectory: tmpDirectory,
-		InitDBFile:   path.Join(GetEnvOrPanic("VTROOT"), "/config/init_db.sql"),
+		InitDBFile:   path.Join(os.Getenv("VTROOT"), "/config/init_db.sql"),
 	}
 	mysqlctl.MySQLPort = mySQLPort
 	mysqlctl.TabletUID = tabletUID
@@ -190,7 +156,7 @@ func StartMySQLAndGetConnection(ctx context.Context, tablet *Vttablet, username
 	}
 	params := mysql.ConnParams{
 		Uname:      username,
-		UnixSocket: path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d", tablet.TabletUID), "/mysql.sock"),
+		UnixSocket: path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d", tablet.TabletUID), "/mysql.sock"),
 	}
 
 	return mysql.Connect(ctx, &params)

go/test/endtoend/cluster/mysqlctld_process.go

@@ -139,7 +139,7 @@ func (mysqlctld *MysqlctldProcess) Stop() error {
 
 // CleanupFiles clean the mysql files to make sure we can start the same process again
 func (mysqlctld *MysqlctldProcess) CleanupFiles(tabletUID int) {
-	os.RemoveAll(path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d", tabletUID)))
+	os.RemoveAll(path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d", tabletUID)))
 }
 
 // MysqlCtldProcessInstance returns a Mysqlctld handle for mysqlctld process
@@ -149,7 +149,7 @@ func MysqlCtldProcessInstance(tabletUID int, mySQLPort int, tmpDirectory string)
 		Name:         "mysqlctld",
 		Binary:       "mysqlctld",
 		LogDirectory: tmpDirectory,
-		InitDBFile:   path.Join(GetEnvOrPanic("VTROOT"), "/config/init_db.sql"),
+		InitDBFile:   path.Join(os.Getenv("VTROOT"), "/config/init_db.sql"),
 	}
 	mysqlctld.MySQLPort = mySQLPort
 	mysqlctld.TabletUID = tabletUID
@@ -159,7 +159,7 @@ func MysqlCtldProcessInstance(tabletUID int, mySQLPort int, tmpDirectory string)
 
 // IsHealthy gives the health status of mysql.
 func (mysqlctld *MysqlctldProcess) IsHealthy() bool {
-	socketFile := path.Join(GetEnvOrPanic("VTDATAROOT"), fmt.Sprintf("/vt_%010d", mysqlctld.TabletUID), "/mysql.sock")
+	socketFile := path.Join(os.Getenv("VTDATAROOT"), fmt.Sprintf("/vt_%010d", mysqlctld.TabletUID), "/mysql.sock")
 	params := NewConnParams(0, mysqlctld.Password, socketFile, "")
 	_, err := mysql.Connect(context.Background(), &params)
 	return err == nil