Only include ports in one container in Kube YAML

[mheon]

This likely broke when we made containers able to detect that they shared a network namespace and grab ports from the dependency container - prior to that, we could grab ports without concern for conflict, only the infra container had them. Now, all containers in a pod will return the same ports, so we have to work around this.

Fixes #3408

Still needs a test, writing one now.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mheon]

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

[rhatdan]

Nice easy fix.
LGTM

[Fodoj]

Don't want to be picky, but doesn't it break portability of generated yaml to k8s environment? Assuming there will be no infra container on k8s it means that pod yaml won't expose any ports.

[mheon]

While writing a test, I realized that all the generate kube tests are verifying the YAML using yaml.Marshal instead of yaml.Unmarshal() which is really not good. Will see about fixing as part of this.

[mheon]

@Fodoj We actually take the ports from the infra container and stick them on the first container in the generated output, so even if there is no infra container, we should be good.

[Fodoj]

@mheon correct me if I am wrong, but then if I have two containers A and B, A has something running on port 1234, B on 4567. Then YAML exposes both ports on container A, but not container B, meaning that B:4567 can't be reached when connecting to the port, and A:4567 is useless, as nothing is running on this port inside A. Of course if 4567 is "exposed" in the container image behind container B, then it probably won't be an issue, but still generated YAML is kind of misleading (A exposes 2 ports, B exposes none via YAML definition, but exposes one as part of an image).

[mheon]

Within Podman alone, if you're using play kube to play the YAML, all ports are properly of the infra container only, and point to the pod as a whole (unless you explicitly opt out of sharing the network namespace when creating a new container). So this will work as expected there - the ports are detected, and added to the infra container once more on container creation.

If moving up to Kube, if you generate a service object, the ports are again forwarded to the pod, so it should work there too.

The question would be importing the YAML into Kube without a service.

(The ability for containers to request individual ports within a pod is coming, hopefully soon, but they'll still all be managed by the infra container - it'll just be taking the requested ports of other containers in the pod into account).

[Fodoj]

If moving up to Kube, if you generate a service object, the ports are again forwarded to the pod, so it should work there too.

My concern is more around that one container (the one that exposes only 1234) would have this config:

    ports:
    - containerPort: 1234
      hostPort: 1234
      protocol: TCP
    - containerPort: 4567
      hostPort: 4567
      protocol: TCP

And another one (that exposes 4567) won't have any port configuration at all.

According to Kubernetes container spec:

List of ports to expose from the container. Exposing a port here gives the system additional information about the network connections a container uses, but is primarily informational. Not specifying a port here DOES NOT prevent that port from being exposed. Any port which is listening on the default "0.0.0.0" address inside a container will be accessible from the network.

So even though not specifying a port wont prevent it from being exposed if container does listen on this port, YAML configuration produced by Podman would be still a bit misleading.

[baude]

@Fodoj can you join us on irc.freenode.net #podman to discuss? we can probably get down to an answer quicker?

[mheon]

My port-detection logic seems to be failing in the tests. Investigating...

[mheon]

Decode is definitely failing - we're seeing 0 containers in the pod.

[mheon]

Throwing a hold on here - even if tests go green I want to do some more work on them.
/hold

[mheon]

Alright. Tests are in a better spot. Cancelling hold
/hold cancel

[baude]

/lgtm