Refactor review findings

Removed IsProfileConfigValid
Added TODOs

cmd/minikube/cmd/config/profile.go

@@ -18,6 +18,7 @@ package config
 
 import (
 	"encoding/json"
+	"fmt"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"io/ioutil"
@@ -29,7 +30,6 @@ import (
 	pkgutil "k8s.io/minikube/pkg/util"
 	"os"
 	"path/filepath"
-	"reflect"
 )
 
 // ProfileCmd represents the profile command
@@ -76,30 +76,34 @@ var ProfileCmd = &cobra.Command{
 	},
 }
 
-func GetAllProfiles() []string {
+func GetAllProfiles() ([]string, error) {
 	miniPath := constants.GetMinipath()
 	profilesPath := filepath.Join(miniPath, "profiles")
 	fileInfos, err := ioutil.ReadDir(profilesPath)
 	if err != nil {
-		console.ErrLn("Unable to list in dir: %s \n Error: %v", profilesPath, err)
+		return nil, fmt.Errorf("Unable to list in dir: %s \n Error: %v", profilesPath, err)
 	}
 
 	var profiles []string
 	for _, fileInfo := range fileInfos {
 		if fileInfo.IsDir() {
 			profilePath := filepath.Join(profilesPath, fileInfo.Name())
-			if isValidProfile(profilePath) {
+			isValidProfile, err := isValidProfile(profilePath)
+			if err != nil {
+				return nil, err
+			}
+			if isValidProfile {
 				profiles = append(profiles, fileInfo.Name())
 			}
 		}
 	}
-	return profiles
+	return profiles, nil
 }
 
-func isValidProfile(profilePath string) bool {
+func isValidProfile(profilePath string) (bool, error) {
 	fileInfos, err := ioutil.ReadDir(profilePath)
 	if err != nil {
-		console.ErrLn("Unable to list in dir: %s \n Error: %v", profilePath, err)
+		return false, fmt.Errorf("Unable to list in dir: %s \n Error: %v", profilePath, err)
 	}
 
 	hasConfigJSON := false
@@ -110,33 +114,22 @@ func isValidProfile(profilePath string) bool {
 	}
 
 	if !hasConfigJSON {
-		return false
+		return false, nil
 	}
 
 	// TODO: Use constants?
 	profileConfigPath := filepath.Join(profilePath, "config.json")
 	bytes, err := ioutil.ReadFile(profileConfigPath)
 	if err != nil {
-		console.ErrLn("Unable to read file: %s \n Error: %v", profileConfigPath, err)
+		return false, fmt.Errorf("Unable to read file: %s \n Error: %v", profileConfigPath, err)
 	}
 
 	var configObject mkConfig.Config
 	errUnmarshal := json.Unmarshal(bytes, &configObject)
 
 	if errUnmarshal != nil {
-		console.ErrLn("Could not unmarshal config json to config object: %s \n Error: %v", profileConfigPath, err)
+		return false, fmt.Errorf("Could not unmarshal config json to config object: %s \n Error: %v", profileConfigPath, err)
+	} else {
+		return true, nil
 	}
-	return IsProfileConfigValid(configObject)
-}
-
-func IsProfileConfigValid(configObject mkConfig.Config) bool {
-	machineConfig := configObject.MachineConfig
-	kubernetesConfig := configObject.KubernetesConfig
-	if reflect.DeepEqual(machineConfig, mkConfig.MachineConfig{}) || reflect.DeepEqual(kubernetesConfig, mkConfig.KubernetesConfig{}) {
-		return false
-	}
-
-	//TODO: Validate MachineConfig and KubernetesConfig?
-
-	return true
 }

cmd/minikube/cmd/delete.go

@@ -48,15 +48,21 @@ associated files.`,
 
 // runDelete handles the executes the flow of "minikube delete"
 func runDelete(cmd *cobra.Command, args []string) {
-	profileFlag, _ := cmd.Flags().GetString("profile")
+	profileFlag, err := cmd.Flags().GetString("profile")
-	deleteAllFlag, _ := cmd.Flags().GetBool("all")
+	if err != nil {
-
+		exit.WithError("Could not get profile flag", err)
-	if profileFlag != constants.DefaultMachineName && deleteAllFlag {
-		exit.Usage("usage: minikube delete --all")
 	}
 
-	if deleteAllFlag {
+	if deleteAll {
-		profiles := cmdcfg.GetAllProfiles()
+		if profileFlag != constants.DefaultMachineName {
+			exit.Usage("usage: minikube delete --all")
+		}
+
+		profiles, err := cmdcfg.GetAllProfiles()
+		if err != nil {
+			exit.WithError("Error getting profiles to delete", err)
+		}
+
 		deleteAllProfiles(profiles)
 	} else {
 		if len(args) > 0 {
@@ -68,7 +74,10 @@ func runDelete(cmd *cobra.Command, args []string) {
 	}
 }
 
+//TODO Refactor: Return errors?
 func deleteProfile(profileName string) {
+	viper.Set(pkg_config.MachineProfile, profileName)
+
 	api, err := machine.NewAPIClient()
 	if err != nil {
 		exit.WithError("Error getting client", err)
@@ -115,13 +124,11 @@ func deleteProfile(profileName string) {
 
 func deleteAllProfiles(profiles []string) {
 	for _, profile := range profiles {
-		// TODO: Refactor: viper.Set seems to be in the wrong place
-		viper.Set(pkg_config.MachineProfile, profile)
 		deleteProfile(profile)
 	}
-	os.Exit(0)
 }
 
+// TODO: Return errors?
 func uninstallKubernetes(api libmachine.API, kc pkg_config.KubernetesConfig, bsName string) {
 	console.OutStyle(console.Resetting, "Uninstalling Kubernetes %s using %s ...", kc.KubernetesVersion, bsName)
 	clusterBootstrapper, err := getClusterBootstrapper(api, bsName)