Conversation
Digest only exists in podman. RepoDigests exists in both podman and docker.
64062e8 to
2f1d8c3
Compare
VERBOSE=3 already shows the container environment variables and command.
|
Down the rabbit hole we go! 🐰 |
netbrain
left a comment
There was a problem hiding this comment.
Thanks for the thorough writeup and the work here. A few inline issues flagged below, plus some architectural thoughts.
I've been looking at this alongside PR 316 (volume variant), which solves the same non-1000 uid/gid pain point from a different angle — by persisting /home/user as a volume so ownership is written once and never needs per-launch chown or remapping.
The rootless container change here (USER directive, dropping gosu/sudo) is a genuine security improvement and I think it's valuable on its own. My concern is with the runtime UID remapping machinery — generating temp Dockerfiles, running docker build at launch, spinning up ephemeral containers for volume ownership checks. That's a lot of moving parts that can fail in ways that are hard to debug for end users, and it still has per-launch overhead.
I think the ideal path might be to split the rootless container change from the runtime remapping. The rootless Dockerfile changes could land independently, and the uid mismatch problem might be better solved by volume persistence than runtime remapping.
Thoughts?
In that sense the two implementations offer the same benefit. Remapping only needs tot be done once after each image update.
There's always going to be some sort of runtime UID remapping machinery. We don't know the UID/GID of the host user, so the container user has to be remapped to the host user some way or another.
The remapping has to be done either in the entrypoint script or in the zwift script. Since this implementation runs the entrypoint script as user instead or root, the remapping can no longer be done in the entrypoint script. In both this implementation and #316 ephemeral containers are used to check the version and to do the remapping if needed. The temporary dockerfile approach to simply update the container user UID/GID seems far less fragile to me than attempting to rsync the updated zwift install into a persistent volume. The dockerfile uses the same commands that were used in entrypoint before (should_change_user_ids part, which is still executed in the volume variant). The second part of the remapping is to make sure the volume has the correct permissions. It is required for the two reasons mentioned in the OP. The container no longer starts as root, but the volume is still automatically created as root, so we need to reown it as user. And second it is possible that the user decides to change his account UID/GID, in that case the volume also has to be updated. If the user changes his UID/GID in the volume variant, it would trigger the change_user_ids funtion. Assuming the volume is owned by the current container user UID/GID, the usermod and groupmod would automatically update the volume. Note that when changing the ownership inside entrypoint, it will also change ownership of other mounted directories and files which are mounted to subdirectories of the volume (whether it is /home/user or just AppData/Local/Zwift). These are files and directories on the host. The implementation in this PR does not have that problem (since it only mounts the volume itself when changing ownership). Note that when running zwift with UID/GID not equal to 1000:1000, the volume variant still runs the change_user_ids code in the entrypoint script each time the container is launched. The implementation in this PR persists the UID/GID change into a new image, so it only has to run once (each time the original image is updated).
The per-launch overhead is neglibible. Normal usage:
Special case, user changes UID/GID:
Special case, user uses local image instead of image hosted on container registry:
Comparing runtime overhead of this implementation vs the volume variant from #316:
With a rootless container we can't do any remapping in the entrypoint script anymore. So we can't really separate those two.
Well, to be frank: the volume variant is doing more runtime remapping than the implementation in this PR. 🙈
Yes 😅. I really don't think we should persist the entire user home in a volume. If anything changes to the directory structure (in the zwift install or elsewhere), this could easily break. The zwift install also isn't user local data, it's image data. I'm all for looking into adding a second volume for other user data if that can improve performance, but in my opinion the zwift install should always be part of the image. A bit off topic, but I did a quick check to see where the shader cache is stored. On my system with NVIDIA graphics card, a |
| # No longer supported configuration environment variables | ||
| readonly ZWIFT_UID="${ZWIFT_UID:-}" | ||
| readonly ZWIFT_GID="${ZWIFT_GID:-}" | ||
| if [[ -n ${ZWIFT_UID} ]] || [[ -n ${ZWIFT_GID} ]]; then | ||
| msgbox error "ZWIFT_UID and ZWIFT_GID options are no longer supported" | ||
| msgbox error " To start Zwift as a different user, use: sudo -i username zwift" | ||
| exit 1 |
There was a problem hiding this comment.
We need to figure out something better here. Obviously it's not as simple as just running sudo -u username zwift. On XOrg that probably works after running xhost +si:localuser:username, but on Wayland it's much more involved.
Do we know of end users who actually use the ZWIFT_UID/ZWIFT_GID feature?
Can we get feedback from them?
There was a problem hiding this comment.
Anyone who has uid/gid other than 1000/1000 i assume. I have two accounts on my main zwift machine, my user account is 1000/1000, and my wife's is 1001:1001 (which means she has to wait a minute or two extra for the container to launch 😈).
But none of us have used zwift actively as of late due to other life responsibilities. 👎
Previously i simply had a single user account 1000/1000 and used USER=user1 zwift and USER=user2 zwift to switch between, which arguably worked better.
I've been pondering if supporting multiple containerizations solution is the wrong way forward, maybe choose one? maybe that isn't docker or even podman, maybe its lxc container, or maybe even something totally different, like snaps, flatpak, appimage. I don't know...
On the flipside, some users have found docker to work, while having trouble with podman, and some probably the other way around.. So there's really no guarantee that we find a isolation technology that would be suitable for all, that was also my reason for having the #28 issue, to provide alternatives to those who don't fit the bill.
So yeah, down the rabbit hole we go 🐇 💯 😅
There was a problem hiding this comment.
Anyone who has uid/gid other than 1000/1000 i assume. I have two accounts on my main zwift machine, my user account is 1000/1000, and my wife's is 1001:1001 (which means she has to wait a minute or two extra for the container to launch 😈).
If she logged into her user account and started zwift from there she'd not be using the ZWIFT_UID/ZWIFT_GID feature. Since they would be automatically set to the ids of her account. Which is supported and streamlined by this PR.
With using the ZWIFT_UID/ZWIFT_GID feature I meant setting them to a different value than the logged in account. An example would be you and your wife zwifting at the same time on the same computer.
- You'd start your zwift instance normally (just run zwift)
- And start your wife's zwift instance as
ZWIFT_UID=1001 ZWIFT_GID=1001 zwift
I think that always had issues since not everything would get mounted with the same ids and you'd need to manually update permissions for some files to make it work. Especially the XDG_RUNTIME_DIR and related stuff I think.
When starting two instances at the same time, indeed using the USER option is probably a more stable and reliable option.
There was a problem hiding this comment.
I've been pondering if supporting multiple containerizations solution is the wrong way forward, maybe choose one? maybe that isn't docker or even podman, maybe its lxc container, or maybe even something totally different, like snaps, flatpak, appimage. I don't know...
I think we have a good solution going here with the zwift container image. Podman seems to be better at doing what we need out of the box with its support for remapping the user. But we got things working with docker, so we might as well keep supporting it.
The UID/GID implementation in this PR brings feature parity between docker and podman. I think it's the closest we can get to make the docker remapping behave the same as the automatic podman remapping.
It comes at the cost of allowing the user to specify a UID/GID different from the one of the linux account from which zwift is started. But that feature always behaved differently on docker and podman (did it even work on podman?) and was fragile with docker at best.
There was a problem hiding this comment.
So yeah, down the rabbit hole we go 🐇 💯 😅
I think we should stick with the containerized solution we got and try to make that as good as we can. And I think there's a few things we can do to make it more reliable and future proof:
- Improve the handling of UID/GID remapping, what we are working on now
- Make docker behave the same as podman
- Possibly find an alternative for manually setting a UID/GID different from those of the current linux account. Perhaps we could look into creating a sudo_zwift.sh helper script that could be used to launch zwift as a different user account that starts a proper dbus session, sets up a wayland socket, whatever is needed.
- Update to the latest wine and winetricks version (and keep them up to date)
- WiP: Update wine to latest version glennvl/zwift-linux#14
- Newer wine versions default to EGL instead of GLX
- We need to figure out how to make EGL work in the container
- I'm kind of stuck here, I don't know enough about EGL to make this work, I need help!
- I only tried with nvidia
- XWayland doesn't work at all
- The launcher fails to completion with Wayland
- The game works with Wayland (but we know wine Wayland still has issues)
- I tried forcing wine to use GLX instead
- The launcher does not work with GLX at all
- The game works with GLX and XWayland
- Allow bluetooth access in the container
- WiP: Allow container access to host bluetooth glennvl/zwift-linux#15
- Install the bluez driver and add some extra arguments, easy
- That way we are ready when bluetooth starts working in wine!
- Fix wine dependencies where we can
- Especially during installation wine spams a lot about missing stuff
- Look if there's anything we can fix by installing missing dependencies
- Improve documentation
- We got documentation site on github pages, great!
- Now we just got to improve the documentation itself
- Release management
- Updates are done from master, we don't have any official releases
- I think that ship has sailed, so just leave that as is
- But we could perhaps add a second unstable branch
- WiP: Unstable branch glennvl/zwift-linux#16
- Allow user to opt into unstable branch for updates
- Push fixes etc immediately to master
- Push major changes (for example a wine update) to the unstable branch first
That's quite the list, and that's mostly stuff I've already been working on. So there's plenty of hole to fill before we start digging entirely new holes to support other ways of using zwift on linux. 🐰 🐇 🕳️
Just my opinion of course. 🙈 😅
Co-authored-by: Glenn <glenn.vanloon@gmail.com>
Related to #315.
Summary
This PR refactors how we deal with UIDs and GIDs.
Current implementation:
New implementation:
Implementation
Exit with error when ZWIFT_UID or ZWIFT_GID are set
We now remap the container user to the host user. This does not work with custom ids.
If the end user has a ZWIFT_UID or ZWIFT_GID set, report an error that they are no longer supported and exit the zwift script. The end user has to remove these configuration options to be able to continue using zwift.
Building the image locally using build-image.sh
The build script is fragile when the host user has a UID/GID that is not equal to 1000:1000. For example the XAUTHORITY cookie may not work, resulting in the installer not having access to X11. This problem is exagerated if the end user manually sets a ZWIFT_UID/ZWIFT_GID. Currently the only safe way to use the build script is with a host user that has UID/GID 1000:1000.
Since we know the UID/GID in advance, we might as well take advantage of it and pass them as build argument to the Dockerfile. We immediately create the container user with the correct UID/GID. This in combination with no longer allowing the end user to specify a ZWIFT_UID/ZWIFT_GID makes it safe to use the build script on a local user with UID/GID not equal to 1000:1000.
Don't launch container as root, use user instead
The following changes were made to the container user:
Changing ownership: Remapping the container user to the host user
Since the entrypoint script now runs as user instead of root, it can't perform a chown. We now remap the container to the host user in the zwift script instead.
The remapping is done in two stages.
1. Remap the container user to the host user by changing its UID/GID to be the same as the host user UID/GID.
We perform this remapping using a simple dockerfile that starts from the requested IMAGE and VERSION (default: docker.io/netbrain/zwift). The dockerfile temporarily switches the USER to root to execute the same commands that used to be executed in the entrypoint script. These commands change the container user UID/GID to be the same as the host user UID/GID. Before switching the USER back to user. The dockerfile is generated on the fly when needed.
A label is also attached with the repository image digest of the requested zwift image IMAGE:VERSION. The remapping only has to be done if the image digest changed (IMAGE:VERSION changed).
The freshly built image is tagged as
netbrain/zwift:remapped_user_$UID_$GIDand used to launch zwift instead of IMAGE:VERSION. Including the UID/GID in the tag name is important for several reasons:2. Make sure the ownership of the zwift-$USER volume is correct
We also need to make sure the UID/GID of the zwift documents directory, mounted from the zwift-$USER volume, is correct. To check its UID/GID, we mount it in a temporary container. If the UID/GID turns out to be wrong, another temporary container is launched, this time with root user, and a recursive chown is performed.
Check if the zwift-$USER volume UID/GID is correct:
docker run \ --rm \ -it \ -v zwift-$USER:/zwift-docs \ --entrypoint bash \ netbrain/zwift:remapped_user_$UID_$GID \ -c "[[ ! -O /zwift-docs ]] || [[ ! -G /zwift-docs ]]"Update the zwift-$USER volume UID/GID:
docker run \ --rm \ --user root \ -it \ -v zwift-$USER:/zwift-docs \ --entrypoint bash \ netbrain/zwift:remapped_user_$UID_$GID \ -c "chown -R $UID:$GID /zwift-docs"The check whether the UID/GID of the volume are correct is always performed. There are two situations where remapping is actually required:
Other changes
Just two minor changes I smuggled into this PR: