Revert "CNTRLPLANE-3890: Add product-cli unit tests for HCP create cluster"

[bryan-cox]

Reverts #9107

Summary by CodeRabbit

• Tests
  • Removed legacy automated tests covering cluster creation commands across Agent, AWS, Azure, KubeVirt, and OpenStack environments.
  • Retained basic coverage for selected command metadata, required flags, and release-stream 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]

@bryan-cox: This pull request references CNTRLPLANE-3890 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:

Reverts #9107

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: 19b5623a-bc89-4df4-891f-deba230b2856

📥 Commits

Reviewing files that changed from the base of the PR and between 53daedc and e5bc91b.

⛔ Files ignored due to path filters (5)

• product-cli/cmd/cluster/agent/testdata/zz_fixture_TestCreateCluster_When_minimal_flags_are_provided__it_should_render_correctly.yaml is excluded by !**/testdata/**
• product-cli/cmd/cluster/aws/testdata/zz_fixture_TestCreateCluster_When_minimal_flags_are_provided__it_should_render_correctly.yaml is excluded by !**/testdata/**
• product-cli/cmd/cluster/azure/testdata/zz_fixture_TestCreateCluster_When_minimal_flags_are_provided__it_should_render_correctly.yaml is excluded by !**/testdata/**
• product-cli/cmd/cluster/kubevirt/testdata/zz_fixture_TestCreateCluster_When_minimal_flags_are_provided__it_should_render_correctly.yaml is excluded by !**/testdata/**
• product-cli/cmd/cluster/openstack/testdata/zz_fixture_TestCreateCluster_When_minimal_flags_are_provided__it_should_render_correctly.yaml is excluded by !**/testdata/**

📒 Files selected for processing (5)

• product-cli/cmd/cluster/agent/create_test.go
• product-cli/cmd/cluster/aws/create_test.go
• product-cli/cmd/cluster/azure/create_test.go
• product-cli/cmd/cluster/kubevirt/create_test.go
• product-cli/cmd/cluster/openstack/create_test.go

💤 Files with no reviewable changes (5)

• product-cli/cmd/cluster/agent/create_test.go
• product-cli/cmd/cluster/aws/create_test.go
• product-cli/cmd/cluster/azure/create_test.go
• product-cli/cmd/cluster/openstack/create_test.go
• product-cli/cmd/cluster/kubevirt/create_test.go

---

📝 Walkthrough

Walkthrough

The pull request deletes cluster creation tests for agent, AWS, Azure, KubeVirt, and OpenStack commands. The removed tests covered command metadata, flags, defaults, manifest rendering, and fixture-based integration behavior. Azure retains its command metadata, required-flag, and release-stream tests. Its imports were reduced to remove dependencies used only by the deleted integration test.

Possibly related PRs

• openshift/hypershift#9107: Added the HCP cluster creation tests that this pull request removes.

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states that the pull request reverts the earlier product-cli unit-test change, matching the stated objective and deleted test files.
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 revert deletes standard testing.T tests and fixtures. It adds or changes no Ginkgo It, Describe, Context, or When titles, so it introduces no unstable test name.
Test Structure And Quality	✅ Passed	The PR only removes standard testing/Gomega tests; no Ginkgo It, BeforeEach, AfterEach, Eventually, or Consistently code is added or retained in the affected cluster tests.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff only deletes create tests and their test fixtures; it adds or modifies no deployment manifests, operators, controllers, or scheduling constraints.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR removes Go unit tests and fixtures only; it adds no Ginkgo e2e tests or IPv4/external-connectivity assumptions.
No-Weak-Crypto	✅ Passed	The PR adds no lines. It only deletes cluster create tests and fixtures; the remaining Azure test has no weak crypto or secret/token comparisons.
Container-Privileges	✅ Passed	The PR adds no content. It deletes tests and YAML fixtures; no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings are introduced.
No-Sensitive-Data-In-Logs	✅ Passed	The patch contains only deletions; it adds no logging or sensitive-data output. The remaining Azure test has no logging calls.

✨ 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: bryan-cox

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

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

[ironcladlou]

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

[bryan-cox]

/override ci/prow/e2e-aks
/override ci/prow/e2e-aws
/override ci/prow/e2e-aws-upgrade-hypershift-operator
/override ci/prow/e2e-kubevirt-aws-ovn-reduced
/override ci/prow/e2e-v2-aws
/override ci/prow/e2e-v2-azure-self-managed
/override ci/prow/e2e-v2-gke

[openshift-ci]

@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aws, ci/prow/e2e-aws-upgrade-hypershift-operator, ci/prow/e2e-kubevirt-aws-ovn-reduced, ci/prow/e2e-v2-aws, ci/prow/e2e-v2-azure-self-managed, ci/prow/e2e-v2-gke

Details

In response to this:

/override ci/prow/e2e-aks
/override ci/prow/e2e-aws
/override ci/prow/e2e-aws-upgrade-hypershift-operator
/override ci/prow/e2e-kubevirt-aws-ovn-reduced
/override ci/prow/e2e-v2-aws
/override ci/prow/e2e-v2-azure-self-managed
/override ci/prow/e2e-v2-gke

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[bryan-cox]

/verified by UT passing

[openshift-ci-robot]

@bryan-cox: This PR has been marked as verified by UT passing.

Details

In response to this:

/verified by UT passing

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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.90%. Comparing base (53daedc) to head (e5bc91b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9201      +/-   ##
==========================================
+ Coverage   44.86%   44.90%   +0.03%     
==========================================
  Files         775      775              
  Lines       97216    97267      +51     
==========================================
+ Hits        43615    43673      +58     
+ Misses      50592    50581      -11     
- Partials     3009     3013       +4     

see 3 files with indirect coverage changes

Flag	Coverage Δ
cmd-support	38.42% <ø> (ø)
cpo-hostedcontrolplane	47.32% <ø> (ø)
cpo-other	45.67% <ø> (ø)
hypershift-operator	54.93% <ø> (ø)
other	34.32% <ø> (+0.40%)	⬆️

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.

[openshift-ci]

@bryan-cox: 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.

[devguyio]

Root Cause Analysis: Semantic Merge Conflict between #9107 and #9177

What happened

#9107 added golden-file fixture tests for product-cli create cluster commands. These fixtures snapshot the rendered HostedCluster manifests.

#9177 bumped sigs.k8s.io dependencies, which regenerated CRDs and changed Go struct definitions (new fields, different defaults), altering what product-cli create cluster --render produces.

Both PRs passed CI independently. Tide merged #9107 at 13:12 UTC, then #9177 at 14:31 UTC. After #9177 merged, make test on main failed because the fixture files from #9107 no longer matched the rendered output produced by the updated types from #9177. This is a semantic merge conflict — git sees no textual conflict, but the combined result is broken.

Why the unit test didn't catch this

The unit and verify jobs were configured as 2nd-stage pipeline jobs (pipeline_skip_if_only_changed). This creates a structural gap where no component in the CI system re-validates these jobs when the base branch moves after another PR merges.

How the gap works

There are three independent systems involved — prowgen, tide, and the pipeline controller — and none of them close the loop.

1. Prowgen keeps pipeline file-change logic out of Prow's reach

When pipeline_skip_if_only_changed is set in the ci-operator config, prowgen stores it only as a ProwJob annotation. It does not set the equivalent Prow-level file matchers (RunIfChanged/SkipIfOnlyChanged). The job is generated with always_run: false and an empty RegexpChangeMatcher — from Prow's perspective, this job can only be triggered by an explicit /test command or by the pipeline controller. Prow has zero knowledge of the annotation.

2. Tide treats pipeline jobs as "required only if present"

Tide classifies each job's status context into three buckets: required (must exist and pass), requiredIfPresent (must pass if it exists, but absence doesn't block), or optional. The classification logic lives in BranchRequirements — since pipeline jobs have always_run: false with no Prow-level file matchers, they are classified as requiredIfPresent.

When tide evaluates whether a PR is safe to merge, it checks the GitHub status contexts on the PR's HEAD commit. For requiredIfPresent contexts, it only checks the status state (Success/Failure/Pending) — not whether the result was tested against the current base branch. When #9107 merged and moved the base, the old ci/prow/unit status on #9177 still showed Success on GitHub. Tide saw "success" and was satisfied.

3. The pipeline controller is fire-once with no base-branch awareness

The pipeline controller watches for 1st-stage jobs to succeed, then posts /test comments to trigger 2nd-stage jobs. But it has no base-branch awareness:

• It fires once per PR HEAD SHA (see the dedup key) when 1st-stage jobs succeed, then prevents re-firing for the same HEAD
• Its 1st-stage completion check only looks at whether jobs passed for the HEAD SHA — it does not check whether the base branch has moved since those jobs ran
• When CNTRLPLANE-3890: Add product-cli unit tests for HCP create cluster #9107 merged and moved the base, nothing triggered re-evaluation of NO-JIRA: build(deps): bump the sigs-k8s-dependencies group across 1 directory with 8 updates #9177's 2nd-stage tests

The result

After #9107 merged:

• Prow wouldn't re-trigger unit for NO-JIRA: build(deps): bump the sigs-k8s-dependencies group across 1 directory with 8 updates #9177 — the job has always_run: false with no file matchers, so Prow has no reason to trigger it
• The pipeline controller wouldn't re-trigger it — it already fired for this HEAD SHA and its dedup prevents re-firing
• Tide wouldn't block the merge — the old Success status was still visible on GitHub, and tide doesn't verify whether requiredIfPresent results are tested against the current base

The structural issue

2nd-stage pipeline jobs are fundamentally unable to protect against semantic merge conflicts. The pipeline controller operates outside of tide's merge protection — it's a fire-once-per-HEAD mechanism with no awareness of base branch changes. Tide, in turn, has no awareness of the pipeline annotations and treats these jobs as required-only-if-present. No component re-validates these jobs when the base branch moves, leaving a window for semantic conflicts to slip through.

---

Generated by Claude Code, reviewed by devguyio