Merge pull request #5422 from afbjorklund/reportcard-1.4

Improve reportcard

cmd/minikube/cmd/delete.go

@@ -88,14 +88,7 @@ func runDelete(cmd *cobra.Command, args []string) {
 	}
 
 	// In case DeleteHost didn't complete the job.
-	machineDir := filepath.Join(localpath.MiniPath(), "machines", profile)
-	if _, err := os.Stat(machineDir); err == nil {
-		out.T(out.DeletingHost, `Removing {{.directory}} ...`, out.V{"directory": machineDir})
-		err := os.RemoveAll(machineDir)
-		if err != nil {
-			exit.WithError("Unable to remove machine directory: %v", err)
-		}
-	}
+	deleteProfileDirectory(profile)
 
 	if err := pkg_config.DeleteProfile(profile); err != nil {
 		if os.IsNotExist(err) {
@@ -126,6 +119,17 @@ func uninstallKubernetes(api libmachine.API, kc pkg_config.KubernetesConfig, bsN
 	}
 }
 
+func deleteProfileDirectory(profile string) {
+	machineDir := filepath.Join(localpath.MiniPath(), "machines", profile)
+	if _, err := os.Stat(machineDir); err == nil {
+		out.T(out.DeletingHost, `Removing {{.directory}} ...`, out.V{"directory": machineDir})
+		err := os.RemoveAll(machineDir)
+		if err != nil {
+			exit.WithError("Unable to remove machine directory: %v", err)
+		}
+	}
+}
+
 // killMountProcess kills the mount process, if it is running
 func killMountProcess() error {
 	pidPath := filepath.Join(localpath.MiniPath(), constants.MountProcessFileName)

cmd/minikube/cmd/start.go

@@ -257,17 +257,7 @@ func platform() string {
 
 // runStart handles the executes the flow of "minikube start"
 func runStart(cmd *cobra.Command, args []string) {
-	prefix := ""
-	if viper.GetString(cfg.MachineProfile) != constants.DefaultMachineName {
-		prefix = fmt.Sprintf("[%s] ", viper.GetString(cfg.MachineProfile))
-	}
-
-	versionState := out.Happy
-	if notify.MaybePrintUpdateTextFromGithub() {
-		versionState = out.Meh
-	}
-
-	out.T(versionState, "{{.prefix}}minikube {{.version}} on {{.platform}}", out.V{"prefix": prefix, "version": version.GetVersion(), "platform": platform()})
+	displayVersion(version.GetVersion())
 	displayEnviron(os.Environ())
 
 	// if --registry-mirror specified when run minikube start,
@@ -295,13 +285,7 @@ func runStart(cmd *cobra.Command, args []string) {
 	validateFlags(driver)
 	validateUser(driver)
 
-	v, err := version.GetSemverVersion()
-	if err != nil {
-		out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err})
-	} else if err := drivers.InstallOrUpdate(driver, localpath.MakeMiniPath("bin"), v, viper.GetBool(interactive)); err != nil {
-		out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driver, "error": err})
-	}
-
+	_ = getMinikubeVersion(driver)
 	k8sVersion, isUpgrade := getKubernetesVersion(oldConfig)
 	config, err := generateCfgFromFlags(cmd, k8sVersion, driver)
 	if err != nil {
@@ -368,6 +352,20 @@ func runStart(cmd *cobra.Command, args []string) {
 	showKubectlConnectInfo(kubeconfig)
 }
 
+func displayVersion(version string) {
+	prefix := ""
+	if viper.GetString(cfg.MachineProfile) != constants.DefaultMachineName {
+		prefix = fmt.Sprintf("[%s] ", viper.GetString(cfg.MachineProfile))
+	}
+
+	versionState := out.Happy
+	if notify.MaybePrintUpdateTextFromGithub() {
+		versionState = out.Meh
+	}
+
+	out.T(versionState, "{{.prefix}}minikube {{.version}} on {{.platform}}", out.V{"prefix": prefix, "version": version, "platform": platform()})
+}
+
 // displayEnviron makes the user aware of environment variables that will affect how minikube operates
 func displayEnviron(env []string) {
 	for _, kv := range env {
@@ -727,23 +725,8 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, driver string)
 	}
 
 	// Feed Docker our host proxy environment by default, so that it can pull images
-	if _, ok := r.(*cruntime.Docker); ok {
-		if !cmd.Flags().Changed("docker-env") {
-			for _, k := range proxy.EnvVars {
-				if v := os.Getenv(k); v != "" {
-					// convert https_proxy to HTTPS_PROXY for linux
-					// TODO (@medyagh): if user has both http_proxy & HTTPS_PROXY set merge them.
-					k = strings.ToUpper(k)
-					if k == "HTTP_PROXY" || k == "HTTPS_PROXY" {
-						if strings.HasPrefix(v, "localhost") || strings.HasPrefix(v, "127.0") {
-							out.WarningT("Not passing {{.name}}={{.value}} to docker env.", out.V{"name": k, "value": v})
-							continue
-						}
-					}
-					dockerEnv = append(dockerEnv, fmt.Sprintf("%s=%s", k, v))
-				}
-			}
-		}
+	if _, ok := r.(*cruntime.Docker); ok && !cmd.Flags().Changed("docker-env") {
+		setDockerProxy()
 	}
 
 	repository := viper.GetString(imageRepository)
@@ -822,6 +805,24 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, driver string)
 	return cfg, nil
 }
 
+// setDockerProxy sets the proxy environment variables in the docker environment.
+func setDockerProxy() {
+	for _, k := range proxy.EnvVars {
+		if v := os.Getenv(k); v != "" {
+			// convert https_proxy to HTTPS_PROXY for linux
+			// TODO (@medyagh): if user has both http_proxy & HTTPS_PROXY set merge them.
+			k = strings.ToUpper(k)
+			if k == "HTTP_PROXY" || k == "HTTPS_PROXY" {
+				if strings.HasPrefix(v, "localhost") || strings.HasPrefix(v, "127.0") {
+					out.WarningT("Not passing {{.name}}={{.value}} to docker env.", out.V{"name": k, "value": v})
+					continue
+				}
+			}
+			dockerEnv = append(dockerEnv, fmt.Sprintf("%s=%s", k, v))
+		}
+	}
+}
+
 // autoSetDriverOptions sets the options needed for specific vm-driver automatically.
 func autoSetDriverOptions(driver string) error {
 	if driver == constants.DriverNone {
@@ -916,6 +917,17 @@ func validateNetwork(h *host.Host) string {
 	return ip
 }
 
+// getMinikubeVersion ensures that the driver binary is up to date
+func getMinikubeVersion(driver string) string {
+	v, err := version.GetSemverVersion()
+	if err != nil {
+		out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err})
+	} else if err := drivers.InstallOrUpdate(driver, localpath.MakeMiniPath("bin"), v, viper.GetBool(interactive)); err != nil {
+		out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driver, "error": err})
+	}
+	return v.String()
+}
+
 // getKubernetesVersion ensures that the requested version is reasonable
 func getKubernetesVersion(old *cfg.Config) (string, bool) {
 	rawVersion := viper.GetString(kubernetesVersion)

pkg/drivers/hyperkit/driver.go

@@ -198,19 +198,11 @@ func (d *Driver) Restart() error {
 	return pkgdrivers.Restart(d)
 }
 
-// Start a host
-func (d *Driver) Start() error {
-	if err := d.verifyRootPermissions(); err != nil {
-		return err
-	}
-
+func (d *Driver) createHost() (*hyperkit.HyperKit, error) {
 	stateDir := filepath.Join(d.StorePath, "machines", d.MachineName)
-	if err := d.recoverFromUncleanShutdown(); err != nil {
-		return err
-	}
 	h, err := hyperkit.New("", d.VpnKitSock, stateDir)
 	if err != nil {
-		return errors.Wrap(err, "new-ing Hyperkit")
+		return nil, errors.Wrap(err, "new-ing Hyperkit")
 	}
 
 	// TODO: handle the rest of our settings.
@@ -227,12 +219,38 @@ func (d *Driver) Start() error {
 	h.SetLogger(logger)
 
 	if vsockPorts, err := d.extractVSockPorts(); err != nil {
-		return err
+		return nil, err
 	} else if len(vsockPorts) >= 1 {
 		h.VSock = true
 		h.VSockPorts = vsockPorts
 	}
 
+	h.Disks = []hyperkit.DiskConfig{
+		{
+			Path:   pkgdrivers.GetDiskPath(d.BaseDriver),
+			Size:   d.DiskSize,
+			Driver: "virtio-blk",
+		},
+	}
+
+	return h, nil
+}
+
+// Start a host
+func (d *Driver) Start() error {
+	if err := d.verifyRootPermissions(); err != nil {
+		return err
+	}
+
+	if err := d.recoverFromUncleanShutdown(); err != nil {
+		return err
+	}
+
+	h, err := d.createHost()
+	if err != nil {
+		return err
+	}
+
 	log.Debugf("Using UUID %s", h.UUID)
 	mac, err := GetMACAddressFromUUID(h.UUID)
 	if err != nil {
@@ -242,18 +260,23 @@ func (d *Driver) Start() error {
 	// Need to strip 0's
 	mac = trimMacAddress(mac)
 	log.Debugf("Generated MAC %s", mac)
-	h.Disks = []hyperkit.DiskConfig{
-		{
-			Path:   pkgdrivers.GetDiskPath(d.BaseDriver),
-			Size:   d.DiskSize,
-			Driver: "virtio-blk",
-		},
-	}
+
 	log.Debugf("Starting with cmdline: %s", d.Cmdline)
 	if err := h.Start(d.Cmdline); err != nil {
 		return errors.Wrapf(err, "starting with cmd line: %s", d.Cmdline)
 	}
 
+	if err := d.setupIP(mac); err != nil {
+		return err
+	}
+
+	if err := d.setupNFSMounts(); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (d *Driver) setupIP(mac string) error {
 	getIP := func() error {
 		st, err := d.GetState()
 		if err != nil {
@@ -270,6 +293,8 @@ func (d *Driver) Start() error {
 		return nil
 	}
 
+	var err error
+
 	// Implement a retry loop without calling any minikube code
 	for i := 0; i < 30; i++ {
 		log.Debugf("Attempt %d", i)
@@ -288,6 +313,12 @@ func (d *Driver) Start() error {
 	}
 	log.Debugf("IP: %s", d.IPAddress)
 
+	return nil
+}
+
+func (d *Driver) setupNFSMounts() error {
+	var err error
+
 	if len(d.NFSShares) > 0 {
 		log.Info("Setting up NFS mounts")
 		// takes some time here for ssh / nfsd to work properly

pkg/minikube/bootstrapper/certs.go

@@ -45,8 +45,10 @@ import (
 )
 
 const (
+	// CACertificatesDir contains CA certificates
 	CACertificatesDir = "/usr/share/ca-certificates"
-	SSLCertStoreDir   = "/etc/ssl/certs"
+	// SSLCertStoreDir contains SSL certificates
+	SSLCertStoreDir = "/etc/ssl/certs"
 )
 
 var (

pkg/minikube/config/profile.go

@@ -50,7 +50,7 @@ func ProfileExists(name string, miniHome ...string) bool {
 	return err == nil
 }
 
-// CreateProfile creates an empty profile stores in $MINIKUBE_HOME/profiles/<profilename>/config.json
+// CreateEmptyProfile creates an empty profile stores in $MINIKUBE_HOME/profiles/<profilename>/config.json
 func CreateEmptyProfile(name string, miniHome ...string) error {
 	cfg := &Config{}
 	return CreateProfile(name, cfg, miniHome...)
@@ -100,6 +100,7 @@ func CreateProfile(name string, cfg *Config, miniHome ...string) error {
 	return nil
 }
 
+// DeleteProfile deletes a profile and removes the profile dir
 func DeleteProfile(profile string, miniHome ...string) error {
 	miniPath := localpath.MiniPath()
 	if len(miniHome) > 0 {

pkg/minikube/constants/constants.go

@@ -28,8 +28,11 @@ import (
 )
 
 const (
-	APIServerPort    = 8443
-	APIServerName    = "minikubeCA"
+	// APIServerPort is the default API server port
+	APIServerPort = 8443
+	// APIServerName is the default API server name
+	APIServerName = "minikubeCA"
+	// ClusterDNSDomain is the default DNS domain
 	ClusterDNSDomain = "cluster.local"
 )
 
@@ -167,7 +170,7 @@ const (
 	GuestManifestsDir = "/etc/kubernetes/manifests"
 	// GuestEphemeralDir is the path where ephemeral data should be stored within the VM
 	GuestEphemeralDir = "/var/tmp/minikube"
-	// PersistentDir is the path where persistent data should be stored within the VM (not tmpfs)
+	// GuestPersistentDir is the path where persistent data should be stored within the VM (not tmpfs)
 	GuestPersistentDir = "/var/lib/minikube"
 	// GuestCertsDir are where Kubernetes certificates are kept on the guest
 	GuestCertsDir = GuestPersistentDir + "/certs"

pkg/minikube/kubeconfig/kubeconfig.go

@@ -82,7 +82,7 @@ func Port(clusterName string, configPath ...string) (int, error) {
 	return port, err
 }
 
-// PathFromEnv() gets the path to the first kubeconfig
+// PathFromEnv gets the path to the first kubeconfig
 func PathFromEnv() string {
 	kubeConfigEnv := os.Getenv(constants.KubeconfigEnvVar)
 	if kubeConfigEnv == "" {

pkg/minikube/kubeconfig/settings.go

@@ -66,7 +66,7 @@ func (k *Settings) filePath() string {
 	return k.kubeConfigFile.Load().(string)
 }
 
-// Populate populates an api.Config object with values from *Settings
+// PopulateFromSettings populates an api.Config object with values from *Settings
 func PopulateFromSettings(cfg *Settings, apiCfg *api.Config) error {
 	var err error
 	clusterName := cfg.ClusterName
@@ -115,7 +115,7 @@ func PopulateFromSettings(cfg *Settings, apiCfg *api.Config) error {
 	return nil
 }
 
-// update reads config from disk, adds the minikube settings, and writes it back.
+// Update reads config from disk, adds the minikube settings, and writes it back.
 // activeContext is true when minikube is the CurrentContext
 // If no CurrentContext is set, the given name will be used.
 func Update(kcs *Settings) error {

pkg/minikube/registry/registry_test.go

@@ -36,21 +36,19 @@ func TestDriverString(t *testing.T) {
 	}
 }
 
-func TestRegistry(t *testing.T) {
-	foo := DriverDef{
-		Name:    "foo",
-		Builtin: true,
-		ConfigCreator: func(_ config.MachineConfig) interface{} {
-			return nil
-		},
-	}
-	bar := DriverDef{
-		Name:    "bar",
+func testDriver(name string) DriverDef {
+	return DriverDef{
+		Name:    name,
 		Builtin: true,
 		ConfigCreator: func(_ config.MachineConfig) interface{} {
 			return nil
 		},
 	}
+}
+
+func TestRegistry1(t *testing.T) {
+	foo := testDriver("foo")
+	bar := testDriver("bar")
 
 	registry := createRegistry()
 	t.Run("registry.Register", func(t *testing.T) {
@@ -82,7 +80,19 @@ func TestRegistry(t *testing.T) {
 			t.Fatalf("expect len(list) to be %d; got %d", 2, len(list))
 		}
 	})
+}
 
+func TestRegistry2(t *testing.T) {
+	foo := testDriver("foo")
+	bar := testDriver("bar")
+
+	registry := createRegistry()
+	if err := registry.Register(foo); err != nil {
+		t.Skipf("error not expect but got: %v", err)
+	}
+	if err := registry.Register(bar); err != nil {
+		t.Skipf("error not expect but got: %v", err)
+	}
 	t.Run("Driver", func(t *testing.T) {
 		driverName := "foo"
 		driver, err := registry.Driver(driverName)

pkg/minikube/storageclass/storageclass.go

@@ -70,7 +70,7 @@ func SetDefaultStorageClass(storage storagev1.StorageV1Interface, name string) e
 	return nil
 }
 
-// GetStorageV1 return storage v1 interface for client
+// GetStoragev1 return storage v1 interface for client
 func GetStoragev1() (storagev1.StorageV1Interface, error) {
 	client, err := getClient()
 	if err != nil {

pkg/util/progressbar.go

@@ -29,6 +29,7 @@ import (
 	"github.com/hashicorp/go-getter"
 )
 
+// DefaultProgressBar is the default cheggaaa progress bar
 var DefaultProgressBar getter.ProgressTracker = &progressBar{}
 
 type progressBar struct {