Merge pull request #3680 from tstromberg/better-none

none UX: Reword warnings and other strings for accuracy
kubernetes · Feb 15, 2019 · 4142c6d · 4142c6d
2 parents 32fbdbd + b80498f
commit 4142c6d
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 174 deletions.
diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go
@@ -52,6 +52,11 @@ var dashboardCmd = &cobra.Command{
 	Short: "Access the kubernetes dashboard running within the minikube cluster",
 	Long:  `Access the kubernetes dashboard running within the minikube cluster`,
 	Run: func(cmd *cobra.Command, args []string) {
+		kubectl, err := exec.LookPath("kubectl")
+		if err != nil {
+			exit.WithCode(exit.NoInput, "kubectl not found in PATH, but is required for the dashboard. Installation guide: https://kubernetes.io/docs/tasks/tools/install-kubectl/")
+		}
+
 		api, err := machine.NewAPIClient()
 		defer func() {
 			err := api.Close()
@@ -81,7 +86,7 @@ var dashboardCmd = &cobra.Command{
 		}
 
 		console.ErrStyle("launch", "Launching proxy ...")
-		p, hostPort, err := kubectlProxy()
+		p, hostPort, err := kubectlProxy(kubectl)
 		if err != nil {
 			exit.WithError("kubectl proxy", err)
 		}
@@ -109,12 +114,7 @@ var dashboardCmd = &cobra.Command{
 }
 
 // kubectlProxy runs "kubectl proxy", returning host:port
-func kubectlProxy() (*exec.Cmd, string, error) {
-	path, err := exec.LookPath("kubectl")
-	if err != nil {
-		return nil, "", errors.Wrap(err, "kubectl not found in PATH")
-	}
-
+func kubectlProxy(path string) (*exec.Cmd, string, error) {
 	// port=0 picks a random system port
 	// config.GetMachineName() respects the -p (profile) flag
 	cmd := exec.Command(path, "--context", config.GetMachineName(), "proxy", "--port=0")

diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go
@@ -61,7 +61,9 @@ associated files.`,
 			bsName := viper.GetString(cmdcfg.Bootstrapper)
 			console.OutStyle("resetting", "Uninstalling Kubernetes %s using %s ...", kc.KubernetesVersion, bsName)
 			clusterBootstrapper, err := GetClusterBootstrapper(api, viper.GetString(cmdcfg.Bootstrapper))
-			if err == nil {
+			if err != nil {
+				console.ErrLn("Unable to get bootstrapper: %v", err)
+			} else {
 				if err = clusterBootstrapper.DeleteCluster(kc); err != nil {
 					console.ErrLn("Failed to delete cluster: %v", err)
 				}
@@ -71,9 +73,9 @@ associated files.`,
 		if err = cluster.DeleteHost(api); err != nil {
 			switch err := errors.Cause(err).(type) {
 			case mcnerror.ErrHostDoesNotExist:
-				console.OutStyle("meh", "%q VM does not exist", profile)
+				console.OutStyle("meh", "%q cluster does not exist", profile)
 			default:
-				exit.WithError("Failed to delete VM", err)
+				exit.WithError("Failed to delete cluster", err)
 			}
 		}
 

diff --git a/cmd/minikube/cmd/root.go b/cmd/minikube/cmd/root.go
@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"runtime"
 	"strings"
 
 	"github.com/docker/machine/libmachine"
@@ -32,7 +31,6 @@ import (
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	configCmd "k8s.io/minikube/cmd/minikube/cmd/config"
-	"k8s.io/minikube/cmd/util"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm"
 	"k8s.io/minikube/pkg/minikube/config"
@@ -94,7 +92,6 @@ var RootCmd = &cobra.Command{
 		if enableUpdateNotification {
 			notify.MaybePrintUpdateTextFromGithub(os.Stderr)
 		}
-		util.MaybePrintKubectlDownloadMsg(runtime.GOOS, os.Stderr)
 	},
 }
 

diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go
@@ -211,6 +211,10 @@ func runStart(cmd *cobra.Command, args []string) {
 	} else {
 		console.OutStyle("kubectl", "kubectl is now configured to use %q", cfg.GetMachineName())
 	}
+	_, err = exec.LookPath("kubectl")
+	if err != nil {
+		console.OutStyle("tip", "For best results, install kubectl: https://kubernetes.io/docs/tasks/tools/install-kubectl/")
+	}
 	console.OutStyle("ready", "Done! Thank you for using minikube!")
 }
 
@@ -303,22 +307,25 @@ func generateConfig(cmd *cobra.Command, kVersion string) (cfg.Config, error) {
 // prepareNone prepares the user and host for the joy of the "none" driver
 func prepareNone() {
 	if viper.GetBool(cfg.WantNoneDriverWarning) {
-		console.ErrLn(`===================
-WARNING: IT IS RECOMMENDED NOT TO RUN THE NONE DRIVER ON PERSONAL WORKSTATIONS
-The 'none' driver will run an insecure kubernetes apiserver as root that may leave the host vulnerable to CSRF attacks` + "\n")
+		console.OutLn("")
+		console.Warning("The 'none' driver provides limited isolation and may reduce system security and reliability.")
+		console.Warning("For more information, see:")
+		console.OutStyle("url", "https://github.com/kubernetes/minikube/blob/master/docs/vmdriver-none.md")
+		console.OutLn("")
 	}
 
 	if os.Getenv("CHANGE_MINIKUBE_NONE_USER") == "" {
-		console.Fatal(`When using the none driver, the kubectl config and credentials generated will be root owned and will appear in the root home directory.
-You will need to move the files to the appropriate location and then set the correct permissions.  An example of this is below:
-
-sudo mv /root/.kube $HOME/.kube # this will write over any previous configuration
-sudo chown -R $USER:$USER $HOME/.kube
+		home := os.Getenv("HOME")
+		console.Warning("kubectl and minikube configuration will be stored in %s", home)
+		console.Warning("To use kubectl or minikube commands as your own user, you may")
+		console.Warning("need to relocate them. For example, to overwrite your own settings:")
 
-sudo mv /root/.minikube $HOME/.minikube # this will write over any previous configuration
-sudo chown -R $USER:$USER $HOME/.minikube
+		console.OutLn("")
+		console.OutStyle("command", "sudo mv %s/.kube %s/.minikube $HOME", home, home)
+		console.OutStyle("command", "sudo chown -R $USER %s/.kube %s/.minikube", home, home)
+		console.OutLn("")
 
-This can also be done automatically by setting the env var CHANGE_MINIKUBE_NONE_USER=true`)
+		console.OutStyle("tip", "This can also be done automatically by setting the env var CHANGE_MINIKUBE_NONE_USER=true")
 	}
 
 	if err := pkgutil.MaybeChownDirRecursiveToMinikubeUser(constants.GetMinipath()); err != nil {

diff --git a/cmd/util/util.go b/cmd/util/util.go
@@ -18,70 +18,16 @@ limitations under the License.
 package util
 
 import (
-	"fmt"
-	"io"
 	"io/ioutil"
 	"net"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"runtime"
 	"strconv"
 
 	"github.com/pkg/errors"
-	"github.com/spf13/viper"
-	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 )
 
-type ServiceContext struct {
-	Service string `json:"service"`
-	Version string `json:"version"`
-}
-
-type LookPath func(filename string) (string, error)
-
-var lookPath LookPath
-
-func init() {
-	lookPath = exec.LookPath
-}
-
-func MaybePrintKubectlDownloadMsg(goos string, out io.Writer) {
-	if !viper.GetBool(config.WantKubectlDownloadMsg) {
-		return
-	}
-
-	verb := "run"
-	installInstructions := "curl -Lo kubectl https://storage.googleapis.com/kubernetes-release/release/%s/bin/%s/%s/kubectl && chmod +x kubectl && sudo cp kubectl /usr/local/bin/ && rm kubectl"
-	if goos == "windows" {
-		verb = "do"
-		installInstructions = `download kubectl from:
-https://storage.googleapis.com/kubernetes-release/release/%s/bin/%s/%s/kubectl.exe
-Add kubectl to your system PATH`
-	}
-
-	_, err := lookPath("kubectl")
-	if err != nil && goos == "windows" {
-		_, err = lookPath("kubectl.exe")
-	}
-	if err != nil {
-		fmt.Fprintf(out,
-			`========================================
-kubectl could not be found on your path. kubectl is a requirement for using minikube
-To install kubectl, please %s the following:
-
-%s
-
-To disable this message, run the following:
-
-minikube config set WantKubectlDownloadMsg false
-========================================
-`,
-			verb, fmt.Sprintf(installInstructions, constants.DefaultKubernetesVersion, goos, runtime.GOARCH))
-	}
-}
-
 // Ask the kernel for a free open port that is ready to use
 func GetPort() (string, error) {
 	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")

diff --git a/cmd/util/util_test.go b/cmd/util/util_test.go
@@ -17,108 +17,12 @@ limitations under the License.
 package util
 
 import (
-	"bytes"
-	"fmt"
-	"net/http"
-	"net/http/httptest"
 	"os"
-	"strings"
 	"testing"
 
-	"github.com/pkg/errors"
-	"github.com/spf13/viper"
 	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/minikube/pkg/minikube/config"
 )
 
-func startTestHTTPServer(returnError bool, response string) *httptest.Server {
-	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if returnError {
-			http.Error(w, response, 400)
-		} else {
-			fmt.Fprintf(w, response)
-		}
-	}))
-}
-
-func revertLookPath(l LookPath) {
-	lookPath = l
-}
-
-func fakeLookPathFound(string) (string, error) { return "/usr/local/bin/kubectl", nil }
-func fakeLookPathError(string) (string, error) { return "", errors.New("") }
-
-func TestKubectlDownloadMsg(t *testing.T) {
-	var tests = []struct {
-		description    string
-		lp             LookPath
-		goos           string
-		matches        string
-		noOutput       bool
-		warningEnabled bool
-	}{
-		{
-			description:    "No output when binary is found windows",
-			goos:           "windows",
-			lp:             fakeLookPathFound,
-			noOutput:       true,
-			warningEnabled: true,
-		},
-		{
-			description:    "No output when binary is found darwin",
-			goos:           "darwin",
-			lp:             fakeLookPathFound,
-			noOutput:       true,
-			warningEnabled: true,
-		},
-		{
-			description:    "windows kubectl not found, has .exe in output",
-			goos:           "windows",
-			lp:             fakeLookPathError,
-			matches:        ".exe",
-			warningEnabled: true,
-		},
-		{
-			description:    "linux kubectl not found",
-			goos:           "linux",
-			lp:             fakeLookPathError,
-			matches:        "WantKubectlDownloadMsg",
-			warningEnabled: true,
-		},
-		{
-			description:    "warning disabled",
-			goos:           "linux",
-			lp:             fakeLookPathError,
-			noOutput:       true,
-			warningEnabled: false,
-		},
-	}
-
-	for _, test := range tests {
-		test := test
-		t.Run(test.description, func(t *testing.T) {
-			defer revertLookPath(lookPath)
-
-			// Remember the original config value and revert to it.
-			origConfig := viper.GetBool(config.WantKubectlDownloadMsg)
-			defer func() {
-				viper.Set(config.WantKubectlDownloadMsg, origConfig)
-			}()
-			viper.Set(config.WantKubectlDownloadMsg, test.warningEnabled)
-			lookPath = test.lp
-			var b bytes.Buffer
-			MaybePrintKubectlDownloadMsg(test.goos, &b)
-			actual := b.String()
-			if actual != "" && test.noOutput {
-				t.Errorf("Got output, but kubectl binary was found")
-			}
-			if !strings.Contains(actual, test.matches) {
-				t.Errorf("Output did not contain substring expected got output %s", actual)
-			}
-		})
-	}
-}
-
 func TestGetKubeConfigPath(t *testing.T) {
 	var tests = []struct {
 		input string

diff --git a/pkg/minikube/console/style.go b/pkg/minikube/console/style.go
@@ -60,6 +60,7 @@ var styles = map[string]style{
 	"sad":          {Prefix: "😿  ", LowPrefix: "*   "},
 	"thumbs-up":    {Prefix: "👍  "},
 	"option":       {Prefix: "    ▪ "}, // Indented bullet
+	"command":      {Prefix: "    ▪ "}, // Indented bullet
 	"log-entry":    {Prefix: "    "},   // Indent
 	"crushed":      {Prefix: "💔  "},
 	"url":          {Prefix: "👉  "},