OCPBUGS-86774: Pin azure-cli to 2.72.0 in e2e Dockerfile#8638
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@bryan-cox: This pull request explicitly references no jira issue. 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. |
|
Skipping CI for Draft Pull Request. |
|
/test e2e-aws |
📝 WalkthroughWalkthroughThe final stage of Dockerfile.e2e now installs a pinned azure-cli package version: the dnf install invocation was changed to install azure-cli-2.72.0 instead of the unpinned azure-cli package. Suggested reviewers
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox 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 |
|
@bryan-cox: This pull request references Jira Issue OCPBUGS-86774, which is invalid:
Comment 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. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Dockerfile.e2e (4)
10-12:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBuild tools present in final image.
The final image intentionally reuses the builder base image to retain the Go toolchain (per the comment on line 10). This violates the container security principle of excluding build tools from the final runtime image, increasing the attack surface and image size.
Consider whether the
gocommand is essential for e2e test execution. If possible, refactor to use a minimal runtime base image and copy only the required compiled binaries and runtime dependencies.As per coding guidelines: "Multi-stage builds; no build tools in final image"
🤖 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 `@Dockerfile.e2e` around lines 10 - 12, The final image reuses the builder base (registry.ci.openshift.org/...-golang-1.25-...) and therefore contains the go toolchain; remove build tools from the runtime image by converting the Dockerfile.e2e into a proper multi-stage build: keep the current image as the build stage (reference to the existing FROM image) that compiles artifacts, then add a lightweight runtime stage (e.g., scratch/distroless/ubi-minimal) that only COPYs the compiled binaries and any needed runtime files; update ci-test-e2e.sh (which expects the go command) to run tests against the compiled binary in the runtime image or invoke go inside the build stage only, ensuring no go binary is present in the final image.
1-1:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBase image does not comply with approved catalog.
The base image
registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.25-openshift-4.23is not UBI minimal or distroless from catalog.redhat.com, and uses a pinned version tag instead of a floating tag. As per coding guidelines, Red Hat base images should use floating tags (Red Hat manages updates), and containers should prefer UBI minimal or distroless images.As per coding guidelines: "Base image: UBI minimal or distroless from catalog.redhat.com" and "Red Hat images: use floating tags (Red Hat manages updates)"
Also applies to: 12-12
🤖 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 `@Dockerfile.e2e` at line 1, The Dockerfile uses a non-approved pinned base image in the builder stage; replace the FROM image reference `registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.25-openshift-4.23` with an approved UBI minimal or distroless image from catalog.redhat.com and use a floating tag (for example the appropriate UBI minimal Go builder image with a floating tag like :latest or the recommended floating stream) while keeping the builder stage name (AS builder) intact; update the single FROM line accordingly so the build uses an approved, floating-tag Red Hat base image.
1-34:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftContainer runs as root, violating security requirements.
The Dockerfile does not specify a non-root
USERdirective, meaning the container will run as root by default. As per coding guidelines, containers must use a non-root user and never run as root.🔒 Proposed fix to add non-root user
COPY --from=builder /hypershift/hack/run-reqserving-e2e.sh /hypershift/hack/run-reqserving-e2e.sh +RUN useradd -r -u 1001 -g 0 hypershift && \ + chown -R 1001:0 /hypershift && \ + chmod -R g=u /hypershift + RUN rpm --import https://packages.microsoft.com/keys/microsoft.asc && \ dnf install -y https://packages.microsoft.com/config/rhel/9/packages-microsoft-prod.rpm && \ mv /etc/yum.repos.d/microsoft-prod.repo /etc/yum.repos.art/ci/ && \ dnf install -y --exclude='*.i686' azure-cli && \ dnf clean all + +USER 1001As per coding guidelines: "USER non-root; never run as root"
🤖 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 `@Dockerfile.e2e` around lines 1 - 34, The image currently runs as root because there's no USER specified; update the final stage (the second FROM block that sets WORKDIR /hypershift and copies binaries like /hypershift/bin/test-e2e and /hypershift/hack/ci-test-e2e.sh) to create a non-root user/group, ensure ownership of runtime directories (/hypershift, /hypershift/bin, /hypershift/hack) is changed to that user, and add a USER directive (e.g., a dedicated uid/gid) before the image exits so the container runs unprivileged instead of root; ensure the RUN step that installs packages still works in the build but file ownership is corrected afterward so the non-root user can execute the copied binaries and scripts.
1-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing HEALTHCHECK directive.
The Dockerfile does not define a
HEALTHCHECKinstruction. Health checks are essential for container orchestration platforms to determine container health and perform automatic recovery.💚 Proposed fix to add HEALTHCHECK
RUN rpm --import https://packages.microsoft.com/keys/microsoft.asc && \ dnf install -y https://packages.microsoft.com/config/rhel/9/packages-microsoft-prod.rpm && \ mv /etc/yum.repos.d/microsoft-prod.repo /etc/yum.repos.art/ci/ && \ dnf install -y --exclude='*.i686' azure-cli && \ dnf clean all + +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD test -f /hypershift/bin/hypershift || exit 1As per coding guidelines: "HEALTHCHECK defined"
🤖 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 `@Dockerfile.e2e` around lines 1 - 34, The Dockerfile lacks a HEALTHCHECK; add a HEALTHCHECK instruction (e.g. using CMD-SHELL) after the final RUN block to probe the container health by invoking a lightweight internal check such as /hypershift/hack/ci-test-e2e.sh or a small command against /hypershift/bin/hypershift, and include sensible options like --interval, --timeout, --start-period and --retries so orchestration can detect and restart unhealthy containers (place the HEALTHCHECK at the end of the Dockerfile, referencing the existing /hypershift/hack/ci-test-e2e.sh or /hypershift/bin/hypershift).
🤖 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.
Outside diff comments:
In `@Dockerfile.e2e`:
- Around line 10-12: The final image reuses the builder base
(registry.ci.openshift.org/...-golang-1.25-...) and therefore contains the go
toolchain; remove build tools from the runtime image by converting the
Dockerfile.e2e into a proper multi-stage build: keep the current image as the
build stage (reference to the existing FROM image) that compiles artifacts, then
add a lightweight runtime stage (e.g., scratch/distroless/ubi-minimal) that only
COPYs the compiled binaries and any needed runtime files; update ci-test-e2e.sh
(which expects the go command) to run tests against the compiled binary in the
runtime image or invoke go inside the build stage only, ensuring no go binary is
present in the final image.
- Line 1: The Dockerfile uses a non-approved pinned base image in the builder
stage; replace the FROM image reference
`registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.25-openshift-4.23`
with an approved UBI minimal or distroless image from catalog.redhat.com and use
a floating tag (for example the appropriate UBI minimal Go builder image with a
floating tag like :latest or the recommended floating stream) while keeping the
builder stage name (AS builder) intact; update the single FROM line accordingly
so the build uses an approved, floating-tag Red Hat base image.
- Around line 1-34: The image currently runs as root because there's no USER
specified; update the final stage (the second FROM block that sets WORKDIR
/hypershift and copies binaries like /hypershift/bin/test-e2e and
/hypershift/hack/ci-test-e2e.sh) to create a non-root user/group, ensure
ownership of runtime directories (/hypershift, /hypershift/bin,
/hypershift/hack) is changed to that user, and add a USER directive (e.g., a
dedicated uid/gid) before the image exits so the container runs unprivileged
instead of root; ensure the RUN step that installs packages still works in the
build but file ownership is corrected afterward so the non-root user can execute
the copied binaries and scripts.
- Around line 1-34: The Dockerfile lacks a HEALTHCHECK; add a HEALTHCHECK
instruction (e.g. using CMD-SHELL) after the final RUN block to probe the
container health by invoking a lightweight internal check such as
/hypershift/hack/ci-test-e2e.sh or a small command against
/hypershift/bin/hypershift, and include sensible options like --interval,
--timeout, --start-period and --retries so orchestration can detect and restart
unhealthy containers (place the HEALTHCHECK at the end of the Dockerfile,
referencing the existing /hypershift/hack/ci-test-e2e.sh or
/hypershift/bin/hypershift).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4a8fcb19-e3c9-4901-81e7-495b6d7833e4
📒 Files selected for processing (1)
Dockerfile.e2e
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8638 +/- ##
==========================================
- Coverage 45.84% 40.68% -5.17%
==========================================
Files 440 755 +315
Lines 52824 93363 +40539
==========================================
+ Hits 24218 37985 +13767
- Misses 26816 52645 +25829
- Partials 1790 2733 +943 see 315 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
azure-cli >= 2.73.0 requires python3.12, which is not available in the E4S/EUS repos that CI now uses after openshift/release#79773 switched from mirror2.openshift.com (GA content) to cdn.redhat.com. Pin to the last version (2.72.0) that depends on python3.9, which is available in E4S. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6dc40a8 to
286c605
Compare
|
/test e2e-aws |
Test Resultse2e-aws
e2e-aks
|
|
/test e2e-aks |
|
/area ci-tooling |
|
/jira refresh |
|
@bryan-cox: This pull request references Jira Issue OCPBUGS-86774, which is valid. The bug has been moved to the POST state. 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. |
|
/test e2e-aws |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/verified by e2e |
|
/pipeline required |
|
Scheduling tests matching the |
|
@bryan-cox: 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. |
|
/override codecov/project |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: codecov/project 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 kubernetes-sigs/prow repository. |
|
@bryan-cox: all tests passed! 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. |
2a57a32
into
openshift:main
|
@bryan-cox: Jira Issue Verification Checks: Jira Issue OCPBUGS-86774 Jira Issue OCPBUGS-86774 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
I now have the complete picture. Here is the analysis: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThe Root CauseThe root cause is a stale Codecov baseline, not any issue with this PR's changes. How Codecov computes project coverage: Codecov compares the full-repo coverage report uploaded for the PR head commit ( Why coverage appears to drop: Between Why the baseline was stale: Codecov's base report lags behind Threshold behavior: The Recommendations
Evidence
|
|
Fix included in release 5.0.0-0.nightly-2026-05-30-072431 |
Summary
azure-clito version 2.72.0 inDockerfile.e2eto fix 100% e2e CI failureProblem
After openshift/release#79773 switched CI RHEL 9 repos from
mirror2.openshift.com(GA content) tocdn.redhat.comE4S/EUS endpoints, thehypershift-tests-amd64Docker image build fails because:azure-cli >= 2.73.0requirespython3.12python3.12is not available in E4S/EUS reposdnf install azure-clipicks the latest version (2.86.0), which cannot be installedError:
Fix
Pin to
azure-cli-2.72.0, the last version that depends onpython3.9(available in E4S). Version boundary verified from Microsoft's RHEL 9 repo metadata:azure-cli <= 2.72.0→ requirespython3.9azure-cli >= 2.73.0→ requirespython3.12Timeline
DockerBuildFailedTest plan
/test e2e-awsto verify image builds successfullyFixes: OCPBUGS-86774
Summary by CodeRabbit