-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix running container from docker client with rootful in rootless podman #21487
Fix running container from docker client with rootful in rootless podman #21487
Conversation
5be393a
to
e189efb
Compare
e189efb
to
3e53617
Compare
Mmmm the build was ok when I submitted it, however in the meantime #21472 was merged and now this pull request fails to cross build for FreeBSD. Shall I add a dependency to |
you can just use |
Mmmh you are right about |
Because rootless.IsRootless() also looks at the UID and as such is a subset of unshare.IsRootless() |
…man. This effectively fix errors like "unable to upgrade to tcp, received 409" like containers#19930 in the special case where podman itself is running rootful but inside a container which itself is rootless. [NO NEW TESTS NEEDED] Signed-off-by: Romain Geissler <[email protected]>
3e53617
to
f59a5f1
Compare
@Luap99 Indeed thanks for the tip, I was able to test the current version of the pull request and it works as expected. CI is failing but I doubt it's related to this change. |
@Romain-Geissler-1A can you add a tests that emulates the issue you saw in the description? |
We do not test nested containers like this at all so it is impossible to actually test it properly right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, Romain-Geissler-1A 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 |
/lgtm |
This effectively fix errors like "unable to upgrade to tcp, received 409" like #19930 in the special case where podman itself is running rootful but inside a container which itself is rootless.
Here is how to reproduce my variant of #19930 which this pull request tries to fix:
With this pull request, the container can run, and a warning like:
is printed.
Not that the underlying issue is really the docker client hardcoding a wrong default of oom_score_adj=0, which I have just reported in docker/cli#4846.
Does this PR introduce a user-facing change?