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.

Sign up for GitHub

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

Jump to bottom

warn about "stuck docker" in minikube delete #6807

Merged
merged 3 commits into from
Feb 26, 2020
Merged

warn about "stuck docker" in minikube delete #6807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8c8554
limit time waiting for delete
medyagh Feb 26, 2020
4709af5
resolve review comments
medyagh Feb 26, 2020
8d0c413
continue instead of nest
medyagh Feb 26, 2020
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
5 changes: 2 additions & 3 deletions cmd/minikube/cmd/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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))
Expand Down
13 changes: 11 additions & 2 deletions pkg/drivers/kic/kic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/drivers/kic/oci/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
63 changes: 49 additions & 14 deletions pkg/drivers/kic/oci/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ limitations under the License.
package oci

import (
"context"
"os"
"path/filepath"
"strconv"
"time"

"bufio"
"bytes"
Expand All @@ -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)
Expand All @@ -51,18 +53,27 @@ 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)
medyagh marked this conversation as resolved.
Show resolved Hide resolved
if out, err := cmd.CombinedOutput(); err != nil {
deleteErrs = append(deleteErrs, errors.Wrapf(err, "delete container %s: output %s", c, out))
}

}
return deleteErrs
}

// 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{
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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}}")
Expand All @@ -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()
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -439,7 +453,6 @@ func listContainersByLabel(ociBinary string, label string) ([]string, error) {
names = append(names, n)
}
}

return names, err
}

Expand All @@ -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
}
27 changes: 21 additions & 6 deletions pkg/drivers/kic/oci/volumes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package oci
import (
"bufio"
"bytes"
"context"
"fmt"
"os/exec"
"strings"
"time"

"github.com/golang/glog"
"github.com/pkg/errors"
Expand All @@ -34,17 +36,23 @@ 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")}
}
}

vs, err := allVolumesByLabel(ociBin, label)
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)))
}
Expand All @@ -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
}

Expand All @@ -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 {
Expand Down
33 changes: 28 additions & 5 deletions pkg/minikube/machine/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -29,10 +31,30 @@ import (
"k8s.io/minikube/pkg/minikube/out"
)

// deleteOrphanedKIC attempts to delete an orphaned docker instance
func deleteOrphanedKIC(name string) {
cmd := exec.Command(oci.Docker, "rm", "-f", "-v", name)
err := cmd.Run()
// deleteOrphanedKIC attempts to delete an orphaned docker instance for machines without a config file
// used as last effort clean up not returning errors, wont warn user.
func deleteOrphanedKIC(ociBin string, name string) {
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!")
}
Expand All @@ -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
}

Expand Down