ARO-28167: support control plane resize zone topologies

[tuxerrante]

Which issue this PR addresses:

Fixes ARO-28167

What this PR does / why we need it:

This updates the control plane resize pre-validation logic so it classifies the supported topology shapes explicitly instead of assuming every valid control plane must span three distinct availability zones. The change now accepts regional/no-zone, single-zone, and three-zone control planes, while still rejecting mixed or partial layouts such as 1/1/2 and zonal/regional combinations.

Reviewer notes:

• Azure VMs with Zones == nil or Zones == [] are treated as regional VMs, not as invalid state.
• A single repeated explicit zone is treated as a valid single-zone topology, which is different from a regional topology.
• We sort AZ names before returning the mixed-topology error so validation failures are deterministic across Go map iteration order. This keeps the error text stable in tests and easier to review/debug.
• The new classifyZoneTopology helper is generic over input type, but it is still intentionally control-plane-scoped because it enforces azurezones.CONTROL_PLANE_MACHINE_COUNT. If future Capacity Reservation / CRG validation needs a different cardinality, the expected count should be passed into the helper or kept at the caller instead of assuming 3.
• Related background for future capacity-reservation-aware resize work: PR #5009

Test plan for issue:

Executed:

• make fmt
• git diff --check
• make lint-go
• make unit-test-go
• go test -count=1 ./pkg/frontend/...
• make test-go was executed to assess the broader repo validator behavior; it currently chains mutating generation/import-fix steps, so unrelated generated churn was reverted and it was not used as the clean publish gate for this branch.

Automated coverage added/updated for:

• regional / no-zone control planes
• single-zone control planes
• three-zone control planes
• mixed and partial topology rejection
• empty and nil Azure VM zone handling
• machine/Azure VM topology mismatch cases

Not executed:

• Live validation on real ARO clusters deployed in regions with different topology shapes (regional, single-zone, three-zone). I did not have enough capacity in this change to stand up and exercise those environments end to end. Follow-up manual validation is TBD.

Is there any documentation that needs to be updated for this PR?

No product or customer-facing documentation changes are needed for this PR because the change is limited to internal admin resize validation behavior.

Suggested separate repo/process documentation follow-up:

• Document more clearly that make test-go is a broader validation workflow, not just a pure unit-test target, or introduce a clearly named non-mutating test target for contributor feedback loops.

How do you know this will function as expected in production?

• Resize execution and rollback behavior are unchanged; this PR only adjusts the pre-flight validation layer.
• Existing safety checks remain in place: machine label/spec consistency, machine/Azure VM consistency, VM power-state checks, and node validation.
• The change is conservative: it broadens acceptance only to topology shapes that are already valid for ARO control planes, while continuing to fail closed on mixed or partial shapes.
• The updated test coverage now matches the intended topology matrix and protects the new regional/single-zone behavior from regressions.
• Additional live validation on real clusters in regional, single-zone, and three-zone regions would still be useful as follow-up confidence, but that is outside the scope of this PR.

[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

[tuxerrante]

/test all

[Copilot]

@copilot review

Here is my review of the changes in this PR.

---

Summary

The core topology classification logic is sound and the test coverage is comprehensive. All tests pass. A few minor items below:

---

classifyZoneTopology — comment is misplaced

In admin_openshiftcluster_resize_validation_helpers.go, the explanatory comment appears after the sort.Strings call it describes rather than before it:

sort.Strings(zoneNames)

// Sort zones to keep validation errors deterministic across map iterations.
return "", fmt.Errorf("items have unsupported mixed zone topology: zones %q", zoneNames)

Should be:

// Sort zones to keep validation errors deterministic across map iterations.
sort.Strings(zoneNames)
return "", fmt.Errorf("items have unsupported mixed zone topology: zones %q", zoneNames)

---

getAzureVMs — inconsistent error collection

Power-state failures are collected and returned together via errors.Join, but a GetVirtualMachine lookup failure returns immediately:

vm, err := azureAction.GetVirtualMachine(...)
if err != nil {
    return nil, api.NewCloudError(...)   // immediate return — other VMs not checked
}
...
err = validateVMPowerState(...)
if err != nil {
    validationErrs = append(validationErrs, err)  // collected
}

If two VMs are unreachable, only the first error is surfaced. This is a reasonable fail-fast design choice for infrastructure errors, but a brief comment explaining the distinction from the collected power-state errors would make the intent explicit.

---

validateVMPowerState — hardcoded count check

The check if len(vmStatuses) != 2 would fail if Azure ever returns an additional status code (e.g., a maintenance or hibernate state). Checking >= 2 for presence (and separately validating unexpected codes) would be more resilient. The existing inline comment acknowledges the intent, so this may be by design.

---

Nits

• validateClusterMachinesAndVMs uses %v for string arguments (machine %v not found, has zone %v) where %s is idiomatic.
• The zoneTopology type is declared after the const block that uses it. Go resolves this fine, but declaring the type first is conventional.

---

What is correct

• The classifyZoneTopology logic handles all edge cases correctly: regional (all-empty zones), single-zone (all same explicit zone), three-zone (all distinct non-empty zones), and the mixed cases that should fail.
• Nil and empty vm.Zones slices are both correctly treated as regional, matching the Azure API semantics.
• errors.Join is used consistently for aggregating multiple validation errors before returning.
• The classifyZoneTopology generic helper is called independently in both validateClusterMachines and getAzureVMs, so topology mismatches are caught at each layer before the cross-layer validateClusterMachinesAndVMs check runs.

[Copilot]

Pull request overview

This PR updates the admin control-plane resize pre-validation logic to explicitly classify supported control-plane availability zone topology shapes (regional/no-zone, single-zone, three-zone) rather than assuming all valid control planes must span three distinct AZs. This aligns validation with real Azure VM zoning semantics (including Zones == nil / empty slice meaning regional) while keeping mixed/partial layouts rejected with deterministic error output.

Changes:

• Introduce classifyZoneTopology to validate/control-plane-scope supported zone topologies and return a topology classification.
• Treat Azure VMs with nil/empty Zones as regional VMs (valid), and accept single-zone control planes.
• Update and expand unit tests to cover the new topology matrix and deterministic mixed-topology error messaging.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/frontend/admin_openshiftcluster_resize_validation_helpers.go	Replace “must be 3 distinct zones” validation with topology classification; treat empty/nil VM zones as regional; improve determinism for mixed-topology errors.
pkg/frontend/admin_openshiftcluster_resize_validation_helpers_test.go	Update tests for topology classification and add coverage for regional/single-zone and nil/empty Azure VM zones.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tuxerrante]

@copilot review

[Copilot]

@copilot review

All the feedback from the previous review has been incorporated in the existing commits:

• Misplaced comment in classifyZoneTopology — the // Sort zones to keep validation errors deterministic... comment now appears before sort.Strings(zoneNames) (line 350–351).
• Non-deterministic node names in error message (PR review thread on lines 435–441) — sort.Strings(nodeNames) is already present before joining them (lines 439–440).
• getAzureVMs error collection asymmetry — a comment at lines 209–212 explains the fail-fast behavior for VM lookup failures vs. aggregated collection for power-state errors.
• validateVMPowerState hardcoded != 2 check — updated to < 2 (line 358), which accepts additional status codes while still requiring the minimum expected two statuses.
• %v for string arguments — all format strings use %s for string values throughout the file.
• zoneTopology type declared after const block — the type declaration is now at line 30, before the const block that uses it (lines 32–47).

The CI node-lint failure is a pre-existing npm vulnerability in portal/v2 (react-router) and api (brace-expansion, tar) — unrelated to the Go changes in this PR. All Go tests pass.

[tuxerrante]

Current CI note: the npm-audit-api / npm-audit-portal/v2 failures look dependent on #5007, which is already open to fix those audit checks in both api and portal/v2. I am treating those as external baseline failures unless/until #5007 lands or proves insufficient.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

pkg/frontend/admin_openshiftcluster_resize_validation_helpers.go:359

• validateVMPowerState currently only checks that there are at least 2 statuses and that all statuses are in the allowed set. This can incorrectly accept a VM that is missing one of the required states (e.g., two copies of "PowerState/running" with no provisioning state, or vice versa), despite the comment saying both provisioning and power states are required.

func validateVMPowerState(log *logrus.Entry, vmStatuses []string, vmName string) error {
	// We require at least the provisioning and power states; any additional
	// statuses are validated below and rejected if unexpected.
	if len(vmStatuses) < 2 {
		err := fmt.Errorf("expected 2 statuses for VM %s, but found %d: %s", vmName, len(vmStatuses), strings.Join(vmStatuses, ", "))