- Notifications
You must be signed in to change notification settings - Fork2.9k
Fix: Rootless Podman-in-Podman on WSL#27412
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR isNOT APPROVED This pull-request has been approved by:dvorst The full list of commands accepted by this bot can be foundhere. Needs approval from an approver in each of these files: Approvers can indicate their approval by writing |
dd60c4b to9c4dbb1Compare[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
baude commentedOct 31, 2025
i think you should add a test to the PR that validates your change; otherwise, if we change how the image is built or someone makes an unknowing chnage, you could easily have a regression. wdyt ? also thanks for your diligence on this this PR. otherwise, this LGTM |
dvorst commentedOct 31, 2025
Sure, I can add a test. Had a quick look at the tests dir but it isn't clear to me at first sight where I should place it. Can you point me in the right direction? Maybe there are already tests that I can use to start from |
TomSweeneyRedHat commentedOct 31, 2025
TomSweeneyRedHat commentedOct 31, 2025
And code LGTM too |
e998a89 to3772effComparecloses:containers#27411Adjust SUB_UID and SUB_GID ranges to support running rootless Podman inside a rootless run Podman container.By default, a new user is assigned the following sub-ID ranges: SUB_UID_MIN=100000, SUB_GID_MIN=100000, SUB_UID_COUNT=65536, SUB_GID_COUNT=65536This means the user’s sub-UID and sub-GID ranges are 100000–165535.When the container is run rootless with the user defined below, ID mappings occur as follows:- Container ID 0 (root) maps to user ID 1000 on the host (which is the user created below).- Container IDs 1–65536 map to IDs 100000–165535 on host (the subid range previously mentioned).If a new user is created inside this container (to build containers for example), it willattempt to use the default sub-ID range (100000–165535). However, this exceeds the container’savailable ID mapping, since only IDs up to 65536 are mapped. This causes nested rootless Podmanto fail.To enable container-in-container builds, the sub-ID ranges for the user must be large enoughto provide at least 65536 usable IDs. A minimum SUB_UID_COUNT and SUB_GID_COUNT of 165536 isrequired, but 200000 is used here to provide additional margin.Signed-off-by: dvorst <87502756+dvorst@users.noreply.github.com>
d702dfc to67371a5Comparein pkg/machine/e2e/init_test.goSigned-off-by: dvorst <87502756+dvorst@users.noreply.github.com>
Luap99 left a comment
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.
You need to rebase your PR to pass the search failure in the integration tests (not related to your code).
And please squash both commits into one, we prefer code and test in the same commits.
| sed -ir 's/SUB_UID_COUNT.*/SUB_UID_COUNT 200000/' /etc/login.defs | ||
| sed -ir 's/SUB_GID_COUNT.*/SUB_GID_COUNT 200000/' /etc/login.defs |
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.
This is fixing this differently on WSL than compared to our providers which only set a range for the one user we create by writing subuid/subgid directly. So I would rather match that.
podman/pkg/machine/ignition/ignition.go
Lines 284 to 289 in69b397a
| subUID:=100000 | |
| subUIDs:=1000000 | |
| ifuid>=subUID&&uid< (subUID+subUIDs) { | |
| subUID=uid+1 | |
| } | |
| etcSubUID:=fmt.Sprintf(`%s:%d:%d`,usrName,subUID,subUIDs) |
In particular they use a range of1000000 uids not200000 which I think is important to keep the behaviours consistent between providers
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.
Thanks for the intermediate review.
I'll change the count to1 000 000 instead of200 000. I’m quite busy with work at the moment, but I might have time to work on this around Christmas.
I'll also modify/etc/subuid and/etc/subgid directly, right after the adduser command is run, to align it with theignition.go script.
My initial approach was based on the fact that adduser doesn’t provide an option to configure the sub-ID ranges, so these had to be set in a different way. Changing the defaults seemed to be a cleaner approach than modifying subuid/subgid directly, since that will give issues if adduser/useradd behavior ever changes (though i don't think that will ever change without backwards compatibility).
| It("init subid range check for rootless PinP",func() { | ||
| /* By default, a new user is assigned the following sub-ID ranges (see manual useradd): | ||
| * SUB_UID_MIN=100000, SUB_GID_MIN=100000, SUB_UID_COUNT=65536, SUB_GID_COUNT=65536 | ||
| * This means the default sub-UID and sub-GID ranges are 100000–165535. | ||
| * | ||
| * When the container is run rootless by the user in WSL, ID mappings occur as follows: | ||
| * Container ID 0 (root) maps to user ID on the host. | ||
| * Container IDs 1–65536 map to IDs 100000–165535 on host (range previously mentioned). | ||
| * | ||
| * If a new user is created inside the container and used to build containers with | ||
| * (rootless PinP), it will attempt to use the default sub-ID range (100000–165535). Given | ||
| * the mapping, this means that the host must at least have a SUB_UID_COUNT and | ||
| * SUB_GID_COUNT of 165536. Since 165536 would only allow rootless PinP for the first | ||
| * user (with ID 1000), the check is run against a count of 166536 (=165536+1000) as to | ||
| * provide additional margin. | ||
| */ |
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.
rootless PinP is not really relevant to this test at all, it could just be called check subuid/gid ranges.
But in any case adding new test cases is quite slow as they all require new Vm to be created. As such I would strongly advise to not a a new It() block for something simple as this. Instead just add a new ssh command at the end ofsimple init with username for example
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.
The end goal is to fix rootles PinP, but i'll make a batch script as you suggest instead.
| ifsshSession.ExitCode()!=0 { | ||
| Fail(fmt.Sprintf("SSH session failed with exit code %d\nstdout:\n%s\nstderr:\n%s", | ||
| sshSession.ExitCode(), | ||
| sshSession.outputToString(), | ||
| sshSession.errorToString(), | ||
| )) | ||
| } |
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.
That doesn't add anything so please don't do that.
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.
I was hoping I could create the test without having to set everything up locally. The test is failing at this point so i was hoping to print out more information to the log, without success though. It seems I have to set up everything locally, which will require resetting the podman machine as what I could tell, which is a bit cumbersome for me as I need it for work. Hopefully I have time for this around Christmas. I was not planning on leaving this in once all tests succeed
| // A user must exist in order to run podman rootless, a line in both subuid and subgid | ||
| // should exist for it, so 2 lines in total. | ||
| Expect(len(counts)).To(BeNumerically(">=",2),"expected at least 1 user/line in /etc/subuid and /etc/subgid each, got %d",len(counts)) | ||
| // Verify the count. At the moment only 1 user is created in the machine. If multiple users | ||
| // are ever created, this will check that all users have a sufficient subid range. | ||
| for_,count:=rangecounts { | ||
| n,err:=strconv.Atoi(count[1]) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(n).To(BeNumerically(">=",count_min),"expected last number %d to be >= %d",n,count_min) | ||
| } |
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.
There should be no reason to harden that for multiple user we only ever create one and should not create multiple. If that ever were to be the case we can adapt tests later. For now I don't see the value in complicating the check for this.
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.
There is a count for the user and group id, so there will be a for loop anyway. The comment is meant to be informative for if a second user is ever created, and that this test will basically be run for all users.
Uh oh!
There was an error while loading.Please reload this page.
Fixes:#27411
pkg/machine/wsl/declares.go
Adjust SUB_UID and SUB_GID ranges to support running rootless Podman inside a rootless run Podman container.
By default, a new user is assigned the following sub-ID ranges:
SUB_UID_MIN=100000, SUB_GID_MIN=100000, SUB_UID_COUNT=65536, SUB_GID_COUNT=65536
This means the user’s sub-UID and sub-GID ranges are 100000–165535.
When the container is run rootless by the user in WSL, ID mappings occur as follows:
If a new user is created inside this container (to build containers for example), it will attempt to use the default sub-ID range (100000–165535). However, this exceeds the container’s available ID mapping, since only IDs up to 65536 are mapped, causing rootless PinP to fail.
To enable container-in-container builds, the sub-ID ranges for the user in WSL must be large enough to provide at least 65536 usable IDs. A minimum SUB_UID_COUNT and SUB_GID_COUNT of 165536 is thus required, but 200000 is used here to provide additional margin.
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. SeeCONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?