Disable base image flag for vm drivers #9300
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @ilya-zuyev! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @ilya-zuyev. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ilya-zuyev The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Can one of the admins verify this patch? |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
ilya-zuyev
changed the title
|
thank you @ilya-zuyev for this PR, please make sure to sign CNCF here https://identity.linuxfoundation.org/projects/cncf |
|
I signed it |
k8s-ci-robot
added
cncf-cla: yes
|
^ following instructions from https://identity.linuxfoundation.org/projects/cncf to activate CNCF CLA |
@medyagh Done! Thanks for the link |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
kvm2 Driver |
cmd/minikube/cmd/start.go
Outdated
| } | ||
| func validBaseImageFlag(baseImageFlagSet bool, driver string) bool { | ||
| if baseImageFlagSet { | ||
| return registry.IsKIC(driver) |
There was a problem hiding this comment.
to make sure we cover Baremetal (none driver)
it is better we check using IsVM here https://github.com/medyagh/minikube/blob/107eb57a110a4a650fc125401719b51e6eccf111/pkg/minikube/driver/driver.go#L105
driver.IsVM(drver)
cmd/minikube/cmd/start.go
Outdated
| @@ -1185,6 +1191,31 @@ func validateKubernetesVersion(old *config.ClusterConfig) { | |||
| } | |||
| } | |||
|
|
|||
| // validateBaseImage checks that --base-image is not passed if the drive being in use is KIC (docker/podman) | |||
| // if so, the function exits the process | |||
| func validateBaseImage(imageFlag *pflag.Flag, driver string) { | |||
There was a problem hiding this comment.
to avoid confusion with driver package consider using a shorter variable name for driver, such as drv
There was a problem hiding this comment.
good catch! fixed.
|
thanks for the unit tests :) |
Codecov Report
@@ Coverage Diff @@
## master #9300 +/- ##
==========================================
- Coverage 29.51% 29.47% -0.05%
==========================================
Files 170 170
Lines 10307 10325 +18
==========================================
+ Hits 3042 3043 +1
- Misses 6842 6859 +17
Partials 423 423
|
|
kvm2 Driver Times for Minikube (PR 9300): 64.1s 67.4s 61.8s Averages Time Per Log docker Driver Times for Minikube (PR 9300): 31.7s 30.5s 28.9s Averages Time Per Log |
Disallow
--base-imageflag when a KIC driver is usedThis PR fixes #8891.
If
--base-imageflag is passed for thestartcommand and the active driver, passed through command line args or minikube config file is KIC, i.e. 'docker' or 'podman' an error message is displayed and minikube exits