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

Allow appending shared namespace to pod (something like --share +pid) #13422

Closed
markkrj opened this issue Mar 4, 2022 · 9 comments · Fixed by #14333
Closed

Allow appending shared namespace to pod (something like --share +pid) #13422

markkrj opened this issue Mar 4, 2022 · 9 comments · Fixed by #14333
Labels
Good First Issue This issue would be a good issue for a first time contributor to undertake. kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue

Comments

@markkrj
Copy link

markkrj commented Mar 4, 2022

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature

Description

Add option to allow appending shared namespace to pod

Steps to reproduce the issue:

  1. podman pod create --name test --share pid

  2. podman pod inspect test --format '{{join .SharedNamespaces ","}}'

Describe the results you received:

pid

Describe the results you expected:

uts,pid,ipc,net

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

Client:       Podman Engine
Version:      4.0.0-dev
API Version:  4.0.0-dev
Go Version:   go1.17.8
Git Commit:   f4d6e8777213880204ccbce92201c47c74b33036-dirty
Built:        Fri Mar  4 12:26:16 2022
OS/Arch:      linux/amd64

Would something like this be acceptable?

diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go
index ab3a6d578..28c61aae5 100644
--- a/cmd/podman/pods/create.go
+++ b/cmd/podman/pods/create.go
@@ -155,6 +155,9 @@ func create(cmd *cobra.Command, args []string) error {
                if strings.Contains(share, "cgroup") && shareParent {
                        return errors.Wrapf(define.ErrInvalidArg, "cannot define the pod as the cgroup parent at the same time as joining the infra container's cgroupNS")
                }
+               if strings.HasPrefix(share, "+") {
+                       share = strings.Join([]string{specgen.DefaultKernelNamespaces, strings.TrimPrefix(share, "+")}, ",")
+               }
                createOptions.Share = strings.Split(share, ",")
                createOptions.ShareParent = &shareParent
                if cmd.Flag("infra-command").Changed {

Or maybe add a new flag --add-share/--share-add

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 4, 2022
@mheon mheon added the Good First Issue This issue would be a good issue for a first time contributor to undertake. label Mar 7, 2022
@markkrj
Copy link
Author

markkrj commented Mar 8, 2022

@mheon if you think that patch is OK, I can open a PR for it (plus docs)...

@mheon
Copy link
Member

mheon commented Mar 8, 2022

Sure, looks fine

@Luap99
Copy link
Member

Luap99 commented Mar 10, 2022

Maybe also add support for -pid to remove a namespace from the default list.

@giuseppe
Copy link
Member

would something like podman pod create --name test --share +pid,+uts work too?

@markkrj
Copy link
Author

markkrj commented Mar 22, 2022

would something like podman pod create --name test --share +pid,+uts work too?

With my patch (above), or you are proposing such behavior?

With patch above, that would fail, as it would strip only leading '+' from --share... but --share +pid,uts would work, even though, uts is already in default shares, so it is not needed. Only namespaces missing from --share are cgroup and pid, but (IIUC) cgroup is inherited from pod, so only pid would be needed.

@giuseppe
Copy link
Member

yes I was proposing something more generic

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Apr 22, 2022

@markkrj Did you continue to work on this. I think it is a good idea, are you going to open a PR?

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

rhatdan added a commit to rhatdan/podman that referenced this issue May 24, 2022
cdoern pushed a commit to cdoern/podman that referenced this issue May 27, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good First Issue This issue would be a good issue for a first time contributor to undertake. kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants