Require Service.ContainerPort

[thockin]

The fallback for now is to use Container[0].Port[0] which is, IMO, confusing.

[brendandburns]

If you have a simple pod, with one port, and a service for that pod, it feels highly redundant and verbose to have to specify the port too.

I'm not going to fight super hard for this default, but that's the reason it's there.

[thockin]

Discussion happening on two threads, I'll leave it on the other one since I
already replied there. TL;DR Clayton to tie-break.

On Wed, Aug 20, 2014 at 8:49 PM, brendandburns notifications@github.com
wrote:

If you have a simple pod, with one port, and a service for that pod, it
feels highly redundant and verbose to have to specify the port too.

I'm not going to fight super hard for this default, but that's the reason
it's there.

Reply to this email directly or view it on GitHub
#983 (comment)
.

[bgrant0607]

See also #974

[dchen1107]

Can we just simply name it as Service.Port, not Service.ContainerPort. ContainerPort here is confusing too.

[bgrant0607]

Service.Port is the port clients use to connect to the service. Service.ContainerPort is the port to which the service proxy connects on the containers. Service.ContainerPort is consistent with the terminology in the pod port spec.

[thockin]

PodPort would make more sense in Service.
On Aug 21, 2014 9:10 AM, "bgrant0607" notifications@github.com wrote:

Service.Port is the port clients use to connect to the service.
Service.ContainerPort is the port to which the service proxy connects on
the containers. Service.ContainerPort is consistent with the terminology in
the pod port spec.

Reply to this email directly or view it on GitHub
#983 (comment)
.

[lavalamp]

'DestinationPort' is super clear and my preference. PodPort is ambiguous, I vote strong no; is it supposed to mean the where the port shows up in the sending pod or the receiving pod? (Likewise for the current ContainerPort; in fact, I had Port and ContainerPort backwards in my head until chatting with Brian yesterday.)

Using the first port implies that there's an ordering to the ports, which may be an impression we don't want to foster. Is there any other sane default we can use?

I honestly don't think it's that redundant to require both. The service describes a flow of traffic, I think it's actually a little weird to not say where the traffic is supposed to go.

[smarterclayton]

Agree port ordering should not be significant.

[thockin]

Clayton: to clarify, if port ordering is not significant, then no "default to first port" applies to the spec, which implies that ContainerPort (possibly renamed to DestinationPort) is a required field?

Do we care about compat here? Changing a field to required may break people.

[smarterclayton]

We could change that when we rev the API if necessary.

[jbeda]

Merging in discussion from #1030.

How about this:

• An unnamed port is given the name "default". There can only be one of these.
• Change ContainerPort to something better. I like DestinationPort or TargetPort.
• If you don't specify ContainerPort on a service we assume default as the port.

[bgrant0607]

Does the service proxy perform the port name->number mapping? Does it work even in the case that not all pods have the same name->number mapping? Do we want that to work? I assume that it doesn't work in the case of an external load balancer.

I like TargetPort.

[jbeda]

I would expect that ​the proxy would handle the name going to different
numbers on different containers. That would let you upgrade/change the
port and things would keep working.

[lavalamp]

TargetPort is less clear than DestinationPort.

I was proposing to name unnamed ports by their port number, not by their index. IMO we should never ever use indexes to refer to ports, we should treat them as a set, not as a list.

[thockin]

I agree, indexing ports is bad. Let's agree to drop that.

I'm fine with the implicit "default", but I am not sure it adds much to the overall system. Why not just say that Service.DestinationPort is required and it can be specified as a port name (string) or a port number (int).

Last topic: the proxy does not resolve names to port numbers, the master does. I am not sure if this part works: Endpoints in api/types.go says it can have port numbers, implying that a service could resolve to different ports per pod. Service.ContainerPort can take port names, which means that, hypothetically, a Service could be made up of pods with different port numbers on the same name. Someone should, like, test that :)

[smarterclayton]

That sounds like a feature I want (migrate ports across pod templates with the same service) if it actually, you know, worked.

[thockin]

It might work. Nobody has tried AFAIK :) I bet it does, with named ports.

On Tue, Aug 26, 2014 at 3:56 PM, Clayton Coleman notifications@github.com
wrote:

That sounds like a feature I want (migrate ports across pod templates with
the same service) if it actually, you know, worked.

Reply to this email directly or view it on GitHub
#983 (comment)
.

[thockin]

More fuel for this.

We happily default the Service.ContainerPort to Pod.Containers[0].Ports[0] in pkg/service/endpoints_controller.go, but when we set env vars in pkg/registry/service/rest.go, we just use Service.ContainerPort, which is 0.

@Gurpartap reported this today.

The defaulting should either be done in the validation of service, by writing back the defaulted port, or should not be done at all.

[bgrant0607]

We should resolve this as part of the switch to v1beta3 -- #1519 and #2585.

[thockin]

I'm torn. On the one hand, requiring it is simpler. On the other hand,
allowing the default to be runtime discovered actually means the values can
be different per-pod (not sure if that is good). Also, because we do not
inspect containers for EXPOSEd ports, we miss some. Also the current
default is dumb, but that can be fixed.

I guess in typing it out I lean towards simpler is better.

On Mon, Dec 1, 2014 at 12:57 PM, bgrant0607 notifications@github.com
wrote:

We should resolve this as part of the switch to v1beta3 -- #1519
#1519 and #2585
#2585.

Reply to this email directly or view it on GitHub
#983 (comment)
.

[pmorie]

+1 to the idea of working with ports only as a set.

One thing I'll add, making it explicit and requiring ContainerPort to be set makes it much easier to write a UI that presents how service align with ports in pods.

ContainerPort isn't the greatest name. I like TargetPodPort the best but it is a bit verbose.

[brendandburns]

Assigning to @thockin since he says his refactor will cover the rename.

[jdef]

xref mesosphere/kubernetes-mesos#59

[brendandburns]

I'm going to close this issue as:

• Port has been renamed to ContainerPort
• We now have improved default behavior, and attempts to implement a (more) complete solution were rejected by @thockin

Re-open if you have any concerns.