Skip to content

NO-ISSUE: Adapt vsphere bootimage tests for mult-vcenter scenarios #6317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Aug 7, 2026
Merged

NO-ISSUE: Adapt vsphere bootimage tests for mult-vcenter scenarios #6317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions test/extended-priv/controlplanemachineset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,10 @@ func (cpms ControlPlaneMachineSet) GetCoreOsBootImage() (string, error) {
return "", err
}
coreOsBootImagePath = fmt.Sprintf(`{.spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.disks[%d].image}`, bootDiskIndex)
case VspherePlatform:
coreOsBootImagePath = `{.spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.template}`
case AzurePlatform:
coreOsBootImagePath = `{.spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.image}`
default:
e2e.Failf("ControlPlaneMachineSet.GetCoreOsBootImage method is only supported for GCP, Vsphere, Azure and AWS infrastructure")
e2e.Failf("ControlPlaneMachineSet.GetCoreOsBootImage method is only supported for GCP, Azure and AWS infrastructure")
}

return cpms.Get(coreOsBootImagePath)
Expand Down
46 changes: 46 additions & 0 deletions test/extended-priv/machineset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,11 @@ func (ms MachineSet) GetCoreOsBootImage() (string, error) {
return ms.Get(coreOsBootImagePath)
}

// GetWorkspaceFolder returns the workspace folder configured in the machineset's providerSpec
func (ms MachineSet) GetWorkspaceFolder() (string, error) {
return ms.Get(`{.spec.template.spec.providerSpec.value.workspace.folder}`)
}

// GetCoreOsBootImageOrFail returns the configured coreOsBootImage in this machineset and fails the test case if any error happened
func (ms MachineSet) GetCoreOsBootImageOrFail() string {
img, err := ms.GetCoreOsBootImage()
Expand Down Expand Up @@ -727,3 +732,44 @@ func (ms MachineSet) SetAutoscalerLabels(labels string) error {
fmt.Sprintf(`[{"op": "add", "path": "/metadata/annotations/capacity.cluster-autoscaler.kubernetes.io~1labels", "value": %s}]`,
string(marshaledLabels)))
}

// GetVSphereFailureDomain returns the failure domain from the infrastructure resource that matches
// the given MachineSet's workspace. It matches by comparing the workspace server and datacenter
// against each failure domain's server and topology.datacenter.
func GetVSphereFailureDomain(ms *MachineSet) (string, error) {
workspace, err := ms.Get(`{.spec.template.spec.providerSpec.value.workspace}`)
if err != nil {
return "", fmt.Errorf("error getting workspace from MachineSet %s: %w", ms.GetName(), err)
}

wsServer := gjson.Get(workspace, "server").String()
wsDataCenter := gjson.Get(workspace, "datacenter").String()
if wsServer == "" || wsDataCenter == "" {
return "", fmt.Errorf("workspace in MachineSet %s is missing server or datacenter", ms.GetName())
}

infra := NewResource(ms.GetOC().AsAdmin(), "infrastructure", "cluster")
failureDomains, err := infra.Get(`{.spec.platformSpec.vsphere.failureDomains}`)
if err != nil {
return "", fmt.Errorf("error getting failure domains from infrastructure resource: %w", err)
}

for _, fd := range gjson.Parse(failureDomains).Array() {
if fd.Get("server").String() == wsServer && fd.Get("topology.datacenter").String() == wsDataCenter {
return fd.Raw, nil
}
}
Comment on lines +757 to +761

@coderabbitai coderabbitai Bot Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the workspace fields available in MachineSet providerSpec usages.
rg -n --type=go -C 3 'providerSpec\.value\.workspace' test

# Inspect how failure domains are consumed elsewhere for matching criteria.
rg -n --type=go -C 5 'failureDomains' test

Repository: openshift/machine-config-operator

Length of output: 3706

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
fd -a 'machineset\.go|vsphere\.go' test | sed 's#^\./##'

echo
echo "== machineset GetVSphereFailureDomain and callers =="
sed -n '720,770p' test/extended-priv/machineset.go
echo
rg -n --type=go -C 4 'GetVSphereFailureDomain|VSphereConnectionInfoFromFailureDomain|failureDomains\[' test

echo
echo "== helper function =="
sed -n '420,485p' test/extended-priv/util/vsphere.go

Repository: openshift/machine-config-operator

Length of output: 7189

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== vSphere Machine API schema/docs references in repo =="
rg -n -i "failureDomains|computeCluster|resourcePool|datastore|topology.datacenter" . \
  -g '!vendor/**' -g '!docs/**' -g '!build/**' -g '!dist/**' \
  | head -n 200

echo
echo "== JSON examples with multiple failure domains or matching topology =="
rg -n -C 4 'failureDomains' . \
  -g '*.yaml' -g '*.yml' -g '*.json' -g '!vendor/**' -g '!build/**' -g '!dist/**' \
  | head -n 250

Repository: openshift/machine-config-operator

Length of output: 7706

Don’t match vSphere failure domains by server and datacenter only.

The workspace contains datastore and resourcePool, but this lookup compares only .spec.template.spec.providerSpec.value.server/datacenter to failureDomain.Topology.Datacenter. Two failure domains can share the same server/datacenter but use different datastore/resourcePool values, so GetVSphereConnectionInfoForMachineSet() may construct connection info from the wrong domain. Match using failureDomain.Topology.Datastore and failureDomain.Topology.ResourcePool from the workspace as well, or require a more specific workspace key unique to the failure domain.

🤖 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-priv/machineset.go` around lines 757 - 761, The failure-domain
lookup in GetVSphereConnectionInfoForMachineSet must not match only server and
datacenter; also compare the workspace’s datastore and resourcePool values
against the corresponding failure-domain topology fields, or use an equivalent
workspace key uniquely identifying the domain. Preserve returning fd.Raw only
for a fully matching failure domain.

@sergiordlr sergiordlr Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be fixed here https://redhat.atlassian.net/browse/OCPBUGS-105430

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sergiordlr, acknowledged. I will keep this finding open for this pull request. The follow-up is tracked in OCPBUGS-105430.

You are interacting with an AI system.

@djoshy djoshy Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be addressed in https://redhat.atlassian.net/browse/OCPBUGS-105430

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


return "", fmt.Errorf("no failure domain found matching server=%s datacenter=%s for MachineSet %s", wsServer, wsDataCenter, ms.GetName())
}

// GetVSphereConnectionInfoForMachineSet returns the vSphere connection info for the failure domain
// that matches the given MachineSet's workspace.
func GetVSphereConnectionInfoForMachineSet(ms *MachineSet) (*exutil.VSphereConnectionInfo, error) {
fd, err := GetVSphereFailureDomain(ms)
if err != nil {
return nil, err
}

return exutil.GetVSphereConnectionInfoFromFailureDomain(ms.GetOC().AsAdmin(), fd)
}
Loading