Add "sudo" to podman calls #7631
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afbjorklund The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Codecov Report
@@ Coverage Diff @@
## master #7631 +/- ##
==========================================
- Coverage 35.50% 35.49% -0.02%
==========================================
Files 148 148
Lines 9330 9334 +4
==========================================
Hits 3313 3313
- Misses 5620 5624 +4
Partials 397 397
|
There was a problem hiding this comment.
as part of PR please change this in Jenkins so it does not run with sudo in intergraiton test either. https://github.com/kubernetes/minikube/blob/master/hack/jenkins/linux_integration_tests_podman.sh#L34
afbjorklund
force-pushed
the
podman-sudo
branch
from
e4cecb4 to
c70c085
Compare
Done. |
|
Also need to document that user needs to setup password-less sudo for podman. Otherwise this is going to throw |
afbjorklund
force-pushed
the
podman-sudo
branch
from
c70c085 to
a4fb263
Compare
k8s-ci-robot
added
size/L
|
Now the API is genuinely horrible, with two strings instead of a class or something. Also found some places where it is called with the Driver in the place of "ociBin"... |
afbjorklund
force-pushed
the
podman-sudo
branch
from
a4fb263 to
4208ca6
Compare
|
There are some nice little status differences as well:
So apparently "running" is not enough to be running. :-( |
|
Here is the current startup with Here is the matching startup with That is: the podman start fails (exit 2) |
|
When doing it manually, I get: |
|
When tracing the entrypoint (<sigh>), it seems it was deleted (in podman): This is something that kind base has, wonder why it works in docker ? |
|
@medyagh: https://github.com/kubernetes/minikube/blob/v1.9.2/pkg/drivers/kic/oci/oci.go#L115_L123 It needs to restore the old contents, after remounting var. Even better, only do /var/lib/minikube ? Once /var is back, it boots properly in podman as well. Now, there was some |
|
One side-effect of mounting /var properly, is that you cannot delete a machine anymore. This is because some of the files under ~/.minikube/machines/*/var are now root-owned... |
|
Need to volume mount the container runtime directories too, as well as Now with everything setup, it starts properly. On my machine it took 38 seconds, not too bad. |
|
I went back to /var - it seems it was the path mount that was causing it With a regular named mount, the image boots up just fine - no missing files |
k8s-ci-robot
added
the
needs-rebase
|
This test result looks a bit odd: Probably need some better output logging for it: // 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 {
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"}
} |
|
One reason could be that it has `podman-remote' installed, but not installed the socket. See https://www.projectatomic.io/blog/2018/05/podman-varlink/ for some background That would lead to that exit code, so will check that by adding some output logging... $ podman version || echo $?
Version: 1.8.2
RemoteAPI Version: 1
Go Version: go1.13.8
Git Commit: 028e3317eb1494b9b2acba4a0a295df80fae66cc
Built: Mon Apr 13 15:02:24 2020
OS/Arch: linux/amd64
$ podman-remote version || echo $?
Error: could not get runtime: dial unix /run/podman/io.podman: connect: permission denied
125I don't really want to support remote podman, but that's another story... It seems like remote docker is used (on Mac/Win), even it works so-so. |
|
And perhaps obvious but, the current approach will not work for So it needs to be either of On Mac and Win they just rename "podman-remote" to "podman", confusing everyone. But that also means that the user is on their own setting up a VM, so more like "generic" ? |
|
kvm2 Driver Times for Minikube (PR 7631): [64.50086596499999 64.237303096 65.17437080799999] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [25.965244934 26.630567117 26.106601369] Averages Time Per Log |
|
@medyagh : this branch has now accumulated some bug fixes, in addition to the original feature:
We still need to fix this if we want the instructions to work but I guess we could look at cherry-picking https://minikube.sigs.k8s.io/docs/drivers/podman/
|
So that it works with the older versions of podman
|
kvm2 Driver Times for Minikube (PR 7631): [67.50268481 66.519921533 65.14401095599999] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [26.793725358000003 27.499485453000002 27.642983155000003] Averages Time Per Log |
It was a configuration issue, we set the cgroup config for docker but not for podman: The real problem (below it) is however that we don't have any user namespaces: So podman (in KIC) needs to be modified, to use $ docker info | grep -i driver
Storage Driver: overlay2
Logging Driver: json-file
Cgroup Driver: cgroupfs
WARNING: No swap limit supportAnd we need to use |
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)
|
kvm2 Driver Times for Minikube (PR 7631): [63.18239188700001 66.35834652199999 67.811414446] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [26.346822923999998 28.038690560000003 26.53012123] Averages Time Per Log |
|
kvm2 Driver Times for Minikube (PR 7631): [63.985020524999996 65.06745273099999 64.395266711] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [29.816686754000003 27.543520982000004 29.870189872999998] Averages Time Per Log |
Travis tests have failedHey @afbjorklund, TravisBuddy Request Identifier: 83536640-889f-11ea-88d0-3540a036c4cf |
|
@TravisBuddy : Hey "buddy", my code seems OK but we have a flaky test https://travis-ci.org/github/kubernetes/minikube/jobs/680154938#L684 |
|
kvm2 Driver Times for Minikube (PR 7631): [64.19505352700001 65.73467504599998 66.63154032800001] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [27.904719327 26.650956044000004 27.638542861999994] Averages Time Per Log |
|
kvm2 Driver Times for Minikube (PR 7631): [64.153992683 65.465271967 65.247266402] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [26.728204794999996 26.904490444000004 26.650722499999993] Averages Time Per Log |
Move the "sudo" prefix to a central location, instead of having it all over the place. Assume only needed on Linux.
|
kvm2 Driver Times for Minikube (PR 7631): [63.905515557 64.47543386 63.948030700000004] Averages Time Per Log docker Driver Times for Minikube (PR 7631): [27.340332497 27.380156425000003 27.201906417000004] Averages Time Per Log |
tstromberg
changed the title
| @@ -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 | |||
There was a problem hiding this comment.
This will make a good insertion point for folks who want to add support for handling rootless. Thank you!
TODO is to replace three hundred places of exec.Command OCIBinary with appropriate "sudo".To make it easier one could use a dummy wrapper like "env", for the docker commands as well.
Then it could be like: exec.Command(OCIPrefix, OCIBinary, ...)
env docker infosudo podman infoInstead of having to manipulate arrays during the interpolation ?
For #7480