distro: move rhel8 into a generic distro by mvo5 · Pull Request #1643 · osbuild/images

mvo5 · 2025-07-02T14:28:31Z

This PR moves rhel8 into a generic distro.

To do this two new concepts were required:

a way to add conditions for platforms, i.e. rhel < 8.9 generates non-uefi ec2 images
a way to transform distro names (i.e. rhel-810 -> rhel-8.10)

pkg/distro/generic/imagetype.go

internal/workload/yaml.go

thozza · 2025-07-12T20:45:43Z

Just a few quick drive-by comments:

CentOS-8 is EOL, you don't need to consider it at all or test it.
I don't think it is worth it to add YAML config for the Workload config. I would be in favor of ditching the use of them for image definitions and specifying everything in the image definition. We initially thought we'd use it for things like SAP, but it never materialized. The fact that you need to add it only for RHEL-8 is a testament that this was never adopted. I think that with YAML, we now have a much saner way of sharing "workload configuration" across image types. Also, using the workload means that the image type can't be customized, which defeats the added value of Image Builder.

mvo5 · 2025-07-14T09:06:05Z

Just a few quick drive-by comments:

* CentOS-8 is EOL, you don't need to consider it at all or test it.

* I don't think it is worth it to add YAML config for the Workload config. I would be in favor of ditching the use of them for image definitions and specifying everything in the image definition. We initially thought we'd use it for things like SAP, but it never materialized. The fact that you need to add it only for RHEL-8 is a testament that this was never adopted. I think that with YAML, we now have a much saner way of sharing "workload configuration" across image types. Also, using the workload means that the image type can't be customized, which defeats the added value of Image Builder.

Thanks, I love that suggestion. I opened #1660 with the minimal amount of work to drop eapWorkload() and the need to add workloadYAML. There is a bigger question if workload could be removed at all, I have a look at this. Currently we use it in the images.go code to create a "custom workload" based on the blueprint, I have not yet looked deep if we should keep that or not.

ondrejbudai

Awesome work, thanks! Thoughts inline. :)

ondrejbudai · 2025-07-17T11:20:26Z

pkg/distro/generic/imagetype.go

 		return warnings, err
 	}
+
+	// (see commit ff53493): We are not enabling disk


Disk customizations are available, but only if they don't contain a swap partition/LV.

pkg/distro/defs/loader.go

pkg/distro/defs/distros.yaml

pkg/distro/defs/rhel-8/distro.yaml

pkg/distro/defs/loader.go

bcl · 2025-07-17T17:24:43Z

pkg/distro/generic/imagetype.go

+				}
+				for _, lv := range partition.LogicalVolumes {
+					if lv.FSType == "swap" {
+						return warnings, fmt.Errorf("swap partition creation is not supported on %s %s", t.Arch().Distro().Name(), t.Arch().Name())


Should this point to an explanation of some kind? Not being able to setup swap is going to be (I suppose it already is) surprising.

pkg/distro/defs/loader_test.go

bcl · 2025-07-17T18:09:49Z

pkg/distro/defs/distros.yaml

    name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
    match: "rhel-8.[0-9]{,[0-9]}"
+    # rhel8 support being named "rhel-81" for "rhel-8.1" or "rhel-810" for "rhel-8.10" etc
+    transform_re: "(?P<name>rhel)-(?P<major>8)(?P<minor>[0-9]+)"


I think defs/README.md probably needs a new section on naming. We now have 3 fields related to it, and anyone not familiar with the naming quirks is going to be confused.

I will do this via a followup (if that is okay), I am wondering if the whole mess can be tamed a bit, maybe having a single name: rhel-(?P<name>rhel)-(?P<major>8)(?P<minor>[0-9]+) or something. We got here because I want:

the ability to specify a simple name like "centos-9"

the ability to match based on some pattern so that we preserve the ability to create new distros releases even if they are not part of the YAML yet

internally the nameVer is used a lot so the name has to be normalized when reading

it might be that the regex becomes a bit unwieldy, I need to play with it. What I liked about the fnmatch/glob style is that its very simple for the common case. But I'm rambling, I will explore this.

Fwiw I played with this a bit in https://github.com/osbuild/images/compare/main...mvo5:yaml-imageconfig-18-transform-re?expand=1 (last commit) but it needs more work and is maybe too ambitious, maabe just merging match/transform is enough.

This is not just about matching the name, but also matching the repositories. rhel-810 should still keep working with rhel-8.10.json repo definition. https://github.com/osbuild/osbuild-composer/blob/main/test/cases/weldr-distro-dot-notation-and-aliases.sh should cover all scenarios that are expected to work.

schutzbot · 2025-07-18T14:24:57Z

A previous version of this PR changed the images API or behaviour causing integration issues with osbuild-composer.
This is now fixed.

mvo5 · 2025-07-22T14:50:28Z

Thanks a lot for the feedback! I had to force push once more to fix a conflict (and fixed one bug that Tomáš spotted along the way.

We actually need to check options based on the "distro-like" distro major version. Between rhel8 and rhel9 it seems some customizations are enabled that did not work before so until we have a more declarative way (and more tests) for the options checking we move a step "back" and bring back a lot of duplicated code from the original options checks, i.e. I went back and collected original "checkOptions()" code from before they all got folded into the "generic" checkOptions() to ensure maximum compatibility with the old options checking. This will make rhel8: #1643 but especially rhel9: #1645 a lot easier. I expect most (all?) of the options.go file to go away when #1216 gets merged.

This test used to test that the rhel distro factory does not match on non-rhel10 distro names. However there is no rhel10 specific distro anymore so these test-cases can get removed.

We currently hardcode `IoT` in the generic images.go helpers, avoid this and get variant from from YAML. Likewise, do not hardcode adding dbus-broker, this is the wrong layer and needs to happen at the YAML layer.

On rhel-8 we change the platform based on the version, i.e ec2 images before rhel-8.9 are bios only. This needs a new conditional. Because it is an override and the order of conditionals is (currently) undefined (because its a go map) it will error if multiple conditions match. Thanks to Ondrej for this idea.

This commit move rhel8 into a generic distro.

This commit adds a new `transform_re` expression to the distro yaml so that a regexp can be used to translate a name into the canonical "name-major.minor". This is needed for rhel-8 where there is support to create a distro with `rhel-810` instead of `rhel-8.10`. This commit also includes the regexp for rhel8

Move the rhel8 specific tests into the generic distro tests so that we don't lose test coverage.

This commit adds `edge-{commit,container}` to the skipTest for TestRH8_DiskCustomizationsCheckOptions(). The reason is that the original rhel8 checkOptions() did not check if partitioning customizations were used (unlike e.g. rhel10) so the original rhel8 check was incomplete, with the new stricter check we need to skip these image types just like we do on rhel-10.

For the special cases in rhel8,rhel9 we need a version of ParseID that comes from YAML (or it could have come from starlark if this was a PR that would have been actually accepted). So this commit implements `defs.ParseID()` that will apply any `transform_re` expressions that can be used to create special cases for naming of distros.

Describe the new `platforms_override` key and also document the new `not_distro_name` condition. Thanks to bcl for suggesting this.

This commit fixes a bug in the kernel options handling when getting the kernel options for the image types. Both `osCustomizations` and `ostreeDeploymentCustomizations` need fixing. Instead of the defaultImageConfig we need to get the inherited value from distro default image config and image type image config when looking for the kernel options. Thanks to Tomáš for spotting this!

tag v0.165.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.165.0 ---------------- * distro: move rhel9 into a generic distro (osbuild/images#1645) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * Revert "distro: drop `ImageType.BasePartitionTable()`" (osbuild/images#1691) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * Update dependencies 2025-07-20 (osbuild/images#1675) * Author: SchutzBot, Reviewers: Achilleas Koutsou, Simon de Vlieger * defs: add missing `bootstrap_containers` (osbuild/images#1679) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * disk: handle adding `PReP` partition on PPC64/s390x (HMS-8884) (osbuild/images#1681) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * distro: bring per-distro checkOptions back (osbuild/images#1678) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * distro: cleanups in the pkg/distro/generic area (osbuild/images#1686) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * distro: move rhel8 into a generic distro (osbuild/images#1643) * Author: Michael Vogt, Reviewers: Nobody * distro: small followups for PR#1682 (osbuild/images#1689) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger, Tomáš Hozza * distro: unify transform/match into a single concept (osbuild/images#1682) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Tomáš Hozza * distros: de-duplicate runner build packages for centos10 (osbuild/images#1680) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * github: disable Go dep updates through dependabot (osbuild/images#1683) * Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza * repos: include almalinux 9.6 (osbuild/images#1677) * Author: Simon de Vlieger, Reviewers: Lukáš Zapletal, Tomáš Hozza * rhel9: wsl distribution config (osbuild/images#1694) * Author: Simon de Vlieger, Reviewers: Michael Vogt, Sanne Raymaekers * test/manifests/all-customizations: don't embed local file via URI (osbuild/images#1684) * Author: Tomáš Hozza, Reviewers: Achilleas Koutsou, Brian C. Lane — Somewhere on the Internet, 2025-07-28 --- tag v0.166.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.166.0 ---------------- * customizations/subscription: conditionally enable semanage call (HMS-8866) (osbuild/images#1673) * Author: Sanne Raymaekers, Reviewers: Achilleas Koutsou, Michael Vogt * distro/rhel-10: versionlock shim-x64 in the azure-cvm image (osbuild/images#1697) * Author: Achilleas Koutsou, Reviewers: Michael Vogt, Simon de Vlieger * manifestmock: move container/pkg/commit mocks into helper (osbuild/images#1700) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * rhel9: `vagrant-libvirt`, `vagrant-virtualbox` (osbuild/images#1693) * Author: Simon de Vlieger, Reviewers: Michael Vogt, Sanne Raymaekers * rhel{9,10}: centos WSL refinement (HMS-8922) (osbuild/images#1690) * Author: Simon de Vlieger, Reviewers: Ondřej Budai, Sanne Raymaekers, Tomáš Hozza — Somewhere on the Internet, 2025-07-29 --- tag v0.167.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.167.0 ---------------- * RHEL/Azure: drop obsolete WAAgentConfig keys [RHEL-93894] and remove loglevel kernel option [RHEL-102372] (osbuild/images#1611) * Author: Achilleas Koutsou, Reviewers: Michael Vogt, Ondřej Budai, Sanne Raymaekers * Update dependencies 2025-07-27 (osbuild/images#1699) * Author: SchutzBot, Reviewers: Achilleas Koutsou, Simon de Vlieger * distro/rhel9: set default_kernel to kernel-uki-virt (osbuild/images#1704) * Author: Achilleas Koutsou, Reviewers: Ondřej Budai, Simon de Vlieger * distro: drop legacy loaders and update tests (osbuild/images#1687) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Tomáš Hozza * distro: fix issues with yaml distro definitions and enable yaml checks (osbuild/images#1702) * Author: Achilleas Koutsou, Reviewers: Michael Vogt, Ondřej Budai, Simon de Vlieger — Somewhere on the Internet, 2025-07-30 ---

tag v0.165.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.165.0 ---------------- * distro: move rhel9 into a generic distro (osbuild/images#1645) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * Revert "distro: drop `ImageType.BasePartitionTable()`" (osbuild/images#1691) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * Update dependencies 2025-07-20 (osbuild/images#1675) * Author: SchutzBot, Reviewers: Achilleas Koutsou, Simon de Vlieger * defs: add missing `bootstrap_containers` (osbuild/images#1679) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * disk: handle adding `PReP` partition on PPC64/s390x (HMS-8884) (osbuild/images#1681) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * distro: bring per-distro checkOptions back (osbuild/images#1678) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * distro: cleanups in the pkg/distro/generic area (osbuild/images#1686) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * distro: move rhel8 into a generic distro (osbuild/images#1643) * Author: Michael Vogt, Reviewers: Nobody * distro: small followups for PR#1682 (osbuild/images#1689) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger, Tomáš Hozza * distro: unify transform/match into a single concept (osbuild/images#1682) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Tomáš Hozza * distros: de-duplicate runner build packages for centos10 (osbuild/images#1680) * Author: Michael Vogt, Reviewers: Simon de Vlieger, Tomáš Hozza * github: disable Go dep updates through dependabot (osbuild/images#1683) * Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza * repos: include almalinux 9.6 (osbuild/images#1677) * Author: Simon de Vlieger, Reviewers: Lukáš Zapletal, Tomáš Hozza * rhel9: wsl distribution config (osbuild/images#1694) * Author: Simon de Vlieger, Reviewers: Michael Vogt, Sanne Raymaekers * test/manifests/all-customizations: don't embed local file via URI (osbuild/images#1684) * Author: Tomáš Hozza, Reviewers: Achilleas Koutsou, Brian C. Lane — Somewhere on the Internet, 2025-07-28 --- tag v0.166.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.166.0 ---------------- * customizations/subscription: conditionally enable semanage call (HMS-8866) (osbuild/images#1673) * Author: Sanne Raymaekers, Reviewers: Achilleas Koutsou, Michael Vogt * distro/rhel-10: versionlock shim-x64 in the azure-cvm image (osbuild/images#1697) * Author: Achilleas Koutsou, Reviewers: Michael Vogt, Simon de Vlieger * manifestmock: move container/pkg/commit mocks into helper (osbuild/images#1700) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * rhel9: `vagrant-libvirt`, `vagrant-virtualbox` (osbuild/images#1693) * Author: Simon de Vlieger, Reviewers: Michael Vogt, Sanne Raymaekers * rhel{9,10}: centos WSL refinement (HMS-8922) (osbuild/images#1690) * Author: Simon de Vlieger, Reviewers: Ondřej Budai, Sanne Raymaekers, Tomáš Hozza — Somewhere on the Internet, 2025-07-29 --- tag v0.167.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.167.0 ---------------- * RHEL/Azure: drop obsolete WAAgentConfig keys [RHEL-93894] and remove loglevel kernel option [RHEL-102372] (osbuild/images#1611) * Author: Achilleas Koutsou, Reviewers: Michael Vogt, Ondřej Budai, Sanne Raymaekers * Update dependencies 2025-07-27 (osbuild/images#1699) * Author: SchutzBot, Reviewers: Achilleas Koutsou, Simon de Vlieger * distro/rhel9: set default_kernel to kernel-uki-virt (osbuild/images#1704) * Author: Achilleas Koutsou, Reviewers: Ondřej Budai, Simon de Vlieger * distro: drop legacy loaders and update tests (osbuild/images#1687) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Tomáš Hozza * distro: fix issues with yaml distro definitions and enable yaml checks (osbuild/images#1702) * Author: Achilleas Koutsou, Reviewers: Michael Vogt, Ondřej Budai, Simon de Vlieger — Somewhere on the Internet, 2025-07-30 --- tag v0.168.0 Tagger: imagebuilder-bot <imagebuilder-bots+imagebuilder-bot@redhat.com> Changes with 0.168.0 ---------------- * distro: fix bug in variable substitution for static distros (osbuild/images#1710) * Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger * rhel{9,10}: azure for non-RHEL (HMS-8949) (osbuild/images#1707) * Author: Simon de Vlieger, Reviewers: Achilleas Koutsou, Michael Vogt — Somewhere on the Internet, 2025-07-30 ---

mvo5 force-pushed the yaml-imageconfig-18 branch 5 times, most recently from b96908a to c6a8b95 Compare July 9, 2025 15:34

mvo5 marked this pull request as ready for review July 9, 2025 15:37

mvo5 requested a review from a team as a code owner July 9, 2025 15:37

mvo5 requested review from achilleas-k, supakeen and thozza July 9, 2025 15:37

mvo5 force-pushed the yaml-imageconfig-18 branch 2 times, most recently from 702a06b to b015ebf Compare July 9, 2025 18:59

mvo5 commented Jul 9, 2025

View reviewed changes

pkg/distro/generic/imagetype.go Outdated Show resolved Hide resolved

supakeen reviewed Jul 10, 2025

View reviewed changes

internal/workload/yaml.go Outdated Show resolved Hide resolved

mvo5 force-pushed the yaml-imageconfig-18 branch from b015ebf to 402e65d Compare July 17, 2025 10:13

mvo5 requested a review from ondrejbudai July 17, 2025 11:00

mvo5 force-pushed the yaml-imageconfig-18 branch from 402e65d to ca538aa Compare July 17, 2025 11:39

ondrejbudai reviewed Jul 17, 2025

View reviewed changes

bcl reviewed Jul 17, 2025

View reviewed changes

pkg/distro/defs/loader_test.go Show resolved Hide resolved

bcl reviewed Jul 17, 2025

View reviewed changes

mvo5 force-pushed the yaml-imageconfig-18 branch 2 times, most recently from 88c85ea to 9cacf59 Compare July 18, 2025 14:18

mvo5 force-pushed the yaml-imageconfig-18 branch 2 times, most recently from a640d5f to 1b47a45 Compare July 21, 2025 09:45

mvo5 requested review from bcl and ondrejbudai July 21, 2025 10:24

mvo5 dismissed bcl’s stale review via 69418d0 July 22, 2025 14:48

mvo5 force-pushed the yaml-imageconfig-18 branch from 29f7226 to 69418d0 Compare July 22, 2025 14:48

mvo5 force-pushed the yaml-imageconfig-18 branch from 69418d0 to bab49f5 Compare July 22, 2025 15:37

mvo5 added 14 commits July 22, 2025 17:52

distro: drop non-rhel10 tests from TestRH10DistroFactory

1f85214

This test used to test that the rhel distro factory does not match on non-rhel10 distro names. However there is no rhel10 specific distro anymore so these test-cases can get removed.

distro: drop rhel-8.4 from TestDistroFactory

3f700e8

distro: fix hardcoded "IoT" variant and dbus-broker

31621ab

We currently hardcode `IoT` in the generic images.go helpers, avoid this and get variant from from YAML. Likewise, do not hardcode adding dbus-broker, this is the wrong layer and needs to happen at the YAML layer.

distro: move rhel8 to the generic distro

f644c18

This commit move rhel8 into a generic distro.

distro: add centos-8 to distros.yaml

08ace26

distro: port rhel8 tests into the generic distro tests

7df8e17

Move the rhel8 specific tests into the generic distro tests so that we don't lose test coverage.

distro: rm -rf rhel8

16f2ecc

distro: describe new platforms_override

06e2c5e

Describe the new `platforms_override` key and also document the new `not_distro_name` condition. Thanks to bcl for suggesting this.

distro: add missing bootstrap_containers for rhel8

5746618

mvo5 force-pushed the yaml-imageconfig-18 branch from bab49f5 to 791769f Compare July 22, 2025 15:53

thozza approved these changes Jul 22, 2025

View reviewed changes

supakeen approved these changes Jul 23, 2025

View reviewed changes

supakeen added this pull request to the merge queue Jul 23, 2025

Merged via the queue into osbuild:main with commit 7eede89 Jul 23, 2025
21 checks passed

achilleas-k mentioned this pull request Jul 30, 2025

go.mod: update osbuild/images to v0.168.0 osbuild/osbuild-composer#4799

Merged

Conversation

mvo5 commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thozza commented Jul 12, 2025

Uh oh!

mvo5 commented Jul 14, 2025

Uh oh!

ondrejbudai left a comment

Choose a reason for hiding this comment

Uh oh!

ondrejbudai Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcl Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bcl Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

mvo5 Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

mvo5 Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

thozza Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

schutzbot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mvo5 commented Jul 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mvo5 commented Jul 2, 2025 •

edited

Loading

schutzbot commented Jul 18, 2025 •

edited

Loading