-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Support setting addons from environmental variables #11469
Support setting addons from environmental variables #11469
Conversation
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
Welcome @Utkarsh-pro! |
Hi @Utkarsh-pro. 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. |
Can one of the admins verify this patch? |
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
pkg/minikube/config/config.go
Outdated
@@ -56,6 +56,8 @@ const ( | |||
AddonImages = "addon-images" | |||
// AddonRegistries stores custom addon images config | |||
AddonRegistries = "addon-registries" | |||
// AddonList represents the key for addons parameter | |||
AddonList = "addons" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets change this to something obvious that it is the flag name how about "AddonListFlag"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense. Making the changes, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done @medyagh 😄
Signed-off-by: Utkarsh Srivastava <srivastavautkarsh8097@gmail.com>
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 49.5s 45.7s 49.4s 45.9s 45.7s Times for minikube (PR 11469) ingress: 42.3s 35.2s 35.7s 35.3s 42.8s docker driver with docker runtime
Times for minikube ingress: 37.5s 38.5s 34.5s 34.0s 35.0s Times for minikube start: 22.9s 21.5s 21.8s 22.3s 22.5s docker driver with containerd runtime
Times for minikube start: 31.2s 46.8s 44.1s 47.4s 47.2s |
@medyagh I am not sure why the test is failing. On checking the logs, it says that I ran
|
/retest |
@Utkarsh-pro failures dont seem to be related to your PR |
@medyagh is there something that needs to be done to move this PR ahead? |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, utkarsh-pro 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 |
thank you @Utkarsh-pro for this PR look forward to see more more contributions form you ! maybe our other flags needs to be fixed too on ENV var? |
@medyagh, I would love to add support for environmental variables to other flags! Do we need this support in all of the flags? |
Signed-off-by: Utkarsh Srivastava srivastavautkarsh8097@gmail.com
This PR fixes #11171
Description
This PR adds support for setting addons from environmental variables (along with flags).
Examples:
minikube start --addons=registry,olm
- will populate the addons list with "registry" and "olm" addons.MINIKUBE_ADDONS="registry olm" minikube start
- will populate the addons list with "registry" and "olm" addons.MINIKUBE_ADDONS="registry olm" minikube start --addons=registry
- will populate the addons list with "registry" addon only as the flag will take precedence.Note For Reviewers
A global variable called
addonList
was removed as it was no longer getting referenced in the code. Instead of using the global variable,viper.GetStringSlice()
is used.