Skip to content
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

Added wait and interval time flags to minikube service command #1651

Merged

Conversation

aaron-prindle
Copy link
Contributor

@aaron-prindle aaron-prindle commented Jun 25, 2017

This might be useful for users trying to automate services coming up with minikube.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 25, 2017
@aaron-prindle aaron-prindle force-pushed the configurable-service-wait branch from 0162221 to 3fa9543 Compare June 25, 2017 19:39
@aaron-prindle aaron-prindle force-pushed the configurable-service-wait branch from 3fa9543 to 79b9e10 Compare June 25, 2017 20:07
@codecov-io
Copy link

Codecov Report

Merging #1651 into master will increase coverage by 0.06%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1651      +/-   ##
==========================================
+ Coverage   38.62%   38.69%   +0.06%     
==========================================
  Files          51       51              
  Lines        2664     2667       +3     
==========================================
+ Hits         1029     1032       +3     
  Misses       1454     1454              
  Partials      181      181
Impacted Files Coverage Δ
pkg/minikube/service/service.go 34.52% <0%> (ø) ⬆️
cmd/minikube/cmd/service.go 25% <50%> (+4.31%) ⬆️
pkg/minikube/kubeconfig/config.go 51.4% <0%> (+0.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40b0533...79b9e10. Read the comment docs.

@@ -110,7 +114,8 @@ You can add one by annotating a service with the label %s:%s
func init() {
addonsOpenCmd.Flags().BoolVar(&addonsURLMode, "url", false, "Display the kubernetes addons URL in the CLI instead of opening it in the default browser")
addonsOpenCmd.Flags().BoolVar(&https, "https", false, "Open the addons URL with https instead of http")

addonsOpenCmd.Flags().IntVar(&wait, "wait", constants.DefaultWait, "Amount of time to wait for service in seconds")
addonsOpenCmd.Flags().IntVar(&interval, "interval", constants.DefaultInterval, "The time interval for each check that wait performs in seconds")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the wait flag, but interval seems a bit overkill. Any reason to not just try every 1-2 seconds, no matter what wait is?

Copy link
Contributor Author

@aaron-prindle aaron-prindle Jun 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue here is that, for users, I tested the timing (w/ 6 second interval) so that there were only ~3-6 outputs "waiting" if a user waited for a sensible service to come up (single pod service). For something automated, there is no real reason to have the wait be so high. I am fine with removing it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this, simply using the wait flag will have a 6 second default, "minikube service --wait 300 nginx"

@aaron-prindle aaron-prindle merged commit b5fd843 into kubernetes:master Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants