Add addons flag to 'minikube start' in order to enable specified addons #5543

govargo · 2019-10-04T15:27:50Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

So far we have to use the minikube addons enable command to enable the addons.
Now, this PR enables us to use addons with minikube start --addons command flag without minikube addons enable.

Which issue(s) this PR fixes:

Fixes #5503

Does this PR introduce a user-facing change?

Yes. this PR add flag option to minikube start command .

Before this PR, addons flow to enable is following

1. Search the available addons
minikube addons list

2. Enable addons
minikube addons enable <name>

3. Start minikube
minikube start

After this PR, addons flow to enable is following

1. Search the available addons
minikube addons list

2. Start minikube with available addons
minikube addons --addons <name1> --addons <name2> --addons <nameN>

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

NONE

k8s-ci-robot · 2019-10-04T15:27:51Z

Welcome @govargo!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2019-10-04T15:27:58Z

Hi @govargo. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

minikube-bot · 2019-10-04T15:28:02Z

Can one of the admins verify this patch?

k8s-ci-robot · 2019-10-04T15:28:40Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: govargo
To complete the pull request process, please assign ra489
You can assign the PR to them by writing /assign @ra489 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

govargo · 2019-10-04T15:31:18Z

/remove-kind bug

govargo · 2019-10-04T15:32:23Z

/assign @RA489

/cc @tstromberg

medyagh · 2019-10-04T20:39:29Z

cmd/minikube/cmd/start.go

@@ -162,6 +164,7 @@ func initMinikubeFlags() {
 	startCmd.Flags().String(containerRuntime, "docker", "The container runtime to be used (docker, crio, containerd).")
 	startCmd.Flags().Bool(createMount, false, "This will start the mount daemon and automatically mount files into minikube.")
 	startCmd.Flags().String(mountString, constants.DefaultMountDir+":/minikube-host", "The argument to pass the minikube mount command on start.")
+	startCmd.Flags().StringArrayVar(&addonList, addons, nil, "Enable addons. see addon list if you want to check them `minikube addons list`")


remove repetitive addon list.
also please provide before after output in the PR description.

@medyagh Thank you for review!
I removed repetitive addon list.

Before

"Enable addons. see addon list if you want to check them `minikube addons list`"

After

"Enable addons. see `minikube addons list` if you want to check"

And I edited the PR description(user-facing change and Additional documentation).
Should I also include this above change in the PR description?

If I misunderstand your intention, please let me know.

medyagh · 2019-10-04T20:42:07Z

Thank you for this PR ! this will provide a better user experience. I added some minor knits but overall looks good.

medyagh · 2019-10-04T20:42:22Z

@minikube-bot OK to test!

medyagh · 2019-10-05T01:45:52Z

Also please check if there is a place in website we could inform the users of this new flag

codecov-io · 2019-10-06T07:23:09Z

Codecov Report

Merging #5543 into master will decrease coverage by 0.01%.
The diff coverage is 20%.

@@            Coverage Diff             @@
##           master    #5543      +/-   ##
==========================================
- Coverage   36.86%   36.85%   -0.02%     
==========================================
  Files         102      102              
  Lines        7349     7354       +5     
==========================================
+ Hits         2709     2710       +1     
- Misses       4289     4293       +4     
  Partials      351      351

Impacted Files	Coverage Δ
cmd/minikube/cmd/start.go	`22.63% <20%> (-0.03%)`	⬇️

govargo · 2019-10-06T07:36:09Z

@medyagh

Also please check if there is a place in website we could inform the users of this new flag

Sorry, I didn't prepare for documents.
So, I added the new --addons flag to documentaion in new additional commits.
・site/content/en/docs/Reference/Commands/start.md
・site/content/en/docs/Reference/Commands/addons.md
・site/content/en/docs/Tasks/addons.md

medyagh · 2019-10-08T05:40:48Z

@govargo thank you for following up and fixing, I commented again just couple more improvements and then we could merge this after those requested changes are done

govargo · 2019-10-08T10:21:26Z

@medyagh Thank you for your reply.
But I'm sorry I cannot find your additional improvements comment anywhere...
Could you tell me what it is?

(I rebased my branch and force-pushed, so comments may be lost...)

sharifelgamal · 2019-10-09T18:13:47Z

cmd/minikube/cmd/start.go

@@ -342,6 +345,15 @@ func runStart(cmd *cobra.Command, args []string) {
 	// pull images or restart cluster
 	bootstrapCluster(bs, cr, mRunner, config.KubernetesConfig, preExists, isUpgrade)
 	configureMounts()
+
+	// enable addons with start command
+	for _, addonName := range addonList {


addonName is a shortlived variable, so it doesn't need to be that long. how about a as a name?

@sharifelgamal Thank you for your review! I understood It's Golang way.
I modified as you mentioned.

sharifelgamal · 2019-10-09T18:15:40Z

site/content/en/docs/Reference/Commands/start.md

@@ -16,6 +16,7 @@ minikube start [flags]
 ### Options

 ```
+--addons                            Enable addons. see `minikube addons list` if you want to check


Let's change this description to "Enables addons. See minikube addons list for a list of valid addon names."

Thanks to you, the help message became easier to understand!
I changed it as you mentioned.

…explanation to documents

govargo · 2019-10-10T02:44:13Z

I modified a shortlived variable and the --addons help message.
Thank you for review @sharifelgamal!

medyagh

Thanks for addressing the comments on the review !

cmd/minikube/cmd/start.go

site/content/en/docs/Reference/Commands/start.md

site/content/en/docs/Tasks/addons.md

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 4, 2019

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 4, 2019

k8s-ci-robot requested review from josedonizetti and sharifelgamal October 4, 2019 15:28

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 4, 2019

k8s-ci-robot removed the kind/bug Categorizes issue or PR as related to a bug. label Oct 4, 2019

k8s-ci-robot assigned RA489 Oct 4, 2019

k8s-ci-robot requested a review from tstromberg October 4, 2019 15:32

govargo mentioned this pull request Oct 4, 2019

Add --addons flag to minikube start #5503

Closed

medyagh reviewed Oct 4, 2019

View reviewed changes

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 6, 2019

sharifelgamal suggested changes Oct 9, 2019

View reviewed changes

govargo added 2 commits October 10, 2019 11:29

Add addons flag to 'minikube start' in order to enable specified addons

1e11576

remove repetitive addon list from command help and add --addons flag …

76e5f7b

…explanation to documents

govargo added 2 commits October 10, 2019 11:29

add --addons flag explanation to Start.md

bbf6953

Modify shortlived variable name and improve --addons flag help message

f111f09

medyagh approved these changes Oct 10, 2019

View reviewed changes

cmd/minikube/cmd/start.go Outdated Show resolved Hide resolved

cmd/minikube/cmd/start.go Outdated Show resolved Hide resolved

site/content/en/docs/Reference/Commands/start.md Outdated Show resolved Hide resolved

site/content/en/docs/Tasks/addons.md Show resolved Hide resolved

medyagh merged commit d139781 into kubernetes:master Oct 10, 2019

tstromberg mentioned this pull request Oct 14, 2019

integration test: jenkins running the wrong binary but showing the right commit id #5605

Closed

govargo deleted the feature/start-with-addons-flag branch October 15, 2019 02:06

radeksm mentioned this pull request Apr 28, 2020

Example how to enable multiple addons on start command #7924

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add addons flag to 'minikube start' in order to enable specified addons #5543

Add addons flag to 'minikube start' in order to enable specified addons #5543

govargo commented Oct 4, 2019 •

edited

Loading

k8s-ci-robot commented Oct 4, 2019

k8s-ci-robot commented Oct 4, 2019

minikube-bot commented Oct 4, 2019

k8s-ci-robot commented Oct 4, 2019

govargo commented Oct 4, 2019

govargo commented Oct 4, 2019

medyagh Oct 4, 2019

govargo Oct 6, 2019

medyagh commented Oct 4, 2019

medyagh commented Oct 4, 2019

medyagh commented Oct 5, 2019

codecov-io commented Oct 6, 2019 •

edited

Loading

govargo commented Oct 6, 2019

medyagh commented Oct 8, 2019

govargo commented Oct 8, 2019 •

edited

Loading

sharifelgamal Oct 9, 2019

govargo Oct 10, 2019

sharifelgamal Oct 9, 2019

govargo Oct 10, 2019 •

edited

Loading

govargo commented Oct 10, 2019

medyagh left a comment

Add addons flag to 'minikube start' in order to enable specified addons #5543

Add addons flag to 'minikube start' in order to enable specified addons #5543

Conversation

govargo commented Oct 4, 2019 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Oct 4, 2019

k8s-ci-robot commented Oct 4, 2019

minikube-bot commented Oct 4, 2019

k8s-ci-robot commented Oct 4, 2019

govargo commented Oct 4, 2019

govargo commented Oct 4, 2019

medyagh Oct 4, 2019

Choose a reason for hiding this comment

govargo Oct 6, 2019

Choose a reason for hiding this comment

medyagh commented Oct 4, 2019

medyagh commented Oct 4, 2019

medyagh commented Oct 5, 2019

codecov-io commented Oct 6, 2019 • edited Loading

Codecov Report

govargo commented Oct 6, 2019

medyagh commented Oct 8, 2019

govargo commented Oct 8, 2019 • edited Loading

sharifelgamal Oct 9, 2019

Choose a reason for hiding this comment

govargo Oct 10, 2019

Choose a reason for hiding this comment

sharifelgamal Oct 9, 2019

Choose a reason for hiding this comment

govargo Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

govargo commented Oct 10, 2019

medyagh left a comment

Choose a reason for hiding this comment

govargo commented Oct 4, 2019 •

edited

Loading

codecov-io commented Oct 6, 2019 •

edited

Loading

govargo commented Oct 8, 2019 •

edited

Loading

govargo Oct 10, 2019 •

edited

Loading