rootless: new function to join existing conmon processes #3188
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[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:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
approved
giuseppe
force-pushed
the
fix-join-existing-containers
branch
6 times, most recently
from
f3a6b8f to
94cb4df
Compare
|
Tested 6bca80bdfebf9d390ce1a9786ae6101c87f1322d for now, WFM, thanks! |
|
@jistr thanks for confirming it! |
|
LGTM |
1 similar comment
|
LGTM |
giuseppe
force-pushed
the
fix-join-existing-containers
branch
from
94cb4df to
7782919
Compare
|
☔ The latest upstream changes (presumably #3196) made this pull request unmergeable. Please resolve the merge conflicts. |
block signals for the pause process, so it can't be killed by mistake. Signed-off-by: Giuseppe Scrivano <[email protected]>
move the logic for joining existing namespaces down to the rootless package. In main_local we still retrieve the list of conmon pid files and use it from the rootless package. In addition, create a temporary user namespace for reading these files, as the unprivileged user might not have enough privileges for reading the conmon pid file, for example when running with a different uidmap and root in the container is different than the rootless user. Closes: containers#3187 Signed-off-by: Giuseppe Scrivano <[email protected]>
otherwise the processes we leave around will be killed once the session terminates. Signed-off-by: Giuseppe Scrivano <[email protected]>
since we now enter the user namespace prior to read the conmon.pid, we can write the conmon.pid file again to the runtime dir. This reverts commit 6c6a865. Signed-off-by: Giuseppe Scrivano <[email protected]>
as it is used only by the rootless package now. Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
fix-join-existing-containers
branch
from
7782919 to
153503e
Compare
|
let's get this merged if there are no objections, so we have time to play with it before the release |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
move the logic for joining existing namespaces down to the rootless package. In main_local we still retrieve the list of conmon pid files and use it from the rootless package.
In addition, create a temporary user namespace for reading these files, as the unprivileged user might not have enough privileges for reading the conmon pid file, for example when running with a different uidmap and root in the container is different than the rootless user.
Closes: #3187
Signed-off-by: Giuseppe Scrivano [email protected]