container: allow clone to an existing pod

[giuseppe]

Closes: #3979

Signed-off-by: Giuseppe Scrivano [email protected]

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

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 [giuseppe]

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

[rhatdan]

Nice and simple.
LGTM

[Luap99]

How does this behave with the namespaces?
Does the container actually use the pod namespaces? If so this would conflict with the network settings and maybe more.

[mheon]

I don't think it would join the namespaces

[giuseppe]

it joins the ipc and uts namespaces, but it doesn't join the network namespace, checking the infra container init pid and the cloned container:

$ readlink  /proc/{2351872,2351892}/ns/ipc
ipc:[4026533360]
ipc:[4026533360]
$ readlink  /proc/{2351872,2351892}/ns/uts
uts:[4026533359]
uts:[4026533359]
$ readlink  /proc/{2351872,2351892}/ns/net
net:[4026533086]
net:[4026533364]

[Luap99]

This sounds inconsistent. From a user experience I would expect it to join all shared namespaces from the pod.

[giuseppe]

This sounds inconsistent. From a user experience I would expect it to join all shared namespaces from the pod.

should we bark if the container sets up the network differently?

/hold

[cdoern]

yeah I would either warn that the namespaces are going to be altered when joining the pod or fail altogether.

[rhatdan]

I think you should just warn that the container is adopting the the pod's namespaces.

[giuseppe]

fixed. Now I get:

$ readlink /proc/{2618221,2618232}/ns/net
net:[4026533449]
net:[4026533449]
$ readlink  /proc/{2618221,2618232}/ns/ipc
ipc:[4026533529]
ipc:[4026533529]
$ readlink /proc/{2618221,2618232}/ns/uts
uts:[4026533528]
uts:[4026533528]

and these that are expected to be different:

$ readlink /proc/{2618221,2618232}/ns/pid
pid:[4026533530]
pid:[4026533534]
$ readlink /proc/{2618221,2618232}/ns/cgroup
cgroup:[4026533531]
cgroup:[4026533535]

[Luap99]

I don't think this works as expected, I think you should only do the check for the namespaces that are actually shared in the pod instead of all of them.

I tries the following commands as rootless with this patch:

podman pod create --name test --share ipc
podman run --network podman  --name con1 alpine ip a
podman container clone --pod test con1
podman start --attach con1-clone

In this case since you reset the net namespace we loose the network information but the namespace was never shared so we should not reset it.

[giuseppe]

I see, thanks for checking it.

Do you like the newer version?

[Luap99]

That looks good but I would like to have a test case for this.

[giuseppe]

test added

[cdoern]

I am doing a large change to ConfigToSpec in #13594 there are a number of things that seem to be broken just a heads up if that merges

[rhatdan]

/lgtm

[rhatdan]

/hold cancel