Getting rid of GetMachineName() (#5868)

* removing GetMachineName()

* replacing GetMachineName with a config lookup

* fix tests

* making unit tests happy

* fixing concurrent map writes, only right to viper once

* debugging

* one last test fix

* DeleteAlwaysSucceeds should actually always succeed

* prevent panic when deleting nonexistant cluster

* remove last GetMachineName reference

cmd/minikube/cmd/config/util.go

@@ -137,7 +137,12 @@ func EnableOrDisableAddon(name string, val string) error {
 		return nil
 	}
 
-	host, err := cluster.CheckIfHostExistsAndLoad(api, config.GetMachineName())
+	cfg, err := config.Load()
+	if err != nil && !os.IsNotExist(err) {
+		exit.WithCodeT(exit.Data, "Unable to load config: {{.error}}", out.V{"error": err})
+	}
+
+	host, err := cluster.CheckIfHostExistsAndLoad(api, cfg.MachineConfig.Name)
 	if err != nil {
 		return errors.Wrap(err, "getting host")
 	}
@@ -147,11 +152,6 @@ func EnableOrDisableAddon(name string, val string) error {
 		return errors.Wrap(err, "command runner")
 	}
 
-	cfg, err := config.Load()
-	if err != nil && !os.IsNotExist(err) {
-		exit.WithCodeT(exit.Data, "Unable to load config: {{.error}}", out.V{"error": err})
-	}
-
 	data := assets.GenerateTemplateData(cfg.KubernetesConfig)
 	return enableOrDisableAddonInternal(addon, cmd, data, enable)
 }

cmd/minikube/cmd/dashboard.go

@@ -74,10 +74,10 @@ var dashboardCmd = &cobra.Command{
 			exit.WithError("Error getting client", err)
 		}
 
-		if _, err = api.Load(pkg_config.GetMachineName()); err != nil {
+		if _, err = api.Load(cc.MachineConfig.Name); err != nil {
 			switch err := errors.Cause(err).(type) {
 			case mcnerror.ErrHostDoesNotExist:
-				exit.WithCodeT(exit.Unavailable, "{{.name}} cluster does not exist", out.V{"name": pkg_config.GetMachineName()})
+				exit.WithCodeT(exit.Unavailable, "{{.name}} cluster does not exist", out.V{"name": cc.MachineConfig.Name})
 			default:
 				exit.WithError("Error getting cluster", err)
 			}
@@ -119,7 +119,7 @@ var dashboardCmd = &cobra.Command{
 		}
 
 		out.ErrT(out.Launch, "Launching proxy ...")
-		p, hostPort, err := kubectlProxy(kubectl)
+		p, hostPort, err := kubectlProxy(kubectl, cc.MachineConfig.Name)
 		if err != nil {
 			exit.WithError("kubectl proxy", err)
 		}
@@ -153,11 +153,10 @@ var dashboardCmd = &cobra.Command{
 }
 
 // kubectlProxy runs "kubectl proxy", returning host:port
-func kubectlProxy(path string) (*exec.Cmd, string, error) {
+func kubectlProxy(path string, machineName string) (*exec.Cmd, string, error) {
 	// port=0 picks a random system port
-	// pkg_config.GetMachineName() respects the -p (profile) flag
 
-	cmd := exec.Command(path, "--context", pkg_config.GetMachineName(), "proxy", "--port=0")
+	cmd := exec.Command(path, "--context", machineName, "proxy", "--port=0")
 
 	stdoutPipe, err := cmd.StdoutPipe()
 	if err != nil {

cmd/minikube/cmd/delete.go

@@ -190,7 +190,6 @@ func deleteProfile(profile *pkg_config.Profile) error {
 
 	cc, err := pkg_config.Load()
 	if err != nil && !os.IsNotExist(err) {
-		out.ErrT(out.Sad, "Error loading profile {{.name}}: {{.error}}", out.V{"name": profile, "error": err})
 		delErr := profileDeletionErr(profile.Name, fmt.Sprintf("error loading profile config: %v", err))
 		return DeletionError{Err: delErr, Errtype: MissingProfile}
 	}
@@ -211,7 +210,7 @@ func deleteProfile(profile *pkg_config.Profile) error {
 		out.T(out.FailureType, "Failed to kill mount process: {{.error}}", out.V{"error": err})
 	}
 
-	if err = cluster.DeleteHost(api); err != nil {
+	if err = cluster.DeleteHost(api, profile.Name); err != nil {
 		switch errors.Cause(err).(type) {
 		case mcnerror.ErrHostDoesNotExist:
 			out.T(out.Meh, `"{{.name}}" cluster does not exist. Proceeding ahead with cleanup.`, out.V{"name": profile.Name})
@@ -235,8 +234,7 @@ func deleteProfile(profile *pkg_config.Profile) error {
 
 	out.T(out.Crushed, `The "{{.name}}" cluster has been deleted.`, out.V{"name": profile.Name})
 
-	machineName := pkg_config.GetMachineName()
-	if err := deleteContext(machineName); err != nil {
+	if err := deleteContext(profile.Name); err != nil {
 		return err
 	}
 	return nil

cmd/minikube/cmd/env.go

@@ -33,6 +33,7 @@ import (
 	"github.com/docker/machine/libmachine/state"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 	"k8s.io/minikube/pkg/minikube/cluster"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/driver"
@@ -171,7 +172,7 @@ func shellCfgSet(api libmachine.API) (*ShellConfig, error) {
 	}
 
 	if noProxy {
-		host, err := api.Load(config.GetMachineName())
+		host, err := api.Load(viper.GetString(config.MachineProfile))
 		if err != nil {
 			return nil, errors.Wrap(err, "Error getting IP")
 		}
@@ -339,14 +340,18 @@ var dockerEnvCmd = &cobra.Command{
 			exit.WithError("Error getting client", err)
 		}
 		defer api.Close()
-		host, err := cluster.CheckIfHostExistsAndLoad(api, config.GetMachineName())
+		cc, err := config.Load()
+		if err != nil {
+			exit.WithError("Error getting config", err)
+		}
+		host, err := cluster.CheckIfHostExistsAndLoad(api, cc.MachineConfig.Name)
 		if err != nil {
 			exit.WithError("Error getting host", err)
 		}
 		if host.Driver.DriverName() == driver.None {
 			exit.UsageT(`'none' driver does not support 'minikube docker-env' command`)
 		}
-		hostSt, err := cluster.GetHostStatus(api)
+		hostSt, err := cluster.GetHostStatus(api, cc.MachineConfig.Name)
 		if err != nil {
 			exit.WithError("Error getting host status", err)
 		}

cmd/minikube/cmd/env_test.go

@@ -21,7 +21,9 @@ import (
 	"testing"
 
 	"github.com/docker/machine/libmachine/host"
+	"github.com/spf13/viper"
 	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/localpath"
 	"k8s.io/minikube/pkg/minikube/tests"
 )
@@ -46,8 +48,8 @@ func (f FakeNoProxyGetter) GetNoProxyVar() (string, string) {
 var defaultAPI = &tests.MockAPI{
 	FakeStore: tests.FakeStore{
 		Hosts: map[string]*host.Host{
-			config.GetMachineName(): {
-				Name:   config.GetMachineName(),
+			constants.DefaultMachineName: {
+				Name:   constants.DefaultMachineName,
 				Driver: &tests.MockDriver{},
 			},
 		},
@@ -232,6 +234,7 @@ func TestShellCfgSet(t *testing.T) {
 		test := test
 		t.Run(test.description, func(t *testing.T) {
 
+			viper.Set(config.MachineProfile, constants.DefaultMachineName)
 			defaultShellDetector = &FakeShellDetector{test.shell}
 			defaultNoProxyGetter = &FakeNoProxyGetter{test.noProxyVar, test.noProxyValue}
 			noProxy = test.noProxyFlag
@@ -241,7 +244,7 @@ func TestShellCfgSet(t *testing.T) {
 				t.Errorf("Shell cfgs differ: expected %+v, \n\n got %+v", test.expectedShellCfg, shellCfg)
 			}
 			if err != nil && !test.shouldErr {
-				t.Errorf("Test should have failed but didn't return error: %s, error: %v", test.description, err)
+				t.Errorf("Unexpected error occurred: %s, error: %v", test.description, err)
 			}
 			if err == nil && test.shouldErr {
 				t.Errorf("Test didn't return error but should have: %s", test.description)

cmd/minikube/cmd/ip.go

@@ -38,11 +38,15 @@ var ipCmd = &cobra.Command{
 		}
 		defer api.Close()
 
-		host, err := api.Load(config.GetMachineName())
+		cc, err := config.Load()
+		if err != nil {
+			exit.WithError("Error getting config", err)
+		}
+		host, err := api.Load(cc.MachineConfig.Name)
 		if err != nil {
 			switch err := errors.Cause(err).(type) {
 			case mcnerror.ErrHostDoesNotExist:
-				exit.WithCodeT(exit.NoInput, `"{{.profile_name}}" host does not exist, unable to show an IP`, out.V{"profile_name": config.GetMachineName()})
+				exit.WithCodeT(exit.NoInput, `"{{.profile_name}}" host does not exist, unable to show an IP`, out.V{"profile_name": cc.MachineConfig.Name})
 			default:
 				exit.WithError("Error getting host", err)
 			}

cmd/minikube/cmd/logs.go

@@ -58,7 +58,7 @@ var logsCmd = &cobra.Command{
 		}
 		defer api.Close()
 
-		h, err := api.Load(config.GetMachineName())
+		h, err := api.Load(cfg.MachineConfig.Name)
 		if err != nil {
 			exit.WithError("api load", err)
 		}

cmd/minikube/cmd/mount.go

@@ -102,7 +102,11 @@ var mountCmd = &cobra.Command{
 			exit.WithError("Error getting client", err)
 		}
 		defer api.Close()
-		host, err := api.Load(config.GetMachineName())
+		cc, err := config.Load()
+		if err != nil {
+			exit.WithError("Error getting config", err)
+		}
+		host, err := api.Load(cc.MachineConfig.Name)
 
 		if err != nil {
 			exit.WithError("Error loading api", err)

cmd/minikube/cmd/ssh-key.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/spf13/cobra"
 	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/exit"
 	"k8s.io/minikube/pkg/minikube/localpath"
 	"k8s.io/minikube/pkg/minikube/out"
 )
@@ -31,6 +32,10 @@ var sshKeyCmd = &cobra.Command{
 	Short: "Retrieve the ssh identity key path of the specified cluster",
 	Long:  "Retrieve the ssh identity key path of the specified cluster.",
 	Run: func(cmd *cobra.Command, args []string) {
-		out.Ln(filepath.Join(localpath.MiniPath(), "machines", config.GetMachineName(), "id_rsa"))
+		cc, err := config.Load()
+		if err != nil {
+			exit.WithError("Getting machine config failed", err)
+		}
+		out.Ln(filepath.Join(localpath.MiniPath(), "machines", cc.MachineConfig.Name, "id_rsa"))
 	},
 }

cmd/minikube/cmd/ssh.go

@@ -42,7 +42,11 @@ var sshCmd = &cobra.Command{
 			exit.WithError("Error getting client", err)
 		}
 		defer api.Close()
-		host, err := cluster.CheckIfHostExistsAndLoad(api, config.GetMachineName())
+		cc, err := config.Load()
+		if err != nil {
+			exit.WithError("Error getting config", err)
+		}
+		host, err := cluster.CheckIfHostExistsAndLoad(api, cc.MachineConfig.Name)
 		if err != nil {
 			exit.WithError("Error getting host", err)
 		}

cmd/minikube/cmd/start.go

@@ -366,7 +366,7 @@ func runStart(cmd *cobra.Command, args []string) {
 		prepareNone()
 	}
 	waitCluster(bs, config)
-	if err := showKubectlInfo(kubeconfig, k8sVersion); err != nil {
+	if err := showKubectlInfo(kubeconfig, k8sVersion, config.MachineConfig.Name); err != nil {
 		glog.Errorf("kubectl info: %v", err)
 	}
 }
@@ -439,7 +439,7 @@ func setupKubeconfig(h *host.Host, c *cfg.Config) (*kubeconfig.Settings, error)
 	}
 
 	kcs := &kubeconfig.Settings{
-		ClusterName:          cfg.GetMachineName(),
+		ClusterName:          c.MachineConfig.Name,
 		ClusterServerAddress: addr,
 		ClientCertificate:    localpath.MakeMiniPath("client.crt"),
 		ClientKey:            localpath.MakeMiniPath("client.key"),
@@ -509,11 +509,11 @@ func showVersionInfo(k8sVersion string, cr cruntime.Manager) {
 	}
 }
 
-func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string) error {
+func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string, machineName string) error {
 	if kcs.KeepContext {
 		out.T(out.Kubectl, "To connect to this cluster, use: kubectl --context={{.name}}", out.V{"name": kcs.ClusterName})
 	} else {
-		out.T(out.Ready, `Done! kubectl is now configured to use "{{.name}}"`, out.V{"name": cfg.GetMachineName()})
+		out.T(out.Ready, `Done! kubectl is now configured to use "{{.name}}"`, out.V{"name": machineName})
 	}
 
 	path, err := exec.LookPath("kubectl")
@@ -622,7 +622,8 @@ func validateDriver(name string, existing *cfg.Config) {
 		return
 	}
 
-	h, err := api.Load(cfg.GetMachineName())
+	machineName := viper.GetString(cfg.MachineProfile)
+	h, err := api.Load(machineName)
 	if err != nil {
 		glog.Warningf("selectDriver api.Load: %v", err)
 		return
@@ -632,8 +633,8 @@ func validateDriver(name string, existing *cfg.Config) {
 		return
 	}
 
-	out.ErrT(out.Conflict, `The existing "{{.profile_name}}" cluster was created using the "{{.old_driver}}" driver, and cannot be started using the "{{.driver}}" driver.`,
-		out.V{"profile_name": cfg.GetMachineName(), "driver": name, "old_driver": h.Driver.DriverName()})
+	out.ErrT(out.Conflict, `The existing "{{.profile_name}}" VM that was created using the "{{.old_driver}}" driver, and is incompatible with the "{{.driver}}" driver.`,
+		out.V{"profile_name": machineName, "driver": name, "old_driver": h.Driver.DriverName()})
 
 	out.ErrT(out.Workaround, `To proceed, either:
 
@@ -642,7 +643,7 @@ func validateDriver(name string, existing *cfg.Config) {
     * or *
 
     2) Start the existing "{{.profile_name}}" cluster using: '{{.command}} start --vm-driver={{.old_driver}}'
-	`, out.V{"command": minikubeCmd(), "old_driver": h.Driver.DriverName(), "profile_name": cfg.GetMachineName()})
+	`, out.V{"command": minikubeCmd(), "old_driver": h.Driver.DriverName(), "profile_name": machineName})
 
 	exit.WithCodeT(exit.Config, "Exiting.")
 }
@@ -758,7 +759,7 @@ func validateFlags(cmd *cobra.Command, drvName string) {
 
 	var cpuCount int
 	if driver.BareMetal(drvName) {
-		if cfg.GetMachineName() != constants.DefaultMachineName {
+		if viper.GetString(cfg.MachineProfile) != constants.DefaultMachineName {
 			exit.WithCodeT(exit.Config, "The 'none' driver does not support multiple profiles: https://minikube.sigs.k8s.io/docs/reference/drivers/none/")
 		}
 
@@ -890,6 +891,7 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, drvName string)
 
 	cfg := cfg.Config{
 		MachineConfig: cfg.MachineConfig{
+			Name:                viper.GetString(cfg.MachineProfile),
 			KeepContext:         viper.GetBool(keepContext),
 			EmbedCerts:          viper.GetBool(embedCerts),
 			MinikubeISO:         viper.GetString(isoURL),
@@ -1003,7 +1005,7 @@ func prepareNone() {
 
 // startHost starts a new minikube host using a VM or None
 func startHost(api libmachine.API, mc cfg.MachineConfig) (*host.Host, bool) {
-	exists, err := api.Exists(cfg.GetMachineName())
+	exists, err := api.Exists(mc.Name)
 	if err != nil {
 		exit.WithError("Failed to check if machine exists", err)
 	}
@@ -1013,7 +1015,7 @@ func startHost(api libmachine.API, mc cfg.MachineConfig) (*host.Host, bool) {
 		host, err = cluster.StartHost(api, mc)
 		if err != nil {
 			out.T(out.Resetting, "Retriable failure: {{.error}}", out.V{"error": err})
-			if derr := cluster.DeleteHost(api); derr != nil {
+			if derr := cluster.DeleteHost(api, mc.Name); derr != nil {
 				glog.Warningf("DeleteHost: %v", derr)
 			}
 		}
@@ -1149,8 +1151,8 @@ func getKubernetesVersion(old *cfg.Config) (string, bool) {
 	if nvs.LT(ovs) {
 		nv = version.VersionPrefix + ovs.String()
 		profileArg := ""
-		if cfg.GetMachineName() != constants.DefaultMachineName {
-			profileArg = fmt.Sprintf("-p %s", cfg.GetMachineName())
+		if old.MachineConfig.Name != constants.DefaultMachineName {
+			profileArg = fmt.Sprintf("-p %s", old.MachineConfig.Name)
 		}
 		exit.WithCodeT(exit.Config, `Error: You have selected Kubernetes v{{.new}}, but the existing cluster for your profile is running Kubernetes v{{.old}}. Non-destructive downgrades are not supported, but you can proceed by performing one of the following options:
 

cmd/minikube/cmd/status.go

@@ -88,7 +88,9 @@ var statusCmd = &cobra.Command{
 		}
 		defer api.Close()
 
-		hostSt, err := cluster.GetHostStatus(api)
+		machineName := viper.GetString(config.MachineProfile)
+
+		hostSt, err := cluster.GetHostStatus(api, machineName)
 		if err != nil {
 			exit.WithError("Error getting host status", err)
 		}
@@ -110,12 +112,12 @@ var statusCmd = &cobra.Command{
 				returnCode |= clusterNotRunningStatusFlag
 			}
 
-			ip, err := cluster.GetHostDriverIP(api, config.GetMachineName())
+			ip, err := cluster.GetHostDriverIP(api, machineName)
 			if err != nil {
 				glog.Errorln("Error host driver ip status:", err)
 			}
 
-			apiserverPort, err := kubeconfig.Port(config.GetMachineName())
+			apiserverPort, err := kubeconfig.Port(machineName)
 			if err != nil {
 				// Fallback to presuming default apiserver port
 				apiserverPort = constants.APIServerPort
@@ -128,7 +130,7 @@ var statusCmd = &cobra.Command{
 				returnCode |= clusterNotRunningStatusFlag
 			}
 
-			ks, err := kubeconfig.IsClusterInConfig(ip, config.GetMachineName())
+			ks, err := kubeconfig.IsClusterInConfig(ip, machineName)
 			if err != nil {
 				glog.Errorln("Error kubeconfig status:", err)
 			}

cmd/minikube/cmd/stop.go

@@ -76,8 +76,7 @@ func runStop(cmd *cobra.Command, args []string) {
 		out.T(out.WarningType, "Unable to kill mount process: {{.error}}", out.V{"error": err})
 	}
 
-	machineName := pkg_config.GetMachineName()
-	err = kubeconfig.UnsetCurrentContext(machineName, constants.KubeconfigPath)
+	err = kubeconfig.UnsetCurrentContext(profile, constants.KubeconfigPath)
 	if err != nil {
 		exit.WithError("update config", err)
 	}