Revert "TRT-2869: Revert "NO-JIRA: Re-enable tests for the recommend cmd if alertsByCVO""

[hongkailiu]

/hold

rebase after #31460 gets in.

Summary by CodeRabbit

• Tests
  • Updated conditional upgrade recommendation tests to reflect different alert evaluation outcomes.
  • Added coverage for guidance to use the upgrade acceptance command when version targets require it.
  • Improved failure diagnostics by including command output when an expected error is missing.

[openshift-ci-robot]

@hongkailiu: This pull request references TRT-2869 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 bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

/hold

rebase after #31460 gets in.

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]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[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: automatic mode

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hongkailiu

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:

• test/extended/cli/adm_upgrade/OWNERS [hongkailiu]

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

[hongkailiu]

/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-2of3

[openshift-ci]

@hongkailiu: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

• periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-1of3
• periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-2of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7464d050-8cd6-11f1-89bd-bf4e4b48806f-0

[coderabbitai]

Walkthrough

The conditional recommendation tests now execute when CVO evaluates alerts. They use alternate listing output, require oc adm upgrade accept guidance for version-target errors, and include captured output in failed assertions.

Changes

Conditional recommendation test behavior

Layer / File(s)	Summary
CVO conditional assertions test/extended/cli/adm_upgrade/recommend.go	The test removes the temporary skip, selects output patterns by alertsByCVO, checks oc adm upgrade accept guidance for CVO version-target errors, and keeps successful assertions for non-CVO cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• openshift/origin#31460: Modifies the same recommendation tests and their alertsByCVO branching.

Suggested reviewers: redhat-chai-bot

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	The new failure diagnostic logs full oc adm upgrade recommend output; this includes the test update service URL built from the cluster-internal Service.Spec.ClusterIP.	Do not log the full command output. Redact or log only fixed, non-sensitive fields such as the error type and sanitized recommendation text.
Test Structure And Quality	⚠️ Warning	The re-enabled Ginkgo code creates deployments and services without cleanup, uses BeforeAll/AfterAll, and contains many bare assertions without diagnostic messages.	Register created resources for cleanup in AfterEach or DeferCleanup, use per-test setup where practical, and add context to every assertion that can fail.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies the change as reverting the prior reversal that re-enabled recommend command tests for alertsByCVO.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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	All Ginkgo Describe, Context, and It titles in the changed file are literal static strings; none contain runtime values, suffixes, timestamps, nodes, namespaces, IPs, or UUIDs.
Microshift Test Compatibility	✅ Passed	No new Ginkgo tests were added. The changed assertion is inside an existing Describe whose BeforeAll skips MicroShift via IsMicroShiftCluster and g.Skip.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The diff adds no new Ginkgo tests or topology assumptions; it only adds output context to an existing assertion, and all test declarations match the parent revision.
Topology-Aware Scheduling Compatibility	✅ Passed	The only changed line adds diagnostic text to a Go test assertion; no deployment, controller, manifest, or scheduling constraint was added or modified.
Ote Binary Stdout Contract	✅ Passed	The PR changes only Ginkgo test assertions and captured oc.Run(...).Output() handling; no process-level stdout, fmt.Print, log, or klog writes were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adjusts existing tests only. No IPv4-only literals or parsing appear; the internal service binds to :: and uses net.JoinHostPort. example.com URLs are fixture data, not fetched.
No-Weak-Crypto	✅ Passed	The PR changes only conditional test output and error assertions; no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret comparisons were added.
Container-Privileges	✅ Passed	The PR changes only a Go test file. The diff contains no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or container root settings.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[hongkailiu]

It requires openshift/oc#2319 too.

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

test/extended/cli/adm_upgrade/recommend.go (1)

136-136: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle a missing FeatureGate/cluster as disabled.

When FeatureGate/cluster is absent, alertsEvaluatedByCVO must return (false, nil). Propagate other errors.

🤖 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/extended/cli/adm_upgrade/recommend.go` at line 136, Update
alertsEvaluatedByCVO so a missing FeatureGate/cluster is treated as disabled and
returns (false, nil), while propagating all other retrieval or parsing errors
unchanged. Use the existing feature-gate lookup and error-handling path rather
than altering unrelated version parsing.

Source: Learnings

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

Outside diff comments:
In `@test/extended/cli/adm_upgrade/recommend.go`:
- Line 136: Update alertsEvaluatedByCVO so a missing FeatureGate/cluster is
treated as disabled and returns (false, nil), while propagating all other
retrieval or parsing errors unchanged. Use the existing feature-gate lookup and
error-handling path rather than altering unrelated version parsing.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 398f236f-0241-4314-a02c-00c698689acb

📥 Commits

Reviewing files that changed from the base of the PR and between 0f26807 and 8092ff4.

📒 Files selected for processing (1)

• test/extended/cli/adm_upgrade/recommend.go

[hongkailiu]

/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-3of3

[openshift-ci]

@hongkailiu: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

• periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-1of3
• periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-2of3
• periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview-serial-3of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/39c59860-8d5a-11f1-9cc2-c83496b50918-0

[hongkailiu]

#31462 (comment) did not work.

INFO[2026-08-01T03:38:37Z] Requesting a release from https://arm64.ocp.releases.ci.openshift.org/api/v1/releasestream/5.0.0-0.nightly-arm64/latest 
INFO[2026-08-01T03:38:37Z] Resolved release nightly-arm64 to registry.ci.openshift.org/ocp-arm64/release-5-arm64:5.0.0-0.nightly-arm64-2026-08-01-030824

and

$ oc adm release info --registry-config /tmp/t.d.json registry.ci.openshift.org/ocp-arm64/release-5-arm64:5.0.0-0.nightly-arm64-2026-08-01-030824 -o json | jq -r '.references.spec.tags[]|select(.name=="cli")|.annotations["io.openshift.build.commit.id"]'
0f340425c9edb4a546c66ce10cc37942b1561861

Waiting for openshift/oc#2336 to get in nightly.