Skip to content

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

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
YamunadeviShanmugam:fix-e2e-tests-inconsistent-timeout-agressive-api-polling
Jun 16, 2026
Merged

OCPBUGS-86662: Optimize CPO deployment polling interval in tests of additional trust bundle propagation#8617
openshift-merge-bot[bot] merged 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 44 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

@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-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
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@hypershift-jira-solve-ci

Copy link
Copy Markdown

The analysis is complete. Both jobs failed with the identical infrastructure error. Here is my report:

Test Failure Analysis Complete

Job Information

  • Prow Jobs: pull-ci-openshift-hypershift-main-e2e-aks-4-22 / pull-ci-openshift-hypershift-main-e2e-aws-4-22
  • Build IDs: 2066350096013856768 (AKS) / 2066350096043216896 (AWS)
  • PR: OCPBUGS-86662: Optimize CPO deployment polling interval in tests of additional trust bundle propagation #8617 — OCPBUGS-86662: Optimize CPO deployment polling interval in tests of additional trust bundle propagation
  • Target: e2e-aks-4-22 / e2e-aws-4-22
  • Both jobs started: 2026-06-15T02:40:38Z
  • Both jobs ended: 2026-06-15T02:56:17Z (15m38s)
  • Failure reason: executing_graph:step_failed:importing_release

Test Failure Analysis

Error

step [release:initial-422] failed: failed to import release 4.22.0-0.ci-2026-06-07-214855
to tag release:initial-422: failed to reimport the tag ci-op-wd3kqbcj/stable-initial-422:agent-installer-ui:
unable to import tag ci-op-wd3kqbcj/stable-initial-422:agent-installer-ui with message
Internal error occurred: [dockerimage.image.openshift.io "quay.io/openshift/ci@sha256:1eab7c85bc5d02bca9618a2f681b24e8da5514d2c302b750df328b047d13caf7" not found,
dockerimage.image.openshift.io "quay-proxy.ci.openshift.org/openshift/ci@sha256:1eab7c85bc5d02bca9618a2f681b24e8da5514d2c302b750df328b047d13caf7" not found]
on the image stream even after (6) imports: timed out waiting for the condition

Summary

Both CI jobs (e2e-aks-4-22 and e2e-aws-4-22) failed identically during the ci-operator release import phase, before any test code from PR #8617 was executed. The initial-422 release payload (4.22.0-0.ci-2026-06-07-214855, built on June 7th — 8 days prior) could not be imported because the agent-installer-ui image (sha256:1eab7c85bc5d02bca9618a2f681b24e8da5514d2c302b750df328b047d13caf7) has been removed from both quay.io/openshift/ci and quay-proxy.ci.openshift.org/openshift/ci. This is a CI infrastructure issue completely unrelated to the PR's changes. The newer latest-422 release (4.22.0-0.ci-2026-06-12-133937) imported successfully, confirming only the stale pinned release is affected.

Root Cause

The root cause is a stale release payload reference in the CI configuration. The CI job is configured to import two 4.22 release payloads:

  1. initial-422 — resolved to 4.22.0-0.ci-2026-06-07-214855 (pinned via ?rel=1, i.e., the oldest/most-stable release)
  2. latest-422 — resolved to 4.22.0-0.ci-2026-06-12-133937 (the newest available CI release)

The initial-422 payload from June 7th is 8 days old and its agent-installer-ui component image (sha256:1eab7c85bc5d02bca9618a2f681b24e8da5514d2c302b750df328b047d13caf7) has been garbage-collected from the Quay.io registry. The ci-operator attempted 6 import retries across both quay.io and quay-proxy.ci.openshift.org mirrors before timing out.

Key facts proving this is infrastructure/registry, not PR-related:

  • Both jobs share identical error — same namespace (ci-op-wd3kqbcj), same failing image, same timestamps
  • No test steps ran — the JUnit XML shows 22 steps with only 1 failure: Import the release payload "initial-422". No e2e test steps appear at all
  • The PR changes are test-only — optimizing CPO deployment polling intervals cannot affect release image availability
  • The newer release imported finelatest-422 (June 12) succeeded, only the older pinned release (June 7) failed
Recommendations
  1. Rerun both jobs — This is a transient infrastructure issue. A /retest should resolve it if the release controller has rotated to a newer initial-422 payload whose images are still available, or if the image has been re-mirrored.

  2. If retests continue to fail — File an infrastructure issue against the OpenShift CI team (Test Platform / DPTP). The ?rel=1 release stream endpoint is returning a payload whose images have already been garbage-collected from Quay.io, indicating the retention window is shorter than the release pin window.

  3. No changes needed to PR OCPBUGS-86662: Optimize CPO deployment polling interval in tests of additional trust bundle propagation #8617 — The failure is entirely unrelated to the PR's code changes (CPO deployment polling interval optimization).

Evidence
Evidence Detail
Failure stage [release:initial-422] — release import phase, before any test step
Failed release 4.22.0-0.ci-2026-06-07-214855 (8 days old, from June 7)
Successful release 4.22.0-0.ci-2026-06-12-133937 (3 days old, from June 12)
Missing image agent-installer-ui @ sha256:1eab7c85bc5d02bca9618a2f681b24e8da5514d2c302b750df328b047d13caf7
Registry lookups Both quay.io/openshift/ci and quay-proxy.ci.openshift.org/openshift/ci returned "not found"
Import attempts 6 retries, all failed
ci-operator reason executing_graph:step_failed:importing_release
Jobs affected Both e2e-aks-4-22 and e2e-aws-4-22 — identical error
Shared namespace ci-op-wd3kqbcj — both jobs ran in the same CI namespace
JUnit result 22 steps total, 1 failure (release import), 0 test steps executed
PR relation None — PR #8617 modifies test polling intervals only

@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 16, 2026
@YamunadeviShanmugam

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@YamunadeviShanmugam: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot Bot merged commit 44f5195 into openshift:main Jun 16, 2026
42 checks passed
@openshift-ci-robot

Copy link
Copy Markdown

@YamunadeviShanmugam: Jira Issue Verification Checks: Jira Issue OCPBUGS-86662
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-86662 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

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

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.

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 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