pkg/utils: add ELN as supported distro - #1814
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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
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
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 |
2 participants
No description provided.