Reduce cyclometric complexity by refactoring

19 cmd generateCfgFromFlags cmd/minikube/cmd/start.go:711:1
19 cmd runStart cmd/minikube/cmd/start.go:257:1
18 hyperkit (*Driver).Start pkg/drivers/hyperkit/driver.go:202:1
17 cmd runDelete cmd/minikube/cmd/delete.go:53:1
16 registry TestRegistry pkg/minikube/registry/registry_test.go:39:1
pull/5422/head
Anders F Björklund 2019-09-20 19:38:13 +02:00
parent 869c08a5bc
commit 67c7a7a81c
4 changed files with 128 additions and 71 deletions

View File

@ -87,14 +87,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) {
@ -125,6 +118,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)

View File

@ -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)

View File

@ -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

View File

@ -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)