Skip to content

OCPBUGS-86662: Optimize CPO deployment polling interval in tests of additional trust bundle propagation#8617

Open
YamunadeviShanmugam wants to merge 1 commit into
openshift:mainfrom
YamunadeviShanmugam:fix-e2e-tests-inconsistent-timeout-agressive-api-polling
Open

OCPBUGS-86662: Optimize CPO deployment polling interval in tests of additional trust bundle propagation#8617
YamunadeviShanmugam wants to merge 1 commit into
openshift:mainfrom
YamunadeviShanmugam:fix-e2e-tests-inconsistent-timeout-agressive-api-polling

Conversation

@YamunadeviShanmugam

@YamunadeviShanmugam YamunadeviShanmugam commented May 28, 2026

Copy link
Copy Markdown
Contributor

This PR addresses inconsistent test timeouts in the trust bundle propagation tests. The root cause was aggressive API polling during the CPO deployment check, which lacked an explicit interval configuration and defaulted to the global 3s threshold.

By explicitly setting this check to 15s, we reduce API polling frequency by 5x, preventing API throttling

Summary by CodeRabbit

  • Tests
    • Centralized and reused timing parameters in node pool end-to-end tests for improved maintainability and consistency
    • Increased timeout for node pool status validation to reduce flakiness and improve reliability

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels May 28, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@YamunadeviShanmugam: This pull request references Jira Issue OCPBUGS-86662, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Problem
CI fails in the TestAdditionalTrustBundlePropagation test suite due to timeout paired with overly aggressive API polling. During the test, injecting a custom trust bundle triggers a rolling update that requires spinning up new cloud vms, executing Ignition configs, and waiting for the new nodes to fully register which takes ~30mins to complete successfully. Because the test cuts off at exactly 20 minutes, it flags healthy, progressing rollouts as false-negative timeouts. Compounding this issue, different segments of the test poll the K8s API server for status updates as frequently as every 3 to 10 seconds. This hyper-aggressive polling floods the management cluster with traffic, triggering client-side rate limiters that cause API starvation and crash the test blocks.

Fix
To resolve this instability, the test has been refactored to replace the hardcoded time constraints with constants. Timeouts and Polling interval are updated accordingly.

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.

@openshift-ci openshift-ci Bot requested review from sdminonne and sjenning May 28, 2026 08:28
@openshift-ci openshift-ci Bot added area/testing Indicates the PR includes changes for e2e testing and removed do-not-merge/needs-area labels May 28, 2026
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR centralizes E2E test polling and timeout values by adding shared constants in test/e2e/nodepool_additionalTrustBundlePropagation_test.go and applying them to all related waits. It increases the NodePool condition wait in test/e2e/nodepool_test.go from 20 to 30 minutes. Dockerfile.e2e is updated to install azure-cli via dnf with added flags: --nobest and --setopt=install_weak_deps=False.

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title mentions optimizing CPO deployment polling interval, but the actual changes also address inconsistent timeouts and include unrelated Docker build optimization for Azure CLI installation. Update the title to reflect all key changes: optimizing polling intervals, increasing timeouts for node pool status validation, and updating Azure CLI installation options. Consider a more comprehensive title like 'OCPBUGS-86662: Fix inconsistent timeouts and aggressive API polling in additional trust bundle propagation tests'.
✅ Passed checks (10 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR introduces no dynamic test names. Test in nodepool_additionalTrustBundlePropagation_test.go uses static name; nodepool_test.go only modified timeout values.
Test Structure And Quality ✅ Passed Test code meets quality standards: proper timeout constants defined, all Eventually calls have explicit timeouts, descriptive error messages present, consistent e2eutil patterns used throughout.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only E2E test files and test Docker infrastructure. No deployment manifests, operator code, controllers, or topology-aware scheduling constraints are introduced.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR doesn't add Ginkgo e2e tests (check requirement). New test uses Go testing.T/t.Run() pattern. No IPv4 assumptions or external connectivity detected in code.
No-Weak-Crypto ✅ Passed No weak cryptography found. Changes involve test timing parameters, timeout adjustments, and Docker build configuration only—no crypto implementations or insecure patterns.
Container-Privileges ✅ Passed PR contains no Kubernetes manifests or container privilege escalation. Changes are E2E test timing refactoring and Dockerfile dnf install constraint flags (security improvement).
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data exposure in logs. Changes involve timeout refactoring and Docker build parameters with existing logging only showing test names and hardcoded constants.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci-robot

Copy link
Copy Markdown

@YamunadeviShanmugam: This pull request references Jira Issue OCPBUGS-86662, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

Problem
CI fails in the TestAdditionalTrustBundlePropagation test suite due to timeout paired with overly aggressive API polling. During the test, injecting a custom trust bundle triggers a rolling update that requires spinning up new cloud vms, executing Ignition configs, and waiting for the new nodes to fully register which takes ~30mins to complete successfully. Because the test cuts off at exactly 20 minutes, it flags healthy, progressing rollouts as false-negative timeouts. Compounding this issue, different segments of the test poll the K8s API server for status updates as frequently as every 3 to 10 seconds. This hyper-aggressive polling floods the management cluster with traffic, triggering client-side rate limiters that cause API starvation and crash the test blocks.

Fix
To resolve this instability, the test has been refactored to replace the hardcoded time constraints with constants. Timeouts and Polling interval are updated accordingly.

Summary by CodeRabbit

  • Tests
  • Refactored timing parameters in node pool end-to-end tests for improved maintainability
  • Increased validation timeout for node pool status checks to enhance test reliability

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.

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/e2e/nodepool_additionalTrustBundlePropagation_test.go (1)

137-137: ⚡ Quick win

Extract hardcoded timeout to a constant for consistency.

Both locations use a hardcoded 5*time.Minute timeout for user-ca-bundle operations, which is inconsistent with the PR's stated objective to "replace hardcoded time constraints with constants." Consider extracting this value as userCABundleTimeout in the const block (lines 57-62) and referencing it here.

♻️ Suggested refactor

Add to the const block:

 const (
 	nodePoolConfigUpdateStartTimeout  = 5 * time.Minute
 	nodePoolConfigUpdateFinishTimeout = 30 * time.Minute
 	nodePoolConfigUpdatePollInterval  = 15 * time.Second
 	cpoDeploymentUpdateTimeout        = 10 * time.Minute
+	userCABundleTimeout               = 5 * time.Minute
 )

Then update both call sites:

-		e2eutil.WithInterval(nodePoolConfigUpdatePollInterval), e2eutil.WithTimeout(5*time.Minute),
+		e2eutil.WithInterval(nodePoolConfigUpdatePollInterval), e2eutil.WithTimeout(userCABundleTimeout),

Also applies to: 219-219

🤖 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/nodepool_additionalTrustBundlePropagation_test.go` at line 137,
Replace the hardcoded 5*time.Minute timeout used in e2eutil.WithTimeout calls
with a named constant by adding userCABundleTimeout to the existing const block
(currently containing other timeouts) and then change the
e2eutil.WithTimeout(5*time.Minute) call sites (e.g., the call that currently
uses e2eutil.WithInterval(nodePoolConfigUpdatePollInterval),
e2eutil.WithTimeout(5*time.Minute)) to use
e2eutil.WithTimeout(userCABundleTimeout) so both locations reference the new
constant.
🤖 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.

Nitpick comments:
In `@test/e2e/nodepool_additionalTrustBundlePropagation_test.go`:
- Line 137: Replace the hardcoded 5*time.Minute timeout used in
e2eutil.WithTimeout calls with a named constant by adding userCABundleTimeout to
the existing const block (currently containing other timeouts) and then change
the e2eutil.WithTimeout(5*time.Minute) call sites (e.g., the call that currently
uses e2eutil.WithInterval(nodePoolConfigUpdatePollInterval),
e2eutil.WithTimeout(5*time.Minute)) to use
e2eutil.WithTimeout(userCABundleTimeout) so both locations reference the new
constant.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 1a09827c-9c36-47e4-8c0e-6753421c9155

📥 Commits

Reviewing files that changed from the base of the PR and between 0aa20fc and e746a44.

📒 Files selected for processing (2)
  • test/e2e/nodepool_additionalTrustBundlePropagation_test.go
  • test/e2e/nodepool_test.go

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.43%. Comparing base (f13c62d) to head (3b08f70).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8617   +/-   ##
=======================================
  Coverage   41.43%   41.43%           
=======================================
  Files         756      756           
  Lines       93647    93647           
=======================================
  Hits        38802    38802           
  Misses      52124    52124           
  Partials     2721     2721           
Flag Coverage Δ
cmd-support 34.87% <ø> (ø)
cpo-hostedcontrolplane 43.50% <ø> (ø)
cpo-other 42.74% <ø> (ø)
hypershift-operator 51.57% <ø> (ø)
other 31.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cwbotbot

cwbotbot commented May 28, 2026

Copy link
Copy Markdown

Test Results

e2e-aws

e2e-aks

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/pipeline required

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (3)
Dockerfile.e2e (3)

1-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

HEALTHCHECK is missing.

No HEALTHCHECK directive is defined. Container orchestrators cannot determine container health without it. As per coding guidelines, "HEALTHCHECK defined."

🏥 Proposed fix to add HEALTHCHECK

Add at the end of the Dockerfile:

     dnf install -y azure-cli-2.85.0-1.el9 && \
     dnf clean all
+
+HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
+    CMD ["/hypershift/bin/hypershift", "version"] || exit 1

Note: Adjust the health check command based on the most appropriate validation for your e2e container.

🤖 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 - 35, The Dockerfile is missing a HEALTHCHECK
directive; add a HEALTHCHECK at the end of the file that runs an appropriate
lightweight check (for example invoking /hypershift/hack/ci-test-e2e.sh or
/hypershift/bin/test-e2e with a quick status subcommand) using CMD-SHELL, and
include sensible options like --interval=30s --timeout=10s --retries=3; ensure
the referenced script/binary (ci-test-e2e.sh or test-e2e) is executable in the
image and that the health command exits with 0 on success so container
orchestrators can detect unhealthy containers.

1-35: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Container runs as root - critical security violation.

No USER directive is specified. The container will run as root, which violates the security guideline. E2E test containers should run as a non-root user. As per coding guidelines, "USER non-root; never run as root."

🔒 Proposed fix to add non-root user

Add before the final RUN instruction:

 COPY --from=builder /hypershift/hack/run-reqserving-e2e.sh /hypershift/hack/run-reqserving-e2e.sh

+RUN useradd -m -u 1001 -s /bin/bash hypershift && \
+    chown -R 1001:1001 /hypershift
+
+USER 1001
+
 RUN rpm --import https://packages.microsoft.com/keys/microsoft.asc && \
🤖 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 - 35, The image currently runs as root because
there's no USER directive; before the final RUN that installs packages (the RUN
starting with "rpm --import ...") add commands to create a non-root user and
group (e.g., create "hypershift" with a fixed UID/GID), chown the WORKDIR and
copied bins (refer to WORKDIR /hypershift and the copied paths like
/hypershift/bin and /hypershift/hack), and then set USER hypershift so the
remaining layers run unprivileged; ensure the created user has a valid shell and
ownership of /hypershift to allow the subsequent scripts (ci-test-e2e.sh,
run-reqserving-e2e.sh) and binaries (test-e2e, hypershift, etc.) to be
executable by that user.

10-12: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Build tools present in final image.

The final stage reuses the golang builder image, which includes the Go compiler and build toolchain. This violates the security guideline. Consider extracting only the necessary runtime dependencies or using a minimal base image for the final stage. 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 currently reuses the
full Go builder image (the FROM line referencing
registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.25-openshift-4.23)
which leaves build tools in the runtime image; change the Dockerfile to use a
proper multi-stage build: keep a builder stage that compiles artifacts, then add
a distinct final stage FROM a minimal runtime base (e.g., ubi-minimal or
scratch/ubi) and COPY only the built binaries, configs and any runtime
dependencies into that final stage; if ci-test-e2e.sh truly requires the go tool
at runtime, either modify the script to avoid needing go or include only the
minimal Go runtime binary/artifacts copied from the builder into the final stage
rather than inheriting the entire builder image, and remove any leftover
compiler/toolchain files so no build tools remain in the final image.
🤖 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 `@Dockerfile.e2e`:
- Line 33: Replace the current version-only dnf install of azure-cli (the line
installing "azure-cli-2.85.0-1.el9") with a digest-pinned workflow: download the
exact Microsoft RPM artifact for azure-cli-2.85.0-1.el9, verify its SHA256 (and
signature if available), install the verified local RPM (instead of installing
from repos), then lock the exact NEVRA with the package manager (e.g., dnf
versionlock) so the exact build is fixed; remove the plain "dnf install
azure-cli-2.85.0-1.el9" step and add a final step to re-run your
vulnerability/support scanner against that specific RPM build to record its
advisories.

---

Outside diff comments:
In `@Dockerfile.e2e`:
- Around line 1-35: The Dockerfile is missing a HEALTHCHECK directive; add a
HEALTHCHECK at the end of the file that runs an appropriate lightweight check
(for example invoking /hypershift/hack/ci-test-e2e.sh or
/hypershift/bin/test-e2e with a quick status subcommand) using CMD-SHELL, and
include sensible options like --interval=30s --timeout=10s --retries=3; ensure
the referenced script/binary (ci-test-e2e.sh or test-e2e) is executable in the
image and that the health command exits with 0 on success so container
orchestrators can detect unhealthy containers.
- Around line 1-35: The image currently runs as root because there's no USER
directive; before the final RUN that installs packages (the RUN starting with
"rpm --import ...") add commands to create a non-root user and group (e.g.,
create "hypershift" with a fixed UID/GID), chown the WORKDIR and copied bins
(refer to WORKDIR /hypershift and the copied paths like /hypershift/bin and
/hypershift/hack), and then set USER hypershift so the remaining layers run
unprivileged; ensure the created user has a valid shell and ownership of
/hypershift to allow the subsequent scripts (ci-test-e2e.sh,
run-reqserving-e2e.sh) and binaries (test-e2e, hypershift, etc.) to be
executable by that user.
- Around line 10-12: The final image currently reuses the full Go builder image
(the FROM line referencing
registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.25-openshift-4.23)
which leaves build tools in the runtime image; change the Dockerfile to use a
proper multi-stage build: keep a builder stage that compiles artifacts, then add
a distinct final stage FROM a minimal runtime base (e.g., ubi-minimal or
scratch/ubi) and COPY only the built binaries, configs and any runtime
dependencies into that final stage; if ci-test-e2e.sh truly requires the go tool
at runtime, either modify the script to avoid needing go or include only the
minimal Go runtime binary/artifacts copied from the builder into the final stage
rather than inheriting the entire builder image, and remove any leftover
compiler/toolchain files so no build tools remain in the final image.
🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 22f8217c-50b2-43f3-a324-993080a88d86

📥 Commits

Reviewing files that changed from the base of the PR and between e746a44 and 091bbaf.

📒 Files selected for processing (1)
  • Dockerfile.e2e

Comment thread Dockerfile.e2e Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@Dockerfile.e2e`:
- Around line 32-34: The final image's RUN that installs python3-pip (the shown
"RUN dnf install -y python3-pip && pip3 install --no-cache-dir azure-cli && dnf
clean all") must not leave build tools in runtime; move the pip installation and
"pip3 install --no-cache-dir azure-cli" into the builder stage, install
azure-cli there, then copy only the runtime artifacts (eg. /usr/local/bin/az and
the installed site-packages or relevant azure-cli files) into the final stage
and remove "python3-pip" and any pip invocation from the final stage's RUN so
the final image contains no build tools.
🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 415cdd0b-56f3-4f0e-95d4-eed965d53f02

📥 Commits

Reviewing files that changed from the base of the PR and between 091bbaf and 3ce0f5a.

📒 Files selected for processing (1)
  • Dockerfile.e2e

Comment thread Dockerfile.e2e Outdated
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
Dockerfile.e2e (2)

1-34: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add USER directive to run as non-root.

The Dockerfile does not set a USER directive, so the container runs as root by default. This violates the container security guideline requiring non-root execution.

🔒 Proposed fix
 COPY --from=builder /hypershift/hack/ci-test-e2e.sh /hypershift/hack/ci-test-e2e.sh
 COPY --from=builder /hypershift/hack/run-reqserving-e2e.sh /hypershift/hack/run-reqserving-e2e.sh

+RUN useradd -r -u 1001 -g root e2euser && \
+    chown -R 1001:root /hypershift && \
+    chmod -R g+rwX /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.d/ci/ && \
     dnf install -y azure-cli --setopt=install_weak_deps=False && \
     dnf clean all

+USER 1001
+

As 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 Dockerfile runs as root; create a
non-root user and switch to it by adding steps in the final stage to create a
dedicated user/group (e.g., "hypershift" or UID 1000), chown the application
dirs and binaries under /hypershift (all files copied from builder, and
/hypershift/bin and /hypershift/hack), set a sensible HOME, and add a USER
directive (USER hypershift or USER 1000) before the image entrypoint; ensure any
install steps that require root remain in the builder or are done before
changing ownership so run-time does not require root.

1-34: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add HEALTHCHECK directive.

The Dockerfile does not define a HEALTHCHECK, which is required by the container security guidelines. While this is an e2e test image (ephemeral workload), the guideline mandates a health check for all containers.

🏥 Proposed fix
 USER 1001

+HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
+  CMD ["/hypershift/bin/hypershift", "version"] || exit 1
+

As 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, Add a HEALTHCHECK directive to the
Dockerfile (at the end of the final image stage) so the container reports
liveness; for example, add a line like HEALTHCHECK --interval=30s --timeout=5s
--start-period=10s --retries=3 CMD-SHELL "test -x /hypershift/bin/hypershift ||
exit 1" (or point it at an existing probe script such as
/hypershift/hack/ci-test-e2e.sh if you prefer); ensure the directive is placed
after the last RUN/COPY in the final stage so the image builds with the
healthcheck baked in.
♻️ Duplicate comments (1)
Dockerfile.e2e (1)

33-33: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Pin azure-cli by digest and version.

The azure-cli package is installed from a third-party (Microsoft) repository without digest or version pinning, violating the guideline that non-Red Hat images must be pinned by digest. This creates supply-chain and reproducibility risks.

As per coding guidelines, "non-RH images: pin by digest".

Run the following script to identify the exact azure-cli RPM and its digest/checksum:

#!/bin/bash
# Description: Query the Microsoft repo for available azure-cli versions and checksums

echo "=== Available azure-cli packages from Microsoft repo ==="
# List recent azure-cli RPMs
curl -s https://packages.microsoft.com/yumrepos/azure-cli/Packages/a/ | \
  grep -oP 'href="azure-cli-[0-9]+\.[0-9]+\.[0-9]+-[0-9]+\.el9[^"]*"' | \
  sed 's/href="//;s/"//' | sort -V | tail -10

echo ""
echo "=== Example: Fetch SHA256 for a specific RPM ==="
echo "# Download the RPM to compute its digest:"
echo "# curl -sO https://packages.microsoft.com/yumrepos/azure-cli/Packages/a/azure-cli-VERSION.el9.x86_64.rpm"
echo "# sha256sum azure-cli-VERSION.el9.x86_64.rpm"
🤖 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 33, The Dockerfile currently installs "azure-cli"
without pinning; replace the generic dnf install line that references azure-cli
with a deterministic installation: download the exact azure-cli RPM for a
specific version/release (azure-cli-VERSION.el9.x86_64.rpm) from the Microsoft
repo, verify its SHA256 digest matches the pinned checksum, and then install
that RPM (or install the exact package name including version-release) instead
of the floating package; update the Dockerfile.e2e azure-cli install step to use
the pinned version and checksum so the build is reproducible and supply-chain
safe.
🤖 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 `@Dockerfile.e2e`:
- Line 33: Remove the --nobest flag from the dnf install invocation to avoid
silent downgrades; locate the RUN line that contains "dnf install -y azure-cli
--nobest --setopt=install_weak_deps=False" and change it to install azure-cli
without --nobest (keeping -y and --setopt=install_weak_deps=False), so
dependency conflicts surface as build failures instead of allowing older package
versions.
- Line 32: The mv command in Dockerfile.e2e currently moves microsoft-prod.repo
to a mistyped directory `/etc/yum.repos.art/ci/`; update the destination to the
correct yum repo directory `/etc/yum.repos.d/` (and ensure the target exists by
creating it with mkdir -p if your Dockerfile sequence might require it) so the
mv in the line showing "mv /etc/yum.repos.d/microsoft-prod.repo
/etc/yum.repos.art/ci/ && \" correctly places the repo file where dnf/yum will
find it.

---

Outside diff comments:
In `@Dockerfile.e2e`:
- Around line 1-34: The Dockerfile runs as root; create a non-root user and
switch to it by adding steps in the final stage to create a dedicated user/group
(e.g., "hypershift" or UID 1000), chown the application dirs and binaries under
/hypershift (all files copied from builder, and /hypershift/bin and
/hypershift/hack), set a sensible HOME, and add a USER directive (USER
hypershift or USER 1000) before the image entrypoint; ensure any install steps
that require root remain in the builder or are done before changing ownership so
run-time does not require root.
- Around line 1-34: Add a HEALTHCHECK directive to the Dockerfile (at the end of
the final image stage) so the container reports liveness; for example, add a
line like HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3
CMD-SHELL "test -x /hypershift/bin/hypershift || exit 1" (or point it at an
existing probe script such as /hypershift/hack/ci-test-e2e.sh if you prefer);
ensure the directive is placed after the last RUN/COPY in the final stage so the
image builds with the healthcheck baked in.

---

Duplicate comments:
In `@Dockerfile.e2e`:
- Line 33: The Dockerfile currently installs "azure-cli" without pinning;
replace the generic dnf install line that references azure-cli with a
deterministic installation: download the exact azure-cli RPM for a specific
version/release (azure-cli-VERSION.el9.x86_64.rpm) from the Microsoft repo,
verify its SHA256 digest matches the pinned checksum, and then install that RPM
(or install the exact package name including version-release) instead of the
floating package; update the Dockerfile.e2e azure-cli install step to use the
pinned version and checksum so the build is reproducible and supply-chain safe.
🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: b7ce2411-ba0e-4ee7-aaf9-b439d9d0a716

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce0f5a and 14cdf7f.

📒 Files selected for processing (1)
  • Dockerfile.e2e

Comment thread Dockerfile.e2e
Comment thread Dockerfile.e2e Outdated
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2026
…I throttling

  Configures the CPO deployment check to 15s instead of 3s default
@YamunadeviShanmugam YamunadeviShanmugam force-pushed the fix-e2e-tests-inconsistent-timeout-agressive-api-polling branch from cfb0b0c to 3b08f70 Compare June 5, 2026 13:05
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/test e2e-aws

2 similar comments
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@jparrill

jparrill commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

/approve

@openshift-ci

openshift-ci Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jparrill, YamunadeviShanmugam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2026
@sdminonne

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-azure-v2-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

2 similar comments
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/verified by CI

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@YamunadeviShanmugam: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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.

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

The optional CI jobs are failing due to the issue with payload

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 832b848 and 2 for PR HEAD 3b08f70 in total

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 4755e9c and 1 for PR HEAD 3b08f70 in total

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 76b27ab and 0 for PR HEAD 3b08f70 in total

@openshift-ci

openshift-ci Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@YamunadeviShanmugam: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-4-22 3b08f70 link true /test e2e-aws-4-22
ci/prow/e2e-aks-4-22 3b08f70 link true /test e2e-aks-4-22

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@hypershift-jira-solve-ci

Copy link
Copy Markdown

Confirmed — both jobs have identical failures. The analysis is complete. Here is the report:

Test Failure Analysis Complete

Job Information

Test Failure Analysis

Error

could not run steps: step [release:latest-419] failed: failed to import release
4.19.0-0.ci-2026-06-03-210413 to tag release:latest-419: failed to reimport the tag
ci-op-k5y5kb2w/stable-latest-419:machine-config-operator: unable to import tag with
message Internal error occurred: dockerimage.image.openshift.io
"quay.io/openshift/ci@sha256:90428a..." not found — timed out after 6 import attempts

could not run steps: step [release:latest-420] failed: failed to import release
4.20.0-0.ci-2026-06-07-204143 to tag release:latest-420: failed to reimport the tag
ci-op-k5y5kb2w/stable-latest-420:hypershift: unable to import tag with message Internal
error occurred: dockerimage.image.openshift.io
"quay.io/openshift/ci@sha256:ef1b30..." not found — timed out after 6 import attempts

could not run steps: step [release:latest-421] failed: failed to import release
4.21.0-0.ci-2026-06-08-003009 to tag release:latest-421: failed to reimport the tag
ci-op-k5y5kb2w/stable-latest-421:agent-installer-ui: unable to import tag with message
Internal error occurred: dockerimage.image.openshift.io
"quay.io/openshift/ci@sha256:abea17..." not found — timed out after 6 import attempts

Summary

Both e2e-aks-4-22 and e2e-aws-4-22 jobs failed identically during the CI infrastructure setup phase — before any test code from PR #8617 was executed. The failures occurred when ci-operator attempted to import release payload images for OCP versions 4.19, 4.20, and 4.21 into the CI namespace. Specific component images within those release payloads (machine-config-operator, hypershift, agent-installer-ui) had been garbage-collected from the quay.io/openshift/ci and quay-proxy.ci.openshift.org/openshift/ci registries, making them unavailable for import. This is a transient CI infrastructure issue completely unrelated to the PR's code changes.

Root Cause

The root cause is stale release payload images that have been garbage-collected from the container registry. Here is the chain of events:

  1. ci-operator resolves release streams: The HyperShift e2e jobs require multiple OCP release payloads for multi-version testing (4.18 through 4.22, plus 5.0). ci-operator queries the release controller for the "latest" release in each stream.

  2. Release controller returns stale releases: The release controller returned releases that were days old:

    • 4.19.0-0.ci-2026-06-03-2104138 days old (created June 3)
    • 4.20.0-0.ci-2026-06-07-2041434 days old (created June 7)
    • 4.21.0-0.ci-2026-06-08-0030093 days old (created June 8)
  3. Component images pruned from registry: Individual component images referenced by these old release payloads (by digest) had already been garbage-collected from quay.io/openshift/ci. The specific images that were no longer available:

    • machine-config-operator at sha256:90428a823afd... (from 4.19 payload)
    • hypershift at sha256:ef1b3047fb89... (from 4.20 payload)
    • agent-installer-ui at sha256:abea17a3a199... (from 4.21 payload)
  4. Import retries exhausted: ci-operator attempted to reimport each image 6 times before giving up with a timeout error.

  5. Job fails before test execution: Because release import is a prerequisite for the test steps, the entire job failed during graph setup — no test code was ever executed.

Key evidence: Both jobs shared the same CI namespace (ci-op-k5y5kb2w), started simultaneously at 12:42:10Z, and failed at the same second (12:57:46Z) with byte-identical error messages. The releases that succeeded (4.18, 4.22, initial-422) were either very recent or had not yet been pruned. This confirms this is a registry-side image lifecycle issue, not a network or permissions problem.

This failure is entirely unrelated to PR #8617's code changes (which optimize CPO deployment polling intervals in trust bundle propagation tests).

Recommendations
  1. Rerun the jobs — This is a transient CI infrastructure failure. A simple /retest on the PR should resolve it, as ci-operator will likely resolve to newer release payloads whose images are still present in the registry.

  2. No code changes needed — The PR's changes to CPO deployment polling intervals were never tested because the failure occurred during CI setup. The code in the PR is not implicated.

  3. If retests continue to fail — File an issue against the OpenShift CI infrastructure (openshift/release or #forum-ocp-crt) reporting that the release controller is returning stale payloads whose component images have been garbage-collected. Include the specific release versions and digest hashes from this analysis.

Evidence
Evidence Detail
Failure phase executing_graph:step_failed:importing_release — pre-test infrastructure setup
Failed releases latest-419 (4.19, June 3), latest-420 (4.20, June 7), latest-421 (4.21, June 8)
Succeeded releases latest-418 (4.18), latest-422 (4.22), initial-422 (4.22)
Failed images machine-config-operator (sha256:90428a…), hypershift (sha256:ef1b30…), agent-installer-ui (sha256:abea17…)
Registry error dockerimage.image.openshift.io "quay.io/openshift/ci@sha256:..." not found on both quay.io and quay-proxy.ci.openshift.org
Import attempts 6 retries per image, all failed
Shared namespace Both jobs used ci-op-k5y5kb2w on build01 cluster
Identical timing Both started 12:42:10Z, failed 12:57:46Z (ran 15m24s)
Test execution None — jobs never reached the e2e test steps
PR code involvement None — failure is in CI infrastructure, not in PR #8617's changes

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/hold

Revision 3b08f70 was retested 3 times: holding

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing Indicates the PR includes changes for e2e testing do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants