CNTRLPLANE-3576: add RestartDateAnnotation propagation tests

[mgencur]

What this PR does / why we need it:

• Convert an E2E test from openshift-tests-private into a unit test that covers the missing functionality as follows:
  • Verify that setDefaultOptions propagates RestartDateAnnotation from HCP to the pod template when set, and omits it when absent. This covers the mechanism that triggers rolling restarts via pod template annotation changes. Rolling out the new ReplicaSet is then Kubernetes' concern. The annotation propagation from HostedCluster to HostedControlPlane is already tested in TestReconcileHostedControlPlaneAnnotations.

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3576

Special notes for your reviewer:

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Tests
  • Enhanced test coverage and robustness for restart date annotation propagation in control plane components, with improved test structure and error handling.

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

@mgencur: This pull request references CNTRLPLANE-3576 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

This PR is WIP because it builds on top of #8731 and will need a rebase once this PR is meged

Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3576

Special notes for your reviewer:

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

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]

📝 Walkthrough

Walkthrough

TestSetDefaultOptions in support/controlplane-component/defaults_test.go receives two changes. First, the scheme initialization block now calls hyperv1.AddToScheme, corev1.AddToScheme, and appsv1.AddToScheme with t.Fatalf on any registration error, replacing the previous silent discard of errors. Second, restart-date annotation behavior is extracted into a new annotationTests table-driven block of parallel subtests, each exercising setDefaultOptions and asserting that hyperv1.RestartDateAnnotation is either propagated to or omitted from the workload pod template annotations based on its presence on the HostedControlPlane.

Suggested Reviewers

• devguyio
• Nirshal

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	The test file uses standard Go testing, not Ginkgo. However, the tests fail quality requirement #4 (assertion messages): most assertions (12 without messages vs 1 with) lack meaningful failure mess...	Add descriptive failure messages to assertions, especially to lines 553, 554-555, 706, 709-712. Example: g.Expect(err).NotTo(HaveOccurred(), "failed to set default options").

✅ 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 accurately reflects the main change: adding RestartDateAnnotation propagation tests to the test file, which aligns with the refactored test logic and table-driven subtests described in the summary.
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 file uses standard Go testing (func Test*, t.Run), not Ginkgo. No Ginkgo test constructs (It, Describe, Context, When) exist. Test names in annotation tests are static strings without dynamic v...
Topology-Aware Scheduling Compatibility	✅ Passed	This PR modifies only test files (defaults_test.go) with unit tests for annotation propagation. No deployment manifests, operator code, or controller modifications that introduce scheduling constra...
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	This PR modifies a standard Go unit test file using testing.T and table-driven tests, not Ginkgo e2e tests. The custom check applies only to Ginkgo e2e tests, so it is not applicable here.
No-Weak-Crypto	✅ Passed	No weak cryptography patterns detected. The PR only modifies test code for annotation propagation and fixes a nil-map bug; no crypto imports or operations are present.
Container-Privileges	✅ Passed	PR contains only unit test additions with no container/K8s manifests. No privileged settings (privileged:true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation:true, root without...
No-Sensitive-Data-In-Logs	✅ Passed	No sensitive data exposed in logging. Only error logging for test setup with generic error messages and test data uses dummy values (test images, component names, and generic test resource identifi...

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

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 `@support/controlplane-component/defaults_test.go`:
- Around line 526-528: In support/controlplane-component/defaults_test.go at
lines 526-528, replace each `_ = ...AddToScheme(scheme)` statement (for hyperv1,
corev1, and appsv1) by capturing the error return value and checking it
explicitly: if the error is not nil, call t.Fatalf with an appropriate failure
message. In
control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go
at line 83, apply the same fix to the appsv1.AddToScheme call by capturing its
error return value and checking it with t.Fatalf on failure. This ensures test
initialization errors in scheme registration are properly caught and reported
rather than silently ignored.

🪄 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: 340b6197-2c17-4dde-8101-7a1e8f3c4713

📥 Commits

Reviewing files that changed from the base of the PR and between 712ba58 and fe42d7c.

📒 Files selected for processing (3)

• control-plane-operator/controllers/hostedcontrolplane/v2/cno/component.go
• control-plane-operator/controllers/hostedcontrolplane/v2/cno/component_test.go
• support/controlplane-component/defaults_test.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.84%. Comparing base (f0536b8) to head (686cc08).
⚠️ Report is 190 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8733      +/-   ##
==========================================
+ Coverage   41.69%   41.84%   +0.14%     
==========================================
  Files         758      759       +1     
  Lines       93945    94073     +128     
==========================================
+ Hits        39175    39364     +189     
+ Misses      52025    51954      -71     
- Partials     2745     2755      +10     

see 17 files with indirect coverage changes

Flag	Coverage Δ
cmd-support	35.14% <ø> (+0.11%)	⬆️
cpo-hostedcontrolplane	44.10% <ø> (+0.09%)	⬆️
cpo-other	43.45% <ø> (ø)
hypershift-operator	52.02% <ø> (+0.31%)	⬆️
other	31.56% <ø> (ø)

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.

[jparrill]

Dropped some comments. Thanks!

[csrwng]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csrwng, mgencur

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

[mgencur]

/hold

I will remove the test for SetRestartAnnotationAndPatch because it's being removed in #8751

[mgencur]

/unhold

Removed the test for SetRestartAnnotationAndPatch and also the bug fix in that function, which also addresses both comments from @jparrill that were related to that part.

[coderabbitai]

🧹 Nitpick comments (1)

support/controlplane-component/defaults_test.go (1)

677-679: Consider adding loop variable rebinding for consistency with defensive coding practices.

Line 677-679 uses a range variable in a parallel subtest. While this is safe on Go 1.22+ (the minimum supported version), adding test := test before the t.Run call follows the pattern recommended in 100go.co for defensive clarity and consistency. However, this is optional refactoring—the file shows this pattern inconsistently applied (lines 618–625 have an identical pattern without rebinding), and the project's .golangci.yml does not enforce the loopvar linter.

Optional adjustment

 for _, test := range annotationTests {
+	test := test
 	t.Run(test.name, func(t *testing.T) {
 		t.Parallel()

🤖 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 `@support/controlplane-component/defaults_test.go` around lines 677 - 679, In
the annotationTests loop (line 677), add a loop variable rebinding line `test :=
test` immediately after the for statement and before the `t.Run` call to create
a local copy of the loop variable for the parallel subtest closure. This follows
defensive coding practices for consistency, even though it is safe on Go 1.22+
and the project does not enforce the loopvar linter.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@support/controlplane-component/defaults_test.go`:
- Around line 677-679: In the annotationTests loop (line 677), add a loop
variable rebinding line `test := test` immediately after the for statement and
before the `t.Run` call to create a local copy of the loop variable for the
parallel subtest closure. This follows defensive coding practices for
consistency, even though it is safe on Go 1.22+ and the project does not enforce
the loopvar linter.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: d75e2f71-8f9d-4239-8a4b-05f6d830e9ee

📥 Commits

Reviewing files that changed from the base of the PR and between 928c70f and 686cc08.

📒 Files selected for processing (1)

• support/controlplane-component/defaults_test.go

[jparrill]

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

[cwbotbot]

Test Results

e2e-aks

• Status: ✅ PASS
• Started: 2026-06-18T08:00:42Z
• View Job
• View Job History

e2e-aws

• Status: ❌ FAIL
• Started: 2026-06-18T08:00:42Z
• View Job
• View Job History

[mgencur]

/retest

[hypershift-jira-solve-ci]

AI Test Failure Analysis

Job: pull-ci-openshift-hypershift-main-e2e-aws | Build: 2067861110131789824 | Cost: $2.7264704999999996 | Failed step: hypershift-aws-run-e2e-nested

View full analysis report

---

_{Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6}

[mgencur]

/retest

[celebdor]

/lgtm
/verified by "unit test covers the functionality it claims"

[openshift-ci-robot]

@celebdor: This PR has been marked as verified by "unit test covers the functionality it claims".

Details

In response to this:

/lgtm
/verified by "unit test covers the functionality it claims"

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-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 2d2b2d0 and 2 for PR HEAD 686cc08 in total

[celebdor]

/override "ci/prow/e2e-aws"

[openshift-ci]

@celebdor: Overrode contexts on behalf of celebdor: ci/prow/e2e-aws

Details

In response to this:

/override "ci/prow/e2e-aws"

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.

[hypershift-jira-solve-ci]

No errors at all before the abort. Every single build step succeeded:

• src-amd64 succeeded (2m2s)
• hypershift-amd64 succeeded (3m39s)
• hypershift-tests-amd64 succeeded (10m44s)
• hypershift-operator-amd64 succeeded (4m36s)
• hypershift-cli-amd64 succeeded (1m52s)

The job was killed during the final "Creating release image" step purely because the PR was merged.

Test Failure Analysis Complete

Job Information

• Prow Job: pull-ci-openshift-hypershift-main-images
• Build ID: 2071891411661230080
• Target: [images], [release:latest], hypershift-cli, hypershift-operator, hypershift-tests
• PR: openshift/hypershift#8733 (CNTRLPLANE-3576: add RestartDateAnnotation propagation tests)
• Job State: aborted

Test Failure Analysis

Error

Entrypoint received interrupt: terminated
Aborted by trigger plugin.

Summary

This is not a real failure. The ci/prow/images job was externally aborted by Prow's trigger plugin because PR #8733 was merged into main while the job was still running. All five image builds (src, hypershift, hypershift-operator, hypershift-tests, hypershift-cli) completed successfully. The job was killed during the final release image assembly step, 8 seconds after the PR merge. No compilation errors, test failures, or build issues occurred.

Root Cause

The job was aborted by Prow's trigger plugin due to PR merge, not due to any code or build issue.

Timeline:

1. Job started at 09:39:41Z and all image builds completed successfully by 10:12:13Z
2. At 10:12:14Z, the job began creating the final release image (registry.build01.ci.openshift.org/ci-op-gqgj446r/release:latest)
3. At 10:13:08Z, PR CNTRLPLANE-3576: add RestartDateAnnotation propagation tests #8733 was merged into main (merge commit 54cb170)
4. At 10:13:16Z (8 seconds after merge), Prow's trigger plugin sent a SIGTERM to the running job, aborting it

When a PR is merged, Prow cancels any still-running presubmit jobs for that PR since their results are no longer relevant — the code is already in main and postsubmit jobs will run against the merged commit instead. This is normal Prow behavior and does not indicate any problem with the PR's code.

Recommendations

• No action required. This job was aborted because the PR was successfully merged. The "failure" state is a cosmetic artifact of the merge timing.
• All image builds completed successfully before the abort. The code compiles and builds correctly.
• If validation of the images target is needed post-merge, check the corresponding postsubmit job runs on the main branch.

Evidence

Evidence	Detail
Job state	aborted (not failure)
Abort reason	Aborted by trigger plugin.
PR merge time	2026-06-30T10:13:08Z
SIGTERM received	2026-06-30T10:13:16Z (8s after merge)
All builds succeeded	src (2m2s), hypershift (3m39s), hypershift-tests (10m44s), hypershift-operator (4m36s), hypershift-cli (1m52s)
Errors before abort	None — zero error/fail lines before the interrupt
PR status	Merged (commit 54cb170408d6063627da3d63a6d5d62a7819b9a6)
Abort point	During final step: "Creating release image"

---