Save old cluster config in memory before overwriting (#3450)

* Save old cluster config in memory before overwriting

In PR #3426, I changed "minikube start" to overwrite the cluster config earlier so that the container runtime could be extracted from it by the buildroot provisioner. This introduced a bug later on, where minikube expected to read the kubernetes version from theold config (which no longer existed, because the config was overwritten).

To fix this, I changed the code to store the old version of the config in memory before overwriting it.

This should fix #3447

Author: priyawadhwa

cmd/minikube/cmd/start.go

@@ -162,6 +162,12 @@ func runStart(cmd *cobra.Command, args []string) {
 		GPU:                 viper.GetBool(gpu),
 	}
 
+	// Load current profile cluster config from file, before overwriting it with the new state
+	oldConfig, err := cfg.Load()
+	if err != nil && !os.IsNotExist(err) {
+		glog.Errorln("Error loading profile config: ", err)
+	}
+
 	// Write profile cluster configuration to file
 	clusterConfig := cfg.Config{
 		MachineConfig: config,
@@ -198,14 +204,8 @@ func runStart(cmd *cobra.Command, args []string) {
 	if strings.Compare(selectedKubernetesVersion, "") == 0 {
 		selectedKubernetesVersion = constants.DefaultKubernetesVersion
 	}
-	// Load profile cluster config from file
-	cc, err := cfg.Load()
-	if err != nil && !os.IsNotExist(err) {
-		glog.Errorln("Error loading profile config: ", err)
-	}
-
-	if err == nil {
-		oldKubernetesVersion, err := semver.Make(strings.TrimPrefix(cc.KubernetesConfig.KubernetesVersion, version.VersionPrefix))
+	if oldConfig != nil {
+		oldKubernetesVersion, err := semver.Make(strings.TrimPrefix(oldConfig.KubernetesConfig.KubernetesVersion, version.VersionPrefix))
 		if err != nil {
 			glog.Errorln("Error parsing version semver: ", err)
 		}

pkg/minikube/config/config.go

@@ -92,35 +92,35 @@ func GetMachineName() string {
 }
 
 // Load loads the kubernetes and machine config for the current machine
-func Load() (Config, error) {
+func Load() (*Config, error) {
 	return DefaultLoader.LoadConfigFromFile(GetMachineName())
 }
 
 // Loader loads the kubernetes and machine config based on the machine profile name
 type Loader interface {
-	LoadConfigFromFile(profile string) (Config, error)
+	LoadConfigFromFile(profile string) (*Config, error)
 }
 
 type simpleConfigLoader struct{}
 
 var DefaultLoader Loader = &simpleConfigLoader{}
 
-func (c *simpleConfigLoader) LoadConfigFromFile(profile string) (Config, error) {
+func (c *simpleConfigLoader) LoadConfigFromFile(profile string) (*Config, error) {
 	var cc Config
 
 	path := constants.GetProfileFile(profile)
 
 	if _, err := os.Stat(path); os.IsNotExist(err) {
-		return cc, err
+		return nil, err
 	}
 
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
-		return cc, err
+		return nil, err
 	}
 
 	if err := json.Unmarshal(data, &cc); err != nil {
-		return cc, err
+		return nil, err
 	}
-	return cc, nil
+	return &cc, nil
 }

pkg/minikube/tunnel/cluster_inspector.go

@@ -63,15 +63,15 @@ func (m *clusterInspector) getStateAndRoute() (HostState, *Route, error) {
 	if err != nil {
 		return hostState, nil, err
 	}
-	var c config.Config
+	var c *config.Config
 	c, err = m.configLoader.LoadConfigFromFile(m.machineName)
 	if err != nil {
 		err = errors.Wrapf(err, "error loading config for %s", m.machineName)
 		return hostState, nil, err
 	}
 
 	var route *Route
-	route, err = getRoute(h, c)
+	route, err = getRoute(h, *c)
 	if err != nil {
 		err = errors.Wrapf(err, "error getting route info for %s", m.machineName)
 		return hostState, nil, err

pkg/minikube/tunnel/cluster_inspector_test.go

@@ -60,7 +60,7 @@ func TestMinikubeCheckReturnsHostInformation(t *testing.T) {
 	}
 
 	configLoader := &stubConfigLoader{
-		c: config.Config{
+		c: &config.Config{
 			KubernetesConfig: config.KubernetesConfig{
 				ServiceCIDR: "96.0.0.0/12",
 			},

pkg/minikube/tunnel/test_doubles.go

@@ -82,10 +82,10 @@ func (r *fakeRouter) Inspect(route *Route) (exists bool, conflict string, overla
 }
 
 type stubConfigLoader struct {
-	c config.Config
+	c *config.Config
 	e error
 }
 
-func (l *stubConfigLoader) LoadConfigFromFile(profile string) (config.Config, error) {
+func (l *stubConfigLoader) LoadConfigFromFile(profile string) (*config.Config, error) {
 	return l.c, l.e
 }

pkg/minikube/tunnel/tunnel_test.go

@@ -395,7 +395,7 @@ func TestTunnel(t *testing.T) {
 				},
 			}
 			configLoader := &stubConfigLoader{
-				c: config.Config{
+				c: &config.Config{
 					KubernetesConfig: config.KubernetesConfig{
 						ServiceCIDR: tc.serviceCIDR,
 					}},
@@ -446,7 +446,7 @@ func TestErrorCreatingTunnel(t *testing.T) {
 	}
 
 	configLoader := &stubConfigLoader{
-		c: config.Config{
+		c: &config.Config{
 			KubernetesConfig: config.KubernetesConfig{
 				ServiceCIDR: "10.96.0.0/12",
 			}},