Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "sudo" to podman calls #7631

Merged
merged 28 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d7dc7bf
The podman driver should not be run as root
afbjorklund Apr 12, 2020
f264ac1
Remove extra single-quotes from format parameter
afbjorklund Apr 13, 2020
401e94c
Don't download podman images to docker daemon
afbjorklund Apr 13, 2020
22aa1af
Handle more state and status for the podman driver
afbjorklund Apr 13, 2020
f57faf8
Only remount the /var/lib/minikube directory
afbjorklund Apr 13, 2020
df3aec6
Missed to wrap one podman inspect in sudo proper
afbjorklund Apr 13, 2020
2234246
Also create and mount the container runtime dirs
afbjorklund Apr 13, 2020
8987b10
Also persist the kubelet and the cni state dirs
afbjorklund Apr 13, 2020
45ec38d
Fix hack binary, as pointed out by golangci-lint
afbjorklund Apr 13, 2020
024cd6b
Use the same kind of named /var mount for podman
afbjorklund Apr 14, 2020
19be561
Resource limits seem to work ok when not rootless
afbjorklund Apr 14, 2020
e99340b
Improve podman status when checking for sudo
afbjorklund Apr 18, 2020
6644c5c
Merge branch 'master' into podman-sudo
afbjorklund Apr 25, 2020
bb37c70
Fix log message to say driver and not cruntime
afbjorklund Apr 25, 2020
7647b1f
Don't try to limit podman memory without cgroup
afbjorklund Apr 25, 2020
7185140
Remove extra quotes added in conflict resolution
afbjorklund Apr 25, 2020
88c8a24
Pass the container implementation on to systemd
afbjorklund Apr 25, 2020
d96d9d3
Include any output from podman version in the log
afbjorklund Apr 26, 2020
28106fa
Use constants instead of the KIC prefix strings
afbjorklund Apr 26, 2020
e95ae32
Fix copy/paste error on the podman usage page
afbjorklund Apr 26, 2020
7951281
Remove old Copy function added in merge conflict
afbjorklund Apr 26, 2020
998ab84
Use simple version of podman info format parameter
afbjorklund Apr 26, 2020
95c9559
Align podman driver with docker and improve check
afbjorklund Apr 27, 2020
a041f40
Podman allows setting --cpus and --memory now
afbjorklund Apr 27, 2020
bf4aa78
Log the version used of the two KIC runtimes
afbjorklund Apr 27, 2020
1744ffe
Restore the configured state added for podman
afbjorklund Apr 27, 2020
f78e00e
Merge branch 'master' into podman-sudo
afbjorklund Apr 28, 2020
78a22f5
Remove prefix parameter and add prefix command
afbjorklund Apr 28, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
The podman driver should not be run as root
Use sudo for the podman commands instead

Wrap the docker commands with env prefix
  • Loading branch information
afbjorklund committed Apr 19, 2020
commit d7dc7bf7b2135cedf835b68285c9b0377720498d
12 changes: 6 additions & 6 deletions cmd/minikube/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
afbjorklund marked this conversation as resolved.
Show resolved Hide resolved
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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
1 change: 0 additions & 1 deletion hack/jenkins/linux_integration_tests_podman.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would happen when the test invokes minikube status , and will invoke sudo podman inspect....wouldn't we need this sudo ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're trying to avoid running sudo minikube (like we do for "none"), since it puts the files in /root or makes them owned by root and such.

You are perfectly right that we still need sudo podman, so it is likely to fail unless the CI servers have a rule to run it (without PASSWD)


EXTRA_ARGS="--container-runtime=containerd"

Expand Down
1 change: 1 addition & 0 deletions hack/preload-images/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 19 additions & 16 deletions pkg/drivers/kic/kic.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type Driver struct {
URL string
exec command.Runner
NodeConfig Config
OCIPrefix string // env, sudo
OCIBinary string // docker,podman
}

Expand All @@ -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
Expand All @@ -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,
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 ")
Expand Down Expand Up @@ -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
}
Expand All @@ -259,29 +262,29 @@ 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
}

// 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")
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/drivers/kic/oci/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 4 additions & 4 deletions pkg/drivers/kic/oci/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)
Expand Down Expand Up @@ -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")
}
Expand Down
Loading