Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kic: fix delete -p not cleaning up & add integration test #7819

Merged
merged 3 commits into from
Apr 21, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 25 additions & 13 deletions cmd/minikube/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
}

Expand All @@ -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)
}
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/drivers/kic/oci/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion test/integration/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions test/integration/pause_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package integration

import (
"context"
"encoding/json"
"os/exec"
"strings"
"testing"
Expand All @@ -45,6 +46,7 @@ func TestPause(t *testing.T) {
{"Unpause", validateUnpause},
{"PauseAgain", validatePause},
{"DeletePaused", validateDelete},
{"VerifyDeletedResources", validateVerifyDeleted},
}
for _, tc := range tests {
tc := tc
Expand Down Expand Up @@ -103,3 +105,44 @@ 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) {
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{}
if err := json.Unmarshal(rr.Stdout.Bytes(), &jsonObject); 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 %q not to show up in profile list but it does! output: %s . args: %q", profile, rr.Stdout.String(), rr.Command())
}

if KicDriver() {
bin := "docker"
if PodmanDriver() {
bin = "podman"
}
rr, err := Run(t, exec.CommandContext(ctx, bin, "ps", "-a"))
if err == nil && strings.Contains(rr.Output(), profile) {
t.Errorf("expected container %q not to exist in output of %s but it does output: %s.", profile, rr.Command(), rr.Output())
}

rr, err = Run(t, exec.CommandContext(ctx, bin, "volume", "inspect", profile))
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())
}

}

}