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

Service port names are required for multi-port #7786

Merged
merged 1 commit into from May 11, 2015

Conversation

thockin
Copy link
Member

@thockin thockin commented May 5, 2015

There is no provision for the first port to be unnamed. I think I
initially allowed that, but then the Subset struct became a sorted
struct, so the first-ness of the port got lost. If you have a Service
with one named and one unnamed port, what happens is that the
EndpointController fails to create Endpoints (validation error).

@mbforbes
Copy link
Contributor

mbforbes commented May 5, 2015

This seems reasonable to me, but @derekwaynecarr you might have a bit more context (feel free to reassign if not).

@thockin
Copy link
Member Author

thockin commented May 7, 2015

ping

@thockin
Copy link
Member Author

thockin commented May 8, 2015

ping - please reassign if you don't want this one. It's a 5 minute review :)

There is no provision for the first port to be unnamed.  I think I
initially allowed that, but then the Subset struct became a sorted
struct, so the first-ness of the port got lost.  If you have a Service
with one named and one unnamed port, what happens is that the
EndpointController fails to create Endpoints (validation error).
@vishh
Copy link
Contributor

vishh commented May 11, 2015

LGTM

vishh added a commit that referenced this pull request May 11, 2015
Service port names are required for multi-port
@vishh vishh merged commit b7289b6 into kubernetes:master May 11, 2015
@thockin thockin deleted the ports branch June 25, 2015 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants