Skip to content

Revert "butane: replace LAYOUT with architecture before render"#4550

Merged
travier merged 1 commit into
mainfrom
revert-4525-butane-config
Jun 3, 2026
Merged

Revert "butane: replace LAYOUT with architecture before render"#4550
travier merged 1 commit into
mainfrom
revert-4525-butane-config

Conversation

@travier

@travier travier commented May 6, 2026

Copy link
Copy Markdown
Member

Reverts #4525

This changes does not work for coreos/fedora-coreos-config#4097 (reverted in coreos/fedora-coreos-config#4101) as the Butane config is parsed before the check to not run on s390x is done.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies the registerTestDir function in mantle/kola/harness.go by removing the manual substitution of the {{LAYOUT}} placeholder in Butane configurations and the corresponding comment. I have no feedback to provide.

@angelcerveraroldan angelcerveraroldan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Rolv-Apneseth

Copy link
Copy Markdown
Member

We dropped this from fedora-coreos-config but it's still used in bootupd: https://github.com/coreos/bootupd/blob/main/tests/kola/raid1/config.bu

@Rolv-Apneseth

Rolv-Apneseth commented May 15, 2026

Copy link
Copy Markdown
Member

I was gonna do the same split over there as we did for fcos, but is it actually worth having it duplicated? Can we just run fcos tests in the bootupd CI? I assume it's just a modification to this line

Edit: Regardless, we can probably revert that {{LAYOUT}} commit in bootupd anyway. I think the CI only runs against x86_64 hence why it wasn't a problem before.

@travier

travier commented May 20, 2026

Copy link
Copy Markdown
Member Author

Good catch. I agree that we should be able to remove the kola test from the bootupd repo now that it is in the main fedora-coreos-config one. I was looking for an example run but the CI is failing on the bootupd repo right now.

@Rolv-Apneseth

Copy link
Copy Markdown
Member

Good catch. I agree that we should be able to remove the kola test from the bootupd repo now that it is in the main fedora-coreos-config one. I was looking for an example run but the CI is failing on the bootupd repo right now.

Yeah, I believe @yasminvalim is looking into it at the moment

@Rolv-Apneseth

Copy link
Copy Markdown
Member

Created coreos/bootupd#1100 for tracking that failing CI in bootupd

@Rolv-Apneseth

Rolv-Apneseth commented May 28, 2026

Copy link
Copy Markdown
Member

That issue is now closed.

Looking at the CI in coreos/bootupd#1091 we can see that the pattern on that previously linked line (ext.*bootupd*) ends up running tests from bootupd and fedora-coreos-config:

[2026-05-28T13:36:21.359Z] + cosa kola run --rerun --allow-rerun-success=tags=needs-internet --build=latest --output-dir=/home/jenkins/agent/workspace/bootupd_PR-1091/tmp/kola-3Vxns/kola --on-warn-failure-exit-77 --arch=x86_64 --exttest /var/tmp/kola/bootupd --inst-insecure 'ext.*bootupd*' --qemu-firmware=uefi --parallel=auto
[2026-05-28T13:36:21.359Z] kola -p qemu --build latest run --rerun --allow-rerun-success=tags=needs-internet --on-warn-failure-exit-77 --arch=x86_64 --exttest /var/tmp/kola/bootupd --inst-insecure ext.*bootupd* --qemu-firmware=uefi --parallel=auto --output-dir /home/jenkins/agent/workspace/bootupd_PR-1091/tmp/kola-3Vxns/kola
[2026-05-28T13:36:21.614Z] === RUN   ext.bootupd.raid1
[2026-05-28T13:36:21.614Z] === RUN   ext.bootupd.test-bootupd
[2026-05-28T13:36:21.614Z] === RUN   ext.config.boot.bootupd-validate
[2026-05-28T13:36:21.614Z] === RUN   ext.config.boot.bootupd

Could we just update this to ext.*boot*, and also run a couple more of the boot-related tests?

Edit: looks like that would match a few tests that aren't relevant, e.g. ext.config.systemd.no-systemd-firstboot. I guess we split it into 2 patterns with ext.bootupd.* and ext.config.boot.*?

@travier

travier commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

Edit: looks like that would match a few tests that aren't relevant, e.g. ext.config.systemd.no-systemd-firstboot. I guess we split it into 2 patterns with ext.bootupd.* and ext.config.boot.*?

Sounds good to me

@Rolv-Apneseth

Copy link
Copy Markdown
Member

After coreos/bootupd#1104 is merged this should be safe to merge too

@travier travier force-pushed the revert-4525-butane-config branch from 3fb549b to 3bd3216 Compare June 2, 2026 22:31

@Rolv-Apneseth Rolv-Apneseth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@travier travier merged commit 12bb1c5 into main Jun 3, 2026
9 checks passed
@travier travier deleted the revert-4525-butane-config branch June 3, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants