From a1d73a7a34788ed80943943d105cb67dddbf7305 Mon Sep 17 00:00:00 2001 From: Marek Schwarz Date: Mon, 29 Jul 2019 23:19:13 +0200 Subject: [PATCH] Added method to delete invalid profiles Added unit tests for DeleteProfiles deleteProfile for not part of api anymore Added more validation for profiles --- cmd/minikube/cmd/delete.go | 44 ++- cmd/minikube/cmd/delete_test.go | 412 ++++++++++++++++++++++++++++ pkg/minikube/config/profile.go | 10 +- pkg/minikube/constants/constants.go | 9 + 4 files changed, 465 insertions(+), 10 deletions(-) create mode 100644 cmd/minikube/cmd/delete_test.go diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index cc50c74ca7..f58d12bf61 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -84,7 +84,7 @@ func runDelete(cmd *cobra.Command, args []string) { exit.WithError("Error getting profiles to delete", err) } - errs := DeleteAllProfiles(profilesToDelete) + errs := DeleteProfiles(profilesToDelete) if len(errs) > 0 { HandleDeletionErrors(errs) } @@ -99,25 +99,33 @@ func runDelete(cmd *cobra.Command, args []string) { out.ErrT(out.Meh, `"{{.name}}" profile does not exist`, out.V{"name": profileName}) } - err = DeleteProfile(profile) + errs := DeleteProfiles([]*pkg_config.Profile{profile}) if err != nil { - HandleDeletionErrors([]error{err}) + HandleDeletionErrors(errs) } } } -func DeleteAllProfiles(profiles []*pkg_config.Profile) []error { +func DeleteProfiles(profiles []*pkg_config.Profile) []error { var errs []error for _, profile := range profiles { - err := DeleteProfile(profile) - if err != nil { + err := deleteProfile(profile) + + _, errStat := os.Stat(constants.GetMachinePath(profile.Name, constants.GetMinipath())) + // TODO: if (err != nil && !profile.IsValid()) || (err != nil && !machineConfig.IsValid()) { + if (err != nil && !profile.IsValid()) || (err != nil && os.IsNotExist(errStat)) { + invalidProfileDeletionErrs := DeleteInvalidProfile(profile) + if len(invalidProfileDeletionErrs) > 0 { + errs = append(errs, invalidProfileDeletionErrs...) + } + } else if err != nil { errs = append(errs, err) } } return errs } -func DeleteProfile(profile *pkg_config.Profile) error { +func deleteProfile(profile *pkg_config.Profile) error { viper.Set(pkg_config.MachineProfile, profile.Name) api, err := machine.NewAPIClient() @@ -180,6 +188,28 @@ func DeleteProfile(profile *pkg_config.Profile) error { return nil } +func DeleteInvalidProfile(profile *pkg_config.Profile) []error { + out.T(out.DeletingHost, "Trying to delete invalid profile {{.profile}}", out.V{"profile": profile.Name}) + + var errs []error + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + err := os.RemoveAll(pathToProfile) + if err != nil { + errs = append(errs, DeletionError{err, Fatal}) + } + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + err := os.RemoveAll(pathToMachine) + if err != nil { + errs = append(errs, DeletionError{err, Fatal}) + } + } + return errs +} + func profileDeletionErr(profileName string, additionalInfo string) error { return fmt.Errorf("error deleting profile \"%s\": %s", profileName, additionalInfo) } diff --git a/cmd/minikube/cmd/delete_test.go b/cmd/minikube/cmd/delete_test.go new file mode 100644 index 0000000000..1e2c03e25c --- /dev/null +++ b/cmd/minikube/cmd/delete_test.go @@ -0,0 +1,412 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/constants" +) + +func TestDeleteProfileWithValidConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p1" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} + +func TestDeleteProfileWithEmptyProfileConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p2_empty_profile_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} + +func TestDeleteProfileWithInvalidProfileConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p3_invalid_profile_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} + +func TestDeleteProfileWithPartialProfileConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p4_partial_profile_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} + +func TestDeleteProfileWithMissingMachineConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p5_missing_machine_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != numberOfMachineDirs { + t.Fatal("Deleted a machine config when it should not") + } +} + +func TestDeleteProfileWithEmptyMachineConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p6_empty_machine_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} + +func TestDeleteProfileWithInvalidMachineConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p7_invalid_machine_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} + +func TestDeleteProfileWithPartialMachineConfig(t *testing.T) { + testMinikubeDir := "../../../pkg/minikube/config/testdata/delete-single/.minikube" + miniDir, err := filepath.Abs(testMinikubeDir) + + if err != nil { + t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err) + } + + err = os.Setenv(constants.MinikubeHome, miniDir) + if err != nil { + fmt.Printf("error setting up test environment. could not set %s", constants.MinikubeHome) + } + + files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")) + numberOfProfileDirs := len(files) + + files, _ = ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")) + numberOfMachineDirs := len(files) + + profileToDelete := "p8_partial_machine_config" + profile, _ := config.LoadProfile(profileToDelete) + + errs := DeleteProfiles([]*config.Profile{profile}) + + if len(errs) > 0 { + HandleDeletionErrors(errs) + t.Fatal("Errors while deleting profiles") + } + + pathToProfile := constants.GetProfilePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToProfile); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + pathToMachine := constants.GetMachinePath(profile.Name, constants.GetMinipath()) + if _, err := os.Stat(pathToMachine); !os.IsNotExist(err) { + t.Fatalf("Profile folder of profile \"%s\" was not deleted", profile.Name) + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "profiles")); len(files) != (numberOfProfileDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } + + if files, _ := ioutil.ReadDir(filepath.Join(constants.GetMinipath(), "machines")); len(files) != (numberOfMachineDirs - 1) { + t.Fatal("Did not delete exactly one profile") + } +} diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go index f95990b463..9d454964dc 100644 --- a/pkg/minikube/config/profile.go +++ b/pkg/minikube/config/profile.go @@ -23,8 +23,12 @@ import ( "k8s.io/minikube/pkg/minikube/constants" ) -// isValid checks if the profile has the essential info needed for a profile -func (p *Profile) isValid() bool { +// IsValid checks if the profile has the essential info needed for a profile +func (p *Profile) IsValid() bool { + if p.Config == nil { + return false + } + if p.Config.MachineConfig.VMDriver == "" { return false } @@ -48,7 +52,7 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, inValidPs = append(inValidPs, p) continue } - if !p.isValid() { + if !p.IsValid() { inValidPs = append(inValidPs, p) continue } diff --git a/pkg/minikube/constants/constants.go b/pkg/minikube/constants/constants.go index 2929079a88..a03582eb37 100644 --- a/pkg/minikube/constants/constants.go +++ b/pkg/minikube/constants/constants.go @@ -208,6 +208,15 @@ func GetProfilePath(profile string, miniHome ...string) string { return filepath.Join(miniPath, "profiles", profile) } +// GetMachinePath returns the Minikube machine path of a machine +func GetMachinePath(machine string, miniHome ...string) string { + miniPath := GetMinipath() + if len(miniHome) > 0 { + miniPath = miniHome[0] + } + return filepath.Join(miniPath, "machines", machine) +} + // AddonsPath is the default path of the addons configuration const AddonsPath = "/etc/kubernetes/addons"