Add support for building and testing container images on C11S and RHEL11#423
Add support for building and testing container images on C11S and RHEL11#423
Conversation
📝 WalkthroughWalkthroughThis pull request adds support for CentOS Stream 11 (c11s) and RHEL 11 (rhel11) across the project's build and testing infrastructure. Changes include updating CI/CD workflows, build configuration logic, testing utilities, documentation, and introducing new test Dockerfiles for the newly supported OS variants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request validationFailed🔴 Failed or pending statuses:
🔴 Approval - missing or changes were requested Success🟢 Review - Reviewed by a member |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test.sh (1)
67-71:⚠️ Potential issue | 🟡 MinorMissing
c11sin theIMAGE_NAMEregistry prefix condition.Line 67 checks for
c9s,c10s, andfedorato prepend$REGISTRYtoIMAGE_NAME, butc11sis not included. Sincec11salso usesREGISTRY := quay.io/(percommon.mk), it should be included here.Proposed fix
- if [ "${OS}" == "c9s" ] || [ "${OS}" == "c10s" ] || [ "${OS}" == "fedora" ]; then + if [ "${OS}" == "c9s" ] || [ "${OS}" == "c10s" ] || [ "${OS}" == "c11s" ] || [ "${OS}" == "fedora" ]; then export IMAGE_NAME="$REGISTRY$IMAGE_NAME"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test.sh` around lines 67 - 71, The OS check that prepends REGISTRY to IMAGE_NAME currently tests for "c9s", "c10s", and "fedora" but omits "c11s"; update the conditional that references the OS variable (the if block using [ "${OS}" == ... ] and exports IMAGE_NAME/REGISTRY) to include "c11s" alongside "c9s" and "c10s" so c11s images are correctly prefixed with $REGISTRY when setting IMAGE_NAME..github/workflows/container-tests.yml (1)
16-16:⚠️ Potential issue | 🟠 MajorMatrix
oslist does not includec11sorrhel11.The workflow matrix includes
["fedora", "c9s", "rhel8", "rhel9", "c10s", "rhel10"]but the newc11shandling added in lines 53-56 will never be executed sincec11sis not in the matrix. If testing on C11S is intended, add it to the matrix. Similarly,rhel11should be added if RHEL11 testing is needed.Proposed fix to include c11s and rhel11
- os: ["fedora", "c9s", "rhel8", "rhel9", "c10s", "rhel10"] + os: ["fedora", "c9s", "rhel8", "rhel9", "c10s", "rhel10", "c11s", "rhel11"]Note: If
rhel11is added, corresponding handling needs to be added in the RHEL section (around lines 60-74).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/container-tests.yml at line 16, The workflow matrix under the "os" array is missing "c11s" and "rhel11", so add "c11s" (and "rhel11" if you intend to test RHEL11) to the os list so the new c11s handling code will run; if you add "rhel11", also extend the RHEL handling logic (the existing rhel8/rhel9/rhel10 conditional/block) to include a branch or cases for "rhel11" to ensure corresponding setup/steps are executed.
🧹 Nitpick comments (1)
.github/workflows/container-tests.yml (1)
49-58: Consider usingeliffor cleaner conditional structure.The
else ifpattern creates nested conditionals requiring matchingfistatements. While syntactically correct, usingelifwould be more idiomatic and readable.Suggested refactor using elif
if [ "${{ matrix.os }}" == "c9s" ]; then compose="CentOS-Stream-9" context="CentOS Stream 9" tmt_plan="/c9s" - else if [ "${{ matrix.os }}" == "c10s" ]; then + elif [ "${{ matrix.os }}" == "c10s" ]; then compose="CentOS-Stream-10" context="CentOS Stream 10" tmt_plan="/c10s" else compose="CentOS-Stream-11" context="CentOS Stream 11" tmt_plan="/c11s" fi - fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/container-tests.yml around lines 49 - 58, Replace the nested "else if [ \"${{ matrix.os }}\" == \"c10s\" ]; then" block with an idiomatic "elif" chain to simplify the conditional; update the branch that sets compose, context, and tmt_plan (the assignments to compose, context, tmt_plan for CentOS-Stream-10/11) so they live under an "elif [ \"${{ matrix.os }}\" == \"c10s\" ]" and a final "else" (or "elif" for c11s) as appropriate, removing the extra nested fi to improve readability and maintain the same variable assignments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/failures/check/v0/Dockerfile.rhel11`:
- Around line 1-2: The base image in Dockerfile.rhel11 is incorrect: replace the
current FROM line that reads "FROM ubi10/s2i-core" with "FROM ubi11/s2i-core" so
the file name Dockerfile.rhel11 matches the RHEL11 base image; update the FROM
instruction in Dockerfile.rhel11 (and adjust any related LABEL or test fixture
metadata if this change affects expected test outputs) unless the use of ubi10
was intentionally part of the failing fixture.
---
Outside diff comments:
In @.github/workflows/container-tests.yml:
- Line 16: The workflow matrix under the "os" array is missing "c11s" and
"rhel11", so add "c11s" (and "rhel11" if you intend to test RHEL11) to the os
list so the new c11s handling code will run; if you add "rhel11", also extend
the RHEL handling logic (the existing rhel8/rhel9/rhel10 conditional/block) to
include a branch or cases for "rhel11" to ensure corresponding setup/steps are
executed.
In `@test.sh`:
- Around line 67-71: The OS check that prepends REGISTRY to IMAGE_NAME currently
tests for "c9s", "c10s", and "fedora" but omits "c11s"; update the conditional
that references the OS variable (the if block using [ "${OS}" == ... ] and
exports IMAGE_NAME/REGISTRY) to include "c11s" alongside "c9s" and "c10s" so
c11s images are correctly prefixed with $REGISTRY when setting IMAGE_NAME.
---
Nitpick comments:
In @.github/workflows/container-tests.yml:
- Around line 49-58: Replace the nested "else if [ \"${{ matrix.os }}\" ==
\"c10s\" ]; then" block with an idiomatic "elif" chain to simplify the
conditional; update the branch that sets compose, context, and tmt_plan (the
assignments to compose, context, tmt_plan for CentOS-Stream-10/11) so they live
under an "elif [ \"${{ matrix.os }}\" == \"c10s\" ]" and a final "else" (or
"elif" for c11s) as appropriate, removing the extra nested fi to improve
readability and maintain the same variable assignments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c3290a2-395d-47a0-bc36-e4e14cd3208c
📒 Files selected for processing (9)
.github/workflows/container-tests.ymlREADME.mdbuild.shcommon.mkgenerate_version_table.pytest-lib.shtest.shtests/failures/check/v0/Dockerfile.c11stests/failures/check/v0/Dockerfile.rhel11
| FROM ubi10/s2i-core | ||
| LABEL name=test-image |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other rhel Dockerfiles specify the FROM directive
fd "Dockerfile.rhel" tests/ --exec head -1 {}Repository: sclorg/container-common-scripts
Length of output: 152
🏁 Script executed:
# Check the directory structure of tests/failures/check
find tests/failures/check -type f -name "Dockerfile*" | head -20Repository: sclorg/container-common-scripts
Length of output: 401
🏁 Script executed:
# Look for any Dockerfiles in tests that DO use registry.redhat.io prefix
rg "registry\.redhat\.io" tests/ --type dockerfileRepository: sclorg/container-common-scripts
Length of output: 109
🏁 Script executed:
# Check the content of a few rhel Dockerfiles to see the full picture
fd "Dockerfile.rhel" tests/ --exec sh -c 'echo "=== {} ==="; head -5 "{}"'Repository: sclorg/container-common-scripts
Length of output: 438
🏁 Script executed:
# Look for any README or documentation in tests/failures/check
find tests/failures/check -type f \( -name "README*" -o -name "*.md" -o -name "*.txt" \) | head -10Repository: sclorg/container-common-scripts
Length of output: 108
🏁 Script executed:
# Check if there are any comments or instructions in the check directory
cat tests/failures/check/README 2>/dev/null || cat tests/failures/check/v0/README 2>/dev/null || echo "No README found"Repository: sclorg/container-common-scripts
Length of output: 90
🏁 Script executed:
# Look for any test scripts that might explain what these fixtures are for
fd "test.*check" tests/ -x head -20 {}Repository: sclorg/container-common-scripts
Length of output: 57
🏁 Script executed:
cat tests/failures/check/v0/README.mdRepository: sclorg/container-common-scripts
Length of output: 110
Version mismatch in base image: Dockerfile.rhel11 uses ubi10 instead of ubi11.
The Dockerfile uses FROM ubi10/s2i-core, but given the filename convention (Dockerfile.rhel11), it should likely use ubi11/s2i-core to be consistent with how other rhel Dockerfiles are named (e.g., Dockerfile.rhel10 uses ubi10/s2i-core). Verify if this mismatch is intentional for this test failure fixture.
🧰 Tools
🪛 Trivy (0.69.3)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/failures/check/v0/Dockerfile.rhel11` around lines 1 - 2, The base image
in Dockerfile.rhel11 is incorrect: replace the current FROM line that reads
"FROM ubi10/s2i-core" with "FROM ubi11/s2i-core" so the file name
Dockerfile.rhel11 matches the RHEL11 base image; update the FROM instruction in
Dockerfile.rhel11 (and adjust any related LABEL or test fixture metadata if this
change affects expected test outputs) unless the use of ubi10 was intentionally
part of the failing fixture.
There was a problem hiding this comment.
I think Petr did this on purpose, we don't have ubi11 images at this point. However, a comment and maybe even an opened issue (or different kind of reminder for ourselves for the future) would help us not forget to change this once it's time.
There was a problem hiding this comment.
🧩 Analysis chain
@hhorak, understood — thanks for the clarification! I'll open a GitHub issue as a reminder and suggest an inline comment for the Dockerfile.
Let me create the issue now:
In the meantime, here's a suggested inline comment for the Dockerfile to make the intent clear:
# TODO: Replace ubi10/s2i-core with ubi11/s2i-core once UBI 11 images are available
FROM ubi10/s2i-core
LABEL name=test-image🏁 Script executed:
#!/bin/bash
gh issue create \
--repo sclorg/container-common-scripts \
--title "Update Dockerfile.rhel11 base image from ubi10 to ubi11 once ubi11 images are available" \
--body "## Background
In \`tests/failures/check/v0/Dockerfile.rhel11\`, the base image is temporarily set to \`ubi10/s2i-core\` because \`ubi11\` images are not yet available.
Once UBI 11 images are published, this file should be updated to use \`FROM ubi11/s2i-core\` instead.
## File to update
- \`tests/failures/check/v0/Dockerfile.rhel11\`
## References
- PR: https://github.com/sclorg/container-common-scripts/pull/423
- Comment: https://github.com/sclorg/container-common-scripts/pull/423#discussion_r3085448970
Requested by: `@hhorak`" \
--assignee hhorak
Length of output: 61
@hhorak, I've opened issue #425 to track the update: #425
Additionally, I'd suggest adding a TODO comment directly in the Dockerfile so the intent is visible right in the code:
# TODO: Replace ubi10/s2i-core with ubi11/s2i-core once UBI 11 images are available
# Tracked in https://github.com/sclorg/container-common-scripts/issues/425
FROM ubi10/s2i-core
LABEL name=test-image✏️ Learnings added
Learnt from: hhorak
Repo: sclorg/container-common-scripts PR: 423
File: tests/failures/check/v0/Dockerfile.rhel11:1-2
Timestamp: 2026-04-15T10:43:19.514Z
Learning: In `tests/failures/check/v0/Dockerfile.rhel11` in the sclorg/container-common-scripts repository, the base image `FROM ubi10/s2i-core` is used intentionally as a temporary placeholder because UBI 11 (`ubi11`) images do not yet exist. This should be updated to `FROM ubi11/s2i-core` once UBI 11 images become available. Do not flag this as a version mismatch — it is a known, intentional workaround.
Learnt from: hhorak
Repo: sclorg/container-common-scripts PR: 423
File: tests/failures/check/v0/Dockerfile.rhel11:1-2
Timestamp: 2026-04-15T10:43:19.514Z
Learning: In `tests/failures/check/v0/Dockerfile.rhel11` in the sclorg/container-common-scripts repository, the base image `FROM ubi10/s2i-core` is used intentionally as a temporary placeholder because UBI 11 (`ubi11`) images do not yet exist. This should be updated to `FROM ubi11/s2i-core` once UBI 11 images become available. Tracked in https://github.com/sclorg/container-common-scripts/issues/425. Do not flag this as a version mismatch — it is a known, intentional workaround.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
hhorak
left a comment
hhorak
left a comment
There was a problem hiding this comment.
Let's add a comment about why using ubi10 in the container file for 11. Otherwise lgtm.
| FROM ubi10/s2i-core | ||
| LABEL name=test-image |
There was a problem hiding this comment.
I think Petr did this on purpose, we don't have ubi11 images at this point. However, a comment and maybe even an opened issue (or different kind of reminder for ourselves for the future) would help us not forget to change this once it's time.
|
Testing of this pull request is blocked by this MRs. https://gitlab.cee.redhat.com/platform-eng-core-services/sclorg-tmt-plans/-/merge_requests/340 |
on C11S and RHEL11 Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
phracek
force-pushed
the
add_support_rhel11_c11s
branch
from
0c92a49 to
935a8d7
Compare
Issue for it is created here: #425 Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
|
[test] |
Testing Farm results
|
2 participants
Add support for building and testing container images on C11S and RHEL11
In case in container source is Dockerfile.rhel11, then this pull request enables it.
No other chnages is done.
Summary by CodeRabbit
New Features
Documentation