Merge pull request #6807 from medyagh/delete_hang

warn about "stuck docker"  in minikube delete

cmd/minikube/cmd/delete.go

@@ -116,7 +116,7 @@ func runDelete(cmd *cobra.Command, args []string) {
 			exit.UsageT("usage: minikube delete --all")
 		}
 		delLabel := fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")
-		errs := oci.DeleteAllContainersByLabel(oci.Docker, delLabel)
+		errs := oci.DeleteContainersByLabel(oci.Docker, delLabel)
 		if len(errs) > 0 { // it will error if there is no container to delete
 			glog.Infof("error delete containers by label %q (might be okay): %+v", delLabel, err)
 		}
@@ -194,7 +194,7 @@ func deleteProfile(profile *pkg_config.Profile) error {
 	viper.Set(pkg_config.MachineProfile, profile.Name)
 
 	delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)
-	errs := oci.DeleteAllContainersByLabel(oci.Docker, delLabel)
+	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", profile.Name, errs)
 	}
@@ -207,7 +207,6 @@ func deleteProfile(profile *pkg_config.Profile) error {
 	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)
 	}
-
 	api, err := machine.NewAPIClient()
 	if err != nil {
 		delErr := profileDeletionErr(profile.Name, fmt.Sprintf("error getting client %v", err))

pkg/drivers/kic/kic.go

@@ -17,11 +17,13 @@ limitations under the License.
 package kic
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"os/exec"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/docker/machine/libmachine/log"
@@ -181,11 +183,18 @@ func (d *Driver) GetURL() (string, error) {
 // GetState returns the state that the host is in (running, stopped, etc)
 func (d *Driver) GetState() (state.State, error) {
 	if err := oci.PointToHostDockerDaemon(); err != nil {
-		return state.Error, errors.Wrap(err, "point host docker-daemon")
+		return state.Error, errors.Wrap(err, "point host docker daemon")
 	}
+	// allow no more than 2 seconds for this. when this takes long this means deadline passed
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
 
-	cmd := exec.Command(d.NodeConfig.OCIBinary, "inspect", "-f", "{{.State.Status}}", d.MachineName)
+	cmd := exec.CommandContext(ctx, d.NodeConfig.OCIBinary, "inspect", "-f", "{{.State.Status}}", d.MachineName)
 	out, err := cmd.CombinedOutput()
+	if ctx.Err() == context.DeadlineExceeded {
+		glog.Errorf("GetState for %s took longer than normal. Restarting your %s daemon might fix this issue.", d.MachineName, d.OCIBinary)
+		return state.Error, fmt.Errorf("inspect %s timeout", d.MachineName)
+	}
 	o := strings.TrimSpace(string(out))
 	if err != nil {
 		return state.Error, errors.Wrapf(err, "get container %s status", d.MachineName)

pkg/drivers/kic/oci/network.go

@@ -43,7 +43,7 @@ func RoutableHostIPFromInside(ociBin string, containerName string) (net.IP, erro
 // digDNS will get the IP record for a dns
 func digDNS(ociBin, containerName, dns string) (net.IP, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return nil, errors.Wrap(err, "point host docker-daemon")
+		return nil, errors.Wrap(err, "point host docker daemon")
 	}
 	cmd := exec.Command(ociBin, "exec", "-t", containerName, "dig", "+short", dns)
 	out, err := cmd.CombinedOutput()
@@ -59,7 +59,7 @@ func digDNS(ociBin, containerName, dns string) (net.IP, error) {
 // gets the ip from user's host docker
 func dockerGatewayIP() (net.IP, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return nil, errors.Wrap(err, "point host docker-daemon")
+		return nil, errors.Wrap(err, "point host docker daemon")
 	}
 	cmd := exec.Command(Docker, "network", "ls", "--filter", "name=bridge", "--format", "{{.ID}}")
 	out, err := cmd.CombinedOutput()

pkg/drivers/kic/oci/oci.go

@@ -17,9 +17,11 @@ limitations under the License.
 package oci
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"strconv"
+	"time"
 
 	"bufio"
 	"bytes"
@@ -34,13 +36,13 @@ import (
 	"strings"
 )
 
-// DeleteAllContainersByLabel deletes all containers that have a specific label
+// DeleteContainersByLabel deletes all containers that have a specific label
 // if there no containers found with the given 	label, it will return nil
-func DeleteAllContainersByLabel(ociBin string, label string) []error {
+func DeleteContainersByLabel(ociBin string, label string) []error {
 	var deleteErrs []error
 	if ociBin == Docker {
 		if err := PointToHostDockerDaemon(); err != nil {
-			return []error{errors.Wrap(err, "point host docker-daemon")}
+			return []error{errors.Wrap(err, "point host docker daemon")}
 		}
 	}
 	cs, err := listContainersByLabel(ociBin, label)
@@ -51,10 +53,19 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error {
 		return nil
 	}
 	for _, c := range cs {
+		_, err := ContainerStatus(ociBin, c)
+		// only try to delete if docker/podman inspect returns
+		// if it doesn't it means docker daemon is stuck and needs restart
+		if err != nil {
+			deleteErrs = append(deleteErrs, errors.Wrapf(err, "delete container %s: %s daemon is stuck. please try again!", c, ociBin))
+			glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s.", ociBin, ociBin)
+			continue
+		}
 		cmd := exec.Command(ociBin, "rm", "-f", "-v", c)
 		if out, err := cmd.CombinedOutput(); err != nil {
 			deleteErrs = append(deleteErrs, errors.Wrapf(err, "delete container %s: output %s", c, out))
 		}
+
 	}
 	return deleteErrs
 }
@@ -62,7 +73,7 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error {
 // CreateContainerNode creates a new container node
 func CreateContainerNode(p CreateParams) error {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return errors.Wrap(err, "point host docker-daemon")
+		return errors.Wrap(err, "point host docker daemon")
 	}
 
 	runArgs := []string{
@@ -141,7 +152,7 @@ func CreateContainerNode(p CreateParams) error {
 // CreateContainer creates a container with "docker/podman run"
 func createContainer(ociBinary string, image string, opts ...createOpt) ([]string, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return nil, errors.Wrap(err, "point host docker-daemon")
+		return nil, errors.Wrap(err, "point host docker daemon")
 	}
 
 	o := &createOpts{}
@@ -185,7 +196,7 @@ func createContainer(ociBinary string, image string, opts ...createOpt) ([]strin
 // Copy copies a local asset into the container
 func Copy(ociBinary string, ociID string, targetDir string, fName string) error {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return errors.Wrap(err, "point host docker-daemon")
+		return errors.Wrap(err, "point host docker daemon")
 	}
 	if _, err := os.Stat(fName); os.IsNotExist(err) {
 		return errors.Wrapf(err, "error source %s does not exist", fName)
@@ -206,7 +217,7 @@ func Copy(ociBinary string, ociID string, targetDir string, fName string) error
 // only supports TCP ports
 func HostPortBinding(ociBinary string, ociID string, contPort int) (int, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return 0, errors.Wrap(err, "point host docker-daemon")
+		return 0, errors.Wrap(err, "point host docker daemon")
 	}
 	var out []byte
 	var err error
@@ -261,7 +272,7 @@ func podmanConttainerIP(name string) (string, string, error) {
 // dockerContainerIP returns ipv4, ipv6 of container or error
 func dockerContainerIP(name string) (string, string, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return "", "", errors.Wrap(err, "point host docker-daemon")
+		return "", "", errors.Wrap(err, "point host docker daemon")
 	}
 	// retrieve the IP address of the node using docker inspect
 	lines, err := inspect(Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}")
@@ -281,7 +292,7 @@ func dockerContainerIP(name string) (string, string, error) {
 // ContainerID returns id of a container name
 func ContainerID(ociBinary string, nameOrID string) (string, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return "", errors.Wrap(err, "point host docker-daemon")
+		return "", errors.Wrap(err, "point host docker daemon")
 	}
 	cmd := exec.Command(ociBinary, "inspect", "-f", "{{.Id}}", nameOrID)
 	id, err := cmd.CombinedOutput()
@@ -299,7 +310,7 @@ func ListOwnedContainers(ociBinary string) ([]string, error) {
 // inspect return low-level information on containers
 func inspect(ociBinary string, containerNameOrID, format string) ([]string, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return nil, errors.Wrap(err, "point host docker-daemon")
+		return nil, errors.Wrap(err, "point host docker daemon")
 	}
 	cmd := exec.Command(ociBinary, "inspect",
 		"-f", format,
@@ -365,7 +376,7 @@ func generateMountBindings(mounts ...Mount) []string {
 // isUsernsRemapEnabled checks if userns-remap is enabled in docker
 func isUsernsRemapEnabled(ociBinary string) (bool, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return false, errors.Wrap(err, "point host docker-daemon")
+		return false, errors.Wrap(err, "point host docker daemon")
 	}
 	cmd := exec.Command(ociBinary, "info", "--format", "'{{json .SecurityOptions}}'")
 	var buff bytes.Buffer
@@ -427,9 +438,12 @@ func withPortMappings(portMappings []PortMapping) createOpt {
 // listContainersByLabel returns all the container names with a specified label
 func listContainersByLabel(ociBinary string, label string) ([]string, error) {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return nil, errors.Wrap(err, "point host docker-daemon")
+		return nil, errors.Wrap(err, "point host docker daemon")
 	}
-	cmd := exec.Command(ociBinary, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}")
+	// allow no more than 5 seconds for docker ps
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, ociBinary, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}")
 	stdout, err := cmd.Output()
 	s := bufio.NewScanner(bytes.NewReader(stdout))
 	var names []string
@@ -439,7 +453,6 @@ func listContainersByLabel(ociBinary string, label string) ([]string, error) {
 			names = append(names, n)
 		}
 	}
-
 	return names, err
 }
 
@@ -461,3 +474,25 @@ func PointToHostDockerDaemon() error {
 	}
 	return nil
 }
+
+// ContainerStatus returns status of a container running,exited,...
+func ContainerStatus(ociBin string, name string) (string, error) {
+	if ociBin == Docker {
+		if err := PointToHostDockerDaemon(); err != nil {
+			return "", errors.Wrap(err, "point host docker daemon")
+		}
+	}
+	// allow no more than 2 seconds for this. when this takes long this means deadline passed
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, ociBin, "inspect", name, "--format={{.State.Status}}")
+	out, err := cmd.CombinedOutput()
+	if ctx.Err() == context.DeadlineExceeded {
+		glog.Warningf("%s inspect %s took longer than normal. Restarting your %s daemon might fix this issue.", ociBin, name, ociBin)
+		return strings.TrimSpace(string(out)), fmt.Errorf("inspect %s timeout", name)
+	}
+	if err != nil {
+		return string(out), errors.Wrapf(err, "inspecting container: output %s", out)
+	}
+	return strings.TrimSpace(string(out)), nil
+}

pkg/drivers/kic/oci/volumes.go

@@ -19,9 +19,11 @@ package oci
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"fmt"
 	"os/exec"
 	"strings"
+	"time"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
@@ -34,7 +36,7 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error {
 	glog.Infof("trying to delete all %s volumes with label %s", ociBin, label)
 	if ociBin == Docker {
 		if err := PointToHostDockerDaemon(); err != nil {
-			return []error{errors.Wrap(err, "point host docker-daemon")}
+			return []error{errors.Wrap(err, "point host docker daemon")}
 		}
 	}
 
@@ -42,9 +44,15 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error {
 	if err != nil {
 		return []error{fmt.Errorf("listing volumes by label %q: %v", label, err)}
 	}
-
 	for _, v := range vs {
-		cmd := exec.Command(ociBin, "volume", "rm", "--force", v)
+		// allow no more than 3 seconds for this. when this takes long this means deadline passed
+		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+		defer cancel()
+		cmd := exec.CommandContext(ctx, ociBin, "volume", "rm", "--force", v)
+		if ctx.Err() == context.DeadlineExceeded {
+			glog.Warningf("removing volume with label %s took longer than normal. Restarting your %s daemon might fix this issue.", label, ociBin)
+			deleteErrs = append(deleteErrs, fmt.Errorf("delete deadline exceeded for %s", label))
+		}
 		if out, err := cmd.CombinedOutput(); err != nil {
 			deleteErrs = append(deleteErrs, fmt.Errorf("deleting volume %s: output: %s", v, string(out)))
 		}
@@ -60,15 +68,22 @@ func PruneAllVolumesByLabel(ociBin string, label string) []error {
 	glog.Infof("trying to prune all %s volumes with label %s", ociBin, label)
 	if ociBin == Docker {
 		if err := PointToHostDockerDaemon(); err != nil {
-			return []error{errors.Wrap(err, "point host docker-daemon")}
+			return []error{errors.Wrap(err, "point host docker daemon")}
 		}
 	}
+	// allow no more than 3 seconds for this. when this takes long this means deadline passed
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
 
 	// try to prune afterwards just in case delete didn't go through
-	cmd := exec.Command(ociBin, "volume", "prune", "-f", "--filter", "label="+label)
+	cmd := exec.CommandContext(ctx, ociBin, "volume", "prune", "-f", "--filter", "label="+label)
 	if out, err := cmd.CombinedOutput(); err != nil {
 		deleteErrs = append(deleteErrs, errors.Wrapf(err, "prune volume by label %s: %s", label, string(out)))
 	}
+	if ctx.Err() == context.DeadlineExceeded {
+		glog.Warningf("pruning volume with label %s took longer than normal. Restarting your %s daemon might fix this issue.", label, ociBin)
+		deleteErrs = append(deleteErrs, fmt.Errorf("prune deadline exceeded for %s", label))
+	}
 	return deleteErrs
 }
 
@@ -93,7 +108,7 @@ func allVolumesByLabel(ociBin string, label string) ([]string, error) {
 // TODO: this should be fixed as a part of https://github.com/kubernetes/minikube/issues/6530
 func createDockerVolume(name string) error {
 	if err := PointToHostDockerDaemon(); err != nil {
-		return errors.Wrap(err, "point host docker-daemon")
+		return errors.Wrap(err, "point host docker daemon")
 	}
 	cmd := exec.Command(Docker, "volume", "create", name, "--label", fmt.Sprintf("%s=%s", ProfileLabelKey, name), "--label", fmt.Sprintf("%s=%s", CreatedByLabelKey, "true"))
 	if out, err := cmd.CombinedOutput(); err != nil {

pkg/minikube/machine/delete.go

@@ -17,7 +17,9 @@ limitations under the License.
 package machine
 
 import (
+	"context"
 	"os/exec"
+	"time"
 
 	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/mcnerror"
@@ -29,10 +31,30 @@ import (
 	"k8s.io/minikube/pkg/minikube/out"
 )
 
-// deleteOrphanedKIC attempts to delete an orphaned docker instance
+// deleteOrphanedKIC attempts to delete an orphaned docker instance for machines without a config file
-func deleteOrphanedKIC(name string) {
+// used as last effort clean up not returning errors, wont warn user.
-	cmd := exec.Command(oci.Docker, "rm", "-f", "-v", name)
+func deleteOrphanedKIC(ociBin string, name string) {
-	err := cmd.Run()
+	if !(ociBin == oci.Podman || ociBin == oci.Docker) {
+		return
+	}
+	if ociBin == oci.Docker {
+		if err := oci.PointToHostDockerDaemon(); err != nil {
+			glog.Infof("Failed to point to host docker-damon: %v", err)
+			return
+		}
+	}
+
+	_, err := oci.ContainerStatus(ociBin, name)
+	if err != nil {
+		glog.Infof("couldn't inspect container %q before deleting, %s-daemon might needs a restart!: %v", name, ociBin, err)
+		return
+	}
+	// allow no more than 5 seconds for delting the container
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctx, ociBin, "rm", "-f", "-v", name)
+	err = cmd.Run()
 	if err == nil {
 		glog.Infof("Found stale kic container and successfully cleaned it up!")
 	}
@@ -42,7 +64,8 @@ func deleteOrphanedKIC(name string) {
 func DeleteHost(api libmachine.API, machineName string) error {
 	host, err := api.Load(machineName)
 	if err != nil && host == nil {
-		deleteOrphanedKIC(machineName)
+		deleteOrphanedKIC(oci.Docker, machineName)
+		deleteOrphanedKIC(oci.Podman, machineName)
 		// Keep going even if minikube does not know about the host
 	}