From ac1b3b0c59957139f8c0bafb1ef5f0af3238eb6f Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 1 Apr 2020 17:32:36 -0700 Subject: [PATCH 01/13] merge --- cmd/minikube/cmd/start.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 56326fa41f72..61965e87cbd7 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -304,11 +304,15 @@ func runStart(cmd *cobra.Command, args []string) { } validateSpecifiedDriver(existing) - ds := selectDriver(existing) + ds, alts := selectDriver(existing) + +} + +func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *config.ClusterConfig) error { driverName := ds.Name glog.Infof("selected driver: %s", driverName) validateDriver(ds, existing) - err = autoSetDriverOptions(cmd, driverName) + err := autoSetDriverOptions(cmd, driverName) if err != nil { glog.Errorf("Error autoSetOptions : %v", err) } @@ -330,7 +334,7 @@ func runStart(cmd *cobra.Command, args []string) { // This is about as far as we can go without overwriting config files if viper.GetBool(dryRun) { out.T(out.DryRun, `dry-run validation complete!`) - return + return nil } if driver.IsVM(driverName) { @@ -388,6 +392,8 @@ func runStart(cmd *cobra.Command, args []string) { if err := showKubectlInfo(kubeconfig, k8sVersion, cc.Name); err != nil { glog.Errorf("kubectl info: %v", err) } + + return nil } func updateDriver(driverName string) { @@ -519,7 +525,7 @@ func kubectlVersion(path string) (string, error) { return cv.ClientVersion.GitVersion, nil } -func selectDriver(existing *config.ClusterConfig) registry.DriverState { +func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []registry.DriverState) { // Technically unrelated, but important to perform before detection driver.SetLibvirtURI(viper.GetString(kvmQemuURI)) @@ -528,7 +534,7 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState { old := hostDriver(existing) ds := driver.Status(old) out.T(out.Sparkle, `Using the {{.driver}} driver based on existing profile`, out.V{"driver": ds.String()}) - return ds + return ds, nil } // Default to looking at the new driver parameter @@ -548,7 +554,7 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState { exit.WithCodeT(exit.Unavailable, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": d, "os": runtime.GOOS}) } out.T(out.Sparkle, `Using the {{.driver}} driver based on user configuration`, out.V{"driver": ds.String()}) - return ds + return ds, nil } // Fallback to old driver parameter @@ -558,7 +564,7 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState { exit.WithCodeT(exit.Unavailable, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": d, "os": runtime.GOOS}) } out.T(out.Sparkle, `Using the {{.driver}} driver based on user configuration`, out.V{"driver": ds.String()}) - return ds + return ds, nil } pick, alts := driver.Suggest(driver.Choices(viper.GetBool("vm"))) @@ -575,7 +581,7 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState { } else { out.T(out.Sparkle, `Automatically selected the {{.driver}} driver`, out.V{"driver": pick.String()}) } - return pick + return pick, alts } // hostDriver returns the actual driver used by a libmachine host, which can differ from our config From 26c747137e4f5d20a0642afc86165f8bb6fafb13 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 2 Apr 2020 14:36:46 -0700 Subject: [PATCH 02/13] failback to alternate drivers if startup fails with automatic choice --- cmd/minikube/cmd/start.go | 65 ++++++++++++++++++++++++++++---------- go.mod | 1 + pkg/minikube/exit/exit.go | 34 +------------------- pkg/minikube/node/start.go | 5 +-- pkg/minikube/out/out.go | 30 ++++++++++++++++++ 5 files changed, 84 insertions(+), 51 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 61965e87cbd7..a00c265701df 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "math" + "math/rand" "net" "net/url" "os" @@ -304,11 +305,41 @@ func runStart(cmd *cobra.Command, args []string) { } validateSpecifiedDriver(existing) - ds, alts := selectDriver(existing) + ds, alts, specified := selectDriver(existing) + err = startWithDriver(cmd, ds, existing) + if err != nil { + if specified { + // User specified an invalid or broken driver + exit.WithCodeT(exit.Failure, "Startup with {{.driver}} driver failed: {{.error}}", out.V{"driver": ds.Name, "error": err}) + } + + // Walk down the rest of the options + for _, alt := range alts { + out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}.", out.V{"old_driver": ds.Name, "new_driver": alt.Name}) + ds = alt + // Delete the existing cluster and try again with the next driver on the list + profile, err := config.LoadProfile(ClusterFlagValue()) + if err != nil { + out.ErrT(out.Meh, `"{{.name}}" profile does not exist, trying anyways.`, out.V{"name": ClusterFlagValue()}) + } + + err = deleteProfile(profile) + if err != nil { + out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": ClusterFlagValue()}) + } + err = startWithDriver(cmd, ds, existing) + if err != nil { + continue + } + } + } } func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *config.ClusterConfig) error { + if rand.Int()%2 == 0 { + return errors.New("OH NO RANDOM FAILURE") + } driverName := ds.Name glog.Infof("selected driver: %s", driverName) validateDriver(ds, existing) @@ -328,7 +359,7 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf k8sVersion := getKubernetesVersion(existing) cc, n, err := generateCfgFromFlags(cmd, k8sVersion, driverName) if err != nil { - exit.WithError("Failed to generate config", err) + return errors.Wrap(err, "Failed to generate config") } // This is about as far as we can go without overwriting config files @@ -340,7 +371,7 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf if driver.IsVM(driverName) { url, err := download.ISO(viper.GetStringSlice(isoURL), cmd.Flags().Changed(isoURL)) if err != nil { - exit.WithError("Failed to cache ISO", err) + return errors.Wrap(err, "Failed to cache ISO") } cc.MinikubeISO = url } @@ -361,7 +392,10 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf kubeconfig, err := node.Start(cc, n, existingAddons, true) if err != nil { - kubeconfig = maybeDeleteAndRetry(cc, n, existingAddons, err) + kubeconfig, err = maybeDeleteAndRetry(cc, n, existingAddons, err) + if err != nil { + return err + } } numNodes := viper.GetInt(nodes) @@ -383,7 +417,7 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf out.Ln("") // extra newline for clarity on the command line err := node.Add(&cc, n) if err != nil { - exit.WithError("adding node", err) + return errors.Wrap(err, "adding node") } } } @@ -468,9 +502,9 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string, machineName st return nil } -func maybeDeleteAndRetry(cc config.ClusterConfig, n config.Node, existingAddons map[string]bool, originalErr error) *kubeconfig.Settings { +func maybeDeleteAndRetry(cc config.ClusterConfig, n config.Node, existingAddons map[string]bool, originalErr error) (*kubeconfig.Settings, error) { if viper.GetBool(deleteOnFailure) { - out.T(out.Warning, "Node {{.name}} failed to start, deleting and trying again.", out.V{"name": n.Name}) + out.WarningT("Node {{.name}} failed to start, deleting and trying again.", out.V{"name": n.Name}) // Start failed, delete the cluster and try again profile, err := config.LoadProfile(cc.Name) if err != nil { @@ -490,14 +524,13 @@ func maybeDeleteAndRetry(cc config.ClusterConfig, n config.Node, existingAddons } if err != nil { // Ok we failed again, let's bail - exit.WithError("Start failed after cluster deletion", err) + return nil, err } } - return kubeconfig + return kubeconfig, nil } // Don't delete the cluster unless they ask - exit.WithError("startup failed", originalErr) - return nil + return nil, errors.Wrap(originalErr, "startup failed") } func kubectlVersion(path string) (string, error) { @@ -525,7 +558,7 @@ func kubectlVersion(path string) (string, error) { return cv.ClientVersion.GitVersion, nil } -func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []registry.DriverState) { +func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []registry.DriverState, bool) { // Technically unrelated, but important to perform before detection driver.SetLibvirtURI(viper.GetString(kvmQemuURI)) @@ -534,7 +567,7 @@ func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []regis old := hostDriver(existing) ds := driver.Status(old) out.T(out.Sparkle, `Using the {{.driver}} driver based on existing profile`, out.V{"driver": ds.String()}) - return ds, nil + return ds, nil, true } // Default to looking at the new driver parameter @@ -554,7 +587,7 @@ func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []regis exit.WithCodeT(exit.Unavailable, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": d, "os": runtime.GOOS}) } out.T(out.Sparkle, `Using the {{.driver}} driver based on user configuration`, out.V{"driver": ds.String()}) - return ds, nil + return ds, nil, true } // Fallback to old driver parameter @@ -564,7 +597,7 @@ func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []regis exit.WithCodeT(exit.Unavailable, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": d, "os": runtime.GOOS}) } out.T(out.Sparkle, `Using the {{.driver}} driver based on user configuration`, out.V{"driver": ds.String()}) - return ds, nil + return ds, nil, true } pick, alts := driver.Suggest(driver.Choices(viper.GetBool("vm"))) @@ -581,7 +614,7 @@ func selectDriver(existing *config.ClusterConfig) (registry.DriverState, []regis } else { out.T(out.Sparkle, `Automatically selected the {{.driver}} driver`, out.V{"driver": pick.String()}) } - return pick, alts + return pick, alts, false } // hostDriver returns the actual driver used by a libmachine host, which can differ from our config diff --git a/go.mod b/go.mod index 29a5ac30d5c2..2a00c7499246 100644 --- a/go.mod +++ b/go.mod @@ -82,6 +82,7 @@ require ( google.golang.org/genproto v0.0.0-20200117163144-32f20d992d24 // indirect google.golang.org/grpc v1.26.0 // indirect gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect + gopkg.in/yaml.v2 v2.2.8 gotest.tools/v3 v3.0.2 // indirect k8s.io/api v0.17.3 k8s.io/apimachinery v0.17.3 diff --git a/pkg/minikube/exit/exit.go b/pkg/minikube/exit/exit.go index 221001f86ea3..77b2cb5c4417 100644 --- a/pkg/minikube/exit/exit.go +++ b/pkg/minikube/exit/exit.go @@ -18,14 +18,11 @@ limitations under the License. package exit import ( - "fmt" "os" "runtime" - "github.com/golang/glog" "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/minikube/problem" - "k8s.io/minikube/pkg/minikube/translate" ) // Exit codes based on sysexits(3) @@ -40,9 +37,6 @@ const ( IO = 74 // IO represents an I/O error Config = 78 // Config represents an unconfigured or misconfigured state Permissions = 77 // Permissions represents a permissions error - - // MaxLogEntries controls the number of log entries to show for each source - MaxLogEntries = 3 ) // UsageT outputs a templated usage error and exits with error code 64 @@ -63,7 +57,7 @@ func WithError(msg string, err error) { if p != nil { WithProblem(msg, err, p) } - displayError(msg, err) + out.DisplayError(msg, err) os.Exit(Software) } @@ -79,29 +73,3 @@ func WithProblem(msg string, err error, p *problem.Problem) { } os.Exit(Config) } - -// WithLogEntries outputs an error along with any important log entries, and exits. -func WithLogEntries(msg string, err error, entries map[string][]string) { - displayError(msg, err) - - for name, lines := range entries { - out.T(out.FailureType, "Problems detected in {{.entry}}:", out.V{"entry": name}) - if len(lines) > MaxLogEntries { - lines = lines[:MaxLogEntries] - } - for _, l := range lines { - out.T(out.LogEntry, l) - } - } - os.Exit(Software) -} - -func displayError(msg string, err error) { - // use Warning because Error will display a duplicate message to stderr - glog.Warningf(fmt.Sprintf("%s: %v", msg, err)) - out.ErrT(out.Empty, "") - out.FatalT("{{.msg}}: {{.err}}", out.V{"msg": translate.T(msg), "err": err}) - out.ErrT(out.Empty, "") - out.ErrT(out.Sad, "minikube is exiting due to an error. If the above message is not useful, open an issue:") - out.ErrT(out.URL, "https://github.com/kubernetes/minikube/issues/new/choose") -} diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 37202613f7ea..5b66e1286a88 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -86,7 +86,7 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo // Abstraction leakage alert: startHost requires the config to be saved, to satistfy pkg/provision/buildroot. // Hence, saveConfig must be called before startHost, and again afterwards when we know the IP. if err := config.SaveProfile(viper.GetString(config.ProfileName), &cc); err != nil { - exit.WithError("Failed to save config", err) + return nil, errors.Wrap(err, "Failed to save config") } handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion) @@ -120,7 +120,8 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo bs = setupKubeAdm(machineAPI, cc, n) err = bs.StartCluster(cc) if err != nil { - exit.WithLogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, cc, mRunner)) + out.LogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, cc, mRunner)) + return nil, err } // write the kubeconfig to the file system after everything required (like certs) are created by the bootstrapper diff --git a/pkg/minikube/out/out.go b/pkg/minikube/out/out.go index 38c817339bc5..253eb09b4e9a 100644 --- a/pkg/minikube/out/out.go +++ b/pkg/minikube/out/out.go @@ -26,6 +26,7 @@ import ( "github.com/golang/glog" isatty "github.com/mattn/go-isatty" + "k8s.io/minikube/pkg/minikube/translate" ) // By design, this package uses global references to language and output objects, in preference @@ -51,6 +52,9 @@ var ( OverrideEnv = "MINIKUBE_IN_STYLE" ) +// MaxLogEntries controls the number of log entries to show for each source +const MaxLogEntries = 3 + // fdWriter is the subset of file.File that implements io.Writer and Fd() type fdWriter interface { io.Writer @@ -175,3 +179,29 @@ func wantsColor(fd uintptr) bool { glog.Infof("isatty.IsTerminal(%d) = %v\n", fd, isT) return isT } + +// LogEntries outputs an error along with any important log entries. +func LogEntries(msg string, err error, entries map[string][]string) { + DisplayError(msg, err) + + for name, lines := range entries { + T(FailureType, "Problems detected in {{.entry}}:", V{"entry": name}) + if len(lines) > MaxLogEntries { + lines = lines[:MaxLogEntries] + } + for _, l := range lines { + T(LogEntry, l) + } + } +} + +// DisplayError +func DisplayError(msg string, err error) { + // use Warning because Error will display a duplicate message to stderr + glog.Warningf(fmt.Sprintf("%s: %v", msg, err)) + ErrT(Empty, "") + FatalT("{{.msg}}: {{.err}}", V{"msg": translate.T(msg), "err": err}) + ErrT(Empty, "") + ErrT(Sad, "minikube is exiting due to an error. If the above message is not useful, open an issue:") + ErrT(URL, "https://github.com/kubernetes/minikube/issues/new/choose") +} From dd957ff2f7b6182ad5100aff1ed8041a518a88c9 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 2 Apr 2020 14:47:18 -0700 Subject: [PATCH 03/13] make sure to print error on failure --- cmd/minikube/cmd/start.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 2361c7fe4561..08bccc971d86 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -307,12 +307,7 @@ func runStart(cmd *cobra.Command, args []string) { validateSpecifiedDriver(existing) ds, alts, specified := selectDriver(existing) err = startWithDriver(cmd, ds, existing) - if err != nil { - if specified { - // User specified an invalid or broken driver - exit.WithCodeT(exit.Failure, "Startup with {{.driver}} driver failed: {{.error}}", out.V{"driver": ds.Name, "error": err}) - } - + if err != nil && !specified { // Walk down the rest of the options for _, alt := range alts { out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}.", out.V{"old_driver": ds.Name, "new_driver": alt.Name}) @@ -330,10 +325,16 @@ func runStart(cmd *cobra.Command, args []string) { err = startWithDriver(cmd, ds, existing) if err != nil { continue + } else { + // Success! + os.Exit(0) } } } + // Use the most recent error + exit.WithError("startup failed", err) + } func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *config.ClusterConfig) error { From b66447ea13408330fd50a70df2a190d2ba76650d Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 2 Apr 2020 14:48:28 -0700 Subject: [PATCH 04/13] remove random failure for testing --- cmd/minikube/cmd/start.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 08bccc971d86..f41ac630306c 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" "math" - "math/rand" "net" "net/url" "os" @@ -338,9 +337,6 @@ func runStart(cmd *cobra.Command, args []string) { } func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *config.ClusterConfig) error { - if rand.Int()%2 == 0 { - return errors.New("OH NO RANDOM FAILURE") - } driverName := ds.Name glog.Infof("selected driver: %s", driverName) validateDriver(ds, existing) From 31f225d90e25be34288ba4e341d10483949c4fa9 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 2 Apr 2020 15:16:11 -0700 Subject: [PATCH 05/13] fix lint --- cmd/minikube/cmd/node_add.go | 6 +++++- cmd/minikube/cmd/node_start.go | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cmd/minikube/cmd/node_add.go b/cmd/minikube/cmd/node_add.go index fa0f96bb3e98..33440657943d 100644 --- a/cmd/minikube/cmd/node_add.go +++ b/cmd/minikube/cmd/node_add.go @@ -20,6 +20,7 @@ import ( "github.com/spf13/cobra" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/driver" + "k8s.io/minikube/pkg/minikube/exit" "k8s.io/minikube/pkg/minikube/mustload" "k8s.io/minikube/pkg/minikube/node" "k8s.io/minikube/pkg/minikube/out" @@ -54,7 +55,10 @@ var nodeAddCmd = &cobra.Command{ } if err := node.Add(cc, n); err != nil { - maybeDeleteAndRetry(*cc, n, nil, err) + _, err := maybeDeleteAndRetry(*cc, n, nil, err) + if err != nil { + exit.WithError("failed to add node", err) + } } out.T(out.Ready, "Successfully added {{.name}} to {{.cluster}}!", out.V{"name": name, "cluster": cc.Name}) diff --git a/cmd/minikube/cmd/node_start.go b/cmd/minikube/cmd/node_start.go index 3b2782263bab..f6e70d9b8485 100644 --- a/cmd/minikube/cmd/node_start.go +++ b/cmd/minikube/cmd/node_start.go @@ -51,7 +51,10 @@ var nodeStartCmd = &cobra.Command{ _, err = node.Start(*cc, *n, nil, false) if err != nil { - maybeDeleteAndRetry(*cc, *n, nil, err) + _, err := maybeDeleteAndRetry(*cc, *n, nil, err) + if err != nil { + exit.WithError("failed to start node", err) + } } }, } From 3d037a887164f157d2ec149943a7a6959cc2420b Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 3 Apr 2020 17:46:42 -0700 Subject: [PATCH 06/13] only failback to next driver if provisioning fails --- cmd/minikube/cmd/node_start.go | 17 ++++- cmd/minikube/cmd/start.go | 92 +++++++++++++++++------- pkg/minikube/node/node.go | 16 ++++- pkg/minikube/node/start.go | 123 +++++++++++++++++++-------------- 4 files changed, 167 insertions(+), 81 deletions(-) diff --git a/cmd/minikube/cmd/node_start.go b/cmd/minikube/cmd/node_start.go index f6e70d9b8485..0b4a918ce1e6 100644 --- a/cmd/minikube/cmd/node_start.go +++ b/cmd/minikube/cmd/node_start.go @@ -49,7 +49,22 @@ var nodeStartCmd = &cobra.Command{ exit.WithError("retrieving node", err) } - _, err = node.Start(*cc, *n, nil, false) + r, p, m, h, err := node.Provision(*cc, *n, false) + if err != nil { + exit.WithError("provisioning host for node", err) + } + + s := node.Starter{ + Runner: r, + PreExists: p, + MachineAPI: m, + Host: h, + Cfg: *cc, + Node: *n, + ExistingAddons: nil, + } + + _, err = node.Start(s, false) if err != nil { _, err := maybeDeleteAndRetry(*cc, *n, nil, err) if err != nil { diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index f41ac630306c..c776b1efbba7 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -305,38 +305,49 @@ func runStart(cmd *cobra.Command, args []string) { validateSpecifiedDriver(existing) ds, alts, specified := selectDriver(existing) - err = startWithDriver(cmd, ds, existing) + starter, err := provisionWithDriver(cmd, ds, existing) if err != nil && !specified { + success := false // Walk down the rest of the options for _, alt := range alts { - out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}.", out.V{"old_driver": ds.Name, "new_driver": alt.Name}) + out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}: {{.error}}", out.V{"old_driver": ds.Name, "new_driver": alt.Name, "error": err}) ds = alt // Delete the existing cluster and try again with the next driver on the list profile, err := config.LoadProfile(ClusterFlagValue()) if err != nil { - out.ErrT(out.Meh, `"{{.name}}" profile does not exist, trying anyways.`, out.V{"name": ClusterFlagValue()}) + glog.Warningf("%s profile does not exist, trying anyways.", ClusterFlagValue()) } err = deleteProfile(profile) if err != nil { out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": ClusterFlagValue()}) } - err = startWithDriver(cmd, ds, existing) + starter, err = provisionWithDriver(cmd, ds, existing) if err != nil { continue } else { // Success! - os.Exit(0) + success = true + break } } + if !success { + exit.WithError("error provisioning host", err) + } + } + + kubeconfig, err := startWithDriver(starter, existing) + if err != nil { + exit.WithError("failed to start node", err) } - // Use the most recent error - exit.WithError("startup failed", err) + if err := showKubectlInfo(kubeconfig, starter.Node.KubernetesVersion, starter.Cfg.Name); err != nil { + glog.Errorf("kubectl info: %v", err) + } } -func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *config.ClusterConfig) error { +func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *config.ClusterConfig) (node.Starter, error) { driverName := ds.Name glog.Infof("selected driver: %s", driverName) validateDriver(ds, existing) @@ -356,19 +367,19 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf k8sVersion := getKubernetesVersion(existing) cc, n, err := generateCfgFromFlags(cmd, k8sVersion, driverName) if err != nil { - return errors.Wrap(err, "Failed to generate config") + return node.Starter{}, errors.Wrap(err, "Failed to generate config") } // This is about as far as we can go without overwriting config files if viper.GetBool(dryRun) { out.T(out.DryRun, `dry-run validation complete!`) - return nil + return node.Starter{}, nil } if driver.IsVM(driverName) { url, err := download.ISO(viper.GetStringSlice(isoURL), cmd.Flags().Changed(isoURL)) if err != nil { - return errors.Wrap(err, "Failed to cache ISO") + return node.Starter{}, errors.Wrap(err, "Failed to cache ISO") } cc.MinikubeISO = url } @@ -387,11 +398,29 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf } } - kubeconfig, err := node.Start(cc, n, existingAddons, true) + mRunner, preExists, mAPI, host, err := node.Provision(cc, n, true) + if err != nil { + return node.Starter{}, err + } + + return node.Starter{ + Runner: mRunner, + PreExists: preExists, + MachineAPI: mAPI, + Host: host, + ExistingAddons: existingAddons, + Cfg: cc, + Node: n, + }, nil +} + +func startWithDriver(starter node.Starter, existing *config.ClusterConfig) (*kubeconfig.Settings, error) { + + kubeconfig, err := node.Start(starter, true) if err != nil { - kubeconfig, err = maybeDeleteAndRetry(cc, n, existingAddons, err) + kubeconfig, err = maybeDeleteAndRetry(starter.Cfg, starter.Node, starter.ExistingAddons, err) if err != nil { - return err + return nil, err } } @@ -400,7 +429,7 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf numNodes = len(existing.Nodes) } if numNodes > 1 { - if driver.BareMetal(driverName) { + if driver.BareMetal(starter.Cfg.Driver) { exit.WithCodeT(exit.Config, "The none driver is not compatible with multi-node clusters.") } else { for i := 1; i < numNodes; i++ { @@ -409,22 +438,18 @@ func startWithDriver(cmd *cobra.Command, ds registry.DriverState, existing *conf Name: nodeName, Worker: true, ControlPlane: false, - KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, + KubernetesVersion: starter.Cfg.KubernetesConfig.KubernetesVersion, } out.Ln("") // extra newline for clarity on the command line - err := node.Add(&cc, n) + err := node.Add(&starter.Cfg, n) if err != nil { - return errors.Wrap(err, "adding node") + return nil, errors.Wrap(err, "adding node") } } } } - if err := showKubectlInfo(kubeconfig, k8sVersion, cc.Name); err != nil { - glog.Errorf("kubectl info: %v", err) - } - - return nil + return kubeconfig, nil } func updateDriver(driverName string) { @@ -514,9 +539,24 @@ func maybeDeleteAndRetry(cc config.ClusterConfig, n config.Node, existingAddons } var kubeconfig *kubeconfig.Settings - for _, v := range cc.Nodes { - k, err := node.Start(cc, v, existingAddons, v.ControlPlane) - if v.ControlPlane { + for _, n := range cc.Nodes { + r, p, m, h, err := node.Provision(cc, n, n.ControlPlane) + s := node.Starter{ + Runner: r, + PreExists: p, + MachineAPI: m, + Host: h, + Cfg: cc, + Node: n, + ExistingAddons: existingAddons, + } + if err != nil { + // Ok we failed again, let's bail + return nil, err + } + + k, err := node.Start(s, n.ControlPlane) + if n.ControlPlane { kubeconfig = k } if err != nil { diff --git a/pkg/minikube/node/node.go b/pkg/minikube/node/node.go index 411275f3de60..6f6e3838446f 100644 --- a/pkg/minikube/node/node.go +++ b/pkg/minikube/node/node.go @@ -39,7 +39,21 @@ func Add(cc *config.ClusterConfig, n config.Node) error { return errors.Wrap(err, "save node") } - _, err := Start(*cc, n, nil, false) + r, p, m, h, err := Provision(*cc, n, false) + if err != nil { + return err + } + s := Starter{ + Runner: r, + PreExists: p, + MachineAPI: m, + Host: h, + Cfg: *cc, + Node: n, + ExistingAddons: nil, + } + + _, err = Start(s, false) return err } diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index e0ad51f5a94c..66c75a89c28a 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -63,63 +63,50 @@ const ( containerRuntime = "container-runtime" ) -// Start spins up a guest and starts the kubernetes node. -func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]bool, apiServer bool) (*kubeconfig.Settings, error) { - cp := "" - if apiServer { - cp = "control plane " - } - - out.T(out.ThumbsUp, "Starting {{.controlPlane}}node {{.name}} in cluster {{.cluster}}", out.V{"controlPlane": cp, "name": n.Name, "cluster": cc.Name}) - - var kicGroup errgroup.Group - if driver.IsKIC(cc.Driver) { - beginDownloadKicArtifacts(&kicGroup) - } - - var cacheGroup errgroup.Group - if !driver.BareMetal(cc.Driver) { - beginCacheKubernetesImages(&cacheGroup, cc.KubernetesConfig.ImageRepository, n.KubernetesVersion, cc.KubernetesConfig.ContainerRuntime) - } - - // Abstraction leakage alert: startHost requires the config to be saved, to satistfy pkg/provision/buildroot. - // Hence, saveConfig must be called before startHost, and again afterwards when we know the IP. - if err := config.SaveProfile(viper.GetString(config.ProfileName), &cc); err != nil { - return nil, errors.Wrap(err, "Failed to save config") - } - - handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion) - waitDownloadKicArtifacts(&kicGroup) +var ( + kicGroup errgroup.Group + cacheGroup errgroup.Group +) - mRunner, preExists, machineAPI, host := startMachine(&cc, &n) - defer machineAPI.Close() +// Starter is a struct with all the necessary information to start a node +type Starter struct { + Runner command.Runner + PreExists bool + MachineAPI libmachine.API + Host *host.Host + Cfg config.ClusterConfig + Node config.Node + ExistingAddons map[string]bool +} +// Start spins up a guest and starts the kubernetes node. +func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { // wait for preloaded tarball to finish downloading before configuring runtimes waitCacheRequiredImages(&cacheGroup) - sv, err := util.ParseKubernetesVersion(n.KubernetesVersion) + sv, err := util.ParseKubernetesVersion(starter.Node.KubernetesVersion) if err != nil { return nil, errors.Wrap(err, "Failed to parse kubernetes version") } // configure the runtime (docker, containerd, crio) - cr := configureRuntimes(mRunner, cc.Driver, cc.KubernetesConfig, sv) - showVersionInfo(n.KubernetesVersion, cr) + cr := configureRuntimes(starter.Runner, starter.Cfg.Driver, starter.Cfg.KubernetesConfig, sv) + showVersionInfo(starter.Node.KubernetesVersion, cr) var bs bootstrapper.Bootstrapper var kcs *kubeconfig.Settings if apiServer { // Must be written before bootstrap, otherwise health checks may flake due to stale IP - kcs = setupKubeconfig(host, &cc, &n, cc.Name) + kcs = setupKubeconfig(starter.Host, &starter.Cfg, &starter.Node, starter.Cfg.Name) if err != nil { return nil, errors.Wrap(err, "Failed to setup kubeconfig") } // setup kubeadm (must come after setupKubeconfig) - bs = setupKubeAdm(machineAPI, cc, n) - err = bs.StartCluster(cc) + bs = setupKubeAdm(starter.MachineAPI, starter.Cfg, starter.Node) + err = bs.StartCluster(starter.Cfg) if err != nil { - out.LogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, cc, mRunner)) + out.LogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, starter.Cfg, starter.Runner)) return nil, err } @@ -128,12 +115,12 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo return nil, errors.Wrap(err, "Failed to update kubeconfig file.") } } else { - bs, err = cluster.Bootstrapper(machineAPI, viper.GetString(cmdcfg.Bootstrapper), cc, n) + bs, err = cluster.Bootstrapper(starter.MachineAPI, viper.GetString(cmdcfg.Bootstrapper), starter.Cfg, starter.Node) if err != nil { return nil, errors.Wrap(err, "Failed to get bootstrapper") } - if err = bs.SetupCerts(cc.KubernetesConfig, n); err != nil { + if err = bs.SetupCerts(starter.Cfg.KubernetesConfig, starter.Node); err != nil { return nil, errors.Wrap(err, "setting up certs") } } @@ -145,43 +132,43 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo } // enable addons, both old and new! - if existingAddons != nil { - addons.Start(viper.GetString(config.ProfileName), existingAddons, config.AddonList) + if starter.ExistingAddons != nil { + addons.Start(viper.GetString(config.ProfileName), starter.ExistingAddons, config.AddonList) } if apiServer { // special ops for none , like change minikube directory. // multinode super doesn't work on the none driver - if cc.Driver == driver.None && len(cc.Nodes) == 1 { + if starter.Cfg.Driver == driver.None && len(starter.Cfg.Nodes) == 1 { prepareNone() } // Skip pre-existing, because we already waited for health - if viper.GetBool(waitUntilHealthy) && !preExists { - if err := bs.WaitForNode(cc, n, viper.GetDuration(waitTimeout)); err != nil { + if viper.GetBool(waitUntilHealthy) && !starter.PreExists { + if err := bs.WaitForNode(starter.Cfg, starter.Node, viper.GetDuration(waitTimeout)); err != nil { return nil, errors.Wrap(err, "Wait failed") } } } else { - if err := bs.UpdateNode(cc, n, cr); err != nil { + if err := bs.UpdateNode(starter.Cfg, starter.Node, cr); err != nil { return nil, errors.Wrap(err, "Updating node") } - cp, err := config.PrimaryControlPlane(&cc) + cp, err := config.PrimaryControlPlane(&starter.Cfg) if err != nil { return nil, errors.Wrap(err, "Getting primary control plane") } - cpBs, err := cluster.Bootstrapper(machineAPI, viper.GetString(cmdcfg.Bootstrapper), cc, cp) + cpBs, err := cluster.Bootstrapper(starter.MachineAPI, viper.GetString(cmdcfg.Bootstrapper), starter.Cfg, cp) if err != nil { return nil, errors.Wrap(err, "Getting bootstrapper") } - joinCmd, err := cpBs.GenerateToken(cc) + joinCmd, err := cpBs.GenerateToken(starter.Cfg) if err != nil { return nil, errors.Wrap(err, "generating join token") } - if err = bs.JoinCluster(cc, n, joinCmd); err != nil { + if err = bs.JoinCluster(starter.Cfg, starter.Node, joinCmd); err != nil { return nil, errors.Wrap(err, "joining cluster") } } @@ -189,6 +176,36 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo return kcs, nil } +// Provision provisions the machine/container for the node +func Provision(cc config.ClusterConfig, n config.Node, apiServer bool) (command.Runner, bool, libmachine.API, *host.Host, error) { + cp := "" + if apiServer { + cp = "control plane " + } + + out.T(out.ThumbsUp, "Starting {{.controlPlane}}node {{.name}} in cluster {{.cluster}}", out.V{"controlPlane": cp, "name": n.Name, "cluster": cc.Name}) + + if driver.IsKIC(cc.Driver) { + beginDownloadKicArtifacts(&kicGroup) + } + + if !driver.BareMetal(cc.Driver) { + beginCacheKubernetesImages(&cacheGroup, cc.KubernetesConfig.ImageRepository, n.KubernetesVersion, cc.KubernetesConfig.ContainerRuntime) + } + + // Abstraction leakage alert: startHost requires the config to be saved, to satistfy pkg/provision/buildroot. + // Hence, saveConfig must be called before startHost, and again afterwards when we know the IP. + if err := config.SaveProfile(viper.GetString(config.ProfileName), &cc); err != nil { + return nil, false, nil, nil, errors.Wrap(err, "Failed to save config") + } + + handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion) + waitDownloadKicArtifacts(&kicGroup) + + return startMachine(&cc, &n) + +} + // ConfigureRuntimes does what needs to happen to get a runtime going. func configureRuntimes(runner cruntime.CommandRunner, drvName string, k8s config.KubernetesConfig, kv semver.Version) cruntime.Manager { co := cruntime.Config{ @@ -281,15 +298,15 @@ func apiServerURL(h host.Host, cc config.ClusterConfig, n config.Node) (string, } // StartMachine starts a VM -func startMachine(cfg *config.ClusterConfig, node *config.Node) (runner command.Runner, preExists bool, machineAPI libmachine.API, host *host.Host) { +func startMachine(cfg *config.ClusterConfig, node *config.Node) (runner command.Runner, preExists bool, machineAPI libmachine.API, host *host.Host, err error) { m, err := machine.NewAPIClient() if err != nil { - exit.WithError("Failed to get machine client", err) + return nil, false, nil, nil, errors.Wrap(err, "Failed to get machine client") } host, preExists = startHost(m, *cfg, *node) runner, err = machine.CommandRunner(host) if err != nil { - exit.WithError("Failed to get command runner", err) + return runner, preExists, m, host, errors.Wrap(err, "Failed to get command runner") } ip := validateNetwork(host, runner) @@ -304,10 +321,10 @@ func startMachine(cfg *config.ClusterConfig, node *config.Node) (runner command. node.IP = ip err = config.SaveNode(cfg, node) if err != nil { - exit.WithError("saving node", err) + return nil, false, nil, nil, errors.Wrap(err, "saving node") } - return runner, preExists, m, host + return runner, preExists, m, host, err } // startHost starts a new minikube host using a VM or None From 78595b13dc6fe1d762eb8426c22613834aa67394 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Mon, 6 Apr 2020 11:45:42 -0700 Subject: [PATCH 07/13] split out node provisioning and starting kubernetes --- cmd/minikube/cmd/node_start.go | 6 +++--- cmd/minikube/cmd/start.go | 16 ++++++++-------- pkg/minikube/node/node.go | 6 +++--- pkg/minikube/node/start.go | 34 +++++++++++++++++----------------- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/cmd/minikube/cmd/node_start.go b/cmd/minikube/cmd/node_start.go index 0b4a918ce1e6..81f9ac6b8028 100644 --- a/cmd/minikube/cmd/node_start.go +++ b/cmd/minikube/cmd/node_start.go @@ -49,7 +49,7 @@ var nodeStartCmd = &cobra.Command{ exit.WithError("retrieving node", err) } - r, p, m, h, err := node.Provision(*cc, *n, false) + r, p, m, h, err := node.Provision(cc, n, false) if err != nil { exit.WithError("provisioning host for node", err) } @@ -59,8 +59,8 @@ var nodeStartCmd = &cobra.Command{ PreExists: p, MachineAPI: m, Host: h, - Cfg: *cc, - Node: *n, + Cfg: cc, + Node: n, ExistingAddons: nil, } diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 60d578837845..7ea3b9a938c5 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -399,7 +399,7 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing * } } - mRunner, preExists, mAPI, host, err := node.Provision(cc, n, true) + mRunner, preExists, mAPI, host, err := node.Provision(&cc, &n, true) if err != nil { return node.Starter{}, err } @@ -410,8 +410,8 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing * MachineAPI: mAPI, Host: host, ExistingAddons: existingAddons, - Cfg: cc, - Node: n, + Cfg: &cc, + Node: &n, }, nil } @@ -419,7 +419,7 @@ func startWithDriver(starter node.Starter, existing *config.ClusterConfig) (*kub kubeconfig, err := node.Start(starter, true) if err != nil { - kubeconfig, err = maybeDeleteAndRetry(starter.Cfg, starter.Node, starter.ExistingAddons, err) + kubeconfig, err = maybeDeleteAndRetry(*starter.Cfg, *starter.Node, starter.ExistingAddons, err) if err != nil { return nil, err } @@ -442,7 +442,7 @@ func startWithDriver(starter node.Starter, existing *config.ClusterConfig) (*kub KubernetesVersion: starter.Cfg.KubernetesConfig.KubernetesVersion, } out.Ln("") // extra newline for clarity on the command line - err := node.Add(&starter.Cfg, n) + err := node.Add(starter.Cfg, n) if err != nil { return nil, errors.Wrap(err, "adding node") } @@ -541,14 +541,14 @@ func maybeDeleteAndRetry(cc config.ClusterConfig, n config.Node, existingAddons var kubeconfig *kubeconfig.Settings for _, n := range cc.Nodes { - r, p, m, h, err := node.Provision(cc, n, n.ControlPlane) + r, p, m, h, err := node.Provision(&cc, &n, n.ControlPlane) s := node.Starter{ Runner: r, PreExists: p, MachineAPI: m, Host: h, - Cfg: cc, - Node: n, + Cfg: &cc, + Node: &n, ExistingAddons: existingAddons, } if err != nil { diff --git a/pkg/minikube/node/node.go b/pkg/minikube/node/node.go index 6f6e3838446f..dcc4f4d7d536 100644 --- a/pkg/minikube/node/node.go +++ b/pkg/minikube/node/node.go @@ -39,7 +39,7 @@ func Add(cc *config.ClusterConfig, n config.Node) error { return errors.Wrap(err, "save node") } - r, p, m, h, err := Provision(*cc, n, false) + r, p, m, h, err := Provision(cc, &n, false) if err != nil { return err } @@ -48,8 +48,8 @@ func Add(cc *config.ClusterConfig, n config.Node) error { PreExists: p, MachineAPI: m, Host: h, - Cfg: *cc, - Node: n, + Cfg: cc, + Node: &n, ExistingAddons: nil, } diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index d890fef49467..3e2584481a01 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -75,8 +75,8 @@ type Starter struct { PreExists bool MachineAPI libmachine.API Host *host.Host - Cfg config.ClusterConfig - Node config.Node + Cfg *config.ClusterConfig + Node *config.Node ExistingAddons map[string]bool } @@ -98,16 +98,16 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { var kcs *kubeconfig.Settings if apiServer { // Must be written before bootstrap, otherwise health checks may flake due to stale IP - kcs = setupKubeconfig(starter.Host, &starter.Cfg, &starter.Node, starter.Cfg.Name) + kcs = setupKubeconfig(starter.Host, starter.Cfg, starter.Node, starter.Cfg.Name) if err != nil { return nil, errors.Wrap(err, "Failed to setup kubeconfig") } // setup kubeadm (must come after setupKubeconfig) - bs = setupKubeAdm(starter.MachineAPI, starter.Cfg, starter.Node) - err = bs.StartCluster(starter.Cfg) + bs = setupKubeAdm(starter.MachineAPI, *starter.Cfg, *starter.Node) + err = bs.StartCluster(*starter.Cfg) if err != nil { - out.LogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, starter.Cfg, starter.Runner)) + out.LogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, *starter.Cfg, starter.Runner)) return nil, err } @@ -116,12 +116,12 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { return nil, errors.Wrap(err, "Failed to update kubeconfig file.") } } else { - bs, err = cluster.Bootstrapper(starter.MachineAPI, viper.GetString(cmdcfg.Bootstrapper), starter.Cfg, starter.Node) + bs, err = cluster.Bootstrapper(starter.MachineAPI, viper.GetString(cmdcfg.Bootstrapper), *starter.Cfg, *starter.Node) if err != nil { return nil, errors.Wrap(err, "Failed to get bootstrapper") } - if err = bs.SetupCerts(starter.Cfg.KubernetesConfig, starter.Node); err != nil { + if err = bs.SetupCerts(starter.Cfg.KubernetesConfig, *starter.Node); err != nil { return nil, errors.Wrap(err, "setting up certs") } } @@ -146,30 +146,30 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { // Skip pre-existing, because we already waited for health if kverify.ShouldWait(starter.Cfg.VerifyComponents) && !starter.PreExists { - if err := bs.WaitForNode(starter.Cfg, starter.Node, viper.GetDuration(waitTimeout)); err != nil { + if err := bs.WaitForNode(*starter.Cfg, *starter.Node, viper.GetDuration(waitTimeout)); err != nil { return nil, errors.Wrap(err, "Wait failed") } } } else { - if err := bs.UpdateNode(starter.Cfg, starter.Node, cr); err != nil { + if err := bs.UpdateNode(*starter.Cfg, *starter.Node, cr); err != nil { return nil, errors.Wrap(err, "Updating node") } - cp, err := config.PrimaryControlPlane(&starter.Cfg) + cp, err := config.PrimaryControlPlane(starter.Cfg) if err != nil { return nil, errors.Wrap(err, "Getting primary control plane") } - cpBs, err := cluster.Bootstrapper(starter.MachineAPI, viper.GetString(cmdcfg.Bootstrapper), starter.Cfg, cp) + cpBs, err := cluster.Bootstrapper(starter.MachineAPI, viper.GetString(cmdcfg.Bootstrapper), *starter.Cfg, cp) if err != nil { return nil, errors.Wrap(err, "Getting bootstrapper") } - joinCmd, err := cpBs.GenerateToken(starter.Cfg) + joinCmd, err := cpBs.GenerateToken(*starter.Cfg) if err != nil { return nil, errors.Wrap(err, "generating join token") } - if err = bs.JoinCluster(starter.Cfg, starter.Node, joinCmd); err != nil { + if err = bs.JoinCluster(*starter.Cfg, *starter.Node, joinCmd); err != nil { return nil, errors.Wrap(err, "joining cluster") } } @@ -178,7 +178,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { } // Provision provisions the machine/container for the node -func Provision(cc config.ClusterConfig, n config.Node, apiServer bool) (command.Runner, bool, libmachine.API, *host.Host, error) { +func Provision(cc *config.ClusterConfig, n *config.Node, apiServer bool) (command.Runner, bool, libmachine.API, *host.Host, error) { if apiServer { out.T(out.ThumbsUp, "Starting control plane node {{.name}} in cluster {{.cluster}}", out.V{"name": n.Name, "cluster": cc.Name}) } else { @@ -195,14 +195,14 @@ func Provision(cc config.ClusterConfig, n config.Node, apiServer bool) (command. // Abstraction leakage alert: startHost requires the config to be saved, to satistfy pkg/provision/buildroot. // Hence, saveConfig must be called before startHost, and again afterwards when we know the IP. - if err := config.SaveProfile(viper.GetString(config.ProfileName), &cc); err != nil { + if err := config.SaveProfile(viper.GetString(config.ProfileName), cc); err != nil { return nil, false, nil, nil, errors.Wrap(err, "Failed to save config") } handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion) waitDownloadKicArtifacts(&kicGroup) - return startMachine(&cc, &n) + return startMachine(cc, n) } From eb42d160873716a8bdfce0fb6169bdb73326f4b3 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Mon, 6 Apr 2020 13:51:30 -0700 Subject: [PATCH 08/13] actually error out when driver is specified --- cmd/minikube/cmd/start.go | 56 +++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 7ea3b9a938c5..8f8e56433760 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -307,34 +307,39 @@ func runStart(cmd *cobra.Command, args []string) { validateSpecifiedDriver(existing) ds, alts, specified := selectDriver(existing) starter, err := provisionWithDriver(cmd, ds, existing) - if err != nil && !specified { - success := false - // Walk down the rest of the options - for _, alt := range alts { - out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}: {{.error}}", out.V{"old_driver": ds.Name, "new_driver": alt.Name, "error": err}) - ds = alt - // Delete the existing cluster and try again with the next driver on the list - profile, err := config.LoadProfile(ClusterFlagValue()) - if err != nil { - glog.Warningf("%s profile does not exist, trying anyways.", ClusterFlagValue()) - } + if err != nil { + if specified { + // If the user specified a driver, don't fallback to anything else + exit.WithError("error provisioning host", err) + } else { + success := false + // Walk down the rest of the options + for _, alt := range alts { + out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}: {{.error}}", out.V{"old_driver": ds.Name, "new_driver": alt.Name, "error": err}) + ds = alt + // Delete the existing cluster and try again with the next driver on the list + profile, err := config.LoadProfile(ClusterFlagValue()) + if err != nil { + glog.Warningf("%s profile does not exist, trying anyways.", ClusterFlagValue()) + } - err = deleteProfile(profile) - if err != nil { - out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": ClusterFlagValue()}) + err = deleteProfile(profile) + if err != nil { + out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": ClusterFlagValue()}) + } + starter, err = provisionWithDriver(cmd, ds, existing) + if err != nil { + continue + } else { + // Success! + success = true + break + } } - starter, err = provisionWithDriver(cmd, ds, existing) - if err != nil { - continue - } else { - // Success! - success = true - break + if !success { + exit.WithError("error provisioning host", err) } } - if !success { - exit.WithError("error provisioning host", err) - } } kubeconfig, err := startWithDriver(starter, existing) @@ -374,7 +379,7 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing * // This is about as far as we can go without overwriting config files if viper.GetBool(dryRun) { out.T(out.DryRun, `dry-run validation complete!`) - return node.Starter{}, nil + os.Exit(0) } if driver.IsVM(driverName) { @@ -416,7 +421,6 @@ func provisionWithDriver(cmd *cobra.Command, ds registry.DriverState, existing * } func startWithDriver(starter node.Starter, existing *config.ClusterConfig) (*kubeconfig.Settings, error) { - kubeconfig, err := node.Start(starter, true) if err != nil { kubeconfig, err = maybeDeleteAndRetry(*starter.Cfg, *starter.Node, starter.ExistingAddons, err) From 5df9ead41cd157783024f999797a26f0759500e5 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 7 Apr 2020 16:29:40 -0700 Subject: [PATCH 09/13] never exit during provisioning --- cmd/minikube/cmd/start.go | 2 +- pkg/minikube/node/cache.go | 1 - pkg/minikube/node/start.go | 45 ++++++++++++++++++++++++-------------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 8f8e56433760..9b20bff2d208 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1038,7 +1038,7 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, drvName string) if strings.ToLower(repository) == "auto" || mirrorCountry != "" { found, autoSelectedRepository, err := selectImageRepository(mirrorCountry, semver.MustParse(strings.TrimPrefix(k8sVersion, version.VersionPrefix))) if err != nil { - exit.WithError("Failed to check main repository and mirrors for images for images", err) + exit.WithError("Failed to check main repository and mirrors for images", err) } if !found { diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index ebeb04fdc0b3..46a51ba8c9d5 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -80,7 +80,6 @@ func handleDownloadOnly(cacheGroup, kicGroup *errgroup.Group, k8sVersion string) } out.T(out.Check, "Download complete!") os.Exit(0) - } // CacheKubectlBinary caches the kubectl binary diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 3e2584481a01..334c9a62fd78 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -313,15 +313,21 @@ func apiServerURL(h host.Host, cc config.ClusterConfig, n config.Node) (string, func startMachine(cfg *config.ClusterConfig, node *config.Node) (runner command.Runner, preExists bool, machineAPI libmachine.API, host *host.Host, err error) { m, err := machine.NewAPIClient() if err != nil { - return nil, false, nil, nil, errors.Wrap(err, "Failed to get machine client") + return runner, preExists, m, host, errors.Wrap(err, "Failed to get machine client") + } + host, preExists, err = startHost(m, *cfg, *node) + if err != nil { + return runner, preExists, m, host, errors.Wrap(err, "Failed to start host") } - host, preExists = startHost(m, *cfg, *node) runner, err = machine.CommandRunner(host) if err != nil { return runner, preExists, m, host, errors.Wrap(err, "Failed to get command runner") } - ip := validateNetwork(host, runner) + ip, err := validateNetwork(host, runner) + if err != nil { + return runner, preExists, m, host, errors.Wrap(err, "Failed to validate network") + } // Bypass proxy for minikube's vm host ip err = proxy.ExcludeIP(ip) @@ -333,17 +339,17 @@ func startMachine(cfg *config.ClusterConfig, node *config.Node) (runner command. node.IP = ip err = config.SaveNode(cfg, node) if err != nil { - return nil, false, nil, nil, errors.Wrap(err, "saving node") + return runner, preExists, m, host, errors.Wrap(err, "saving node") } return runner, preExists, m, host, err } // startHost starts a new minikube host using a VM or None -func startHost(api libmachine.API, cc config.ClusterConfig, n config.Node) (*host.Host, bool) { +func startHost(api libmachine.API, cc config.ClusterConfig, n config.Node) (*host.Host, bool, error) { host, exists, err := machine.StartHost(api, cc, n) if err == nil { - return host, exists + return host, exists, nil } out.ErrT(out.Embarrassed, "StartHost failed, but will try again: {{.error}}", out.V{"error": err}) @@ -360,20 +366,20 @@ func startHost(api libmachine.API, cc config.ClusterConfig, n config.Node) (*hos host, exists, err = machine.StartHost(api, cc, n) if err == nil { - return host, exists + return host, exists, nil } // Don't use host.Driver to avoid nil pointer deref drv := cc.Driver - exit.WithError(fmt.Sprintf(`Failed to start %s %s. "%s" may fix it.`, drv, driver.MachineType(drv), mustload.ExampleCmd(cc.Name, "start")), err) - return host, exists + out.ErrT(out.Sad, `Failed to start {{.driver}} {{.driver_type}}. "{{.cmd}}" may fix it: {{.error}}`, out.V{"driver": drv, "driver_type": driver.MachineType(drv), "cmd": mustload.ExampleCmd(cc.Name, "start"), "error": err}) + return host, exists, err } // validateNetwork tries to catch network problems as soon as possible -func validateNetwork(h *host.Host, r command.Runner) string { +func validateNetwork(h *host.Host, r command.Runner) (string, error) { ip, err := h.Driver.GetIP() if err != nil { - exit.WithError("Unable to get VM IP address", err) + return ip, err } optSeen := false @@ -395,17 +401,19 @@ func validateNetwork(h *host.Host, r command.Runner) string { } if !driver.BareMetal(h.Driver.DriverName()) && !driver.IsKIC(h.Driver.DriverName()) { - trySSH(h, ip) + if err := trySSH(h, ip); err != nil { + return ip, err + } } // Non-blocking go tryRegistry(r, h.Driver.DriverName()) - return ip + return ip, nil } -func trySSH(h *host.Host, ip string) { +func trySSH(h *host.Host, ip string) error { if viper.GetBool("force") { - return + return nil } sshAddr := net.JoinHostPort(ip, "22") @@ -421,8 +429,9 @@ func trySSH(h *host.Host, ip string) { return nil } - if err := retry.Expo(dial, time.Second, 13*time.Second); err != nil { - exit.WithCodeT(exit.IO, `minikube is unable to connect to the VM: {{.error}} + err := retry.Expo(dial, time.Second, 13*time.Second) + if err != nil { + out.ErrT(out.FailureType, `minikube is unable to connect to the VM: {{.error}} This is likely due to one of two reasons: @@ -438,6 +447,8 @@ func trySSH(h *host.Host, ip string) { - Use --force to override this connectivity check `, out.V{"error": err, "hypervisor": h.Driver.DriverName(), "ip": ip}) } + + return err } // tryRegistry tries to connect to the image repository From d2dbf539ab931134f5c69547e49c5191e5630a02 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 9 Apr 2020 09:32:05 -0700 Subject: [PATCH 10/13] fix node name regression --- pkg/minikube/node/start.go | 8 +++++--- pkg/minikube/out/out.go | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index e86ccc48a354..0fda6de5959f 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -114,7 +114,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { bs = setupKubeAdm(starter.MachineAPI, *starter.Cfg, *starter.Node) err = bs.StartCluster(*starter.Cfg) - if err != nil { + if err != nil { out.LogEntries("Error starting cluster", err, logs.FindProblems(cr, bs, *starter.Cfg, starter.Runner)) return nil, err } @@ -187,10 +187,12 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { // Provision provisions the machine/container for the node func Provision(cc *config.ClusterConfig, n *config.Node, apiServer bool) (command.Runner, bool, libmachine.API, *host.Host, error) { + + name := driver.MachineName(*cc, *n) if apiServer { - out.T(out.ThumbsUp, "Starting control plane node {{.name}} in cluster {{.cluster}}", out.V{"name": n.Name, "cluster": cc.Name}) + out.T(out.ThumbsUp, "Starting control plane node {{.name}} in cluster {{.cluster}}", out.V{"name": name, "cluster": cc.Name}) } else { - out.T(out.ThumbsUp, "Starting node {{.name}} in cluster {{.cluster}}", out.V{"name": n.Name, "cluster": cc.Name}) + out.T(out.ThumbsUp, "Starting node {{.name}} in cluster {{.cluster}}", out.V{"name": name, "cluster": cc.Name}) } if driver.IsKIC(cc.Driver) { diff --git a/pkg/minikube/out/out.go b/pkg/minikube/out/out.go index 4b6dd0bf6f3a..618b5fd36e0c 100644 --- a/pkg/minikube/out/out.go +++ b/pkg/minikube/out/out.go @@ -195,7 +195,7 @@ func LogEntries(msg string, err error, entries map[string][]string) { } } -// DisplayError +// DisplayError prints the error and displays the standard minikube error messaging func DisplayError(msg string, err error) { // use Warning because Error will display a duplicate message to stderr glog.Warningf(fmt.Sprintf("%s: %v", msg, err)) From 198247d9f1fb8f4c4f2e7203ea3fe84664d1677b Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 9 Apr 2020 10:32:25 -0700 Subject: [PATCH 11/13] make alternate driver text more clear --- cmd/minikube/cmd/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 545694a85f37..7855e910853e 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -160,7 +160,7 @@ func runStart(cmd *cobra.Command, args []string) { success := false // Walk down the rest of the options for _, alt := range alts { - out.WarningT("Startup with {{.old_driver}} driver failed, trying with {{.new_driver}}: {{.error}}", out.V{"old_driver": ds.Name, "new_driver": alt.Name, "error": err}) + out.WarningT("Startup with {{.old_driver}} driver failed, trying with alternate driver {{.new_driver}}: {{.error}}", out.V{"old_driver": ds.Name, "new_driver": alt.Name, "error": err}) ds = alt // Delete the existing cluster and try again with the next driver on the list profile, err := config.LoadProfile(ClusterFlagValue()) From 98532748df266bca1ac8a5ed36c74728782365ca Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 9 Apr 2020 11:02:08 -0700 Subject: [PATCH 12/13] fix build failure --- pkg/minikube/exit/exit.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/minikube/exit/exit.go b/pkg/minikube/exit/exit.go index d9e4403e5226..4f9f734b2c9e 100644 --- a/pkg/minikube/exit/exit.go +++ b/pkg/minikube/exit/exit.go @@ -22,6 +22,7 @@ import ( "runtime" "runtime/debug" + "github.com/golang/glog" "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/minikube/problem" ) From 322b5d52f6c09987f7e603933e73261df672eb58 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 9 Apr 2020 11:05:31 -0700 Subject: [PATCH 13/13] fix build errors for realsies this time --- pkg/minikube/node/start.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 6ef365e8da3a..5abfa40151c2 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -147,7 +147,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { // enable addons, both old and new! if starter.ExistingAddons != nil { - go addons.Start(&wg, &cc, starter.ExistingAddons, config.AddonList) + go addons.Start(&wg, starter.Cfg, starter.ExistingAddons, config.AddonList) } if apiServer { @@ -190,7 +190,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { wg.Wait() // Write enabled addons to the config before completion - return kcs, config.Write(viper.GetString(config.ProfileName), &cc) + return kcs, config.Write(viper.GetString(config.ProfileName), starter.Cfg) } // Provision provisions the machine/container for the node