-
Notifications
You must be signed in to change notification settings - Fork 75
distro: move rpm based bootc anaconda-iso generation into the images library #2010
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
Conversation
1e85415 to
c02ed88
Compare
| # - its "indirect" in the sense that it pulls the RPMs from a | ||
| # "real" distro, i.e. if a bootc centos-10 is found it will | ||
| # load the imagetypes.yaml for that to load the | ||
| # bootc-rpm-installer |
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.
This is a very important comment ❤️.
| if err := cnt.InitDNF(); err != nil { | ||
| // Not all bootc container have dnf, so check if it can | ||
| // be run here and if not just return nil which will | ||
| // ensure the depsolver of the host is used |
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.
Not all hosts have DNF either, do we need to verify that as well?
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.
If dnf is missing on the host images should already DTRT, i.e. if its not needed (like for pure bootc types) it will just continue and for rpm based ones it will print an error.
| rng := rand.New(rand.NewSource(seed)) | ||
|
|
||
| switch { | ||
| // XXX: make this a yaml property |
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.
Follow-up or will you update this PR?
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.
I have not fully made up my mind yet, followup might be nice but maybe its enough to update the comment, given that its extremely unlikely that we ever add another legacy rpm based rpm installer we might keep hardcoding it. But making it YAML is slightly nicer (but maybe extra work)
This commit tries to address the concern from Achilleas in osbuild#2010 (comment) When using a custom Depsolve function it is now mandatory to pass a valid `depsolvednf.Solver` instance. So the custom depsolve function will no longer create it on its own as this is no longer feasiable. With the bootc distro that requires a different depsolvednf.Solver it is no longer something that a user can override. The commit also renames the function from Depsolver to Depsolve as its doing the (dep) solving with an existing "solver". Getting the terminology more disentangled would be nice but I couldn't come up with better terms, maybe `DepsolveRun` ?
The bootc based distros need a custom depsolver because they need to use the dnf from the container to ensure that all the plugins/custom config is used. So manifestgen now checks if the distro defines its own way to create a depsovler and if so passes it into the manifestgen.Depsolver. With that bootc ISOs (that require rpms) are compatible with custom depsolve helpers and we can use a custom depsolver to catpure the mTLS keys for the rpms that are required by bootc.
This commit moves the code to generate the rpm anaconda-iso installer type into the images library. This is possible because we use the distro specific depsolver. In bib we need to define a custom depsolver function that wraps the `manifestgen.DefaultDepsolver()` so that we can captures the repositories and extract the mTLS keys from there. The image type is ugly as it violates our layering, i.e. it will first inspect the bootc container to find the distro id and then look into our "regular" imagetypes.yaml for the found distro for the rpms to install. This is convenient as we maintain our rpms all in the same place but its also annyoing. Ideally we would get rid of this image type and use the container based bootc-installer type [0] but we will probably need this RPM one for a while still. [0] osbuild#1906
Ideally we would just use `name_aliases` but in the bib/ibcli code we use the low-level interface of the loader to get the image which does not look at name_alises. As this is quite a special case lets just be pragmatic and define "iso" via a YAML alias/anchor.
Not all bootc container have dnf, so check if it can be run here and if not just return nil which will ensure the depsolver of the host is used.
Now that the `anaconda-iso` image type for bootc is part of the image library we can (re)use the existing manifest testing system to generate fake manifests that validate that our images do not change accidentially. This commit adds: rhel-10.1, centos-9, fedora-43, fedora-42 which should be a good sample.
This commit tries to address the concern from Achilleas in osbuild#2010 (comment) When using a custom Depsolve function it is now mandatory to pass a valid `depsolvednf.Solver` instance. So the custom depsolve function will no longer create it on its own as this is no longer feasiable. With the bootc distro that requires a different depsolvednf.Solver it is no longer something that a user can override. The commit also renames the function from Depsolver to Depsolve as its doing the (dep) solving with an existing "solver". Getting the terminology more disentangled would be nice but I couldn't come up with better terms, maybe `DepsolveRun` ?
9a7af45 to
c1ce9a0
Compare
|
Had to force push because of conflicts, sorry. |
|
Linties |
This commit tries to address the concern from Achilleas in osbuild#2010 (comment) When using a custom Depsolve function it is now mandatory to pass a valid `depsolvednf.Solver` instance. So the custom depsolve function will no longer create it on its own as this is no longer feasiable. With the bootc distro that requires a different depsolvednf.Solver it is no longer something that a user can override. The commit also renames the function from Depsolver to Depsolve as its doing the (dep) solving with an existing "solver". Getting the terminology more disentangled would be nice but I couldn't come up with better terms, maybe `DepsolveRun` ?
c1ce9a0 to
6ba8221
Compare
This PR moves all the logic for anaconda-iso bootc ISO generation into the images library and extends manifestgen.Depsolve() so that we can pass a custom depsolver (which is needed for the bootc anaconda isos as they use a custom solver that point to the mounted container)
This is working end-to-end in osbuild/bootc-image-builder#1131 - all existing tests pass (might be good to do a manual test with a rhel container on a rhel system, this part is not well tested).