From 530714580262292c9365a87689c599b94f0576fd Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 2 Apr 2020 09:58:13 -0700 Subject: [PATCH 1/4] Move errors and warnings to output to stderr --- cmd/minikube/cmd/config/profile.go | 4 +-- cmd/minikube/cmd/config/profile_list.go | 8 +++--- cmd/minikube/cmd/config/validations.go | 2 +- cmd/minikube/cmd/delete.go | 4 +-- cmd/minikube/cmd/logs.go | 2 +- cmd/minikube/cmd/mount.go | 4 +-- cmd/minikube/cmd/node_add.go | 2 +- cmd/minikube/cmd/service.go | 2 +- cmd/minikube/cmd/start.go | 18 +++++++------- cmd/minikube/cmd/stop.go | 2 +- go.mod | 1 + pkg/drivers/kic/oci/oci.go | 2 +- pkg/minikube/bootstrapper/kubeadm/kubeadm.go | 4 +-- pkg/minikube/exit/exit.go | 4 +-- pkg/minikube/logs/logs.go | 2 +- pkg/minikube/machine/fix.go | 2 +- pkg/minikube/mustload/mustload.go | 4 +-- pkg/minikube/node/start.go | 26 ++++++++++---------- 18 files changed, 47 insertions(+), 46 deletions(-) diff --git a/cmd/minikube/cmd/config/profile.go b/cmd/minikube/cmd/config/profile.go index 4161ee3aaced..46afa5237a7e 100644 --- a/cmd/minikube/cmd/config/profile.go +++ b/cmd/minikube/cmd/config/profile.go @@ -23,6 +23,7 @@ import ( "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/exit" "k8s.io/minikube/pkg/minikube/kubeconfig" + "k8s.io/minikube/pkg/minikube/mustload" "k8s.io/minikube/pkg/minikube/out" ) @@ -48,8 +49,7 @@ var ProfileCmd = &cobra.Command{ name is in the list of reserved keywords */ if config.ProfileNameInReservedKeywords(profile) { - out.ErrT(out.FailureType, `Profile name "{{.profilename}}" is minikube keyword. To delete profile use command minikube delete -p `, out.V{"profilename": profile}) - os.Exit(0) + exit.WithCodeT(exit.Config, `Profile name "{{.profilename}}" is reserved keyword. To delete this profile, run: "{{.cmd}}"`, out.V{"profilename": profile, "cmd": mustload.ExampleCmd(profile, "delete")}) } if profile == "default" { diff --git a/cmd/minikube/cmd/config/profile_list.go b/cmd/minikube/cmd/config/profile_list.go index d3b4bf1b9f2f..3a7cfa6069a4 100644 --- a/cmd/minikube/cmd/config/profile_list.go +++ b/cmd/minikube/cmd/config/profile_list.go @@ -91,13 +91,13 @@ var printProfilesTable = func() { table.Render() if invalidProfiles != nil { - out.T(out.Warning, "Found {{.number}} invalid profile(s) ! ", out.V{"number": len(invalidProfiles)}) + out.WarningT("Found {{.number}} invalid profile(s) ! ", out.V{"number": len(invalidProfiles)}) for _, p := range invalidProfiles { - out.T(out.Empty, "\t "+p.Name) + out.ErrT(out.Empty, "\t "+p.Name) } - out.T(out.Tip, "You can delete them using the following command(s): ") + out.ErrT(out.Tip, "You can delete them using the following command(s): ") for _, p := range invalidProfiles { - out.String(fmt.Sprintf("\t $ minikube delete -p %s \n", p.Name)) + out.Err(fmt.Sprintf("\t $ minikube delete -p %s \n", p.Name)) } } diff --git a/cmd/minikube/cmd/config/validations.go b/cmd/minikube/cmd/config/validations.go index 8f57e13c45b4..071512daea9c 100644 --- a/cmd/minikube/cmd/config/validations.go +++ b/cmd/minikube/cmd/config/validations.go @@ -40,7 +40,7 @@ func IsValidDriver(string, name string) error { // RequiresRestartMsg returns the "requires restart" message func RequiresRestartMsg(string, string) error { - out.T(out.Warning, "These changes will take effect upon a minikube delete and then a minikube start") + out.WarningT("These changes will take effect upon a minikube delete and then a minikube start") return nil } diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 770868f045e3..5f4ad12513ff 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -236,7 +236,7 @@ func deleteProfile(profile *config.Profile) error { } if err := killMountProcess(); err != nil { - out.T(out.FailureType, "Failed to kill mount process: {{.error}}", out.V{"error": err}) + out.FailureT("Failed to kill mount process: {{.error}}", out.V{"error": err}) } deleteHosts(api, cc) @@ -264,7 +264,7 @@ func deleteHosts(api libmachine.API, cc *config.ClusterConfig) { case mcnerror.ErrHostDoesNotExist: glog.Infof("Host %s does not exist. Proceeding ahead with cleanup.", machineName) default: - out.T(out.FailureType, "Failed to delete cluster: {{.error}}", out.V{"error": err}) + out.FailureT("Failed to delete cluster: {{.error}}", out.V{"error": err}) out.T(out.Notice, `You may need to manually remove the "{{.name}}" VM from your hypervisor`, out.V{"name": machineName}) } } diff --git a/cmd/minikube/cmd/logs.go b/cmd/minikube/cmd/logs.go index 0c3854ea1478..109938dc34c4 100644 --- a/cmd/minikube/cmd/logs.go +++ b/cmd/minikube/cmd/logs.go @@ -78,7 +78,7 @@ var logsCmd = &cobra.Command{ if err != nil { out.Ln("") // Avoid exit.WithError, since it outputs the issue URL - out.T(out.Warning, "{{.error}}", out.V{"error": err}) + out.WarningT("{{.error}}", out.V{"error": err}) os.Exit(exit.Unavailable) } }, diff --git a/cmd/minikube/cmd/mount.go b/cmd/minikube/cmd/mount.go index 4e97c7b46318..fa0db9f75d34 100644 --- a/cmd/minikube/cmd/mount.go +++ b/cmd/minikube/cmd/mount.go @@ -143,7 +143,7 @@ var mountCmd = &cobra.Command{ // An escape valve to allow future hackers to try NFS, VirtFS, or other FS types. if !supportedFilesystems[cfg.Type] { - out.T(out.Warning, "{{.type}} is not yet a supported filesystem. We will try anyways!", out.V{"type": cfg.Type}) + out.WarningT("{{.type}} is not yet a supported filesystem. We will try anyways!", out.V{"type": cfg.Type}) } bindIP := ip.String() // the ip to listen on the user's host machine @@ -179,7 +179,7 @@ var mountCmd = &cobra.Command{ out.T(out.Unmount, "Unmounting {{.path}} ...", out.V{"path": vmPath}) err := cluster.Unmount(co.CP.Runner, vmPath) if err != nil { - out.ErrT(out.FailureType, "Failed unmount: {{.error}}", out.V{"error": err}) + out.FailureT("Failed unmount: {{.error}}", out.V{"error": err}) } exit.WithCodeT(exit.Interrupted, "Received {{.name}} signal", out.V{"name": sig}) } diff --git a/cmd/minikube/cmd/node_add.go b/cmd/minikube/cmd/node_add.go index ec97540cf97c..fa0f96bb3e98 100644 --- a/cmd/minikube/cmd/node_add.go +++ b/cmd/minikube/cmd/node_add.go @@ -38,7 +38,7 @@ var nodeAddCmd = &cobra.Command{ cc := co.Config if driver.BareMetal(cc.Driver) { - out.ErrT(out.FailureType, "none driver does not support multi-node clusters") + out.FailureT("none driver does not support multi-node clusters") } name := node.Name(len(cc.Nodes) + 1) diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index 0302df7c3b30..039afefc285d 100644 --- a/cmd/minikube/cmd/service.go +++ b/cmd/minikube/cmd/service.go @@ -137,7 +137,7 @@ func startKicServiceTunnel(svc, configName string) { service.PrintServiceList(os.Stdout, data) openURLs(svc, urls) - out.T(out.Warning, "Because you are using docker driver on Mac, the terminal needs to be open to run it.") + out.WarningT("Because you are using docker driver on Mac, the terminal needs to be open to run it.") <-ctrlC diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 56326fa41f72..883b368f4282 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -434,7 +434,7 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string, machineName st path, err := exec.LookPath("kubectl") if err != nil { - out.T(out.Tip, "For best results, install kubectl: https://kubernetes.io/docs/tasks/tools/install-kubectl/") + out.ErrT(out.Tip, "For best results, install kubectl: https://kubernetes.io/docs/tasks/tools/install-kubectl/") return nil } @@ -454,9 +454,9 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string, machineName st if client.Major != cluster.Major || minorSkew > 1 { out.Ln("") - out.T(out.Warning, "{{.path}} is v{{.client_version}}, which may be incompatible with Kubernetes v{{.cluster_version}}.", + out.WarningT("{{.path}} is v{{.client_version}}, which may be incompatible with Kubernetes v{{.cluster_version}}.", out.V{"path": path, "client_version": client, "cluster_version": cluster}) - out.T(out.Tip, "You can also use 'minikube kubectl -- get pods' to invoke a matching version", + out.ErrT(out.Tip, "You can also use 'minikube kubectl -- get pods' to invoke a matching version", out.V{"path": path, "client_version": client}) } return nil @@ -464,7 +464,7 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string, machineName st 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}) + 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 { @@ -541,7 +541,7 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState { If vm-driver is set in the global config, please run "minikube config unset vm-driver" to resolve this warning. ` - out.T(out.Warning, warning, out.V{"driver": d, "vmd": vmd}) + out.WarningT(warning, out.V{"driver": d, "vmd": vmd}) } ds := driver.Status(d) if ds.Name == "" { @@ -744,9 +744,9 @@ func validateUser(drvName string) { return } - out.T(out.Stopped, `The "{{.driver_name}}" driver should not be used with root privileges.`, out.V{"driver_name": drvName}) - out.T(out.Tip, "If you are running minikube within a VM, consider using --driver=none:") - out.T(out.Documentation, " https://minikube.sigs.k8s.io/docs/reference/drivers/none/") + out.ErrT(out.Stopped, `The "{{.driver_name}}" driver should not be used with root privileges.`, out.V{"driver_name": drvName}) + out.ErrT(out.Tip, "If you are running minikube within a VM, consider using --driver=none:") + out.ErrT(out.Documentation, " https://minikube.sigs.k8s.io/docs/reference/drivers/none/") if !useForce { os.Exit(exit.Permissions) @@ -754,7 +754,7 @@ func validateUser(drvName string) { cname := ClusterFlagValue() _, err = config.Load(cname) if err == nil || !config.IsNotExist(err) { - out.T(out.Tip, "Tip: To remove this root owned cluster, run: sudo {{.cmd}}", out.V{"cmd": mustload.ExampleCmd(cname, "delete")}) + out.ErrT(out.Tip, "Tip: To remove this root owned cluster, run: sudo {{.cmd}}", out.V{"cmd": mustload.ExampleCmd(cname, "delete")}) } if !useForce { exit.WithCodeT(exit.Permissions, "Exiting") diff --git a/cmd/minikube/cmd/stop.go b/cmd/minikube/cmd/stop.go index 257f95a857e0..a0b90f878b67 100644 --- a/cmd/minikube/cmd/stop.go +++ b/cmd/minikube/cmd/stop.go @@ -59,7 +59,7 @@ func runStop(cmd *cobra.Command, args []string) { } if err := killMountProcess(); err != nil { - out.T(out.Warning, "Unable to kill mount process: {{.error}}", out.V{"error": err}) + out.WarningT("Unable to kill mount process: {{.error}}", out.V{"error": err}) } if err := kubeconfig.UnsetCurrentContext(cname, kubeconfig.PathFromEnv()); err != nil { 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/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index af6cd19c5031..a1265dbe24ba 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -248,7 +248,7 @@ func WarnIfSlow(arg ...string) ([]byte, error) { d := time.Since(start) if d > warnTime { out.WarningT(`Executing "{{.command}}" took an unusually long time: {{.duration}}`, out.V{"command": strings.Join(cmd.Args, " "), "duration": d}) - out.T(out.Tip, `Restarting the {{.name}} service may improve performance.`, out.V{"name": arg[0]}) + out.ErrT(out.Tip, `Restarting the {{.name}} service may improve performance.`, out.V{"name": arg[0]}) } if ctx.Err() == context.DeadlineExceeded { diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index ea2c15f6f2e0..0bf2f9b99456 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go @@ -292,7 +292,7 @@ func (k *Bootstrapper) StartCluster(cfg config.ClusterConfig) error { if rerr == nil { return nil } - out.T(out.Embarrassed, "Unable to restart cluster, will reset it: {{.error}}", out.V{"error": rerr}) + out.ErrT(out.Embarrassed, "Unable to restart cluster, will reset it: {{.error}}", out.V{"error": rerr}) if err := k.DeleteCluster(cfg.KubernetesConfig); err != nil { glog.Warningf("delete failed: %v", err) } @@ -309,7 +309,7 @@ func (k *Bootstrapper) StartCluster(cfg config.ClusterConfig) error { return nil } - out.T(out.Conflict, "initialization failed, will try again: {{.error}}", out.V{"error": err}) + out.ErrT(out.Conflict, "initialization failed, will try again: {{.error}}", out.V{"error": err}) if err := k.DeleteCluster(cfg.KubernetesConfig); err != nil { glog.Warningf("delete failed: %v", err) } diff --git a/pkg/minikube/exit/exit.go b/pkg/minikube/exit/exit.go index 221001f86ea3..3290363ef4a6 100644 --- a/pkg/minikube/exit/exit.go +++ b/pkg/minikube/exit/exit.go @@ -70,7 +70,7 @@ func WithError(msg string, err error) { // WithProblem outputs info related to a known problem and exits. func WithProblem(msg string, err error, p *problem.Problem) { out.ErrT(out.Empty, "") - out.ErrT(out.FailureType, "[{{.id}}] {{.msg}} {{.error}}", out.V{"msg": msg, "id": p.ID, "error": p.Err}) + out.FailureT("[{{.id}}] {{.msg}} {{.error}}", out.V{"msg": msg, "id": p.ID, "error": p.Err}) p.Display() if p.ShowIssueLink { out.ErrT(out.Empty, "") @@ -85,7 +85,7 @@ 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}) + out.FailureT("Problems detected in {{.entry}}:", out.V{"entry": name}) if len(lines) > MaxLogEntries { lines = lines[:MaxLogEntries] } diff --git a/pkg/minikube/logs/logs.go b/pkg/minikube/logs/logs.go index 97afcd59a4ad..aedf55457ddc 100644 --- a/pkg/minikube/logs/logs.go +++ b/pkg/minikube/logs/logs.go @@ -143,7 +143,7 @@ func FindProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg config.C // OutputProblems outputs discovered problems. func OutputProblems(problems map[string][]string, maxLines int) { for name, lines := range problems { - out.T(out.FailureType, "Problems detected in {{.name}}:", out.V{"name": name}) + out.FailureT("Problems detected in {{.name}}:", out.V{"name": name}) if len(lines) > maxLines { lines = lines[len(lines)-maxLines:] } diff --git a/pkg/minikube/machine/fix.go b/pkg/minikube/machine/fix.go index 7323b330893b..8aa74723e136 100644 --- a/pkg/minikube/machine/fix.go +++ b/pkg/minikube/machine/fix.go @@ -161,7 +161,7 @@ func maybeWarnAboutEvalEnv(drver string, name string) { } out.T(out.Notice, "Noticed you have an activated docker-env on {{.driver_name}} driver in this terminal:", out.V{"driver_name": drver}) // TODO: refactor docker-env package to generate only eval command per shell. https://github.com/kubernetes/minikube/issues/6887 - out.T(out.Warning, `Please re-eval your docker-env, To ensure your environment variables have updated ports: + out.WarningT(`Please re-eval your docker-env, To ensure your environment variables have updated ports: 'minikube -p {{.profile_name}} docker-env' diff --git a/pkg/minikube/mustload/mustload.go b/pkg/minikube/mustload/mustload.go index b6e24bdfac1a..7fb016b7500d 100644 --- a/pkg/minikube/mustload/mustload.go +++ b/pkg/minikube/mustload/mustload.go @@ -151,7 +151,7 @@ func Healthy(name string) ClusterController { as, err := kverify.APIServerStatus(co.CP.Runner, co.CP.ForwardedIP, co.CP.ForwardedPort) if err != nil { - out.T(out.FailureType, `Unable to get control plane status: {{.error}}`, out.V{"error": err}) + out.FailureT(`Unable to get control plane status: {{.error}}`, out.V{"error": err}) exitTip("delete", name, exit.Unavailable) } @@ -162,7 +162,7 @@ func Healthy(name string) ClusterController { if as != state.Running { out.T(out.Shrug, `This control plane is not running! (state={{.state}})`, out.V{"state": as.String()}) - out.T(out.Warning, `This is unusual - you may want to investigate using "{{.command}}"`, out.V{"command": ExampleCmd(name, "logs")}) + out.WarningT(`This is unusual - you may want to investigate using "{{.command}}"`, out.V{"command": ExampleCmd(name, "logs")}) exitTip("start", name, exit.Unavailable) } return co diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 37202613f7ea..fff3f9d7ead2 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -141,7 +141,7 @@ func Start(cc config.ClusterConfig, n config.Node, existingAddons map[string]boo configureMounts() if err := CacheAndLoadImagesInConfig(); err != nil { - out.T(out.FailureType, "Unable to load cached images from config file.") + out.FailureT("Unable to load cached images from config file.") } // enable addons, both old and new! @@ -211,7 +211,7 @@ func configureRuntimes(runner cruntime.CommandRunner, drvName string, k8s config if err := cr.Preload(k8s); err != nil { switch err.(type) { case *cruntime.ErrISOFeature: - out.T(out.Tip, "Existing disk is missing new features ({{.error}}). To upgrade, run 'minikube delete'", out.V{"error": err}) + out.ErrT(out.Tip, "Existing disk is missing new features ({{.error}}). To upgrade, run 'minikube delete'", out.V{"error": err}) default: glog.Warningf("%s preload failed: %v, falling back to caching images", cr.Name(), err) } @@ -314,7 +314,7 @@ func startMachine(cfg *config.ClusterConfig, node *config.Node) (runner command. // Bypass proxy for minikube's vm host ip err = proxy.ExcludeIP(ip) if err != nil { - out.ErrT(out.FailureType, "Failed to set NO_PROXY Env. Please use `export NO_PROXY=$NO_PROXY,{{.ip}}`.", out.V{"ip": ip}) + out.FailureT("Failed to set NO_PROXY Env. Please use `export NO_PROXY=$NO_PROXY,{{.ip}}`.", out.V{"ip": ip}) } // Save IP to config file for subsequent use @@ -333,7 +333,7 @@ func startHost(api libmachine.API, cc config.ClusterConfig, n config.Node) (*hos if err == nil { return host, exists } - out.T(out.Embarrassed, "StartHost failed, but will try again: {{.error}}", out.V{"error": err}) + out.ErrT(out.Embarrassed, "StartHost failed, but will try again: {{.error}}", out.V{"error": err}) // NOTE: People get very cranky if you delete their prexisting VM. Only delete new ones. if !exists { @@ -447,7 +447,7 @@ func tryRegistry(r command.Runner, driverName string) { if rr, err := r.RunCmd(exec.Command("curl", opts...)); err != nil { glog.Warningf("%s failed: %v", rr.Args, err) out.WarningT("This {{.type}} is having trouble accessing https://{{.repository}}", out.V{"repository": repo, "type": driver.MachineType(driverName)}) - out.T(out.Tip, "To pull new external images, you may need to configure a proxy: https://minikube.sigs.k8s.io/docs/reference/networking/proxy/") + out.ErrT(out.Tip, "To pull new external images, you may need to configure a proxy: https://minikube.sigs.k8s.io/docs/reference/networking/proxy/") } } @@ -455,11 +455,11 @@ func tryRegistry(r command.Runner, driverName string) { func prepareNone() { out.T(out.StartingNone, "Configuring local host environment ...") if viper.GetBool(config.WantNoneDriverWarning) { - out.T(out.Empty, "") + out.ErrT(out.Empty, "") out.WarningT("The 'none' driver provides limited isolation and may reduce system security and reliability.") out.WarningT("For more information, see:") - out.T(out.URL, "https://minikube.sigs.k8s.io/docs/reference/drivers/none/") - out.T(out.Empty, "") + out.ErrT(out.URL, "https://minikube.sigs.k8s.io/docs/reference/drivers/none/") + out.ErrT(out.Empty, "") } if os.Getenv("CHANGE_MINIKUBE_NONE_USER") == "" { @@ -467,12 +467,12 @@ func prepareNone() { out.WarningT("kubectl and minikube configuration will be stored in {{.home_folder}}", out.V{"home_folder": home}) out.WarningT("To use kubectl or minikube commands as your own user, you may need to relocate them. For example, to overwrite your own settings, run:") - out.T(out.Empty, "") - out.T(out.Command, "sudo mv {{.home_folder}}/.kube {{.home_folder}}/.minikube $HOME", out.V{"home_folder": home}) - out.T(out.Command, "sudo chown -R $USER $HOME/.kube $HOME/.minikube") - out.T(out.Empty, "") + out.ErrT(out.Empty, "") + out.ErrT(out.Command, "sudo mv {{.home_folder}}/.kube {{.home_folder}}/.minikube $HOME", out.V{"home_folder": home}) + out.ErrT(out.Command, "sudo chown -R $USER $HOME/.kube $HOME/.minikube") + out.ErrT(out.Empty, "") - out.T(out.Tip, "This can also be done automatically by setting the env var CHANGE_MINIKUBE_NONE_USER=true") + out.ErrT(out.Tip, "This can also be done automatically by setting the env var CHANGE_MINIKUBE_NONE_USER=true") } if err := util.MaybeChownDirRecursiveToMinikubeUser(localpath.MiniPath()); err != nil { From 5cde362650c55fb5451540abc53ccb0d0f9fdfad Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 2 Apr 2020 10:03:12 -0700 Subject: [PATCH 2/4] Revert unrelated go.mod change --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index 2a00c7499246..29a5ac30d5c2 100644 --- a/go.mod +++ b/go.mod @@ -82,7 +82,6 @@ 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 From ae52a11692b4d478b4bf13beff3e05572128f499 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 2 Apr 2020 12:27:19 -0700 Subject: [PATCH 3/4] Swap stdout/stderr in test --- test/integration/functional_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 6623f1f36202..1571328322aa 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -481,7 +481,7 @@ func validateConfigCmd(ctx context.Context, t *testing.T, profile string) { }{ {[]string{"unset", "cpus"}, "", ""}, {[]string{"get", "cpus"}, "", "Error: specified key could not be found in config"}, - {[]string{"set", "cpus", "2"}, "! These changes will take effect upon a minikube delete and then a minikube start", ""}, + {[]string{"set", "cpus", "2"}, "", "! These changes will take effect upon a minikube delete and then a minikube start"}, {[]string{"get", "cpus"}, "2", ""}, {[]string{"unset", "cpus"}, "", ""}, {[]string{"get", "cpus"}, "", "Error: specified key could not be found in config"}, From 5b3834ec63b53c5a27f42ed2079103d9902a4691 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 2 Apr 2020 12:39:32 -0700 Subject: [PATCH 4/4] Merge to master --- cmd/minikube/cmd/start.go | 10 ++++++++-- go.mod | 1 + pkg/minikube/driver/driver.go | 21 +++++++++++++++++---- pkg/minikube/driver/driver_test.go | 16 +++++++++++++++- pkg/minikube/node/start.go | 6 +++--- pkg/minikube/out/style.go | 1 + pkg/minikube/out/style_enum.go | 1 + pkg/minikube/registry/global.go | 2 ++ test/integration/functional_test.go | 4 ++-- 9 files changed, 50 insertions(+), 12 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 883b368f4282..5330c98026d2 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -561,9 +561,15 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState { return ds } - pick, alts := driver.Suggest(driver.Choices(viper.GetBool("vm"))) + choices := driver.Choices(viper.GetBool("vm")) + pick, alts, rejects := driver.Suggest(choices) if pick.Name == "" { - exit.WithCodeT(exit.Config, "Unable to determine a default driver to use. Try specifying --driver, or see https://minikube.sigs.k8s.io/docs/start/") + out.T(out.ThumbsDown, "Unable to pick a default driver. Here is what was considered, in preference order:") + for _, r := range rejects { + out.T(out.Option, "{{ .name }}: {{ .rejection }}", out.V{"name": r.Name, "rejection": r.Rejection}) + } + out.T(out.Workaround, "Try specifying a --driver, or see https://minikube.sigs.k8s.io/docs/start/") + os.Exit(exit.Unavailable) } if len(alts) > 1 { 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/driver/driver.go b/pkg/minikube/driver/driver.go index ed0cbdee236a..7f67a76fe754 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -174,8 +174,8 @@ func Choices(vm bool) []registry.DriverState { return options } -// Suggest returns a suggested driver from a set of options -func Suggest(options []registry.DriverState) (registry.DriverState, []registry.DriverState) { +// Suggest returns a suggested driver, alternate drivers, and rejected drivers +func Suggest(options []registry.DriverState) (registry.DriverState, []registry.DriverState, []registry.DriverState) { pick := registry.DriverState{} for _, ds := range options { if !ds.State.Installed { @@ -198,17 +198,30 @@ func Suggest(options []registry.DriverState) (registry.DriverState, []registry.D } alternates := []registry.DriverState{} + rejects := []registry.DriverState{} for _, ds := range options { if ds != pick { - if !ds.State.Healthy || !ds.State.Installed { + glog.Errorf("%s: %s", ds.Name, ds.Rejection) + if !ds.State.Installed { + ds.Rejection = fmt.Sprintf("Not installed: %v", ds.State.Error) + rejects = append(rejects, ds) continue } + + if !ds.State.Healthy { + ds.Rejection = fmt.Sprintf("Not healthy: %v", ds.State.Error) + rejects = append(rejects, ds) + continue + } + + ds.Rejection = fmt.Sprintf("%s is preferred", pick.Name) alternates = append(alternates, ds) } } glog.Infof("Picked: %+v", pick) glog.Infof("Alternatives: %+v", alternates) - return pick, alternates + glog.Infof("Rejects: %+v", rejects) + return pick, alternates, rejects } // Status returns the status of a driver diff --git a/pkg/minikube/driver/driver_test.go b/pkg/minikube/driver/driver_test.go index 5d0bfd400924..b6afd6a62c9e 100644 --- a/pkg/minikube/driver/driver_test.go +++ b/pkg/minikube/driver/driver_test.go @@ -112,6 +112,7 @@ func TestSuggest(t *testing.T) { choices []string pick string alts []string + rejects []string }{ { def: registry.DriverDef{ @@ -122,6 +123,7 @@ func TestSuggest(t *testing.T) { choices: []string{"unhealthy"}, pick: "", alts: []string{}, + rejects: []string{"unhealthy"}, }, { def: registry.DriverDef{ @@ -132,6 +134,7 @@ func TestSuggest(t *testing.T) { choices: []string{"discouraged", "unhealthy"}, pick: "", alts: []string{"discouraged"}, + rejects: []string{"unhealthy"}, }, { def: registry.DriverDef{ @@ -142,6 +145,7 @@ func TestSuggest(t *testing.T) { choices: []string{"default", "discouraged", "unhealthy"}, pick: "default", alts: []string{"discouraged"}, + rejects: []string{"unhealthy"}, }, { def: registry.DriverDef{ @@ -152,6 +156,7 @@ func TestSuggest(t *testing.T) { choices: []string{"preferred", "default", "discouraged", "unhealthy"}, pick: "preferred", alts: []string{"default", "discouraged"}, + rejects: []string{"unhealthy"}, }, } for _, tc := range tests { @@ -172,7 +177,7 @@ func TestSuggest(t *testing.T) { t.Errorf("choices mismatch (-want +got):\n%s", diff) } - pick, alts := Suggest(got) + pick, alts, rejects := Suggest(got) if pick.Name != tc.pick { t.Errorf("pick = %q, expected %q", pick.Name, tc.pick) } @@ -184,6 +189,15 @@ func TestSuggest(t *testing.T) { if diff := cmp.Diff(gotAlts, tc.alts); diff != "" { t.Errorf("alts mismatch (-want +got):\n%s", diff) } + + gotRejects := []string{} + for _, r := range rejects { + gotRejects = append(gotRejects, r.Name) + } + if diff := cmp.Diff(gotRejects, tc.rejects); diff != "" { + t.Errorf("rejects mismatch (-want +got):\n%s", diff) + } + }) } } diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index fff3f9d7ead2..f2da83ecd4d6 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -456,9 +456,9 @@ func prepareNone() { out.T(out.StartingNone, "Configuring local host environment ...") if viper.GetBool(config.WantNoneDriverWarning) { out.ErrT(out.Empty, "") - out.WarningT("The 'none' driver provides limited isolation and may reduce system security and reliability.") - out.WarningT("For more information, see:") - out.ErrT(out.URL, "https://minikube.sigs.k8s.io/docs/reference/drivers/none/") + out.WarningT("The 'none' driver is designed for experts who need to integrate with an existing VM") + out.ErrT(out.Tip, "Most users should use the newer 'docker' driver instead, which does not require root!") + out.ErrT(out.Documentation, "For more information, see: https://minikube.sigs.k8s.io/docs/reference/drivers/none/") out.ErrT(out.Empty, "") } diff --git a/pkg/minikube/out/style.go b/pkg/minikube/out/style.go index c67188e12dc3..a8eee7811e0c 100644 --- a/pkg/minikube/out/style.go +++ b/pkg/minikube/out/style.go @@ -68,6 +68,7 @@ var styles = map[StyleEnum]style{ Launch: {Prefix: "🚀 "}, Sad: {Prefix: "😿 "}, ThumbsUp: {Prefix: "👍 "}, + ThumbsDown: {Prefix: "👎 "}, Option: {Prefix: " ▪ ", LowPrefix: lowIndent}, // Indented bullet Command: {Prefix: " ▪ ", LowPrefix: lowIndent}, // Indented bullet LogEntry: {Prefix: " "}, // Indent diff --git a/pkg/minikube/out/style_enum.go b/pkg/minikube/out/style_enum.go index 097f4452dc72..1437b26823a9 100644 --- a/pkg/minikube/out/style_enum.go +++ b/pkg/minikube/out/style_enum.go @@ -41,6 +41,7 @@ const ( Launch Sad ThumbsUp + ThumbsDown Option Command LogEntry diff --git a/pkg/minikube/registry/global.go b/pkg/minikube/registry/global.go index 16ede79a27eb..3f608ef4712c 100644 --- a/pkg/minikube/registry/global.go +++ b/pkg/minikube/registry/global.go @@ -68,6 +68,8 @@ type DriverState struct { Name string Priority Priority State State + // Rejection is why we chose not to use this driver + Rejection string } func (d DriverState) String() string { diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 1571328322aa..702addce46db 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -774,7 +774,7 @@ func validateFileSync(ctx context.Context, t *testing.T, profile string) { vp := vmSyncTestPath() t.Logf("Checking for existence of %s within VM", vp) - rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", fmt.Sprintf("cat %s", vp))) + rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", fmt.Sprintf("sudo cat %s", vp))) if err != nil { t.Errorf("%s failed: %v", rr.Command(), err) } @@ -811,7 +811,7 @@ func validateCertSync(ctx context.Context, t *testing.T, profile string) { } for _, vp := range paths { t.Logf("Checking for existence of %s within VM", vp) - rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", fmt.Sprintf("cat %s", vp))) + rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", fmt.Sprintf("sudo cat %s", vp))) if err != nil { t.Errorf("failed to check existence of %q inside minikube. args %q: %v", vp, rr.Command(), err) }