WIP: Add support for pinned namespaces #5210

saschagrunert · 2020-02-14T12:41:44Z

The idea is to pin the namespaces to a dedicated on-disk location by not having the need for a pause container any more (analogous to the CRI-O implementation).

This is still in WIP and would need documentation, testing and some pre-defined design decisions.

Demo

What seems to work right now, when a pinns exectuable (from CRI-O pinns or pinns.rs) is in $PATH.

Namespace life-cycle handling

> sudo ./bin/podman pod create -n mypod -i
b73890f9b3143b66a7b78dc2755fc36df1efd2989a3b759448448ecf72d75164

> sudo ls /var/run/libpod/ns/b73890f9b3143b66a7b78dc2755fc36df1efd2989a3b759448448ecf72d75164
ipc  net  uts

> sudo ./bin/podman pod rm mypod
b73890f9b3143b66a7b78dc2755fc36df1efd2989a3b759448448ecf72d75164

> sudo ls /var/run/libpod/ns/b73890f9b3143b66a7b78dc2755fc36df1efd2989a3b759448448ecf72d75164
ls: cannot access '/var/run/libpod/ns/b73890f9b3143b66a7b78dc2755fc36df1efd2989a3b759448448ecf72d75164': No such file or directory

Containers in pods

> sudo ./bin/podman pod create -n mypod -i
f16880f174021ed71861213b4ec4ed64ddc2bbe5de09405efb2a1f32b607fdee

> sudo ./bin/podman run --pod mypod -d alpine sleep infinity
26b52409249670abccc929e7e9a94eae561a888cac3e2765f5052a322070e339

> sudo ./bin/podman run --pod mypod -d alpine sleep infinity
07637abb5078d43bb6679eea14436ebb8586d0634c22ee06d568cd2a97db721e

> sudo ./bin/podman run --pod mypod -d alpine sleep infinity
fe6564f40fdbc27f57f33728eeed156f89891b4e7ccd84606207d46d7310ac14

> sudo ./bin/podman ps --ns
CONTAINER ID  NAMES           PID    CGROUPNS    IPC         MNT         NET         PIDNS       USERNS      UTS
fe6564f40fdb  charming_cerf   15084  4026531835  4026541537  4026541669  4026541540  4026541670  4026531837  4026541469
07637abb5078  laughing_nash   14929  4026531835  4026541537  4026541667  4026541540  4026541668  4026531837  4026541469
26b524092496  vibrant_wright  14281  4026531835  4026541537  4026541665  4026541540  4026541666  4026531837  4026541469

Still TODO

Documentation
Testing
Find a way to provide pinns here

openshift-ci-robot · 2020-02-14T12:41:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: saschagrunert
To complete the pull request process, please assign mheon
You can assign the PR to them by writing /assign @mheon in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

saschagrunert · 2020-02-14T12:51:06Z

libpod/runtime_pod_namespaces_linux.go

Might clash with #5209

Yes. We want to get rid of direct dependencies on docker/docker.

haircommander · 2020-02-14T13:58:42Z

cmd/podman/pod_create.go

for now, the "instead of using an infra container" isn't quite correct. I would want to update the description once we can actually drop the infra container

oh I see that's the eventual intention of this PR, nevermind 😄

vrothberg

Really cool idea!

vrothberg · 2020-02-14T14:17:25Z

libpod/runtime_pod_namespaces_linux.go

Can we use the stdlib instead?

Thinking about it: couldn't we do everything in a go package with C bindings? github.com/containers/libpod/pkg/pinns? That would be faster, easier to maintain (as most folks don't speak rust unfortunately), and there would be no need to package a new tool for all distributions.

cgo isn't a great option, because the go runtime and locking threads is funky. We thought about this approach, but ultimately decided exec'ing the binary was a more guarenteed way of preventing an interrupt from changing the context too much. here's more info https://github.com/containernetworking/plugins/tree/master/pkg/ns

That's an omnipresent problem in go but locking the thread has proven to work. I didn't yet buy the idea of adding another dependency on top.

Just saw https://github.com/cri-o/cri-o/tree/master/pinns now. Easier than Rust. I try to catch up with the discussion in the CRI-O PR.

Don’t take my Rust stuff too serious, we had a hackweek and I’m totally open to stick to the C version if that’s the preferred one. 🙂

I was at the same point in thinking to unshare+pin as subcommand rather than having another binary, but now I see totally the arguments against something like that. 🤔

Which kinds of namespaces do we want to unshare beside NET, IPC and UTS? What about PID (dedicated feature called “pns“ in k8s), CGROUP and the USER (permissions) ns?

rh-atomic-bot · 2020-02-14T19:14:00Z

☔ The latest upstream changes (presumably #5203) made this pull request unmergeable. Please resolve the merge conflicts.

mheon · 2020-02-14T21:11:17Z

I think we need to talk about cgroup... My initial impression is yes, though.
User and PID should be supported, but opt-in - I think you should have to explicitly tell podman pod create you want to share those.

haircommander · 2020-02-14T21:15:39Z

I think we need to talk about cgroup... My initial impression is yes, though.
User and PID should be supported, but opt-in - I think you should have to explicitly tell podman pod create you want to share those.

the conversation of a pod level cgroupns hasn't been resolved in k8s, it's kind of been deferred until cgroupsv2 comes along more. As such, I could see it being useful to wire up the capability in podman so we don't have to add it later and fuss with different versions of pinns

rhatdan · 2020-02-15T12:23:10Z

This seems to be a containers.conf decision, so Distros/Administrators can make the decision.

Signed-off-by: Sascha Grunert <[email protected]>

saschagrunert · 2020-02-19T08:21:44Z

This seems to be a containers.conf decision, so Distros/Administrators can make the decision.

I agree.

Do we want to put pinns into a dedicated project, I'd be happy to support you with that but I do not have the power 😁

Maybe @vrothberg can support here as well? Are there any open discussion points regarding pinns?

vrothberg · 2020-02-19T10:27:40Z

Do we want to put pinns into a dedicated project, I'd be happy to support you with that but I do not have the power grin

Maybe @vrothberg can support here as well? Are there any open discussion points regarding pinns?

I would love to prevent us from having (yet) another dependency and project. They need to be setup, maintained and packaged; all causing additional work for upstream and downstream.

Hope dies last :) Why can't we have a go package with C bindings doing the work? @haircommander mentioned the common issues regarding cgo, thread locking in go etc. but is there a concrete technical reason/blocker not to go with go? If there's a clear blocker, then I stop nagging :^)

pinns doesn't do much and is really small, which would hurt even more to create a new project etc. In case we need to go with a dedicated binary, I prefer to move it over to libpod at some point. This way, we spare us from creating a new project, setting up CI, wiring that into libpod etc. With the CRI-O/libpod migration, I believe that libpod might be a better home for it in the long term.

rh-atomic-bot · 2020-02-19T16:31:29Z

☔ The latest upstream changes (presumably #5225) made this pull request unmergeable. Please resolve the merge conflicts.

openshift-ci-robot · 2020-02-19T16:31:36Z

@saschagrunert: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

github-actions · 2020-03-21T00:04:33Z

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

rhatdan · 2020-03-21T11:28:19Z

@saschagrunert @haircommander Any movement on this?

saschagrunert · 2020-03-23T11:06:11Z

Closing this for now since I think main questions like how we want to distribute pinns are not solved yet.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2020

openshift-ci-robot requested review from baude and jwhonce February 14, 2020 12:41

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 14, 2020

saschagrunert mentioned this pull request Feb 14, 2020

Update pinns build and add small cleanups cri-o/cri-o#3281

Merged

saschagrunert force-pushed the pinned-namespaces branch from b0273c2 to 542b54a Compare February 14, 2020 12:50

saschagrunert commented Feb 14, 2020

View reviewed changes

haircommander reviewed Feb 14, 2020

View reviewed changes

vrothberg reviewed Feb 14, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2020

saschagrunert added 2 commits February 17, 2020 12:56

Add support for pinned namespaces via pinns

3499deb

Signed-off-by: Sascha Grunert <[email protected]>

Make pod network setup work when pinning namespaces

61ae08c

Signed-off-by: Sascha Grunert <[email protected]>

saschagrunert force-pushed the pinned-namespaces branch from 8cc4dfb to 61ae08c Compare February 17, 2020 11:57

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 17, 2020

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 19, 2020

github-actions bot added the stale-pr label Mar 21, 2020

saschagrunert closed this Mar 23, 2020

saschagrunert deleted the pinned-namespaces branch March 23, 2020 11:06

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 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023

WIP: Add support for pinned namespaces #5210

WIP: Add support for pinned namespaces #5210

Uh oh!

Conversation

saschagrunert commented Feb 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Demo

Namespace life-cycle handling

Containers in pods

Still TODO

Uh oh!

openshift-ci-robot commented Feb 14, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vrothberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rh-atomic-bot commented Feb 14, 2020

Uh oh!

mheon commented Feb 14, 2020

Uh oh!

haircommander commented Feb 14, 2020

Uh oh!

rhatdan commented Feb 15, 2020

Uh oh!

saschagrunert commented Feb 19, 2020

Uh oh!

vrothberg commented Feb 19, 2020

Uh oh!

rh-atomic-bot commented Feb 19, 2020

Uh oh!

openshift-ci-robot commented Feb 19, 2020

Uh oh!

github-actions bot commented Mar 21, 2020

Uh oh!

rhatdan commented Mar 21, 2020

Uh oh!

saschagrunert commented Mar 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

saschagrunert commented Feb 14, 2020 •

edited

Loading