Merge pull request #6471 from tstromberg/addons-startup2

Fix bugs that prevented previously-enabled addons from starting up

cmd/minikube/cmd/start.go

@@ -367,8 +367,12 @@ func runStart(cmd *cobra.Command, args []string) {
 	bootstrapCluster(bs, cr, mRunner, mc, preExists, isUpgrade)
 	configureMounts()
 
-	// enable addons
-	addons.Start(viper.GetString(config.MachineProfile), addonList)
+	// enable addons, both old and new!
+	existingAddons := map[string]bool{}
+	if existing != nil && existing.Addons != nil {
+		existingAddons = existing.Addons
+	}
+	addons.Start(viper.GetString(config.MachineProfile), existingAddons, addonList)
 
 	if err = cacheAndLoadImagesInConfig(); err != nil {
 		out.T(out.FailureType, "Unable to load cached images from config file.")

pkg/addons/addons.go

@@ -19,6 +19,7 @@ package addons
 import (
 	"fmt"
 	"path"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -68,8 +69,7 @@ func Set(name, value, profile string) error {
 		return errors.Wrap(err, "running callbacks")
 	}
 
-	glog.Infof("Writing new config for %q ...", profile)
-	// Write the value
+	glog.Infof("Writing out %q config to set %s=%v...", profile, name, value)
 	return config.Write(profile, c)
 }
 
@@ -263,33 +263,42 @@ func enableOrDisableStorageClasses(name, val, profile string) error {
 }
 
 // Start enables the default addons for a profile, plus any additional
-func Start(profile string, additional []string) {
+func Start(profile string, toEnable map[string]bool, additional []string) {
 	start := time.Now()
-	glog.Infof("enableAddons")
+	glog.Infof("enableAddons start: toEnable=%v, additional=%s", toEnable, additional)
 	defer func() {
 		glog.Infof("enableAddons completed in %s", time.Since(start))
 	}()
-	toEnable := []string{}
 
-	// Apply addons that are enabled by default
+	// Get the default values of any addons not saved to our config
 	for name, a := range assets.Addons {
-		enabled, err := a.IsEnabled(profile)
+		defaultVal, err := a.IsEnabled(profile)
 		if err != nil {
 			glog.Errorf("is-enabled failed for %q: %v", a.Name(), err)
 			continue
 		}
-		if enabled {
-			toEnable = append(toEnable, name)
+
+		_, exists := toEnable[name]
+		if !exists {
+			toEnable[name] = defaultVal
 		}
 	}
 
-	toEnable = append(toEnable, additional...)
-	if len(toEnable) == 0 {
-		return
+	// Apply new addons
+	for _, name := range additional {
+		toEnable[name] = true
 	}
 
-	out.T(out.AddonEnable, "Enabling addons: {{.addons}}", out.V{"addons": strings.Join(toEnable, ", ")})
-	for _, a := range toEnable {
+	toEnableList := []string{}
+	for k, v := range toEnable {
+		if v {
+			toEnableList = append(toEnableList, k)
+		}
+	}
+	sort.Strings(toEnableList)
+
+	out.T(out.AddonEnable, "Enabling addons: {{.addons}}", out.V{"addons": strings.Join(toEnableList, ", ")})
+	for _, a := range toEnableList {
 		err := Set(a, "true", profile)
 		if err != nil {
 			// Intentionally non-fatal

pkg/addons/addons_test.go

@@ -119,7 +119,7 @@ func TestEnableAndDisableAddon(t *testing.T) {
 
 func TestStart(t *testing.T) {
 	profile := createTestProfile(t)
-	Start(profile, []string{"dashboard"})
+	Start(profile, map[string]bool{}, []string{"dashboard"})
 
 	enabled, err := assets.Addons["dashboard"].IsEnabled(profile)
 	if err != nil {

pkg/minikube/cluster/cluster.go

@@ -191,6 +191,10 @@ func configureHost(h *host.Host, e *engine.Options) error {
 		glog.Infof("configureHost completed within %s", time.Since(start))
 	}()
 
+	if err := createRequiredDirectories(h); err != nil {
+		return errors.Wrap(err, "required directories")
+	}
+
 	if len(e.Env) > 0 {
 		h.HostOptions.EngineOptions.Env = e.Env
 		glog.Infof("Detecting provisioner ...")

test/integration/start_stop_delete_test.go

@@ -113,6 +113,12 @@ func TestStartStop(t *testing.T) {
 					}
 				}
 
+				// Enable an addon to assert it comes up afterwards
+				rr, err = Run(t, exec.CommandContext(ctx, Target(), "addons", "enable", "dashboard", "-p", profile))
+				if err != nil {
+					t.Errorf("%s failed: %v", rr.Args, err)
+				}
+
 				rr, err = Run(t, exec.CommandContext(ctx, Target(), startArgs...))
 				if err != nil {
 					// Explicit fatal so that failures don't move directly to deletion
@@ -121,8 +127,13 @@ func TestStartStop(t *testing.T) {
 
 				if strings.Contains(tc.name, "cni") {
 					t.Logf("WARNING: cni mode requires additional setup before pods can schedule :(")
-				} else if _, err := PodWait(ctx, t, profile, "default", "integration-test=busybox", 4*time.Minute); err != nil {
-					t.Fatalf("wait: %v", err)
+				} else {
+					if _, err := PodWait(ctx, t, profile, "default", "integration-test=busybox", 4*time.Minute); err != nil {
+						t.Fatalf("post-stop-start pod wait: %v", err)
+					}
+					if _, err := PodWait(ctx, t, profile, "kubernetes-dashboard", "k8s-app=kubernetes-dashboard", 4*time.Minute); err != nil {
+						t.Fatalf("post-stop-start addon wait: %v", err)
+					}
 				}
 
 				got := Status(ctx, t, Target(), profile, "Host")