-
Notifications
You must be signed in to change notification settings - Fork 57
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
manifest: use experimentalflags.String("bootstrap") #1263
base: main
Are you sure you want to change the base?
Conversation
pkg/manifest/build.go
Outdated
@@ -149,6 +156,33 @@ func (p *BuildrootFromPackages) getSELinuxLabels() map[string]string { | |||
return labels | |||
} | |||
|
|||
// maybeAddExperimentalContainerBootstrap will return a container buildroot | |||
// if if the "IMAGE_BUILDER_EXPERIMENTAL=bootstrap=<container-ref>" is |
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.
typo 'if if'
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.
Thank you! Fixed now (and while reading this again expanded about what a bootstrap container actually needs).
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.
Looks good to me other than the small typo.
What's the plan for documenting the experimental flags? Is the commit message + function docstring enough or should there be a central document listing them all?
As we don't want to share these to the public in general (unless we specifically request someone to tinker) I think just internalized knowledge of them is fine :) |
9a1d0f3
to
8f78c04
Compare
For now I agree with @supakeen - we can keep this internal. I would love to see this generalized eventually (see also the commit message) so that a user just types (on their x86_64 box) |
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.
Awesome! :) It might be a good idea to also cover the selinux special case: a test that checks that the bootstrap pipeline doesn't have an selinux stage.
// A "bootstrap" container has only these requirements: | ||
// - python3 for the runners | ||
// - rpm so that the real buildroot rpms can get instaleld | ||
// - setfiles so that the selinux stage for the real buildroot can run |
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.
The second commit invalidates this.
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.
Unless I miss something this is still true. The flow is:
- bootstrap buildroot gets mounted, no selinux stage is generated here (so host does not need to have setfiles
- buildroot is generated, it uses the boostrap buildroot for that, a bunch of rpms get installed, then "setfiles" is run because the buildroot is expected to contain valid selinux config. but this uses the setfiles binary from the bootstrap env not from the just created buildroot
Maybe I need to explain this better? (fwiw, to test just use theIMAGE_BUILDER_EXPERIMENTAL=bootstrap=fedora.riscv.rocks:3000/davidlt/fedora-toolbox:41
container which does not contain setfiles (policycoreutils) - here the buildroot generation fails.
This commit allows to use a container to bootstrap the buildroot via the experimtnal "bootstrap" flag: ``` $ IMAGE_BUILDER_EXPERIMENTAL=bootstrap=ghcr.io/mvo5/fedora-buildroot:41 \ image-builder manifest --arch=riscv64 minimal-raw --distro=fedora-41 ``` and it will use the given container to bootstap the buildroot. A bootstrap container has very few requirements: - python3 for the runners - rpm so that the real buildroot rpms can get instaleld - setfiles so that the selinux stage for the real buildroot can run (so most upstream containers, e.g. fedora-toolbox or ubi work). This will allow to do cross-arch building for riscv. The generated manifest can be build with osbuild (as long as qemu-user is installed) and will generate a minimal-raw for riscv64. Cross-building is useful because qemu-user is a lot faster (and easier to setup) than full riscv system emulation. A minimal raw build on a really fast AWS machine takes about 100 minutes. A full minimal-raw build on my (moderate) PC takes about 20min - which is fast enough so that we can run this in GH workflows as part of the CI. The approach with the bootstrap buildroot [0] allows us to generalize this later so that each distro would (in addition to the repos) an upstream container ref. With that we could auto-detect cross builds and we would just add this upstream container to bootstrap the buildroot. [0] Thanks to Ondrej, Achilleas, Thozza, Simon and Florian.
We cannot make assumption about the bootstrap container being used, so disable selinux when setting up the bootstrap container. The real buildroot will be correctly labeld, no change here.
8f78c04
to
a103731
Compare
This commit allows to use a container to bootstrap the buildroot via the experimtnal "bootstrap" flag:
and it will use the given container to bootstap the buildroot.
This will allow to do cross-arch building for riscv. The generated manifest can be build with osbuild (as long as qemu-user is installed) and will generate a minimal-raw for riscv64.
Cross-building is useful because qemu-user is a lot faster (and easier to setup) than full riscv system emulation. A minimal raw build on a really fast AWS machine takes about 100 minutes. A full minimal-raw build on my (moderate) PC takes about 20min - which is fast enough so that we can run this in GH workflows as part of the CI.
The approach with the bootstrap buildroot [0] allows us to generalize this later so that each distro would (in addition to the repos) an upstream container ref. With that we could auto-detect cross builds and we would just add this upstream container to bootstrap the buildroot.
[0] Thanks to Ondrej, Achilleas, Thozza, Simon and Florian.
P.S. With that we can also do:
so we will not have to maintain our own containers (there are still some assumption like that the container contains python3 for the runners).