ROSAENG-61173 | test: complete delete cluster command coverage

[olucasfreitas]

PR Summary

Complete the remaining coverage for rosa delete cluster by adding command-level tests for the legacy delete flow and a small refactor that makes the command body testable without changing CLI behavior.

Detailed Description of the Issue

PR #3298 added helper coverage for handleClusterDelete() and buildCommands(), but the actual rosa delete cluster command flow was still untested. Phase 2B still required coverage for confirmation handling, best-effort output, STS cleanup messaging, uninstall log watching, and wrapper-level error propagation.

This change extracts a testable runWithRuntime(...) helper in cmd/dlt/cluster/cmd.go and adds focused Ginkgo coverage for the remaining command behaviors while preserving the existing Run: run entrypoint, flags, and user-facing flow.

Related Issues and PRs

• Jira: ROSAENG-61173
• Fixes: N/A
• Related PR(s): ROSAENG-60112 | test: add tests for delete cluster command #3298
• Related design/docs: coverage planning tracked under ROSAENG-60112

Type of Change

• feat - adds a new user-facing capability.
• fix - resolves an incorrect behavior or bug.
• docs - updates documentation only.
• style - formatting or naming changes with no logic impact.
• refactor - code restructuring with no behavior change.
• test - adds or updates tests only.
• chore - maintenance work (tooling, housekeeping, non-product code).
• build - changes build system, packaging, or dependencies for build output.
• ci - changes CI pipelines, jobs, or automation workflows.
• perf - improves performance without changing intended behavior.

Previous Behavior

cmd/dlt/cluster had helper-level coverage, but the full command flow remained untested. Important behaviors like confirmation decline, --best-effort, STS cleanup output, --watch, and wrapper-level error handling were still gaps in Phase 2B.

Behavior After This Change

The delete-cluster command now has focused command-level coverage for:

• non-STS happy path with uninstall-log suggestion output
• STS happy path with operator role output, OIDC provider output, and cleanup commands
• STS clusters with no operator roles
• --best-effort warning and passthrough behavior
• confirmation decline
• already-uninstalling clusters
• GetClusterState and DeleteCluster errors
• --watch invoking uninstall logs while restoring the region deprecation warning flag

The command shape, flags, and user-facing behavior remain unchanged.

How to Test (Step-by-Step)

Preconditions

• Go toolchain compatible with the version required by go.mod
• If your shell exports GOTOOLCHAIN=local, use GOTOOLCHAIN=auto for the commands below

Test Steps

1. GOTOOLCHAIN=auto make fmt
2. GOTOOLCHAIN=auto go test ./cmd/dlt/cluster/... -count=1
3. GOTOOLCHAIN=auto make lint
4. GOTOOLCHAIN=auto make rosa
5. GOTOOLCHAIN=auto make test

Expected Results

• The cmd/dlt/cluster package tests pass
• Lint passes with no issues
• The rosa binary builds successfully
• The full repo test suite passes

Proof of the Fix

• Logs/CLI output:
  • go test ./cmd/dlt/cluster/... -count=1 passed
  • make lint passed (0 issues)
  • make rosa passed
  • make test passed
  • git push pre-push checks: 5/5 passed (format, build, lint, changed-files coverage, unit/integration tests)

Breaking Changes

• No breaking changes

Developer Verification Checklist

• Commit subject/title follows [JIRA-TICKET] | [TYPE]: <MESSAGE>.
• PR description clearly explains both what changed and why.
• Relevant Jira/GitHub issues and related PRs are linked.
• make install-hooks has been run in this clone.
• Tests were added/updated where appropriate.
• I manually tested the change.
• make test passes.
• make lint passes.
• make rosa passes.
• Documentation or repo-local agent guidance was added/updated where appropriate.
• Any risk, limitation, or follow-up work is documented.

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes

  • Improved rosa delete cluster control flow, error propagation, and output when confirmation is declined.
  • Clearer guidance for clusters already uninstalling and for failures during cluster state checks or delete operations (including forbidden errors).

• User Experience

  • Enhanced --watch behavior to run uninstall logs automatically and adjust messaging/hints accordingly.
  • Better handling of best-effort warnings and STS operator-role cleanup guidance.

• Tests

  • Expanded coverage for runtime, watch mode, warning behavior, messaging, and failure scenarios.

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: olucasfreitas

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

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

[coderabbitai]

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

Refactors rosa delete cluster into separate runtime and execution flows, returning errors from the inner handler and exiting centrally. Confirmation and uninstall-log execution are extracted behind helpers. Watch mode preserves deprecation-warning flag restoration, while non-watch mode retains the logs hint. Tests cover confirmation, deletion outcomes, STS cleanup messaging, watch behavior, flag restoration, and error paths.

Suggested reviewers: davidleerh, gdbranco

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is specific, matches the main change, and uses the required ticket/type format.
Description check	✅ Passed	The description covers summary, issue, related items, change type, behavior, testing, proof, and checklist with the required sections.
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 titles in cmd/dlt/cluster/cmd_test.go are static, descriptive strings with no generated or interpolated values.
Test Structure And Quality	✅ Passed	PASS: The new Ginkgo cases are narrowly scoped, reset shared state in BeforeEach, add no indefinite waits or cluster-scoped resources, and match existing capture-test patterns.
Microshift Test Compatibility	✅ Passed	These are command/unit Ginkgo tests using mocked OCM data only; no MicroShift-unsupported OpenShift APIs, namespaces, or cluster features are exercised.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The added Ginkgo specs are command/unit tests for rosa delete cluster; no SNO/topology assumptions are present.
Topology-Aware Scheduling Compatibility	✅ Passed	Only a delete-cluster CLI refactor and tests changed; no deployment manifests, operator code, controllers, or scheduling constraints were added.
Ote Binary Stdout Contract	✅ Passed	No stdout writes were added in init/main/TestMain/RunSpecs setup; the only new fmt.Print is inside command execution, not process-level code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The new Ginkgo specs are command-level unit tests, and I found no IPv4-only assumptions or external network calls.
No-Weak-Crypto	✅ Passed	Touched files add CLI flow/tests only; no weak crypto or secret-comparison code was introduced.
Container-Privileges	✅ Passed	PR only changes Go command/test code; no privileged/host* allowPrivilegeEscalation/SYS_ADMIN settings appear in touched files or repo manifests.
No-Sensitive-Data-In-Logs	✅ Passed	No new sensitive logging was introduced; the refactor preserved existing cluster/account/OIDC output and tests use synthetic values.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[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 `@cmd/dlt/cluster/cmd.go`:
- Around line 85-94: Wrap the error returned by handleClusterDelete in
runWithRuntime with operation and cluster-key context using %w, preserving the
original error for matching. Keep run’s existing reporter and exit behavior
unchanged so the centralized reporting receives the contextualized failure.

🪄 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: caa151cc-eb84-4f79-9d67-6944c84ca026

📥 Commits

Reviewing files that changed from the base of the PR and between e4eecdd and 85ab16a.

📒 Files selected for processing (2)

• cmd/dlt/cluster/cmd.go
• cmd/dlt/cluster/cmd_test.go

[coderabbitai]

🧹 Nitpick comments (1)

cmd/dlt/cluster/cmd.go (1)

136-139: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Restore the previous warning state with defer.

Because this is package-level state, capture its prior value and defer restoration around uninstallLogsFn. The current hard-coded reset only handles normal returns and can leak or overwrite state after callback failures.

🤖 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 `@cmd/dlt/cluster/cmd.go` around lines 136 - 139, Update the args.watch branch
around DisableRegionDeprecationWarning and uninstallLogsFn to capture the flag’s
existing value, defer restoring that exact value, then temporarily set it to
true while uninstallLogsFn runs; remove the hard-coded false reset so callback
failures also restore package-level state.

🤖 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 `@cmd/dlt/cluster/cmd.go`:
- Around line 136-139: Update the args.watch branch around
DisableRegionDeprecationWarning and uninstallLogsFn to capture the flag’s
existing value, defer restoring that exact value, then temporarily set it to
true while uninstallLogsFn runs; remove the hard-coded false reset so callback
failures also restore package-level state.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: c319e62d-08f0-4caa-9363-a32d51c5f667

📥 Commits

Reviewing files that changed from the base of the PR and between e7c9126 and 3c4c631.

📒 Files selected for processing (2)

• cmd/dlt/cluster/cmd.go
• cmd/dlt/cluster/cmd_test.go

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

• cmd/dlt/cluster/cmd_test.go