OCPBUGS-104569: "vSphere boot image reconciler overwrites/renames current custom-named templates" by jcpowermac · Pull Request #6354 · openshift/machine-config-operator

jcpowermac · 2026-07-30T21:42:31Z

Will fill in this later

- What I did

- How to verify it

- Description for the changelog

Summary by CodeRabbit

Bug Fixes
- vSphere credentials are now selected reliably for the configured server, improving multi-vCenter operation.
- Boot image reconciliation preserves valid custom template names and independently handles multiple vCenters and failure domains.
- Improved error reporting for failed ignition upgrades.
Tests
- Expanded coverage for OVA caching, vSphere templates, resource discovery, networking, storage, secure boot, and platform support.
- Added end-to-end validation for vSphere OVA imports, recovery scenarios, and multi-vCenter reconciliation.

openshift-merge-bot · 2026-07-30T21:42:34Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci · 2026-07-30T21:42:36Z

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

jcpowermac · 2026-07-30T21:42:42Z

/test ?

coderabbitai · 2026-07-30T21:43:30Z

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

The change adds unit, simulator-backed, OVF import, and extended-privilege coverage for boot image caching, platform handling, vSphere template reconciliation, and workspace-specific vSphere workflows.

Changes

Boot image and vSphere workflows

Layer / File(s)	Summary
Cache and platform validation `pkg/controller/bootimage/cache/cache_test.go`, `pkg/controller/bootimage/ms_helpers_test.go`, `pkg/controller/bootimage/platform_helpers_test.go`, `pkg/controller/common/featuregates_test.go`, `pkg/controller/bootimage/helpers.go`	Tests cache hits, cache repair, download failures, provider-spec handling, platform feature-gate values, and corrected error wrapping.
vSphere simulator and resource helpers `pkg/controller/bootimage/vsphere_simulator_test.go`, `pkg/controller/bootimage/vsphere_object_helpers_test.go`	Adds simulator fixtures and tests client creation, inventory lookup, host selection, VM lifecycle operations, template swaps, and tag attachment.
Template reconciliation and OVF import `pkg/controller/bootimage/vsphere_helpers.go`, `pkg/controller/bootimage/vsphere_helpers_test.go`, `pkg/controller/bootimage/vsphere_create_template_test.go`, `pkg/controller/bootimage/vsphere_ova_import_spike_test.go`	Tests OVF helpers, template replacement, rollback recovery, version and name validation, custom template preservation, and simulator OVF import.
Workspace-specific connection and reconciliation `test/extended-priv/util/vsphere.go`, `test/extended-priv/mco_bootimages.go`, `test/extended-priv/mco_scale.go`	Adds server-specific credential and failure-domain lookup, workspace-specific image upload, and reconciliation coverage across vCenters and failure domains.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MachineSet
  participant Controller
  participant VCenterClient
  participant OVA
  MachineSet->>Controller: reconcile workspace template
  Controller->>VCenterClient: resolve server and datacenter
  Controller->>OVA: upload selected OVA
  OVA-->>VCenterClient: provide OVA content
  VCenterClient-->>Controller: return template release
  Controller-->>MachineSet: preserve or update template reference

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	Added logging in mco_bootimages.go prints the vSphere server hostname via `server %s`, which can expose internal hostnames in test logs.	Remove the server value from logs or replace it with a non-sensitive failure-domain identifier or redacted hostname.
Ipv6 And Disconnected Network Test Compatibility	⚠️ Warning	Three new vSphere Ginkgo e2e tests require external connectivity to raw.githubusercontent.com to download RHCOS image metadata, which will fail in disconnected IPv6-only CI environments.	Add `[Skipped:Disconnected]` to the test names (lines 685, 763, 776 in mco_bootimages.go) to automatically skip them on disconnected clusters, or mock/cache the external GitHub calls.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add VMware test coverage' directly matches the main objective of the pull request: adding comprehensive test coverage for VMware/vSphere functionality across multiple test files and helper modules.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
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 test names across 8 new test files and extended tests contain only static, descriptive strings with no dynamic values like timestamps, UUIDs, pod names, or generated identifiers.
Test Structure And Quality	✅ Passed	Unit tests follow Go testing best practices with meaningful assertion messages, proper cleanup via t.Cleanup(), single responsibility per t.Run() block, and helper functions for fixture creation. E...
Microshift Test Compatibility	✅ Passed	Three new Ginkgo e2e tests were added to test/extended-priv/mco_bootimages.go. Although they use the MachineSet API (unavailable on MicroShift), they are protected by JustBeforeEach platform checks...
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The three new Ginkgo e2e tests in test/extended-priv/mco_bootimages.go are protected from SNO execution by the parent Describe block's JustBeforeEach, which calls SkipTestIfWorkersCannotBeScaled. T...
Topology-Aware Scheduling Compatibility	✅ Passed	PR adds test coverage for vSphere bootimage controller. Production changes are limited to a vSphere template helper function and a typo fix. No scheduling constraints, deployments, pod affinity, or...
Ote Binary Stdout Contract	✅ Passed	All new test files comply with OTE Binary Stdout Contract: no process-level stdout writes detected, klog properly configured via custom GinkgoWriter wrapper, no suite-level violations found.
No-Weak-Crypto	✅ Passed	The cumulative diff adds only SHA-256 hashing and standard TLS setup; no MD5, SHA-1, DES, RC4, Blowfish, ECB, custom crypto, or secret-value equality checks were found.
Container-Privileges	✅ Passed	This PR adds test coverage for vSphere boot image controller functionality using only Go test files and source code. No Kubernetes manifests or container definitions are introduced or modified, mak...

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

jcpowermac · 2026-07-30T21:43:46Z

/test unit
/test e2e-vsphere

jcpowermac · 2026-07-30T21:47:10Z

/test e2e-vsphere-ovn-zones

coderabbitai

🧹 Nitpick comments (3)

pkg/controller/bootimage/vsphere_helpers.go (1)
232-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale sibling comment: "17 chars" still present elsewhere in this file.

This comment was correctly fixed to say 16 chars, but createNewVMTemplateWithNameForFailureDomain in the same file still documents tempName/oldTempName as "fixed-length (17 chars)", which is now inconsistent with both this fix and TestAtomicTempName's len(nameA) != 16 assertion. Please update that comment too for consistency.
📝 Suggested fix
-	// tempName is where the new OVA is imported; oldTempName holds the existing template during
-	// the swap. Both are fixed-length (17 chars) so they always fit within vSphere's 80-char limit.
+	// tempName is where the new OVA is imported; oldTempName holds the existing template during
+	// the swap. Both are fixed-length (16 chars) so they always fit within vSphere's 80-char limit.
🤖 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 `@pkg/controller/bootimage/vsphere_helpers.go` at line 232, Update the
documentation in createNewVMTemplateWithNameForFailureDomain so tempName and
oldTempName are described as fixed-length 16-character names, matching
atomicTempName and the TestAtomicTempName assertion.
pkg/controller/bootimage/vsphere_ova_import_spike_test.go (1)
145-268: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Is this spike test still needed now that full coverage exists?

This test self-documents as "a throwaway spike (see plan A0)", and its "vcsim supports OVF import" premise is now proven more thoroughly by TestCreateNewVMTemplate in vsphere_create_template_test.go (which also exercises rollback/error paths this spike doesn't). It also re-implements ~50 lines of simulator bring-up already centralized in newSimulatedVCenter/newSimulatedVCenters (vsphere_simulator_test.go).

Consider either removing TestSpikeVcsimSupportsOvfImport (keeping buildMinimalOVA, which is still used by vsphere_create_template_test.go) or refactoring it to reuse newSimulatedVCenter to cut the duplication if it's being kept intentionally as a standalone smoke test.
🤖 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 `@pkg/controller/bootimage/vsphere_ova_import_spike_test.go` around lines 145 -
268, The standalone TestSpikeVcsimSupportsOvfImport is now redundant with
TestCreateNewVMTemplate and duplicates simulator setup. Remove the spike test
while retaining buildMinimalOVA because it is still used elsewhere; if the test
is intentionally preserved, refactor its setup to use newSimulatedVCenter or
newSimulatedVCenters instead of creating the simulator inline.
pkg/controller/bootimage/vsphere_create_template_test.go (1)
98-329: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

LGTM! One small optional note: the newSimulatedVCenters(t, 1) / buildFailureDomain / buildVSphereInfra / buildVSphereCredsSecret quartet is repeated verbatim across most subtests here — could be pulled into a small newSingleVCenterFixture(t) helper, but the current explicit style keeps each subtest self-contained, so this is purely optional.
🤖 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 `@pkg/controller/bootimage/vsphere_create_template_test.go` around lines 98 -
329, Optionally reduce repeated single-vCenter test setup by introducing a
newSingleVCenterFixture helper that encapsulates newSimulatedVCenters,
buildFailureDomain, buildVSphereInfra, and buildVSphereCredsSecret. Update
applicable TestCreateNewVMTemplate subtests to use the helper while preserving
their independent setup and behavior; this refactor is optional.

🤖 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 `@pkg/controller/bootimage/vsphere_create_template_test.go`:
- Around line 98-329: Optionally reduce repeated single-vCenter test setup by
introducing a newSingleVCenterFixture helper that encapsulates
newSimulatedVCenters, buildFailureDomain, buildVSphereInfra, and
buildVSphereCredsSecret. Update applicable TestCreateNewVMTemplate subtests to
use the helper while preserving their independent setup and behavior; this
refactor is optional.

In `@pkg/controller/bootimage/vsphere_helpers.go`:
- Line 232: Update the documentation in
createNewVMTemplateWithNameForFailureDomain so tempName and oldTempName are
described as fixed-length 16-character names, matching atomicTempName and the
TestAtomicTempName assertion.

In `@pkg/controller/bootimage/vsphere_ova_import_spike_test.go`:
- Around line 145-268: The standalone TestSpikeVcsimSupportsOvfImport is now
redundant with TestCreateNewVMTemplate and duplicates simulator setup. Remove
the spike test while retaining buildMinimalOVA because it is still used
elsewhere; if the test is intentionally preserved, refactor its setup to use
newSimulatedVCenter or newSimulatedVCenters instead of creating the simulator
inline.

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: c8939da4-5ac0-4168-b71f-c060c96fd52f

📥 Commits

Reviewing files that changed from the base of the PR and between 23687b4 and ca627e9.

⛔ Files ignored due to path filters (113)

vendor/github.com/vmware/govmomi/simulator/alarm_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/authorization_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/cluster_compute_resource.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/container.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/container_host_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/container_virtual_machine.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/crypto_manager_kmip.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/custom_fields_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/customization_spec_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/datacenter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/dataset.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/datastore.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/datastore_namespace_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/dvs.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/dvs_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/entity.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/environment_browser.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/authorization_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/datacenter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/event_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_capability.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_config_filesystemvolume.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_config_info.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_firewall_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_hardware_info.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_storage_device_info.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/host_vnic_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/performance_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/performance_manager_data.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/resource_pool.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/root_folder.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/service_content.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/setting.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/task_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/esx/virtual_device.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/event_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/extension_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/file_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/folder.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/guest_id.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/guest_operations_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/history_collector.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_certificate_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_datastore_browser.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_datastore_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_firewall_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_local_account_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_network_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_storage_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_system.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/host_vnic_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/http_nfc_lease.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/internal/object_lock.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/internal/server.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/internal/testcert.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/internal/types.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/ip_pool_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/license_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/model.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/object.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/option_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/ovf_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/performance_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/portgroup.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/property_collector.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/property_filter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/registry.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/resource_pool.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/search_index.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/service_instance.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/session_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/simulator.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/snapshot.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/storage_resource_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/task.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/task_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/tenant_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/user_directory.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/view_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/virtual_disk_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/virtual_machine.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vm_compatibility_checker.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vm_provisioning_checker.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/alarm_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/performance_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/performance_manager_data.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/root_folder.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/service_content.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/setting.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vpx/task_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/simulator/vstorage_object_manager.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/hgfs/archive.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/hgfs/encoding.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/hgfs/hgfs_linux.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/hgfs/hgfs_other.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/hgfs/protocol.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/hgfs/server.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/process/process.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/vix/property.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/toolbox/vix/protocol.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/units/size.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/resource.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/simulator/simulator.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/vcenter/vcenter_ovf.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/vcenter/vcenter_vmtx.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/vm/dataset/dataset.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/vmware/govmomi/vapi/vm/internal/internal.go is excluded by !**/vendor/**, !vendor/**
vendor/modules.txt is excluded by !**/vendor/**, !vendor/**

📒 Files selected for processing (12)

pkg/controller/bootimage/cache/cache_test.go
pkg/controller/bootimage/ms_helpers_test.go
pkg/controller/bootimage/platform_helpers_test.go
pkg/controller/bootimage/vsphere_create_template_test.go
pkg/controller/bootimage/vsphere_helpers.go
pkg/controller/bootimage/vsphere_helpers_test.go
pkg/controller/bootimage/vsphere_object_helpers_test.go
pkg/controller/bootimage/vsphere_ova_import_spike_test.go
pkg/controller/bootimage/vsphere_simulator_test.go
pkg/controller/common/featuregates_test.go
test/extended-priv/mco_bootimages.go
test/extended-priv/util/vsphere.go

…ator main Adds three optional/on-demand presubmit jobs (plain IPI, zonal single-vCenter, multi-vCenter) scoped to the MCO Bootimages longduration spec group, so the new vSphere multi-vCenter/multi-failure-domain reconciliation e2e specs in openshift/machine-config-operator#6354 can be exercised via /test before they land in a periodic. Reuses the existing openshift-e2e-vsphere and openshift-e2e-vsphere-zones workflows, differentiated only by MAX_VCENTERS per SPLAT-2238 (openshift#81215), instead of a separate hardcoded-pool multi-vcenter workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ator main (#82723) Adds three optional/on-demand presubmit jobs (plain IPI, zonal single-vCenter, multi-vCenter) scoped to the MCO Bootimages longduration spec group, so the new vSphere multi-vCenter/multi-failure-domain reconciliation e2e specs in openshift/machine-config-operator#6354 can be exercised via /test before they land in a periodic. Reuses the existing openshift-e2e-vsphere and openshift-e2e-vsphere-zones workflows, differentiated only by MAX_VCENTERS per SPLAT-2238 (#81215), instead of a separate hardcoded-pool multi-vcenter workflow. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

jcpowermac · 2026-08-03T11:24:57Z

/test ?

jcpowermac · 2026-08-03T11:25:18Z

/test e2e-vsphere-multi-vcenter-mco-bootimages

jcpowermac · 2026-08-03T15:03:45Z

/test ?

jcpowermac · 2026-08-03T15:04:27Z

/test e2e-vsphere-mco-bootimages
/test e2e-vsphere-multi-vcenter-mco-bootimages
/test e2e-vsphere-zones-mco-bootimages

coderabbitai

Actionable comments posted: 5

🧹 Nitpick comments (1)

test/extended-priv/mco_bootimages.go (1)
862-866: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unused fdGroup type.

fdGroup is declared but never instantiated; the code uses fdByServer/fdOrder instead. This is dead code that a static-analysis unused check could flag.
🤖 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/mco_bootimages.go` around lines 862 - 866, Remove the
unused fdGroup type declaration near fdOrder, leaving the fdByServer and fdOrder
implementation unchanged.

🤖 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 `@pkg/controller/bootimage/vsphere_helpers.go`:
- Around line 622-630: Update the isTemplateAtRelease call in the
providerSpec.Template preservation check to capture its error instead of
discarding it. Ignore only *find.NotFoundError as the benign “not current” case;
propagate any other lookup or Properties() error, matching the existing
existingTemplateVM error handling, while preserving the current-template early
return.

In `@test/extended-priv/mco_bootimages.go`:
- Around line 1342-1352: Handle and propagate errors from both MachineSet
workspace lookups in the block before Eventually, rather than discarding them.
Update the surrounding verification flow so failures from ms.Get for
workspace.server or workspace.datacenter are surfaced and prevent
getReleaseFromVsphereTemplate from proceeding with empty values or the default
vCenter fallback.
- Around line 831-949: Fix buildWorkspaceGroupsAcrossVCenters so partial
failures clean up synthetic MachineSets created before the failure. Define the
cleanup closure before the server-processing loop, keep it referencing the
evolving syntheticMachineSets slice, and return that cleanup function on every
subsequent error path instead of func() {}.
- Around line 896-928: Update the CloneResource transformation callback to set
the synthetic MachineSet’s network.devices[0].networkName field to fd.Network,
alongside the existing workspace overrides. Preserve the donor clone behavior
for all other fields so scaling uses the requested topology network.

In `@test/extended-priv/util/vsphere.go`:
- Around line 409-444: Update GetAllVSphereFailureDomains to capture the first
base64 decoding error encountered while processing credsMap instead of silently
continuing. After the gjson ForEach loop, return that error before the
no-failure-domains check, matching GetVSphereConnectionInfo’s fail-fast behavior
and preventing incomplete credentials from being returned.

---

Nitpick comments:
In `@test/extended-priv/mco_bootimages.go`:
- Around line 862-866: Remove the unused fdGroup type declaration near fdOrder,
leaving the fdByServer and fdOrder implementation unchanged.

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

Review profile: CHILL

Plan: Enterprise

Run ID: ac320457-3f72-45cc-89b8-a0ce03f1bb26

📥 Commits

Reviewing files that changed from the base of the PR and between ca627e9 and 7380358.

📒 Files selected for processing (5)

pkg/controller/bootimage/vsphere_create_template_test.go
pkg/controller/bootimage/vsphere_helpers.go
test/extended-priv/mco_bootimages.go
test/extended-priv/mco_scale.go
test/extended-priv/util/vsphere.go

jcpowermac · 2026-08-03T17:53:45Z

/test e2e-vsphere-mco-bootimages
/test e2e-vsphere-multi-vcenter-mco-bootimages
/test e2e-vsphere-zones-mco-bootimages

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 `@test/extended-priv/util/vsphere.go`:
- Around line 422-436: Update the credential resolution around
GetAllVSphereFailureDomains and its ForEach callback to decode credsMap once
before iterating failure domains, then use direct lookups for each domain’s
userKey and passKey. Explicitly detect either missing credential key and return
false with the existing error mechanism, while preserving decode-error handling
and assigning info.User and info.Password only from the resolved values.

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

Review profile: CHILL

Plan: Enterprise

Run ID: 2a3727a2-a681-4546-8e40-0742ca362831

📥 Commits

Reviewing files that changed from the base of the PR and between 7380358 and 394bb7b.

📒 Files selected for processing (6)

pkg/controller/bootimage/helpers.go
pkg/controller/bootimage/vsphere_create_template_test.go
pkg/controller/bootimage/vsphere_helpers.go
test/extended-priv/mco_bootimages.go
test/extended-priv/mco_scale.go
test/extended-priv/util/vsphere.go

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

pkg/controller/bootimage/vsphere_helpers.go
test/extended-priv/mco_scale.go
pkg/controller/bootimage/vsphere_create_template_test.go
test/extended-priv/mco_bootimages.go

jcpowermac · 2026-08-04T11:14:20Z

/test e2e-vsphere-multi-vcenter-mco-bootimages

jcpowermac · 2026-08-04T14:23:27Z

/test e2e-vsphere-multi-vcenter-mco-bootimages

jcpowermac · 2026-08-04T14:25:46Z

/test unit

jcpowermac · 2026-08-04T16:45:33Z

/test e2e-vsphere-multi-vcenter-mco-bootimages
/test unit

openshift-ci · 2026-08-04T19:05:58Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

jcpowermac · 2026-08-04T19:06:09Z

/test e2e-vsphere-multi-vcenter-mco-bootimages
/test unit

openshift-ci · 2026-08-04T19:46:41Z

@jcpowermac: 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/e2e-vsphere-zones-mco-bootimages	`394bb7b`	link	false	`/test e2e-vsphere-zones-mco-bootimages`

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.

jcpowermac · 2026-08-04T19:47:07Z

/test e2e-vsphere-multi-vcenter-mco-bootimages
/test unit

jcpowermac · 2026-08-04T20:20:51Z

/test unit

…mplates Bug: when a vSphere MachineSet's providerSpec.Template pointed at a non-standard (customer-managed) name, createNewVMTemplate treated any mismatch against the computed canonical name as drift and any RHCOS version mismatch as staleness, without checking whether the custom-named template already held the current release. This caused the controller to rename or rebuild valid, up-to-date custom templates on every reconcile, and rebuilding into the computed name could collide with a current template already sitting there under that name from before the custom name was adopted. createNewVMTemplate now calls the new isTemplateAtRelease helper to check the embedded RHCOS product version before acting: a custom name that already matches the target release is left alone, and an outdated custom template converges to the computed name only when the computed name doesn't already have its own current template (in which case the controller just switches back to it instead of rebuilding). Converging a custom-named template to the computed name during a rebuild requires swapping out a VM known under a name other than the one being created, so findAllRequiredResources and createNewVMTemplateWithNameForFailureDomain now take the already-resolved *object.VirtualMachine directly instead of re-resolving it by the (possibly different) target name. atomicTempName is exported as AtomicTempName so e2e coverage can compute the same "mco-old-<hash>" rollback name without duplicating the hashing logic. Also fixes an unrelated %wt -> %w format-verb typo in upgradeStubIgnitionIfRequired's error wrapping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Adds a govmomi simulator-backed unit test harness for the vSphere boot image path: pure-function coverage for the OVA/atomic-swap helpers, simulator-backed coverage for vSphere object lookups and createNewVMTemplate's decision tree (including the multi-vCenter and multi-failure-domain cases and the custom-template-preservation fix), TestReconcileVSphereProviderSpec, hot-loop-by-version coverage, and DownloadOva and CheckBootImagePlatform unit tests. Also adds e2e specs under test/extended-priv covering multi-vCenter and multi-failure-domain reconciliation and non-standard providerSpec.Template names, and supporting helpers for datacenter-aware template lookups and backdated image uploads scoped to a MachineSet's own workspace instead of failureDomains[0]. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…rage Vendors additional packages from the already-present github.com/vmware/govmomi module (no version bump): the simulator package (and its esx/vpx/internal subpackages) used to back the vSphere boot image unit tests with a fake vCenter, plus toolbox, units, vapi, and vapi/{simulator,vcenter,vm} needed transitively. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ator main (openshift#82723) Adds three optional/on-demand presubmit jobs (plain IPI, zonal single-vCenter, multi-vCenter) scoped to the MCO Bootimages longduration spec group, so the new vSphere multi-vCenter/multi-failure-domain reconciliation e2e specs in openshift/machine-config-operator#6354 can be exercised via /test before they land in a periodic. Reuses the existing openshift-e2e-vsphere and openshift-e2e-vsphere-zones workflows, differentiated only by MAX_VCENTERS per SPLAT-2238 (openshift#81215), instead of a separate hardcoded-pool multi-vcenter workflow. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

openshift-ci-robot · 2026-08-05T11:51:30Z

@jcpowermac: This pull request references Jira Issue OCPBUGS-104569, 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:

Will fill in this later

- What I did

- How to verify it

- Description for the changelog

Summary by CodeRabbit

Bug Fixes

vSphere credentials are now selected reliably for the configured server, improving multi-vCenter operation.

Boot image reconciliation preserves valid custom template names and independently handles multiple vCenters and failure domains.

Improved error reporting for failed ignition upgrades.

Tests

Expanded coverage for OVA caching, vSphere templates, resource discovery, networking, storage, secure boot, and platform support.

Added end-to-end validation for vSphere OVA imports, recovery scenarios, and multi-vCenter reconciliation.

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.

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

jcpowermac mentioned this pull request Jul 31, 2026

Add optional vSphere MCO bootimage presubmits for machine-config-oper… openshift/release#82723

Merged

coderabbitai Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread pkg/controller/bootimage/vsphere_helpers.go Outdated

Comment thread test/extended-priv/mco_bootimages.go

Comment thread test/extended-priv/mco_bootimages.go

Comment thread test/extended-priv/mco_bootimages.go

Comment thread test/extended-priv/util/vsphere.go

jcpowermac force-pushed the vsphere-ova-test-coverage branch from 7380358 to 394bb7b Compare August 3, 2026 17:47

coderabbitai Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread test/extended-priv/util/vsphere.go

jcpowermac force-pushed the vsphere-ova-test-coverage branch from 394bb7b to 05c2177 Compare August 3, 2026 20:34

jcpowermac force-pushed the vsphere-ova-test-coverage branch from 05c2177 to 7d804d9 Compare August 4, 2026 14:20

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2026

jcpowermac force-pushed the vsphere-ova-test-coverage branch from 7d804d9 to 0d7bb24 Compare August 4, 2026 16:45

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2026

jcpowermac force-pushed the vsphere-ova-test-coverage branch from 0d7bb24 to 85b9bb9 Compare August 4, 2026 19:05

jcpowermac force-pushed the vsphere-ova-test-coverage branch from 85b9bb9 to dd74bc9 Compare August 4, 2026 19:46

jcpowermac and others added 3 commits August 4, 2026 16:35

jcpowermac force-pushed the vsphere-ova-test-coverage branch from dd74bc9 to 1f57c13 Compare August 5, 2026 11:51

jcpowermac changed the title ~~Add VMware test coverage~~ OCPBUGS-104569: "vSphere boot image reconciler overwrites/renames current custom-named templates" Aug 5, 2026

openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Aug 5, 2026

Uh oh!

Conversation

jcpowermac commented Jul 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Jul 30, 2026

Uh oh!

openshift-ci Bot commented Jul 30, 2026

Uh oh!

jcpowermac commented Jul 30, 2026

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Pre-merge checks failed

❌ Failed checks (1 error, 1 warning)

Uh oh!

jcpowermac commented Jul 30, 2026

Uh oh!

jcpowermac commented Jul 30, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jcpowermac commented Aug 3, 2026

Uh oh!

jcpowermac commented Aug 3, 2026

Uh oh!

jcpowermac commented Aug 3, 2026

Uh oh!

jcpowermac commented Aug 3, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcpowermac commented Aug 3, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

openshift-ci Bot commented Aug 4, 2026

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

openshift-ci Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

jcpowermac commented Aug 4, 2026

Uh oh!

openshift-ci-robot commented Aug 5, 2026

Summary by CodeRabbit

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jcpowermac commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

openshift-ci Bot commented Aug 4, 2026 •

edited

Loading