install: Do not clean boot directories on ostree systems #1193
Conversation
lib/src/install.rs
Outdated
| // On ostree systems, the boot directory already has our desired format, we should only | ||
| // remove the bootupd-state.json file to avoid bootupctl complaining it already exists. | ||
| bootdir | ||
| .remove_file_optional("bootdupd-state.json") |
There was a problem hiding this comment.
It feels wrong to remove bootupd-state.json, but I saw bootupctl complaining if I don't. I don't really understand why. Would appreciate some clarification about what's happening and whether this is a good idea
There was a problem hiding this comment.
There's a typo in the filename.
Mmm...yes, this is an interesting topic. I think the best fix actually is perhaps to detect when we already have a bootupd-managed state and just run a bootupd update instead of an install (matches logically how we're thinking of this install to-existing-root for the ostree/container base).
In the short term though as far as bootupd goes, it's going to really be mostly the same to just delete the state file and trick it into doing a reinstall, especially because right now we wipe the ESP.
There was a problem hiding this comment.
Ah I see, didn't realize the distinction between bootupd update and install. I'll try to adjust the PR
There was a problem hiding this comment.
Running bootupctl update would require us to have a "-v" "/run/bootupd.sock:/run/bootupd.sock" (which can be trivial through #919)
And it seems like update is more picky about the user's choice of boot firmware. (I'm running on BIOS and it's complaining, install didn't complain)
So I think for now I'll stick to the reinstall flow. What do you think?
There was a problem hiding this comment.
Running bootupctl update would require us to have a "-v" "/run/bootupd.sock:/run/bootupd.sock" (which can be trivial through #919)
Though in recent bootupd it no longer has that socket, so we should be able to avoid that. Ref coreos/bootupd#663
There was a problem hiding this comment.
So I think for now I'll stick to the reinstall flow. What do you think?
In my ideal world...we make these operations less jarring/differentiated to the user. And again ideally more symmetric between "package based previous install" and "ostree previous install". I should be able to just reboot into the package based install if I choose to...another way to think of this is maybe bootc switch should actually Just Work too from a package system.
Anyways...the problem is there's lots of Details here, some of which have user relevant differences. But we'll slowly work through that.
On ostree systems, the boot directory already has our desired format, we should only remove the bootupd-state.json file to avoid bootupctl complaining about it already existing. The motivation is that this will preserve the boot entry for the original deployment, allowing the user to boot into it if they want to. This also makes sure `ostree admin status` continues working - since if we're in a booted ostree system but `ostree` can't find its physically (through boot entries) it complains.
|
|
||
| if is_ostree { | ||
| // On ostree systems, the boot directory already has our desired format, we should only | ||
| // remove the bootupd-state.json file to avoid bootupctl complaining it already exists. |
There was a problem hiding this comment.
Hmm though I think this is going to work when staying within a single OS major usually, in the general case I think we can get some skew here when we don't clear the ESP (as is done below for the non-ostree/bootupd case).
Related to this on the flip side, ostree and bootupd are separate, orthogonal projects and we were actually talking about using bootupd for package-based Fedora too. In that scenario, we would still need to blow away the bootupd state.
But then again really the right fix in the medium term here is again to basically treat this as an upgrade, which bootupd already supports. So, eh.
|
/packit test |
On ostree systems, the boot directory already has our desired format, we should only remove the bootupd-state.json file to avoid bootupctl complaining about it already existing.
The motivation is that this will preserve the boot entry for the original deployment, allowing the user to boot into it if they want to.
This also makes sure
ostree admin statuscontinues working - since if we're in a booted ostree system butostreecan't find its physically (through boot entries) it complains.