rootless: use a pause process to keep namespaces alive #3084

giuseppe · 2019-05-08T12:16:55Z

use a pause process to keep the user and mount namespace alive.

The pause process is created immediately on reload, and all successive Podman processes will refer to it for joining the user&mount namespace.

This solves all the race conditions we had on joining the correct namespaces using the conmon processes.

As a fallback if the join fails for any reason (e.g. the pause process was killed), then we try to join the running containers as we were doing before.

This PR also adds an alternative way of joining the namespaces. We can now attempt to join them as soon as Podman starts, before the Go runtime kicks in, so that we don't need to re-exec and use just one process 🎉

This version doesn't try to recover and re-create a pause process if for any reason it terminated, but prints an error. This is not trivial to recover from, as there can be more podman processes already running and will require some sort of locking.

openshift-ci-robot · 2019-05-08T12:16:57Z

[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

giuseppe · 2019-05-13T16:09:45Z

tests are passing here, @mheon if there are no objections, I'd like to get this into the next Podman release so that we have some time to play with it before 8.1

mheon · 2019-05-13T16:39:51Z

I'm reluctant to get this into 1.3.1, which is supposed to be a small bugfix release... Maybe we cut a 1.4 very soon (before end of month) to get this out quickly?

mheon · 2019-05-13T16:41:27Z

cmd/podman/main_local.go

@@ -113,6 +114,35 @@ func setupRootless(cmd *cobra.Command, args []string) error {
 		MainGlobalOpts,
 		remoteclient,
 	}
+
+	pausePidPath, err := util.GetRootlessPauseProcessPidPath()


Can we start moving things like this block of code out of cmd/podman and consolidating them someplace? Maybe pkg/rootless? I want to make it easier to integrate this into other Libpod consumers

yes, I agree. The only reason I've left it here for now, is that the logic for detecting running containers and try to join their namespaces (and this is particularly important to not break rootless containers once again on an upgrade) cannot be easily moved around.

I had a look at it now, but there are parts that cannot be easily moved down to the pkg/rootless package. For example we depend on util.GetRootlessRuntimeDir() and in the case of not having a pause process running, we need to retrieve a list of conmon processes.

rh-atomic-bot · 2019-05-17T17:54:38Z

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

this leaves the containers stopped but we won't risk to use the wrong user namespace. Signed-off-by: Giuseppe Scrivano <[email protected]>

use a pause process to keep the user and mount namespace alive. The pause process is created immediately on reload, and all successive Podman processes will refer to it for joining the user&mount namespace. This solves all the race conditions we had on joining the correct namespaces using the conmon processes. As a fallback if the join fails for any reason (e.g. the pause process was killed), then we try to join the running containers as we were doing before. Signed-off-by: Giuseppe Scrivano <[email protected]>

add a shortcut for joining immediately the namespace so we don't need to re-exec Podman. With the pause process simplificaton, we can now attempt to join the namespaces as soon as Podman starts (and before the Go runtime kicks in), so that we don't need to re-exec and use just one process. Signed-off-by: Giuseppe Scrivano <[email protected]>

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

giuseppe · 2019-05-21T07:46:56Z

@mheon @rhatdan any other comment on this PR or could we merge it?

mheon · 2019-05-21T17:18:11Z

pkg/rootless/rootless_linux.c

+
+          /* There can be another process at this point trying to configure the user namespace and the pause
+           process, do not override the pid file if it already exists. */
+          if (renameat2 (AT_FDCWD, tmp_file_path, AT_FDCWD, pause_pid_file_path, RENAME_NOREPLACE) < 0)


So this doesn't really look racy, but I think it's still worth asking: should we consider a file lock here to ensure only one process is trying to make the PID file and become the init process?

I tried to avoid the lock using only a renameat2(RENAME_NOREPLACE). If the file already exists, then it means another process managed to create it faster as it wasn't there earlier. If we fail at this point then Podman attempts to join the pause process instead (which we assume was created by the process who won the race).

If the file exists but the process is not running, then Podman raises an error and asks the user to fix it by deleting the file. We can improve this in future, but for now I preferred to have a clear error message

mheon · 2019-05-21T17:18:39Z

One question, but overall I think we're pretty good to merge. LGTM

baude · 2019-05-21T19:40:32Z

/lgtm

rufoa · 2019-05-27T20:41:58Z

@giuseppe
I'm running into a problem which I believe is related to this change.

It appears that pause.pid is not being deleted after use, so next time I run podman I get errors like this:

cannot join pause process pid 18740.  You may need to remove /run/user/111/libpod/pause.pid and stop all containers
you can use `system migrate` to recreate the pause process
open /proc/18740/ns/user: no such file or directory

note:

process 18740 had indeed exited (cleanly, I believe), but the pid file was left behind
I'm running podman rootlessly as uid 111
Deleting pause.pid resolves the problem temporarily, but then the same problem reoccurs next run

I'm still trying to figure out the root cause. It occurs reliably every time a particular CI job of mine runs, which executes various podman commands in quick succession (build, run, top, inspect, rm).

Do you have a hunch what might be the cause of this, so I can investigate further? Where in podman is the call which is supposed to delete pause.pid? Many thanks.

mheon · 2019-05-27T23:23:21Z

The pause process does not appear to be pausing - which is our problem. If it's exiting cleanly, it's not doing its job of living as long as a user's session to preserve namespaces

giuseppe · 2019-05-28T07:36:17Z

the pause process should be left alive after the first execution. It is implemented to avoid some race conditions trying to join the existing namespaces.

Does it exist immediately after you run podman? What do you see if you run something like the following?

$ podman images
$ pgrep -fa pause

If you exit the current user session, the pause process might be killed by systemd. There is a fix for this issue in: #3188

In particular this patch: 30ef6ba

You can try it by yourself with loginctl enable-linger $UID and see if it makes any difference

rufoa · 2019-05-28T15:33:42Z

Thanks for the info! It appears to work correctly now.

I had to manually enable-linger for my user as root, because Debian appears not to install policykit by default, so non-root users cannot enable-linger for themselves.

rufoa · 2019-05-28T15:35:37Z

Perhaps the deb package should recommend/suggest policykit-1 as a dependency

giuseppe · 2019-05-28T15:46:11Z

@lsm5 can we add that dependency?

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 May 8, 2019

openshift-ci-robot requested review from mrunalp and umohnani8 May 8, 2019 12:16

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L labels May 8, 2019

giuseppe force-pushed the rootless-pause-process branch 8 times, most recently from 0353ec2 to 3d4f40f Compare May 8, 2019 20:28

openshift-ci-robot added size/XL and removed size/L labels May 8, 2019

giuseppe force-pushed the rootless-pause-process branch 3 times, most recently from 20f2b54 to 9c8ffc5 Compare May 9, 2019 09:24

giuseppe changed the title ~~[WIP] rootless: introduce a pause process to keep namespaces alive~~ rootless: introduce a pause process to keep namespaces alive May 9, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 9, 2019

giuseppe changed the title ~~rootless: introduce a pause process to keep namespaces alive~~ rootless: use a pause process to keep namespaces alive May 9, 2019

giuseppe force-pushed the rootless-pause-process branch 3 times, most recently from 3766e4f to e80d342 Compare May 9, 2019 13:41

giuseppe changed the title ~~rootless: use a pause process to keep namespaces alive~~ [WIP] rootless: use a pause process to keep namespaces alive May 9, 2019

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 May 9, 2019

giuseppe force-pushed the rootless-pause-process branch 3 times, most recently from 74fe761 to 7d7f7cb Compare May 9, 2019 19:19

giuseppe force-pushed the rootless-pause-process branch 2 times, most recently from dfa45eb to a8f585c Compare May 10, 2019 08:17

giuseppe changed the title ~~[WIP] rootless: use a pause process to keep namespaces alive~~ rootless: use a pause process to keep namespaces alive May 10, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2019

giuseppe force-pushed the rootless-pause-process branch from a8f585c to e3a2152 Compare May 13, 2019 08:48

mheon reviewed May 13, 2019

View reviewed changes

giuseppe added 5 commits May 17, 2019 20:47

migrate: not create a new namespace

2e0fef5

this leaves the containers stopped but we won't risk to use the wrong user namespace. Signed-off-by: Giuseppe Scrivano <[email protected]>

system: migrate stops the pause process

9dabb16

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

troubleshooting.md: add note about updating subuid/subgid

53a7622

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

giuseppe force-pushed the rootless-pause-process branch from e3a2152 to 53a7622 Compare May 17, 2019 18:48

mheon reviewed May 21, 2019

View reviewed changes

openshift-ci-robot assigned baude May 21, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2019

openshift-merge-robot merged commit 536fd6a into containers:master May 21, 2019

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rootless: use a pause process to keep namespaces alive #3084

rootless: use a pause process to keep namespaces alive #3084

giuseppe commented May 8, 2019 •

edited

Loading

openshift-ci-robot commented May 8, 2019

giuseppe commented May 13, 2019

mheon commented May 13, 2019

mheon May 13, 2019

giuseppe May 13, 2019

giuseppe May 17, 2019

rh-atomic-bot commented May 17, 2019

giuseppe commented May 21, 2019

mheon May 21, 2019

giuseppe May 21, 2019

mheon commented May 21, 2019

baude commented May 21, 2019

rufoa commented May 27, 2019

mheon commented May 27, 2019

giuseppe commented May 28, 2019

rufoa commented May 28, 2019

rufoa commented May 28, 2019

giuseppe commented May 28, 2019

rootless: use a pause process to keep namespaces alive #3084

rootless: use a pause process to keep namespaces alive #3084

Conversation

giuseppe commented May 8, 2019 • edited Loading

openshift-ci-robot commented May 8, 2019

giuseppe commented May 13, 2019

mheon commented May 13, 2019

mheon May 13, 2019

Choose a reason for hiding this comment

giuseppe May 13, 2019

Choose a reason for hiding this comment

giuseppe May 17, 2019

Choose a reason for hiding this comment

rh-atomic-bot commented May 17, 2019

giuseppe commented May 21, 2019

mheon May 21, 2019

Choose a reason for hiding this comment

giuseppe May 21, 2019

Choose a reason for hiding this comment

mheon commented May 21, 2019

baude commented May 21, 2019

rufoa commented May 27, 2019

mheon commented May 27, 2019

giuseppe commented May 28, 2019

rufoa commented May 28, 2019

rufoa commented May 28, 2019

giuseppe commented May 28, 2019

giuseppe commented May 8, 2019 •

edited

Loading