Add podman driver #6515
Merged
Add podman driver #6515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
do-not-merge/work-in-progress
k8s-ci-robot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh 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
approved
tstromberg
suggested changes
Feb 6, 2020
k8s-ci-robot
added
size/XL
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
Codecov Report
@@ Coverage Diff @@
## master #6515 +/- ##
==========================================
- Coverage 37.58% 37.51% -0.07%
==========================================
Files 138 138
Lines 8775 8790 +15
==========================================
Hits 3298 3298
- Misses 5061 5076 +15
Partials 416 416
|
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
|
All Times minikube: [ 92.899320 92.780025 95.546809] Average minikube: 93.742051 Averages Time Per Log |
|
All Times minikube: [ 93.474811 90.618187 93.899242] Average minikube: 92.664080 Averages Time Per Log |
tstromberg
suggested changes
Feb 7, 2020
pkg/minikube/bootstrapper/bsutil/testdata/v1.11/containerd-api-port.yaml
Outdated
Show resolved
Hide resolved
Travis tests have failedHey @medyagh, TravisBuddy Request Identifier: ff7a20f0-49df-11ea-83a0-77379528e44b |
|
All Times minikube: [ 100.684360 95.099886 93.801945] Average minikube: 96.528730 Averages Time Per Log |
|
All Times Minikube (PR 6515): [ 94.764254 94.648696 92.291526] Average minikube: 94.385628 Averages Time Per Log |
|
All Times minikube: [ 92.691382 92.238937 88.440288] Average minikube: 91.123536 Averages Time Per Log |
|
All Times minikube: [ 92.912872 92.366315 96.167349] Average minikube: 93.815512 Averages Time Per Log |
|
All Times minikube: [ 97.908205 92.100632 92.691896] Average minikube: 94.233578 Averages Time Per Log |
|
All Times minikube: [ 97.445583 94.545706 95.357992] Average minikube: 95.783094 Averages Time Per Log |
|
All Times minikube: [ 94.863253 92.646258 91.128265] Average minikube: 92.879259 Averages Time Per Log |
|
All Times minikube: [ 95.267725 95.190305 95.986354] Average minikube: 95.481461 Averages Time Per Log |
|
All Times minikube: [ 95.245625 90.047538 96.860100] Average minikube: 94.051088 Averages Time Per Log |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
tstromberg
suggested changes
Feb 12, 2020
| runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var:exec", hostVarVolPath)) | ||
| } | ||
| if p.OCIBinary == Docker { | ||
| runArgs = append(runArgs, "--volume", fmt.Sprintf("%s:/var", hostVarVolPath)) |
There was a problem hiding this comment.
Could we limit this so that it is only mounted this way on Linux? It potentially offers poor performance when a VM is involved.
| @@ -106,6 +108,11 @@ func BareMetal(name string) bool { | |||
| return name == None || name == Mock | |||
| } | |||
|
|
|||
| // NeedsRoot returns true if driver needs to run with root privileges | |||
| func NeedsRoot(name string) bool { | |||
| return name == None || name == Podman | |||
There was a problem hiding this comment.
This is fine for a first iteration, but do know that is possible to use podman without root:
I'm not sure what the fastest way to notice whether or not the system configuration requires root or not, however.
| cmd := exec.CommandContext(ctx, oci.Docker, "version", "-f", "'{{.Server.Version}}'") | ||
| o, err := cmd.CombinedOutput() | ||
| if err != nil { | ||
| return registry.State{Error: err, Installed: true, Healthy: false, Fix: "Cant verify minimum required version for docker . See docker website for installation guide.", Doc: "https://docs.docker.com/"} |
There was a problem hiding this comment.
Add apostrophe to can't.
|
Just nits - this looks ready to merge otherwise. |
|
All Times minikube: [ 91.568545 96.639706 94.855292] Average Minikube (PR 6515): 92.177536 Averages Time Per Log |
|
@medyagh Would this vm-driver mode need docker engine? When I try to start with podman vm driver, I got following error: |
|
Yes I have tried to remove the existing cluster yet gotten the same warning on docker: |
|
See #7071 for the IBM POWER (ppc64le), rather than commenting on this closed PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes Add support for VM-free deployments using Podman #4390