pkg/utils: add ELN as supported distro - #1814
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for Fedora ELN to Toolbx by introducing ELN-specific utility functions, registering the "eln" distribution, and updating the documentation and tests. The reviewer noted that the host detection logic in getHostID() must be updated to check for VARIANT_ID == "eln" when ID == "fedora" to ensure Fedora ELN hosts are correctly identified and default to ELN containers instead of standard Fedora containers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
ping @debarshiray |
debarshiray
left a comment
There was a problem hiding this comment.
Interesting idea!
If Fedora ELN has ID=eln in os-release(5), then it seems natural to add it as a separate distro. Although, I hope we don't run into any namespace problems with other downstream distributors by not having a fedora prefix.
However, does this really work? From reading the changes, it seems that it uses the quay.io/fedora/eln:latest image. If I got that right, then:
$ toolbox create --image quay.io/fedora/eln:latest
Created container: eln-latest
Enter with: toolbox enter eln-latest
$ toolbox enter eln-latest
Error: failed to modify user rishi with UID 1000: usermod(1) not foundPoking at the image with podman run confirms the problem:
$ podman run --interactive --rm --tty quay.io/fedora/eln:latest /bin/bash
[root@bb0c5c79fdcc /]# useradd
bash: useradd: command not found
[root@bb0c5c79fdcc /]# usermod
bash: usermod: command not found
[root@bb0c5c79fdcc /]# passwd
bash: passwd: command not foundIn terms of having a similar user experience as the default host shell on Fedora or Red Hat Enterprise Linux Workstation, I see:
$ podman run --interactive --rm --tty quay.io/fedora/eln:latest /bin/bash
[root@e8a5adf33a34 /]#
[root@e8a5adf33a34 /]# which
bash: which: command not found
[root@e8a5adf33a34 /]# man
bash: man: command not found
Here is a complete list of image requirements for Toolbx with some context and explanation.
I wonder if it will be possible to add an eln-toolbox image, just like we have the fedora-toolbox and the UBI-based toolbox images. The reason for the separate Toolbx images is to reliably deal with problems like these without getting into a tug-of-war with the default base images for the distribution, which have different goals.
In this particular case, just like ELN is (Red Hat) Enterprise Linux Next, the eln-toolbox image can help with the onboarding of the toolbox image for the upcoming RHEL.
These CI failures should be solved by rebasing against |
|
We already have a toolbox image, |
That's fantastic! My apologies for misreading the code and assuming that this pull request uses Could you please point me to the sources of the We try to have a copy of the image sources in |
debarshiray
left a comment
There was a problem hiding this comment.
This looks very good to me! Just a few bureaucratic and nitpicky details:
Would you be willing to add yourself to .github/CODEOWNERS for the src/pkg/utils/eln.go file? You clearly know more about Fedora ELN than me. If you are unavailable, then I will be the fallback, but having you there will formally clarify who the subject matter expert is.
It will be good to add some system tests for the Fedora ELN image to test/system. The tests are tagged to make it possible run subsets of them. eg., simultaneously on different CI hosts to reduce time. It's done using a mixture of Bats' tags and a hacky TOOLBX_TEST_SYSTEM_TAGS environment variable to work around some missing feature in Bats.
The are some deficiencies in how the system tests handle multiple distributions and releases, but it shouldn't matter for a rolling-release distribution like ELN and you can mimic what's done for Arch Linux. eg., you have to add the name of the ELN Toolbx image to the IMAGES associative array or hash table in test/system/libs/helpers.bash, do the _pull_and_cache_distro_image dance in test/system/setup_suite.bash, and then sprinkle the tests across the test/system/*.bats files.
Some of the tests look a bit silly, but it seems worth it because of the growing host and container distro matrix. We might as well automate everything at the risk of repeating some things for the sake of mental peace and reassurance. :)
Nitpick: could you please add a link to this pull request to the Git commit message?
Aha, cool! I should have noticed the |
Could you please rebase against |
containers#1814 Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
|
Rebased and incorporated your suggestions. Haven't looked at |
|
recheck |
|
recheck |
|
recheck |
|
Hey @Rolv-Apneseth @TristanCacqueray ! Do we have a problem with setting up Fedora CoreOS hosts using the Next and Testing streams on Software Factory? They are consistently encountering |
Hey @debarshiray. I imagine there may have been an update breaking these on Software Factory if |
|
It looks like SELinux is preventing the image to boot, here are the console log for coreos-next: We are still investigating and we'll rebuild the image shortly. |
|
It's odd these only started failing on I'll have a look at the butane/ignition and see if there's some kind of mistake there. |
|
@Rolv-Apneseth images are rebuilt every day. Here is the recipe for coreos: https://gitlab.com/softwarefactory-project/config/-/blob/main/nodepool/dib-ansible/cloud-fedora-coreos-next.yaml?ref_type=heads The zuul-worker seems to be created with https://gitlab.com/softwarefactory-project/config/-/blob/main/nodepool/providers/fcos-ignition.bu?ref_type=heads , though it's odd that produces a relabeling error. |
|
Here is the full console log, could it be there is a race condition between |
This allows using the 'distro' option to create and enter Fedora ELN containers. Due to ELN's rolling-release model, the 'release' option isn't required. If 'release' is used, the accepted value is 'latest'. Note that the VERSION_ID field in os-release(5) on Fedora ELN contains a number representing the next major version of Red Hat Enterprise Linux, even though it follows a rolling-release model. Right now it is: VERSION_ID=11 containers#1814 Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
This allows using the 'distro' option to create and enter Fedora ELN containers. Due to ELN's rolling-release model, the 'release' option isn't required. If 'release' is used, the accepted value is 'latest'. Note that the VERSION_ID field in os-release(5) on Fedora ELN contains a number representing the next major version of Red Hat Enterprise Linux, even though it follows a rolling-release model. Right now it is: VERSION_ID=11 containers#1814 Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
This allows using the 'distro' option to create and enter Fedora ELN containers. Due to ELN's rolling-release model, the 'release' option isn't required. If 'release' is used, the accepted value is 'latest'. Note that the VERSION_ID field in os-release(5) on Fedora ELN contains a number representing the next major version of Red Hat Enterprise Linux, even though it follows a rolling-release model. Right now it is: VERSION_ID=11 containers#1814 Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
This allows using the 'distro' option to create and enter Fedora ELN containers. Due to ELN's rolling-release model, the 'release' option isn't required. If 'release' is used, the accepted value is 'latest'. Note that the VERSION_ID field in os-release(5) on Fedora ELN contains a number representing the next major version of Red Hat Enterprise Linux, even though it follows a rolling-release model. Right now it is: VERSION_ID=11 Some changes by Debarshi Ray. containers#1814 Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
recheck |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
This allows using the 'distro' option to create and enter Fedora ELN containers. Due to ELN's rolling-release model, the 'release' option isn't required. If 'release' is used, the accepted value is 'latest'. Note that the VERSION_ID field in os-release(5) on Fedora ELN contains a number representing the next major version of Red Hat Enterprise Linux, even though it follows a rolling-release model. Right now it is: VERSION_ID=11 Some changes by Debarshi Ray. containers#1814 Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
recheck |
|
Build failed. ❌ unit-test NODE_FAILURE Node(set) request 200-0000193282 failed in 0s |
|
recheck |
Thanks for poking at this on a Sunday! :) |
|
Build failed. ❌ unit-test NODE_FAILURE Node(set) request 200-0000193445 failed in 0s |
|
Looks like the provider is having some difficulties, we'll have a look tomorrow! |
|
recheck |
|
FYI there was a planned maintenance on vexxhost (our cloud provider) over this weekend, which probably resulted in the issues you faced. Now that the maintenance is over, you shouldn't see this type of issues anymore. |
I see! Yes, that makes sense. Thanks, @mhuin |
|
Thanks for the contribution, @yselkowitz ! @mhuin @Rolv-Apneseth @TristanCacqueray thanks for all the help with the CI! |
No description provided.