fix pod creation with "new:" syntax #7152
Conversation
|
LGTM |
cmd/podman/containers/create.go
Outdated
| Name: podName, | ||
| Infra: true, | ||
| Net: netOpts, | ||
| Share: strings.Split(specgen.DefaultKernelNamespaces, ","), |
There was a problem hiding this comment.
It looks like we need to make sure that the defaults are used in Specgen if this is unset.
There was a problem hiding this comment.
Yes - that looks right. If the argument to that is nil, we should return the defaults. A 0-length non-nil array would indicate nothing is shared.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99 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:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
| Name: podName, | ||
| Infra: true, | ||
| Net: netOpts, | ||
| CreateCommand: os.Args, |
There was a problem hiding this comment.
Probably best to leave this empty - it's mainly used for podman generate systemd --new, and it could have misleading results there
There was a problem hiding this comment.
I believe it is correct to set here. I checked podman generate systemd --new and it throws an error that you have to use podman pod create. This can later be used to implement this.
There was a problem hiding this comment.
It looks good to me. As @Luap99 mentioned, generate systemd requires a pod to be generated via pod create and will error out otherwise (see https://github.com/containers/podman/blob/master/pkg/systemd/generate/pods.go#L265).
Setting it here will be more accurate as generate systemd will otherwise error out with "no create command found" which would be less informative than "pod does not appear to be created via podman pod create"
There was a problem hiding this comment.
Alright. We should be good to merge after a rebase, then.
|
One more comment, LGTM once it's addressed |
|
Needs a rebase to fix CI |
|
LGTM |
When you execute podman create/run with the --pod new:<name> syntax the pod was created but the namespaces where not shared and therefore containers could not communicate over localhost. Add the default namespaces and pass the network options to the pod create options. Signed-off-by: Paul Holzinger <[email protected]>
|
One test still failed! A Flake? |
|
Looking... |
|
I've never seen this one before - going to assume it's a flake and restart... |
|
Now it timed out! :( |
|
@mheon Test is green :) |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
When you execute podman create/run with the --pod new:<name> syntax
the pod was created but the namespaces where not shared and
therefore containers could not communicate over localhost.
Add the default namespaces and pass the network options to the
pod create options.
Closes #7087