OCPBUGS-86978: modify tests to use rhel10 as the default stream#6167
OCPBUGS-86978: modify tests to use rhel10 as the default stream#6167sergiordlr wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/severity-important
|
@sergiordlr: This pull request references Jira Issue OCPBUGS-86978, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThis PR updates the machine-config-operator test suite to reflect RHEL10 as the default OS image stream. New helper functions enable dynamic discovery of the cluster-default OS image stream, the ChangesRHEL10 Default OS Image Stream Upgrade
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-ipv4-mco-disruptive-1of2 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-ipv4-mco-disruptive-2of2 periodic-ci-openshift-machine-config-operator-release-5.0-arm64-periodics-e2e-aws-mco-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-arm64-periodics-e2e-aws-mco-disruptive-techpreview-2of3 periodic-ci-openshift-machine-config-operator-release-5.0-arm64-periodics-e2e-aws-mco-disruptive-techpreview-3of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-azure-mco-disruptive |
|
@sergiordlr: trigger 8 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/053b64c0-64b6-11f1-8909-bf4cf000156e-0 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended-priv/const.go`:
- Line 213: The test currently assumes DefaultOSImageStream ("rhel-10") is
present and asserts cluster.status.defaultStream equals it; make this
verification conditional on the OSStreams feature gate by detecting whether
OSStreams is enabled before running the assertion in
test/extended-priv/mco_osimagestream.go: if the OSStreams gate is disabled, skip
the check (or the whole test); if enabled, either run the existing assertion
comparing the cluster.status.defaultStream to DefaultOSImageStream or
proactively run the equivalent command (oc get
osimagestreams.machineconfiguration.openshift.io cluster -o
jsonpath='{.status.defaultStream}') and assert it equals DefaultOSImageStream.
Ensure the code references the DefaultOSImageStream constant and the
cluster.status.defaultStream field when performing the conditional check.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f9053e2a-a7e1-49a1-a985-34fd9df32b83
📒 Files selected for processing (2)
test/extended-priv/const.gotest/extended-priv/mco_osimagestream.go
|
looks good |
|
/test unit |
|
/lgtm |
|
Scheduling tests matching the |
openshift-ci
Bot
added
the
approved
sergiordlr
force-pushed
the
use_rhel10_as_default_stream_in_tests
branch
from
4589ab3 to
69f1f49
Compare
6 passed, 0 failed, 4 skipped
We have executed the tests locally. |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-ipv4-mco-disruptive-1of2 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-ipv4-mco-disruptive-2of2 periodic-ci-openshift-machine-config-operator-release-5.0-arm64-periodics-e2e-aws-mco-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-arm64-periodics-e2e-aws-mco-disruptive-techpreview-2of3 periodic-ci-openshift-machine-config-operator-release-5.0-arm64-periodics-e2e-aws-mco-disruptive-techpreview-3of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-azure-mco-disruptive |
|
@sergiordlr: trigger 8 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/81208d40-68cb-11f1-98a4-a4895ce9e032-0 |
|
Scheduling tests matching the |
|
/test unit |
sergiordlr
force-pushed
the
use_rhel10_as_default_stream_in_tests
branch
from
69f1f49 to
394438e
Compare
|
Scheduling tests matching the |
|
/verified by @sergiordlr |
openshift-ci-robot
added
the
verified
|
@sergiordlr: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
openshift-ci-robot
removed
the
verified
|
@sergiordlr: This pull request references Jira Issue OCPBUGS-86978, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/e2e-ocl-shared/Containerfile.cowsay (2)
1-10: ⚖️ Poor tradeoffContainer security directives missing.
The following container security best practices from the coding guidelines are not implemented:
- No
USERdirective (container runs as root)- No
HEALTHCHECKdefined- Package manager cache not explicitly cleared in the final layer
While this is test infrastructure, these directives improve security posture and align with prodsec guidelines. As per coding guidelines, containers should run as non-root, include health checks, and avoid package manager cache in the final layer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e-ocl-shared/Containerfile.cowsay` around lines 1 - 10, Add three security directives to align with container security best practices: First, add a USER directive before the final ostree container commit to ensure the container runs as a non-root user rather than root. Second, add a HEALTHCHECK directive to define container health status monitoring. Third, explicitly clear the package manager cache by adding dnf clean all after the rpm-ostree operations in the final stage to remove unnecessary package manager metadata and reduce the image layer size. These changes should be incorporated into the final stage after the COPY operations and before the ostree container commit.Source: Coding guidelines
1-1: ⚖️ Poor tradeoffBase image violates container security guidelines.
The base image uses
quay.io/centos/centos:stream10instead of UBI minimal or distroless fromcatalog.redhat.com, and the floating tagstream10is not pinned by digest as required for non-Red Hat images. As per coding guidelines, Red Hat images should use floating tags (Red Hat manages updates), but non-RH images must be pinned by digest for reproducibility and security.Consider switching to a UBI-based image from the Red Hat catalog and pinning non-RH images by digest.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e-ocl-shared/Containerfile.cowsay` at line 1, The FROM statement uses a non-Red Hat image (quay.io/centos/centos:stream10) with a floating tag, which violates container security guidelines. Replace this base image with a UBI minimal or distroless image from the Red Hat catalog (catalog.redhat.com), which are the approved alternatives. If you must use a non-Red Hat image, pin it to a specific digest instead of using the floating tag stream10 to ensure reproducibility and security compliance.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e-ocl-shared/Containerfile.cowsay`:
- Line 8: The rpm-ostree override replace command with the --experimental flag
has || true appended, which silently masks build failures that should be caught.
Determine if this override is truly optional or expected to succeed. If it is
essential, remove the || true suffix to fail fast on errors. If the override is
genuinely optional, keep || true but add a detailed comment above the command
explaining why failures are acceptable (such as expected unavailability in
certain stream versions or experimental flag compatibility issues).
---
Nitpick comments:
In `@test/e2e-ocl-shared/Containerfile.cowsay`:
- Around line 1-10: Add three security directives to align with container
security best practices: First, add a USER directive before the final ostree
container commit to ensure the container runs as a non-root user rather than
root. Second, add a HEALTHCHECK directive to define container health status
monitoring. Third, explicitly clear the package manager cache by adding dnf
clean all after the rpm-ostree operations in the final stage to remove
unnecessary package manager metadata and reduce the image layer size. These
changes should be incorporated into the final stage after the COPY operations
and before the ostree container commit.
- Line 1: The FROM statement uses a non-Red Hat image
(quay.io/centos/centos:stream10) with a floating tag, which violates container
security guidelines. Replace this base image with a UBI minimal or distroless
image from the Red Hat catalog (catalog.redhat.com), which are the approved
alternatives. If you must use a non-Red Hat image, pin it to a specific digest
instead of using the floating tag stream10 to ensure reproducibility and
security compliance.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5ec8d580-9c3f-42f4-a25a-20a2a49e87f1
📒 Files selected for processing (1)
test/e2e-ocl-shared/Containerfile.cowsay
| COPY --from=centos /etc/pki/rpm-gpg/RPM-GPG-KEY-* /etc/pki/rpm-gpg/ | ||
| RUN sed -i 's/\$stream/9-stream/g' /etc/yum.repos.d/centos*.repo && \ | ||
| RUN sed -i 's/\$stream/10-stream/g' /etc/yum.repos.d/centos*.repo && \ | ||
| rpm-ostree override replace --experimental --from repo=baseos ncurses ncurses-libs ncurses-base || true && \ |
There was a problem hiding this comment.
Silent failure in rpm-ostree override may mask build issues.
The || true suffix on the rpm-ostree override replace --experimental command causes the build to continue even if the override fails. This could silently mask legitimate failures such as package unavailability in stream10, experimental flag rejection, or repository misconfiguration.
If the override is truly optional, add a comment explaining why failures are expected and acceptable. If the override is expected to succeed, remove || true to fail fast on errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e-ocl-shared/Containerfile.cowsay` at line 8, The rpm-ostree override
replace command with the --experimental flag has || true appended, which
silently masks build failures that should be caught. Determine if this override
is truly optional or expected to succeed. If it is essential, remove the || true
suffix to fail fast on errors. If the override is genuinely optional, keep ||
true but add a detailed comment above the command explaining why failures are
acceptable (such as expected unavailability in certain stream versions or
experimental flag compatibility issues).
|
/verified by @sergiordlr |
openshift-ci-robot
added
the
verified
|
@sergiordlr: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling tests matching the |
|
/retest-required |
sergiordlr
force-pushed
the
use_rhel10_as_default_stream_in_tests
branch
from
2c07de2 to
394438e
Compare
openshift-ci-robot
removed
the
verified
|
/verified by @sergiordlr |
openshift-ci-robot
added
the
verified
|
@sergiordlr: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: isabella-janssen, pablintino, sergiordlr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@sergiordlr: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
6 participants
- What I did
Modify the tests so that they check that the default stream is rhel10 and not rhel9
4.23 and 5.0 have the same code base, so we need to support both cases. In 5.0 the default stream will be rhel10, and in 4.23 the fault stream will be rhel9.
Trying to save some time we will only execute stream changes starting with the default value (10 -> 9 in 5.0, and 9 -> 10 in 4.23), we will skip the test cases testing the other way around.
Bundle:
Fix the e2e test to use the right stream for rhel10
- How to verify it
All tests should pass
Summary by CodeRabbit
Summary by CodeRabbit
cowsayContainerfile to move the CentOS base stage from stream9 to stream10 and adjust repo stream rewriting and rpm-ostree behavior accordingly.