Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fallback to alternate drivers on failure #7389

Merged
merged 22 commits into from
Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ac1b3b0
merge
sharifelgamal Apr 2, 2020
357b5bb
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 2, 2020
26c7471
failback to alternate drivers if startup fails with automatic choice
sharifelgamal Apr 2, 2020
af8d01d
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 2, 2020
dd957ff
make sure to print error on failure
sharifelgamal Apr 2, 2020
b66447e
remove random failure for testing
sharifelgamal Apr 2, 2020
31f225d
fix lint
sharifelgamal Apr 2, 2020
3d037a8
only failback to next driver if provisioning fails
sharifelgamal Apr 4, 2020
66f9e13
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 4, 2020
f290aa9
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 6, 2020
78595b1
split out node provisioning and starting kubernetes
sharifelgamal Apr 6, 2020
eb42d16
actually error out when driver is specified
sharifelgamal Apr 6, 2020
81ac4d2
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 7, 2020
5df9ead
never exit during provisioning
sharifelgamal Apr 7, 2020
45bf65c
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 8, 2020
c40a942
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 9, 2020
aee3c1d
Merge branch 'master' into driver-fallback
tstromberg Apr 9, 2020
d2dbf53
fix node name regression
sharifelgamal Apr 9, 2020
198247d
make alternate driver text more clear
sharifelgamal Apr 9, 2020
e4b4a5a
Merge branch 'master' of github.com:kubernetes/minikube into driver-f…
sharifelgamal Apr 9, 2020
9853274
fix build failure
sharifelgamal Apr 9, 2020
322b5d5
fix build errors for realsies this time
sharifelgamal Apr 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cmd/minikube/cmd/node_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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})
Expand Down
5 changes: 4 additions & 1 deletion cmd/minikube/cmd/node_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
},
}
Expand Down
70 changes: 53 additions & 17 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,43 @@ func runStart(cmd *cobra.Command, args []string) {
}

validateSpecifiedDriver(existing)
ds := selectDriver(existing)
ds, alts, specified := selectDriver(existing)
err = startWithDriver(cmd, ds, existing)
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})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start by outputting the error message. It's unlikely to be terribly long, and it should be very informative.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change 'trying with'
To 'trying with alternative driver'
So it's obvious when we will see it in the outputs we are trying alternative

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()})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be hidden in a log message.

}

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
} 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 {
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)
}
Expand All @@ -324,19 +356,19 @@ func runStart(cmd *cobra.Command, args []string) {
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
if viper.GetBool(dryRun) {
out.T(out.DryRun, `dry-run validation complete!`)
return
return nil
}

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
}
Expand All @@ -357,7 +389,10 @@ func runStart(cmd *cobra.Command, args []string) {

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)
Expand All @@ -379,7 +414,7 @@ func runStart(cmd *cobra.Command, args []string) {
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")
}
}
}
Expand All @@ -388,6 +423,8 @@ func runStart(cmd *cobra.Command, args []string) {
if err := showKubectlInfo(kubeconfig, k8sVersion, cc.Name); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be moved out of startWithDriver and into the main loop. It raises no error, but it also has nothing to do with drivers.

glog.Errorf("kubectl info: %v", err)
}

return nil
}

func updateDriver(driverName string) {
Expand Down Expand Up @@ -462,7 +499,7 @@ 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.WarningT("Node {{.name}} failed to start, deleting and trying again.", out.V{"name": n.Name})
// Start failed, delete the cluster and try again
Expand All @@ -484,14 +521,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glog.Info the error saying..ok we are giving up here is the error btw:

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) {
Expand Down Expand Up @@ -519,7 +555,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, bool) {
// Technically unrelated, but important to perform before detection
driver.SetLibvirtURI(viper.GetString(kvmQemuURI))

Expand All @@ -528,7 +564,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, true
}

// Default to looking at the new driver parameter
Expand All @@ -548,7 +584,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, true
}

// Fallback to old driver parameter
Expand All @@ -558,7 +594,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, true
}

choices := driver.Choices(viper.GetBool("vm"))
Expand All @@ -581,7 +617,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, false
}

// hostDriver returns the actual driver used by a libmachine host, which can differ from our config
Expand Down
34 changes: 1 addition & 33 deletions pkg/minikube/exit/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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.FailureT("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")
}
5 changes: 3 additions & 2 deletions pkg/minikube/node/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,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)
Expand Down Expand Up @@ -119,7 +119,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
Expand Down
30 changes: 30 additions & 0 deletions pkg/minikube/out/out.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
}