Merge pull request #4558 from afbjorklund/reportcard-new

Fix the reportcard: gofmt, golint and gocyclo

cmd/minikube/cmd/start.go

@@ -195,15 +195,10 @@ func runStart(cmd *cobra.Command, args []string) {
 	}
 
 	// For non-"none", the ISO is required to boot, so block until it is downloaded
-	if viper.GetString(vmDriver) != constants.DriverNone {
-		if err := cluster.CacheISO(config.MachineConfig); err != nil {
-			exit.WithError("Failed to cache ISO", err)
-		}
-	} else {
-		// With "none", images are persistently stored in Docker, so internal caching isn't necessary.
-		viper.Set(cacheImages, false)
-		config.KubernetesConfig.ShouldLoadCachedImages = false
-	}
+	downloadISO(config)
+
+	// With "none", images are persistently stored in Docker, so internal caching isn't necessary.
+	skipCache(config)
 
 	// Now that the ISO is downloaded, pull images in the background while the VM boots.
 	var cacheGroup errgroup.Group
@@ -252,14 +247,7 @@ func runStart(cmd *cobra.Command, args []string) {
 	}
 
 	cr := configureRuntimes(runner)
-	version, _ := cr.Version()
-	console.OutStyle(cr.Style(), "Configuring environment for Kubernetes %s on %s %s", k8sVersion, cr.Name(), version)
-	for _, v := range dockerOpt {
-		console.OutStyle(console.Option, "opt %s", v)
-	}
-	for _, v := range dockerEnv {
-		console.OutStyle(console.Option, "env %s", v)
-	}
+	showVersionInfo(k8sVersion, cr)
 
 	// prepareHostEnvironment uses the downloaded images, so we need to wait for background task completion.
 	waitCacheImages(&cacheGroup)
@@ -286,6 +274,32 @@ func runStart(cmd *cobra.Command, args []string) {
 
 }
 
+func downloadISO(config cfg.Config) {
+	if viper.GetString(vmDriver) != constants.DriverNone {
+		if err := cluster.CacheISO(config.MachineConfig); err != nil {
+			exit.WithError("Failed to cache ISO", err)
+		}
+	}
+}
+
+func skipCache(config cfg.Config) {
+	if viper.GetString(vmDriver) == constants.DriverNone {
+		viper.Set(cacheImages, false)
+		config.KubernetesConfig.ShouldLoadCachedImages = false
+	}
+}
+
+func showVersionInfo(k8sVersion string, cr cruntime.Manager) {
+	version, _ := cr.Version()
+	console.OutStyle(cr.Style(), "Configuring environment for Kubernetes %s on %s %s", k8sVersion, cr.Name(), version)
+	for _, v := range dockerOpt {
+		console.OutStyle(console.Option, "opt %s", v)
+	}
+	for _, v := range dockerEnv {
+		console.OutStyle(console.Option, "env %s", v)
+	}
+}
+
 func showKubectlConnectInfo(kubeconfig *pkgutil.KubeConfigSetup) {
 	if kubeconfig.KeepContext {
 		console.OutStyle(console.Kubectl, "To connect to this cluster, use: kubectl --context=%s", kubeconfig.ClusterName)

pkg/drivers/kvm/network.go

@@ -151,20 +151,6 @@ func (d *Driver) createNetwork() error {
 }
 
 func (d *Driver) deleteNetwork() error {
-	type source struct {
-		//XMLName xml.Name `xml:"source"`
-		Network string `xml:"network,attr"`
-	}
-	type iface struct {
-		//XMLName xml.Name `xml:"interface"`
-		Source source `xml:"source"`
-	}
-	type result struct {
-		//XMLName xml.Name `xml:"domain"`
-		Name       string  `xml:"name"`
-		Interfaces []iface `xml:"devices>interface"`
-	}
-
 	conn, err := getConnection()
 	if err != nil {
 		return errors.Wrap(err, "getting libvirt connection")
@@ -187,6 +173,41 @@ func (d *Driver) deleteNetwork() error {
 	}
 	log.Debugf("Network %s exists", d.PrivateNetwork)
 
+	err = d.checkDomains(conn)
+	if err != nil {
+		return err
+	}
+
+	// when we reach this point, it means it is safe to delete the network
+	log.Debugf("Trying to destroy network %s...", d.PrivateNetwork)
+	err = network.Destroy()
+	if err != nil {
+		return errors.Wrap(err, "network destroy")
+	}
+	log.Debugf("Trying to undefine network %s...", d.PrivateNetwork)
+	err = network.Undefine()
+	if err != nil {
+		return errors.Wrap(err, "network undefine")
+	}
+
+	return nil
+}
+
+func (d *Driver) checkDomains(conn *libvirt.Connect) error {
+	type source struct {
+		//XMLName xml.Name `xml:"source"`
+		Network string `xml:"network,attr"`
+	}
+	type iface struct {
+		//XMLName xml.Name `xml:"interface"`
+		Source source `xml:"source"`
+	}
+	type result struct {
+		//XMLName xml.Name `xml:"domain"`
+		Name       string  `xml:"name"`
+		Interfaces []iface `xml:"devices>interface"`
+	}
+
 	// iterate over every (also turned off) domains, and check if it
 	// is using the private network. Do *not* delete the network if
 	// that is the case
@@ -244,18 +265,6 @@ func (d *Driver) deleteNetwork() error {
 		}
 	}
 
-	// when we reach this point, it means it is safe to delete the network
-	log.Debugf("Trying to destroy network %s...", d.PrivateNetwork)
-	err = network.Destroy()
-	if err != nil {
-		return errors.Wrap(err, "network destroy")
-	}
-	log.Debugf("Trying to undefine network %s...", d.PrivateNetwork)
-	err = network.Undefine()
-	if err != nil {
-		return errors.Wrap(err, "network undefine")
-	}
-
 	return nil
 }
 

pkg/minikube/bootstrapper/kubeadm/kubeadm_test.go

@@ -130,8 +130,8 @@ ExecStart=/usr/bin/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/
 	}
 }
 
-func TestGenerateConfig(t *testing.T) {
-	extraOpts := util.ExtraOptionSlice{
+func getExtraOpts() []util.ExtraOption {
+	return util.ExtraOptionSlice{
 		util.ExtraOption{
 			Component: Apiserver,
 			Key:       "fail-no-swap",
@@ -158,15 +158,19 @@ func TestGenerateConfig(t *testing.T) {
 			Value:     "true",
 		},
 	}
+}
 
-	extraOptsPodCidr := util.ExtraOptionSlice{
+func getExtraOptsPodCidr() []util.ExtraOption {
+	return util.ExtraOptionSlice{
 		util.ExtraOption{
 			Component: Kubeadm,
 			Key:       "pod-network-cidr",
 			Value:     "192.168.32.0/20",
 		},
 	}
+}
 
+func recentReleases() ([]string, error) {
 	// test the 6 most recent releases
 	versions := []string{"v1.15", "v1.14", "v1.13", "v1.12", "v1.11", "v1.10"}
 	foundNewest := false
@@ -182,13 +186,23 @@ func TestGenerateConfig(t *testing.T) {
 	}
 
 	if !foundNewest {
-		t.Errorf("No tests exist yet for newest minor version: %s", constants.NewestKubernetesVersion)
+		return nil, fmt.Errorf("No tests exist yet for newest minor version: %s", constants.NewestKubernetesVersion)
 	}
 
 	if !foundDefault {
-		t.Errorf("No tests exist yet for default minor version: %s", constants.DefaultKubernetesVersion)
+		return nil, fmt.Errorf("No tests exist yet for default minor version: %s", constants.DefaultKubernetesVersion)
 	}
 
+	return versions, nil
+}
+
+func TestGenerateConfig(t *testing.T) {
+	extraOpts := getExtraOpts()
+	extraOptsPodCidr := getExtraOptsPodCidr()
+	versions, err := recentReleases()
+	if err != nil {
+		t.Errorf("versions: %v", err)
+	}
 	tests := []struct {
 		name      string
 		runtime   string

pkg/minikube/console/style_enum.go

@@ -16,8 +16,10 @@ limitations under the License.
 
 package console
 
+// StyleEnum is an enumeration of Style
 type StyleEnum int
 
+// All the Style constants available
 const (
 	Happy StyleEnum = iota
 	SuccessType

pkg/minikube/cruntime/containerd.go

@@ -35,6 +35,7 @@ func (r *Containerd) Name() string {
 	return "containerd"
 }
 
+// Style is the console style for containerd
 func (r *Containerd) Style() console.StyleEnum {
 	return console.Containerd
 }

pkg/minikube/cruntime/crio.go

@@ -35,7 +35,7 @@ func (r *CRIO) Name() string {
 	return "CRI-O"
 }
 
-// Name is a human readable name for CRIO
+// Style is the console style for CRIO
 func (r *CRIO) Style() console.StyleEnum {
 	return console.CRIO
 }

pkg/minikube/cruntime/docker.go

@@ -36,7 +36,7 @@ func (r *Docker) Name() string {
 	return "Docker"
 }
 
-// Name is a human readable name for Docker
+// Style is the console style for Docker
 func (r *Docker) Style() console.StyleEnum {
 	return console.Docker
 }

pkg/minikube/drivers/hyperv/driver.go

@@ -46,7 +46,7 @@ func createHypervHost(config cfg.MachineConfig) interface{} {
 	d.CPU = config.CPUs
 	d.DiskSize = int(config.DiskSize)
 	d.SSHUser = "docker"
-	d.DisableDynamicMemory  = true //default to disable dynamic memory as minikube is unlikely to work properly with dynamic memory
+	d.DisableDynamicMemory = true //default to disable dynamic memory as minikube is unlikely to work properly with dynamic memory
 
 	return d
 }

pkg/minikube/extract/extract.go

@@ -237,6 +237,10 @@ func checkCallExpression(s *ast.CallExpr, e *state) {
 		return
 	}
 
+	checkArguments(s, e)
+}
+
+func checkArguments(s *ast.CallExpr, e *state) {
 	matched := false
 	for _, arg := range s.Args {
 		// This argument is an identifier.

pkg/minikube/tests/dir_utils.go

@@ -50,10 +50,12 @@ type FakeFile struct {
 	b bytes.Buffer
 }
 
+// NewFakeFile creates a FakeFile
 func NewFakeFile() *FakeFile {
 	return &FakeFile{}
 }
 
+// Fd returns the file descriptor
 func (f *FakeFile) Fd() uintptr {
 	return uintptr(0)
 }

pkg/minikube/tests/ssh_mock.go

@@ -72,14 +72,8 @@ type execRequest struct {
 	Command string
 }
 
-// Start starts the mock SSH Server, and returns the port it's listening on.
-func (s *SSHServer) Start() (int, error) {
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		return 0, errors.Wrap(err, "Error creating tcp listener for ssh server")
-	}
-
-	// Main loop, listen for connections and store the commands.
+// Main loop, listen for connections and store the commands.
+func (s *SSHServer) mainLoop(listener net.Listener) {
 	go func() {
 		for {
 			nConn, err := listener.Accept()
@@ -109,54 +103,68 @@ func (s *SSHServer) Start() (int, error) {
 					for req := range requests {
 						glog.Infoln("Got Req: ", req.Type)
 						// Store anything that comes in over stdin.
-						go func() {
-							if _, err := io.Copy(s.Transfers, channel); err != nil {
-								panic(fmt.Sprintf("copy failed: %v", err))
-							}
-							channel.Close()
-						}()
-						switch req.Type {
-						case "exec":
-							if err := req.Reply(true, nil); err != nil {
-								panic(fmt.Sprintf("reply failed: %v", err))
-							}
-
-							// Note: string(req.Payload) adds additional characters to start of input.
-							var cmd execRequest
-							if err := ssh.Unmarshal(req.Payload, &cmd); err != nil {
-								glog.Errorf("Unmarshall encountered error: %v with req: %v", err, req.Type)
-								return
-							}
-							s.Commands[cmd.Command] = 1
-
-							// Write specified command output as mocked ssh output
-							if val, err := s.GetCommandToOutput(cmd.Command); err == nil {
-								if _, err := channel.Write([]byte(val)); err != nil {
-									glog.Errorf("Write failed: %v", err)
-									return
-								}
-							}
-							if _, err := channel.SendRequest("exit-status", false, []byte{0, 0, 0, 0}); err != nil {
-								glog.Errorf("SendRequest failed: %v", err)
-								return
-							}
-
-						case "pty-req":
-							if err := req.Reply(true, nil); err != nil {
-								glog.Errorf("Reply failed: %v", err)
-								return
-							}
-
-							if _, err := channel.SendRequest("exit-status", false, []byte{0, 0, 0, 0}); err != nil {
-								glog.Errorf("SendRequest failed: %v", err)
-								return
-							}
-						}
+						s.handleRequest(channel, req)
 					}
 				}
 			}()
 		}
 	}()
+}
+
+func (s *SSHServer) handleRequest(channel ssh.Channel, req *ssh.Request) {
+	go func() {
+		if _, err := io.Copy(s.Transfers, channel); err != nil {
+			panic(fmt.Sprintf("copy failed: %v", err))
+		}
+		channel.Close()
+	}()
+	switch req.Type {
+	case "exec":
+		if err := req.Reply(true, nil); err != nil {
+			panic(fmt.Sprintf("reply failed: %v", err))
+		}
+
+		// Note: string(req.Payload) adds additional characters to start of input.
+		var cmd execRequest
+		if err := ssh.Unmarshal(req.Payload, &cmd); err != nil {
+			glog.Errorf("Unmarshall encountered error: %v with req: %v", err, req.Type)
+			return
+		}
+		s.Commands[cmd.Command] = 1
+
+		// Write specified command output as mocked ssh output
+		if val, err := s.GetCommandToOutput(cmd.Command); err == nil {
+			if _, err := channel.Write([]byte(val)); err != nil {
+				glog.Errorf("Write failed: %v", err)
+				return
+			}
+		}
+		if _, err := channel.SendRequest("exit-status", false, []byte{0, 0, 0, 0}); err != nil {
+			glog.Errorf("SendRequest failed: %v", err)
+			return
+		}
+
+	case "pty-req":
+		if err := req.Reply(true, nil); err != nil {
+			glog.Errorf("Reply failed: %v", err)
+			return
+		}
+
+		if _, err := channel.SendRequest("exit-status", false, []byte{0, 0, 0, 0}); err != nil {
+			glog.Errorf("SendRequest failed: %v", err)
+			return
+		}
+	}
+}
+
+// Start starts the mock SSH Server, and returns the port it's listening on.
+func (s *SSHServer) Start() (int, error) {
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		return 0, errors.Wrap(err, "Error creating tcp listener for ssh server")
+	}
+
+	s.mainLoop(listener)
 
 	// Parse and return the port.
 	_, p, err := net.SplitHostPort(listener.Addr().String())

test/integration/tunnel_test.go

@@ -89,30 +89,13 @@ func testTunnel(t *testing.T) {
 
 	t.Log("getting nginx ingress...")
 
-	nginxIP := ""
-
-	err = wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
-		cmd := []string{"get", "svc", "nginx-svc", "-o", "jsonpath={.status.loadBalancer.ingress[0].ip}"}
-		stdout, err := kubectlRunner.RunCommand(cmd)
-		switch {
-		case err == nil:
-			nginxIP = string(stdout)
-			return len(stdout) != 0, nil
-		case !commonutil.IsRetryableAPIError(err):
-			t.Errorf("`%s` failed with non retriable error: %v", cmd, err)
-			return false, err
-		default:
-			t.Errorf("`%s` failed: %v", cmd, err)
-			return false, nil
-		}
-	})
-
+	nginxIP, err := getIngress(kubectlRunner)
 	if err != nil {
 		t.Errorf("error getting ingress IP for nginx: %s", err)
 	}
 
 	if len(nginxIP) == 0 {
-		stdout, err := kubectlRunner.RunCommand([]string{"get", "svc", "nginx-svc", "-o", "jsonpath={.status}"})
+		stdout, err := describeIngress(kubectlRunner)
 
 		if err != nil {
 			t.Errorf("error debugging nginx service: %s", err)
@@ -130,6 +113,34 @@ func testTunnel(t *testing.T) {
 	}
 }
 
+func getIngress(kubectlRunner *util.KubectlRunner) (string, error) {
+	nginxIP := ""
+	var ret error
+	err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
+		cmd := []string{"get", "svc", "nginx-svc", "-o", "jsonpath={.status.loadBalancer.ingress[0].ip}"}
+		stdout, err := kubectlRunner.RunCommand(cmd)
+		switch {
+		case err == nil:
+			nginxIP = string(stdout)
+			return len(stdout) != 0, nil
+		case !commonutil.IsRetryableAPIError(err):
+			ret = fmt.Errorf("`%s` failed with non retriable error: %v", cmd, err)
+			return false, err
+		default:
+			ret = fmt.Errorf("`%s` failed: %v", cmd, err)
+			return false, nil
+		}
+	})
+	if err != nil {
+		return "", err
+	}
+	return nginxIP, ret
+}
+
+func describeIngress(kubectlRunner *util.KubectlRunner) ([]byte, error) {
+	return kubectlRunner.RunCommand([]string{"get", "svc", "nginx-svc", "-o", "jsonpath={.status}"})
+}
+
 // getResponseBody returns the contents of a URL
 func getResponseBody(address string) (string, error) {
 	httpClient := http.DefaultClient