main: add --bootc-defaultfs option
#324
Conversation
mvo5
requested review from
croissanne,
supakeen and
thozza
and removed request for
a team
While allowing setting the default FS type for a distro (i.e Fedora defaulting to Moreover, shouldn't the bootc distro definition have a default FS type defined in it, instead of requiring it to be specified on the CLI? |
"Should", not all do (Fedora doesn't).
Maybe. I like a CLI argument. |
I understand that. I meant in our bootc distro definition.
That's fine. I understand that it is more convenient. Nevertheless, it just adds friction and opens the opportunity to achieve the same thing in a multitude of different ways on-prem, in the Service, etc... |
|
[..]
Thanks, this is a really good question/consideration. I'm honestly a bit torn, what I don't like is the extra friction of a blueprint because its a little bit more complicated than just But the root of the issue is that some containers do not set a default fs - so how about we just pick ext4 as the safe choice if the container does not declare one? We could show a warning with a URL that explain how to change it but that would eliminate all friction and still make it easy for our users to discover how to change the fs, wdyt? |
This makes sense. I don't see a problem with making specific customizations more convenient to use by providing a CLI argument. Still, we should consider whether this should be an addition to a BP customization that does the same thing.
Yes, this is the core of my proposal/idea, which may not make sense for bootc world. To have a sane default that gets used when the container does not declare the default FS, instead of forcing the user to provide it explicitly. Providing a way to change the default IMHO makes sense. |
|
[..]
Cool, I think we are on the same page. I'm almost inclined to close this PR and work on the blueprint option instead (and just use that exclusively) and just set a default (ext4) for now. But that might disrupt the timeline that @supakeen has for the koji integration(?) as for a few days (until we have it in blueprints) we would not be able to generate fedora bootc disk iamges with anything but ext4. |
I have no timeline; but if it's in the blueprint it can still be passed along in Koji. My main concern as always is that I really prefer command line options to having to keep some state in a blueprint (separate discussion) :) |
supakeen
force-pushed
the
bootc-rootfs
branch
from
c065add to
3ee7e84
Compare
My €0.02: I think I prefer this, having an internal global default to fall back on when everything is missing (container config, blueprint, command line option if we end up having one). |
Some distros do not have a default filesystem and some distros have a default filesystem but users want a different one. So provide an easy and early way to override the default filesystem for bootc directly from bootc.NewBootcDistro(). Needed for osbuild/image-builder-cli#324
We need a way to a) set the bootc default filesystem b) detect when its unset and provide a clear error This commit fixes both issue, we had a way to setup the default filesystem but that is now applied too late because we now share the partition table loading with the generic image types which have no concept of an empty default. So this moves the default-fs setting into the `bootc.NewBootcDistro()` as a new `DistroOptions` struct. It also adds a proper error type for when no default-fs is set, this way ibcli can detect this and pick a default on its own (this is what Achilleas suggested in [0]). There is also a bugfix in setupDefaultFS() so that we do not switch to "btrfs" on the /boot partition. This is the wrong layer to know about /boot and details like this, we will need a followup that moves this into disk instead. [0] osbuild/image-builder-cli#324 (comment)
We need a way to a) set the bootc default filesystem b) detect when its unset and provide a clear error This commit fixes both issue, we had a way to setup the default filesystem but that is now applied too late because we now share the partition table loading with the generic image types which have no concept of an empty default. So this moves the default-fs setting into the `bootc.NewBootcDistro()` as a new `DistroOptions` struct. It also adds a proper error type for when no default-fs is set, this way ibcli can detect this and pick a default on its own (this is what Achilleas suggested in [0]). There is also a bugfix in setupDefaultFS() so that we do not switch to "btrfs" on the /boot partition. This is the wrong layer to know about /boot and details like this, we will need a followup that moves this into disk instead. [0] osbuild/image-builder-cli#324 (comment)
We need a way to a) set the bootc default filesystem b) detect when its unset and provide a clear error This commit fixes both issue, we had a way to setup the default filesystem but that is now applied too late because we now share the partition table loading with the generic image types which have no concept of an empty default. So this moves the default-fs setting into the `bootc.NewBootcDistro()` as a new `DistroOptions` struct. It also adds a proper error type for when no default-fs is set, this way ibcli can detect this and pick a default on its own (this is what Achilleas suggested in [0]). There is also a bugfix in setupDefaultFS() so that we do not switch to "btrfs" on the /boot partition. This is the wrong layer to know about /boot and details like this, we will need a followup that moves this into disk instead. [0] osbuild/image-builder-cli#324 (comment)
mvo5
force-pushed
the
bootc-rootfs
branch
2 times, most recently
from
c939bca to
6aeb0ac
Compare
Not all bootc containers declare a default filesystem, most noatbly fedora does not. So this commit adds `--bootc-defaultfs` to set it. It *might* be interesting in the future to consider making this more broad, i.e. to allow `--defaultfs` for package based distros too (with the generic distro we would set it via `DistroYAMl.DefaultFStype`). But definitely followup material (and requires more discussion if we want it). Closes: osbuild#323
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
This is erroring due to a new Anubis deployment update at Fedora making the ostree manifests fail again. I'll retry and otherwise admin merge later on as we've done previously for this issue and CI is green inside the PR. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
6 participants
Not all bootc containers declare a default filesystem, most noatbly fedora does not. So this commit adds
--bootc-defaultfsto set it.It might be interesting in the future to consider making this more broad, i.e. to allow
--defaultfsfor package based distros too (with the generic distro we would set it viaDistroYAMl.DefaultFStype). But definitely followup material (and requires more discussion if we want it).Closes: #323