From d7dc7bf7b2135cedf835b68285c9b0377720498d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 12 Apr 2020 16:12:30 +0200 Subject: [PATCH 01/26] The podman driver should not be run as root Use sudo for the podman commands instead Wrap the docker commands with env prefix --- cmd/minikube/cmd/delete.go | 12 +-- .../jenkins/linux_integration_tests_podman.sh | 1 - hack/preload-images/generate.go | 1 + pkg/drivers/kic/kic.go | 35 +++++---- pkg/drivers/kic/oci/info.go | 2 +- pkg/drivers/kic/oci/network.go | 8 +- pkg/drivers/kic/oci/oci.go | 74 +++++++++---------- pkg/drivers/kic/oci/types.go | 1 + pkg/drivers/kic/oci/volumes.go | 14 ++-- pkg/drivers/kic/types.go | 1 + pkg/minikube/command/kic_runner.go | 11 ++- pkg/minikube/config/profile.go | 2 +- pkg/minikube/driver/driver.go | 2 +- pkg/minikube/machine/delete.go | 12 +-- pkg/minikube/machine/stop.go | 2 +- pkg/minikube/registry/drvs/docker/docker.go | 3 +- pkg/minikube/registry/drvs/podman/podman.go | 7 +- 17 files changed, 99 insertions(+), 89 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index b25696ae3458..e37949957c5b 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -90,17 +90,17 @@ func init() { func deleteContainersAndVolumes() { delLabel := fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true") - errs := oci.DeleteContainersByLabel(oci.Docker, delLabel) + errs := oci.DeleteContainersByLabel("env", 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, errs) } - errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) + errs = oci.DeleteAllVolumesByLabel("env", oci.Docker, delLabel) 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) + errs = oci.PruneAllVolumesByLabel("env", oci.Docker, delLabel) 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) } @@ -191,16 +191,16 @@ func DeleteProfiles(profiles []*config.Profile) []error { func deleteProfileContainersAndVolumes(name string) { delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, name) - errs := oci.DeleteContainersByLabel(oci.Docker, delLabel) + errs := oci.DeleteContainersByLabel("env", 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", name, errs) } - errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) + errs = oci.DeleteAllVolumesByLabel("env", oci.Docker, delLabel) if errs != nil { // it will not error if there is nothing to delete 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) + errs = oci.PruneAllVolumesByLabel("env", oci.Docker, delLabel) 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) } diff --git a/hack/jenkins/linux_integration_tests_podman.sh b/hack/jenkins/linux_integration_tests_podman.sh index 1dfbdb445600..455af2955bbb 100755 --- a/hack/jenkins/linux_integration_tests_podman.sh +++ b/hack/jenkins/linux_integration_tests_podman.sh @@ -31,7 +31,6 @@ JOB_NAME="Podman_Linux" mkdir -p cron && gsutil -qm rsync "gs://minikube-builds/${MINIKUBE_LOCATION}/cron" cron || echo "FAILED TO GET CRON FILES" sudo install cron/cleanup_and_reboot_Linux.sh /etc/cron.hourly/cleanup_and_reboot || echo "FAILED TO INSTALL CLEANUP" -SUDO_PREFIX="sudo -E " EXTRA_ARGS="--container-runtime=containerd" diff --git a/hack/preload-images/generate.go b/hack/preload-images/generate.go index 1a22e7404de4..7475901fae50 100644 --- a/hack/preload-images/generate.go +++ b/hack/preload-images/generate.go @@ -44,6 +44,7 @@ func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string driver := kic.NewDriver(kic.Config{ KubernetesVersion: kubernetesVersion, ContainerRuntime: driver.Docker, + OCIPrefix: "env", OCIBinary: oci.Docker, MachineName: profile, ImageDigest: kic.BaseImage, diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 05b1e325ca28..cd7dae2803a3 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -48,6 +48,7 @@ type Driver struct { URL string exec command.Runner NodeConfig Config + OCIPrefix string // env, sudo OCIBinary string // docker,podman } @@ -58,8 +59,9 @@ func NewDriver(c Config) *Driver { MachineName: c.MachineName, StorePath: c.StorePath, }, - exec: command.NewKICRunner(c.MachineName, c.OCIBinary), + exec: command.NewKICRunner(c.MachineName, c.OCIPrefix, c.OCIBinary), NodeConfig: c, + OCIPrefix: c.OCIPrefix, OCIBinary: c.OCIBinary, } return d @@ -76,6 +78,7 @@ func (d *Driver) Create() error { Memory: strconv.Itoa(d.NodeConfig.Memory) + "mb", Envs: d.NodeConfig.Envs, ExtraArgs: []string{"--expose", fmt.Sprintf("%d", d.NodeConfig.APIServerPort)}, + OCIPrefix: d.NodeConfig.OCIPrefix, OCIBinary: d.NodeConfig.OCIBinary, APIServerPort: d.NodeConfig.APIServerPort, } @@ -99,15 +102,15 @@ func (d *Driver) Create() error { }, ) - exists, err := oci.ContainerExists(d.OCIBinary, params.Name) + exists, err := oci.ContainerExists(d.OCIPrefix, d.OCIBinary, params.Name) if err != nil { 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) { + if oci.IsCreatedByMinikube(d.OCIPrefix, 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 { + if err := oci.DeleteContainer(d.OCIPrefix, 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: %v", params.Name, params.OCIBinary, err) } } else { @@ -159,7 +162,7 @@ func (d *Driver) prepareSSH() error { return errors.Wrap(err, "generate ssh key") } - cmder := command.NewKICRunner(d.NodeConfig.MachineName, d.NodeConfig.OCIBinary) + cmder := command.NewKICRunner(d.NodeConfig.MachineName, d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary) f, err := assets.NewFileAsset(d.GetSSHKeyPath()+".pub", "/home/docker/.ssh/", "authorized_keys", "0644") if err != nil { return errors.Wrap(err, "create pubkey assetfile ") @@ -234,7 +237,7 @@ 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) { - out, err := oci.WarnIfSlow(d.NodeConfig.OCIBinary, "inspect", "-f", "{{.State.Status}}", d.MachineName) + out, err := oci.WarnIfSlow(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "inspect", "-f", "{{.State.Status}}", d.MachineName) if err != nil { return state.Error, err } @@ -259,17 +262,17 @@ func (d *Driver) GetState() (state.State, error) { // Kill stops a host forcefully, including any containers that we are managing. func (d *Driver) Kill() error { // on init this doesn't get filled when called from cmd - d.exec = command.NewKICRunner(d.MachineName, d.OCIBinary) + d.exec = command.NewKICRunner(d.MachineName, d.OCIPrefix, d.OCIBinary) if err := sysinit.New(d.exec).ForceStop("kubelet"); err != nil { glog.Warningf("couldn't force stop kubelet. will continue with kill anyways: %v", err) } - if err := oci.ShutDown(d.OCIBinary, d.MachineName); err != nil { + if err := oci.ShutDown(d.OCIPrefix, d.OCIBinary, d.MachineName); err != nil { glog.Warningf("couldn't shutdown the container, will continue with kill anyways: %v", err) } cr := command.NewExecRunner() // using exec runner for interacting with dameon. - if _, err := cr.RunCmd(exec.Command(d.NodeConfig.OCIBinary, "kill", d.MachineName)); err != nil { + if _, err := cr.RunCmd(exec.Command(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "kill", d.MachineName)); err != nil { return errors.Wrapf(err, "killing %q", d.MachineName) } return nil @@ -277,11 +280,11 @@ func (d *Driver) Kill() error { // Remove will delete the Kic Node Container func (d *Driver) Remove() error { - if _, err := oci.ContainerID(d.OCIBinary, d.MachineName); err != nil { + if _, err := oci.ContainerID(d.OCIPrefix, d.OCIBinary, d.MachineName); err != nil { glog.Infof("could not find the container %s to remove it. will try anyways", d.MachineName) } - if err := oci.DeleteContainer(d.NodeConfig.OCIBinary, d.MachineName); err != nil { + if err := oci.DeleteContainer(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, d.MachineName); err != nil { if strings.Contains(err.Error(), "is already in progress") { return errors.Wrap(err, "stuck delete") } @@ -292,7 +295,7 @@ func (d *Driver) Remove() error { } // check there be no container left after delete - if id, err := oci.ContainerID(d.OCIBinary, d.MachineName); err == nil && id != "" { + if id, err := oci.ContainerID(d.OCIPrefix, d.OCIBinary, d.MachineName); err == nil && id != "" { return fmt.Errorf("expected no container ID be found for %q after delete. but got %q", d.MachineName, id) } return nil @@ -320,11 +323,11 @@ func (d *Driver) Restart() error { // Start an already created kic container func (d *Driver) Start() error { cr := command.NewExecRunner() // using exec runner for interacting with docker/podman daemon - if _, err := cr.RunCmd(exec.Command(d.NodeConfig.OCIBinary, "start", d.MachineName)); err != nil { + if _, err := cr.RunCmd(exec.Command(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "start", d.MachineName)); err != nil { return errors.Wrap(err, "start") } checkRunning := func() error { - s, err := oci.ContainerStatus(d.NodeConfig.OCIBinary, d.MachineName) + s, err := oci.ContainerStatus(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, d.MachineName) if err != nil { return err } @@ -344,7 +347,7 @@ func (d *Driver) Start() error { // Stop a host gracefully, including any containers that we are managing. func (d *Driver) Stop() error { // on init this doesn't get filled when called from cmd - d.exec = command.NewKICRunner(d.MachineName, d.OCIBinary) + d.exec = command.NewKICRunner(d.MachineName, d.OCIPrefix, d.OCIBinary) // docker does not send right SIG for systemd to know to stop the systemd. // to avoid bind address be taken on an upgrade. more info https://github.com/kubernetes/minikube/issues/7171 if err := sysinit.New(d.exec).Stop("kubelet"); err != nil { @@ -379,7 +382,7 @@ func (d *Driver) Stop() error { glog.Warningf("couldn't stop kube-apiserver proc: %v", err) } - cmd := exec.Command(d.NodeConfig.OCIBinary, "stop", d.MachineName) + cmd := exec.Command(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "stop", d.MachineName) if err := cmd.Run(); err != nil { return errors.Wrapf(err, "stopping %s", d.MachineName) } diff --git a/pkg/drivers/kic/oci/info.go b/pkg/drivers/kic/oci/info.go index 548c071f16d5..73a944d7bbb5 100644 --- a/pkg/drivers/kic/oci/info.go +++ b/pkg/drivers/kic/oci/info.go @@ -234,7 +234,7 @@ func dockerSystemInfo() (dockerSysInfo, error) { // podmanSysInfo returns podman system info --format '{{json .}}' func podmanSystemInfo() (podmanSysInfo, error) { var ps podmanSysInfo - cmd := exec.Command(Podman, "system", "info", "--format", "'{{json .}}'") + cmd := exec.Command("sudo", Podman, "system", "info", "--format", "'{{json .}}'") out, err := cmd.CombinedOutput() if err != nil { return ps, errors.Wrap(err, "get podman system info") diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index 1dbb8232a56c..600e77ab9556 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -66,7 +66,7 @@ func dockerGatewayIP() (net.IP, error) { } bridgeID := strings.TrimSpace(string(out)) - cmd = exec.Command(Docker, "inspect", + cmd = exec.Command("env", Docker, "inspect", "--format", "{{(index .IPAM.Config 0).Gateway}}", bridgeID) out, err = cmd.CombinedOutput() @@ -90,13 +90,13 @@ func ForwardedPort(ociBinary string, ociID string, contPort int) (int, error) { if ociBinary == Podman { //podman inspect -f "{{range .NetworkSettings.Ports}}{{if eq .ContainerPort "80"}}{{.HostPort}}{{end}}{{end}}" - cmd := exec.Command(ociBinary, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID) + cmd := exec.Command("sudo", ociBinary, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID) out, err = cmd.CombinedOutput() if err != nil { return 0, errors.Wrapf(err, "get host-bind port %d for %q, output %s", contPort, ociID, out) } } else { - cmd := exec.Command(ociBinary, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID) + cmd := exec.Command("env", ociBinary, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID) out, err = cmd.CombinedOutput() if err != nil { return 0, errors.Wrapf(err, "get host-bind port %d for %q, output %s", contPort, ociID, out) @@ -141,7 +141,7 @@ func podmanConttainerIP(name string) (string, string, error) { // dockerContainerIP returns ipv4, ipv6 of container or error func dockerContainerIP(name string) (string, string, error) { // retrieve the IP address of the node using docker inspect - lines, err := inspect(Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}") + lines, err := inspect("env", Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}") if err != nil { return "", "", errors.Wrap(err, "inspecting NetworkSettings.Networks") } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index d508784874ee..cf3ebe15c2b7 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -40,10 +40,10 @@ import ( // DeleteContainersByLabel deletes all containers that have a specific label // if there no containers found with the given label, it will return nil -func DeleteContainersByLabel(ociBin string, label string) []error { +func DeleteContainersByLabel(prefix string, ociBin string, label string) []error { var deleteErrs []error - cs, err := listContainersByLabel(ociBin, label) + cs, err := listContainersByLabel(prefix, ociBin, label) if err != nil { return []error{fmt.Errorf("listing containers by label %q", label)} } @@ -53,7 +53,7 @@ func DeleteContainersByLabel(ociBin string, label string) []error { } for _, c := range cs { - _, err := ContainerStatus(ociBin, c) + _, err := ContainerStatus(prefix, 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 { @@ -61,10 +61,10 @@ func DeleteContainersByLabel(ociBin string, label string) []error { glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s. :%v", ociBin, ociBin, err) continue } - if err := ShutDown(ociBin, c); err != nil { + if err := ShutDown(prefix, ociBin, c); err != nil { glog.Infof("couldn't shut down %s (might be okay): %v ", c, err) } - cmd := exec.Command(ociBin, "rm", "-f", "-v", c) + cmd := exec.Command(prefix, 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)) } @@ -74,17 +74,17 @@ func DeleteContainersByLabel(ociBin string, label string) []error { } // DeleteContainer deletes a container by ID or Name -func DeleteContainer(ociBin string, name string) error { +func DeleteContainer(prefix string, ociBin string, name string) error { - _, err := ContainerStatus(ociBin, name) + _, err := ContainerStatus(prefix, ociBin, name) if err != nil { 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 - if err := ShutDown(ociBin, name); err != nil { + if err := ShutDown(prefix, ociBin, name); err != nil { glog.Infof("couldn't shut down %s (might be okay): %v ", name, err) } - cmd := exec.Command(ociBin, "rm", "-f", "-v", name) + cmd := exec.Command(prefix, ociBin, "rm", "-f", "-v", name) if out, err := cmd.CombinedOutput(); err != nil { return errors.Wrapf(err, "delete container %s: output %s", name, out) } @@ -157,18 +157,18 @@ func CreateContainerNode(p CreateParams) error { // adds node specific args runArgs = append(runArgs, p.ExtraArgs...) - if enabled := isUsernsRemapEnabled(p.OCIBinary); enabled { + if enabled := isUsernsRemapEnabled(p.OCIPrefix, p.OCIBinary); enabled { // We need this argument in order to make this command work // in systems that have userns-remap enabled on the docker daemon runArgs = append(runArgs, "--userns=host") } - if err := createContainer(p.OCIBinary, p.Image, withRunArgs(runArgs...), withMounts(p.Mounts), withPortMappings(p.PortMappings)); err != nil { + if err := createContainer(p.OCIPrefix, p.OCIBinary, p.Image, withRunArgs(runArgs...), withMounts(p.Mounts), withPortMappings(p.PortMappings)); err != nil { return errors.Wrap(err, "create container") } checkRunning := func() error { - s, err := ContainerStatus(p.OCIBinary, p.Name) + s, err := ContainerStatus(p.OCIPrefix, p.OCIBinary, p.Name) if err != nil { return fmt.Errorf("temporary error checking status for %q : %v", p.Name, err) } @@ -188,7 +188,7 @@ func CreateContainerNode(p CreateParams) error { } // CreateContainer creates a container with "docker/podman run" -func createContainer(ociBinary string, image string, opts ...createOpt) error { +func createContainer(prefix string, ociBin string, image string, opts ...createOpt) error { o := &createOpts{} for _, opt := range opts { o = opt(o) @@ -202,10 +202,10 @@ func createContainer(ociBinary string, image string, opts ...createOpt) error { runArgs = append(runArgs, generatePortMappings(portMapping)...) } // construct the actual docker run argv - args := []string{"run"} + args := []string{ociBin, "run"} // to run nested container from privileged container in podman https://bugzilla.redhat.com/show_bug.cgi?id=1687713 - if ociBinary == Podman { + if ociBin == Podman { args = append(args, "--cgroup-manager", "cgroupfs") } @@ -213,7 +213,7 @@ func createContainer(ociBinary string, image string, opts ...createOpt) error { args = append(args, image) args = append(args, o.ContainerArgs...) - out, err := exec.Command(ociBinary, args...).CombinedOutput() + out, err := exec.Command(prefix, args...).CombinedOutput() if err != nil { return errors.Wrapf(err, "failed args: %v output: %s", args, out) } @@ -222,13 +222,13 @@ func createContainer(ociBinary string, image string, opts ...createOpt) error { } // Copy copies a local asset into the container -func Copy(ociBinary string, ociID string, targetDir string, fName string) error { +func Copy(prefix string, ociBin string, ociID string, targetDir string, fName string) error { if _, err := os.Stat(fName); os.IsNotExist(err) { return errors.Wrapf(err, "error source %s does not exist", fName) } destination := fmt.Sprintf("%s:%s", ociID, targetDir) - cmd := exec.Command(ociBinary, "cp", fName, destination) + cmd := exec.Command(prefix, ociBin, "cp", fName, destination) if err := cmd.Run(); err != nil { return errors.Wrapf(err, "error copying %s into node", fName) } @@ -237,8 +237,8 @@ func Copy(ociBinary string, ociID string, targetDir string, fName string) error } // ContainerID returns id of a container name -func ContainerID(ociBinary string, nameOrID string) (string, error) { - cmd := exec.Command(ociBinary, "inspect", "-f", "{{.Id}}", nameOrID) +func ContainerID(prefix string, ociBin string, nameOrID string) (string, error) { + cmd := exec.Command(prefix, ociBin, "inspect", "-f", "{{.Id}}", nameOrID) out, err := cmd.CombinedOutput() if err != nil { // don't return error if not found, only return empty string @@ -287,8 +287,8 @@ func WarnIfSlow(args ...string) ([]byte, error) { } // ContainerExists checks if container name exists (either running or exited) -func ContainerExists(ociBin string, name string) (bool, error) { - out, err := WarnIfSlow(ociBin, "ps", "-a", "--format", "{{.Names}}") +func ContainerExists(prefix string, ociBin string, name string) (bool, error) { + out, err := WarnIfSlow(prefix, ociBin, "ps", "-a", "--format", "{{.Names}}") if err != nil { return false, errors.Wrapf(err, string(out)) } @@ -305,8 +305,8 @@ func ContainerExists(ociBin string, name string) (bool, error) { // 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 { - cmd := exec.Command(ociBinary, "inspect", nameOrID, "--format", "{{.Config.Labels}}") +func IsCreatedByMinikube(prefix string, ociBin string, nameOrID string) bool { + cmd := exec.Command(prefix, ociBin, "inspect", nameOrID, "--format", "{{.Config.Labels}}") out, err := cmd.CombinedOutput() if err != nil { @@ -321,14 +321,14 @@ func IsCreatedByMinikube(ociBinary string, nameOrID string) bool { } // 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, ProfileLabelKey) +func ListOwnedContainers(prefix string, ociBin string) ([]string, error) { + return listContainersByLabel(prefix, ociBin, ProfileLabelKey) } // inspect return low-level information on containers -func inspect(ociBinary string, containerNameOrID, format string) ([]string, error) { +func inspect(prefix string, ociBin string, containerNameOrID, format string) ([]string, error) { - cmd := exec.Command(ociBinary, "inspect", + cmd := exec.Command(prefix, ociBin, "inspect", "-f", format, containerNameOrID) // ... against the "node" container var buff bytes.Buffer @@ -390,8 +390,8 @@ func generateMountBindings(mounts ...Mount) []string { } // isUsernsRemapEnabled checks if userns-remap is enabled in docker -func isUsernsRemapEnabled(ociBinary string) bool { - cmd := exec.Command(ociBinary, "info", "--format", "'{{json .SecurityOptions}}'") +func isUsernsRemapEnabled(prefix string, ociBin string) bool { + cmd := exec.Command(prefix, ociBin, "info", "--format", "'{{json .SecurityOptions}}'") var buff bytes.Buffer cmd.Stdout = &buff cmd.Stderr = &buff @@ -453,8 +453,8 @@ func withPortMappings(portMappings []PortMapping) createOpt { } // listContainersByLabel returns all the container names with a specified label -func listContainersByLabel(ociBinary string, label string) ([]string, error) { - stdout, err := WarnIfSlow(ociBinary, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}") +func listContainersByLabel(prefix string, ociBin string, label string) ([]string, error) { + stdout, err := WarnIfSlow(prefix, ociBin, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}") if err != nil { return nil, err } @@ -489,8 +489,8 @@ func PointToHostDockerDaemon() error { } // ContainerStatus returns status of a container running,exited,... -func ContainerStatus(ociBin string, name string) (state.State, error) { - out, err := WarnIfSlow(ociBin, "inspect", name, "--format={{.State.Status}}") +func ContainerStatus(prefix string, ociBin string, name string) (state.State, error) { + out, err := WarnIfSlow(prefix, ociBin, "inspect", name, "--format={{.State.Status}}") o := strings.TrimSpace(string(out)) switch o { case "running": @@ -511,8 +511,8 @@ func ContainerStatus(ociBin string, name string) (state.State, error) { // Shutdown will run command to shut down the container // to ensure the containers process and networking bindings are all closed // to avoid containers getting stuck before delete https://github.com/kubernetes/minikube/issues/7657 -func ShutDown(ociBin string, name string) error { - cmd := exec.Command(ociBin, "exec", "--privileged", "-t", name, "/bin/bash", "-c", "sudo init 0") +func ShutDown(prefix string, ociBin string, name string) error { + cmd := exec.Command(prefix, ociBin, "exec", "--privileged", "-t", name, "/bin/bash", "-c", "sudo init 0") if out, err := cmd.CombinedOutput(); err != nil { glog.Infof("error shutdown %s output %q : %v", name, out, err) } @@ -520,7 +520,7 @@ func ShutDown(ociBin string, name string) error { time.Sleep(time.Second * 1) // wait till it is stoped stopped := func() error { - st, err := ContainerStatus(ociBin, name) + st, err := ContainerStatus(prefix, ociBin, name) if st == state.Stopped { glog.Infof("container %s status is %s", name, st) return nil diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 8c43ce7d8b1c..1a13e1708e73 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -47,6 +47,7 @@ type CreateParams struct { Memory string // memory (mbs) to assign to the container Envs map[string]string // environment variables to pass to the container ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... + OCIPrefix string // env or sudo OCIBinary string // docker or podman } diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index 2fbd9f32b057..9babe0196b25 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -29,18 +29,18 @@ import ( // DeleteAllVolumesByLabel deletes all volumes that have a specific label // if there is no volume to delete it will return nil -func DeleteAllVolumesByLabel(ociBin string, label string) []error { +func DeleteAllVolumesByLabel(prefix string, ociBin string, label string) []error { var deleteErrs []error glog.Infof("trying to delete all %s volumes with label %s", ociBin, label) - vs, err := allVolumesByLabel(ociBin, label) + vs, err := allVolumesByLabel(prefix, ociBin, label) if err != nil { return []error{fmt.Errorf("listing volumes by label %q: %v", label, err)} } for _, v := range vs { - if _, err := WarnIfSlow(ociBin, "volume", "rm", "--force", v); err != nil { + if _, err := WarnIfSlow(prefix, ociBin, "volume", "rm", "--force", v); err != nil { deleteErrs = append(deleteErrs, fmt.Errorf("deleting %q", v)) } } @@ -51,11 +51,11 @@ func DeleteAllVolumesByLabel(ociBin string, label string) []error { // 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 { +func PruneAllVolumesByLabel(prefix string, ociBin string, label string) []error { var deleteErrs []error glog.Infof("trying to prune all %s volumes with label %s", ociBin, label) - if _, err := WarnIfSlow(ociBin, "volume", "prune", "-f", "--filter", "label="+label); err != nil { + if _, err := WarnIfSlow(prefix, ociBin, "volume", "prune", "-f", "--filter", "label="+label); err != nil { deleteErrs = append(deleteErrs, errors.Wrapf(err, "prune volume by label %s", label)) } @@ -64,8 +64,8 @@ func PruneAllVolumesByLabel(ociBin string, label string) []error { // 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}}") +func allVolumesByLabel(prefix string, ociBin string, label string) ([]string, error) { + cmd := exec.Command(prefix, ociBin, "volume", "ls", "--filter", "label="+label, "--format", "{{.Name}}") stdout, err := cmd.Output() s := bufio.NewScanner(bytes.NewReader(stdout)) var vols []string diff --git a/pkg/drivers/kic/types.go b/pkg/drivers/kic/types.go index 2b01019b2ed8..3d6826937833 100644 --- a/pkg/drivers/kic/types.go +++ b/pkg/drivers/kic/types.go @@ -49,6 +49,7 @@ type Config struct { CPU int // Number of CPU cores assigned to the container Memory int // max memory in MB StorePath string // libmachine store path + OCIPrefix string // prefix to use (env, sudo, ...) OCIBinary string // oci tool to use (docker, podman,...) ImageDigest string // image name with sha to use for the node Mounts []oci.Mount // mounts diff --git a/pkg/minikube/command/kic_runner.go b/pkg/minikube/command/kic_runner.go index 018fe3bfae83..4b5a2233719b 100644 --- a/pkg/minikube/command/kic_runner.go +++ b/pkg/minikube/command/kic_runner.go @@ -38,19 +38,22 @@ import ( // It implements the CommandRunner interface. type kicRunner struct { nameOrID string + prefix string ociBin string } // NewKICRunner returns a kicRunner implementor of runner which runs cmds inside a container -func NewKICRunner(containerNameOrID string, oci string) Runner { +func NewKICRunner(containerNameOrID string, prefix string, oci string) Runner { return &kicRunner{ nameOrID: containerNameOrID, - ociBin: oci, // docker or podman + prefix: prefix, // env or sudo + ociBin: oci, // docker or podman } } func (k *kicRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) { args := []string{ + k.ociBin, "exec", // run with privileges so we can remount etc.. "--privileged", @@ -81,7 +84,7 @@ func (k *kicRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) { args, cmd.Args..., ) - oc := exec.Command(k.ociBin, args...) + oc := exec.Command(k.prefix, args...) oc.Stdin = cmd.Stdin oc.Stdout = cmd.Stdout oc.Stderr = cmd.Stderr @@ -199,7 +202,7 @@ func (k *kicRunner) chmod(dst string, perm string) error { // Podman cp command doesn't match docker and doesn't have -a func copyToPodman(src string, dest string) error { - if out, err := exec.Command(oci.Podman, "cp", src, dest).CombinedOutput(); err != nil { + if out, err := exec.Command("sudo", oci.Podman, "cp", src, dest).CombinedOutput(); err != nil { return errors.Wrapf(err, "podman copy %s into %s, output: %s", src, dest, string(out)) } return nil diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go index 8345341c5107..404800b6708b 100644 --- a/pkg/minikube/config/profile.go +++ b/pkg/minikube/config/profile.go @@ -203,7 +203,7 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, return nil, nil, err } // try to get profiles list based on all contrainers created by docker driver - cs, err := oci.ListOwnedContainers(oci.Docker) + cs, err := oci.ListOwnedContainers("env", oci.Docker) if err == nil { pDirs = append(pDirs, cs...) } diff --git a/pkg/minikube/driver/driver.go b/pkg/minikube/driver/driver.go index a55f7de4412a..1990f39733b7 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -126,7 +126,7 @@ func BareMetal(name string) bool { // NeedsRoot returns true if driver needs to run with root privileges func NeedsRoot(name string) bool { - return name == None || name == Podman + return name == None } // NeedsPortForward returns true if driver is unable provide direct IP connectivity diff --git a/pkg/minikube/machine/delete.go b/pkg/minikube/machine/delete.go index 8fc568e43ad8..2e4fdb473c00 100644 --- a/pkg/minikube/machine/delete.go +++ b/pkg/minikube/machine/delete.go @@ -35,12 +35,12 @@ import ( // 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) { +func deleteOrphanedKIC(prefix string, ociBin string, name string) { if !(ociBin == oci.Podman || ociBin == oci.Docker) { return } - _, err := oci.ContainerStatus(ociBin, name) + _, err := oci.ContainerStatus(prefix, 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 @@ -49,10 +49,10 @@ func deleteOrphanedKIC(ociBin string, name string) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := oci.ShutDown(ociBin, name); err != nil { + if err := oci.ShutDown(prefix, ociBin, name); err != nil { glog.Infof("couldn't shut down %s (might be okay): %v ", name, err) } - cmd := exec.CommandContext(ctx, ociBin, "rm", "-f", "-v", name) + cmd := exec.CommandContext(ctx, prefix, ociBin, "rm", "-f", "-v", name) err = cmd.Run() if err == nil { glog.Infof("Found stale kic container and successfully cleaned it up!") @@ -63,8 +63,8 @@ func deleteOrphanedKIC(ociBin string, name string) { func DeleteHost(api libmachine.API, machineName string) error { host, err := api.Load(machineName) if err != nil && host == nil { - deleteOrphanedKIC(oci.Docker, machineName) - deleteOrphanedKIC(oci.Podman, machineName) + deleteOrphanedKIC("env", oci.Docker, machineName) + deleteOrphanedKIC("sudo", oci.Podman, machineName) // Keep going even if minikube does not know about the host } diff --git a/pkg/minikube/machine/stop.go b/pkg/minikube/machine/stop.go index d5cac2b2d3fe..f8d5fa3cf3bb 100644 --- a/pkg/minikube/machine/stop.go +++ b/pkg/minikube/machine/stop.go @@ -80,7 +80,7 @@ func trySSHPowerOff(h *host.Host) error { out.T(out.Shutdown, `Powering off "{{.profile_name}}" via SSH ...`, out.V{"profile_name": h.Name}) // differnet for kic because RunSSHCommand is not implemented by kic if driver.IsKIC(h.DriverName) { - err := oci.ShutDown(h.DriverName, h.Name) + err := oci.ShutDown("sudo", h.DriverName, h.Name) glog.Infof("shutdown container: err=%v", err) } else { out, err := h.RunSSHCommand("sudo poweroff") diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index 88a190897ad5..ee651b487dbc 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -47,7 +47,7 @@ func init() { if err := registry.Register(registry.DriverDef{ Name: driver.Docker, Config: configure, - Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIBinary: oci.Docker}) }, + Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: "env", OCIBinary: oci.Docker}) }, Status: status, Priority: priority, }); err != nil { @@ -62,6 +62,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { ImageDigest: kic.BaseImage, CPU: cc.CPUs, Memory: cc.Memory, + OCIPrefix: "env", OCIBinary: oci.Docker, APIServerPort: cc.Nodes[0].Port, KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index 468b1814b8a6..2d199c75234b 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -41,7 +41,7 @@ func init() { if err := registry.Register(registry.DriverDef{ Name: driver.Podman, Config: configure, - Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIBinary: oci.Podman}) }, + Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: "sudo", OCIBinary: oci.Podman}) }, Status: status, Priority: registry.Experimental, }); err != nil { @@ -56,6 +56,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { ImageDigest: strings.Split(kic.BaseImage, "@")[0], // for podman does not support docker images references with both a tag and digest. CPU: cc.CPUs, Memory: cc.Memory, + OCIPrefix: "sudo", OCIBinary: oci.Podman, APIServerPort: cc.Nodes[0].Port, }), nil @@ -71,7 +72,7 @@ func status() registry.State { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - cmd := exec.CommandContext(ctx, oci.Podman, "version", "-f", "{{.Version}}") + cmd := exec.CommandContext(ctx, "sudo", oci.Podman, "version", "-f", "{{.Version}}") o, err := cmd.CombinedOutput() output := string(o) if err != nil { @@ -89,7 +90,7 @@ func status() registry.State { // Allow no more than 3 seconds for querying state ctx, cancel = context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - err = exec.CommandContext(ctx, oci.Podman, "info").Run() + err = exec.CommandContext(ctx, "sudo", oci.Podman, "info").Run() if err != nil { return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Podman is not running or taking too long to respond. Try: restarting podman."} } From f264ac171e7b12334c550067202839e9e739f907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 10:35:34 +0200 Subject: [PATCH 02/26] Remove extra single-quotes from format parameter podman Error: template parsing error: template: image:1: function "json" not defined --- pkg/drivers/kic/oci/info.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/info.go b/pkg/drivers/kic/oci/info.go index 73a944d7bbb5..b0fcbc981ee8 100644 --- a/pkg/drivers/kic/oci/info.go +++ b/pkg/drivers/kic/oci/info.go @@ -234,7 +234,7 @@ func dockerSystemInfo() (dockerSysInfo, error) { // podmanSysInfo returns podman system info --format '{{json .}}' func podmanSystemInfo() (podmanSysInfo, error) { var ps podmanSysInfo - cmd := exec.Command("sudo", Podman, "system", "info", "--format", "'{{json .}}'") + cmd := exec.Command("sudo", Podman, "system", "info", "--format", "{{json .}}") out, err := cmd.CombinedOutput() if err != nil { return ps, errors.Wrap(err, "get podman system info") From 401e94c854bee74330d0b479c7bee9072f368c3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 10:39:25 +0200 Subject: [PATCH 03/26] Don't download podman images to docker daemon --- pkg/minikube/node/cache.go | 20 +++++++++++++------- pkg/minikube/node/start.go | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index dd01433ca8ab..96176b6abd71 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -99,14 +99,20 @@ func doCacheBinaries(k8sVersion string) error { } // BeginDownloadKicArtifacts downloads the kic image + preload tarball, returns true if preload is available -func beginDownloadKicArtifacts(g *errgroup.Group) { +func beginDownloadKicArtifacts(g *errgroup.Group, driver string, cRuntime string) { glog.Info("Beginning downloading kic artifacts") - if !image.ExistsImageInDaemon(kic.BaseImage) { - out.T(out.Pulling, "Pulling base image ...") - g.Go(func() error { - glog.Infof("Downloading %s to local daemon", kic.BaseImage) - return image.WriteImageToDaemon(kic.BaseImage) - }) + glog.Infof("Beginning downloading kic artifacts for %s with %s", driver, cRuntime) + if driver == "docker" { + if !image.ExistsImageInDaemon(kic.BaseImage) { + out.T(out.Pulling, "Pulling base image ...") + g.Go(func() error { + glog.Infof("Downloading %s to local daemon", kic.BaseImage) + return image.WriteImageToDaemon(kic.BaseImage) + }) + } + } else { + // TODO: driver == "podman" + glog.Info("Container runtime isn't docker, skipping download") } } diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 5f2aecb0d96c..6bc46d9c1f82 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -201,7 +201,7 @@ func Provision(cc *config.ClusterConfig, n *config.Node, apiServer bool) (comman } if driver.IsKIC(cc.Driver) { - beginDownloadKicArtifacts(&kicGroup) + beginDownloadKicArtifacts(&kicGroup, cc.Driver, cc.KubernetesConfig.ContainerRuntime) } if !driver.BareMetal(cc.Driver) { From 22aa1aff228a11036110ed1893c487ca4b8c43ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 13:43:45 +0200 Subject: [PATCH 04/26] Handle more state and status for the podman driver --- pkg/drivers/kic/kic.go | 2 ++ pkg/drivers/kic/oci/oci.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index cd7dae2803a3..89ff735e8616 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -244,6 +244,8 @@ func (d *Driver) GetState() (state.State, error) { o := strings.TrimSpace(string(out)) switch o { + case "configured": + return state.Stopped, nil case "running": return state.Running, nil case "exited": diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index cf3ebe15c2b7..0edd983af8f0 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -36,6 +36,7 @@ import ( "fmt" "os/exec" "strings" + "strconv" ) // DeleteContainersByLabel deletes all containers that have a specific label @@ -168,6 +169,13 @@ func CreateContainerNode(p CreateParams) error { } checkRunning := func() error { + r, err := ContainerRunning(p.OCIPrefix, p.OCIBinary, p.Name) + if err != nil { + return fmt.Errorf("temporary error checking running for %q : %v", p.Name, err) + } + if !r { + return fmt.Errorf("temporary error created container %q is not running yet", p.Name) + } s, err := ContainerStatus(p.OCIPrefix, p.OCIBinary, p.Name) if err != nil { return fmt.Errorf("temporary error checking status for %q : %v", p.Name, err) @@ -488,6 +496,15 @@ func PointToHostDockerDaemon() error { return nil } +// ContainerRunning returns running state of a container +func ContainerRunning(prefix string, ociBin string, name string) (bool, error) { + out, err := WarnIfSlow(prefix, ociBin, "inspect", name, "--format={{.State.Running}}") + if err != nil { + return false, err + } + return strconv.ParseBool(strings.TrimSpace(string(out))) +} + // ContainerStatus returns status of a container running,exited,... func ContainerStatus(prefix string, ociBin string, name string) (state.State, error) { out, err := WarnIfSlow(prefix, ociBin, "inspect", name, "--format={{.State.Status}}") From f57faf8036c8ecbcb93a6621324e2b547b794d40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 16:04:43 +0200 Subject: [PATCH 05/26] Only remount the /var/lib/minikube directory --- pkg/drivers/kic/oci/oci.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 0edd983af8f0..eeb491e87327 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -137,12 +137,12 @@ func CreateContainerNode(p CreateParams) error { 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") + hostVarVolPath := filepath.Join(localpath.MiniPath(), "machines", p.Name, "var", "lib", "minikube") 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)) + runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var/lib/minikube:exec", hostVarVolPath)) } if p.OCIBinary == Docker { runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", p.Name)) From df3aec6b0a494e72351f2e9807b9594a1e45606a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 16:06:12 +0200 Subject: [PATCH 06/26] Missed to wrap one podman inspect in sudo proper --- pkg/drivers/kic/oci/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index 600e77ab9556..cdb9ce091b2c 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -124,7 +124,7 @@ func ContainerIPs(ociBinary string, name string) (string, string, error) { // podmanConttainerIP returns ipv4, ipv6 of container or error func podmanConttainerIP(name string) (string, string, error) { - cmd := exec.Command(Podman, "inspect", + cmd := exec.Command("sudo", Podman, "inspect", "-f", "{{.NetworkSettings.IPAddress}}", name) out, err := cmd.CombinedOutput() From 223424652cbce7d638b722e09a32520c9873d721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 16:49:07 +0200 Subject: [PATCH 07/26] Also create and mount the container runtime dirs --- pkg/drivers/kic/oci/oci.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index eeb491e87327..20397cbe57f1 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -35,8 +35,8 @@ import ( "fmt" "os/exec" - "strings" "strconv" + "strings" ) // DeleteContainersByLabel deletes all containers that have a specific label @@ -137,12 +137,24 @@ func CreateContainerNode(p CreateParams) error { 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", "lib", "minikube") - if err := os.MkdirAll(hostVarVolPath, 0711); err != nil { + hostVarVolPath := filepath.Join(localpath.MiniPath(), "machines", p.Name, "var", "lib") + if err := os.MkdirAll(hostVarVolPath, 0755); err != nil { return errors.Wrapf(err, "create var dir %s", hostVarVolPath) } + if err := os.Mkdir(fmt.Sprintf("%s/%s", hostVarVolPath, "minikube"), 0711); err != nil { + return errors.Wrapf(err, "create var dir %s/%s", hostVarVolPath, "minikube") + } + hostVarLibSubdirs := []string{"docker", "containerd", "containers"} + for _, subdir := range hostVarLibSubdirs { + if err := os.Mkdir(fmt.Sprintf("%s/%s", hostVarVolPath, subdir), 0711); err != nil { + return errors.Wrapf(err, "create var dir %s/%s", hostVarVolPath, subdir) + } + } // podman mounts var/lib with no-exec by default https://github.com/containers/libpod/issues/5103 - runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var/lib/minikube:exec", hostVarVolPath)) + runArgs = append(runArgs, "--volume", fmt.Sprintf("%s/minikube:/var/lib/minikube:exec", hostVarVolPath)) + for _, subdir := range hostVarLibSubdirs { + runArgs = append(runArgs, "--volume", fmt.Sprintf("%s/%s:/var/lib/%s", hostVarVolPath, subdir, subdir)) + } } if p.OCIBinary == Docker { runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", p.Name)) From 8987b1067acf2a6b5c8915de149723e67b8540bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 17:33:48 +0200 Subject: [PATCH 08/26] Also persist the kubelet and the cni state dirs --- 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 20397cbe57f1..55a4a24f3108 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -144,7 +144,7 @@ func CreateContainerNode(p CreateParams) error { if err := os.Mkdir(fmt.Sprintf("%s/%s", hostVarVolPath, "minikube"), 0711); err != nil { return errors.Wrapf(err, "create var dir %s/%s", hostVarVolPath, "minikube") } - hostVarLibSubdirs := []string{"docker", "containerd", "containers"} + hostVarLibSubdirs := []string{"docker", "containerd", "containers", "kubelet", "cni"} for _, subdir := range hostVarLibSubdirs { if err := os.Mkdir(fmt.Sprintf("%s/%s", hostVarVolPath, subdir), 0711); err != nil { return errors.Wrapf(err, "create var dir %s/%s", hostVarVolPath, subdir) From 45ec38d2e63ee7b576e32e5addc5fb962a6c46c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 13 Apr 2020 18:07:45 +0200 Subject: [PATCH 09/26] Fix hack binary, as pointed out by golangci-lint --- hack/preload-images/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/preload-images/generate.go b/hack/preload-images/generate.go index 7475901fae50..b915a2a9b3e2 100644 --- a/hack/preload-images/generate.go +++ b/hack/preload-images/generate.go @@ -87,7 +87,7 @@ func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string kcfg := config.KubernetesConfig{ KubernetesVersion: kubernetesVersion, } - runner := command.NewKICRunner(profile, driver.OCIBinary) + runner := command.NewKICRunner(profile, driver.OCIPrefix, driver.OCIBinary) sm := sysinit.New(runner) if err := bsutil.TransferBinaries(kcfg, runner, sm); err != nil { From 024cd6b108ed35aa17cee599bb0c1cc353a718bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Tue, 14 Apr 2020 21:39:00 +0200 Subject: [PATCH 10/26] Use the same kind of named /var mount for podman The internal anonymous mounts do not seem to have the same issues as the external path mounts, so we can do all of /var --- pkg/drivers/kic/oci/oci.go | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 55a4a24f3108..78714ba0eaa7 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -19,7 +19,6 @@ package oci import ( "context" "os" - "path/filepath" "time" "bufio" @@ -29,7 +28,6 @@ import ( "github.com/golang/glog" "github.com/pkg/errors" "k8s.io/minikube/pkg/minikube/constants" - "k8s.io/minikube/pkg/minikube/localpath" "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/util/retry" @@ -136,25 +134,8 @@ func CreateContainerNode(p CreateParams) error { } 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", "lib") - if err := os.MkdirAll(hostVarVolPath, 0755); err != nil { - return errors.Wrapf(err, "create var dir %s", hostVarVolPath) - } - if err := os.Mkdir(fmt.Sprintf("%s/%s", hostVarVolPath, "minikube"), 0711); err != nil { - return errors.Wrapf(err, "create var dir %s/%s", hostVarVolPath, "minikube") - } - hostVarLibSubdirs := []string{"docker", "containerd", "containers", "kubelet", "cni"} - for _, subdir := range hostVarLibSubdirs { - if err := os.Mkdir(fmt.Sprintf("%s/%s", hostVarVolPath, subdir), 0711); err != nil { - return errors.Wrapf(err, "create var dir %s/%s", hostVarVolPath, subdir) - } - } // podman mounts var/lib with no-exec by default https://github.com/containers/libpod/issues/5103 - runArgs = append(runArgs, "--volume", fmt.Sprintf("%s/minikube:/var/lib/minikube:exec", hostVarVolPath)) - for _, subdir := range hostVarLibSubdirs { - runArgs = append(runArgs, "--volume", fmt.Sprintf("%s/%s:/var/lib/%s", hostVarVolPath, subdir, subdir)) - } + runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var:exec", p.Name)) } if p.OCIBinary == Docker { runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", p.Name)) From 19be5619d6b5a0fe02d02c76a0a4f76958244088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Tue, 14 Apr 2020 21:40:20 +0200 Subject: [PATCH 11/26] Resource limits seem to work ok when not rootless --- pkg/drivers/kic/oci/oci.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 78714ba0eaa7..60c799749805 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -139,11 +139,10 @@ func CreateContainerNode(p CreateParams) error { } if p.OCIBinary == Docker { 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)) } + runArgs = append(runArgs, fmt.Sprintf("--cpus=%s", p.CPUs), fmt.Sprintf("--memory=%s", p.Memory)) + for key, val := range p.Envs { runArgs = append(runArgs, "-e", fmt.Sprintf("%s=%s", key, val)) } From e99340b4ac8daa800ae9955a377afd4c44cba16a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 18 Apr 2020 18:19:37 +0200 Subject: [PATCH 12/26] Improve podman status when checking for sudo Only use sudo for info and running on linux --- pkg/minikube/registry/drvs/podman/podman.go | 46 +++++++++++++++++---- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index 2d199c75234b..d7034c9df09c 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -19,7 +19,10 @@ package podman import ( "context" "fmt" + "os" "os/exec" + "os/user" + "runtime" "strings" "time" @@ -63,16 +66,17 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { } func status() registry.State { - _, err := exec.LookPath(oci.Podman) + docURL := "https://minikube.sigs.k8s.io/docs/drivers/podman/" + podman, err := exec.LookPath(oci.Podman) if err != nil { - return registry.State{Error: err, Installed: false, Healthy: false, Fix: "Podman is required.", Doc: "https://minikube.sigs.k8s.io/docs/reference/drivers/podman/"} + return registry.State{Error: err, Installed: false, Healthy: false, Fix: "Install Podman", Doc: docURL} } // Allow no more than 2 seconds for version command ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - cmd := exec.CommandContext(ctx, "sudo", oci.Podman, "version", "-f", "{{.Version}}") + cmd := exec.CommandContext(ctx, oci.Podman, "version", "-f", "{{.Version}}") o, err := cmd.CombinedOutput() output := string(o) if err != nil { @@ -87,13 +91,41 @@ func status() registry.State { if v.LT(minReqPodmanVer) { glog.Warningf("Warning ! mininim required version for podman is %s. your version is %q. minikube might not work. use at your own risk. To install latest version please see https://podman.io/getting-started/installation.html ", minReqPodmanVer.String(), v.String()) } + // Allow no more than 3 seconds for querying state ctx, cancel = context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - err = exec.CommandContext(ctx, "sudo", oci.Podman, "info").Run() - if err != nil { - return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Podman is not running or taking too long to respond. Try: restarting podman."} + + // Run with sudo on linux (local), otherwise podman-remote (as podman) + if runtime.GOOS == "linux" { + cmd = exec.CommandContext(ctx, "sudo", "-n", oci.Podman, "info") + cmd.Env = append(os.Environ(), "LANG=C", "LC_ALL=C") // sudo is localized + } else { + cmd = exec.CommandContext(ctx, oci.Podman, "info") + } + _, err = cmd.Output() + if err == nil { + return registry.State{Installed: true, Healthy: true} + } + + glog.Warningf("podman returned error: %v", err) + + if exitErr, ok := err.(*exec.ExitError); ok { + stderr := strings.TrimSpace(string(exitErr.Stderr)) + newErr := fmt.Errorf(`%q %v: %s`, strings.Join(cmd.Args, " "), exitErr, stderr) + + username := "$USER" + if u, err := user.Current(); err == nil { + username = u.Username + } + + if strings.Contains(stderr, "a password is required") && runtime.GOOS == "linux" { + return registry.State{Error: newErr, Installed: true, Healthy: false, Fix: fmt.Sprintf("Add your user to the 'sudoers' file: '%s ALL=(ALL) NOPASSWD: %s'", username, podman), Doc: "https://podman.io"} + } + + // We don't have good advice, but at least we can provide a good error message + return registry.State{Error: newErr, Installed: true, Healthy: false, Doc: docURL} } - return registry.State{Installed: true, Healthy: true} + return registry.State{Error: err, Installed: true, Healthy: false, Doc: docURL} } From bb37c7098ca9d3f047e4f967d4259e6bcb581b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 25 Apr 2020 21:56:40 +0200 Subject: [PATCH 13/26] Fix log message to say driver and not cruntime --- pkg/minikube/node/cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index 282ceeb1d6b6..1b032f320d2b 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -112,7 +112,7 @@ func beginDownloadKicArtifacts(g *errgroup.Group, driver string, cRuntime string } } else { // TODO: driver == "podman" - glog.Info("Container runtime isn't docker, skipping download") + glog.Info("Driver isn't docker, skipping base-image download") } } From 7647b1f998b36401a396acceddd7c147fb736f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 25 Apr 2020 22:01:51 +0200 Subject: [PATCH 14/26] Don't try to limit podman memory without cgroup Podman requires both memcg and memcg_swap for it (--memory). Docker will still limit memory, without swap limit support. --- pkg/drivers/kic/oci/oci.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 64da623425c2..79bb2e9976f8 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -31,6 +31,7 @@ import ( "fmt" "os/exec" + "runtime" "strconv" "strings" ) @@ -139,7 +140,23 @@ func CreateContainerNode(p CreateParams) error { runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", p.Name)) } - runArgs = append(runArgs, fmt.Sprintf("--cpus=%s", p.CPUs), fmt.Sprintf("--memory=%s", p.Memory)) + runArgs = append(runArgs, fmt.Sprintf("--cpus=%s", p.CPUs)) + + memcgSwap := true + if runtime.GOOS == "linux" { + if _, err := os.Stat("/sys/fs/cgroup/memory/memsw.limit_in_bytes"); os.IsNotExist(err) { + // requires CONFIG_MEMCG_SWAP_ENABLED or cgroup_enable=memory in grub + glog.Warning("Your kernel does not support swap limit capabilities or the cgroup is not mounted.") + memcgSwap = false + } + } + + if p.OCIBinary == Podman && memcgSwap { // swap is required for memory + runArgs = append(runArgs, fmt.Sprintf("--memory=%s", p.Memory)) + } + if p.OCIBinary == Docker { // swap is only required for --memory-swap + runArgs = append(runArgs, fmt.Sprintf("--memory=%s", p.Memory)) + } for key, val := range p.Envs { runArgs = append(runArgs, "-e", fmt.Sprintf("%s=%s", key, val)) From 7185140c57c4212a9d61747d5bf4e2f9f377092d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 25 Apr 2020 22:09:52 +0200 Subject: [PATCH 15/26] Remove extra quotes added in conflict resolution Reverted f264ac171e7b12334c550067202839e9e739f907 by accident --- pkg/drivers/kic/oci/info.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/info.go b/pkg/drivers/kic/oci/info.go index 633c154804ea..41dd09ef56cf 100644 --- a/pkg/drivers/kic/oci/info.go +++ b/pkg/drivers/kic/oci/info.go @@ -231,7 +231,7 @@ func dockerSystemInfo() (dockerSysInfo, error) { // podmanSysInfo returns podman system info --format '{{json .}}' func podmanSystemInfo() (podmanSysInfo, error) { var ps podmanSysInfo - rr, err := runCmd(exec.Command("sudo", Podman, "system", "info", "--format", "'{{json .}}'")) + rr, err := runCmd(exec.Command("sudo", Podman, "system", "info", "--format", "{{json .}}")) if err != nil { return ps, errors.Wrap(err, "get podman system info") } From 88c8a24d55501670613e9c472714122246c59ce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 25 Apr 2020 22:43:40 +0200 Subject: [PATCH 16/26] Pass the container implementation on to systemd --- pkg/drivers/kic/oci/oci.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 79bb2e9976f8..7ff83d4737de 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -158,6 +158,16 @@ func CreateContainerNode(p CreateParams) error { runArgs = append(runArgs, fmt.Sprintf("--memory=%s", p.Memory)) } + // https://www.freedesktop.org/wiki/Software/systemd/ContainerInterface/ + var virtualization string + if p.OCIBinary == Podman { + virtualization = "podman" // VIRTUALIZATION_PODMAN + } + if p.OCIBinary == Docker { + virtualization = "docker" // VIRTUALIZATION_DOCKER + } + runArgs = append(runArgs, "-e", fmt.Sprintf("%s=%s", "container", virtualization)) + for key, val := range p.Envs { runArgs = append(runArgs, "-e", fmt.Sprintf("%s=%s", key, val)) } From d96d9d34ed4df2e375a2389003a733cb66803029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 26 Apr 2020 15:12:04 +0200 Subject: [PATCH 17/26] Include any output from podman version in the log --- pkg/minikube/registry/drvs/podman/podman.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index 8f72a4deb02e..6191c8b46ee2 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -82,6 +82,7 @@ func status() registry.State { o, err := cmd.CombinedOutput() output := string(o) if err != nil { + glog.Warningf("podman version returned %s", output) return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Cant verify mininim required version for podman . See podman website for installation guide.", Doc: "https://podman.io/getting-started/installation.html"} } From 28106fa2d71449a85506e7d8b28fa857bfaa078a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 26 Apr 2020 15:22:37 +0200 Subject: [PATCH 18/26] Use constants instead of the KIC prefix strings --- cmd/minikube/cmd/delete.go | 8 ++++---- hack/preload-images/generate.go | 2 +- pkg/drivers/kic/oci/info.go | 2 +- pkg/drivers/kic/oci/network.go | 12 ++++++------ pkg/drivers/kic/oci/types.go | 4 ++++ pkg/minikube/command/kic_runner.go | 2 +- pkg/minikube/config/profile.go | 2 +- pkg/minikube/machine/delete.go | 4 ++-- pkg/minikube/registry/drvs/docker/docker.go | 4 ++-- pkg/minikube/registry/drvs/podman/podman.go | 6 +++--- 10 files changed, 25 insertions(+), 21 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index 052d393f5797..df9aa7930126 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -90,17 +90,17 @@ func init() { func deleteContainersAndVolumes() { delLabel := fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true") - errs := oci.DeleteContainersByLabel("env", oci.Docker, delLabel) + errs := oci.DeleteContainersByLabel(oci.Env, 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, errs) } - errs = oci.DeleteAllVolumesByLabel("env", oci.Docker, delLabel) + errs = oci.DeleteAllVolumesByLabel(oci.Env, oci.Docker, delLabel) 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("env", oci.Docker, delLabel) + errs = oci.PruneAllVolumesByLabel(oci.Env, oci.Docker, delLabel) 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) } @@ -193,7 +193,7 @@ func DeleteProfiles(profiles []*config.Profile) []error { func deletePossibleKicLeftOver(name string) { delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, name) - prefixes := []string{"env", "sudo"} + prefixes := []string{oci.Env, oci.Sudo} for i, bin := range []string{oci.Docker, oci.Podman} { prefix := prefixes[i] cs, err := oci.ListContainersByLabel(prefix, bin, delLabel) diff --git a/hack/preload-images/generate.go b/hack/preload-images/generate.go index 52b9c90af6e7..054f5c5c6090 100644 --- a/hack/preload-images/generate.go +++ b/hack/preload-images/generate.go @@ -41,7 +41,7 @@ func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string driver := kic.NewDriver(kic.Config{ KubernetesVersion: kubernetesVersion, ContainerRuntime: containerRuntime, - OCIPrefix: "env", + OCIPrefix: oci.Env, OCIBinary: oci.Docker, MachineName: profile, ImageDigest: kic.BaseImage, diff --git a/pkg/drivers/kic/oci/info.go b/pkg/drivers/kic/oci/info.go index 41dd09ef56cf..e5835eea6ca6 100644 --- a/pkg/drivers/kic/oci/info.go +++ b/pkg/drivers/kic/oci/info.go @@ -231,7 +231,7 @@ func dockerSystemInfo() (dockerSysInfo, error) { // podmanSysInfo returns podman system info --format '{{json .}}' func podmanSystemInfo() (podmanSysInfo, error) { var ps podmanSysInfo - rr, err := runCmd(exec.Command("sudo", Podman, "system", "info", "--format", "{{json .}}")) + rr, err := runCmd(exec.Command(Sudo, Podman, "system", "info", "--format", "{{json .}}")) if err != nil { return ps, errors.Wrap(err, "get podman system info") } diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index 5dc39b2c08fb..1bb5cdeaaf39 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -56,13 +56,13 @@ func digDNS(ociBin, containerName, dns string) (net.IP, error) { // dockerGatewayIP gets the default gateway ip for the docker bridge on the user's host machine // gets the ip from user's host docker func dockerGatewayIP() (net.IP, error) { - rr, err := runCmd(exec.Command(Docker, "network", "ls", "--filter", "name=bridge", "--format", "{{.ID}}")) + rr, err := runCmd(exec.Command(Env, Docker, "network", "ls", "--filter", "name=bridge", "--format", "{{.ID}}")) if err != nil { return nil, errors.Wrapf(err, "get network bridge") } bridgeID := strings.TrimSpace(rr.Stdout.String()) - rr, err = runCmd(exec.Command("env", Docker, "inspect", + rr, err = runCmd(exec.Command(Env, Docker, "inspect", "--format", "{{(index .IPAM.Config 0).Gateway}}", bridgeID)) if err != nil { return nil, errors.Wrapf(err, "inspect IP bridge network %q.", bridgeID) @@ -84,12 +84,12 @@ func ForwardedPort(ociBin string, ociID string, contPort int) (int, error) { if ociBin == Podman { //podman inspect -f "{{range .NetworkSettings.Ports}}{{if eq .ContainerPort "80"}}{{.HostPort}}{{end}}{{end}}" - rr, err = runCmd(exec.Command("sudo", ociBin, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID)) + rr, err = runCmd(exec.Command(Sudo, ociBin, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID)) if err != nil { return 0, errors.Wrapf(err, "get port %d for %q", contPort, ociID) } } else { - rr, err = runCmd(exec.Command("env", ociBin, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID)) + rr, err = runCmd(exec.Command(Env, ociBin, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID)) if err != nil { return 0, errors.Wrapf(err, "get port %d for %q", contPort, ociID) } @@ -116,7 +116,7 @@ func ContainerIPs(ociBin string, name string) (string, string, error) { // podmanConttainerIP returns ipv4, ipv6 of container or error func podmanConttainerIP(name string) (string, string, error) { - rr, err := runCmd(exec.Command("sudo", Podman, "inspect", + rr, err := runCmd(exec.Command(Sudo, Podman, "inspect", "-f", "{{.NetworkSettings.IPAddress}}", name)) if err != nil { @@ -132,7 +132,7 @@ func podmanConttainerIP(name string) (string, string, error) { // dockerContainerIP returns ipv4, ipv6 of container or error func dockerContainerIP(name string) (string, string, error) { // retrieve the IP address of the node using docker inspect - lines, err := inspect("env", Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}") + lines, err := inspect(Env, Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}") if err != nil { return "", "", errors.Wrap(err, "inspecting NetworkSettings.Networks") } diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 1a13e1708e73..45c38522b98b 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -19,6 +19,10 @@ package oci const ( // DefaultBindIPV4 is The default IP the container will listen on. DefaultBindIPV4 = "127.0.0.1" + // Env is env + Env = "env" + // Sudo is sudo + Sudo = "sudo" // Docker is docker Docker = "docker" // Podman is podman diff --git a/pkg/minikube/command/kic_runner.go b/pkg/minikube/command/kic_runner.go index 4b5a2233719b..98c66e5426c0 100644 --- a/pkg/minikube/command/kic_runner.go +++ b/pkg/minikube/command/kic_runner.go @@ -202,7 +202,7 @@ func (k *kicRunner) chmod(dst string, perm string) error { // Podman cp command doesn't match docker and doesn't have -a func copyToPodman(src string, dest string) error { - if out, err := exec.Command("sudo", oci.Podman, "cp", src, dest).CombinedOutput(); err != nil { + if out, err := exec.Command(oci.Sudo, oci.Podman, "cp", src, dest).CombinedOutput(); err != nil { return errors.Wrapf(err, "podman copy %s into %s, output: %s", src, dest, string(out)) } return nil diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go index 404800b6708b..788d98afea26 100644 --- a/pkg/minikube/config/profile.go +++ b/pkg/minikube/config/profile.go @@ -203,7 +203,7 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, return nil, nil, err } // try to get profiles list based on all contrainers created by docker driver - cs, err := oci.ListOwnedContainers("env", oci.Docker) + cs, err := oci.ListOwnedContainers(oci.Env, oci.Docker) if err == nil { pDirs = append(pDirs, cs...) } diff --git a/pkg/minikube/machine/delete.go b/pkg/minikube/machine/delete.go index f8c4b44ec41c..134c6a74eb6e 100644 --- a/pkg/minikube/machine/delete.go +++ b/pkg/minikube/machine/delete.go @@ -63,8 +63,8 @@ func deleteOrphanedKIC(prefix string, ociBin string, name string) { func DeleteHost(api libmachine.API, machineName string) error { host, err := api.Load(machineName) if err != nil && host == nil { - deleteOrphanedKIC("env", oci.Docker, machineName) - deleteOrphanedKIC("sudo", oci.Podman, machineName) + deleteOrphanedKIC(oci.Env, oci.Docker, machineName) + deleteOrphanedKIC(oci.Sudo, oci.Podman, machineName) // Keep going even if minikube does not know about the host } diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index ab5439a044cc..13a4102e69ff 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -48,7 +48,7 @@ func init() { if err := registry.Register(registry.DriverDef{ Name: driver.Docker, Config: configure, - Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: "env", OCIBinary: oci.Docker}) }, + Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: oci.Env, OCIBinary: oci.Docker}) }, Status: status, Priority: priority, }); err != nil { @@ -63,7 +63,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { ImageDigest: viper.GetString("base-image"), CPU: cc.CPUs, Memory: cc.Memory, - OCIPrefix: "env", + OCIPrefix: oci.Env, OCIBinary: oci.Docker, APIServerPort: cc.Nodes[0].Port, KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index 6191c8b46ee2..79815c0d2e30 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -45,7 +45,7 @@ func init() { if err := registry.Register(registry.DriverDef{ Name: driver.Podman, Config: configure, - Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: "sudo", OCIBinary: oci.Podman}) }, + Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: oci.Sudo, OCIBinary: oci.Podman}) }, Status: status, Priority: registry.Experimental, }); err != nil { @@ -61,7 +61,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { ImageDigest: strings.Split(baseImage, "@")[0], // for podman does not support docker images references with both a tag and digest. CPU: cc.CPUs, Memory: cc.Memory, - OCIPrefix: "sudo", + OCIPrefix: oci.Sudo, OCIBinary: oci.Podman, APIServerPort: cc.Nodes[0].Port, }), nil @@ -101,7 +101,7 @@ func status() registry.State { // Run with sudo on linux (local), otherwise podman-remote (as podman) if runtime.GOOS == "linux" { - cmd = exec.CommandContext(ctx, "sudo", "-n", oci.Podman, "info") + cmd = exec.CommandContext(ctx, oci.Sudo, "-n", oci.Podman, "info") cmd.Env = append(os.Environ(), "LANG=C", "LC_ALL=C") // sudo is localized } else { cmd = exec.CommandContext(ctx, oci.Podman, "info") From e95ae3280c2130d17993a65cbefe46b4ddf4c156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 26 Apr 2020 22:45:38 +0200 Subject: [PATCH 19/26] Fix copy/paste error on the podman usage page --- site/content/en/docs/drivers/includes/podman_usage.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/en/docs/drivers/includes/podman_usage.inc b/site/content/en/docs/drivers/includes/podman_usage.inc index 186c5bb6043e..5472e8faa4d2 100644 --- a/site/content/en/docs/drivers/includes/podman_usage.inc +++ b/site/content/en/docs/drivers/includes/podman_usage.inc @@ -9,7 +9,7 @@ for a better kubernetes in container experience, use docker [driver](https://min ## Usage -Start a cluster using the docker driver: +Start a cluster using the podman driver: ```shell minikube start --driver=podman From 795128131f17e3115d148879ad7f235f97ac680e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 26 Apr 2020 22:52:10 +0200 Subject: [PATCH 20/26] Remove old Copy function added in merge conflict --- pkg/drivers/kic/oci/oci.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 7ff83d4737de..633d8912d315 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -245,20 +245,6 @@ func createContainer(prefix string, ociBin string, image string, opts ...createO return nil } -// Copy copies a local asset into the container -func Copy(prefix string, ociBin string, ociID string, targetDir string, fName string) error { - if _, err := os.Stat(fName); os.IsNotExist(err) { - return errors.Wrapf(err, "error source %s does not exist", fName) - } - - destination := fmt.Sprintf("%s:%s", ociID, targetDir) - if _, err := runCmd(exec.Command(prefix, ociBin, "cp", fName, destination)); err != nil { - return err - } - - return nil -} - // ContainerID returns id of a container name func ContainerID(prefix, ociBin string, nameOrID string) (string, error) { rr, err := runCmd(exec.Command(prefix, ociBin, "inspect", "-f", "{{.Id}}", nameOrID)) From 998ab840fbb139b0f9bdda71aa509b19554fab15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 26 Apr 2020 22:54:45 +0200 Subject: [PATCH 21/26] Use simple version of podman info format parameter So that it works with the older versions of podman --- pkg/drivers/kic/oci/info.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/info.go b/pkg/drivers/kic/oci/info.go index e5835eea6ca6..45752f19594e 100644 --- a/pkg/drivers/kic/oci/info.go +++ b/pkg/drivers/kic/oci/info.go @@ -231,7 +231,7 @@ func dockerSystemInfo() (dockerSysInfo, error) { // podmanSysInfo returns podman system info --format '{{json .}}' func podmanSystemInfo() (podmanSysInfo, error) { var ps podmanSysInfo - rr, err := runCmd(exec.Command(Sudo, Podman, "system", "info", "--format", "{{json .}}")) + rr, err := runCmd(exec.Command(Sudo, Podman, "system", "info", "--format", "json")) if err != nil { return ps, errors.Wrap(err, "get podman system info") } From 95c95599c2b266d23ab6d3487e53195b73f4b8fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 27 Apr 2020 12:41:16 +0200 Subject: [PATCH 22/26] Align podman driver with docker and improve check Make sure to use "sudo podman version" on Linux (need user namespace support for "podman version") And improve error output handling for podman-remote (when the remote service is not running properly) --- pkg/minikube/registry/drvs/podman/podman.go | 91 +++++++++++---------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index 79815c0d2e30..dd54e998d8bb 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -42,12 +42,18 @@ import ( var minReqPodmanVer = semver.Version{Major: 1, Minor: 7, Patch: 0} func init() { + priority := registry.Experimental + // Staged rollout for default: + // - Linux + // - macOS (podman-remote) + // - Windows (podman-remote) + if err := registry.Register(registry.DriverDef{ Name: driver.Podman, Config: configure, Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: oci.Sudo, OCIBinary: oci.Podman}) }, Status: status, - Priority: registry.Experimental, + Priority: priority, }); err != nil { panic(fmt.Sprintf("register failed: %v", err)) } @@ -56,14 +62,16 @@ func init() { func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { baseImage := viper.GetString("base-image") return kic.NewDriver(kic.Config{ - MachineName: driver.MachineName(cc, n), - StorePath: localpath.MiniPath(), - ImageDigest: strings.Split(baseImage, "@")[0], // for podman does not support docker images references with both a tag and digest. - CPU: cc.CPUs, - Memory: cc.Memory, - OCIPrefix: oci.Sudo, - OCIBinary: oci.Podman, - APIServerPort: cc.Nodes[0].Port, + MachineName: driver.MachineName(cc, n), + StorePath: localpath.MiniPath(), + ImageDigest: strings.Split(baseImage, "@")[0], // for podman does not support docker images references with both a tag and digest. + CPU: cc.CPUs, + Memory: cc.Memory, + OCIPrefix: oci.Sudo, + OCIBinary: oci.Podman, + APIServerPort: cc.Nodes[0].Port, + KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, + ContainerRuntime: cc.KubernetesConfig.ContainerRuntime, }), nil } @@ -74,58 +82,59 @@ func status() registry.State { return registry.State{Error: err, Installed: false, Healthy: false, Fix: "Install Podman", Doc: docURL} } - // Allow no more than 2 seconds for version command - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - - cmd := exec.CommandContext(ctx, oci.Podman, "version", "-f", "{{.Version}}") - o, err := cmd.CombinedOutput() - output := string(o) - if err != nil { - glog.Warningf("podman version returned %s", output) - return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Cant verify mininim required version for podman . See podman website for installation guide.", Doc: "https://podman.io/getting-started/installation.html"} - } - - v, err := semver.Make(output) - if err != nil { - return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Cant verify mininim required version for podman . See podman website for installation guide.", Doc: "https://podman.io/getting-started/installation.html"} - } - - if v.LT(minReqPodmanVer) { - glog.Warningf("Warning ! mininim required version for podman is %s. your version is %q. minikube might not work. use at your own risk. To install latest version please see https://podman.io/getting-started/installation.html ", minReqPodmanVer.String(), v.String()) - } - - // Allow no more than 3 seconds for querying state - ctx, cancel = context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second) defer cancel() + // Quickly returns an error code if service is not running + cmd := exec.CommandContext(ctx, oci.Podman, "version", "--format", "{{.Server.Version}}") // Run with sudo on linux (local), otherwise podman-remote (as podman) if runtime.GOOS == "linux" { - cmd = exec.CommandContext(ctx, oci.Sudo, "-n", oci.Podman, "info") + cmd = exec.CommandContext(ctx, oci.Sudo, "-n", oci.Podman, "version", "--format", "{{.Version}}") cmd.Env = append(os.Environ(), "LANG=C", "LC_ALL=C") // sudo is localized - } else { - cmd = exec.CommandContext(ctx, oci.Podman, "info") } - _, err = cmd.Output() + o, err := cmd.Output() + output := string(o) if err == nil { + v, err := semver.Make(output) + if err != nil { + return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Cant verify minimum required version for podman . See podman website for installation guide.", Doc: "https://podman.io/getting-started/installation.html"} + } + + if v.LT(minReqPodmanVer) { + glog.Warningf("Warning ! minimum required version for podman is %s. your version is %q. minikube might not work. use at your own risk. To install latest version please see https://podman.io/getting-started/installation.html ", minReqPodmanVer.String(), v.String()) + } + return registry.State{Installed: true, Healthy: true} } glog.Warningf("podman returned error: %v", err) + // Basic timeout + if ctx.Err() == context.DeadlineExceeded { + return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Restart the Podman service", Doc: docURL} + } + + username := "$USER" + if u, err := user.Current(); err == nil { + username = u.Username + } + if exitErr, ok := err.(*exec.ExitError); ok { stderr := strings.TrimSpace(string(exitErr.Stderr)) newErr := fmt.Errorf(`%q %v: %s`, strings.Join(cmd.Args, " "), exitErr, stderr) - username := "$USER" - if u, err := user.Current(); err == nil { - username = u.Username - } - if strings.Contains(stderr, "a password is required") && runtime.GOOS == "linux" { return registry.State{Error: newErr, Installed: true, Healthy: false, Fix: fmt.Sprintf("Add your user to the 'sudoers' file: '%s ALL=(ALL) NOPASSWD: %s'", username, podman), Doc: "https://podman.io"} } + // Typical low-level errors from running podman-remote: + // - local: "dial unix /run/podman/io.podman: connect: no such file or directory" + // - remote: "unexpected EOF" (ssh varlink isn't so great at handling rejections) + + if strings.Contains(stderr, "could not get runtime") || strings.Contains(stderr, "Unable to obtain server version information") { + return registry.State{Error: newErr, Installed: true, Healthy: false, Fix: "Start the Podman service", Doc: docURL} + } + // We don't have good advice, but at least we can provide a good error message return registry.State{Error: newErr, Installed: true, Healthy: false, Doc: docURL} } From a041f4012ae96150b2b0a2e940df4689aaf88fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 27 Apr 2020 17:02:13 +0200 Subject: [PATCH 23/26] Podman allows setting --cpus and --memory now --- pkg/minikube/driver/driver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/driver/driver.go b/pkg/minikube/driver/driver.go index 1990f39733b7..d4dd5e27623b 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -137,7 +137,7 @@ func NeedsPortForward(name string) bool { // HasResourceLimits returns true if driver can set resource limits such as memory size or CPU count. func HasResourceLimits(name string) bool { - return !(name == None || name == Podman) + return name != None } // NeedsShutdown returns true if driver needs manual shutdown command before stopping. From bf4aa78ca6367aba419a9a935a9124a14d1a367b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 27 Apr 2020 21:58:21 +0200 Subject: [PATCH 24/26] Log the version used of the two KIC runtimes --- pkg/minikube/registry/drvs/docker/docker.go | 4 +++- pkg/minikube/registry/drvs/podman/podman.go | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index 13a4102e69ff..ada73686c023 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -83,8 +83,10 @@ func status() registry.State { // Quickly returns an error code if server is not running cmd := exec.CommandContext(ctx, oci.Docker, "version", "--format", "{{.Server.Version}}") - _, err = cmd.Output() + o, err := cmd.Output() + output := string(o) if err == nil { + glog.Infof("docker version: %s", output) return registry.State{Installed: true, Healthy: true} } diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index dd54e998d8bb..ffde7a19b977 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -95,6 +95,8 @@ func status() registry.State { o, err := cmd.Output() output := string(o) if err == nil { + glog.Infof("podman version: %s", output) + v, err := semver.Make(output) if err != nil { return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Cant verify minimum required version for podman . See podman website for installation guide.", Doc: "https://podman.io/getting-started/installation.html"} From 1744ffe7732954e6e760e91adae6c8d399dfdc36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 27 Apr 2020 22:27:30 +0200 Subject: [PATCH 25/26] Restore the configured state added for podman Reverted 22aa1aff228a11036110ed1893c487ca4b8c43ca by accident --- pkg/drivers/kic/oci/oci.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 633d8912d315..10120ed97cb0 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -470,6 +470,8 @@ func ContainerStatus(prefix string, ociBin string, name string, warnSlow ...bool rr, err := runCmd(cmd, warnSlow...) o := strings.TrimSpace(rr.Stdout.String()) switch o { + case "configured": + return state.Stopped, nil case "running": return state.Running, nil case "exited": From 78a22f5056eb171d480d1d932e67c4cf991b390d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Tue, 28 Apr 2020 12:28:00 +0200 Subject: [PATCH 26/26] Remove prefix parameter and add prefix command Move the "sudo" prefix to a central location, instead of having it all over the place. Assume only needed on Linux. --- cmd/minikube/cmd/delete.go | 18 +++-- hack/preload-images/generate.go | 3 +- pkg/drivers/kic/kic.go | 35 +++++----- pkg/drivers/kic/oci/cli_runner.go | 17 +++++ pkg/drivers/kic/oci/info.go | 2 +- pkg/drivers/kic/oci/network.go | 12 ++-- pkg/drivers/kic/oci/oci.go | 73 +++++++++++---------- pkg/drivers/kic/oci/types.go | 5 -- pkg/drivers/kic/oci/volumes.go | 14 ++-- pkg/drivers/kic/types.go | 1 - pkg/minikube/command/kic_runner.go | 16 ++--- pkg/minikube/config/profile.go | 2 +- pkg/minikube/machine/delete.go | 12 ++-- pkg/minikube/machine/stop.go | 2 +- pkg/minikube/registry/drvs/docker/docker.go | 3 +- pkg/minikube/registry/drvs/podman/podman.go | 5 +- 16 files changed, 112 insertions(+), 108 deletions(-) diff --git a/cmd/minikube/cmd/delete.go b/cmd/minikube/cmd/delete.go index df9aa7930126..c2b05800aad3 100644 --- a/cmd/minikube/cmd/delete.go +++ b/cmd/minikube/cmd/delete.go @@ -90,17 +90,17 @@ func init() { func deleteContainersAndVolumes() { delLabel := fmt.Sprintf("%s=%s", oci.CreatedByLabelKey, "true") - errs := oci.DeleteContainersByLabel(oci.Env, 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, errs) } - errs = oci.DeleteAllVolumesByLabel(oci.Env, oci.Docker, delLabel) + errs = oci.DeleteAllVolumesByLabel(oci.Docker, delLabel) 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.Env, oci.Docker, delLabel) + errs = oci.PruneAllVolumesByLabel(oci.Docker, delLabel) 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) } @@ -193,14 +193,12 @@ func DeleteProfiles(profiles []*config.Profile) []error { func deletePossibleKicLeftOver(name string) { delLabel := fmt.Sprintf("%s=%s", oci.ProfileLabelKey, name) - prefixes := []string{oci.Env, oci.Sudo} - for i, bin := range []string{oci.Docker, oci.Podman} { - prefix := prefixes[i] - cs, err := oci.ListContainersByLabel(prefix, bin, delLabel) + for _, bin := range []string{oci.Docker, oci.Podman} { + cs, err := oci.ListContainersByLabel(bin, delLabel) if err == nil && len(cs) > 0 { for _, c := range cs { out.T(out.DeletingHost, `Deleting container "{{.name}}" ...`, out.V{"name": name}) - err := oci.DeleteContainer(prefix, bin, c) + err := oci.DeleteContainer(bin, c) if err != nil { // it will error if there is no container to delete glog.Errorf("error deleting container %q. you might want to delete that manually :\n%v", name, err) } @@ -208,12 +206,12 @@ func deletePossibleKicLeftOver(name string) { } } - errs := oci.DeleteAllVolumesByLabel(prefix, bin, delLabel) + errs := oci.DeleteAllVolumesByLabel(bin, delLabel) if errs != nil { // it will not error if there is nothing to delete glog.Warningf("error deleting volumes (might be okay).\nTo see the list of volumes run: 'docker volume ls'\n:%v", errs) } - errs = oci.PruneAllVolumesByLabel(prefix, bin, delLabel) + errs = oci.PruneAllVolumesByLabel(bin, delLabel) 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) } diff --git a/hack/preload-images/generate.go b/hack/preload-images/generate.go index d03b38f1a777..54c680bc480c 100644 --- a/hack/preload-images/generate.go +++ b/hack/preload-images/generate.go @@ -41,7 +41,6 @@ func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string driver := kic.NewDriver(kic.Config{ KubernetesVersion: kubernetesVersion, ContainerRuntime: containerRuntime, - OCIPrefix: oci.Env, OCIBinary: oci.Docker, MachineName: profile, ImageDigest: kic.BaseImage, @@ -70,7 +69,7 @@ func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string imgs = append(imgs, kic.OverlayImage) } - runner := command.NewKICRunner(profile, driver.OCIPrefix, driver.OCIBinary) + runner := command.NewKICRunner(profile, driver.OCIBinary) // will need to do this to enable the container run-time service sv, err := util.ParseKubernetesVersion(kubernetesVersion) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index fb9536113a36..1e8e3df9d212 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -48,7 +48,6 @@ type Driver struct { URL string exec command.Runner NodeConfig Config - OCIPrefix string // env, sudo OCIBinary string // docker,podman } @@ -59,9 +58,8 @@ func NewDriver(c Config) *Driver { MachineName: c.MachineName, StorePath: c.StorePath, }, - exec: command.NewKICRunner(c.MachineName, c.OCIPrefix, c.OCIBinary), + exec: command.NewKICRunner(c.MachineName, c.OCIBinary), NodeConfig: c, - OCIPrefix: c.OCIPrefix, OCIBinary: c.OCIBinary, } return d @@ -78,7 +76,6 @@ func (d *Driver) Create() error { Memory: strconv.Itoa(d.NodeConfig.Memory) + "mb", Envs: d.NodeConfig.Envs, ExtraArgs: []string{"--expose", fmt.Sprintf("%d", d.NodeConfig.APIServerPort)}, - OCIPrefix: d.NodeConfig.OCIPrefix, OCIBinary: d.NodeConfig.OCIBinary, APIServerPort: d.NodeConfig.APIServerPort, } @@ -102,15 +99,15 @@ func (d *Driver) Create() error { }, ) - exists, err := oci.ContainerExists(d.OCIPrefix, d.OCIBinary, params.Name, true) + exists, err := oci.ContainerExists(d.OCIBinary, params.Name, true) if err != nil { 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.OCIPrefix, d.OCIBinary, params.Name) { + if oci.IsCreatedByMinikube(d.OCIBinary, params.Name) { glog.Info("Found already existing abandoned minikube container, will try to delete.") - if err := oci.DeleteContainer(d.OCIPrefix, 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: %v", params.Name, params.OCIBinary, err) } } else { @@ -162,7 +159,7 @@ func (d *Driver) prepareSSH() error { return errors.Wrap(err, "generate ssh key") } - cmder := command.NewKICRunner(d.NodeConfig.MachineName, d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary) + cmder := command.NewKICRunner(d.NodeConfig.MachineName, d.NodeConfig.OCIBinary) f, err := assets.NewFileAsset(d.GetSSHKeyPath()+".pub", "/home/docker/.ssh/", "authorized_keys", "0644") if err != nil { return errors.Wrap(err, "create pubkey assetfile ") @@ -237,23 +234,23 @@ 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) { - return oci.ContainerStatus(d.OCIPrefix, d.OCIBinary, d.MachineName, true) + return oci.ContainerStatus(d.OCIBinary, d.MachineName, true) } // Kill stops a host forcefully, including any containers that we are managing. func (d *Driver) Kill() error { // on init this doesn't get filled when called from cmd - d.exec = command.NewKICRunner(d.MachineName, d.OCIPrefix, d.OCIBinary) + d.exec = command.NewKICRunner(d.MachineName, d.OCIBinary) if err := sysinit.New(d.exec).ForceStop("kubelet"); err != nil { glog.Warningf("couldn't force stop kubelet. will continue with kill anyways: %v", err) } - if err := oci.ShutDown(d.OCIPrefix, d.OCIBinary, d.MachineName); err != nil { + if err := oci.ShutDown(d.OCIBinary, d.MachineName); err != nil { glog.Warningf("couldn't shutdown the container, will continue with kill anyways: %v", err) } cr := command.NewExecRunner() // using exec runner for interacting with dameon. - if _, err := cr.RunCmd(exec.Command(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "kill", d.MachineName)); err != nil { + if _, err := cr.RunCmd(oci.PrefixCmd(exec.Command(d.NodeConfig.OCIBinary, "kill", d.MachineName))); err != nil { return errors.Wrapf(err, "killing %q", d.MachineName) } return nil @@ -261,11 +258,11 @@ func (d *Driver) Kill() error { // Remove will delete the Kic Node Container func (d *Driver) Remove() error { - if _, err := oci.ContainerID(d.OCIPrefix, d.OCIBinary, d.MachineName); err != nil { + if _, err := oci.ContainerID(d.OCIBinary, d.MachineName); err != nil { glog.Infof("could not find the container %s to remove it. will try anyways", d.MachineName) } - if err := oci.DeleteContainer(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, d.MachineName); err != nil { + if err := oci.DeleteContainer(d.NodeConfig.OCIBinary, d.MachineName); err != nil { if strings.Contains(err.Error(), "is already in progress") { return errors.Wrap(err, "stuck delete") } @@ -276,7 +273,7 @@ func (d *Driver) Remove() error { } // check there be no container left after delete - if id, err := oci.ContainerID(d.OCIPrefix, d.OCIBinary, d.MachineName); err == nil && id != "" { + if id, err := oci.ContainerID(d.OCIBinary, d.MachineName); err == nil && id != "" { return fmt.Errorf("expected no container ID be found for %q after delete. but got %q", d.MachineName, id) } return nil @@ -304,11 +301,11 @@ func (d *Driver) Restart() error { // Start an already created kic container func (d *Driver) Start() error { cr := command.NewExecRunner() // using exec runner for interacting with docker/podman daemon - if _, err := cr.RunCmd(exec.Command(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "start", d.MachineName)); err != nil { + if _, err := cr.RunCmd(oci.PrefixCmd(exec.Command(d.NodeConfig.OCIBinary, "start", d.MachineName))); err != nil { return errors.Wrap(err, "start") } checkRunning := func() error { - s, err := oci.ContainerStatus(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, d.MachineName) + s, err := oci.ContainerStatus(d.NodeConfig.OCIBinary, d.MachineName) if err != nil { return err } @@ -328,7 +325,7 @@ func (d *Driver) Start() error { // Stop a host gracefully, including any containers that we are managing. func (d *Driver) Stop() error { // on init this doesn't get filled when called from cmd - d.exec = command.NewKICRunner(d.MachineName, d.OCIPrefix, d.OCIBinary) + d.exec = command.NewKICRunner(d.MachineName, d.OCIBinary) // docker does not send right SIG for systemd to know to stop the systemd. // to avoid bind address be taken on an upgrade. more info https://github.com/kubernetes/minikube/issues/7171 if err := sysinit.New(d.exec).Stop("kubelet"); err != nil { @@ -363,7 +360,7 @@ func (d *Driver) Stop() error { glog.Warningf("couldn't stop kube-apiserver proc: %v", err) } - cmd := exec.Command(d.NodeConfig.OCIPrefix, d.NodeConfig.OCIBinary, "stop", d.MachineName) + cmd := exec.Command(d.NodeConfig.OCIBinary, "stop", d.MachineName) if err := cmd.Run(); err != nil { return errors.Wrapf(err, "stopping %s", d.MachineName) } diff --git a/pkg/drivers/kic/oci/cli_runner.go b/pkg/drivers/kic/oci/cli_runner.go index f740580423a8..173b509d27bc 100644 --- a/pkg/drivers/kic/oci/cli_runner.go +++ b/pkg/drivers/kic/oci/cli_runner.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "os/exec" + "runtime" "strings" "time" @@ -63,8 +64,24 @@ func (rr RunResult) Output() string { return sb.String() } +// PrefixCmd adds any needed prefix (such as sudo) to the command +func PrefixCmd(cmd *exec.Cmd) *exec.Cmd { + if cmd.Args[0] == Podman && runtime.GOOS == "linux" { // want sudo when not running podman-remote + cmdWithSudo := exec.Command("sudo", cmd.Args...) + cmdWithSudo.Env = cmd.Env + cmdWithSudo.Dir = cmd.Dir + cmdWithSudo.Stdin = cmd.Stdin + cmdWithSudo.Stdout = cmd.Stdout + cmdWithSudo.Stderr = cmd.Stderr + cmd = cmdWithSudo + } + return cmd +} + // runCmd runs a command exec.Command against docker daemon or podman func runCmd(cmd *exec.Cmd, warnSlow ...bool) (*RunResult, error) { + cmd = PrefixCmd(cmd) + warn := false if len(warnSlow) > 0 { warn = warnSlow[0] diff --git a/pkg/drivers/kic/oci/info.go b/pkg/drivers/kic/oci/info.go index 45752f19594e..4fe381ede52b 100644 --- a/pkg/drivers/kic/oci/info.go +++ b/pkg/drivers/kic/oci/info.go @@ -231,7 +231,7 @@ func dockerSystemInfo() (dockerSysInfo, error) { // podmanSysInfo returns podman system info --format '{{json .}}' func podmanSystemInfo() (podmanSysInfo, error) { var ps podmanSysInfo - rr, err := runCmd(exec.Command(Sudo, Podman, "system", "info", "--format", "json")) + rr, err := runCmd(exec.Command(Podman, "system", "info", "--format", "json")) if err != nil { return ps, errors.Wrap(err, "get podman system info") } diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index 1bb5cdeaaf39..2ba57b06be57 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -56,13 +56,13 @@ func digDNS(ociBin, containerName, dns string) (net.IP, error) { // dockerGatewayIP gets the default gateway ip for the docker bridge on the user's host machine // gets the ip from user's host docker func dockerGatewayIP() (net.IP, error) { - rr, err := runCmd(exec.Command(Env, Docker, "network", "ls", "--filter", "name=bridge", "--format", "{{.ID}}")) + rr, err := runCmd(exec.Command(Docker, "network", "ls", "--filter", "name=bridge", "--format", "{{.ID}}")) if err != nil { return nil, errors.Wrapf(err, "get network bridge") } bridgeID := strings.TrimSpace(rr.Stdout.String()) - rr, err = runCmd(exec.Command(Env, Docker, "inspect", + rr, err = runCmd(exec.Command(Docker, "inspect", "--format", "{{(index .IPAM.Config 0).Gateway}}", bridgeID)) if err != nil { return nil, errors.Wrapf(err, "inspect IP bridge network %q.", bridgeID) @@ -84,12 +84,12 @@ func ForwardedPort(ociBin string, ociID string, contPort int) (int, error) { if ociBin == Podman { //podman inspect -f "{{range .NetworkSettings.Ports}}{{if eq .ContainerPort "80"}}{{.HostPort}}{{end}}{{end}}" - rr, err = runCmd(exec.Command(Sudo, ociBin, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID)) + rr, err = runCmd(exec.Command(ociBin, "inspect", "-f", fmt.Sprintf("{{range .NetworkSettings.Ports}}{{if eq .ContainerPort %s}}{{.HostPort}}{{end}}{{end}}", fmt.Sprint(contPort)), ociID)) if err != nil { return 0, errors.Wrapf(err, "get port %d for %q", contPort, ociID) } } else { - rr, err = runCmd(exec.Command(Env, ociBin, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID)) + rr, err = runCmd(exec.Command(ociBin, "inspect", "-f", fmt.Sprintf("'{{(index (index .NetworkSettings.Ports \"%d/tcp\") 0).HostPort}}'", contPort), ociID)) if err != nil { return 0, errors.Wrapf(err, "get port %d for %q", contPort, ociID) } @@ -116,7 +116,7 @@ func ContainerIPs(ociBin string, name string) (string, string, error) { // podmanConttainerIP returns ipv4, ipv6 of container or error func podmanConttainerIP(name string) (string, string, error) { - rr, err := runCmd(exec.Command(Sudo, Podman, "inspect", + rr, err := runCmd(exec.Command(Podman, "inspect", "-f", "{{.NetworkSettings.IPAddress}}", name)) if err != nil { @@ -132,7 +132,7 @@ func podmanConttainerIP(name string) (string, string, error) { // dockerContainerIP returns ipv4, ipv6 of container or error func dockerContainerIP(name string) (string, string, error) { // retrieve the IP address of the node using docker inspect - lines, err := inspect(Env, Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}") + lines, err := inspect(Docker, name, "{{range .NetworkSettings.Networks}}{{.IPAddress}},{{.GlobalIPv6Address}}{{end}}") if err != nil { return "", "", errors.Wrap(err, "inspecting NetworkSettings.Networks") } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 10120ed97cb0..9e633b9a8eff 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -38,10 +38,10 @@ import ( // DeleteContainersByLabel deletes all containers that have a specific label // if there no containers found with the given label, it will return nil -func DeleteContainersByLabel(prefix string, ociBin string, label string) []error { +func DeleteContainersByLabel(ociBin string, label string) []error { var deleteErrs []error - cs, err := ListContainersByLabel(prefix, ociBin, label) + cs, err := ListContainersByLabel(ociBin, label) if err != nil { return []error{fmt.Errorf("listing containers by label %q", label)} } @@ -51,7 +51,7 @@ func DeleteContainersByLabel(prefix string, ociBin string, label string) []error } for _, c := range cs { - _, err := ContainerStatus(prefix, ociBin, c) + _, 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 { @@ -59,11 +59,11 @@ func DeleteContainersByLabel(prefix string, ociBin string, label string) []error glog.Errorf("%s daemon seems to be stuck. Please try restarting your %s. :%v", ociBin, ociBin, err) continue } - if err := ShutDown(prefix, ociBin, c); err != nil { + if err := ShutDown(ociBin, c); err != nil { glog.Infof("couldn't shut down %s (might be okay): %v ", c, err) } - if _, err := runCmd(exec.Command(prefix, ociBin, "rm", "-f", "-v", c)); err != nil { + if _, err := runCmd(exec.Command(ociBin, "rm", "-f", "-v", c)); err != nil { deleteErrs = append(deleteErrs, errors.Wrapf(err, "delete container %s: output %s", c, err)) } @@ -72,18 +72,18 @@ func DeleteContainersByLabel(prefix string, ociBin string, label string) []error } // DeleteContainer deletes a container by ID or Name -func DeleteContainer(prefix string, ociBin string, name string) error { +func DeleteContainer(ociBin string, name string) error { - _, err := ContainerStatus(prefix, ociBin, name) + _, err := ContainerStatus(ociBin, name) if err != nil { 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 - if err := ShutDown(prefix, ociBin, name); err != nil { + if err := ShutDown(ociBin, name); err != nil { glog.Infof("couldn't shut down %s (might be okay): %v ", name, err) } - if _, err := runCmd(exec.Command(prefix, ociBin, "rm", "-f", "-v", name)); err != nil { + if _, err := runCmd(exec.Command(ociBin, "rm", "-f", "-v", name)); err != nil { return errors.Wrapf(err, "delete %s", name) } return nil @@ -175,25 +175,25 @@ func CreateContainerNode(p CreateParams) error { // adds node specific args runArgs = append(runArgs, p.ExtraArgs...) - if enabled := isUsernsRemapEnabled(p.OCIPrefix, p.OCIBinary); enabled { + if enabled := isUsernsRemapEnabled(p.OCIBinary); enabled { // We need this argument in order to make this command work // in systems that have userns-remap enabled on the docker daemon runArgs = append(runArgs, "--userns=host") } - if err := createContainer(p.OCIPrefix, p.OCIBinary, p.Image, withRunArgs(runArgs...), withMounts(p.Mounts), withPortMappings(p.PortMappings)); err != nil { + if err := createContainer(p.OCIBinary, p.Image, withRunArgs(runArgs...), withMounts(p.Mounts), withPortMappings(p.PortMappings)); err != nil { return errors.Wrap(err, "create container") } checkRunning := func() error { - r, err := ContainerRunning(p.OCIPrefix, p.OCIBinary, p.Name) + r, err := ContainerRunning(p.OCIBinary, p.Name) if err != nil { return fmt.Errorf("temporary error checking running for %q : %v", p.Name, err) } if !r { return fmt.Errorf("temporary error created container %q is not running yet", p.Name) } - s, err := ContainerStatus(p.OCIPrefix, p.OCIBinary, p.Name) + s, err := ContainerStatus(p.OCIBinary, p.Name) if err != nil { return fmt.Errorf("temporary error checking status for %q : %v", p.Name, err) } @@ -213,7 +213,7 @@ func CreateContainerNode(p CreateParams) error { } // CreateContainer creates a container with "docker/podman run" -func createContainer(prefix string, ociBin string, image string, opts ...createOpt) error { +func createContainer(ociBin string, image string, opts ...createOpt) error { o := &createOpts{} for _, opt := range opts { o = opt(o) @@ -227,7 +227,7 @@ func createContainer(prefix string, ociBin string, image string, opts ...createO runArgs = append(runArgs, generatePortMappings(portMapping)...) } // construct the actual docker run argv - args := []string{ociBin, "run"} + args := []string{"run"} // to run nested container from privileged container in podman https://bugzilla.redhat.com/show_bug.cgi?id=1687713 if ociBin == Podman { @@ -238,7 +238,7 @@ func createContainer(prefix string, ociBin string, image string, opts ...createO args = append(args, image) args = append(args, o.ContainerArgs...) - if _, err := runCmd(exec.Command(prefix, args...)); err != nil { + if _, err := runCmd(exec.Command(ociBin, args...)); err != nil { return err } @@ -246,8 +246,8 @@ func createContainer(prefix string, ociBin string, image string, opts ...createO } // ContainerID returns id of a container name -func ContainerID(prefix, ociBin string, nameOrID string) (string, error) { - rr, err := runCmd(exec.Command(prefix, ociBin, "inspect", "-f", "{{.Id}}", nameOrID)) +func ContainerID(ociBin string, nameOrID string) (string, error) { + rr, err := runCmd(exec.Command(ociBin, "inspect", "-f", "{{.Id}}", nameOrID)) if err != nil { // don't return error if not found, only return empty string if strings.Contains(rr.Stdout.String(), "Error: No such object:") || strings.Contains(rr.Stdout.String(), "unable to find") { err = nil @@ -258,7 +258,7 @@ func ContainerID(prefix, ociBin string, nameOrID string) (string, error) { } // ContainerExists checks if container name exists (either running or exited) -func ContainerExists(prefix string, ociBin string, name string, warnSlow ...bool) (bool, error) { +func ContainerExists(ociBin string, name string, warnSlow ...bool) (bool, error) { rr, err := runCmd(exec.Command(ociBin, "ps", "-a", "--format", "{{.Names}}"), warnSlow...) if err != nil { return false, err @@ -276,8 +276,8 @@ func ContainerExists(prefix string, ociBin string, name string, warnSlow ...bool // 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(prefix, ociBin string, nameOrID string) bool { - rr, err := runCmd(exec.Command(prefix, ociBin, "inspect", nameOrID, "--format", "{{.Config.Labels}}")) +func IsCreatedByMinikube(ociBin string, nameOrID string) bool { + rr, err := runCmd(exec.Command(ociBin, "inspect", nameOrID, "--format", "{{.Config.Labels}}")) if err != nil { return false } @@ -289,13 +289,14 @@ func IsCreatedByMinikube(prefix, ociBin string, nameOrID string) bool { return false } -func ListOwnedContainers(prefix string, ociBin string) ([]string, error) { - return ListContainersByLabel(prefix, ociBin, ProfileLabelKey) +// ListOwnedContainers lists all the containres that kic driver created on user's machine using a label +func ListOwnedContainers(ociBin string) ([]string, error) { + return ListContainersByLabel(ociBin, ProfileLabelKey) } // inspect return low-level information on containers -func inspect(prefix string, ociBin string, containerNameOrID, format string) ([]string, error) { - cmd := exec.Command(prefix, ociBin, "inspect", +func inspect(ociBin string, containerNameOrID, format string) ([]string, error) { + cmd := exec.Command(ociBin, "inspect", "-f", format, containerNameOrID) // ... against the "node" container var buff bytes.Buffer @@ -357,8 +358,8 @@ func generateMountBindings(mounts ...Mount) []string { } // isUsernsRemapEnabled checks if userns-remap is enabled in docker -func isUsernsRemapEnabled(prefix string, ociBin string) bool { - cmd := exec.Command(prefix, ociBin, "info", "--format", "'{{json .SecurityOptions}}'") +func isUsernsRemapEnabled(ociBin string) bool { + cmd := exec.Command(ociBin, "info", "--format", "'{{json .SecurityOptions}}'") var buff bytes.Buffer cmd.Stdout = &buff cmd.Stderr = &buff @@ -420,8 +421,8 @@ func withPortMappings(portMappings []PortMapping) createOpt { } // ListContainersByLabel returns all the container names with a specified label -func ListContainersByLabel(prefix string, ociBin string, label string, warnSlow ...bool) ([]string, error) { - rr, err := runCmd(exec.Command(prefix, ociBin, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}"), warnSlow...) +func ListContainersByLabel(ociBin string, label string, warnSlow ...bool) ([]string, error) { + rr, err := runCmd(exec.Command(ociBin, "ps", "-a", "--filter", fmt.Sprintf("label=%s", label), "--format", "{{.Names}}"), warnSlow...) if err != nil { return nil, err } @@ -456,8 +457,8 @@ func PointToHostDockerDaemon() error { } // ContainerRunning returns running state of a container -func ContainerRunning(prefix string, ociBin string, name string, warnSlow ...bool) (bool, error) { - rr, err := runCmd(exec.Command(prefix, ociBin, "inspect", name, "--format={{.State.Running}}"), warnSlow...) +func ContainerRunning(ociBin string, name string, warnSlow ...bool) (bool, error) { + rr, err := runCmd(exec.Command(ociBin, "inspect", name, "--format={{.State.Running}}"), warnSlow...) if err != nil { return false, err } @@ -465,8 +466,8 @@ func ContainerRunning(prefix string, ociBin string, name string, warnSlow ...boo } // ContainerStatus returns status of a container running,exited,... -func ContainerStatus(prefix string, ociBin string, name string, warnSlow ...bool) (state.State, error) { - cmd := exec.Command(prefix, ociBin, "inspect", name, "--format={{.State.Status}}") +func ContainerStatus(ociBin string, name string, warnSlow ...bool) (state.State, error) { + cmd := exec.Command(ociBin, "inspect", name, "--format={{.State.Status}}") rr, err := runCmd(cmd, warnSlow...) o := strings.TrimSpace(rr.Stdout.String()) switch o { @@ -490,15 +491,15 @@ func ContainerStatus(prefix string, ociBin string, name string, warnSlow ...bool // ShutDown will run command to shut down the container // to ensure the containers process and networking bindings are all closed // to avoid containers getting stuck before delete https://github.com/kubernetes/minikube/issues/7657 -func ShutDown(prefix string, ociBin string, name string) error { - if _, err := runCmd(exec.Command(prefix, ociBin, "exec", "--privileged", "-t", name, "/bin/bash", "-c", "sudo init 0")); err != nil { +func ShutDown(ociBin string, name string) error { + if _, err := runCmd(exec.Command(ociBin, "exec", "--privileged", "-t", name, "/bin/bash", "-c", "sudo init 0")); err != nil { glog.Infof("error shutdown %s: %v", name, err) } // helps with allowing docker realize the container is exited and report its status correctly. time.Sleep(time.Second * 1) // wait till it is stoped stopped := func() error { - st, err := ContainerStatus(prefix, ociBin, name) + st, err := ContainerStatus(ociBin, name) if st == state.Stopped { glog.Infof("container %s status is %s", name, st) return nil diff --git a/pkg/drivers/kic/oci/types.go b/pkg/drivers/kic/oci/types.go index 45c38522b98b..8c43ce7d8b1c 100644 --- a/pkg/drivers/kic/oci/types.go +++ b/pkg/drivers/kic/oci/types.go @@ -19,10 +19,6 @@ package oci const ( // DefaultBindIPV4 is The default IP the container will listen on. DefaultBindIPV4 = "127.0.0.1" - // Env is env - Env = "env" - // Sudo is sudo - Sudo = "sudo" // Docker is docker Docker = "docker" // Podman is podman @@ -51,7 +47,6 @@ type CreateParams struct { Memory string // memory (mbs) to assign to the container Envs map[string]string // environment variables to pass to the container ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... - OCIPrefix string // env or sudo OCIBinary string // docker or podman } diff --git a/pkg/drivers/kic/oci/volumes.go b/pkg/drivers/kic/oci/volumes.go index 37dea4d35eee..319d07c342b6 100644 --- a/pkg/drivers/kic/oci/volumes.go +++ b/pkg/drivers/kic/oci/volumes.go @@ -29,18 +29,18 @@ import ( // DeleteAllVolumesByLabel deletes all volumes that have a specific label // if there is no volume to delete it will return nil -func DeleteAllVolumesByLabel(prefix string, ociBin string, label string, warnSlow ...bool) []error { +func DeleteAllVolumesByLabel(ociBin string, label string, warnSlow ...bool) []error { var deleteErrs []error glog.Infof("trying to delete all %s volumes with label %s", ociBin, label) - vs, err := allVolumesByLabel(prefix, ociBin, label) + vs, err := allVolumesByLabel(ociBin, label) if err != nil { return []error{fmt.Errorf("listing volumes by label %q: %v", label, err)} } for _, v := range vs { - if _, err := runCmd(exec.Command(prefix, ociBin, "volume", "rm", "--force", v), warnSlow...); err != nil { + if _, err := runCmd(exec.Command(ociBin, "volume", "rm", "--force", v), warnSlow...); err != nil { deleteErrs = append(deleteErrs, fmt.Errorf("deleting %q", v)) } } @@ -51,10 +51,10 @@ func DeleteAllVolumesByLabel(prefix string, ociBin string, label string, warnSlo // 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(prefix string, ociBin string, label string, warnSlow ...bool) []error { +func PruneAllVolumesByLabel(ociBin string, label string, warnSlow ...bool) []error { var deleteErrs []error glog.Infof("trying to prune all %s volumes with label %s", ociBin, label) - cmd := exec.Command(prefix, ociBin, "volume", "prune", "-f", "--filter", "label="+label) + cmd := exec.Command(ociBin, "volume", "prune", "-f", "--filter", "label="+label) if _, err := runCmd(cmd, warnSlow...); err != nil { deleteErrs = append(deleteErrs, errors.Wrapf(err, "prune volume by label %s", label)) } @@ -64,8 +64,8 @@ func PruneAllVolumesByLabel(prefix string, ociBin string, label string, warnSlow // allVolumesByLabel returns name of all docker volumes by a specific label // will not return error if there is no volume found. -func allVolumesByLabel(prefix string, ociBin string, label string) ([]string, error) { - rr, err := runCmd(exec.Command(prefix, ociBin, "volume", "ls", "--filter", "label="+label, "--format", "{{.Name}}")) +func allVolumesByLabel(ociBin string, label string) ([]string, error) { + rr, err := runCmd(exec.Command(ociBin, "volume", "ls", "--filter", "label="+label, "--format", "{{.Name}}")) s := bufio.NewScanner(bytes.NewReader(rr.Stdout.Bytes())) var vols []string for s.Scan() { diff --git a/pkg/drivers/kic/types.go b/pkg/drivers/kic/types.go index 3d6826937833..2b01019b2ed8 100644 --- a/pkg/drivers/kic/types.go +++ b/pkg/drivers/kic/types.go @@ -49,7 +49,6 @@ type Config struct { CPU int // Number of CPU cores assigned to the container Memory int // max memory in MB StorePath string // libmachine store path - OCIPrefix string // prefix to use (env, sudo, ...) OCIBinary string // oci tool to use (docker, podman,...) ImageDigest string // image name with sha to use for the node Mounts []oci.Mount // mounts diff --git a/pkg/minikube/command/kic_runner.go b/pkg/minikube/command/kic_runner.go index 98c66e5426c0..667b3cb1e6c5 100644 --- a/pkg/minikube/command/kic_runner.go +++ b/pkg/minikube/command/kic_runner.go @@ -38,22 +38,19 @@ import ( // It implements the CommandRunner interface. type kicRunner struct { nameOrID string - prefix string ociBin string } // NewKICRunner returns a kicRunner implementor of runner which runs cmds inside a container -func NewKICRunner(containerNameOrID string, prefix string, oci string) Runner { +func NewKICRunner(containerNameOrID string, oci string) Runner { return &kicRunner{ nameOrID: containerNameOrID, - prefix: prefix, // env or sudo - ociBin: oci, // docker or podman + ociBin: oci, // docker or podman } } func (k *kicRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) { args := []string{ - k.ociBin, "exec", // run with privileges so we can remount etc.. "--privileged", @@ -84,7 +81,7 @@ func (k *kicRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) { args, cmd.Args..., ) - oc := exec.Command(k.prefix, args...) + oc := exec.Command(k.ociBin, args...) oc.Stdin = cmd.Stdin oc.Stdout = cmd.Stdout oc.Stderr = cmd.Stderr @@ -111,6 +108,9 @@ func (k *kicRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) { oc.Stdout = outb oc.Stderr = errb + oc = oci.PrefixCmd(oc) + glog.Infof("Args: %v", oc.Args) + start := time.Now() err := oc.Run() @@ -202,14 +202,14 @@ func (k *kicRunner) chmod(dst string, perm string) error { // Podman cp command doesn't match docker and doesn't have -a func copyToPodman(src string, dest string) error { - if out, err := exec.Command(oci.Sudo, oci.Podman, "cp", src, dest).CombinedOutput(); err != nil { + if out, err := oci.PrefixCmd(exec.Command(oci.Podman, "cp", src, dest)).CombinedOutput(); err != nil { return errors.Wrapf(err, "podman copy %s into %s, output: %s", src, dest, string(out)) } return nil } func copyToDocker(src string, dest string) error { - if out, err := exec.Command(oci.Docker, "cp", "-a", src, dest).CombinedOutput(); err != nil { + if out, err := oci.PrefixCmd(exec.Command(oci.Docker, "cp", "-a", src, dest)).CombinedOutput(); err != nil { return errors.Wrapf(err, "docker copy %s into %s, output: %s", src, dest, string(out)) } return nil diff --git a/pkg/minikube/config/profile.go b/pkg/minikube/config/profile.go index 788d98afea26..8345341c5107 100644 --- a/pkg/minikube/config/profile.go +++ b/pkg/minikube/config/profile.go @@ -203,7 +203,7 @@ func ListProfiles(miniHome ...string) (validPs []*Profile, inValidPs []*Profile, return nil, nil, err } // try to get profiles list based on all contrainers created by docker driver - cs, err := oci.ListOwnedContainers(oci.Env, oci.Docker) + cs, err := oci.ListOwnedContainers(oci.Docker) if err == nil { pDirs = append(pDirs, cs...) } diff --git a/pkg/minikube/machine/delete.go b/pkg/minikube/machine/delete.go index 134c6a74eb6e..4cd55d37d06c 100644 --- a/pkg/minikube/machine/delete.go +++ b/pkg/minikube/machine/delete.go @@ -35,12 +35,12 @@ import ( // 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(prefix string, ociBin string, name string) { +func deleteOrphanedKIC(ociBin string, name string) { if !(ociBin == oci.Podman || ociBin == oci.Docker) { return } - _, err := oci.ContainerStatus(prefix, ociBin, name) + _, err := oci.ContainerStatus(ociBin, name) if err != nil { glog.Infof("couldn't inspect container %q before deleting: %v", name, err) return @@ -49,10 +49,10 @@ func deleteOrphanedKIC(prefix string, ociBin string, name string) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := oci.ShutDown(prefix, ociBin, name); err != nil { + if err := oci.ShutDown(ociBin, name); err != nil { glog.Infof("couldn't shut down %s (might be okay): %v ", name, err) } - cmd := exec.CommandContext(ctx, prefix, ociBin, "rm", "-f", "-v", name) + 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!") @@ -63,8 +63,8 @@ func deleteOrphanedKIC(prefix string, ociBin string, name string) { func DeleteHost(api libmachine.API, machineName string) error { host, err := api.Load(machineName) if err != nil && host == nil { - deleteOrphanedKIC(oci.Env, oci.Docker, machineName) - deleteOrphanedKIC(oci.Sudo, oci.Podman, machineName) + deleteOrphanedKIC(oci.Docker, machineName) + deleteOrphanedKIC(oci.Podman, machineName) // Keep going even if minikube does not know about the host } diff --git a/pkg/minikube/machine/stop.go b/pkg/minikube/machine/stop.go index f8d5fa3cf3bb..d5cac2b2d3fe 100644 --- a/pkg/minikube/machine/stop.go +++ b/pkg/minikube/machine/stop.go @@ -80,7 +80,7 @@ func trySSHPowerOff(h *host.Host) error { out.T(out.Shutdown, `Powering off "{{.profile_name}}" via SSH ...`, out.V{"profile_name": h.Name}) // differnet for kic because RunSSHCommand is not implemented by kic if driver.IsKIC(h.DriverName) { - err := oci.ShutDown("sudo", h.DriverName, h.Name) + err := oci.ShutDown(h.DriverName, h.Name) glog.Infof("shutdown container: err=%v", err) } else { out, err := h.RunSSHCommand("sudo poweroff") diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index ada73686c023..6f47a34a1dfb 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -48,7 +48,7 @@ func init() { if err := registry.Register(registry.DriverDef{ Name: driver.Docker, Config: configure, - Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: oci.Env, OCIBinary: oci.Docker}) }, + Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIBinary: oci.Docker}) }, Status: status, Priority: priority, }); err != nil { @@ -63,7 +63,6 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { ImageDigest: viper.GetString("base-image"), CPU: cc.CPUs, Memory: cc.Memory, - OCIPrefix: oci.Env, OCIBinary: oci.Docker, APIServerPort: cc.Nodes[0].Port, KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, diff --git a/pkg/minikube/registry/drvs/podman/podman.go b/pkg/minikube/registry/drvs/podman/podman.go index ffde7a19b977..7f985110b9fc 100644 --- a/pkg/minikube/registry/drvs/podman/podman.go +++ b/pkg/minikube/registry/drvs/podman/podman.go @@ -51,7 +51,7 @@ func init() { if err := registry.Register(registry.DriverDef{ Name: driver.Podman, Config: configure, - Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIPrefix: oci.Sudo, OCIBinary: oci.Podman}) }, + Init: func() drivers.Driver { return kic.NewDriver(kic.Config{OCIBinary: oci.Podman}) }, Status: status, Priority: priority, }); err != nil { @@ -67,7 +67,6 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { ImageDigest: strings.Split(baseImage, "@")[0], // for podman does not support docker images references with both a tag and digest. CPU: cc.CPUs, Memory: cc.Memory, - OCIPrefix: oci.Sudo, OCIBinary: oci.Podman, APIServerPort: cc.Nodes[0].Port, KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, @@ -89,7 +88,7 @@ func status() registry.State { cmd := exec.CommandContext(ctx, oci.Podman, "version", "--format", "{{.Server.Version}}") // Run with sudo on linux (local), otherwise podman-remote (as podman) if runtime.GOOS == "linux" { - cmd = exec.CommandContext(ctx, oci.Sudo, "-n", oci.Podman, "version", "--format", "{{.Version}}") + cmd = exec.CommandContext(ctx, "sudo", "-n", oci.Podman, "version", "--format", "{{.Version}}") cmd.Env = append(os.Environ(), "LANG=C", "LC_ALL=C") // sudo is localized } o, err := cmd.Output()