From b235255196850b3b7bcf40423ae60eead25c9551 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 13:26:24 -0800 Subject: [PATCH 01/13] fix volume clean up --- cmd/minikube/cmd/delete.go | 2 +- pkg/drivers/kic/oci/volumes.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 1ec4093b13b3..f72109160e3b 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -114,7 +114,7 @@ func runDelete(cmd *cobra.Command, args []string) { exit.UsageT("usage: minikube delete --all") } - err := oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "=true")) + err := oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) if err != nil { // if there is no volume there won't be any error glog.Warningf("error deleting left docker volumes. \n%v:\nfor more information please refer to docker documentation: https://docs.docker.com/engine/reference/commandline/volume_prune/", err) } diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index e23b35c394db..db9af4b132a8 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -17,6 +17,7 @@ limitations under the License. package oci import ( + "fmt" "os/exec" "github.com/golang/glog" @@ -47,7 +48,7 @@ func createDockerVolume(name string) error { if err := PointToHostDockerDaemon(); err != nil { return errors.Wrap(err, "point host docker-daemon") } - cmd := exec.Command(Docker, "volume", "create", name, "--label", "name.minikube.sigs.k8s.io="+name, "--label", "craeted_by_minikube.minikube.sigs.k8s.io=true") + 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 { return errors.Wrapf(err, "output %s", string(out)) } From efd4aabde4cbf7db214e6a543e5ee595a9dfd0de Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 14:31:08 -0800 Subject: [PATCH 02/13] clean up containers first then delete volume --- cmd/minikube/cmd/delete.go | 24 +++++++++++++------- pkg/drivers/kic/oci/oci.go | 41 ++++++++++++++++++++++++++-------- pkg/drivers/kic/oci/volumes.go | 41 +++++++++++++++++++++++++++++----- 3 files changed, 84 insertions(+), 22 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index f72109160e3b..53ed8346621c 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -113,13 +113,16 @@ func runDelete(cmd *cobra.Command, args []string) { if profileFlag != constants.DefaultMachineName { exit.UsageT("usage: minikube delete --all") } - - err := oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) - if err != nil { // if there is no volume there won't be any error - glog.Warningf("error deleting left docker volumes. \n%v:\nfor more information please refer to docker documentation: https://docs.docker.com/engine/reference/commandline/volume_prune/", err) + errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) + if errs != nil { // it will error if there is no container to delete + glog.Infof("no left over kic container for %s found.", errs) + } + errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) + if errs != nil { // it will not error if there is nothing to delete + glog.Warningf("error deleting left docker volumes. To see the list of volumes run: 'docker volume ls' \n:%+v", errs) } - errs := DeleteProfiles(profilesToDelete) + errs = DeleteProfiles(profilesToDelete) if len(errs) > 0 { HandleDeletionErrors(errs) } else { @@ -180,9 +183,14 @@ func DeleteProfiles(profiles []*pkg_config.Profile) []error { func deleteProfile(profile *pkg_config.Profile) error { viper.Set(pkg_config.MachineProfile, profile.Name) - err := oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) - if err != nil { // if there is no volume there wont be any error - glog.Warningf("error deleting left docker volumes. To see the list of volumes run: 'docker volume ls' \n:%v", err) + + errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) + if errs != nil { // it will error if there is no container to delete + glog.Infof("no left over kic container for %s found to delete.", profile.Name, errs) + } + errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) + if errs != nil { // it will not error if there is nothing to delete + glog.Warningf("error deleting left docker volumes. To see the list of volumes run: 'docker volume ls' \n:%+v", errs) } api, err := machine.NewAPIClient() diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 6380fd34b5f5..3baaa60e4d32 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -34,6 +34,32 @@ import ( "strings" ) +// DeleteAllContainersByLabel deletes all containers that have a specific label +// if contatiners founds will not return an error +// example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube +func DeleteAllContainersByLabel(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")} + } + } + cs, err := listContainersByLabel(ociBin, label) + if err != nil { + glog.Infof("error listing containers by label %q: %v", label, err) + } + if cs != nil { + for _, c := range cs { + cmd := exec.Command(ociBin, "rm", "-f", "-v", c) + if out, err := cmd.CombinedOutput(); err != nil { + glog.Infof("error deleting container %s: output: %s", c, string(out)) + deleteErrs = append(deleteErrs, err) + } + } + } + return deleteErrs +} + // CreateContainerNode creates a new container node func CreateContainerNode(p CreateParams) error { if err := PointToHostDockerDaemon(); err != nil { @@ -406,16 +432,13 @@ func listContainersByLabel(ociBinary string, label string) ([]string, error) { return nil, errors.Wrap(err, "point host docker-daemon") } cmd := exec.Command(ociBinary, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}") - var b bytes.Buffer - cmd.Stdout = &b - cmd.Stderr = &b - err := cmd.Run() - var lines []string - sc := bufio.NewScanner(&b) - for sc.Scan() { - lines = append(lines, sc.Text()) + stdout, err := cmd.Output() + outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") + var names []string + for _, o := range outs { + names = append(names, strings.TrimSpace(o)) } - return lines, err + return names, err } // PointToHostDockerDaemon will unset env variables that point to docker inside minikube diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index db9af4b132a8..bd748620c759 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -19,6 +19,7 @@ package oci import ( "fmt" "os/exec" + "strings" "github.com/golang/glog" "github.com/pkg/errors" @@ -27,18 +28,48 @@ import ( // DeleteAllVolumesByLabel deletes all volumes that have a specific label // if there is no volume to delete it will return nil // example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube -func DeleteAllVolumesByLabel(ociBin string, label string) error { +func DeleteAllVolumesByLabel(ociBin string, label string) []error { + var deleteErrs []error glog.Infof("trying to prune all %s volumes with label %s", ociBin, label) if ociBin == Docker { if err := PointToHostDockerDaemon(); err != nil { - return errors.Wrap(err, "point host docker-daemon") + return []error{errors.Wrap(err, "point host docker-daemon")} } } - cmd := exec.Command(ociBin, "volume", "prune", "-f", "--filter", label) + + vs, err := allVolumesByLabel(ociBin, label) + if err != nil { + glog.Infof("error listing volumes by label %q: %v", label, err) + } + if vs != nil { + for _, v := range vs { + cmd := exec.Command(ociBin, "volume", "rm", "--force", v) + if out, err := cmd.CombinedOutput(); err != nil { + glog.Infof("error deleting volume %s: output: %s", v, string(out)) + deleteErrs = append(deleteErrs, err) + } + } + } + + // try to prune afterwards just in case delete didn't go through + cmd := exec.Command(ociBin, "volume", "prune", "-f", "--filter", "label="+label) if out, err := cmd.CombinedOutput(); err != nil { - return errors.Wrapf(err, "output %s", string(out)) + deleteErrs = append(deleteErrs, errors.Wrapf(err, "prune volume %s", string(out))) } - return nil + return deleteErrs +} + +// allVolumesByLabel returns name of all docker volumes by a specific label +// will not return error if there is no volume found. +func allVolumesByLabel(ociBin string, label string) ([]string, error) { + cmd := exec.Command(ociBin, "volume", "ls", "--filter", "label="+label, "--format", "{{.Name}}") + stdout, err := cmd.Output() + outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") + var tirmOuts []string + for _, o := range outs { + tirmOuts = append(tirmOuts, strings.TrimSpace(o)) + } + return tirmOuts, err } // createDockerVolume creates a docker volume to be attached to the container with correct labels and prefixes based on profile name From d3b86cfeb9d0533023860b915b7ebf7ff211b0db Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 14:34:35 -0800 Subject: [PATCH 03/13] improve logging --- cmd/minikube/cmd/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 53ed8346621c..4a4295b07d63 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -186,7 +186,7 @@ func deleteProfile(profile *pkg_config.Profile) error { errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) if errs != nil { // it will error if there is no container to delete - glog.Infof("no left over kic container for %s found to delete.", profile.Name, errs) + glog.Infof("no left over kic container for %s found to delete. %+v", profile.Name, errs) } errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) if errs != nil { // it will not error if there is nothing to delete From b1d139c24bcba0bd6c942c3c17b3e76787f786e8 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 15:05:25 -0800 Subject: [PATCH 04/13] code review comments --- cmd/minikube/cmd/delete.go | 2 +- pkg/drivers/kic/oci/volumes.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 4a4295b07d63..0181baf69f8a 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -115,7 +115,7 @@ func runDelete(cmd *cobra.Command, args []string) { } errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) if errs != nil { // it will error if there is no container to delete - glog.Infof("no left over kic container for %s found.", errs) + glog.Infof("no left over minikube container found.", errs) } errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) if errs != nil { // it will not error if there is nothing to delete diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index bd748620c759..4e15f5efa022 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -65,11 +65,11 @@ func allVolumesByLabel(ociBin string, label string) ([]string, error) { cmd := exec.Command(ociBin, "volume", "ls", "--filter", "label="+label, "--format", "{{.Name}}") stdout, err := cmd.Output() outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") - var tirmOuts []string + var vols []string for _, o := range outs { - tirmOuts = append(tirmOuts, strings.TrimSpace(o)) + vols = append(vols, strings.TrimSpace(o)) } - return tirmOuts, err + return vols, err } // createDockerVolume creates a docker volume to be attached to the container with correct labels and prefixes based on profile name From d84aa9f44726f279f88c0fe5951f36981398d4ad Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 15:18:18 -0800 Subject: [PATCH 05/13] lint --- cmd/minikube/cmd/delete.go | 2 +- pkg/drivers/kic/oci/oci.go | 4 ++-- pkg/drivers/kic/oci/volumes.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 0181baf69f8a..37089b511f6c 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -115,7 +115,7 @@ func runDelete(cmd *cobra.Command, args []string) { } errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) if errs != nil { // it will error if there is no container to delete - glog.Infof("no left over minikube container found.", errs) + glog.Infof("no left over minikube container found. %v", errs) } errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) if errs != nil { // it will not error if there is nothing to delete diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 3baaa60e4d32..8b3173aaef20 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -48,8 +48,8 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error { if err != nil { glog.Infof("error listing containers by label %q: %v", label, err) } - if cs != nil { - for _, c := range cs { + for _, c := range cs { + if c != "" { cmd := exec.Command(ociBin, "rm", "-f", "-v", c) if out, err := cmd.CombinedOutput(); err != nil { glog.Infof("error deleting container %s: output: %s", c, string(out)) diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index 4e15f5efa022..3d0c996ef163 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -41,8 +41,8 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error { if err != nil { glog.Infof("error listing volumes by label %q: %v", label, err) } - if vs != nil { - for _, v := range vs { + for _, v := range vs { + if v != "" { cmd := exec.Command(ociBin, "volume", "rm", "--force", v) if out, err := cmd.CombinedOutput(); err != nil { glog.Infof("error deleting volume %s: output: %s", v, string(out)) From 6c94880149427ce15a3be82e75f1244ba2d82b0c Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 15:22:22 -0800 Subject: [PATCH 06/13] dont list empty ones --- pkg/drivers/kic/oci/oci.go | 14 +++++++------- pkg/drivers/kic/oci/volumes.go | 15 ++++++++------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 8b3173aaef20..49611fa7570f 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -49,12 +49,10 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error { glog.Infof("error listing containers by label %q: %v", label, err) } for _, c := range cs { - if c != "" { - cmd := exec.Command(ociBin, "rm", "-f", "-v", c) - if out, err := cmd.CombinedOutput(); err != nil { - glog.Infof("error deleting container %s: output: %s", c, string(out)) - deleteErrs = append(deleteErrs, err) - } + cmd := exec.Command(ociBin, "rm", "-f", "-v", c) + if out, err := cmd.CombinedOutput(); err != nil { + glog.Infof("error deleting container %s: output: %s", c, string(out)) + deleteErrs = append(deleteErrs, err) } } return deleteErrs @@ -436,7 +434,9 @@ func listContainersByLabel(ociBinary string, label string) ([]string, error) { outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") var names []string for _, o := range outs { - names = append(names, strings.TrimSpace(o)) + if strings.TrimSpace(o) != "" { + names = append(names, strings.TrimSpace(o)) + } } return names, err } diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index 3d0c996ef163..b32b81dffda7 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -42,12 +42,10 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error { glog.Infof("error listing volumes by label %q: %v", label, err) } for _, v := range vs { - if v != "" { - cmd := exec.Command(ociBin, "volume", "rm", "--force", v) - if out, err := cmd.CombinedOutput(); err != nil { - glog.Infof("error deleting volume %s: output: %s", v, string(out)) - deleteErrs = append(deleteErrs, err) - } + cmd := exec.Command(ociBin, "volume", "rm", "--force", v) + if out, err := cmd.CombinedOutput(); err != nil { + glog.Infof("error deleting volume %s: output: %s", v, string(out)) + deleteErrs = append(deleteErrs, err) } } @@ -67,7 +65,10 @@ func allVolumesByLabel(ociBin string, label string) ([]string, error) { outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") var vols []string for _, o := range outs { - vols = append(vols, strings.TrimSpace(o)) + if strings.TrimSpace(o) != "" { + vols = append(vols, strings.TrimSpace(o)) + } + } return vols, err } From 8d45b236de91ef5dff452e2f54e8ea3dfbed8bd3 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 20:45:34 -0800 Subject: [PATCH 07/13] less ambigous error log --- cmd/minikube/cmd/delete.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 37089b511f6c..938166428320 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -113,13 +113,14 @@ func runDelete(cmd *cobra.Command, args []string) { if profileFlag != constants.DefaultMachineName { exit.UsageT("usage: minikube delete --all") } - errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) - if errs != nil { // it will error if there is no container to delete - glog.Infof("no left over minikube container found. %v", errs) + delLabel := fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true") + errs := oci.DeleteAllContainersByLabel(oci.Docker, delLabel) + if errs != nil && 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) } - errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true")) - if errs != nil { // it will not error if there is nothing to delete - glog.Warningf("error deleting left docker volumes. To see the list of volumes run: 'docker volume ls' \n:%+v", errs) + errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) + if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete + glog.Warningf("error delete volumes by label %q (might be okay): %+v", delLabel, errs) } errs = DeleteProfiles(profilesToDelete) From 354ff6256d47efea92444bc361be58484926ca93 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 20:49:31 -0800 Subject: [PATCH 08/13] improve comments --- pkg/drivers/kic/oci/oci.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 49611fa7570f..46da8b02675b 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -35,8 +35,7 @@ import ( ) // DeleteAllContainersByLabel deletes all containers that have a specific label -// if contatiners founds will not return an error -// example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube +// if there are no containers found with the label, it will return nil func DeleteAllContainersByLabel(ociBin string, label string) []error { var deleteErrs []error if ociBin == Docker { @@ -48,6 +47,9 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error { if err != nil { glog.Infof("error listing containers by label %q: %v", label, err) } + if len(cs) == 0 { + return nil + } for _, c := range cs { cmd := exec.Command(ociBin, "rm", "-f", "-v", c) if out, err := cmd.CombinedOutput(); err != nil { @@ -423,8 +425,7 @@ func withPortMappings(portMappings []PortMapping) createOpt { } } -// listContainersByLabel lists all the containres that kic driver created on user's machine using a label -// io.x-k8s.kic.cluster +// 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") From e1ffa5fb0f6beb882f2e57ed351e900b701ff77a Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 20:52:07 -0800 Subject: [PATCH 09/13] return error instead of logging --- pkg/drivers/kic/oci/oci.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 46da8b02675b..8b2aa2444418 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -45,7 +45,7 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error { } cs, err := listContainersByLabel(ociBin, label) if err != nil { - glog.Infof("error listing containers by label %q: %v", label, err) + return []error{fmt.Errorf("listing containers by label %q", label)} } if len(cs) == 0 { return nil @@ -53,7 +53,6 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error { for _, c := range cs { cmd := exec.Command(ociBin, "rm", "-f", "-v", c) if out, err := cmd.CombinedOutput(); err != nil { - glog.Infof("error deleting container %s: output: %s", c, string(out)) deleteErrs = append(deleteErrs, err) } } From 4fe9ca5f054ab664a46b5f2fdf405d9ff1e4b75f Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 21:18:07 -0800 Subject: [PATCH 10/13] break done delete and prune into two funcs better error handling --- cmd/minikube/cmd/delete.go | 20 ++++++++++++++---- pkg/drivers/kic/oci/oci.go | 12 ++++++----- pkg/drivers/kic/oci/volumes.go | 37 ++++++++++++++++++++++++---------- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 938166428320..fe7aec743e57 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -118,11 +118,17 @@ func runDelete(cmd *cobra.Command, args []string) { if errs != nil && 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) } + errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete glog.Warningf("error delete volumes by label %q (might be okay): %+v", delLabel, errs) } + errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) + if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete + glog.Warningf("error pruning volumes by label %q (might be okay): %+v", delLabel, errs) + } + errs = DeleteProfiles(profilesToDelete) if len(errs) > 0 { HandleDeletionErrors(errs) @@ -185,13 +191,19 @@ func DeleteProfiles(profiles []*pkg_config.Profile) []error { func deleteProfile(profile *pkg_config.Profile) error { viper.Set(pkg_config.MachineProfile, profile.Name) - errs := oci.DeleteAllContainersByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) + delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name) + errs := oci.DeleteAllContainersByLabel(oci.Docker, delLabel) if errs != nil { // it will error if there is no container to delete - glog.Infof("no left over kic container for %s found to delete. %+v", profile.Name, errs) + glog.Infof("error deleting containers (might be okay):\n%v", profile.Name, errs) } - errs = oci.DeleteAllVolumesByLabel(oci.Docker, fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name)) + errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) if errs != nil { // it will not error if there is nothing to delete - glog.Warningf("error deleting left docker volumes. To see the list of volumes run: 'docker volume ls' \n:%+v", errs) + glog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs) + } + + errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) + if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete + glog.Warningf("error pruning volume (might be okay):\n%v", delLabel, errs) } api, err := machine.NewAPIClient() diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 8b2aa2444418..d92fa2a0d42c 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -35,7 +35,7 @@ import ( ) // DeleteAllContainersByLabel deletes all containers that have a specific label -// if there are no containers found with the label, it will return nil +// if there no containers found with the given label, it will return nil func DeleteAllContainersByLabel(ociBin string, label string) []error { var deleteErrs []error if ociBin == Docker { @@ -431,13 +431,15 @@ func listContainersByLabel(ociBinary string, label string) ([]string, error) { } cmd := exec.Command(ociBinary, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}") stdout, err := cmd.Output() - outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") + s := bufio.NewScanner(bytes.NewReader(stdout)) var names []string - for _, o := range outs { - if strings.TrimSpace(o) != "" { - names = append(names, strings.TrimSpace(o)) + for s.Scan() { + n := strings.TrimSpace(s.Text()) + if n != "" { + names = append(names, n) } } + return names, err } diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index b32b81dffda7..a06f471d043d 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -17,6 +17,8 @@ limitations under the License. package oci import ( + "bufio" + "bytes" "fmt" "os/exec" "strings" @@ -27,10 +29,9 @@ import ( // DeleteAllVolumesByLabel deletes all volumes that have a specific label // if there is no volume to delete it will return nil -// example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube func DeleteAllVolumesByLabel(ociBin string, label string) []error { var deleteErrs []error - glog.Infof("trying to prune all %s volumes with label %s", ociBin, label) + 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")} @@ -39,20 +40,34 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error { vs, err := allVolumesByLabel(ociBin, label) if err != nil { - glog.Infof("error listing volumes by label %q: %v", label, err) + return []error{fmt.Errorf("listing volumes by label %q: %v", label, err)} } + for _, v := range vs { cmd := exec.Command(ociBin, "volume", "rm", "--force", v) if out, err := cmd.CombinedOutput(); err != nil { - glog.Infof("error deleting volume %s: output: %s", v, string(out)) - deleteErrs = append(deleteErrs, err) + deleteErrs = append(deleteErrs, fmt.Errorf("deleting volume %s: output: %s", v, string(out))) + } + } + return deleteErrs +} + +// PruneAllVolumesByLabel deletes all volumes that have a specific label +// if there is no volume to delete it will return nil +// example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube +func PruneAllVolumesByLabel(ociBin string, label string) []error { + var deleteErrs []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")} } } // try to prune afterwards just in case delete didn't go through cmd := exec.Command(ociBin, "volume", "prune", "-f", "--filter", "label="+label) if out, err := cmd.CombinedOutput(); err != nil { - deleteErrs = append(deleteErrs, errors.Wrapf(err, "prune volume %s", string(out))) + deleteErrs = append(deleteErrs, errors.Wrapf(err, "prune volume by label %s: %s", label, string(out))) } return deleteErrs } @@ -62,13 +77,13 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error { func allVolumesByLabel(ociBin string, label string) ([]string, error) { cmd := exec.Command(ociBin, "volume", "ls", "--filter", "label="+label, "--format", "{{.Name}}") stdout, err := cmd.Output() - outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") + s := bufio.NewScanner(bytes.NewReader(stdout)) var vols []string - for _, o := range outs { - if strings.TrimSpace(o) != "" { - vols = append(vols, strings.TrimSpace(o)) + for s.Scan() { + v := strings.TrimSpace(s.Text()) + if v != "" { + vols = append(vols, v) } - } return vols, err } From 30a38e5a9dda4f3ed88142e86edc0aba7024cde0 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 22:15:30 -0800 Subject: [PATCH 11/13] wrap error --- pkg/drivers/kic/oci/oci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index d92fa2a0d42c..630b6db4b92a 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -53,7 +53,7 @@ func DeleteAllContainersByLabel(ociBin string, label string) []error { for _, c := range cs { cmd := exec.Command(ociBin, "rm", "-f", "-v", c) if out, err := cmd.CombinedOutput(); err != nil { - deleteErrs = append(deleteErrs, err) + deleteErrs = append(deleteErrs, errors.Wrapf(err, "delete container %s: output %s", c, out)) } } return deleteErrs From dcff758753d3b4bd2d1694ae2cced0e635815247 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 19 Feb 2020 22:37:10 -0800 Subject: [PATCH 12/13] lint --- cmd/minikube/cmd/delete.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index fe7aec743e57..6267c79009f9 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -115,17 +115,17 @@ func runDelete(cmd *cobra.Command, args []string) { } delLabel := fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true") errs := oci.DeleteAllContainersByLabel(oci.Docker, delLabel) - if errs != nil && len(errs) > 0 { // it will error if there is no container to delete + 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) } errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) - if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete + if len(errs) > 0 { // it will not error if there is nothing to delete glog.Warningf("error delete volumes by label %q (might be okay): %+v", delLabel, errs) } errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) - if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete + if len(errs) > 0 { // it will not error if there is nothing to delete glog.Warningf("error pruning volumes by label %q (might be okay): %+v", delLabel, errs) } @@ -194,7 +194,7 @@ func deleteProfile(profile *pkg_config.Profile) error { delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, profile.Name) errs := oci.DeleteAllContainersByLabel(oci.Docker, delLabel) if errs != nil { // it will error if there is no container to delete - glog.Infof("error deleting containers (might be okay):\n%v", profile.Name, errs) + glog.Infof("error deleting containers for %s (might be okay):\n%v", profile.Name, errs) } errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) if errs != nil { // it will not error if there is nothing to delete @@ -203,7 +203,7 @@ func deleteProfile(profile *pkg_config.Profile) error { errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete - glog.Warningf("error pruning volume (might be okay):\n%v", delLabel, errs) + glog.Warningf("error pruning volume (might be okay):\n%v", errs) } api, err := machine.NewAPIClient() From 78c192cb40731a103f20b385c44eedb3a8727328 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Thu, 20 Feb 2020 00:07:30 -0800 Subject: [PATCH 13/13] lint --- cmd/minikube/cmd/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 6267c79009f9..2eefcce6a986 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -202,7 +202,7 @@ func deleteProfile(profile *pkg_config.Profile) error { } errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) - if errs != nil && len(errs) > 0 { // it will not error if there is nothing to delete + 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) }