Merge branch 'master' of github.com:kubernetes/minikube into driver-fallback

cmd/minikube/cmd/config/addons_list.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/exit"
+	"k8s.io/minikube/pkg/minikube/mustload"
 	"k8s.io/minikube/pkg/minikube/out"
 )
 
@@ -49,11 +50,12 @@ var addonsListCmd = &cobra.Command{
 			exit.UsageT("usage: minikube addons list")
 		}
 
+		_, cc := mustload.Partial(ClusterFlagValue())
 		switch strings.ToLower(addonListOutput) {
 		case "list":
-			printAddonsList()
+			printAddonsList(cc)
 		case "json":
-			printAddonsJSON()
+			printAddonsJSON(cc)
 		default:
 			exit.WithCodeT(exit.BadUsage, fmt.Sprintf("invalid output format: %s. Valid values: 'list', 'json'", addonListOutput))
 		}
@@ -85,27 +87,24 @@ var stringFromStatus = func(addonStatus bool) string {
 	return "disabled"
 }
 
-var printAddonsList = func() {
+var printAddonsList = func(cc *config.ClusterConfig) {
 	addonNames := make([]string, 0, len(assets.Addons))
 	for addonName := range assets.Addons {
 		addonNames = append(addonNames, addonName)
 	}
 	sort.Strings(addonNames)
+
 	var tData [][]string
 	table := tablewriter.NewWriter(os.Stdout)
 	table.SetHeader([]string{"Addon Name", "Profile", "Status"})
 	table.SetAutoFormatHeaders(true)
 	table.SetBorders(tablewriter.Border{Left: true, Top: true, Right: true, Bottom: true})
 	table.SetCenterSeparator("|")
-	pName := ClusterFlagValue()
 
 	for _, addonName := range addonNames {
 		addonBundle := assets.Addons[addonName]
-		addonStatus, err := addonBundle.IsEnabled(pName)
+		enabled := addonBundle.IsEnabled(cc)
-		if err != nil {
+		tData = append(tData, []string{addonName, cc.Name, fmt.Sprintf("%s %s", stringFromStatus(enabled), iconFromStatus(enabled))})
-			out.WarningT("Unable to get addon status for {{.name}}: {{.error}}", out.V{"name": addonName, "error": err})
-		}
-		tData = append(tData, []string{addonName, pName, fmt.Sprintf("%s %s", stringFromStatus(addonStatus), iconFromStatus(addonStatus))})
 	}
 
 	table.AppendBulk(tData)
@@ -120,9 +119,8 @@ var printAddonsList = func() {
 	}
 }
 
-var printAddonsJSON = func() {
+var printAddonsJSON = func(cc *config.ClusterConfig) {
 	addonNames := make([]string, 0, len(assets.Addons))
-	pName := ClusterFlagValue()
 	for addonName := range assets.Addons {
 		addonNames = append(addonNames, addonName)
 	}
@@ -132,16 +130,11 @@ var printAddonsJSON = func() {
 
 	for _, addonName := range addonNames {
 		addonBundle := assets.Addons[addonName]
-
+		enabled := addonBundle.IsEnabled(cc)
-		addonStatus, err := addonBundle.IsEnabled(pName)
-		if err != nil {
-			glog.Errorf("Unable to get addon status for %s: %v", addonName, err)
-			continue
-		}
 
 		addonsMap[addonName] = map[string]interface{}{
-			"Status":  stringFromStatus(addonStatus),
+			"Status":  stringFromStatus(enabled),
-			"Profile": pName,
+			"Profile": cc.Name,
 		}
 	}
 	jsonString, _ := json.Marshal(addonsMap)

cmd/minikube/cmd/config/disable.go

@@ -33,7 +33,7 @@ var addonsDisableCmd = &cobra.Command{
 		}
 
 		addon := args[0]
-		err := addons.Set(addon, "false", ClusterFlagValue())
+		err := addons.SetAndSave(ClusterFlagValue(), addon, "false")
 		if err != nil {
 			exit.WithError("disable failed", err)
 		}

cmd/minikube/cmd/config/enable.go

@@ -32,7 +32,7 @@ var addonsEnableCmd = &cobra.Command{
 			exit.UsageT("usage: minikube addons enable ADDON_NAME")
 		}
 		addon := args[0]
-		err := addons.Set(addon, "true", ClusterFlagValue())
+		err := addons.SetAndSave(ClusterFlagValue(), addon, "true")
 		if err != nil {
 			exit.WithError("enable failed", err)
 		}

cmd/minikube/cmd/config/open.go

@@ -66,11 +66,9 @@ var addonsOpenCmd = &cobra.Command{
 To see the list of available addons run:
 minikube addons list`, out.V{"name": addonName})
 		}
-		ok, err := addon.IsEnabled(cname)
+
-		if err != nil {
+		enabled := addon.IsEnabled(co.Config)
-			exit.WithError("IsEnabled failed", err)
+		if !enabled {
-		}
-		if !ok {
 			exit.WithCodeT(exit.Unavailable, `addon '{{.name}}' is currently not enabled.
 To enable this addon run:
 minikube addons enable {{.name}}`, out.V{"name": addonName})

cmd/minikube/cmd/dashboard.go

@@ -67,13 +67,14 @@ var dashboardCmd = &cobra.Command{
 		var err error
 
 		// Check dashboard status before enabling it
-		dashboardAddon := assets.Addons["dashboard"]
+		addon := assets.Addons["dashboard"]
-		dashboardStatus, _ := dashboardAddon.IsEnabled(cname)
+		enabled := addon.IsEnabled(co.Config)
-		if !dashboardStatus {
+
+		if !enabled {
 			// Send status messages to stderr for folks re-using this output.
 			out.ErrT(out.Enabling, "Enabling dashboard ...")
 			// Enable the dashboard add-on
-			err = addons.Set("dashboard", "true", cname)
+			err = addons.SetAndSave(cname, "dashboard", "true")
 			if err != nil {
 				exit.WithError("Unable to enable dashboard", err)
 			}

pkg/addons/addons.go

@@ -23,6 +23,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/golang/glog"
@@ -41,32 +42,49 @@ import (
 // defaultStorageClassProvisioner is the name of the default storage class provisioner
 const defaultStorageClassProvisioner = "standard"
 
-// Set sets a value
+// RunCallbacks runs all actions associated to an addon, but does not set it (thread-safe)
-func Set(name, value, profile string) error {
+func RunCallbacks(cc *config.ClusterConfig, name string, value string) error {
-	glog.Infof("Setting %s=%s in profile %q", name, value, profile)
+	glog.Infof("Setting %s=%s in profile %q", name, value, cc.Name)
 	a, valid := isAddonValid(name)
 	if !valid {
 		return errors.Errorf("%s is not a valid addon", name)
 	}
 
-	cc, err := config.Load(profile)
-	if err != nil {
-		return errors.Wrap(err, "loading profile")
-	}
-
 	// Run any additional validations for this property
 	if err := run(cc, name, value, a.validations); err != nil {
 		return errors.Wrap(err, "running validations")
 	}
 
-	if err := a.set(cc, name, value); err != nil {
-		return errors.Wrap(err, "setting new value of addon")
-	}
-
 	// Run any callbacks for this property
 	if err := run(cc, name, value, a.callbacks); err != nil {
 		return errors.Wrap(err, "running callbacks")
 	}
+	return nil
+}
+
+// Set sets a value in the config (not threadsafe)
+func Set(cc *config.ClusterConfig, name string, value string) error {
+	a, valid := isAddonValid(name)
+	if !valid {
+		return errors.Errorf("%s is not a valid addon", name)
+	}
+	return a.set(cc, name, value)
+}
+
+// SetAndSave sets a value and saves the config
+func SetAndSave(profile string, name string, value string) error {
+	cc, err := config.Load(profile)
+	if err != nil {
+		return errors.Wrap(err, "loading profile")
+	}
+
+	if err := RunCallbacks(cc, name, value); err != nil {
+		return errors.Wrap(err, "run callbacks")
+	}
+
+	if err := Set(cc, name, value); err != nil {
+		return errors.Wrap(err, "set")
+	}
 
 	glog.Infof("Writing out %q config to set %s=%v...", profile, name, value)
 	return config.Write(profile, cc)
@@ -87,7 +105,7 @@ func run(cc *config.ClusterConfig, name string, value string, fns []setFn) error
 	return nil
 }
 
-// SetBool sets a bool value
+// SetBool sets a bool value in the config (not threadsafe)
 func SetBool(cc *config.ClusterConfig, name string, val string) error {
 	b, err := strconv.ParseBool(val)
 	if err != nil {
@@ -110,13 +128,7 @@ func enableOrDisableAddon(cc *config.ClusterConfig, name string, val string) err
 	addon := assets.Addons[name]
 
 	// check addon status before enabling/disabling it
-	alreadySet, err := isAddonAlreadySet(addon, enable, cc.Name)
+	if isAddonAlreadySet(cc, addon, enable) {
-	if err != nil {
-		out.ErrT(out.Conflict, "{{.error}}", out.V{"error": err})
-		return err
-	}
-
-	if alreadySet {
 		glog.Warningf("addon %s should already be in state %v", name, val)
 		if !enable {
 			return nil
@@ -160,7 +172,7 @@ https://github.com/kubernetes/minikube/issues/7332`, out.V{"driver_name": cc.Dri
 	mName := driver.MachineName(*cc, cp)
 	host, err := machine.LoadHost(api, mName)
 	if err != nil || !machine.IsRunning(api, mName) {
-		glog.Warningf("%q is not running, writing %s=%v to disk and skipping enablement (err=%v)", mName, addon.Name(), enable, err)
+		glog.Warningf("%q is not running, setting %s=%v and skipping enablement (err=%v)", mName, addon.Name(), enable, err)
 		return nil
 	}
 
@@ -173,19 +185,17 @@ https://github.com/kubernetes/minikube/issues/7332`, out.V{"driver_name": cc.Dri
 	return enableOrDisableAddonInternal(cc, addon, cmd, data, enable)
 }
 
-func isAddonAlreadySet(addon *assets.Addon, enable bool, profile string) (bool, error) {
+func isAddonAlreadySet(cc *config.ClusterConfig, addon *assets.Addon, enable bool) bool {
-	addonStatus, err := addon.IsEnabled(profile)
+	enabled := addon.IsEnabled(cc)
-	if err != nil {
+	if enabled && enable {
-		return false, errors.Wrap(err, "is enabled")
+		return true
 	}
 
-	if addonStatus && enable {
+	if !enabled && !enable {
-		return true, nil
+		return true
-	} else if !addonStatus && !enable {
-		return true, nil
 	}
 
-	return false, nil
+	return false
 }
 
 func enableOrDisableAddonInternal(cc *config.ClusterConfig, addon *assets.Addon, cmd command.Runner, data interface{}, enable bool) error {
@@ -287,7 +297,10 @@ func enableOrDisableStorageClasses(cc *config.ClusterConfig, name string, val st
 }
 
 // Start enables the default addons for a profile, plus any additional
-func Start(profile string, toEnable map[string]bool, additional []string) {
+func Start(wg *sync.WaitGroup, cc *config.ClusterConfig, toEnable map[string]bool, additional []string) {
+	wg.Add(1)
+	defer wg.Done()
+
 	start := time.Now()
 	glog.Infof("enableAddons start: toEnable=%v, additional=%s", toEnable, additional)
 	defer func() {
@@ -296,11 +309,7 @@ func Start(profile string, toEnable map[string]bool, additional []string) {
 
 	// Get the default values of any addons not saved to our config
 	for name, a := range assets.Addons {
-		defaultVal, err := a.IsEnabled(profile)
+		defaultVal := a.IsEnabled(cc)
-		if err != nil {
-			glog.Errorf("is-enabled failed for %q: %v", a.Name(), err)
-			continue
-		}
 
 		_, exists := toEnable[name]
 		if !exists {
@@ -321,12 +330,25 @@ func Start(profile string, toEnable map[string]bool, additional []string) {
 	}
 	sort.Strings(toEnableList)
 
+	var awg sync.WaitGroup
+
 	out.T(out.AddonEnable, "Enabling addons: {{.addons}}", out.V{"addons": strings.Join(toEnableList, ", ")})
 	for _, a := range toEnableList {
-		err := Set(a, "true", profile)
+		awg.Add(1)
-		if err != nil {
+		go func(name string) {
-			// Intentionally non-fatal
+			err := RunCallbacks(cc, name, "true")
-			out.WarningT("Enabling '{{.name}}' returned an error: {{.error}}", out.V{"name": a, "error": err})
+			if err != nil {
+				out.WarningT("Enabling '{{.name}}' returned an error: {{.error}}", out.V{"name": name, "error": err})
+			}
+			awg.Done()
+		}(a)
+	}
+
+	// Wait until all of the addons are enabled before updating the config (not thread safe)
+	awg.Wait()
+	for _, a := range toEnableList {
+		if err := Set(cc, a, "true"); err != nil {
+			glog.Errorf("store failed: %v", err)
 		}
 	}
 }

pkg/addons/addons_test.go

@@ -20,6 +20,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 
 	"k8s.io/minikube/pkg/minikube/assets"
@@ -59,47 +60,42 @@ func createTestProfile(t *testing.T) string {
 }
 
 func TestIsAddonAlreadySet(t *testing.T) {
-	profile := createTestProfile(t)
+	cc := &config.ClusterConfig{Name: "test"}
-	if err := Set("registry", "true", profile); err != nil {
+
+	if err := Set(cc, "registry", "true"); err != nil {
 		t.Errorf("unable to set registry true: %v", err)
 	}
-	enabled, err := assets.Addons["registry"].IsEnabled(profile)
+	if !assets.Addons["registry"].IsEnabled(cc) {
-	if err != nil {
-		t.Errorf("registry: %v", err)
-	}
-	if !enabled {
 		t.Errorf("expected registry to be enabled")
 	}
 
-	enabled, err = assets.Addons["ingress"].IsEnabled(profile)
+	if assets.Addons["ingress"].IsEnabled(cc) {
-	if err != nil {
-		t.Errorf("ingress: %v", err)
-	}
-	if enabled {
 		t.Errorf("expected ingress to not be enabled")
 	}
 
 }
 
 func TestDisableUnknownAddon(t *testing.T) {
-	profile := createTestProfile(t)
+	cc := &config.ClusterConfig{Name: "test"}
-	if err := Set("InvalidAddon", "false", profile); err == nil {
+
+	if err := Set(cc, "InvalidAddon", "false"); err == nil {
 		t.Fatalf("Disable did not return error for unknown addon")
 	}
 }
 
 func TestEnableUnknownAddon(t *testing.T) {
-	profile := createTestProfile(t)
+	cc := &config.ClusterConfig{Name: "test"}
-	if err := Set("InvalidAddon", "true", profile); err == nil {
+
+	if err := Set(cc, "InvalidAddon", "true"); err == nil {
 		t.Fatalf("Enable did not return error for unknown addon")
 	}
 }
 
-func TestEnableAndDisableAddon(t *testing.T) {
+func TestSetAndSave(t *testing.T) {
 	profile := createTestProfile(t)
 
 	// enable
-	if err := Set("dashboard", "true", profile); err != nil {
+	if err := SetAndSave(profile, "dashboard", "true"); err != nil {
 		t.Errorf("Disable returned unexpected error: " + err.Error())
 	}
 
@@ -112,7 +108,7 @@ func TestEnableAndDisableAddon(t *testing.T) {
 	}
 
 	// disable
-	if err := Set("dashboard", "false", profile); err != nil {
+	if err := SetAndSave(profile, "dashboard", "false"); err != nil {
 		t.Errorf("Disable returned unexpected error: " + err.Error())
 	}
 
@@ -126,14 +122,18 @@ func TestEnableAndDisableAddon(t *testing.T) {
 }
 
 func TestStart(t *testing.T) {
-	profile := createTestProfile(t)
+	cc := &config.ClusterConfig{
-	Start(profile, map[string]bool{}, []string{"dashboard"})
+		Name:             "start",
-
+		CPUs:             2,
-	enabled, err := assets.Addons["dashboard"].IsEnabled(profile)
+		Memory:           2500,
-	if err != nil {
+		KubernetesConfig: config.KubernetesConfig{},
-		t.Errorf("dashboard: %v", err)
 	}
-	if !enabled {
+
+	var wg sync.WaitGroup
+	Start(&wg, cc, map[string]bool{}, []string{"dashboard"})
+	wg.Wait()
+
+	if !assets.Addons["dashboard"].IsEnabled(cc) {
 		t.Errorf("expected dashboard to be enabled")
 	}
 }

pkg/minikube/assets/addons.go

@@ -19,8 +19,6 @@ package assets
 import (
 	"runtime"
 
-	"github.com/golang/glog"
-	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/vmpath"
@@ -49,21 +47,14 @@ func (a *Addon) Name() string {
 }
 
 // IsEnabled checks if an Addon is enabled for the given profile
-func (a *Addon) IsEnabled(profile string) (bool, error) {
+func (a *Addon) IsEnabled(cc *config.ClusterConfig) bool {
-	c, err := config.Load(profile)
+	status, ok := cc.Addons[a.Name()]
-	if err != nil {
-		return false, errors.Wrap(err, "load")
-	}
-
-	// Is this addon explicitly listed in their configuration?
-	status, ok := c.Addons[a.Name()]
-	glog.V(1).Infof("IsEnabled %q = %v (listed in config=%v)", a.Name(), status, ok)
 	if ok {
-		return status, nil
+		return status
 	}
 
 	// Return the default unconfigured state of the addon
-	return a.enabled, nil
+	return a.enabled
 }
 
 // Addons is the list of addons

pkg/minikube/exit/exit.go

@@ -20,6 +20,7 @@ package exit
 import (
 	"os"
 	"runtime"
+	"runtime/debug"
 
 	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/problem"
@@ -53,6 +54,7 @@ func WithCodeT(code int, format string, a ...out.V) {
 
 // WithError outputs an error and exits.
 func WithError(msg string, err error) {
+	glog.Infof("WithError(%s)=%v called from:\n%s", msg, err, debug.Stack())
 	p := problem.FromError(err, runtime.GOOS)
 	if p != nil {
 		WithProblem(msg, err, p)

pkg/minikube/node/config.go

@@ -22,6 +22,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"sync"
 
 	"github.com/golang/glog"
 	"github.com/spf13/viper"
@@ -46,7 +47,10 @@ func showVersionInfo(k8sVersion string, cr cruntime.Manager) {
 }
 
 // configureMounts configures any requested filesystem mounts
-func configureMounts() {
+func configureMounts(wg *sync.WaitGroup) {
+	wg.Add(1)
+	defer wg.Done()
+
 	if !viper.GetBool(createMount) {
 		return
 	}

pkg/minikube/node/start.go

@@ -134,15 +134,20 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) {
 		}
 	}
 
-	configureMounts()
+	var wg sync.WaitGroup
+	go configureMounts(&wg)
 
-	if err := CacheAndLoadImagesInConfig(); err != nil {
+	wg.Add(1)
-		out.FailureT("Unable to load cached images from config file.")
+	go func() {
-	}
+		if err := CacheAndLoadImagesInConfig(); err != nil {
+			out.FailureT("Unable to load cached images from config file: {{error}}", out.V{"error": err})
+		}
+		wg.Done()
+	}()
 
 	// enable addons, both old and new!
 	if starter.ExistingAddons != nil {
-		addons.Start(viper.GetString(config.ProfileName), starter.ExistingAddons, config.AddonList)
+		go addons.Start(&wg, &cc, starter.ExistingAddons, config.AddonList)
 	}
 
 	if apiServer {
@@ -182,7 +187,10 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) {
 		}
 	}
 
-	return kcs, nil
+	wg.Wait()
+
+	// Write enabled addons to the config before completion
+	return kcs, config.Write(viper.GetString(config.ProfileName), &cc)
 }
 
 // Provision provisions the machine/container for the node