CNTRLPLANE-3871: e2e: Remove osImageStream cleanup that violates immutability

[csrwng]

Summary

• Remove the DeferCleanup block in NodePoolOSImageStreamExplicitDefaultNoRolloutTest that attempted to unset osImageStream after setting it to the version-derived default
• A CEL validation rule prevents removing osImageStream once set, causing cleanup to fail with a 422 error
• No restoration is needed since the test sets the field to the same value the controller derives implicitly

Jira

https://issues.redhat.com/browse/CNTRLPLANE-3871

Test plan

• e2e-v2-aws-techpreview-osimagestream job passes without the 422 cleanup error

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Updated end-to-end coverage for default OS image stream behavior.
  • Simplified cleanup handling while preserving validation of version-derived defaults.

[openshift-merge-bot]

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]

@csrwng: This pull request references CNTRLPLANE-3871 which is a valid jira issue.

Details

In response to this:

Summary

• Remove the DeferCleanup block in NodePoolOSImageStreamExplicitDefaultNoRolloutTest that attempted to unset osImageStream after setting it to the version-derived default
• A CEL validation rule prevents removing osImageStream once set, causing cleanup to fail with a 422 error
• No restoration is needed since the test sets the field to the same value the controller derives implicitly

Jira

https://issues.redhat.com/browse/CNTRLPLANE-3871

Test plan

• e2e-v2-aws-techpreview-osimagestream job passes without the 422 cleanup error

🤖 Generated with Claude Code

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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: e2e78162-1524-4b5f-a964-b31754913535

📥 Commits

Reviewing files that changed from the base of the PR and between d19dbf2 and 52a147e.

📒 Files selected for processing (1)

• test/e2e/v2/tests/nodepool_osimagestream_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• test/e2e/v2/tests/nodepool_osimagestream_test.go

---

📝 Walkthrough

Walkthrough

The explicit-default OS image stream test no longer fetches the NodePool or resets spec.osImageStream during cleanup. The test retains the version-derived default because the field is immutable after assignment.

Suggested reviewers: jparrill, muraee

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	The test patches the shared default NodePool and leaves spec.osImageStream set; the sibling default-status test then skips when this field is set, violating test isolation and cleanup requirements.	Run the explicit-default behavior against an isolated disposable NodePool and register DeferCleanup to delete it, or provide a supported restoration strategy.

✅ Passed checks (10 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the e2e cleanup removal and the immutability issue it addresses.
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	The PR changes only cleanup code. All Ginkgo Describe and It titles in the target file are static string literals with no run-dependent values.
Topology-Aware Scheduling Compatibility	✅ Passed	The only changed file is an e2e Go test; the patch removes cleanup logic and adds no deployment, operator, controller, replica, affinity, topology, selector, toleration, or PDB scheduling constraints.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR only removes cleanup from an existing Ginkgo test; it adds no test and the affected code uses cluster management APIs without IPv4 assumptions or external connectivity.
No-Weak-Crypto	✅ Passed	The PR only removes a NodePool cleanup patch and adds comments; the diff introduces no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons.
Container-Privileges	✅ Passed	The patch changes only one Go e2e test and adds no container or Kubernetes manifest privilege settings; the patch contains none of the flagged indicators.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds no logging statements. Existing logs contain NodePool names, a config hash, and RHEL stream identifiers, with no passwords, tokens, API keys, PII, hostnames, or customer data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csrwng

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:

• OWNERS [csrwng]

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

[csrwng]

When openshift/release#82438 merges, verify this change with:

/test e2e-v2-aws-techpreview-osimagestream

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.94%. Comparing base (923aba2) to head (52a147e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9206   +/-   ##
=======================================
  Coverage   44.94%   44.94%           
=======================================
  Files         778      778           
  Lines       97427    97427           
=======================================
  Hits        43790    43790           
  Misses      50616    50616           
  Partials     3021     3021           

Flag	Coverage Δ
cmd-support	38.62% <ø> (ø)
cpo-hostedcontrolplane	47.28% <ø> (ø)
cpo-other	45.67% <ø> (ø)
hypershift-operator	54.93% <ø> (ø)
other	34.32% <ø> (ø)

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.

[csrwng]

/test e2e-v2-aws-techpreview-osimagestream

[csrwng]

/verified by e2e

[openshift-ci-robot]

@csrwng: This PR has been marked as verified by e2e.

Details

In response to this:

/verified by e2e

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.

[muraee]

/lgtm

[openshift-merge-bot]

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-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-azure-self-managed
/test e2e-v2-gke

[cwbotbot]

Test Results

e2e-aws

• Status: ❌ FAIL
• Started: 2026-08-04T12:04:46Z
• View Job
• View Job History

e2e-aks

• Status: ✅ PASS
• Started: 2026-08-04T09:32:22Z
• View Job
• View Job History

[jparrill]

/lgtm

Good catch on the DeferCleanup — the CEL immutability rule at nodepool_types.go:104 made that cleanup impossible (422 on every run).

A few additional gaps I found while auditing the same file for our PR #9099 (already have fixes there):

Nit (this PR):

• nodepool_osimagestream_test.go:368-369: Stale docstring still says "and restores it on cleanup" — should update since cleanup is removed.

Pre-existing gaps (not blocking, tracked in #9099):

1. nodepool_osimagestream_test.go:287-301: expectedStream is declared as OSImageStreamRHEL10 but never used in the actual assertion — osImageStreamSetPredicate() accepts any recognized stream (rhel-9 or rhel-10) instead of validating the exact expected value. Should use a predicate that matches expectedStream exactly, and make it version-aware (rhel-10 on 5.0+, rhel-9 on < 5.0).

2. nodepool_osimagestream_test.go:62: Comment says "skips when ... platform does not use RHCOS nodes" but the code only checks CRD schema fields — no platform check exists.

3. nodepool_osimagestream_test.go:532: expectedStream hardcoded to rhel-9 in upgrade test, assumes E2E_PREVIOUS_RELEASE_IMAGE is always 4.x. Will break when 5.x→5.y upgrade tests are introduced.

I have fixes for all of these in #9099 — will rebase on top of yours once this merges.

[jparrill]

/retest-required

[jparrill]

/test ?

[jparrill]

/test e2e-v2-aws-techpreview-osimagestream

[openshift-ci]

@csrwng: 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-v2-azure-self-managed	52a147e	link	true	/test e2e-v2-azure-self-managed
ci/prow/e2e-aws-upgrade-hypershift-operator	52a147e	link	true	/test e2e-aws-upgrade-hypershift-operator
ci/prow/e2e-kubevirt-aws-ovn-reduced	52a147e	link	true	/test e2e-kubevirt-aws-ovn-reduced
ci/prow/e2e-aks-4-22	52a147e	link	true	/test e2e-aks-4-22
ci/prow/e2e-aws	52a147e	link	true	/test e2e-aws

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.