-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add support for --replace-mode=alongside
for ostree target
#137
base: main
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
86f6038
to
22671b5
Compare
22671b5
to
e4f00af
Compare
Sorry @cgwalters , accidentally pushed the rebase to your fork instead of mine
EDIT2: Continuing here |
e4f00af
to
22671b5
Compare
I think you can just take over this PR too if you want, or open a new PR from your fork - either way. |
22671b5
to
e4f00af
Compare
Rebased. Without any changes, I'm facing an issue where in an ostree system, the mounted I'll see how I can tweak it so that it finds the right device |
With additional |
@cgwalters thoughts on the above mounts? Do we want to require them for install on ostree targets, or should I figure out a way to make this work without them, using just the already-documented install mounts (i.e. |
We should learn how to peel that. This is really the same thing as https://bugzilla.redhat.com/show_bug.cgi?id=2308594 and ostreedev/ostree#3198 and containers/composefs#280 Short term the simplest is the same logic as the grub patch - detect overlayfs for |
e4f00af
to
923b0ed
Compare
OK. Changed it so that when the target rootfs is an overlay, we'll implicitly try targetting It wasn't working at first and was a bit of a headache for me to debug because apparently if you mount Current code might need a bit of touch-ups, but do you think the direction of the code in its current state is good? Should I clean it up and undraft? |
I think that's possibly because it's bootc that's special casing mounting |
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 picking this up!!
3561a64
to
ed94f1e
Compare
d392280
to
eed91ff
Compare
The current experience is: echo "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOTVytyhnSfX20smAsNKYG5Zpz6vSzDZu22S8PCDJ2Iw omer" > authkeys
sudo podman run --rm --privileged -v $PWD/authkeys:/authkeys -v /dev:/dev -v /var/lib/containers:/var/lib/containers -v /:/target --pid=host --security-opt label=type:unconfined_t -e RUST_LOG=trace quay.io/otuchfel/bootc:latest bootc install to-existing-root --replace alongside --acknowledge-destructive --stateroot foobar --root-ssh-authorized-keys /authkeys Following that, when I reset, it immediately boots into the new I assume this is not our desired experience, and I should look into having the original boot entry preserved? |
It's a great question. I think for Let's then target merging this as is? I think then what we should look at is weaving in this functionality into #404 right? That said we may end up wanting to expose this as something like The super messy thing is interoperating with non-ostree-ready bootloader setups so IOW:
|
sgtm |
--replace-mode=alongside
for ostree target--replace-mode=alongside
for ostree target
OK so we should have CI coverage here...I think it's actually as easy as doing another install after an initial one without doing the manual "wipe ostree" stuff we have in one of the install tests right? |
f465ae7
to
9876584
Compare
Not sure I agree, ideally we should actually boot into the installed ostree system first? But getting reboots to work with the current But anyway can't hurt having a test like you suggested, even without a reboot |
Yes, testing with reboots is going to require some more infrastructure here. |
9876584
to
72642fe
Compare
Unscratch that even after removing the call to |
Ironically our support for `--replace-mode=alongside` breaks when we're targeting an already extant ostree host, because when we first blow away the `/boot` directory, this means the ostree stack loses its knowledge that we're in a booted deployment, and will attempt to GC it... ostreedev/ostree-rs-ext@8fa019b is a key part of the fix for that. However, a notable improvement we can do here is to grow this whole thing into a real "factory reset" mode, and this will be a compelling answer to coreos/fedora-coreos-tracker#399 To implement this though we need to support configuring the stateroot and not just hardcode `default`. Signed-off-by: Omer Tuchfeld <[email protected]>
72642fe
to
30b3e31
Compare
We talked about this and realized that while the new test passes, it would pass already today because it's actually the "install --stateroot" that makes it work. The main fix we need here is preserving existing deployments when we detect we're booted via ostree. So actually a way we could test this is via our tmt tests instead. But in the end again I'm good to merge as is. Since I wrote this PR I can't approve it, you (or someone else) needs to do so. |
Ironically our support for
--replace-mode=alongside
breaks when we're targeting an already extant ostree host, because when we first blow away the/boot
directory, this means the ostree stack loses its knowledge that we're in a booted deployment, and will attempt to GC it...ostreedev/ostree-rs-ext@8fa019b is a key part of the fix for that.
However, a notable improvement we can do here is to grow this whole thing into a real "factory reset" mode, and this will be a compelling answer to
coreos/fedora-coreos-tracker#399
To implement this though we need to support configuring the stateroot and not just hardcode
default
.