From a4ee4186bd32270a14ee4deb079ff975bf5fd6b6 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Fri, 14 Feb 2020 01:01:53 -0800 Subject: [PATCH 1/5] create docker volumes explictily with a label --- pkg/drivers/kic/kic.go | 2 +- pkg/drivers/kic/oci/oci.go | 36 +++++++++++++++++++++++++++--------- pkg/drivers/kic/oci/types.go | 9 ++++++--- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index bb76fbd5ef2c..0ecb6a2ade9f 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -65,7 +65,7 @@ func (d *Driver) Create() error { params := oci.CreateParams{ Name: d.NodeConfig.MachineName, Image: d.NodeConfig.ImageDigest, - ClusterLabel: oci.ClusterLabelKey + "=" + d.MachineName, + ClusterLabel: oci.ProfileLabelKey + "=" + d.MachineName, CPUs: strconv.Itoa(d.NodeConfig.CPU), Memory: strconv.Itoa(d.NodeConfig.Memory) + "mb", Envs: d.NodeConfig.Envs, diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 9442eda7bcfd..7786b8075557 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -57,24 +57,28 @@ func CreateContainerNode(p CreateParams) error { "-v", "/lib/modules:/lib/modules:ro", "--hostname", p.Name, // make hostname match container name "--name", p.Name, // ... and set the container name + "--label", fmt.Sprintf("%s=%s", CreatedByLabelKey, "true"), // label the node with the cluster ID "--label", p.ClusterLabel, // label the node with the role ID - "--label", fmt.Sprintf("%s=%s", nodeRoleKey, p.Role), - } - - // volume path in minikube home folder to mount to /var - hostVarVolPath := filepath.Join(localpath.MiniPath(), "machines", p.Name, "var") - if err := os.MkdirAll(hostVarVolPath, 0711); err != nil { - return errors.Wrapf(err, "create var dir %s", hostVarVolPath) + "--label", fmt.Sprintf("%s=%s", nodeRoleLabelKey, p.Role), } if p.OCIBinary == Podman { // enable execing in /var + // volume path in minikube home folder to mount to /var + hostVarVolPath := filepath.Join(localpath.MiniPath(), "machines", p.Name, "var") + if err := os.MkdirAll(hostVarVolPath, 0711); err != nil { + return errors.Wrapf(err, "create var dir %s", hostVarVolPath) + } // podman mounts var/lib with no-exec by default https://github.com/containers/libpod/issues/5103 runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var:exec", hostVarVolPath)) } if p.OCIBinary == Docker { - runArgs = append(runArgs, "--volume", "/var") + if err := createDockerVolume(p.Name); err != nil { + return errors.Wrapf(err, "creating volume for %s container", p.Name) + } + glog.Infof("Successfully created a docker volume %s", p.Name) + runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", p.Name)) // setting resource limit in privileged mode is only supported by docker // podman error: "Error: invalid configuration, cannot set resources with rootless containers not using cgroups v2 unified mode" runArgs = append(runArgs, fmt.Sprintf("--cpus=%s", p.CPUs), fmt.Sprintf("--memory=%s", p.Memory)) @@ -109,6 +113,20 @@ func CreateContainerNode(p CreateParams) error { return nil } +// createDockerVolume creates a docker volume to be attached to the container with correct labels and prefixes based on profile name +// Caution ! if volume already exists does NOT return an error and will not apply the minikube labels on it. +// 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") + } + cmd := exec.Command(Docker, "volume", "create", name, "--label", "name.minikube.sigs.k8s.io="+name, "--label", "craeted_by_minikube.minikube.sigs.k8s.io=true") + if out, err := cmd.CombinedOutput(); err != nil { + return errors.Wrapf(err, "output %s", string(out)) + } + return nil +} + // CreateContainer creates a container with "docker/podman run" func createContainer(ociBinary string, image string, opts ...createOpt) ([]string, error) { if err := PointToHostDockerDaemon(); err != nil { @@ -264,7 +282,7 @@ func ContainerID(ociBinary string, nameOrID string) (string, error) { // 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, ClusterLabelKey) + return listContainersByLabel(ociBinary, ProfileLabelKey) } // inspect return low-level information on containers diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 70ffbe5dbbe7..ffc71f57c0c4 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -21,12 +21,15 @@ const ( DefaultBindIPV4 = "127.0.0.1" Docker = "docker" Podman = "podman" - // ClusterLabelKey is applied to each node docker container for identification - ClusterLabelKey = "io.x-k8s.kic.cluster" + // ProfileLabelKey is applied to any container or volume created by a specific minikube profile name.minikube.sigs.k8s.io=PROFILE_NAME + ProfileLabelKey = "name.minikube.sigs.k8s.io" // NodeRoleKey is used to identify if it is control plane or worker - nodeRoleKey = "io.k8s.sigs.kic.role" + nodeRoleLabelKey = "role.minikube.sigs.k8s.io" + // CreatedByLabelKey is applied to any container/volume that is created by minikube created_by.minikube.sigs.k8s.io=true + CreatedByLabelKey = "created_by.minikube.sigs.k8s.io" ) +// CreateParams are parameters needed to create a container type CreateParams struct { Name string // used for container name and hostname Image string // container image to use to create the node. From a007c0790c7713c5319739bc7970011ef8433fc0 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Fri, 14 Feb 2020 01:39:16 -0800 Subject: [PATCH 2/5] Delete volumes created by kic --- cmd/minikube/cmd/delete.go | 12 ++++++-- pkg/drivers/kic/oci/oci.go | 14 --------- pkg/drivers/kic/oci/volumes.go | 52 ++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 pkg/drivers/kic/oci/volumes.go diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index f58bd5440e3c..7523b9a0e00b 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -32,6 +32,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" cmdcfg "k8s.io/minikube/cmd/minikube/cmd/config" + "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/cluster" "k8s.io/minikube/pkg/minikube/config" pkg_config "k8s.io/minikube/pkg/minikube/config" @@ -97,7 +98,7 @@ func runDelete(cmd *cobra.Command, args []string) { validProfiles, invalidProfiles, err := pkg_config.ListProfiles() profilesToDelete := append(validProfiles, invalidProfiles...) - + glog.Infof("error listing profiless %v", err) // If the purge flag is set, go ahead and delete the .minikube directory. if purge && len(profilesToDelete) > 1 && !deleteAll { out.ErrT(out.Notice, "Multiple minikube profiles were found - ") @@ -112,8 +113,9 @@ func runDelete(cmd *cobra.Command, args []string) { exit.UsageT("usage: minikube delete --all") } - if err != nil { - exit.WithError("Error getting profiles to delete", err) + 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. To see the list of volumes run: 'docker volume ls' \n:", err) } errs := DeleteProfiles(profilesToDelete) @@ -177,6 +179,10 @@ 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) + } api, err := machine.NewAPIClient() if err != nil { diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 7786b8075557..6380fd34b5f5 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -113,20 +113,6 @@ func CreateContainerNode(p CreateParams) error { return nil } -// createDockerVolume creates a docker volume to be attached to the container with correct labels and prefixes based on profile name -// Caution ! if volume already exists does NOT return an error and will not apply the minikube labels on it. -// 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") - } - cmd := exec.Command(Docker, "volume", "create", name, "--label", "name.minikube.sigs.k8s.io="+name, "--label", "craeted_by_minikube.minikube.sigs.k8s.io=true") - if out, err := cmd.CombinedOutput(); err != nil { - return errors.Wrapf(err, "output %s", string(out)) - } - return nil -} - // CreateContainer creates a container with "docker/podman run" func createContainer(ociBinary string, image string, opts ...createOpt) ([]string, error) { if err := PointToHostDockerDaemon(); err != nil { diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go new file mode 100644 index 000000000000..5aedd6aff538 --- /dev/null +++ b/pkg/drivers/kic/oci/volumes.go @@ -0,0 +1,52 @@ +/* +Copyright 2020 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oci + +import ( + "os/exec" + + "github.com/pkg/errors" +) + +// DeleteAllVolumesByLabel delets all volumes that have a specific label +// example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube +func DeleteAllVolumesByLabel(ociBin string, label string) error { + if ociBin == Docker { + if err := PointToHostDockerDaemon(); err != nil { + return errors.Wrap(err, "point host docker-daemon") + } + } + cmd := exec.Command(ociBin, "volume", "prune", "-f", "--filter", label) + if out, err := cmd.CombinedOutput(); err != nil { + return errors.Wrapf(err, "output %s", string(out)) + } + return nil +} + +// createDockerVolume creates a docker volume to be attached to the container with correct labels and prefixes based on profile name +// Caution ! if volume already exists does NOT return an error and will not apply the minikube labels on it. +// 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") + } + cmd := exec.Command(Docker, "volume", "create", name, "--label", "name.minikube.sigs.k8s.io="+name, "--label", "craeted_by_minikube.minikube.sigs.k8s.io=true") + if out, err := cmd.CombinedOutput(); err != nil { + return errors.Wrapf(err, "output %s", string(out)) + } + return nil +} From 3ce01642814e3b180596a6d083a5c795a20e50fa Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Sun, 16 Feb 2020 11:54:56 -0800 Subject: [PATCH 3/5] add logging for prune --- pkg/drivers/kic/oci/volumes.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index 5aedd6aff538..74b84991bf95 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -19,12 +19,14 @@ package oci import ( "os/exec" + "github.com/golang/glog" "github.com/pkg/errors" ) // DeleteAllVolumesByLabel delets all volumes that have a specific label // example: docker volume prune -f --filter label=name.minikube.sigs.k8s.io=minikube func DeleteAllVolumesByLabel(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 errors.Wrap(err, "point host docker-daemon") From 23ef3f6d164951993df3442dd58cf4545d88be74 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Sun, 16 Feb 2020 19:51:59 -0800 Subject: [PATCH 4/5] 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 7523b9a0e00b..1051cae854c8 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -115,7 +115,7 @@ func runDelete(cmd *cobra.Command, args []string) { 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. To see the list of volumes run: 'docker volume ls' \n:", err) + glog.Warningf("error deleting left docker volumes. To see the list of volumes run: 'docker volume ls' \n%v:", err) } errs := DeleteProfiles(profilesToDelete) From 1da0a370ffce69b5c21c37f0e4dd21dc41161ccb Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 17 Feb 2020 10:55:53 -0800 Subject: [PATCH 5/5] code review comments --- cmd/minikube/cmd/delete.go | 7 ++++--- pkg/drivers/kic/oci/volumes.go | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 1051cae854c8..1ec4093b13b3 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -97,9 +97,10 @@ func runDelete(cmd *cobra.Command, args []string) { profileFlag := viper.GetString(config.MachineProfile) validProfiles, invalidProfiles, err := pkg_config.ListProfiles() + glog.Warningf("Couldn't find any profiles in minikube home %q: %v", localpath.MiniPath(), err) profilesToDelete := append(validProfiles, invalidProfiles...) - glog.Infof("error listing profiless %v", err) - // If the purge flag is set, go ahead and delete the .minikube directory. + // in the case user has more than 1 profile and runs --purge + // to prevent abandoned VMs/containers, force user to run with delete --all if purge && len(profilesToDelete) > 1 && !deleteAll { out.ErrT(out.Notice, "Multiple minikube profiles were found - ") for _, p := range profilesToDelete { @@ -115,7 +116,7 @@ func runDelete(cmd *cobra.Command, args []string) { 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. To see the list of volumes run: 'docker volume ls' \n%v:", err) + 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 := DeleteProfiles(profilesToDelete) diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index 74b84991bf95..e23b35c394db 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -23,7 +23,8 @@ import ( "github.com/pkg/errors" ) -// DeleteAllVolumesByLabel delets all volumes that have a specific label +// 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 { glog.Infof("trying to prune all %s volumes with label %s", ociBin, label)