rhel10: move os package sets to yaml files #1104
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "os" | ||
| "path" | ||
|
|
||
| "github.com/osbuild/images/pkg/definition" | ||
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| func main() { | ||
| dir := os.DirFS(".") | ||
| path := path.Clean(os.Args[1]) | ||
| merged, err := definition.MergeConfig(dir, path) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| err = yaml.NewEncoder(os.Stdout).Encode(merged) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Image definituons | ||
| This directory contains definitions of image types that this library can build. | ||
|
|
||
| > Currently, only packages are defined in YAML. The rest is still in Go. | ||
|
|
||
| The supporting library for these files is in ../pkg/definition. | ||
|
|
||
| ## Definition searching | ||
| When a user wants to build an ami for rhel 10.2 on x86_64, the library searches for the most suitable definition: | ||
|
|
||
| - firstly, it tries to open `rhel/10.2/x86_64/ami.yaml` | ||
| - if it doesn't exist, it tries to open `rhel/10.2/generic/ami.yaml` | ||
| - if it doesn't exist, it tries to open `rhel/X.Y/x86_64/ami.yaml` or `rhel/X.Y/generic/ami.yaml`, with X.Y being the closest older version to 10.2 (10.1 will be prefered over 10.0) | ||
| - if it doesn't exist, the build fails | ||
|
Comment on lines
+11
to
+14
There was a problem hiding this comment. To avoid repetition, I'd rephrase the part above this list to describe that we're going to list a series of paths that are checked in order until one is found. For example: To find the definition for a specific image configuration (distribution, architecture, image type combination), the definition loader will search for the most suitable definition by starting with the most accurate path and widening the search to more generic paths at every step. For example, to find the definition for RHEL 10.2, x86_64, AMI, the loader will check the following paths in order and load the first file it finds:
There was a problem hiding this comment. Should it really fall back to a previous major version? eg. no matching 10.Y entries so it uses 9.6. I think the X should stay fixed on the major release and it should fail if there is no X.0 or higher directories found. Also, I'd slightly prefer a well-formed filename instead of a directory tree. It's easier to explore and work with manually, or compare file contents. eg. There was a problem hiding this comment.
I think the code in the PR does fall back to lower major versions, so maybe that's something to discuss.
That's funny, I made the exact opposite suggestion (adding one more level) to my main comment above :) There was a problem hiding this comment. I'm missing a bit of motivation as to why we want to do this fallback dance in the first place. It seems some of it could be managed more explicitly with symlinks in the repository and it would make it (to me) easier to reason about what its doing. Is it purely to avoid us creating those possible symlinks? There was a problem hiding this comment. the main benefit I could see is when a new release appears but there aren't any new/specific definitions for it like when Fedora branches from rawhide. |
||
|
|
||
| ## Definition format | ||
| There are two top-level fields: `from` and `def`. | ||
|
|
||
| ### `from` | ||
| An array of relative paths to include. This works recursively, so an included file will also process its includes. The merging rules are explained in the invidual fields under `def`. | ||
|
|
||
| The includes are processed using [DFS postordering](https://en.wikipedia.org/wiki/Depth-first_search#Vertex_orderings). | ||
|
|
||
| ### `def` | ||
|
There was a problem hiding this comment. Why does this exist? I'd put the package sets at the top instead of them being the single key under def. There was a problem hiding this comment. This is likely because @ondrejbudai considers the inheritance/merging story to be separate from the definitions in the file. For that I understand the choice and quite like it. Definition files contain definitions and things-to-mix-together-with-those-definitions. I would prefer to rename |
||
| `def` is the core of the definition file. It contains following keys: | ||
|
|
||
| #### `packages` | ||
| Map of package sets installed in the image type. The keys are currently defined in Go code. The values are structs, each one represents inputs for one DNF transaction. | ||
|
There was a problem hiding this comment. Should we mention what the keys mean? That they're keywords for the purpose of each package set, e.g. There was a problem hiding this comment. I think so, as well as examples. Reading this I have no idea what it really means or what it should look like. I expect that will be clearer when I get to the yaml part, but coming at it as a new users it leaves me feeling uneasy. I would change the name, packages is incorrect and confusing. I'd call it |
||
|
|
||
| When merging package sets, the list of includes and excludes are simply appended together. | ||
|
There was a problem hiding this comment. That makes it sound like the merged set is includes + exclues, which doesn't make sense. Maybe something like: |
||
|
|
||
| The struct has following keys: | ||
|
There was a problem hiding this comment. Instead of struct say |
||
|
|
||
| ##### `include` | ||
| List of packages that must be in the depsolved transaction. | ||
|
|
||
| ##### `exclude` | ||
| List of packages that must not be in the depsolved transaction. | ||
|
Comment on lines
+37
to
+38
There was a problem hiding this comment. Is it worth explaining what the purpose of the excludes are? I don't know if we have it documented anywhere for newcomers. For example: Excludes are useful for avoiding installing unwanted packages that might be weak dependencies of a selected package. Adding an exclude which is a hard dependency of an included package will cause the build to fail. There was a problem hiding this comment. Also, what can the name include? Just the package name? The name-version? Full NEVRA? This should be specified to prevent future confusion (or attempts to use something it doesn't support). |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| def: | ||
| packages: | ||
| os: | ||
| include: | ||
| - "@core" | ||
| - chrony | ||
| - cloud-init | ||
| - cloud-utils-growpart | ||
| - dhcpcd | ||
| - yum-utils | ||
| - dracut-config-generic | ||
| - grub2 | ||
| - langpacks-en | ||
| - NetworkManager-cloud-setup | ||
| - redhat-release | ||
| - redhat-release-eula | ||
| - rsync | ||
| - tuned | ||
| - tar | ||
| exclude: | ||
| - aic94xx-firmware | ||
| - alsa-firmware | ||
| - alsa-tools-firmware | ||
| - biosdevname | ||
| - firewalld | ||
| - iprutils | ||
| - ivtv-firmware | ||
| - iwl1000-firmware | ||
| - iwl100-firmware | ||
| - iwl105-firmware | ||
| - iwl135-firmware | ||
| - iwl2000-firmware | ||
| - iwl2030-firmware | ||
| - iwl3160-firmware | ||
| - iwl3945-firmware | ||
| - iwl4965-firmware | ||
| - iwl5000-firmware | ||
| - iwl5150-firmware | ||
| - iwl6000-firmware | ||
| - iwl6000g2a-firmware | ||
| - iwl6000g2b-firmware | ||
| - iwl6050-firmware | ||
| - iwl7260-firmware | ||
| - libertas-sd8686-firmware | ||
| - libertas-sd8787-firmware | ||
| - libertas-usb8388-firmware | ||
| - plymouth | ||
| # RHBZ#2064087 | ||
| - dracut-config-rescue | ||
| # RHBZ#2075815 | ||
| - qemu-guest-agent |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| def: | ||
| packages: | ||
| os: | ||
| include: | ||
| - "@core" | ||
| - langpacks-en # not in Google's KS | ||
| - acpid | ||
| - dnf-automatic | ||
| - net-tools | ||
| - python3 | ||
| - rng-tools | ||
| - tar | ||
| - vim | ||
|
|
||
| # GCE guest tools | ||
| # TODO: uncomment once the package is available | ||
| # the el9 version depends on libboost_regex.so.1.75.0()(64bit), which is not available on el10 | ||
| # - google-compute-engine | ||
| - google-osconfig-agent | ||
| # Requires gdisk which was removed late in the RHEL 10 development cycle | ||
| # - gce-disk-expand | ||
| # cloud-init is a replacement for "google-compute-engine", remove once the package is available | ||
| - cloud-init | ||
|
|
||
| # Not explicitly included in GCP kickstart, but present on the image | ||
| # for time synchronization | ||
| - chrony | ||
| - timedatex | ||
| # EFI | ||
| - grub2-tools | ||
| - grub2-tools-minimal | ||
| # Performance tuning | ||
| - tuned | ||
| exclude: | ||
| - alsa-utils | ||
| - b43-fwcutter | ||
| - dmraid | ||
| - dracut-config-rescue | ||
| - eject | ||
| - gpm | ||
| - irqbalance | ||
| - microcode_ctl | ||
| - smartmontools | ||
| - aic94xx-firmware | ||
| - atmel-firmware | ||
| - b43-openfwwf | ||
| - bfa-firmware | ||
| - ipw2100-firmware | ||
| - ipw2200-firmware | ||
| - ivtv-firmware | ||
| - iwl100-firmware | ||
| - iwl105-firmware | ||
| - iwl135-firmware | ||
| - iwl1000-firmware | ||
| - iwl2000-firmware | ||
| - iwl2030-firmware | ||
| - iwl3160-firmware | ||
| - iwl3945-firmware | ||
| - iwl4965-firmware | ||
| - iwl5000-firmware | ||
| - iwl5150-firmware | ||
| - iwl6000-firmware | ||
| - iwl6000g2a-firmware | ||
| - iwl6050-firmware | ||
| - iwl7260-firmware | ||
| - kernel-firmware | ||
| - libertas-usb8388-firmware | ||
| - ql2100-firmware | ||
| - ql2200-firmware | ||
| - ql23xx-firmware | ||
| - ql2400-firmware | ||
| - ql2500-firmware | ||
| - rt61pci-firmware | ||
| - rt73usb-firmware | ||
| - xorg-x11-drv-ati-firmware | ||
| - zd1211-firmware | ||
| # RHBZ#2075815 | ||
| - qemu-guest-agent |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| def: | ||
| packages: | ||
| os: | ||
| include: | ||
| - "@core" | ||
| - chrony | ||
| - cockpit-system | ||
| - cockpit-ws | ||
| - dnf-utils | ||
| - dosfstools | ||
| - firewalld | ||
| - iwl1000-firmware | ||
| - iwl100-firmware | ||
| - iwl105-firmware | ||
| - iwl135-firmware | ||
| - iwl2000-firmware | ||
| - iwl2030-firmware | ||
| - iwl3160-firmware | ||
| - iwl5000-firmware | ||
| - iwl5150-firmware | ||
| - iwl6000g2a-firmware | ||
| - iwl6000g2b-firmware | ||
| - iwl6050-firmware | ||
| - iwl7260-firmware | ||
| - lvm2 | ||
| - net-tools | ||
| - nfs-utils | ||
| - oddjob | ||
| - oddjob-mkhomedir | ||
| - policycoreutils | ||
| - psmisc | ||
| - python3-jsonschema | ||
| - qemu-guest-agent | ||
| - redhat-release | ||
| - redhat-release-eula | ||
| - rsync | ||
| - tar | ||
| - tcpdump | ||
| - tuned | ||
|
|
||
| # This was pulled from distroBuildPackageSet for some reason | ||
| - dnf | ||
| - dosfstools | ||
| - e2fsprogs | ||
| - glibc | ||
| - lorax-templates-generic | ||
| - lorax-templates-rhel | ||
| - lvm2 | ||
| - policycoreutils | ||
| - python3-iniparse | ||
| - qemu-img | ||
| - selinux-policy-targeted | ||
| - systemd | ||
| - tar | ||
| - xfsprogs | ||
| - xz | ||
| exclude: | ||
| - dracut-config-rescue |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| from: | ||
| - qcow2.yaml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| from: | ||
| - vmdk.yaml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| def: | ||
| packages: | ||
| os: | ||
| include: | ||
| - "@core" | ||
| - chrony | ||
| - cloud-init | ||
| - cloud-utils-growpart | ||
| - cockpit-system | ||
| - cockpit-ws | ||
| - dnf-utils | ||
| - dosfstools | ||
| - nfs-utils | ||
| - oddjob | ||
| - oddjob-mkhomedir | ||
| - psmisc | ||
| - python3-jsonschema | ||
| - qemu-guest-agent | ||
| - redhat-release | ||
| - redhat-release-eula | ||
| - rsync | ||
| - tar | ||
| - tuned | ||
| - tcpdump | ||
| exclude: | ||
| - aic94xx-firmware | ||
| - alsa-firmware | ||
| - alsa-lib | ||
| - alsa-tools-firmware | ||
| - biosdevname | ||
| - dnf-plugin-spacewalk | ||
| - dracut-config-rescue | ||
| - fedora-release | ||
| - fedora-repos | ||
| - firewalld | ||
| - iprutils | ||
| - ivtv-firmware | ||
| - iwl1000-firmware | ||
| - iwl100-firmware | ||
| - iwl105-firmware | ||
| - iwl135-firmware | ||
| - iwl2000-firmware | ||
| - iwl2030-firmware | ||
| - iwl3160-firmware | ||
| - iwl3945-firmware | ||
| - iwl4965-firmware | ||
| - iwl5000-firmware | ||
| - iwl5150-firmware | ||
| - iwl6000-firmware | ||
| - iwl6000g2a-firmware | ||
| - iwl6000g2b-firmware | ||
| - iwl6050-firmware | ||
| - iwl7260-firmware | ||
| - langpacks-* | ||
| - langpacks-en | ||
| - libertas-sd8787-firmware | ||
| - plymouth | ||
| - rng-tools | ||
| - udisks2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| def: | ||
| packages: | ||
| os: | ||
| include: | ||
| - policycoreutils | ||
| - selinux-policy-targeted | ||
| exclude: | ||
| - rng-tools |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| def: | ||
| packages: | ||
| os: | ||
| include: | ||
| - alternatives | ||
| - audit-libs | ||
| - basesystem | ||
| - bash | ||
| - ca-certificates | ||
| - coreutils-single | ||
| - crypto-policies-scripts | ||
| - curl-minimal | ||
| - dejavu-sans-fonts | ||
| - dnf | ||
| - filesystem | ||
| - findutils | ||
| - gdb-gdbserver | ||
| # Differs from official UBI, as we don't include CRB repos | ||
| # - gdbm | ||
| - glibc-minimal-langpack | ||
| - gmp | ||
| - gnupg2 | ||
| - gobject-introspection | ||
| - hostname | ||
| - langpacks-en | ||
| - libcurl-minimal | ||
| - openssl | ||
| - pam | ||
| - passwd | ||
| - procps-ng | ||
| - python3 | ||
| - python3-inotify | ||
| - redhat-release | ||
| - rootfiles | ||
| - rpm | ||
| - sed | ||
| - setup | ||
| - shadow-utils | ||
| - subscription-manager | ||
| - systemd | ||
| - tar | ||
| - tpm2-tss | ||
| - tzdata | ||
| - util-linux | ||
| - vim-minimal | ||
| - yum | ||
| exclude: | ||
| - gawk-all-langpacks | ||
| - glibc-gconv-extra | ||
| - glibc-langpack-en | ||
| - openssl-pkcs11 | ||
| - python-unversioned-command | ||
| - redhat-release-eula | ||
| - rpm-plugin-systemd-inhibit |
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.
Using
> ...block quotes for emphasis might look nice here, but it will vary based on the markdown renderer (some will put quote marks around the block to make it clear it's a quote, for example). I think it's misleading and it's more important to have proper semantic markup for various reasons (like screen readers).If we want emphasis here, I'd go with bold or italics... or both.