fix: OCPBUGS-86012: reset associatedVCenter in failure domain validation loop

[chdeshpa-hue]

Summary

• Reset associatedVCenter variable inside the failure domain validation loop so each failure domain is validated independently
• Add regression tests to verify invalid vCenter server references are caught regardless of failure domain ordering

Problem

When an install-config has multiple vSphere failure domains and one of them references a vCenter server that does not exist in the vcenters list, the installer silently skips the validation error if a valid failure domain appears earlier in the list. The validation function remembers the vCenter it found for the previous failure domain, so when it processes the next one with a non-existent server, it still thinks a match was found. This causes:

1. The "server does not exist in vcenters" error is never raised for the invalid failure domain
2. Datacenter validation runs against the wrong vCenter (the one from the previous iteration), producing either a misleading error or no error at all

The installer then proceeds past validation and crashes during machine asset generation with:

FATAL failed to fetch Master Machines: failed to generate asset "Master Machines":
  failed to create master machine objects: unable to find vCenter in failure domains:
  unable to find vCenter vcenter-TYPO.example.com

This error gives no indication that the root cause is a wrong server name in a failure domain. The expected behavior is an early validation error:

ERROR failed to load asset "Install Config": failed to create install config:
  invalid "install-config.yaml" file: platform.vsphere.failureDomains.server:
  Invalid value: "vcenter-TYPO.example.com": server does not exist in vcenters

The behavior is also order-dependent — swapping the order of failure domains in the YAML changes whether the bug triggers.

Fix

Move the var associatedVCenter *vsphere.VCenter declaration from outside the for loop to inside the loop body in validateFailureDomains(). This ensures the pointer resets to nil on each iteration, so every failure domain is validated against the vcenters list independently.

Test plan

• Run unit test TestValidateFailureDomainStaleVCenter — confirms "server does not exist in vcenters" error is raised when an invalid server appears after a valid one
• Create install-config with two failure domains: first valid, second referencing a non-existent vCenter server. Run openshift-install create manifests — should get a clear validation error pointing at the invalid server field instead of a late crash during machine generation
• Verify single failure domain configs (valid and invalid) continue to work as before
• Verify multi-failure-domain configs with all valid servers continue to work unchanged

Made with Cursor

Summary by CodeRabbit

• Bug Fixes

  • Updated failure domain validation to independently verify each domain's vCenter server and datacenter relationships, ensuring proper error detection and reporting for non-existent server references across multiple failure domains.

• Tests

  • Added comprehensive test coverage for failure domain validation to verify proper error handling and messaging when referencing non-existent vCenter servers.

[openshift-ci-robot]

@chdeshpa-hue: This pull request references Jira Issue OCPBUGS-86012, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

• Reset associatedVCenter variable inside the failure domain validation loop so each failure domain is validated independently
• Add regression tests to verify invalid vCenter server references are caught regardless of failure domain ordering

Problem

When an install-config has multiple vSphere failure domains and one of them references a vCenter server that does not exist in the vcenters list, the installer silently skips the validation error if a valid failure domain appears earlier in the list. The validation function remembers the vCenter it found for the previous failure domain, so when it processes the next one with a non-existent server, it still thinks a match was found. This causes:

1. The "server does not exist in vcenters" error is never raised for the invalid failure domain
2. Datacenter validation runs against the wrong vCenter (the one from the previous iteration), producing either a misleading error or no error at all

The installer then proceeds past validation and crashes during machine asset generation with:

FATAL failed to fetch Master Machines: failed to generate asset "Master Machines":
 failed to create master machine objects: unable to find vCenter in failure domains:
 unable to find vCenter vcenter-TYPO.example.com

This error gives no indication that the root cause is a wrong server name in a failure domain. The expected behavior is an early validation error:

ERROR failed to load asset "Install Config": failed to create install config:
 invalid "install-config.yaml" file: platform.vsphere.failureDomains.server:
 Invalid value: "vcenter-TYPO.example.com": server does not exist in vcenters

The behavior is also order-dependent — swapping the order of failure domains in the YAML changes whether the bug triggers.

Fix

Move the var associatedVCenter *vsphere.VCenter declaration from outside the for loop to inside the loop body in validateFailureDomains(). This ensures the pointer resets to nil on each iteration, so every failure domain is validated against the vcenters list independently.

Test plan

• Run unit test TestValidateFailureDomainStaleVCenter — confirms "server does not exist in vcenters" error is raised when an invalid server appears after a valid one
• Create install-config with two failure domains: first valid, second referencing a non-existent vCenter server. Run openshift-install create manifests — should get a clear validation error pointing at the invalid server field instead of a late crash during machine generation
• Verify single failure domain configs (valid and invalid) continue to work as before
• Verify multi-failure-domain configs with all valid servers continue to work unchanged

Made with Cursor

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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 48589351-9bfa-4009-8ef1-0b6750465974

📥 Commits

Reviewing files that changed from the base of the PR and between ceef9e4 and e22d81d.

📒 Files selected for processing (2)

• pkg/types/vsphere/validation/platform.go
• pkg/types/vsphere/validation/platform_test.go

---

Walkthrough

This PR fixes a variable scoping issue in vCenter failure-domain validation and adds test coverage for the fix. The associatedVCenter pointer is moved inside the per-failure-domain loop in validateFailureDomains to reset state correctly for each iteration. A new test verifies proper error detection when a failure domain references a non-existent vCenter server.

Changes

vCenter Validation Scoping

Layer / File(s)	Summary
Per-iteration vCenter validation fix pkg/types/vsphere/validation/platform.go	associatedVCenter pointer is declared within each failure-domain loop iteration instead of once before the loop, ensuring clean per-iteration state during server and datacenter validation. zoneNames map remains initialized outside the loop.
Stale vCenter reference test pkg/types/vsphere/validation/platform_test.go	TestValidateFailureDomainStaleVCenter verifies error detection when a failure domain references a non-existent vCenter server, testing both matching and mismatched topology scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

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.
Test Structure And Quality	⚠️ Warning	Check requires Ginkgo test review but repository uses standard Go testing package. Ginkgo not in go.mod. Check is inapplicable to this PR.	Clarify that this check applies only to Ginkgo BDD-style tests with Describe/It blocks. Standard Go tests use different patterns. Either exempt this PR or apply appropriate standard Go test quality checks.

✅ Passed checks (10 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely identifies the main bug fix: moving associatedVCenter declaration inside the loop to prevent stale pointer retention across failure domain iterations, matching the primary change in the changeset.
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 PR adds standard Go tests (testing.T) not Ginkgo BDD-style tests. The custom check applies only to Ginkgo test names (It(), Describe(), etc.). This check is not applicable to the PR.
Microshift Test Compatibility	✅ Passed	PR adds a standard Go unit test, not a Ginkgo e2e test. Custom check only applies to Ginkgo tests with It(), Describe(), etc. Check not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The new test (TestValidateFailureDomainStaleVCenter) is a standard Go unit test using testing.T, not a Ginkgo e2e test. The custom check applies only to Ginkgo e2e tests. This check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	This PR modifies installer configuration validation code, not deployment manifests, operator code, or controllers. The check is not applicable to changes in installer validation logic.
Ote Binary Stdout Contract	✅ Passed	No OTE Binary Stdout Contract violations detected. logrus calls write to stderr by default and are not in process-level code. fmt.Printf in test function is explicitly allowed per check rules.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adds a standard Go unit test, not a Ginkgo e2e test. The custom check only applies to Ginkgo e2e tests, so it is not applicable.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

Hi @chdeshpa-hue. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign dav1x 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:

• pkg/types/vsphere/validation/OWNERS

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

[chdeshpa-hue]

/jira refresh

[openshift-ci-robot]

@chdeshpa-hue: This pull request references Jira Issue OCPBUGS-86012, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

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.

[vr4manta]

/lgtm
/ok-to-test

[chdeshpa-hue]

/retest

[chdeshpa-hue]

@jcpowermac can this PR now be approved to merge?

[openshift-ci]

@chdeshpa-hue: The following tests 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/e2e-aws-ovn	0605d9e	link	true	/test e2e-aws-ovn
ci/prow/e2e-vsphere-ovn-devpreview	0605d9e	link	false	/test e2e-vsphere-ovn-devpreview

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.