bootc icon indicating copy to clipboard operation
bootc copied to clipboard

add `/usr/lib/bootc/kargs.d` support

Open lukewarmtemp opened this issue 1 year ago • 10 comments

Fixes https://github.com/containers/bootc/issues/255. Allows users to create files within /usr/lib/bootc/kargs.d with kernel arguments. These arguments can now be applied on a switch, upgrade, or edit.

General process:

  • use ostree commit of fetched container image to return the file tree
  • navigate to /usr/lib/bootc/kargs.d
  • read each file within the directory
  • push the contents of each file (kargs) into a vector containing all the desired kargs
  • pass the kargs to the stage() function

Example Containerfile:

FROM quay.io/fedora/fedora-coreos:stable

RUN mkdir -p /usr/lib/bootc/kargs.d
RUN cat <<EOF >> /usr/lib/bootc/kargs.d/00-console.toml
kargs = ["console=ttyS0,114800n8"]
supported-arch = ["x86_64"]
EOF

RUN cat <<EOF >> /usr/lib/bootc/kargs.d/01-mitigations.toml
kargs = ["mitigations=on", "systemd.unified_cgroup_hierarchy=0"]
supported-arch = ["x86_64", "aarch64"]
EOF

Looking for some input as to how files within /usr/lib/bootc/kargs.d should be formatted (which will also affect how the contents of the file are parsed)

Signed-off-by: Luke Yang [email protected]

lukewarmtemp avatar Mar 18 '24 19:03 lukewarmtemp

Let's bikeshed some of the file format/semantics.

One angle: I think there are use cases for conditionally-applied kernel arguments. One that very much comes up is the console= kernel argument being different on architectures and platforms - and dealing with that is painful for people deploying across platforms. We could just say "You define your own build process which writes to /usr/lib/bootc/kargs.d" which would be fine...but, something like what systemd does with .unit files or perhaps with .link files could make sense.

IOW

[match]
architecture = x86_64

kargs = ["console=ttyS0,114800n8"]

maybe?

BTW, a whole thing running through all of this is that I think a number of bootc use cases are better off actually generating custom kernel binaries, where you don't need or want this at all; that's something to keep in mind.

cgwalters avatar Mar 21 '24 18:03 cgwalters

related to console https://github.com/coreos/fedora-coreos-config/blob/testing-devel/platforms.yaml

cgwalters avatar Mar 28 '24 19:03 cgwalters

@cgwalters Do you know if there's there a way for Rust to detect what platform your system is running? I'm able to find and match against the arch of the system using std::env::consts::ARCH, but not sure if there's something similar for platform.

lukewarmtemp avatar Apr 26 '24 19:04 lukewarmtemp

@cgwalters Do you know if there's there a way for Rust to detect what platform your system is running? I'm able to find and match against the arch of the system using std::env::consts::ARCH, but not sure if there's something similar for platform.

There's a bunch of things you can trigger conditional compilation on, such as target_os, maybe that can cover what you need?

jeckersb avatar Apr 26 '24 19:04 jeckersb

By "platform" I meant e.g. AWS vs GCP vs OpenStack etc. We're not going to build distinct bootc binaries for each of those, and runtime detection is "interesting". What Fedora CoreOS and derivatives do is https://github.com/coreos/fedora-coreos-tracker/blob/main/internals/README-internals.md#ignitionplatformid

At a practical level though I think we will likely end up with distinct container builds per "platform" in this sense, so we probably don't need to do dynamic dispatch here.

That said, for base images that do include ignition they'll need to have ignition.platform.id anyways...so in theory what we could support is a generic matching against an existing kernel argument (which would make this whole thing slightly recursive of course).

cgwalters avatar Apr 26 '24 20:04 cgwalters

By "platform" I meant e.g. AWS vs GCP vs OpenStack etc.

Ah gotcha. That's what I get for not reading back far enough for context. Carry on!

jeckersb avatar Apr 26 '24 20:04 jeckersb

It's a bit ugly but we could also consider matching what Cargo does for architecture conditionals... https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies

Maybe even lift the code. Dunno.

I think my instinct here is that anything we do with kargs we should do with install configs too, as that's also a made up TOML thing but we may as well try to be consistent.

Specifically it makes sense to me to support "install only" kargs having architecture conditionals too.

cgwalters avatar Apr 26 '24 20:04 cgwalters

Thanks so much for working on this! The code is looking good at a very high level. We'll need to add documentation, but we can start on that once we're a bit more certain of the design.

I don't want to unnecessarily delay this, but I just keep honestly waffling on the file formats and location.

We could consider placing these files in e.g. /usr/lib/modules/$kver which is our canonical location for kernel data. Maybe /usr/lib/modules/$kver/kargs.d ? Uncertain...

A lot of things would get simpler if one actually just included the kargs in the kernel image (but, doing so breaks any upstream secure boot signatures, and just in general changes the binary, which is why it's not typically done outside of custom embedded builds).

cgwalters avatar May 14 '24 13:05 cgwalters

Also, an integration test would really help. But you don't need to block on writing it, I can help

cgwalters avatar May 14 '24 17:05 cgwalters

We could consider placing these files in e.g. /usr/lib/modules/$kver which is our canonical location for kernel data. Maybe /usr/lib/modules/$kver/kargs.d ? Uncertain...

Any new thoughts relating to this? @cgwalters

lukewarmtemp avatar May 22 '24 15:05 lukewarmtemp

Any new thoughts relating to this?

I guess a downside of using the modules directory (aka /usr/lib/modules/$kver) is that anyone doing a derived build would need to be more careful to detect the container kernel version as is done for e.g. https://docs.fedoraproject.org/en-US/bootc/initramfs/#_regenerating_the_initrd and drop into that directory vs just a straight COPY 20-mykargs.toml /usr/lib/bootc/kargs.d.

So...I don't have anything better than /usr/lib/bootc/kargs.d. I feel somewhat uncomfortable in inventing a new thing though. One thing I'd like to do is try to standardize some of the bootc stuff in OCI upstream, and it'd be around exactly things like this.

cgwalters avatar May 23 '24 17:05 cgwalters

One thing I'd like to do is try to standardize some of the bootc stuff in OCI upstream, and it'd be around exactly things like this.

:arrow_right: https://groups.google.com/a/opencontainers.org/g/dev/c/OtLlhn4jxBg

cgwalters avatar May 23 '24 18:05 cgwalters

the big gap I see is around integration testing

Oh sorry, I was going to look at this but it slipped my mind. I can probably have a crack at it to see if I can get it working. If I can't see to get it, I'll probably reach out to @henrywang.

Can you also take an action item to do a merge request to https://gitlab.com/fedora/bootc/examples say?

Will do.

lukewarmtemp avatar Jun 04 '24 19:06 lukewarmtemp

MR updating bootc examples: https://gitlab.com/fedora/bootc/examples/-/merge_requests/48

lukewarmtemp avatar Jun 05 '24 16:06 lukewarmtemp

I tested this out and hit:

OSTree:ERROR:src/libostree/ostree-repo-file.c:828:ostree_repo_file_tree_query_child: assertion failed: (self->tree_contents)
Bail out! OSTree:ERROR:src/libostree/ostree-repo-file.c:828:ostree_repo_file_tree_query_child: assertion failed: (self->tree_contents)

cgwalters avatar Jun 07 '24 21:06 cgwalters