WIP: feat: initial test for multi network arbiter support

[eggfoobar]

List all the issues related to this PR

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see CONTRIBUTING guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Highly available arbiter clusters can now accept multiple machine networks during cluster registration and configuration updates.
  • Enhanced network validation logic to properly support different cluster topology configurations while maintaining restrictions for non-arbiter clusters.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 71c57876-438b-46a8-a22e-bc9f2f206b9b

📥 Commits

Reviewing files that changed from the base of the PR and between 5b0651e and 64b5dfb.

📒 Files selected for processing (4)

• internal/bminventory/inventory.go
• internal/bminventory/inventory_test.go
• internal/cluster/validations/validation_test.go
• internal/cluster/validations/validations.go

---

Walkthrough

ValidateDualStackNetworks gains a new isArbiterCluster bool parameter. The validation that rejects single-stack clusters with multiple machine networks is now skipped when the cluster is an arbiter topology. Both call sites in inventory.go are updated, and unit and integration tests are added.

Changes

Arbiter dual-stack validation

Layer / File(s)	Summary
ValidateDualStackNetworks signature and logic change internal/cluster/validations/validations.go	Adds isArbiterCluster bool to the exported function signature; guards the multiple-machine-network rejection with !isArbiterCluster; validateVIPAddresses derives and passes the flag via common.IsClusterTopologyHighlyAvailableArbiter.
Inventory call site updates and integration test internal/bminventory/inventory.go, internal/bminventory/inventory_test.go	Both ValidateDualStackNetworks calls are updated: registration passes false, update passes the computed arbiter flag. An integration test asserts that a TNA cluster update with two machine-network CIDRs is accepted.
ValidateDualStackNetworks unit tests internal/cluster/validations/validation_test.go	Four Ginkgo cases verify rejection for non-arbiter single-stack, acceptance for arbiter, acceptance for user-managed load balancer, and acceptance of a single machine network for non-arbiter clusters.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is entirely boilerplate template content with no substantive information filled in. All checkboxes remain unchecked, no issues are linked, no testing details are provided, and there is no explanation of the actual changes being introduced.	Complete the PR description by: selecting appropriate issue type(s), linking related issues, explaining the motivation and context for arbiter support changes, detailing how the tests were validated, and checking relevant checklist items.
Test Structure And Quality	⚠️ Warning	Tests lack assertion messages as required by the custom check. All Expect() calls in both validation_test.go and inventory_test.go lack meaningful failure messages that would help diagnose failures.	Add descriptive messages to all assertions: e.g., Expect(err).NotTo(HaveOccurred(), "ValidateDualStackNetworks should allow arbiter clusters") and Expect(reply).To(BeAssignableToTypeOf(...), "V2UpdateCluster should succeed for TNA with m...
Ipv6 And Disconnected Network Test Compatibility	⚠️ Warning	New Ginkgo tests added contain hardcoded IPv4-only network CIDRs without IPv6 equivalents, violating IPv6-only CI compatibility requirements.	Update tests to use dual-stack CIDRs or detect IP family and use appropriate addresses. In ValidateDualStackNetworks, use both IPv4 (10.32.96.0/20) and IPv6 (2001:db8::/32) networks; in TNA cluster test, add IPv6 machine networks.
Title check	❓ Inconclusive	The title 'WIP: feat: initial test for multi network arbiter support' is partially related to the changeset but lacks clarity. While it references testing arbiter support, it omits the critical detail that the change also modifies the ValidateDualStackNetworks function signature to accept an isArbiterCluster parameter, which is a significant functional change beyond just adding tests.	Consider a more precise title that captures both the test addition and the underlying validation logic changes, such as 'feat: add arbiter support to dual-stack network validation' or clarify if this is truly WIP and should proceed to merge.

✅ Passed checks (11 passed)

Check name	Status	Explanation
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 four new test cases in the ValidateDualStackNetworks suite use stable, deterministic test names with no dynamic information, generated IDs, timestamps, or runtime values.
Microshift Test Compatibility	✅ Passed	The new tests are unit tests in internal/ packages with mocked services, not e2e tests. MicroShift compatibility check applies only to e2e tests per instructions.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The tests added are unit/integration tests using Ginkgo (in validation_test.go and inventory_test.go) that validate cluster parameter and network configurations with mocks, not e2e cluster tests. T...
Topology-Aware Scheduling Compatibility	✅ Passed	PR modifies only network validation logic and cluster inventory management; does not introduce deployment manifests, pod scheduling constraints, or controller code that would need topology-aware sc...
Ote Binary Stdout Contract	✅ Passed	PR contains no stdout writes in process-level code. Test functions only call RegisterFailHandler/RunSpecs, BeforeSuite only initializes DB, and all test code is properly wrapped in Ginkgo blocks.
No-Weak-Crypto	✅ Passed	No new weak cryptography (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB) or custom crypto implementations detected. Pre-existing MD5 usage is for proxy hash computation with #nosec comments and is not p...
Container-Privileges	✅ Passed	All PR changes are Go source/test files. The container-privileges check targets K8s/container manifests, which are not present or modified in this PR.
No-Sensitive-Data-In-Logs	✅ Passed	No logging statements were added in this PR, and no sensitive data (passwords, tokens, API keys, PII, session IDs, etc.) is being logged.

_{✏️ 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.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: eggfoobar
Once this PR has been reviewed and has the lgtm label, please assign rccrdpccl for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.29%. Comparing base (5b0651e) to head (64b5dfb).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/bminventory/inventory.go	0.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10480      +/-   ##
==========================================
+ Coverage   44.27%   44.29%   +0.01%     
==========================================
  Files         420      420              
  Lines       73332    73332              
==========================================
+ Hits        32471    32484      +13     
+ Misses      37934    37922      -12     
+ Partials     2927     2926       -1     

Files with missing lines	Coverage Δ
internal/cluster/validations/validations.go	49.15% <100.00%> (+2.94%)	⬆️
internal/bminventory/inventory.go	71.23% <0.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci]

@eggfoobar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-unit-test	64b5dfb	link	true	/test edge-unit-test

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.