Skip to content
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: allow envbuilder on sysbox container runtime #51

Closed
wants to merge 3 commits into from

Conversation

janLo
Copy link
Contributor

@janLo janLo commented Aug 16, 2023

This fixes #50 by temporary bind-mounting all readonly mounts within the MagicDir to keep them out of the way for kaniko.

After kaniko finished it's build, the original mountpoints are restored at their original location.

@janLo janLo force-pushed the sysbox-issues branch 2 times, most recently from 97d3ff1 to 42c7956 Compare August 16, 2023 22:19
@janLo
Copy link
Contributor Author

janLo commented Aug 16, 2023

It probably needs a lot of polishing as I'm not native to the go ecosystem. I'm sorry about that. But I can confirm that it allows to successfully use envbuilder on top of a sysbox runtime.

This fixes coder#50 by temporary bind-mounting all readonly mounts within
the MagicDir to keep them out of the way for kaniko.

After kaniko finished it's build, the original mountpoints are restored
at their original location.

Signed-off-by: Jan Losinski <[email protected]>
@janLo
Copy link
Contributor Author

janLo commented Aug 17, 2023

To give a bit more of an explanation here:

The base-image I'm using in the envbuilder-dockerfile (debian:bookworm) contains a symlink from /lib to /usr/lib/ and kaniko tries to create this. To make way for this symlink, kaniko tries to remove /lib in the image - but lib is there because sysbox mounts /lib/modules/<kernelversion> read-only in the container. Removing the ro mount-point fails, of course.

I first attempted to add all ro mount points to the ignore list of kaniko - but this means that /lib/modules/<kernelversion> is on the ignore-list and this does not match the removal of /lib - neither with exact match nor as a prefix. I also think that adding /lib to the ignore list would be somewhat invasive.

So my "fix" is now to bind-mount all ro mounts which are not in /proc, /sys or the MagicDir into the MagicDir and unmount them at their original locations. After kaniko finished, I bind-mount them back in their original place. I considered to "just unmount" them - but I thought, that there was a reason for them and I couldn't oversee the implications.

One question that remains is, if this scheme should also apply to the RW mounts - as this would then be more close to how a "normal" container works (runtime-mounts are "on top" of the image-state).

@spikecurtis spikecurtis requested a review from sreya August 23, 2023 13:01
@janLo
Copy link
Contributor Author

janLo commented Nov 13, 2023

🖐️

@matifali matifali requested a review from bpmct April 19, 2024 11:47
@matifali matifali requested review from mtojek and removed request for sreya and kylecarbs April 29, 2024 08:55
@janLo
Copy link
Contributor Author

janLo commented May 8, 2024

🙌

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat trick, TIL you can unmount the source of a bind-mount. 😄

err = syscall.Unmount(src, 0)
if err != nil {
logf(codersdk.LogLevelError, "Could not unmount %s: %s", src, err.Error())
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to continue in both of these cases? Wouldn't it be safer to error out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it now I think you might be right. On the other hand, the error will catch you later anyway when kaniko tries to delete the containing directory to make room for the base image.

But I do think fail fast is better. Feel free to make any changes you feel suitable.

@janLo
Copy link
Contributor Author

janLo commented May 8, 2024

Neat trick, TIL you can unmount the source of a bind-mount. 😄

There is stuff you learn in 20 years of Linux that you wish you would have never be exposed to 🫣

What do you think about RW mounts? It would mean that we could mount a persistent home directory In an envbuilder container without having to meddle with the ignorelist.

@johnstcn
Copy link
Member

Creating a separate branch for resolving conflicts: https://github.com/coder/envbuilder/compare/cj/janl/sysbox-issues

Validated using sysbox v0.6.4 in a Lima VM using envbuilder's own devcontainer:

docker run -it --rm \
    -v /tmp/envbuilder:/workspaces \
    -e GIT_URL=https://github.com/coder/envbuilder \
    -e INIT_SCRIPT=bash \
    envbuilder:janl-sysbox-issues # local image

Note: dockerd does need to be started manually in the absence of any init or systemd. Some separate fiddling may be necessary there.

@johnstcn
Copy link
Member

Continuing in #183

@johnstcn
Copy link
Member

Closing this out as #183 has been merged.

b8e2947

Thanks for the contribution @janLo !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Envbuilder does not run in a sysbox container
4 participants