Decrease cyclomatic complexity for code

Mostly by using the "extract method" pattern.

Here was the report (from gocyclo), before:

16 kubeadm (*KubeadmBootstrapper).UpdateCluster pkg/minikube/bootstrapper/kubeadm/kubeadm.go:374:1
16 cluster StartHost pkg/minikube/cluster/cluster.go:64:1
16 tunnel setupRoute pkg/minikube/tunnel/tunnel.go:135:1
16 provision configureAuth pkg/provision/buildroot.go:218:1
21 util setElement pkg/util/config.go:50:1

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -393,40 +393,10 @@ func (k *KubeadmBootstrapper) UpdateCluster(cfg config.KubernetesConfig) error {
 	}
 	glog.Infof("kubelet %s config:\n%s", cfg.KubernetesVersion, kubeletCfg)
 
-	files := []assets.CopyableFile{
+	var files []assets.CopyableFile
-		assets.NewMemoryAssetTarget([]byte(kubeletService), constants.KubeletServiceFile, "0640"),
+	files = copyConfig(cfg, files, kubeadmCfg, kubeletCfg)
-		assets.NewMemoryAssetTarget([]byte(kubeletCfg), constants.KubeletSystemdConfFile, "0640"),
-		assets.NewMemoryAssetTarget([]byte(kubeadmCfg), constants.KubeadmConfigFile, "0640"),
-	}
 
-	// Copy the default CNI config (k8s.conf), so that kubelet can successfully
+	if err := downloadBinaries(cfg, k.c); err != nil {
-	// start a Pod in the case a user hasn't manually installed any CNI plugin
-	// and minikube was started with "--extra-config=kubelet.network-plugin=cni".
-	if cfg.EnableDefaultCNI {
-		files = append(files,
-			assets.NewMemoryAssetTarget([]byte(defaultCNIConfig), constants.DefaultCNIConfigPath, "0644"),
-			assets.NewMemoryAssetTarget([]byte(defaultCNIConfig), constants.DefaultRktNetConfigPath, "0644"))
-	}
-
-	var g errgroup.Group
-	for _, bin := range []string{"kubelet", "kubeadm"} {
-		bin := bin
-		g.Go(func() error {
-			path, err := maybeDownloadAndCache(bin, cfg.KubernetesVersion)
-			if err != nil {
-				return errors.Wrapf(err, "downloading %s", bin)
-			}
-			f, err := assets.NewFileAsset(path, "/usr/bin", bin, "0641")
-			if err != nil {
-				return errors.Wrap(err, "new file asset")
-			}
-			if err := k.c.Copy(f); err != nil {
-				return errors.Wrapf(err, "copy")
-			}
-			return nil
-		})
-	}
-	if err := g.Wait(); err != nil {
 		return errors.Wrap(err, "downloading binaries")
 	}
 
@@ -521,6 +491,46 @@ func generateConfig(k8s config.KubernetesConfig, r cruntime.Manager) (string, er
 	return b.String(), nil
 }
 
+func copyConfig(cfg config.KubernetesConfig, files []assets.CopyableFile, kubeadmCfg string, kubeletCfg string) []assets.CopyableFile {
+	files = append(files,
+		assets.NewMemoryAssetTarget([]byte(kubeletService), constants.KubeletServiceFile, "0640"),
+		assets.NewMemoryAssetTarget([]byte(kubeletCfg), constants.KubeletSystemdConfFile, "0640"),
+		assets.NewMemoryAssetTarget([]byte(kubeadmCfg), constants.KubeadmConfigFile, "0640"))
+
+	// Copy the default CNI config (k8s.conf), so that kubelet can successfully
+	// start a Pod in the case a user hasn't manually installed any CNI plugin
+	// and minikube was started with "--extra-config=kubelet.network-plugin=cni".
+	if cfg.EnableDefaultCNI {
+		files = append(files,
+			assets.NewMemoryAssetTarget([]byte(defaultCNIConfig), constants.DefaultCNIConfigPath, "0644"),
+			assets.NewMemoryAssetTarget([]byte(defaultCNIConfig), constants.DefaultRktNetConfigPath, "0644"))
+	}
+
+	return files
+}
+
+func downloadBinaries(cfg config.KubernetesConfig, c bootstrapper.CommandRunner) error {
+	var g errgroup.Group
+	for _, bin := range []string{"kubelet", "kubeadm"} {
+		bin := bin
+		g.Go(func() error {
+			path, err := maybeDownloadAndCache(bin, cfg.KubernetesVersion)
+			if err != nil {
+				return errors.Wrapf(err, "downloading %s", bin)
+			}
+			f, err := assets.NewFileAsset(path, "/usr/bin", bin, "0641")
+			if err != nil {
+				return errors.Wrap(err, "new file asset")
+			}
+			if err := c.Copy(f); err != nil {
+				return errors.Wrapf(err, "copy")
+			}
+			return nil
+		})
+	}
+	return g.Wait()
+}
+
 func maybeDownloadAndCache(binary, version string) (string, error) {
 	targetDir := constants.MakeMiniPath("cache", version)
 	targetFilepath := path.Join(targetDir, binary)

pkg/minikube/cluster/cluster.go

@@ -111,6 +111,16 @@ func StartHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error)
 	e := engineOptions(config)
 	glog.Infof("engine options: %+v", e)
 
+	err = waitForSSHAccess(h, e)
+	if err != nil {
+		return nil, err
+	}
+
+	return h, nil
+}
+
+func waitForSSHAccess(h *host.Host, e *engine.Options) error {
+
 	// Slightly counter-intuitive, but this is what DetectProvisioner & ConfigureAuth block on.
 	console.OutStyle("waiting", "Waiting for SSH access ...")
 
@@ -118,19 +128,20 @@ func StartHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error)
 		h.HostOptions.EngineOptions.Env = e.Env
 		provisioner, err := provision.DetectProvisioner(h.Driver)
 		if err != nil {
-			return nil, errors.Wrap(err, "detecting provisioner")
+			return errors.Wrap(err, "detecting provisioner")
 		}
 		if err := provisioner.Provision(*h.HostOptions.SwarmOptions, *h.HostOptions.AuthOptions, *h.HostOptions.EngineOptions); err != nil {
-			return nil, errors.Wrap(err, "provision")
+			return errors.Wrap(err, "provision")
 		}
 	}
 
 	if h.Driver.DriverName() != "none" {
 		if err := h.ConfigureAuth(); err != nil {
-			return nil, &util.RetriableError{Err: errors.Wrap(err, "Error configuring auth on host")}
+			return &util.RetriableError{Err: errors.Wrap(err, "Error configuring auth on host")}
 		}
 	}
-	return h, nil
+
+	return nil
 }
 
 // trySSHPowerOff runs the poweroff command on the guest VM to speed up deletion

pkg/minikube/tunnel/tunnel.go

@@ -154,37 +154,8 @@ func setupRoute(t *tunnel, h *host.Host) {
 
 		if h.DriverName == "hyperkit" {
 			//the virtio-net interface acts up with ip tunnels :(
-			command := exec.Command("ifconfig", "bridge100")
+			setupBridge(t, h)
-			glog.Infof("About to run command: %s\n", command.Args)
+			if t.status.RouteError != nil {
-			response, err := command.CombinedOutput()
-			if err != nil {
-				t.status.RouteError = fmt.Errorf("running %v: %v", command.Args, err)
-				return
-			}
-			iface := string(response)
-			pattern := regexp.MustCompile(`.*member: (en\d+) flags=.*`)
-			submatch := pattern.FindStringSubmatch(iface)
-			if len(submatch) != 2 {
-				t.status.RouteError = fmt.Errorf("couldn't find member in bridge100 interface: %s", iface)
-				return
-			}
-
-			member := submatch[1]
-			command = exec.Command("sudo", "ifconfig", "bridge100", "deletem", member)
-			glog.Infof("About to run command: %s\n", command.Args)
-			response, err = command.CombinedOutput()
-			glog.Infof(string(response))
-			if err != nil {
-				t.status.RouteError = fmt.Errorf("couldn't remove member %s: %s", member, err)
-				return
-			}
-
-			command = exec.Command("sudo", "ifconfig", "bridge100", "addm", member)
-			glog.Infof("About to run command: %s\n", command.Args)
-			response, err = command.CombinedOutput()
-			glog.Infof(string(response))
-			if err != nil {
-				t.status.RouteError = fmt.Errorf("couldn't re-add member %s: %s", member, err)
 				return
 			}
 		}
@@ -221,3 +192,39 @@ func setupRoute(t *tunnel, h *host.Host) {
 	}
 
 }
+
+func setupBridge(t *tunnel, h *host.Host) {
+	command := exec.Command("ifconfig", "bridge100")
+	glog.Infof("About to run command: %s\n", command.Args)
+	response, err := command.CombinedOutput()
+	if err != nil {
+		t.status.RouteError = fmt.Errorf("running %v: %v", command.Args, err)
+		return
+	}
+	iface := string(response)
+	pattern := regexp.MustCompile(`.*member: (en\d+) flags=.*`)
+	submatch := pattern.FindStringSubmatch(iface)
+	if len(submatch) != 2 {
+		t.status.RouteError = fmt.Errorf("couldn't find member in bridge100 interface: %s", iface)
+		return
+	}
+
+	member := submatch[1]
+	command = exec.Command("sudo", "ifconfig", "bridge100", "deletem", member)
+	glog.Infof("About to run command: %s\n", command.Args)
+	response, err = command.CombinedOutput()
+	glog.Infof(string(response))
+	if err != nil {
+		t.status.RouteError = fmt.Errorf("couldn't remove member %s: %s", member, err)
+		return
+	}
+
+	command = exec.Command("sudo", "ifconfig", "bridge100", "addm", member)
+	glog.Infof("About to run command: %s\n", command.Args)
+	response, err = command.CombinedOutput()
+	glog.Infof(string(response))
+	if err != nil {
+		t.status.RouteError = fmt.Errorf("couldn't re-add member %s: %s", member, err)
+		return
+	}
+}

pkg/provision/buildroot.go

@@ -227,21 +227,9 @@ func configureAuth(p *BuildrootProvisioner) error {
 		return errors.Wrap(err, "error getting ip during provisioning")
 	}
 
-	execRunner := &bootstrapper.ExecRunner{}
+	err = copyHostCerts(authOptions)
-	hostCerts := map[string]string{
+	if err != nil {
-		authOptions.CaCertPath:     path.Join(authOptions.StorePath, "ca.pem"),
+		return err
-		authOptions.ClientCertPath: path.Join(authOptions.StorePath, "cert.pem"),
-		authOptions.ClientKeyPath:  path.Join(authOptions.StorePath, "key.pem"),
-	}
-
-	for src, dst := range hostCerts {
-		f, err := assets.NewFileAsset(src, path.Dir(dst), filepath.Base(dst), "0777")
-		if err != nil {
-			return errors.Wrapf(err, "open cert file: %s", src)
-		}
-		if err := execRunner.Copy(f); err != nil {
-			return errors.Wrapf(err, "transferring file: %+v", f)
-		}
 	}
 
 	// The Host IP is always added to the certificate's SANs list
@@ -268,25 +256,9 @@ func configureAuth(p *BuildrootProvisioner) error {
 		return fmt.Errorf("error generating server cert: %v", err)
 	}
 
-	remoteCerts := map[string]string{
+	err = copyRemoteCerts(authOptions, driver)
-		authOptions.CaCertPath:     authOptions.CaCertRemotePath,
-		authOptions.ServerCertPath: authOptions.ServerCertRemotePath,
-		authOptions.ServerKeyPath:  authOptions.ServerKeyRemotePath,
-	}
-
-	sshClient, err := sshutil.NewSSHClient(driver)
 	if err != nil {
-		return errors.Wrap(err, "provisioning: error getting ssh client")
+		return err
-	}
-	sshRunner := bootstrapper.NewSSHRunner(sshClient)
-	for src, dst := range remoteCerts {
-		f, err := assets.NewFileAsset(src, path.Dir(dst), filepath.Base(dst), "0640")
-		if err != nil {
-			return errors.Wrapf(err, "error copying %s to %s", src, dst)
-		}
-		if err := sshRunner.Copy(f); err != nil {
-			return errors.Wrapf(err, "transferring file to machine %v", f)
-		}
 	}
 
 	config, err := config.Load()
@@ -318,3 +290,49 @@ func configureAuth(p *BuildrootProvisioner) error {
 
 	return nil
 }
+
+func copyHostCerts(authOptions auth.Options) error {
+	execRunner := &bootstrapper.ExecRunner{}
+	hostCerts := map[string]string{
+		authOptions.CaCertPath:     path.Join(authOptions.StorePath, "ca.pem"),
+		authOptions.ClientCertPath: path.Join(authOptions.StorePath, "cert.pem"),
+		authOptions.ClientKeyPath:  path.Join(authOptions.StorePath, "key.pem"),
+	}
+
+	for src, dst := range hostCerts {
+		f, err := assets.NewFileAsset(src, path.Dir(dst), filepath.Base(dst), "0777")
+		if err != nil {
+			return errors.Wrapf(err, "open cert file: %s", src)
+		}
+		if err := execRunner.Copy(f); err != nil {
+			return errors.Wrapf(err, "transferring file: %+v", f)
+		}
+	}
+
+	return nil
+}
+
+func copyRemoteCerts(authOptions auth.Options, driver drivers.Driver) error {
+	remoteCerts := map[string]string{
+		authOptions.CaCertPath:     authOptions.CaCertRemotePath,
+		authOptions.ServerCertPath: authOptions.ServerCertRemotePath,
+		authOptions.ServerKeyPath:  authOptions.ServerKeyRemotePath,
+	}
+
+	sshClient, err := sshutil.NewSSHClient(driver)
+	if err != nil {
+		return errors.Wrap(err, "provisioning: error getting ssh client")
+	}
+	sshRunner := bootstrapper.NewSSHRunner(sshClient)
+	for src, dst := range remoteCerts {
+		f, err := assets.NewFileAsset(src, path.Dir(dst), filepath.Base(dst), "0640")
+		if err != nil {
+			return errors.Wrapf(err, "error copying %s to %s", src, dst)
+		}
+		if err := sshRunner.Copy(f); err != nil {
+			return errors.Wrapf(err, "transferring file to machine %v", f)
+		}
+	}
+
+	return nil
+}

pkg/util/config.go

@@ -58,29 +58,13 @@ func setElement(e reflect.Value, v string) error {
 	case bool:
 		return convertBool(e, v)
 	case net.IP:
-		ip := net.ParseIP(v)
+		return convertIP(e, v)
-		if ip == nil {
-			return fmt.Errorf("Error converting input %s to an IP.", v)
-		}
-		e.Set(reflect.ValueOf(ip))
 	case net.IPNet:
-		_, cidr, err := net.ParseCIDR(v)
+		return convertCIDR(e, v)
-		if err != nil {
-			return fmt.Errorf("Error converting input %s to a CIDR: %v", v, err)
-		}
-		e.Set(reflect.ValueOf(*cidr))
 	case utilnet.PortRange:
-		pr, err := utilnet.ParsePortRange(v)
+		return convertPortRange(e, v)
-		if err != nil {
-			return fmt.Errorf("Error converting input %s to PortRange: %v", v, err)
-		}
-		e.Set(reflect.ValueOf(*pr))
 	case time.Duration:
-		dur, err := time.ParseDuration(v)
+		return convertDuration(e, v)
-		if err != nil {
-			return fmt.Errorf("Error converting input %s to Duration: %v", v, err)
-		}
-		e.Set(reflect.ValueOf(dur))
 	case []string:
 		vals := strings.Split(v, ",")
 		e.Set(reflect.ValueOf(vals))
@@ -89,18 +73,7 @@ func setElement(e reflect.Value, v string) error {
 	default:
 		// Last ditch attempt to convert anything based on its underlying kind.
 		// This covers any types that are aliased to a native type
-		switch e.Kind() {
+		return convertKind(e, v)
-		case reflect.Int, reflect.Int32, reflect.Int64:
-			return convertInt(e, v)
-		case reflect.String:
-			return convertString(e, v)
-		case reflect.Float32, reflect.Float64:
-			return convertFloat(e, v)
-		case reflect.Bool:
-			return convertBool(e, v)
-		default:
-			return fmt.Errorf("Unable to set type %T.", e.Kind())
-		}
 	}
 
 	return nil
@@ -123,6 +96,22 @@ func convertMap(e reflect.Value, v string) error {
 	return nil
 }
 
+func convertKind(e reflect.Value, v string) error {
+	switch e.Kind() {
+	case reflect.Int, reflect.Int32, reflect.Int64:
+		return convertInt(e, v)
+	case reflect.String:
+		return convertString(e, v)
+	case reflect.Float32, reflect.Float64:
+		return convertFloat(e, v)
+	case reflect.Bool:
+		return convertBool(e, v)
+	default:
+		return fmt.Errorf("Unable to set type %T.", e.Kind())
+	}
+	return nil
+}
+
 func convertInt(e reflect.Value, v string) error {
 	i, err := strconv.Atoi(v)
 	if err != nil {
@@ -155,6 +144,42 @@ func convertBool(e reflect.Value, v string) error {
 	return nil
 }
 
+func convertIP(e reflect.Value, v string) error {
+	ip := net.ParseIP(v)
+	if ip == nil {
+		return fmt.Errorf("Error converting input %s to an IP.", v)
+	}
+	e.Set(reflect.ValueOf(ip))
+	return nil
+}
+
+func convertCIDR(e reflect.Value, v string) error {
+	_, cidr, err := net.ParseCIDR(v)
+	if err != nil {
+		return fmt.Errorf("Error converting input %s to a CIDR: %v", v, err)
+	}
+	e.Set(reflect.ValueOf(*cidr))
+	return nil
+}
+
+func convertPortRange(e reflect.Value, v string) error {
+	pr, err := utilnet.ParsePortRange(v)
+	if err != nil {
+		return fmt.Errorf("Error converting input %s to PortRange: %v", v, err)
+	}
+	e.Set(reflect.ValueOf(*pr))
+	return nil
+}
+
+func convertDuration(e reflect.Value, v string) error {
+	dur, err := time.ParseDuration(v)
+	if err != nil {
+		return fmt.Errorf("Error converting input %s to Duration: %v", v, err)
+	}
+	e.Set(reflect.ValueOf(dur))
+	return nil
+}
+
 // FindAndSet sets the nested value.
 func FindAndSet(path string, c interface{}, value string) error {
 	elem, err := findNestedElement(path, c)