From e1f682033531c3ab6d5ded0fc6d30a7ef6015403 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Thu, 5 Mar 2020 16:42:56 -0800 Subject: [PATCH 1/6] fix already in use --- pkg/drivers/kic/kic.go | 19 ++++++++++++++++ pkg/drivers/kic/oci/oci.go | 46 +++++++++++++++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index f33bda1f3dfb..5089c4ee3110 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -92,6 +92,25 @@ func (d *Driver) Create() error { ContainerPort: constants.DockerDaemonPort, }, ) + + id, err := oci.ContainerID(d.OCIBinary, params.Name) + if err != nil { + return errors.Wrap(err, "check container exists") + } + if id != "" { + // if container was created by minikube it is safe to delete it. + if oci.IsCreatedByMinikube(d.OCIBinary, id) { + if err := oci.DeleteContainersByID(d.OCIBinary, id); err != nil { + glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", id) + return errors.Wrapf(err, "deleting already in-use mini-created container %s", id) + } + } + // if not the conflicting container name is not created by minikube + // that means it is a user has a container that conflicts with minikube profile name + glog.Errorf("You have a container named %s that conflicts with minikube profile name %s, please either remove manually or choose a different minikbue profile name", id) + return errors.Wrap(err, "conflicting name with user's container") + } + if err := oci.CreateContainerNode(params); err != nil { return errors.Wrap(err, "create kic node") } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 22473fb895ba..512e6046b08e 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -69,6 +69,25 @@ func DeleteContainersByLabel(ociBin string, label string) []error { return deleteErrs } +// DeleteContainersByID deletes a container by ID or Name +func DeleteContainersByID(ociBin string, name string) error { + if err := PointToHostDockerDaemon(); err != nil { + return errors.Wrap(err, "point host docker daemon") + } + _, err := ContainerStatus(ociBin, name) + // 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 { + glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s. Will try to delete anyways", ociBin, ociBin) + } + // try to delete anyways + cmd := exec.Command(ociBin, "rm", "-f", "-v", name) + if out, err := cmd.CombinedOutput(); err != nil { + return errors.Wrapf(err, "delete container %s: output %s", name, out) + } + return nil +} + // CreateContainerNode creates a new container node func CreateContainerNode(p CreateParams) error { if err := PointToHostDockerDaemon(); err != nil { @@ -217,11 +236,32 @@ func ContainerID(ociBinary string, nameOrID string) (string, error) { return "", errors.Wrap(err, "point host docker daemon") } cmd := exec.Command(ociBinary, "inspect", "-f", "{{.Id}}", nameOrID) - id, err := cmd.CombinedOutput() + out, err := cmd.CombinedOutput() + if err != nil { // don't return error if not found, only return empty string + if strings.Contains(string(out), "Error: No such object:") || strings.Contains(string(out), "unable to find") { + err = nil + } + out = []byte{} + } + return string(out), err +} + +// IsCreatedByMinikube returns true if the container was created by minikube +// with default assumption that it is not created by minikube when we don't know for sure +func IsCreatedByMinikube(ociBinary string, nameOrID string) bool { + if err := PointToHostDockerDaemon(); err != nil { + glog.Warningf("Failed to point to host docker daemon") + return false + } + cmd := exec.Command(ociBinary, "inspect", nameOrID, "--format", "{{.Config.Labels}}") + out, err := cmd.CombinedOutput() if err != nil { - id = []byte{} + return false + } + if strings.Contains(string(out), fmt.Sprintf("%s:true", CreatedByLabelKey)) { + return true } - return string(id), err + return false } // ListOwnedContainers lists all the containres that kic driver created on user's machine using a label From 1e8dd94a77dc7bb3c243d84a372e1f6d55583420 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Thu, 5 Mar 2020 22:31:44 -0800 Subject: [PATCH 2/6] fix logic --- pkg/drivers/kic/kic.go | 28 +++++++++++++++------------- pkg/drivers/kic/oci/oci.go | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 5089c4ee3110..3cdd49e01dc0 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -93,22 +93,24 @@ func (d *Driver) Create() error { }, ) - id, err := oci.ContainerID(d.OCIBinary, params.Name) + exists, err := oci.ContainerExists(d.OCIBinary, params.Name) if err != nil { - return errors.Wrap(err, "check container exists") - } - if id != "" { - // if container was created by minikube it is safe to delete it. - if oci.IsCreatedByMinikube(d.OCIBinary, id) { - if err := oci.DeleteContainersByID(d.OCIBinary, id); err != nil { - glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", id) - return errors.Wrapf(err, "deleting already in-use mini-created container %s", id) + glog.Infof("failed to check if container already exists: %v", err) + } + if exists { + // if container was created by minikube it is safe to delete and recreate it. + if oci.IsCreatedByMinikube(d.OCIBinary, params.Name) { + glog.Info("Found already existing abandoned minikube container, will try to delete.") + if err := oci.DeleteContainersByID(d.OCIBinary, params.Name); err != nil { + glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", params.Name) + return errors.Wrapf(err, "deleting already in-use mini-created container %s", params.Name) } + } else { + // if not the conflicting container name is not created by minikube + // that means it is a user has a container that conflicts with minikube profile name + glog.Errorf("You have a container named %s that conflicts with minikube profile name %s, please either remove manually or choose a different minikbue profile name", params.Name, params.Name) + return errors.Wrap(err, "conflicting name with user's container") } - // if not the conflicting container name is not created by minikube - // that means it is a user has a container that conflicts with minikube profile name - glog.Errorf("You have a container named %s that conflicts with minikube profile name %s, please either remove manually or choose a different minikbue profile name", id) - return errors.Wrap(err, "conflicting name with user's container") } if err := oci.CreateContainerNode(params); err != nil { diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 512e6046b08e..8805b2bc6b78 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -246,6 +246,28 @@ func ContainerID(ociBinary string, nameOrID string) (string, error) { return string(out), err } +// ContainerExists checks if container name exists (either running or exited) +func ContainerExists(ociBin string, name string) (bool, error) { + // allow no more than 3 seconds for this. + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx,ociBin, "ps", "-a", "--format", "{{.Names}}") + out, err := cmd.CombinedOutput() + if ctx.Err() == context.DeadlineExceeded { + return false,fmt.Errorf("time out running %s ps -a",ociBin) + } + if err != nil { + return false, errors.Wrapf(err, string(out)) + } + containers := strings.Split(string(out), "\n") + for _, c := range containers { + if strings.TrimSpace(c) == name { + return true, nil + } + } + return false, nil +} + // IsCreatedByMinikube returns true if the container was created by minikube // with default assumption that it is not created by minikube when we don't know for sure func IsCreatedByMinikube(ociBinary string, nameOrID string) bool { From 2a37317a726855c3397e0525773fa8fedd6ce0cc Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Thu, 5 Mar 2020 22:34:10 -0800 Subject: [PATCH 3/6] improve comment --- pkg/drivers/kic/kic.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 3cdd49e01dc0..cd005625bef3 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -106,8 +106,8 @@ func (d *Driver) Create() error { return errors.Wrapf(err, "deleting already in-use mini-created container %s", params.Name) } } else { - // if not the conflicting container name is not created by minikube - // that means it is a user has a container that conflicts with minikube profile name + // The conflicting container name was not created by minikube + // user has a container that conflicts with minikube profile name, will not delete users container. glog.Errorf("You have a container named %s that conflicts with minikube profile name %s, please either remove manually or choose a different minikbue profile name", params.Name, params.Name) return errors.Wrap(err, "conflicting name with user's container") } From 0335fb1ad7c8f68324950d4369e5864ab55acc43 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Thu, 5 Mar 2020 22:38:47 -0800 Subject: [PATCH 4/6] change names --- pkg/drivers/kic/kic.go | 4 ++-- pkg/drivers/kic/oci/oci.go | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index cd005625bef3..577df792832c 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -95,13 +95,13 @@ func (d *Driver) Create() error { exists, err := oci.ContainerExists(d.OCIBinary, params.Name) if err != nil { - glog.Infof("failed to check if container already exists: %v", err) + glog.Warningf("failed to check if container already exists: %v", err) } if exists { // if container was created by minikube it is safe to delete and recreate it. if oci.IsCreatedByMinikube(d.OCIBinary, params.Name) { glog.Info("Found already existing abandoned minikube container, will try to delete.") - if err := oci.DeleteContainersByID(d.OCIBinary, params.Name); err != nil { + if err := oci.DeleteContainer(d.OCIBinary, params.Name); err != nil { glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", params.Name) return errors.Wrapf(err, "deleting already in-use mini-created container %s", params.Name) } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 8805b2bc6b78..e936d14df646 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -69,8 +69,8 @@ func DeleteContainersByLabel(ociBin string, label string) []error { return deleteErrs } -// DeleteContainersByID deletes a container by ID or Name -func DeleteContainersByID(ociBin string, name string) error { +// DeleteContainer deletes a container by ID or Name +func DeleteContainer(ociBin string, name string) error { if err := PointToHostDockerDaemon(); err != nil { return errors.Wrap(err, "point host docker daemon") } @@ -248,13 +248,16 @@ func ContainerID(ociBinary string, nameOrID string) (string, error) { // ContainerExists checks if container name exists (either running or exited) func ContainerExists(ociBin string, name string) (bool, error) { - // allow no more than 3 seconds for this. + if err := PointToHostDockerDaemon(); err != nil { + return false, errors.Wrap(err, "point host docker daemon") + } + // allow no more than 3 seconds for this. ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - cmd := exec.CommandContext(ctx,ociBin, "ps", "-a", "--format", "{{.Names}}") + cmd := exec.CommandContext(ctx, ociBin, "ps", "-a", "--format", "{{.Names}}") out, err := cmd.CombinedOutput() if ctx.Err() == context.DeadlineExceeded { - return false,fmt.Errorf("time out running %s ps -a",ociBin) + return false, fmt.Errorf("time out running %s ps -a", ociBin) } if err != nil { return false, errors.Wrapf(err, string(out)) From 1c37fc550540e53614348cd6c3076f58eb6bfce3 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Thu, 5 Mar 2020 22:45:38 -0800 Subject: [PATCH 5/6] lint --- pkg/drivers/kic/kic.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 577df792832c..6d9f716680e6 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -102,7 +102,7 @@ func (d *Driver) Create() error { if oci.IsCreatedByMinikube(d.OCIBinary, params.Name) { glog.Info("Found already existing abandoned minikube container, will try to delete.") if err := oci.DeleteContainer(d.OCIBinary, params.Name); err != nil { - glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", params.Name) + glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", params.Name, params.OCIBinary) return errors.Wrapf(err, "deleting already in-use mini-created container %s", params.Name) } } else { From 38ccb88e3f6d07522d983e82cf2edb85b722f899 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Fri, 6 Mar 2020 10:25:48 -0800 Subject: [PATCH 6/6] address code review --- pkg/drivers/kic/kic.go | 6 ++---- pkg/drivers/kic/oci/oci.go | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 6d9f716680e6..3839b1b957f7 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -102,14 +102,12 @@ func (d *Driver) Create() error { if oci.IsCreatedByMinikube(d.OCIBinary, params.Name) { glog.Info("Found already existing abandoned minikube container, will try to delete.") if err := oci.DeleteContainer(d.OCIBinary, params.Name); err != nil { - glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again.", params.Name, params.OCIBinary) - return errors.Wrapf(err, "deleting already in-use mini-created container %s", params.Name) + glog.Errorf("Failed to delete a conflicting minikube container %s. You might need to restart your %s daemon and delete it manually and try again: %v", params.Name, params.OCIBinary, err) } } else { // The conflicting container name was not created by minikube // user has a container that conflicts with minikube profile name, will not delete users container. - glog.Errorf("You have a container named %s that conflicts with minikube profile name %s, please either remove manually or choose a different minikbue profile name", params.Name, params.Name) - return errors.Wrap(err, "conflicting name with user's container") + return errors.Wrapf(err, "user has a conflicting container name %q with minikube container. Needs to be deleted by user's consent.", params.Name) } } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index e936d14df646..32835d705604 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -57,7 +57,7 @@ func DeleteContainersByLabel(ociBin string, label string) []error { // 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) + glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s. :%v", ociBin, ociBin, err) continue } cmd := exec.Command(ociBin, "rm", "-f", "-v", c) @@ -75,10 +75,8 @@ func DeleteContainer(ociBin string, name string) error { return errors.Wrap(err, "point host docker daemon") } _, err := ContainerStatus(ociBin, name) - // 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 { - glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s. Will try to delete anyways", ociBin, ociBin) + glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s. Will try to delete anyways: %v", ociBin, ociBin, err) } // try to delete anyways cmd := exec.Command(ociBin, "rm", "-f", "-v", name)