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

Make expose use introspection to grab the port value if possible. #6312

Merged
merged 1 commit into from
Apr 2, 2015

Conversation

brendandburns
Copy link
Contributor

Also improve service printing to include public IP addresses.

@bprashanth
Copy link
Contributor

Jeff, because kubectl

if len(ports) == 0 {
return util.UsageError(cmd, "couldn't find a suitable port via --port flag or introspection")
}
params["port"] = ports[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

This means expose will introspect and select the port of the first container in the case of a multi-container pod. If that is the intended behavior, it should be documented in the help string, though tbh, I think you should fail here if there is more than one port, as we don't know what the user actually intends.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@brendandburns brendandburns force-pushed the services branch 2 times, most recently from cfaf847 to 674efe6 Compare April 2, 2015 20:08
Also improve service printing to include public IP addresses.
@brendandburns
Copy link
Contributor Author

Comments addressed. ptal.

@j3ffml
Copy link
Contributor

j3ffml commented Apr 2, 2015

LGTM. Will merge on green.

@j3ffml j3ffml added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 2, 2015
j3ffml added a commit that referenced this pull request Apr 2, 2015
Make expose use introspection to grab the port value if possible.
@j3ffml j3ffml merged commit 96bdee8 into kubernetes:master Apr 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants