to-disk: Switch to a minimal boot target for to-disk installation - #370
Conversation
|
I did have a question about the initial design of to-disk. Why is the source image for the to-disk operation used for the ephemeral VM that runs bootc install? It seems that you may have to keep adding workarounds / fixes like this to handle differences in images that people try to test with this tool. Wouldn't it be better to use a standard upstream image (like the centos stream or fedora image) as the VM used for running the bootc install command? |
| mount -t tmpfs -o {TMPFS_SIZE} tmpfs /var/tmp | ||
| mkdir -p /var/tmp/containers | ||
|
|
||
| # Stop and mask quadlet units that started at boot. Their containers open |
There was a problem hiding this comment.
Hmm. I think what we actually should be doing here is booting up in a custom target.
Quadlets are just one special case of systemd units that one may not expect to run during a to-disk invocation.
There was a problem hiding this comment.
a custom target
I think this should be relatively easy - we already have code to inject system units, so we just add e.g. bcvk-to-disk.target and then have that basically only Wants=openssh.service.
In fact of course it would make huge amounts of sense to stop using ssh at all and report status back out to the host via a virtio-serial or so, but that had some quite more nontrivial things (e.g. handling terminal progress rendering).
There was a problem hiding this comment.
I hadn't thought of a custom target, I'll look at doing that instead. I don't think I'm quite familiar enough with rust or terminal rendering to go directly to virtio-serial; but that might be something interesting to learn in the future.
There was a problem hiding this comment.
I hadn't thought of a custom target, I'll look at doing that instead.
If you need help on this or have questions feel free to ask live on Slack, we also have a community meeting today!
(That said, if it turns out to be too hairy, I'm not opposed to masking quadlets for now - but if we do that I think it'd be cleaner in the code to mask the whole generator which we could likely do by injecting a symlink to /dev/null in /run for it in the initramfs?)
I don't think I'm quite familiar enough with rust or terminal rendering to go directly to virtio-serial; but that might be something interesting to learn in the future.
(Sorry that was meant more of an aside, changing that in this PR is definitely not needed)
There was a problem hiding this comment.
Thanks for the tip about the Slack. I've contributed a few small changes so far so I think it's a good idea to join.
It was very much by design that the system is "self contained" - it means if you want to e.g. change the version of (As far as separate installers and generation tools, those of course exist in image-builder and Anaconda and others) |
b01a757 to
dc37edd
Compare
petracvv
changed the title
|
I've updated this PR title and description with the alternative approach. It wasn't that bad. We can't go super minimal and only have SSH starting in the target since we need virtiofs and a few other base OS features like udev to start. So the custom target is set after I kept the quadlet test as that serves as a decent regression test as it will fail without the custom target |
Since we are using the user-provided image as the source for the ephemeral VM in the to-disk process, it is possible that there are services in that image that can break the installation (i.e. quadlets). Add a custom systemd target the only loads services essential to the to-disk installation process. An integration test is included to test for any regressions in the future. Booting into a default target will cause that test to fail. Closes: bootc-dev#351 Assisted-By: AI Signed-off-by: Mihai Petracovici <petracvv@users.noreply.github.com>
dc37edd to
2e1b6d6
Compare
| # that can interfere with the installation, e.g. by opening files under | ||
| # /var/lib/containers. This target starts only what the install needs: | ||
| # - basic.target (pulls in sysinit.target: udevd, journald, ...) | ||
| # - network-online.target and sshd.service (the host drives the install over SSH) |
There was a problem hiding this comment.
These types of comments are better right next to the Requires line.
| COPY --chown=0:0 <<EOF /etc/containers/systemd/sleep.container | ||
| [Container] | ||
| Image=docker.io/library/alpine:latest | ||
| Exec=sleep 600 |
There was a problem hiding this comment.
This test could also run with --privileged and run poweroff, then we'd really know if quadlets started running unintentionally 😄
| ro_bind_mounts: Vec::new(), // No additional ro bind mounts needed | ||
| systemd_units_dir: None, // No custom systemd units | ||
| bind_storage_ro: true, // Mount host container storage read-only | ||
| systemd_units_dir: Some(units_dir.path().to_string_lossy().to_string()), |
There was a problem hiding this comment.
The (pre-existing, unrelated to this PR) implementation of systemd_units_dir isn't my favorite - with newer systemd we can more reliably use credentials for this, but for the case of to-disk this is probably the right choice.
2 participants
Since we are using the user-provided image as the source for the
ephemeral VM in the to-disk process, it is possible that there are
services in that image that can break the installation (i.e. quadlets).
Add a custom systemd target the only loads services essential to the
to-disk installation process.
An integration test is included to test for any regressions in the
future. Booting into a default target will cause that test to fail.
Closes: #351
Some extra context: