From fcb5c462f4c9ee30d2034453b441669bf9e1b8ac Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 31 Mar 2020 15:06:16 -0700 Subject: [PATCH 1/4] add delete-on-failure flag --- cmd/minikube/cmd/start.go | 24 +++++++++++++++++++++++- pkg/minikube/node/start.go | 26 +++++++++++++------------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 780e2ec766e3..5e5797042697 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -124,6 +124,7 @@ const ( natNicType = "nat-nic-type" nodes = "nodes" preload = "preload" + deleteOnFailure = "delete-on-failure" ) var ( @@ -177,6 +178,7 @@ func initMinikubeFlags() { startCmd.Flags().Bool(installAddons, true, "If set, install addons. Defaults to true.") startCmd.Flags().IntP(nodes, "n", 1, "The number of nodes to spin up. Defaults to 1.") startCmd.Flags().Bool(preload, true, "If set, download tarball of preloaded images if available to improve start time. Defaults to true.") + startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again.") } // initKubernetesFlags inits the commandline flags for kubernetes related options @@ -353,7 +355,27 @@ func runStart(cmd *cobra.Command, args []string) { } } - kubeconfig := node.Start(cc, n, existingAddons, true) + kubeconfig, err := node.Start(cc, n, existingAddons, true) + if err != nil && viper.GetBool(deleteOnFailure) { + out.T(out.Warning, "Cluster {{.name}} failed to start, deleting and trying again.", out.V{"name": cc.Name}) + // Start failed, delete the cluster and try again + profile, err := config.LoadProfile(cc.Name) + if err != nil { + out.ErrT(out.Meh, `"{{.name}}" profile does not exist, trying anyways.`, out.V{"name": cc.Name}) + } + + err = deleteProfile(profile) + if err != nil { + // We failed to delete? That's not good. Error out. + exit.WithError("deleting cluster failed", err) + } + + kubeconfig, err = node.Start(cc, n, existingAddons, true) + if err != nil { + // Ok we failed again, let's bail + exit.WithError("Start failed after cluster deletion", err) + } + } numNodes := viper.GetInt(nodes) if numNodes == 1 && existing != nil { diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index eaa1232e119d..a8b3b509ab8a 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -65,7 +65,7 @@ const ( ) // 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 { +func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]bool, apiServer bool) (*kubeconfig.Settings, error) { cp := "" if apiServer { cp = "control plane " @@ -100,7 +100,7 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo sv, err := util.ParseKubernetesVersion(n.KubernetesVersion) if err != nil { - exit.WithError("Failed to parse kubernetes version", err) + return nil, errors.Wrap(err, "Failed to parse kubernetes version") } // configure the runtime (docker, containerd, crio) @@ -113,7 +113,7 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo // Must be written before bootstrap, otherwise health checks may flake due to stale IP kcs = setupKubeconfig(host, &cc, &n, cc.Name) if err != nil { - exit.WithError("Failed to setup kubeconfig", err) + return nil, errors.Wrap(err, "Failed to setup kubeconfig") } // setup kubeadm (must come after setupKubeconfig) @@ -125,16 +125,16 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo // write the kubeconfig to the file system after everything required (like certs) are created by the bootstrapper if err := kubeconfig.Update(kcs); err != nil { - exit.WithError("Failed to update kubeconfig file.", err) + return nil, errors.Wrap(err, "Failed to update kubeconfig file.") } } else { bs, err = cluster.Bootstrapper(machineAPI, viper.GetString(cmdcfg.Bootstrapper), cc, n) if err != nil { - exit.WithError("Failed to get bootstrapper", err) + return nil, errors.Wrap(err, "Failed to get bootstrapper") } if err = bs.SetupCerts(cc.KubernetesConfig, n); err != nil { - exit.WithError("setting up certs", err) + return nil, errors.Wrap(err, "setting up certs") } } @@ -159,34 +159,34 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo // 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 { - exit.WithError("Wait failed", err) + return nil, errors.Wrap(err, "Wait failed") } } } else { if err := bs.UpdateNode(cc, n, cr); err != nil { - exit.WithError("Updating node", err) + return nil, errors.Wrap(err, "Updating node") } cp, err := config.PrimaryControlPlane(&cc) if err != nil { - exit.WithError("Getting primary control plane", err) + return nil, errors.Wrap(err, "Getting primary control plane") } cpBs, err := cluster.Bootstrapper(machineAPI, viper.GetString(cmdcfg.Bootstrapper), cc, cp) if err != nil { - exit.WithError("Getting bootstrapper", err) + return nil, errors.Wrap(err, "Getting bootstrapper") } joinCmd, err := cpBs.GenerateToken(cc) if err != nil { - exit.WithError("generating join token", err) + return nil, errors.Wrap(err, "generating join token") } if err = bs.JoinCluster(cc, n, joinCmd); err != nil { - exit.WithError("joining cluster", err) + return nil, errors.Wrap(err, "joining cluster") } } - return kcs + return kcs, nil } // ConfigureRuntimes does what needs to happen to get a runtime going. From 673922c8d3dad4aa90c1560d4964ac39e47e2419 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 31 Mar 2020 15:38:59 -0700 Subject: [PATCH 2/4] code comments --- cmd/minikube/cmd/node_start.go | 5 ++++- cmd/minikube/cmd/start.go | 7 +++---- pkg/minikube/node/node.go | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cmd/minikube/cmd/node_start.go b/cmd/minikube/cmd/node_start.go index 285de93d7ba7..511ac2323d37 100644 --- a/cmd/minikube/cmd/node_start.go +++ b/cmd/minikube/cmd/node_start.go @@ -50,7 +50,10 @@ var nodeStartCmd = &cobra.Command{ } // Start it up baby - node.Start(*cc, *n, nil, false) + _, err = node.Start(*cc, *n, nil, false) + if err != nil { + exit.WithError("starting node", err) + } }, } diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 5e5797042697..7253154cecb3 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -178,7 +178,7 @@ func initMinikubeFlags() { startCmd.Flags().Bool(installAddons, true, "If set, install addons. Defaults to true.") startCmd.Flags().IntP(nodes, "n", 1, "The number of nodes to spin up. Defaults to 1.") startCmd.Flags().Bool(preload, true, "If set, download tarball of preloaded images if available to improve start time. Defaults to true.") - startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again.") + startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") } // initKubernetesFlags inits the commandline flags for kubernetes related options @@ -357,7 +357,7 @@ func runStart(cmd *cobra.Command, args []string) { kubeconfig, err := node.Start(cc, n, existingAddons, true) if err != nil && viper.GetBool(deleteOnFailure) { - out.T(out.Warning, "Cluster {{.name}} failed to start, deleting and trying again.", out.V{"name": cc.Name}) + out.T(out.Warning, "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 { @@ -366,8 +366,7 @@ func runStart(cmd *cobra.Command, args []string) { err = deleteProfile(profile) if err != nil { - // We failed to delete? That's not good. Error out. - exit.WithError("deleting cluster failed", err) + out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": cc.Name}) } kubeconfig, err = node.Start(cc, n, existingAddons, true) diff --git a/pkg/minikube/node/node.go b/pkg/minikube/node/node.go index 03bddc49bee3..231ce632d0d5 100644 --- a/pkg/minikube/node/node.go +++ b/pkg/minikube/node/node.go @@ -40,8 +40,8 @@ func Add(cc *config.ClusterConfig, n config.Node) error { } // TODO: Start should return an error rather than calling exit! - Start(*cc, n, nil, false) - return nil + _, err := Start(*cc, n, nil, false) + return err } // Delete stops and deletes the given node from the given cluster From d8151730f4bbb1b42e306a3c1ab242ad45439600 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 1 Apr 2020 11:00:39 -0700 Subject: [PATCH 3/4] retry anytime node.Start fails, not just in minikube start --- cmd/minikube/cmd/node_add.go | 13 +++------ cmd/minikube/cmd/node_start.go | 4 +-- cmd/minikube/cmd/start.go | 52 ++++++++++++++++++++++------------ pkg/minikube/node/node.go | 1 - 4 files changed, 40 insertions(+), 30 deletions(-) diff --git a/cmd/minikube/cmd/node_add.go b/cmd/minikube/cmd/node_add.go index b923b9e93519..ef25f4680dc0 100644 --- a/cmd/minikube/cmd/node_add.go +++ b/cmd/minikube/cmd/node_add.go @@ -18,10 +18,8 @@ package cmd import ( "github.com/spf13/cobra" - "github.com/spf13/pflag" "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" @@ -56,7 +54,7 @@ var nodeAddCmd = &cobra.Command{ } if err := node.Add(cc, n); err != nil { - exit.WithError("Error adding node to cluster", err) + maybeDeleteAndRetry(*cc, n, nil, err) } out.T(out.Ready, "Successfully added {{.name}} to {{.cluster}}!", out.V{"name": name, "cluster": cc.Name}) @@ -64,13 +62,10 @@ var nodeAddCmd = &cobra.Command{ } func init() { + //We should figure out which minikube start flags to actually import nodeAddCmd.Flags().BoolVar(&cp, "control-plane", false, "If true, the node added will also be a control plane in addition to a worker.") nodeAddCmd.Flags().BoolVar(&worker, "worker", true, "If true, the added node will be marked for work. Defaults to true.") - //We should figure out which of these flags to actually import - startCmd.Flags().Visit( - func(f *pflag.Flag) { - nodeAddCmd.Flags().AddFlag(f) - }, - ) + nodeAddCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") + nodeCmd.AddCommand(nodeAddCmd) } diff --git a/cmd/minikube/cmd/node_start.go b/cmd/minikube/cmd/node_start.go index 511ac2323d37..3b2782263bab 100644 --- a/cmd/minikube/cmd/node_start.go +++ b/cmd/minikube/cmd/node_start.go @@ -49,15 +49,15 @@ var nodeStartCmd = &cobra.Command{ exit.WithError("retrieving node", err) } - // Start it up baby _, err = node.Start(*cc, *n, nil, false) if err != nil { - exit.WithError("starting node", err) + maybeDeleteAndRetry(*cc, *n, nil, err) } }, } func init() { nodeStartCmd.Flags().String("name", "", "The name of the node to start") + nodeStartCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") nodeCmd.AddCommand(nodeStartCmd) } diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 7253154cecb3..57922fea1508 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -356,24 +356,8 @@ func runStart(cmd *cobra.Command, args []string) { } kubeconfig, err := node.Start(cc, n, existingAddons, true) - if err != nil && viper.GetBool(deleteOnFailure) { - out.T(out.Warning, "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 { - out.ErrT(out.Meh, `"{{.name}}" profile does not exist, trying anyways.`, out.V{"name": cc.Name}) - } - - err = deleteProfile(profile) - if err != nil { - out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": cc.Name}) - } - - kubeconfig, err = node.Start(cc, n, existingAddons, true) - if err != nil { - // Ok we failed again, let's bail - exit.WithError("Start failed after cluster deletion", err) - } + if err != nil { + kubeconfig = maybeDeleteAndRetry(cc, n, existingAddons, err) } numNodes := viper.GetInt(nodes) @@ -488,6 +472,38 @@ 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 { + if viper.GetBool(deleteOnFailure) { + out.T(out.Warning, "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 { + out.ErrT(out.Meh, `"{{.name}}" profile does not exist, trying anyways.`, out.V{"name": cc.Name}) + } + + err = deleteProfile(profile) + if err != nil { + out.WarningT("Failed to delete cluster {{.name}}, proceeding with retry anyway.", out.V{"name": cc.Name}) + } + + var kubeconfig *kubeconfig.Settings + for _, v := range cc.Nodes { + k, err := node.Start(cc, v, existingAddons, v.ControlPlane) + if v.ControlPlane { + kubeconfig = k + } + if err != nil { + // Ok we failed again, let's bail + exit.WithError("Start failed after cluster deletion", err) + } + } + return kubeconfig + } + // Don't delete the cluster unless they ask + exit.WithError("startup failed", originalErr) + return nil +} + func selectDriver(existing *config.ClusterConfig) registry.DriverState { // Technically unrelated, but important to perform before detection driver.SetLibvirtURI(viper.GetString(kvmQemuURI)) diff --git a/pkg/minikube/node/node.go b/pkg/minikube/node/node.go index 231ce632d0d5..411275f3de60 100644 --- a/pkg/minikube/node/node.go +++ b/pkg/minikube/node/node.go @@ -39,7 +39,6 @@ func Add(cc *config.ClusterConfig, n config.Node) error { return errors.Wrap(err, "save node") } - // TODO: Start should return an error rather than calling exit! _, err := Start(*cc, n, nil, false) return err } From 0357c89ee57e0579937a42a6a6604b9dbac39de4 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 1 Apr 2020 17:35:41 -0700 Subject: [PATCH 4/4] add issue to todo comment --- cmd/minikube/cmd/node_add.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/node_add.go b/cmd/minikube/cmd/node_add.go index ef25f4680dc0..ec97540cf97c 100644 --- a/cmd/minikube/cmd/node_add.go +++ b/cmd/minikube/cmd/node_add.go @@ -62,7 +62,7 @@ var nodeAddCmd = &cobra.Command{ } func init() { - //We should figure out which minikube start flags to actually import + // TODO(https://github.com/kubernetes/minikube/issues/7366): We should figure out which minikube start flags to actually import nodeAddCmd.Flags().BoolVar(&cp, "control-plane", false, "If true, the node added will also be a control plane in addition to a worker.") nodeAddCmd.Flags().BoolVar(&worker, "worker", true, "If true, the added node will be marked for work. Defaults to true.") nodeAddCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.")