From b639c6112db1be56f492cfec932e13fc8c85a66d Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 20 Apr 2020 20:38:31 -0700 Subject: [PATCH] fix delete and add integeration test --- cmd/minikube/cmd/delete.go | 38 ++++++++++++++++++--------- pkg/drivers/kic/oci/oci.go | 8 +++--- test/integration/main.go | 12 ++++++++- test/integration/pause_test.go | 48 ++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 18 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index b25696ae34..c2b05800aa 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -147,6 +147,8 @@ func runDelete(cmd *cobra.Command, args []string) { out.ErrT(out.Meh, `"{{.name}}" profile does not exist, trying anyways.`, out.V{"name": cname}) } + deletePossibleKicLeftOver(cname) + errs := DeleteProfiles([]*config.Profile{profile}) if len(errs) > 0 { HandleDeletionErrors(errs) @@ -189,20 +191,30 @@ func DeleteProfiles(profiles []*config.Profile) []error { return errs } -func deleteProfileContainersAndVolumes(name string) { +func deletePossibleKicLeftOver(name string) { delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, name) - errs := oci.DeleteContainersByLabel(oci.Docker, delLabel) - if errs != nil { // it will error if there is no container to delete - glog.Infof("error deleting containers for %s (might be okay):\n%v", name, errs) - } - errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) - if errs != nil { // it will not error if there is nothing to delete - glog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs) - } + for _, bin := range []string{oci.Docker, oci.Podman} { + cs, err := oci.ListContainersByLabel(bin, delLabel) + if err == nil && len(cs) > 0 { + for _, c := range cs { + out.T(out.DeletingHost, `Deleting container "{{.name}}" ...`, out.V{"name": name}) + err := oci.DeleteContainer(bin, c) + if err != nil { // it will error if there is no container to delete + glog.Errorf("error deleting container %q. you might want to delete that manually :\n%v", name, err) + } - errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) - if len(errs) > 0 { // it will not error if there is nothing to delete - glog.Warningf("error pruning volume (might be okay):\n%v", errs) + } + } + + errs := oci.DeleteAllVolumesByLabel(bin, delLabel) + if errs != nil { // it will not error if there is nothing to delete + glog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs) + } + + errs = oci.PruneAllVolumesByLabel(bin, delLabel) + if len(errs) > 0 { // it will not error if there is nothing to delete + glog.Warningf("error pruning volume (might be okay):\n%v", errs) + } } } @@ -212,7 +224,7 @@ func deleteProfile(profile *config.Profile) error { // if driver is oci driver, delete containers and volumes if driver.IsKIC(profile.Config.Driver) { out.T(out.DeletingHost, `Deleting "{{.profile_name}}" in {{.driver_name}} ...`, out.V{"profile_name": profile.Name, "driver_name": profile.Config.Driver}) - deleteProfileContainersAndVolumes(profile.Name) + deletePossibleKicLeftOver(profile.Name) } } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index d508784874..89bc22df71 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -43,7 +43,7 @@ import ( func DeleteContainersByLabel(ociBin string, label string) []error { var deleteErrs []error - cs, err := listContainersByLabel(ociBin, label) + cs, err := ListContainersByLabel(ociBin, label) if err != nil { return []error{fmt.Errorf("listing containers by label %q", label)} } @@ -322,7 +322,7 @@ func IsCreatedByMinikube(ociBinary string, nameOrID string) bool { // ListOwnedContainers lists all the containres that kic driver created on user's machine using a label func ListOwnedContainers(ociBinary string) ([]string, error) { - return listContainersByLabel(ociBinary, ProfileLabelKey) + return ListContainersByLabel(ociBinary, ProfileLabelKey) } // inspect return low-level information on containers @@ -452,8 +452,8 @@ func withPortMappings(portMappings []PortMapping) createOpt { } } -// listContainersByLabel returns all the container names with a specified label -func listContainersByLabel(ociBinary string, label string) ([]string, error) { +// ListContainersByLabel returns all the container names with a specified label +func ListContainersByLabel(ociBinary string, label string) ([]string, error) { stdout, err := WarnIfSlow(ociBinary, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}") if err != nil { return nil, err diff --git a/test/integration/main.go b/test/integration/main.go index 2ef2d90731..fb706aebdd 100644 --- a/test/integration/main.go +++ b/test/integration/main.go @@ -69,9 +69,19 @@ func HyperVDriver() bool { return strings.Contains(*startArgs, "--driver=hyperv") || strings.Contains(*startArgs, "--vm-driver=hyperv") } +// DockerDriver returns whether or not this test is using the docker or podman driver +func DockerDriver() bool { + return strings.Contains(*startArgs, "--driver=docker") || strings.Contains(*startArgs, "--vm-driver=docker") +} + +// PodmanDriver returns whether or not this test is using the docker or podman driver +func PodmanDriver() bool { + return strings.Contains(*startArgs, "--vm-driver=podman") || strings.Contains(*startArgs, "driver=podman") +} + // KicDriver returns whether or not this test is using the docker or podman driver func KicDriver() bool { - return strings.Contains(*startArgs, "--driver=docker") || strings.Contains(*startArgs, "--vm-driver=docker") || strings.Contains(*startArgs, "--vm-driver=podman") || strings.Contains(*startArgs, "driver=podman") + return DockerDriver() || PodmanDriver() } // NeedsPortForward returns access to endpoints with this driver needs port forwarding diff --git a/test/integration/pause_test.go b/test/integration/pause_test.go index 6f949a87dd..88f87c7636 100644 --- a/test/integration/pause_test.go +++ b/test/integration/pause_test.go @@ -20,6 +20,7 @@ package integration import ( "context" + "encoding/json" "os/exec" "strings" "testing" @@ -45,6 +46,7 @@ func TestPause(t *testing.T) { {"Unpause", validateUnpause}, {"PauseAgain", validatePause}, {"DeletePaused", validateDelete}, + {"VerifyDeletedResources", validateVerifyDeleted}, } for _, tc := range tests { tc := tc @@ -103,3 +105,49 @@ func validateDelete(ctx context.Context, t *testing.T, profile string) { t.Errorf("failed to delete minikube with args: %q : %v", rr.Command(), err) } } + +func validateVerifyDeleted(ctx context.Context, t *testing.T, profile string) { + // vervose logging because this might go wrong, if container get stuck + args := []string{"profile", "list"} + rr, err := Run(t, exec.CommandContext(ctx, Target(), "profile", "list", "--output", "json")) + if err != nil { + t.Errorf("failed to list profiles with json format after it was deleted. args %q: %v", rr.Command(), err) + } + var jsonObject map[string][]map[string]interface{} + err = json.Unmarshal(rr.Stdout.Bytes(), &jsonObject) + if err != nil { + t.Errorf("failed to decode json from profile list: args %q: %v", rr.Command(), err) + } + validProfiles := jsonObject["valid"] + profileExists := false + for _, profileObject := range validProfiles { + if profileObject["Name"] == profile { + profileExists = true + break + } + } + if profileExists { + t.Errorf("expected the deleted profile doesnt show up in profile list anymore but got *%q*. args: %q", profile, rr.Stdout.String(), rr.Command()) + } + + if KicDriver() { + bin := "docker" + if PodmanDriver() { + bin = "podman" + } + args = []string{"ps", "-a"} + rr, err := Run(t, exec.CommandContext(ctx, bin, args...)) + if err == nil && strings.Contains(rr.Output(), profile) { + + t.Errorf("expected container %q not to exist in output of %s but got : %s", rr.Command(), rr.Output()) + } + + args = []string{"volume", "inspect", profile} + rr, err := Run(t, exec.CommandContext(ctx, bin, args...)) + if err == nil { + t.Errorf("expected to see error and volume %q to not exist after deletion but got no error and this output: %s", rr.Command(), rr.Output()) + } + + } + +}