From 48d01467e5dd7ef4e6b90d7487e31b4e3020a78f Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Wed, 3 Dec 2025 17:45:46 -0800 Subject: [PATCH 1/8] Enable vSphere in MAPI<->CAPI conversion framework This enables bidirectional conversion between vSphere Machine API Machines/MachineSets and Cluster API Machines/MachineTemplates, including comprehensive fuzz and unit tests. # Conflicts: # cmd/machine-api-migration/main.go --- cmd/machine-api-migration/main.go | 6 +- .../machineset_sync_controller.go | 26 + .../machinesync/machine_sync_controller.go | 18 + .../machine_sync_mapi2capi_infrastructure.go | 24 + pkg/conversion/capi2mapi/vsphere.go | 495 +++++++++++++++++ pkg/conversion/capi2mapi/vsphere_fuzz_test.go | 266 +++++++++ pkg/conversion/capi2mapi/vsphere_test.go | 400 ++++++++++++++ pkg/conversion/mapi2capi/util.go | 7 + pkg/conversion/mapi2capi/vsphere.go | 445 +++++++++++++++ pkg/conversion/mapi2capi/vsphere_fuzz_test.go | 192 +++++++ pkg/conversion/mapi2capi/vsphere_test.go | 514 ++++++++++++++++++ 11 files changed, 2391 insertions(+), 2 deletions(-) create mode 100644 pkg/conversion/capi2mapi/vsphere.go create mode 100644 pkg/conversion/capi2mapi/vsphere_fuzz_test.go create mode 100644 pkg/conversion/capi2mapi/vsphere_test.go create mode 100644 pkg/conversion/mapi2capi/vsphere.go create mode 100644 pkg/conversion/mapi2capi/vsphere_fuzz_test.go create mode 100644 pkg/conversion/mapi2capi/vsphere_test.go diff --git a/cmd/machine-api-migration/main.go b/cmd/machine-api-migration/main.go index c550ad43a..f66829407 100644 --- a/cmd/machine-api-migration/main.go +++ b/cmd/machine-api-migration/main.go @@ -30,6 +30,7 @@ import ( "k8s.io/utils/clock" awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" openstackv1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "github.com/openshift/api/features" @@ -66,6 +67,7 @@ func initScheme(scheme *runtime.Scheme) { utilruntime.Must(configv1.Install(scheme)) utilruntime.Must(awsv1.AddToScheme(scheme)) utilruntime.Must(openstackv1.AddToScheme(scheme)) + utilruntime.Must(vspherev1.AddToScheme(scheme)) utilruntime.Must(clusterv1.AddToScheme(scheme)) } @@ -151,8 +153,8 @@ func checkFeatureGates(ctx context.Context, log logr.Logger, mgr ctrl.Manager) e func checkPlatformSupported(ctx context.Context, log logr.Logger, platform configv1.PlatformType) { switch platform { - case configv1.AWSPlatformType, configv1.OpenStackPlatformType: - log.Info("starting controllers", "platform", platform) + case configv1.AWSPlatformType, configv1.OpenStackPlatformType, configv1.VSpherePlatformType: + klog.Infof("MachineAPIMigration: starting %s controllers", platform) default: log.Info("MachineAPIMigration not implemented for platform, nothing to do. Waiting for termination signal.", "platform", platform) diff --git a/pkg/controllers/machinesetsync/machineset_sync_controller.go b/pkg/controllers/machinesetsync/machineset_sync_controller.go index 97bc6185e..c87b1e024 100644 --- a/pkg/controllers/machinesetsync/machineset_sync_controller.go +++ b/pkg/controllers/machinesetsync/machineset_sync_controller.go @@ -47,6 +47,7 @@ import ( awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" ibmpowervsv1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2" openstackv1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/util/annotations" ctrl "sigs.k8s.io/controller-runtime" @@ -391,6 +392,12 @@ func filterOutdatedInfraMachineTemplates(infraMachineTemplateList client.ObjectL outdatedTemplates = append(outdatedTemplates, &template) } } + case *vspherev1.VSphereMachineTemplateList: + for _, template := range list.Items { + if template.GetName() != newInfraMachineTemplateName { + outdatedTemplates = append(outdatedTemplates, &template) + } + } default: return nil, fmt.Errorf("%w: got unknown type %T", errUnexpectedInfraMachineTemplateListType, list) } @@ -705,6 +712,20 @@ func (r *MachineSetSyncReconciler) convertCAPIToMAPIMachineSet(capiMachineSet *c return capi2mapi.FromMachineSetAndPowerVSMachineTemplateAndPowerVSCluster( //nolint: wrapcheck capiMachineSet, machineTemplate, cluster, ).ToMachineSet() + case configv1.VSpherePlatformType: + machineTemplate, ok := infraMachineTemplate.(*vspherev1.VSphereMachineTemplate) + if !ok { + return nil, nil, fmt.Errorf("%w, expected VSphereMachineTemplate, got %T", errUnexpectedInfraMachineTemplateType, infraMachineTemplate) + } + + cluster, ok := infraCluster.(*vspherev1.VSphereCluster) + if !ok { + return nil, nil, fmt.Errorf("%w, expected VSphereCluster, got %T", errUnexpectedInfraClusterType, infraCluster) + } + + return capi2mapi.FromMachineSetAndVSphereMachineTemplateAndVSphereCluster( //nolint: wrapcheck + capiMachineSet, machineTemplate, cluster, + ).ToMachineSet() default: return nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, r.Platform) } @@ -719,6 +740,8 @@ func (r *MachineSetSyncReconciler) convertMAPIToCAPIMachineSet(mapiMachineSet *m return mapi2capi.FromOpenStackMachineSetAndInfra(mapiMachineSet, r.Infra).ToMachineSetAndMachineTemplate() //nolint:wrapcheck case configv1.PowerVSPlatformType: return mapi2capi.FromPowerVSMachineSetAndInfra(mapiMachineSet, r.Infra).ToMachineSetAndMachineTemplate() //nolint:wrapcheck + case configv1.VSpherePlatformType: + return mapi2capi.FromVSphereMachineSetAndInfra(mapiMachineSet, r.Infra).ToMachineSetAndMachineTemplate() //nolint:wrapcheck default: return nil, nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, r.Platform) } @@ -1321,6 +1344,8 @@ func initInfraMachineTemplateListAndInfraClusterListFromProvider(platform config return &openstackv1.OpenStackMachineTemplateList{}, &openstackv1.OpenStackClusterList{}, nil case configv1.PowerVSPlatformType: return &ibmpowervsv1.IBMPowerVSMachineTemplateList{}, &ibmpowervsv1.IBMPowerVSClusterList{}, nil + case configv1.VSpherePlatformType: + return &vspherev1.VSphereMachineTemplateList{}, &vspherev1.VSphereClusterList{}, nil default: return nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform) } @@ -1334,6 +1359,7 @@ func compareCAPIInfraMachineTemplates(platform configv1.PlatformType, infraMachi case configv1.AWSPlatformType: case configv1.OpenStackPlatformType: case configv1.PowerVSPlatformType: + case configv1.VSpherePlatformType: default: return nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform) } diff --git a/pkg/controllers/machinesync/machine_sync_controller.go b/pkg/controllers/machinesync/machine_sync_controller.go index 292409e56..9573fbd18 100644 --- a/pkg/controllers/machinesync/machine_sync_controller.go +++ b/pkg/controllers/machinesync/machine_sync_controller.go @@ -46,6 +46,7 @@ import ( "k8s.io/utils/ptr" awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" openstackv1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/labels/format" @@ -100,6 +101,9 @@ var ( // errAssertingCAPIBMPowerVSMachine is returned when we encounter an issue asserting a client.Object into an IBMPowerVSMachine. errAssertingCAPIIBMPowerVSMachine = errors.New("error asserting the Cluster API IBMPowerVSMachine object") + // errAssertingCAPIVSphereMachine is returned when we encounter an issue asserting a client.Object into a VSphereMachine. + errAssertingCAPIVSphereMachine = errors.New("error asserting the Cluster API VSphereMachine object") + // errCAPIMachineNotFound is returned when the AuthoritativeAPI is set to CAPI on the MAPI machine, // but we can't find the CAPI machine. //lint:ignore ST1005 Cluster API is a name. @@ -610,6 +614,8 @@ func (r *MachineSyncReconciler) convertMAPIToCAPIMachine(mapiMachine *mapiv1beta return mapi2capi.FromOpenStackMachineAndInfra(mapiMachine, r.Infra).ToMachineAndInfrastructureMachine() //nolint:wrapcheck case configv1.PowerVSPlatformType: return mapi2capi.FromPowerVSMachineAndInfra(mapiMachine, r.Infra).ToMachineAndInfrastructureMachine() //nolint:wrapcheck + case configv1.VSpherePlatformType: + return mapi2capi.FromVSphereMachineAndInfra(mapiMachine, r.Infra).ToMachineAndInfrastructureMachine() //nolint:wrapcheck default: return nil, nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, r.Platform) } @@ -641,6 +647,18 @@ func (r *MachineSyncReconciler) convertCAPIToMAPIMachine(capiMachine *clusterv1. } return capi2mapi.FromMachineAndOpenStackMachineAndOpenStackCluster(capiMachine, openStackMachine, openStackCluster).ToMachine() //nolint:wrapcheck + case configv1.VSpherePlatformType: + vsphereMachine, ok := infraMachine.(*vspherev1.VSphereMachine) + if !ok { + return nil, nil, fmt.Errorf("%w, expected VSphereMachine, got %T", errUnexpectedInfraMachineType, infraMachine) + } + + vsphereCluster, ok := infraCluster.(*vspherev1.VSphereCluster) + if !ok { + return nil, nil, fmt.Errorf("%w, expected VSphereCluster, got %T", errUnexpectedInfraClusterType, infraCluster) + } + + return capi2mapi.FromMachineAndVSphereMachineAndVSphereCluster(capiMachine, vsphereMachine, vsphereCluster).ToMachine() //nolint:wrapcheck default: return nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, r.Platform) } diff --git a/pkg/controllers/machinesync/machine_sync_mapi2capi_infrastructure.go b/pkg/controllers/machinesync/machine_sync_mapi2capi_infrastructure.go index bda95f00d..5485d7d7f 100644 --- a/pkg/controllers/machinesync/machine_sync_mapi2capi_infrastructure.go +++ b/pkg/controllers/machinesync/machine_sync_mapi2capi_infrastructure.go @@ -30,6 +30,7 @@ import ( awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" ibmpowervsv1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2" openstackv1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -255,6 +256,7 @@ func compareCAPIInfraMachines(platform configv1.PlatformType, infraMachine1, inf case configv1.AWSPlatformType: case configv1.OpenStackPlatformType: case configv1.PowerVSPlatformType: + case configv1.VSpherePlatformType: default: return nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform) } @@ -273,6 +275,8 @@ func compareCAPIInfraMachines(platform configv1.PlatformType, infraMachine1, inf } // setChangedCAPIInfraMachineStatusFields sets the updated fields in the Cluster API Infrastructure machine status. +// +//nolint:funlen func setChangedCAPIInfraMachineStatusFields(platform configv1.PlatformType, existingCAPIInfraMachine, convertedCAPIInfraMachine client.Object) error { switch platform { case configv1.AWSPlatformType: @@ -332,6 +336,26 @@ func setChangedCAPIInfraMachineStatusFields(platform configv1.PlatformType, exis // Finally overwrite the entire existing status with the convertedCAPIMachine status. existing.Status = converted.Status + return nil + case configv1.VSpherePlatformType: + existing, ok := existingCAPIInfraMachine.(*vspherev1.VSphereMachine) + if !ok { + return errAssertingCAPIVSphereMachine + } + + converted, ok := convertedCAPIInfraMachine.(*vspherev1.VSphereMachine) + if !ok { + return errAssertingCAPIVSphereMachine + } + + util.EnsureCAPIV1Beta1Conditions(existing, converted) + + // Merge the v1beta2 conditions. + util.EnsureCAPIV1Beta2Conditions(existing, converted) + + // Finally overwrite the entire existing status with the convertedCAPIMachine status. + existing.Status = converted.Status + return nil default: return fmt.Errorf("%w: %s", errPlatformNotSupported, platform) diff --git a/pkg/conversion/capi2mapi/vsphere.go b/pkg/conversion/capi2mapi/vsphere.go new file mode 100644 index 000000000..e52db5d82 --- /dev/null +++ b/pkg/conversion/capi2mapi/vsphere.go @@ -0,0 +1,495 @@ +/* +Copyright 2024 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package capi2mapi + +import ( + "errors" + "fmt" + "reflect" + + mapiv1beta1 "github.com/openshift/api/machine/v1beta1" + "github.com/openshift/cluster-capi-operator/pkg/conversion/consts" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" +) + +var ( + errCAPIMachineVSphereMachineVSphereClusterCannotBeNil = errors.New("provided Machine, VSphereMachine and VSphereCluster can not be nil") + errCAPIMachineSetVSphereMachineTemplateVSphereClusterCannotBeNil = errors.New("provided MachineSet, VSphereMachineTemplate and VSphereCluster can not be nil") +) + +const ( + errUnsupportedCAPVProvisioningMode = "unable to convert provisioning mode, unknown value" + errUnsupportedCAPVCloneMode = "unable to convert clone mode, unknown value" + vsphereCredentialsSecretName = "vsphere-cloud-credentials" //#nosec G101 -- False positive, not actually a credential. + +) + +// machineAndVSphereMachineAndVSphereCluster stores the details of a Cluster API Machine and VSphereMachine and VSphereCluster. +type machineAndVSphereMachineAndVSphereCluster struct { + machine *clusterv1.Machine + vSphereMachine *vspherev1.VSphereMachine + vSphereCluster *vspherev1.VSphereCluster + excludeMachineAPILabelsAndAnnotations bool +} + +// machineSetAndVSphereMachineTemplateAndVSphereCluster stores the details of a Cluster API MachineSet and VSphereMachineTemplate and VSphereCluster. +type machineSetAndVSphereMachineTemplateAndVSphereCluster struct { + machineSet *clusterv1.MachineSet + vSphereMachineTemplate *vspherev1.VSphereMachineTemplate + vSphereCluster *vspherev1.VSphereCluster + *machineAndVSphereMachineAndVSphereCluster +} + +// FromMachineAndVSphereMachineAndVSphereCluster wraps a CAPI Machine and CAPV VSphereMachine and CAPV VSphereCluster into a capi2mapi MachineAndInfrastructureMachine. +func FromMachineAndVSphereMachineAndVSphereCluster(m *clusterv1.Machine, vm *vspherev1.VSphereMachine, vc *vspherev1.VSphereCluster) MachineAndInfrastructureMachine { + return &machineAndVSphereMachineAndVSphereCluster{machine: m, vSphereMachine: vm, vSphereCluster: vc} +} + +// FromMachineSetAndVSphereMachineTemplateAndVSphereCluster wraps a CAPI MachineSet and CAPV VSphereMachineTemplate and CAPV VSphereCluster into a capi2mapi MachineSetAndMachineTemplate. +func FromMachineSetAndVSphereMachineTemplateAndVSphereCluster(ms *clusterv1.MachineSet, vmt *vspherev1.VSphereMachineTemplate, vc *vspherev1.VSphereCluster) MachineSetAndMachineTemplate { + return &machineSetAndVSphereMachineTemplateAndVSphereCluster{ + machineSet: ms, + vSphereMachineTemplate: vmt, + vSphereCluster: vc, + machineAndVSphereMachineAndVSphereCluster: &machineAndVSphereMachineAndVSphereCluster{ + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: ms.Spec.Template.ObjectMeta.Labels, //nolint:staticcheck // ObjectMeta is a field, not embedded + Annotations: ms.Spec.Template.ObjectMeta.Annotations, //nolint:staticcheck // ObjectMeta is a field, not embedded + }, + Spec: ms.Spec.Template.Spec, + }, + vSphereMachine: &vspherev1.VSphereMachine{ + Spec: vmt.Spec.Template.Spec, + }, + vSphereCluster: vc, + excludeMachineAPILabelsAndAnnotations: true, + }, + } +} + +// ToMachine converts a capi2mapi MachineAndVSphereMachineTemplate into a MAPI Machine. +func (m machineAndVSphereMachineAndVSphereCluster) ToMachine() (*mapiv1beta1.Machine, []string, error) { + if m.machine == nil || m.vSphereMachine == nil || m.vSphereCluster == nil { + return nil, nil, errCAPIMachineVSphereMachineVSphereClusterCannotBeNil + } + + var errs field.ErrorList + + mapaSpec, warning, err := m.toProviderSpec() + if err != nil { + errs = append(errs, err...) + } + + vsphereSpecRawExt, errRaw := RawExtensionFromInterface(mapaSpec) + if errRaw != nil { + return nil, nil, fmt.Errorf("unable to convert vSphere providerSpec to raw extension: %w", errRaw) + } + + additionalMachineAPIMetadataLabels, additionalMachineAPIMetadataAnnotations := m.buildAdditionalMetadata() + + mapiMachine, err := fromCAPIMachineToMAPIMachine(m.machine, additionalMachineAPIMetadataLabels, additionalMachineAPIMetadataAnnotations) + + if err != nil { + errs = append(errs, err...) + } + + mapiMachine.Spec.ProviderSpec.Value = vsphereSpecRawExt + // Note: ProviderStatus is not set during conversion, similar to OpenStack and PowerVS providers. + // During the migration period when MAPI is authoritative, the MAPI controller manages the status. + // When CAPI becomes authoritative, the ProviderStatus will remain stale but unused, as status + // will be tracked in the CAPI VSphereMachine status instead. + + if len(errs) > 0 { + return nil, warning, errs.ToAggregate() + } + + return mapiMachine, warning, nil +} + +// ToMachineSet converts a capi2mapi MachineSetAndVSphereMachineTemplate into a MAPI MachineSet. +func (m machineSetAndVSphereMachineTemplateAndVSphereCluster) ToMachineSet() (*mapiv1beta1.MachineSet, []string, error) { + if m.machineSet == nil || m.vSphereMachineTemplate == nil || m.vSphereCluster == nil || m.machineAndVSphereMachineAndVSphereCluster == nil { + return nil, nil, errCAPIMachineSetVSphereMachineTemplateVSphereClusterCannotBeNil + } + + var errs []error + + // Run the full ToMachine conversion so that we can check for + // any Machine level conversion errs in the spec translation. + mapiMachine, warn, err := m.ToMachine() + if err != nil { + errs = append(errs, err) + } + + warnings := make([]string, 0, len(warn)) + warnings = append(warnings, warn...) + + mapiMachineSet, err := fromCAPIMachineSetToMAPIMachineSet(m.machineSet) + if err != nil { + errs = append(errs, err) + } + + if len(errs) > 0 { + return nil, warnings, utilerrors.NewAggregate(errs) + } + + mapiMachineSet.Spec.Template.Spec = mapiMachine.Spec + + // Copy the labels and annotations from the Machine to the template. + // Note: The fuzzer ensures template.spec.metadata and template.metadata have the same labels/annotations + // because CAPI only has one metadata location (template.metadata), and during roundtrip conversion + // both MAPI locations must match to preserve the original values. + mapiMachineSet.Spec.Template.ObjectMeta.Annotations = mapiMachine.ObjectMeta.Annotations //nolint:staticcheck // ObjectMeta is a field, not embedded + mapiMachineSet.Spec.Template.ObjectMeta.Labels = mapiMachine.ObjectMeta.Labels //nolint:staticcheck // ObjectMeta is a field, not embedded + + return mapiMachineSet, warnings, nil +} + +// toProviderSpec converts a capi2mapi MachineAndVSphereMachineTemplateAndVSphereCluster into a MAPI VSphereMachineProviderSpec. +// +//nolint:funlen +func (m machineAndVSphereMachineAndVSphereCluster) toProviderSpec() (*mapiv1beta1.VSphereMachineProviderSpec, []string, field.ErrorList) { + var errs field.ErrorList + + fldPath := field.NewPath("spec") + + // Convert clone mode + mapiCloneMode, err := convertCAPVCloneModeToMAPI(fldPath.Child("cloneMode"), m.vSphereMachine.Spec.CloneMode) + if err != nil { + errs = append(errs, err) + } + + // Convert network configuration + mapiNetworkSpec, networkWarnings, networkErrors := convertCAPVNetworkSpecToMAPI(fldPath.Child("network"), m.vSphereMachine.Spec.Network) + if len(networkErrors) > 0 { + errs = append(errs, networkErrors...) + } + + // Convert data disks + // AdditionalDisksGiB is deprecated in CAPV in favor of DataDisks. + // If AdditionalDisksGiB is set, convert it to DataDisks format first. + allDataDisks := m.vSphereMachine.Spec.DataDisks + if len(m.vSphereMachine.Spec.AdditionalDisksGiB) > 0 { + // Convert AdditionalDisksGiB to VSphereDisk format and append to existing DataDisks + for i, sizeGiB := range m.vSphereMachine.Spec.AdditionalDisksGiB { + allDataDisks = append(allDataDisks, vspherev1.VSphereDisk{ + Name: fmt.Sprintf("additional-disk-%d", i), + SizeGiB: sizeGiB, + }) + } + } + + mapiDataDisks, diskWarnings, diskErrors := convertCAPVDataDisksToMAPI(fldPath.Child("dataDisks"), allDataDisks) + if len(diskErrors) > 0 { + errs = append(errs, diskErrors...) + } + + warnings := make([]string, 0, len(networkWarnings)+len(diskWarnings)) + warnings = append(warnings, networkWarnings...) + warnings = append(warnings, diskWarnings...) + + mapiProviderConfig := mapiv1beta1.VSphereMachineProviderSpec{ + TypeMeta: metav1.TypeMeta{ + Kind: "VSphereMachineProviderSpec", + APIVersion: "machine.openshift.io/v1beta1", + }, + Template: m.vSphereMachine.Spec.Template, + NumCPUs: m.vSphereMachine.Spec.NumCPUs, + NumCoresPerSocket: m.vSphereMachine.Spec.NumCoresPerSocket, + MemoryMiB: m.vSphereMachine.Spec.MemoryMiB, + DiskGiB: m.vSphereMachine.Spec.DiskGiB, + TagIDs: m.vSphereMachine.Spec.TagIDs, + Snapshot: m.vSphereMachine.Spec.Snapshot, + CloneMode: mapiCloneMode, + Network: mapiNetworkSpec, + DataDisks: mapiDataDisks, + } + + // Create workspace from the CAPV spec fields + workspace := &mapiv1beta1.Workspace{ + Server: m.vSphereMachine.Spec.Server, + Datacenter: m.vSphereMachine.Spec.Datacenter, + Folder: m.vSphereMachine.Spec.Folder, + Datastore: m.vSphereMachine.Spec.Datastore, + ResourcePool: m.vSphereMachine.Spec.ResourcePool, + } + + userDataSecretName := ptr.Deref(m.machine.Spec.Bootstrap.DataSecretName, "") + if userDataSecretName != "" { + mapiProviderConfig.UserDataSecret = &corev1.LocalObjectReference{ + Name: userDataSecretName, + } + } + + mapiProviderConfig.CredentialsSecret = &corev1.LocalObjectReference{ + Name: vsphereCredentialsSecretName, + } + + // Only set workspace if any field is set + if workspace.Server != "" || workspace.Datacenter != "" || workspace.Folder != "" || + workspace.Datastore != "" || workspace.ResourcePool != "" { + mapiProviderConfig.Workspace = workspace + } + + // Below this line are fields not converted from the CAPI VSphereMachine. + + // ProviderID - Populated at a different level. + // FailureDomain - Handled via zone label in buildAdditionalMetadata. + // PowerOffMode - Controls VM power operations (hard vs soft power off), not part of machine provisioning spec. + // GuestSoftPowerOffTimeout - Timeout for soft power operations, not part of machine provisioning spec. + // NamingStrategy - Controls VM naming behavior, not part of machine provisioning spec. + // Thumbprint - Ignore - Not required, TLS validation handled by cluster-level configuration. + // StoragePolicyName - Ignore - Not supported in MAPI VSphereMachineProviderSpec. + // Resources - Ignore - CPU/memory reservations, limits, and shares not supported in MAPI. + // AdditionalDisksGiB - Deprecated in CAPV, converted to DataDisks above. + // CustomVMXKeys - Ignore - Not supported in MAPI VSphereMachineProviderSpec. + // PciDevices - Ignore - Not supported in MAPI VSphereMachineProviderSpec. + // OS - Ignore - Not supported in MAPI VSphereMachineProviderSpec. + // HardwareVersion - Ignore - Not supported in MAPI VSphereMachineProviderSpec. + + // There are quite a few unsupported fields, so break them out for now. + errs = append(errs, handleUnsupportedVSphereFields(fldPath, m.vSphereMachine.Spec)...) + + if len(errs) > 0 { + return nil, warnings, errs + } + + return &mapiProviderConfig, warnings, nil +} + +// handleUnsupportedVSphereFields checks for CAPV fields that are not supported in MAPI and returns a list of errors. +func handleUnsupportedVSphereFields(fldPath *field.Path, spec vspherev1.VSphereMachineSpec) field.ErrorList { + errs := field.ErrorList{} + + if spec.Thumbprint != "" { + // TLS validation is handled at the cluster level, not per-machine. + errs = append(errs, field.Invalid(fldPath.Child("thumbprint"), spec.Thumbprint, "thumbprint is not supported")) + } + + if spec.StoragePolicyName != "" { + // Not required for our use case. + errs = append(errs, field.Invalid(fldPath.Child("storagePolicyName"), spec.StoragePolicyName, "storagePolicyName is not supported")) + } + + if !reflect.DeepEqual(spec.Resources, vspherev1.VirtualMachineResources{}) { + // CPU/memory reservations, limits, and shares are not supported in MAPI. + errs = append(errs, field.Invalid(fldPath.Child("resources"), spec.Resources, "resources are not supported")) + } + + // AdditionalDisksGiB is deprecated in CAPV in favor of DataDisks. + // This field is handled during conversion by merging with DataDisks. + + if len(spec.CustomVMXKeys) > 0 { + // Not required for our use case. + errs = append(errs, field.Invalid(fldPath.Child("customVMXKeys"), spec.CustomVMXKeys, "customVMXKeys are not supported")) + } + + if len(spec.PciDevices) > 0 { + // Not required for our use case. + errs = append(errs, field.Invalid(fldPath.Child("pciDevices"), spec.PciDevices, "pciDevices are not supported")) + } + + if spec.OS != "" { + // Not required for our use case. + errs = append(errs, field.Invalid(fldPath.Child("os"), spec.OS, "os is not supported")) + } + + if spec.HardwareVersion != "" { + // Not required for our use case. + errs = append(errs, field.Invalid(fldPath.Child("hardwareVersion"), spec.HardwareVersion, "hardwareVersion is not supported")) + } + + if spec.PowerOffMode != "" && spec.PowerOffMode != vspherev1.VirtualMachinePowerOpModeHard { + // We always use hard power off mode. Other modes are not supported in MAPI. + errs = append(errs, field.Invalid(fldPath.Child("powerOffMode"), spec.PowerOffMode, "powerOffMode must be hard or unset")) + } + + if spec.GuestSoftPowerOffTimeout != nil { + // This is only used with soft power off modes, which we don't support. + errs = append(errs, field.Invalid(fldPath.Child("guestSoftPowerOffTimeout"), spec.GuestSoftPowerOffTimeout, "guestSoftPowerOffTimeout is not supported")) + } + + if spec.NamingStrategy != nil { + // Not required for our use case. + errs = append(errs, field.Invalid(fldPath.Child("namingStrategy"), spec.NamingStrategy, "namingStrategy is not supported")) + } + + return errs +} + +//////// Conversion helpers + +// buildAdditionalMetadata constructs the additional labels and annotations for the MAPI machine. +func (m machineAndVSphereMachineAndVSphereCluster) buildAdditionalMetadata() (map[string]string, map[string]string) { + var additionalMachineAPIMetadataLabels, additionalMachineAPIMetadataAnnotations map[string]string + + // vSphere MUST set the zone label when FailureDomain is present because it's not stored in the ProviderSpec + // (unlike AWS which has Placement.AvailabilityZone). The zone label is the only place to preserve + // the FailureDomain during roundtrip conversion. + if m.machine.Spec.FailureDomain != "" { + additionalMachineAPIMetadataLabels = map[string]string{ + consts.MAPIMachineMetadataLabelZone: m.machine.Spec.FailureDomain, + } + } + + if !m.excludeMachineAPILabelsAndAnnotations { + if additionalMachineAPIMetadataLabels == nil { + additionalMachineAPIMetadataLabels = make(map[string]string) + } + // For vSphere, we use template name as instance type and datacenter as region + additionalMachineAPIMetadataLabels[consts.MAPIMachineMetadataLabelInstanceType] = m.vSphereMachine.Spec.Template + additionalMachineAPIMetadataLabels[consts.MAPIMachineMetadataLabelRegion] = m.vSphereMachine.Spec.Datacenter + + // Get instance state from VM status - use empty string if VM is not yet provisioned + instanceState := m.getInstanceState() + + additionalMachineAPIMetadataAnnotations = map[string]string{ + consts.MAPIMachineMetadataAnnotationInstanceState: instanceState, + } + } + + return additionalMachineAPIMetadataLabels, additionalMachineAPIMetadataAnnotations +} + +// getInstanceState determines the instance state from the VSphereMachine status. +// Returns empty string if VM is not yet provisioned, "ready" if provisioned and ready, +// or "not-ready" if provisioned but not ready. +// This matches behavior of other providers (AWS, OpenStack, PowerVS). +func (m machineAndVSphereMachineAndVSphereCluster) getInstanceState() string { + // We check if addresses are set to determine if the VM has been provisioned + if len(m.vSphereMachine.Status.Addresses) == 0 { + return "" + } + + if m.vSphereMachine.Status.Ready { + return "ready" + } + + return "not-ready" +} + +// convertCAPVCloneModeToMAPI converts CAPV CloneMode to MAPI CloneMode. +func convertCAPVCloneModeToMAPI(fldPath *field.Path, capvMode vspherev1.CloneMode) (mapiv1beta1.CloneMode, *field.Error) { + switch capvMode { + case vspherev1.FullClone: + return mapiv1beta1.FullClone, nil + case vspherev1.LinkedClone: + return mapiv1beta1.LinkedClone, nil + case "": + return "", nil + default: + return "", field.Invalid(fldPath, capvMode, errUnsupportedCAPVCloneMode) + } +} + +// convertCAPVNetworkSpecToMAPI converts CAPV NetworkSpec to MAPI NetworkSpec. +// +//nolint:unparam +func convertCAPVNetworkSpecToMAPI(_ *field.Path, capvNetwork vspherev1.NetworkSpec) (mapiv1beta1.NetworkSpec, []string, field.ErrorList) { + var ( + errs field.ErrorList + warnings []string + ) + + // Return nil devices slice if empty to match MAPI's JSON marshaling behavior + // (produces "devices": null instead of "devices": []) + var devices []mapiv1beta1.NetworkDeviceSpec + if len(capvNetwork.Devices) > 0 { + devices = make([]mapiv1beta1.NetworkDeviceSpec, len(capvNetwork.Devices)) + for i, device := range capvNetwork.Devices { + devices[i] = mapiv1beta1.NetworkDeviceSpec{ + NetworkName: device.NetworkName, + Gateway: device.Gateway4, // Map IPv4 gateway + IPAddrs: device.IPAddrs, + Nameservers: device.Nameservers, + } + + // Convert AddressesFromPools + if len(device.AddressesFromPools) > 0 { + addressesFromPools := make([]mapiv1beta1.AddressesFromPool, len(device.AddressesFromPools)) + for j, pool := range device.AddressesFromPools { + addressesFromPools[j] = mapiv1beta1.AddressesFromPool{ + Group: ptr.Deref(pool.APIGroup, ""), + Resource: pool.Kind, // This might need adjustment based on actual mapping + Name: pool.Name, + } + } + + devices[i].AddressesFromPools = addressesFromPools + } + + // Note: DHCP settings are not directly represented in MAPI NetworkDeviceSpec + // The presence of DHCP4/DHCP6 in CAPV is inferred from the absence of static IPs + if device.DHCP4 || device.DHCP6 { + if len(device.IPAddrs) > 0 { + warnings = append(warnings, fmt.Sprintf("device %d has both DHCP and static IPs configured, MAPI will use static IPs", i)) + } + } + } + } + + return mapiv1beta1.NetworkSpec{ + Devices: devices, + }, warnings, errs +} + +// convertCAPVDataDisksToMAPI converts CAPV DataDisks to MAPI DataDisks. +// +//nolint:unparam +func convertCAPVDataDisksToMAPI(fldPath *field.Path, capvDisks []vspherev1.VSphereDisk) ([]mapiv1beta1.VSphereDisk, []string, field.ErrorList) { + var ( + errs field.ErrorList + warnings []string + ) + + // Return nil disks slice if empty to match MAPI's JSON marshaling behavior + // (produces "dataDisks": null instead of "dataDisks": []) + if len(capvDisks) == 0 { + return nil, warnings, errs + } + + mapiDisks := make([]mapiv1beta1.VSphereDisk, len(capvDisks)) + for i, disk := range capvDisks { + mapiDisks[i] = mapiv1beta1.VSphereDisk{ + Name: disk.Name, + SizeGiB: disk.SizeGiB, + } + + // Convert provisioning mode + switch disk.ProvisioningMode { + case vspherev1.ThinProvisioningMode: + mapiDisks[i].ProvisioningMode = mapiv1beta1.ProvisioningModeThin + case vspherev1.ThickProvisioningMode: + mapiDisks[i].ProvisioningMode = mapiv1beta1.ProvisioningModeThick + case vspherev1.EagerlyZeroedProvisioningMode: + mapiDisks[i].ProvisioningMode = mapiv1beta1.ProvisioningModeEagerlyZeroed + case "": + // Default - no setting + default: + errs = append(errs, field.Invalid(fldPath.Index(i).Child("provisioningMode"), disk.ProvisioningMode, errUnsupportedCAPVProvisioningMode)) + } + } + + return mapiDisks, warnings, errs +} diff --git a/pkg/conversion/capi2mapi/vsphere_fuzz_test.go b/pkg/conversion/capi2mapi/vsphere_fuzz_test.go new file mode 100644 index 000000000..6beec913f --- /dev/null +++ b/pkg/conversion/capi2mapi/vsphere_fuzz_test.go @@ -0,0 +1,266 @@ +/* +Copyright 2025 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package capi2mapi_test + +import ( + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + randfill "sigs.k8s.io/randfill" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/cluster-capi-operator/pkg/conversion/capi2mapi" + "github.com/openshift/cluster-capi-operator/pkg/conversion/mapi2capi" + conversiontest "github.com/openshift/cluster-capi-operator/pkg/conversion/test/fuzz" + + runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + vsphereMachineKind = "VSphereMachine" + vsphereTemplateKind = "VSphereMachineTemplate" +) + +var _ = Describe("vSphere Fuzz (capi2mapi)", func() { + infra := &configv1.Infrastructure{ + Spec: configv1.InfrastructureSpec{}, + Status: configv1.InfrastructureStatus{ + InfrastructureName: "sample-cluster-name", + }, + } + + infraCluster := &vspherev1.VSphereCluster{ + Spec: vspherev1.VSphereClusterSpec{ + Server: "vcenter.example.com", + }, + } + + Context("VSphereMachine Conversion", func() { + fromMachineAndVSphereMachineAndVSphereCluster := func(machine *clusterv1.Machine, infraMachine client.Object, infraCluster client.Object) capi2mapi.MachineAndInfrastructureMachine { + vsphereMachine, ok := infraMachine.(*vspherev1.VSphereMachine) + Expect(ok).To(BeTrue(), "input infra machine should be of type %T, got %T", &vspherev1.VSphereMachine{}, infraMachine) + + vsphereCluster, ok := infraCluster.(*vspherev1.VSphereCluster) + Expect(ok).To(BeTrue(), "input infra cluster should be of type %T, got %T", &vspherev1.VSphereCluster{}, infraCluster) + + return capi2mapi.FromMachineAndVSphereMachineAndVSphereCluster(machine, vsphereMachine, vsphereCluster) + } + + conversiontest.CAPI2MAPIMachineRoundTripFuzzTest( + scheme, + infra, + infraCluster, + &vspherev1.VSphereMachine{}, + mapi2capi.FromVSphereMachineAndInfra, + fromMachineAndVSphereMachineAndVSphereCluster, + conversiontest.ObjectMetaFuzzerFuncs(capiNamespace), + conversiontest.CAPIMachineFuzzerFuncs(vsphereProviderIDFuzzer, vsphereMachineKind, vspherev1.GroupVersion.Group, infra.Status.InfrastructureName), + vsphereMachineFuzzerFuncs, + ) + }) + + Context("VSphereMachineSet Conversion", func() { + fromMachineSetAndVSphereMachineTemplateAndVSphereCluster := func(machineSet *clusterv1.MachineSet, infraMachineTemplate client.Object, infraCluster client.Object) capi2mapi.MachineSetAndMachineTemplate { + vsphereMachineTemplate, ok := infraMachineTemplate.(*vspherev1.VSphereMachineTemplate) + Expect(ok).To(BeTrue(), "input infra machine template should be of type %T, got %T", &vspherev1.VSphereMachineTemplate{}, infraMachineTemplate) + + vsphereCluster, ok := infraCluster.(*vspherev1.VSphereCluster) + Expect(ok).To(BeTrue(), "input infra cluster should be of type %T, got %T", &vspherev1.VSphereCluster{}, infraCluster) + + return capi2mapi.FromMachineSetAndVSphereMachineTemplateAndVSphereCluster(machineSet, vsphereMachineTemplate, vsphereCluster) + } + + conversiontest.CAPI2MAPIMachineSetRoundTripFuzzTest( + scheme, + infra, + infraCluster, + &vspherev1.VSphereMachineTemplate{}, + mapi2capi.FromVSphereMachineSetAndInfra, + fromMachineSetAndVSphereMachineTemplateAndVSphereCluster, + conversiontest.ObjectMetaFuzzerFuncs(capiNamespace), + conversiontest.CAPIMachineFuzzerFuncs(vsphereProviderIDFuzzer, vsphereTemplateKind, vspherev1.GroupVersion.Group, infra.Status.InfrastructureName), + conversiontest.CAPIMachineSetFuzzerFuncs(vsphereTemplateKind, vspherev1.GroupVersion.Group, infra.Status.InfrastructureName), + vsphereMachineFuzzerFuncs, + vsphereMachineTemplateFuzzerFuncs, + ) + }) +}) + +func vsphereProviderIDFuzzer(c randfill.Continue) string { + return "vsphere://" + strings.ReplaceAll(c.String(0), "/", "") +} + +func vsphereMachineFuzzerFuncs(codecs runtimeserializer.CodecFactory) []interface{} { + return []interface{}{ + func(m *vspherev1.VSphereMachine, c randfill.Continue) { + c.FillNoCustom(m) + + // Ensure the type meta is set correctly. + m.TypeMeta.APIVersion = vspherev1.GroupVersion.String() //nolint:staticcheck // TypeMeta is embedded but we set it explicitly here + m.TypeMeta.Kind = vsphereMachineKind //nolint:staticcheck // TypeMeta is embedded but we set it explicitly here + }, + func(cloneMode *vspherev1.CloneMode, c randfill.Continue) { + switch c.Int31n(3) { + case 0: + *cloneMode = vspherev1.FullClone + case 1: + *cloneMode = vspherev1.LinkedClone + case 2: + *cloneMode = "" + } + }, + func(provisioningMode *vspherev1.ProvisioningMode, c randfill.Continue) { + switch c.Int31n(4) { + case 0: + *provisioningMode = vspherev1.ThinProvisioningMode + case 1: + *provisioningMode = vspherev1.ThickProvisioningMode + case 2: + *provisioningMode = vspherev1.EagerlyZeroedProvisioningMode + case 3: + *provisioningMode = "" + } + }, + func(spec *vspherev1.VSphereMachineSpec, c randfill.Continue) { + c.FillNoCustom(spec) + + // Ensure required fields are set + if spec.Template == "" { + spec.Template = "test-template" + } + + if spec.Server == "" { + spec.Server = "vcenter.example.com" + } + + if spec.Datacenter == "" { + spec.Datacenter = "test-datacenter" + } + + // Fields not supported in MAPI conversion - clear them + spec.PowerOffMode = "" + spec.GuestSoftPowerOffTimeout = nil + spec.FailureDomain = nil + spec.NamingStrategy = nil + spec.OS = "" + spec.HardwareVersion = "" + spec.StoragePolicyName = "" + spec.Thumbprint = "" + spec.Resources = vspherev1.VirtualMachineResources{} + spec.PciDevices = nil + spec.CustomVMXKeys = nil + spec.AdditionalDisksGiB = nil + spec.ProviderID = nil + + // Simplify network spec for compatibility + for i := range spec.Network.Devices { + // Clear fields not directly supported in MAPI + spec.Network.Devices[i].MACAddr = "" + spec.Network.Devices[i].MTU = nil + spec.Network.Devices[i].Gateway6 = "" + spec.Network.Devices[i].Routes = nil + spec.Network.Devices[i].SearchDomains = nil + spec.Network.Devices[i].DeviceName = "" + spec.Network.Devices[i].DHCP4 = false + spec.Network.Devices[i].DHCP6 = false + spec.Network.Devices[i].SkipIPAllocation = false + + // Clear AddressesFromPools API group as MAPI uses empty string + for j := range spec.Network.Devices[i].AddressesFromPools { + spec.Network.Devices[i].AddressesFromPools[j].APIGroup = nil + } + } + + // Clear network preferences field + spec.Network.PreferredAPIServerCIDR = "" + }, + func(status *vspherev1.VSphereMachineStatus, c randfill.Continue) { + c.FillNoCustom(status) + + // Clear v1beta2 conditions and other fields not needed in conversion + status.V1Beta2 = nil + status.FailureReason = nil + status.FailureMessage = nil + }, + } +} + +func vsphereMachineTemplateFuzzerFuncs(codecs runtimeserializer.CodecFactory) []interface{} { + return []interface{}{ + func(m *vspherev1.VSphereMachineTemplate, c randfill.Continue) { + c.FillNoCustom(m) + + // Ensure the type meta is set correctly. + m.TypeMeta.APIVersion = vspherev1.GroupVersion.String() //nolint:staticcheck // TypeMeta is embedded but we set it explicitly here + m.TypeMeta.Kind = vsphereTemplateKind //nolint:staticcheck // TypeMeta is embedded but we set it explicitly here + }, + func(spec *vspherev1.VSphereMachineTemplateSpec, c randfill.Continue) { + c.FillNoCustom(spec) + + // Apply same constraints as VSphereMachineSpec + if spec.Template.Spec.Template == "" { + spec.Template.Spec.Template = "test-template" + } + + if spec.Template.Spec.Server == "" { + spec.Template.Spec.Server = "vcenter.example.com" + } + + if spec.Template.Spec.Datacenter == "" { + spec.Template.Spec.Datacenter = "test-datacenter" + } + + // Fields not supported in MAPI conversion + spec.Template.Spec.PowerOffMode = "" + spec.Template.Spec.GuestSoftPowerOffTimeout = nil + spec.Template.Spec.FailureDomain = nil + spec.Template.Spec.NamingStrategy = nil + spec.Template.Spec.OS = "" + spec.Template.Spec.HardwareVersion = "" + spec.Template.Spec.StoragePolicyName = "" + spec.Template.Spec.Thumbprint = "" + spec.Template.Spec.Resources = vspherev1.VirtualMachineResources{} + spec.Template.Spec.PciDevices = nil + spec.Template.Spec.CustomVMXKeys = nil + spec.Template.Spec.AdditionalDisksGiB = nil + spec.Template.Spec.ProviderID = nil + + // Simplify network spec + for i := range spec.Template.Spec.Network.Devices { + spec.Template.Spec.Network.Devices[i].MACAddr = "" + spec.Template.Spec.Network.Devices[i].MTU = nil + spec.Template.Spec.Network.Devices[i].Gateway6 = "" + spec.Template.Spec.Network.Devices[i].Routes = nil + spec.Template.Spec.Network.Devices[i].SearchDomains = nil + spec.Template.Spec.Network.Devices[i].DeviceName = "" + spec.Template.Spec.Network.Devices[i].DHCP4 = false + spec.Template.Spec.Network.Devices[i].DHCP6 = false + spec.Template.Spec.Network.Devices[i].SkipIPAllocation = false + + for j := range spec.Template.Spec.Network.Devices[i].AddressesFromPools { + spec.Template.Spec.Network.Devices[i].AddressesFromPools[j].APIGroup = nil + } + } + + spec.Template.Spec.Network.PreferredAPIServerCIDR = "" + }, + } +} diff --git a/pkg/conversion/capi2mapi/vsphere_test.go b/pkg/conversion/capi2mapi/vsphere_test.go new file mode 100644 index 000000000..108f59bb2 --- /dev/null +++ b/pkg/conversion/capi2mapi/vsphere_test.go @@ -0,0 +1,400 @@ +/* +Copyright 2025 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package capi2mapi + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/openshift/cluster-capi-operator/pkg/conversion/test/matchers" + "k8s.io/utils/ptr" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("capi2mapi vSphere conversion", func() { + var ( + vsphereCAPIMachineBase = &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + Namespace: "test-namespace", + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To("test-bootstrap-secret"), + }, + }, + } + + vsphereCAPIVSphereMachineBase = &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + Server: "vcenter.example.com", + Datacenter: "test-datacenter", + Folder: "test-folder", + Datastore: "test-datastore", + NumCPUs: 4, + MemoryMiB: 8192, + DiskGiB: 120, + }, + }, + } + + vsphereCAPIVSphereClusterBase = &vspherev1.VSphereCluster{ + Spec: vspherev1.VSphereClusterSpec{ + Server: "vcenter.example.com", + }, + } + ) + + type vsphereCAPI2MAPIMachineConversionInput struct { + machine *clusterv1.Machine + vsphereMachine *vspherev1.VSphereMachine + vsphereCluster *vspherev1.VSphereCluster + expectedErrors []string + expectedWarnings []string + } + + type vsphereCAPI2MAPIMachinesetConversionInput struct { + machineSet *clusterv1.MachineSet + vsphereMachineTemplate *vspherev1.VSphereMachineTemplate + vsphereCluster *vspherev1.VSphereCluster + expectedErrors []string + expectedWarnings []string + } + + var _ = DescribeTable("capi2mapi vSphere convert CAPI Machine/VSphereMachine/VSphereCluster to a MAPI Machine", + func(in vsphereCAPI2MAPIMachineConversionInput) { + _, warns, err := FromMachineAndVSphereMachineAndVSphereCluster( + in.machine, + in.vsphereMachine, + in.vsphereCluster, + ).ToMachine() + Expect(err).To(matchers.ConsistOfMatchErrorSubstrings(in.expectedErrors), + "should match expected errors while converting vSphere CAPI resources to MAPI Machine") + Expect(warns).To(matchers.ConsistOfSubstrings(in.expectedWarnings), + "should match expected warnings while converting vSphere CAPI resources to MAPI Machine") + }, + + // Base Case. + Entry("With a Base configuration", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: vsphereCAPIVSphereMachineBase, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Clone Mode Tests. + Entry("With full clone mode", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + CloneMode: vspherev1.FullClone, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With linked clone mode", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + CloneMode: vspherev1.LinkedClone, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With unsupported clone mode", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + CloneMode: "unsupported-mode", + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{ + "spec.cloneMode: Invalid value: \"unsupported-mode\": unable to convert clone mode, unknown value", + }, + expectedWarnings: []string{}, + }), + + // Data Disk Tests. + Entry("With data disk - thin provisioning", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + DataDisks: []vspherev1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: vspherev1.ThinProvisioningMode, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With data disk - thick provisioning", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + DataDisks: []vspherev1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: vspherev1.ThickProvisioningMode, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With data disk - eagerly zeroed provisioning", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + DataDisks: []vspherev1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: vspherev1.EagerlyZeroedProvisioningMode, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With unsupported provisioning mode in data disk", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + DataDisks: []vspherev1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: "invalid-mode", + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{ + "spec.dataDisks[0].provisioningMode: Invalid value: \"invalid-mode\": unable to convert provisioning mode, unknown value", + }, + expectedWarnings: []string{}, + }), + + Entry("With multiple data disks", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + DataDisks: []vspherev1.VSphereDisk{ + {Name: "disk1", SizeGiB: 100, ProvisioningMode: vspherev1.ThinProvisioningMode}, + {Name: "disk2", SizeGiB: 200, ProvisioningMode: vspherev1.ThickProvisioningMode}, + {Name: "disk3", SizeGiB: 50}, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Network Tests. + Entry("With network devices", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + Network: vspherev1.NetworkSpec{ + Devices: []vspherev1.NetworkDeviceSpec{ + { + NetworkName: "VM Network", + Gateway4: "192.168.1.1", + IPAddrs: []string{"192.168.1.100/24"}, + Nameservers: []string{"8.8.8.8"}, + }, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With DHCP network configuration", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + Network: vspherev1.NetworkSpec{ + Devices: []vspherev1.NetworkDeviceSpec{ + { + NetworkName: "VM Network", + DHCP4: true, + }, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With both DHCP and static IPs (warning case)", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + Network: vspherev1.NetworkSpec{ + Devices: []vspherev1.NetworkDeviceSpec{ + { + NetworkName: "VM Network", + DHCP4: true, + IPAddrs: []string{"192.168.1.100/24"}, + }, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{ + "device 0 has both DHCP and static IPs configured, MAPI will use static IPs", + }, + }), + + // Tags Test. + Entry("With tags", vsphereCAPI2MAPIMachineConversionInput{ + machine: vsphereCAPIMachineBase, + vsphereMachine: &vspherev1.VSphereMachine{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + TagIDs: []string{ + "urn:vmomi:InventoryServiceTag:5736bf56-49f5-4667-b38c-b97e09dc9578:GLOBAL", + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + ) + + var _ = DescribeTable("capi2mapi vSphere convert CAPI MachineSet/VSphereMachineTemplate/VSphereCluster to a MAPI MachineSet", + func(in vsphereCAPI2MAPIMachinesetConversionInput) { + _, warns, err := FromMachineSetAndVSphereMachineTemplateAndVSphereCluster( + in.machineSet, + in.vsphereMachineTemplate, + in.vsphereCluster, + ).ToMachineSet() + Expect(err).To(matchers.ConsistOfMatchErrorSubstrings(in.expectedErrors), + "should match expected errors while converting vSphere CAPI resources to MAPI MachineSet") + Expect(warns).To(matchers.ConsistOfSubstrings(in.expectedWarnings), + "should match expected warnings while converting vSphere CAPI resources to MAPI MachineSet") + }, + + // Base Case. + Entry("With a Base configuration", vsphereCAPI2MAPIMachinesetConversionInput{ + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machineset", + Namespace: "test-namespace", + }, + Spec: clusterv1.MachineSetSpec{ + Replicas: ptr.To(int32(3)), + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To("test-bootstrap-secret"), + }, + }, + }, + }, + }, + vsphereMachineTemplate: &vspherev1.VSphereMachineTemplate{ + Spec: vspherev1.VSphereMachineTemplateSpec{ + Template: vspherev1.VSphereMachineTemplateResource{ + Spec: vspherev1.VSphereMachineSpec{ + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: "test-template", + Server: "vcenter.example.com", + Datacenter: "test-datacenter", + NumCPUs: 4, + MemoryMiB: 8192, + DiskGiB: 120, + }, + }, + }, + }, + }, + vsphereCluster: vsphereCAPIVSphereClusterBase, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + ) +}) diff --git a/pkg/conversion/mapi2capi/util.go b/pkg/conversion/mapi2capi/util.go index e844c023d..cc15937a7 100644 --- a/pkg/conversion/mapi2capi/util.go +++ b/pkg/conversion/mapi2capi/util.go @@ -42,6 +42,13 @@ func ProviderSpecFromRawExtension(platform configv1.PlatformType, rawExtension * return nil, fmt.Errorf("unable to parse AWS providerSpec: %w", err) } + return providerConfig, nil + case configv1.VSpherePlatformType: + providerConfig, err := vSphereProviderSpecFromRawExtension(rawExtension) + if err != nil { + return nil, fmt.Errorf("unable to parse vSphere providerSpec: %w", err) + } + return providerConfig, nil default: return nil, fmt.Errorf("%w: %s", errUnsupportedPlatform, platform) diff --git a/pkg/conversion/mapi2capi/vsphere.go b/pkg/conversion/mapi2capi/vsphere.go new file mode 100644 index 000000000..ecb538b3d --- /dev/null +++ b/pkg/conversion/mapi2capi/vsphere.go @@ -0,0 +1,445 @@ +/* +Copyright 2024 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package mapi2capi + +import ( + "fmt" + "reflect" + + configv1 "github.com/openshift/api/config/v1" + mapiv1beta1 "github.com/openshift/api/machine/v1beta1" + "github.com/openshift/cluster-capi-operator/pkg/conversion/consts" + "github.com/openshift/cluster-capi-operator/pkg/util" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/validation/field" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" +) + +const ( + // DefaultVSphereCredentialsSecretName is the name of the default secret containing vSphere cloud credentials. + DefaultVSphereCredentialsSecretName = "vsphere-cloud-credentials" //#nosec G101 -- This is a false positive. + + vSphereMachineKind = "VSphereMachine" + vSphereMachineTemplateKind = "VSphereMachineTemplate" +) + +// vSphereMachineAndInfra stores the details of a Machine API VSphereMachine and Infra. +type vSphereMachineAndInfra struct { + machine *mapiv1beta1.Machine + infrastructure *configv1.Infrastructure +} + +// vSphereMachineSetAndInfra stores the details of a Machine API VSphereMachineSet and Infra. +type vSphereMachineSetAndInfra struct { + machineSet *mapiv1beta1.MachineSet + infrastructure *configv1.Infrastructure + *vSphereMachineAndInfra +} + +// FromVSphereMachineAndInfra wraps a Machine API Machine for vSphere and the OCP Infrastructure object into a mapi2capi VSphereMachine. +func FromVSphereMachineAndInfra(m *mapiv1beta1.Machine, i *configv1.Infrastructure) Machine { + return &vSphereMachineAndInfra{machine: m, infrastructure: i} +} + +// FromVSphereMachineSetAndInfra wraps a Machine API MachineSet for vSphere and the OCP Infrastructure object into a mapi2capi VSphereMachineSet. +func FromVSphereMachineSetAndInfra(m *mapiv1beta1.MachineSet, i *configv1.Infrastructure) MachineSet { + return &vSphereMachineSetAndInfra{ + machineSet: m, + infrastructure: i, + vSphereMachineAndInfra: &vSphereMachineAndInfra{ + machine: &mapiv1beta1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: m.Spec.Template.ObjectMeta.Labels, //nolint:staticcheck // ObjectMeta is a field, not embedded + Annotations: m.Spec.Template.ObjectMeta.Annotations, //nolint:staticcheck // ObjectMeta is a field, not embedded + }, + Spec: m.Spec.Template.Spec, + }, + infrastructure: i, + }, + } +} + +// ToMachineAndInfrastructureMachine converts a MAPI Machine to a CAPI Machine and VSphereMachine. +func (v *vSphereMachineAndInfra) ToMachineAndInfrastructureMachine() (*clusterv1.Machine, client.Object, []string, error) { + machine, infraMachine, warnings, errs := v.toMachineAndInfrastructureMachine() + if len(errs) > 0 { + return nil, nil, warnings, errs.ToAggregate() + } + + return machine, infraMachine, warnings, nil +} + +// toMachineAndInfrastructureMachine is the internal implementation of the conversion. +func (v *vSphereMachineAndInfra) toMachineAndInfrastructureMachine() (*clusterv1.Machine, client.Object, []string, field.ErrorList) { + var errs field.ErrorList + + vSphereProviderConfig, err := vSphereProviderSpecFromRawExtension(v.machine.Spec.ProviderSpec.Value) + if err != nil { + return nil, nil, nil, field.ErrorList{field.Invalid(field.NewPath("spec", "providerSpec", "value"), v.machine.Spec.ProviderSpec.Value, err.Error())} + } + + capvMachine, warnings, machineErrs := v.toVSphereMachine(vSphereProviderConfig) + if machineErrs != nil { + errs = append(errs, machineErrs...) + } + + capiMachine, machineErrs := fromMAPIMachineToCAPIMachine(v.machine, vspherev1.GroupVersion.Group, vSphereMachineKind) + if machineErrs != nil { + errs = append(errs, machineErrs...) + } + + // Set ProviderID if available + if v.machine.Spec.ProviderID != nil { + capvMachine.Spec.ProviderID = v.machine.Spec.ProviderID + } + + // Set FailureDomain from MAPI machine zone label + // vSphere doesn't have a FailureDomain field in the provider spec, so it's stored in metadata + if zone, ok := v.machine.Labels[consts.MAPIMachineMetadataLabelZone]; ok && zone != "" { + capiMachine.Spec.FailureDomain = zone + } + + // Plug into Core CAPI Machine fields that come from the MAPI ProviderConfig which belong here instead of the CAPI VSphereMachineTemplate. + if vSphereProviderConfig.UserDataSecret != nil && vSphereProviderConfig.UserDataSecret.Name != "" { + capiMachine.Spec.Bootstrap = clusterv1.Bootstrap{ + DataSecretName: &vSphereProviderConfig.UserDataSecret.Name, + } + } + + // Populate the CAPI Machine ClusterName from the OCP Infrastructure object + if v.infrastructure == nil || v.infrastructure.Status.InfrastructureName == "" { + errs = append(errs, field.Invalid(field.NewPath("infrastructure", "status", "infrastructureName"), v.infrastructure.Status.InfrastructureName, "infrastructure cannot be nil and infrastructure.Status.InfrastructureName cannot be empty")) + } else { + capiMachine.Spec.ClusterName = v.infrastructure.Status.InfrastructureName + capiMachine.Labels[clusterv1.ClusterNameLabel] = v.infrastructure.Status.InfrastructureName + } + + // The InfraMachine should always have the same labels and annotations as the Machine. + // See https://github.com/kubernetes-sigs/cluster-api/blob/f88d7ae5155700c2cc367b31ddcc151c9ad579e4/internal/controllers/machineset/machineset_controller.go#L578-L579 + capiMachineAnnotations := capiMachine.GetAnnotations() + if len(capiMachineAnnotations) > 0 { + capvMachine.SetAnnotations(capiMachineAnnotations) + } + + capiMachineLabels := capiMachine.GetLabels() + if len(capiMachineLabels) > 0 { + capvMachine.SetLabels(capiMachineLabels) + } + + return capiMachine, capvMachine, warnings, errs +} + +// ToMachineSetAndMachineTemplate converts a mapi2capi vSphereMachineSetAndInfra into a CAPI MachineSet and CAPV vSphereMachineTemplate. +func (v *vSphereMachineSetAndInfra) ToMachineSetAndMachineTemplate() (*clusterv1.MachineSet, client.Object, []string, error) { + var errors []error + + // Run the full ToMachine conversion to check for errors + capiMachine, capvMachineObj, warning, machineErrs := v.toMachineAndInfrastructureMachine() + if machineErrs != nil { + errors = append(errors, machineErrs.ToAggregate().Errors()...) + } + + capvMachine, ok := capvMachineObj.(*vspherev1.VSphereMachine) + if !ok { + panic(fmt.Errorf("%w: %T", errUnexpectedObjectTypeForMachine, capvMachineObj)) + } + + capvMachineTemplate, err := vSphereMachineToVSphereMachineTemplate(capvMachine, v.machineSet.Name, capiNamespace) + if err != nil { + errors = append(errors, err) + } + + capiMachineSet, machineSetErrs := fromMAPIMachineSetToCAPIMachineSet(v.machineSet) + if machineSetErrs != nil { + errors = append(errors, machineSetErrs.Errors()...) + } + + if capiMachine.Spec.MinReadySeconds == nil { + capiMachine.Spec.MinReadySeconds = capiMachineSet.Spec.Template.Spec.MinReadySeconds + } + + capiMachineSet.Spec.Template.Spec = capiMachine.Spec + + // We have to merge these two maps so that labels and annotations added to the template objectmeta are persisted + // along with the labels and annotations from the machine objectmeta. + capiMachineSet.Spec.Template.ObjectMeta.Labels = util.MergeMaps(capiMachineSet.Spec.Template.ObjectMeta.Labels, capiMachine.Labels) //nolint:staticcheck // ObjectMeta is a field, not embedded + capiMachineSet.Spec.Template.ObjectMeta.Annotations = util.MergeMaps(capiMachineSet.Spec.Template.ObjectMeta.Annotations, capiMachine.Annotations) //nolint:staticcheck // ObjectMeta is a field, not embedded + + // Override the reference so that it matches the VSphereMachineTemplate. + capiMachineSet.Spec.Template.Spec.InfrastructureRef.Kind = vSphereMachineTemplateKind + capiMachineSet.Spec.Template.Spec.InfrastructureRef.Name = capvMachineTemplate.Name + + if v.infrastructure == nil || v.infrastructure.Status.InfrastructureName == "" { + errors = append(errors, field.Invalid(field.NewPath("infrastructure", "status", "infrastructureName"), v.infrastructure.Status.InfrastructureName, "infrastructure cannot be nil and infrastructure.Status.InfrastructureName cannot be empty")) + } else { + capiMachineSet.Spec.Template.Spec.ClusterName = v.infrastructure.Status.InfrastructureName + capiMachineSet.Spec.ClusterName = v.infrastructure.Status.InfrastructureName + capiMachineSet.Labels[clusterv1.ClusterNameLabel] = v.infrastructure.Status.InfrastructureName + } + + if len(errors) > 0 { + return nil, nil, warning, utilerrors.NewAggregate(errors) + } + + return capiMachineSet, capvMachineTemplate, warning, nil +} + +// toVSphereMachine converts a MAPI VSphereMachineProviderConfig to a CAPI VSphereMachine. +// +//nolint:funlen +func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphereMachineProviderSpec) (*vspherev1.VSphereMachine, []string, field.ErrorList) { + var errs field.ErrorList + + fldPath := field.NewPath("spec", "providerSpec", "value") + + // Convert network configuration + capiNetworkSpec, warnings, networkErrs := convertVSphereNetworkSpecMAPIToCAPI(fldPath.Child("network"), providerSpec.Network) + if len(networkErrs) > 0 { + errs = append(errs, networkErrs...) + } + + // Convert data disks + capiDataDisks, diskErrs := convertVSphereDataDisksMAPIToCAPI(fldPath.Child("dataDisks"), providerSpec.DataDisks) + if len(diskErrs) > 0 { + errs = append(errs, diskErrs...) + } + + // Convert clone mode + capiCloneMode := convertVSphereCloneModeMAPIToCAPI(providerSpec.CloneMode) + + spec := vspherev1.VSphereMachineSpec{ + PowerOffMode: vspherev1.VirtualMachinePowerOpModeHard, + VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ + Template: providerSpec.Template, + CloneMode: capiCloneMode, + Snapshot: providerSpec.Snapshot, + NumCPUs: providerSpec.NumCPUs, + NumCoresPerSocket: providerSpec.NumCoresPerSocket, + MemoryMiB: providerSpec.MemoryMiB, + DiskGiB: providerSpec.DiskGiB, + TagIDs: providerSpec.TagIDs, + Network: capiNetworkSpec, + DataDisks: capiDataDisks, + // Server - Set below from Workspace. + // Datacenter - Set below from Workspace. + // Folder - Set below from Workspace. + // Datastore - Set below from Workspace. + // ResourcePool - Set below from Workspace. + // Thumbprint - Not supported in MAPI. + // StoragePolicyName - Not supported in MAPI. + // Resources - Not supported in MAPI. + // AdditionalDisksGiB - Deprecated in CAPV, using DataDisks instead. + // CustomVMXKeys - Not supported in MAPI. + // PciDevices - Not supported in MAPI. + // OS - Not supported in MAPI. + // HardwareVersion - Not supported in MAPI. + }, + // ProviderID - Set at a higher level in ToMachine(). + // FailureDomain - Set at a higher level from machine.Spec.FailureDomain. + // GuestSoftPowerOffTimeout - Not supported in MAPI. + // NamingStrategy - Not supported in MAPI. + } + + // Set workspace fields if available + if providerSpec.Workspace != nil { + spec.Server = providerSpec.Workspace.Server + spec.Datacenter = providerSpec.Workspace.Datacenter + spec.Folder = providerSpec.Workspace.Folder + spec.Datastore = providerSpec.Workspace.Datastore + spec.ResourcePool = providerSpec.Workspace.ResourcePool + // VMGroup - MAPI-specific field for vm-host group based zoning, not supported in CAPV. + } + + // Unused fields - Below this line are fields not used from the MAPI VSphereMachineProviderSpec. + + // TypeMeta - Only for the purpose of the raw extension, not used for any functionality. + // UserDataSecret - Handled at a higher level in fromMAPIMachineToCAPIMachine. + + if !reflect.DeepEqual(providerSpec.ObjectMeta, metav1.ObjectMeta{}) { + // We don't support setting the object metadata in the provider spec. + // It's only present for the purpose of the raw extension and doesn't have any functionality. + errs = append(errs, field.Invalid(fldPath.Child("metadata"), providerSpec.ObjectMeta, "metadata is not supported")) + } + + // Only take action when a non-default credentials secret is being used in MAPI. + // If the user is using the default, then their CAPV secret will already be configured and no action is necessary. + if providerSpec.CredentialsSecret != nil && + providerSpec.CredentialsSecret.Name != "" && + providerSpec.CredentialsSecret.Name != DefaultVSphereCredentialsSecretName { + // Not convertable; need custom credential configuration + errs = append(errs, field.Invalid(fldPath.Child("credentialsSecret"), providerSpec.CredentialsSecret.Name, fmt.Sprintf("credentialsSecret does not match the default of %q, credentials must be configured at the cluster level", DefaultVSphereCredentialsSecretName))) + } + + if providerSpec.Workspace != nil && providerSpec.Workspace.VMGroup != "" { + // VMGroup is a MAPI-specific field for vm-host group based zoning that doesn't exist in CAPV. + errs = append(errs, field.Invalid(fldPath.Child("workspace", "vmGroup"), providerSpec.Workspace.VMGroup, "vmGroup is not supported in Cluster API")) + } + + return &vspherev1.VSphereMachine{ + TypeMeta: metav1.TypeMeta{ + APIVersion: vspherev1.GroupVersion.String(), + Kind: vSphereMachineKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: v.machine.Name, + Namespace: capiNamespace, + }, + Spec: spec, + }, warnings, errs +} + +// vSphereProviderSpecFromRawExtension unmarshals a raw extension into a VSphereMachineProviderSpec type. +func vSphereProviderSpecFromRawExtension(rawExtension *runtime.RawExtension) (mapiv1beta1.VSphereMachineProviderSpec, error) { + if rawExtension == nil { + return mapiv1beta1.VSphereMachineProviderSpec{}, nil + } + + spec := mapiv1beta1.VSphereMachineProviderSpec{} + if err := yaml.Unmarshal(rawExtension.Raw, &spec); err != nil { + return mapiv1beta1.VSphereMachineProviderSpec{}, fmt.Errorf("error unmarshalling providerSpec: %w", err) + } + + return spec, nil +} + +// vSphereMachineToVSphereMachineTemplate converts a VSphereMachine to a VSphereMachineTemplate. +func vSphereMachineToVSphereMachineTemplate(vSphereMachine *vspherev1.VSphereMachine, name string, namespace string) (*vspherev1.VSphereMachineTemplate, error) { + nameWithHash, err := util.GenerateInfraMachineTemplateNameWithSpecHash(name, vSphereMachine.Spec) + if err != nil { + return nil, fmt.Errorf("failed to generate infrastructure machine template name with spec hash: %w", err) + } + + return &vspherev1.VSphereMachineTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: vspherev1.GroupVersion.String(), + Kind: vSphereMachineTemplateKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: nameWithHash, + Namespace: namespace, + }, + Spec: vspherev1.VSphereMachineTemplateSpec{ + Template: vspherev1.VSphereMachineTemplateResource{ + Spec: vSphereMachine.Spec, + }, + }, + }, nil +} + +//////// Conversion helpers + +// convertMAPINetworkSpecToCAPI converts MAPI NetworkSpec to CAPI NetworkSpec. +func convertVSphereNetworkSpecMAPIToCAPI(fldPath *field.Path, mapiNetwork mapiv1beta1.NetworkSpec) (vspherev1.NetworkSpec, []string, field.ErrorList) { //nolint:unparam + var ( + errs field.ErrorList + warnings []string + ) + + if len(mapiNetwork.Devices) == 0 { + return vspherev1.NetworkSpec{ + Devices: nil, + }, warnings, errs + } + + devices := make([]vspherev1.NetworkDeviceSpec, len(mapiNetwork.Devices)) + for i, device := range mapiNetwork.Devices { + devices[i] = vspherev1.NetworkDeviceSpec{ + NetworkName: device.NetworkName, + DHCP4: len(device.IPAddrs) == 0 && len(device.AddressesFromPools) == 0, // Use DHCP if no static IPs + Gateway4: device.Gateway, + IPAddrs: device.IPAddrs, + Nameservers: device.Nameservers, + } + + // Convert AddressesFromPools + if len(device.AddressesFromPools) > 0 { + addressesFromPools := make([]corev1.TypedLocalObjectReference, len(device.AddressesFromPools)) + for j, pool := range device.AddressesFromPools { + addressesFromPools[j] = corev1.TypedLocalObjectReference{ + APIGroup: &pool.Group, + Kind: pool.Resource, // This might need adjustment based on actual mapping + Name: pool.Name, + } + } + + devices[i].AddressesFromPools = addressesFromPools + } + } + + return vspherev1.NetworkSpec{ + Devices: devices, + }, warnings, errs +} + +// convertMAPIDataDisksToCAPI converts MAPI DataDisks to CAPI DataDisks. +func convertVSphereDataDisksMAPIToCAPI(fldPath *field.Path, mapiDisks []mapiv1beta1.VSphereDisk) ([]vspherev1.VSphereDisk, field.ErrorList) { + var ( + errs field.ErrorList + ) + + // Return nil disks slice if empty to ensure roundtrip consistency + // (MAPI nil -> CAPI nil -> MAPI nil) + if len(mapiDisks) == 0 { + return nil, errs + } + + capiDisks := make([]vspherev1.VSphereDisk, len(mapiDisks)) + for i, disk := range mapiDisks { + capiDisks[i] = vspherev1.VSphereDisk{ + Name: disk.Name, + SizeGiB: disk.SizeGiB, + } + + // Convert provisioning mode + switch disk.ProvisioningMode { + case mapiv1beta1.ProvisioningModeThin: + capiDisks[i].ProvisioningMode = vspherev1.ThinProvisioningMode + case mapiv1beta1.ProvisioningModeThick: + capiDisks[i].ProvisioningMode = vspherev1.ThickProvisioningMode + case mapiv1beta1.ProvisioningModeEagerlyZeroed: + capiDisks[i].ProvisioningMode = vspherev1.EagerlyZeroedProvisioningMode + case "": + // Default - no setting + default: + errs = append(errs, field.Invalid(fldPath.Index(i).Child("provisioningMode"), disk.ProvisioningMode, "unsupported provisioning mode")) + } + } + + return capiDisks, errs +} + +// convertVSphereCloneModeMAPIToCAPI converts MAPI vSphere CloneMode to CAPI vSphere CloneMode. +func convertVSphereCloneModeMAPIToCAPI(mapiMode mapiv1beta1.CloneMode) vspherev1.CloneMode { + switch mapiMode { + case mapiv1beta1.FullClone: + return vspherev1.FullClone + case mapiv1beta1.LinkedClone: + return vspherev1.LinkedClone + case "": + // MAPI defaults to FullClone when empty, but CAPV defaults to LinkedClone. + // Explicitly set FullClone to match MAPI's default behavior. + return vspherev1.FullClone + default: + // Unknown value - default to FullClone to match MAPI's default + return vspherev1.FullClone + } +} diff --git a/pkg/conversion/mapi2capi/vsphere_fuzz_test.go b/pkg/conversion/mapi2capi/vsphere_fuzz_test.go new file mode 100644 index 000000000..89171d79c --- /dev/null +++ b/pkg/conversion/mapi2capi/vsphere_fuzz_test.go @@ -0,0 +1,192 @@ +/* +Copyright 2025 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package mapi2capi_test + +import ( + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + configv1 "github.com/openshift/api/config/v1" + mapiv1beta1 "github.com/openshift/api/machine/v1beta1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" + vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/randfill" + + "github.com/openshift/cluster-capi-operator/pkg/conversion/capi2mapi" + "github.com/openshift/cluster-capi-operator/pkg/conversion/mapi2capi" + conversiontest "github.com/openshift/cluster-capi-operator/pkg/conversion/test/fuzz" +) + +const ( + vSphereProviderSpecKind = "VSphereMachineProviderSpec" +) + +var _ = Describe("vSphere Fuzz (mapi2capi)", func() { + infra := &configv1.Infrastructure{ + Spec: configv1.InfrastructureSpec{}, + Status: configv1.InfrastructureStatus{ + InfrastructureName: "sample-cluster-name", + }, + } + + infraCluster := &vspherev1.VSphereCluster{ + Spec: vspherev1.VSphereClusterSpec{}, + } + + Context("VSphereMachine Conversion", func() { + fromMachineAndVSphereMachineAndVSphereCluster := func(machine *clusterv1.Machine, infraMachine client.Object, infraCluster client.Object) capi2mapi.MachineAndInfrastructureMachine { + vsphereMachine, ok := infraMachine.(*vspherev1.VSphereMachine) + Expect(ok).To(BeTrue(), "input infra machine should be of type %T, got %T", &vspherev1.VSphereMachine{}, infraMachine) + + vsphereCluster, ok := infraCluster.(*vspherev1.VSphereCluster) + Expect(ok).To(BeTrue(), "input infra cluster should be of type %T, got %T", &vspherev1.VSphereCluster{}, infraCluster) + + return capi2mapi.FromMachineAndVSphereMachineAndVSphereCluster(machine, vsphereMachine, vsphereCluster) + } + + f := &vsphereProviderFuzzer{} + + conversiontest.MAPI2CAPIMachineRoundTripFuzzTest( + scheme, + infra, + infraCluster, + mapi2capi.FromVSphereMachineAndInfra, + fromMachineAndVSphereMachineAndVSphereCluster, + conversiontest.ObjectMetaFuzzerFuncs(mapiNamespace), + conversiontest.MAPIMachineFuzzerFuncs(&mapiv1beta1.VSphereMachineProviderSpec{}, nil, vsphereProviderIDFuzzer), + f.FuzzerFuncsMachine, + ) + }) + + Context("VSphereMachineSet Conversion", func() { + fromMachineSetAndVSphereMachineTemplateAndVSphereCluster := func(machineSet *clusterv1.MachineSet, infraMachineTemplate client.Object, infraCluster client.Object) capi2mapi.MachineSetAndMachineTemplate { + vsphereMachineTemplate, ok := infraMachineTemplate.(*vspherev1.VSphereMachineTemplate) + Expect(ok).To(BeTrue(), "input infra machine template should be of type %T, got %T", &vspherev1.VSphereMachineTemplate{}, infraMachineTemplate) + + vsphereCluster, ok := infraCluster.(*vspherev1.VSphereCluster) + Expect(ok).To(BeTrue(), "input infra cluster should be of type %T, got %T", &vspherev1.VSphereCluster{}, infraCluster) + + return capi2mapi.FromMachineSetAndVSphereMachineTemplateAndVSphereCluster(machineSet, vsphereMachineTemplate, vsphereCluster) + } + + f := &vsphereProviderFuzzer{} + + conversiontest.MAPI2CAPIMachineSetRoundTripFuzzTest( + scheme, + infra, + infraCluster, + mapi2capi.FromVSphereMachineSetAndInfra, + fromMachineSetAndVSphereMachineTemplateAndVSphereCluster, + conversiontest.ObjectMetaFuzzerFuncs(mapiNamespace), + conversiontest.MAPIMachineFuzzerFuncs(&mapiv1beta1.VSphereMachineProviderSpec{}, nil, vsphereProviderIDFuzzer), + conversiontest.MAPIMachineSetFuzzerFuncs(), + f.FuzzerFuncsMachineSet, + ) + }) +}) + +func vsphereProviderIDFuzzer(c randfill.Continue) string { + return "vsphere://" + strings.ReplaceAll(c.String(0), "/", "") +} + +type vsphereProviderFuzzer struct { + conversiontest.MAPIMachineFuzzer +} + +func (f *vsphereProviderFuzzer) fuzzProviderSpec(providerSpec *mapiv1beta1.VSphereMachineProviderSpec, c randfill.Continue) { + c.FillNoCustom(providerSpec) + + // The type meta is always set to these values by the conversion. + providerSpec.APIVersion = mapiv1beta1.GroupVersion.String() + providerSpec.Kind = vSphereProviderSpecKind + + // Clear fields that are not supported in the provider spec. + providerSpec.ObjectMeta = metav1.ObjectMeta{} + + // Clear vmGroup field - it's a MAPI-specific field that doesn't exist in CAPV + // and is not preserved during conversion (similar to OpenStack's FloatingIP, PrimarySubnet, etc.) + if providerSpec.Workspace != nil { + providerSpec.Workspace.VMGroup = "" + } + + // Only one value here is valid in terms of fuzzing, so it is hardcoded. + providerSpec.CredentialsSecret = &corev1.LocalObjectReference{ + Name: "vsphere-cloud-credentials", + } + + // Clear pointers to empty structs. + if providerSpec.UserDataSecret != nil && providerSpec.UserDataSecret.Name == "" { + providerSpec.UserDataSecret = nil + } + + // Normalize empty network devices to nil to match conversion behavior. + // Empty slices are marshaled as [] but conversion returns nil. + if providerSpec.Network.Devices != nil && len(providerSpec.Network.Devices) == 0 { + providerSpec.Network.Devices = nil + } + + // Copy template and datacenter to the struct so they can be set at the machine labels too. + // For vSphere: template is used as instance-type, datacenter as region. + // Zone comes from machine.Spec.FailureDomain (not from provider spec) so we leave it empty. + f.InstanceType = providerSpec.Template + if providerSpec.Workspace != nil { + f.Region = providerSpec.Workspace.Datacenter + } +} + +func (f *vsphereProviderFuzzer) FuzzerFuncsMachineSet(codecs runtimeserializer.CodecFactory) []interface{} { + return []interface{}{ + func(cloneMode *mapiv1beta1.CloneMode, c randfill.Continue) { + switch c.Int31n(2) { + case 0: + *cloneMode = mapiv1beta1.FullClone + case 1: + *cloneMode = mapiv1beta1.LinkedClone + // case 2: + // *cloneMode = "" // Do not fuzz MAPI CloneMode to the empty value. + // It will otherwise get converted to CAPV FullClone which + // if converted back to MAPI will become FullClone, + // resulting in a documented lossy roundtrip conversion, which would make the test to fail. + } + }, + func(provisioningMode *mapiv1beta1.ProvisioningMode, c randfill.Continue) { + switch c.Int31n(4) { + case 0: + *provisioningMode = mapiv1beta1.ProvisioningModeThin + case 1: + *provisioningMode = mapiv1beta1.ProvisioningModeThick + case 2: + *provisioningMode = mapiv1beta1.ProvisioningModeEagerlyZeroed + case 3: + *provisioningMode = "" + } + }, + f.fuzzProviderSpec, + } +} + +func (f *vsphereProviderFuzzer) FuzzerFuncsMachine(codecs runtimeserializer.CodecFactory) []interface{} { + return append( + f.FuzzerFuncsMachineSet(codecs), + f.FuzzMachine, + ) +} diff --git a/pkg/conversion/mapi2capi/vsphere_test.go b/pkg/conversion/mapi2capi/vsphere_test.go new file mode 100644 index 000000000..f5404e810 --- /dev/null +++ b/pkg/conversion/mapi2capi/vsphere_test.go @@ -0,0 +1,514 @@ +/* +Copyright 2025 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package mapi2capi + +import ( + "encoding/json" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/openshift/cluster-capi-operator/pkg/conversion/test/matchers" + + configv1 "github.com/openshift/api/config/v1" + mapiv1beta1 "github.com/openshift/api/machine/v1beta1" + machinebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1beta1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var _ = Describe("mapi2capi VSphere conversion", func() { + var ( + vsphereBaseProviderSpec = machinebuilder.VSphereProviderSpec() + vsphereMAPIMachineBase = machinebuilder.Machine().WithProviderSpecBuilder(vsphereBaseProviderSpec) + vsphereMAPIMachineSetBase = machinebuilder.MachineSet().WithProviderSpecBuilder(vsphereBaseProviderSpec) + + infra = &configv1.Infrastructure{ + Spec: configv1.InfrastructureSpec{}, + Status: configv1.InfrastructureStatus{InfrastructureName: "sample-cluster-name"}, + } + ) + + type vsphereMAPI2CAPIConversionInput struct { + machineBuilder machinebuilder.MachineBuilder + infra *configv1.Infrastructure + expectedErrors []string + expectedWarnings []string + } + + type vsphereMAPI2CAPIMachinesetConversionInput struct { + machineSetBuilder machinebuilder.MachineSetBuilder + infra *configv1.Infrastructure + expectedErrors []string + expectedWarnings []string + } + + var mustConvertVSphereProviderSpecToRawExtension = func(spec *mapiv1beta1.VSphereMachineProviderSpec) *runtime.RawExtension { + if spec == nil { + return &runtime.RawExtension{} + } + + rawBytes, err := json.Marshal(spec) + if err != nil { + panic(fmt.Sprintf("unable to convert (marshal) test VSphereMachineProviderSpec to runtime.RawExtension: %v", err)) + } + + return &runtime.RawExtension{ + Raw: rawBytes, + } + } + + var _ = DescribeTable("mapi2capi VSphere convert MAPI Machine", + func(in vsphereMAPI2CAPIConversionInput) { + _, _, warns, err := FromVSphereMachineAndInfra(in.machineBuilder.Build(), in.infra).ToMachineAndInfrastructureMachine() + Expect(err).To(matchers.ConsistOfMatchErrorSubstrings(in.expectedErrors), "should match expected errors while converting a VSphere MAPI Machine to CAPI") + Expect(warns).To(matchers.ConsistOfSubstrings(in.expectedWarnings), "should match expected warnings while converting a VSphere MAPI Machine to CAPI") + }, + + // Base Case. + Entry("With a Base configuration", vsphereMAPI2CAPIConversionInput{ + machineBuilder: vsphereMAPIMachineBase, + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With tags", vsphereMAPI2CAPIConversionInput{ + machineBuilder: vsphereMAPIMachineBase.WithProviderSpecBuilder( + vsphereBaseProviderSpec.WithTags([]string{ + "urn:vmomi:InventoryServiceTag:5736bf56-49f5-4667-b38c-b97e09dc9578:GLOBAL", + }), + ), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With IP pool", vsphereMAPI2CAPIConversionInput{ + machineBuilder: vsphereMAPIMachineBase.WithProviderSpecBuilder( + vsphereBaseProviderSpec.WithIPPool(), + ), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Error Cases. + Entry("With empty infrastructure name", vsphereMAPI2CAPIConversionInput{ + machineBuilder: vsphereMAPIMachineBase, + infra: &configv1.Infrastructure{ + Status: configv1.InfrastructureStatus{InfrastructureName: ""}, + }, + expectedErrors: []string{ + "infrastructure.status.infrastructureName: Invalid value: \"\": infrastructure cannot be nil and infrastructure.Status.InfrastructureName cannot be empty", + }, + expectedWarnings: []string{}, + }), + + Entry("With unsupported provisioning mode in data disk", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: "invalid-mode", + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{ + "spec.providerSpec.value.dataDisks[0].provisioningMode: Invalid value: \"invalid-mode\": unsupported provisioning mode", + }, + expectedWarnings: []string{}, + }), + + // Data Disk Tests. + Entry("With data disk - thin provisioning", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: mapiv1beta1.ProvisioningModeThin, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With data disk - thick provisioning", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: mapiv1beta1.ProvisioningModeThick, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With data disk - eagerly zeroed provisioning", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + ProvisioningMode: mapiv1beta1.ProvisioningModeEagerlyZeroed, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With data disk - no provisioning mode specified", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + { + Name: "disk1", + SizeGiB: 100, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With multiple data disks", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + {Name: "disk1", SizeGiB: 100, ProvisioningMode: mapiv1beta1.ProvisioningModeThin}, + {Name: "disk2", SizeGiB: 200, ProvisioningMode: mapiv1beta1.ProvisioningModeThick}, + {Name: "disk3", SizeGiB: 50}, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Clone Mode Tests. + Entry("With full clone mode", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + CloneMode: mapiv1beta1.FullClone, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With linked clone mode", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + CloneMode: mapiv1beta1.LinkedClone, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With linked clone mode and snapshot", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + CloneMode: mapiv1beta1.LinkedClone, + Snapshot: "snapshot-1", + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Workspace Tests. + Entry("With workspace configuration", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Workspace: &mapiv1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + Folder: "/vm/folder", + Datastore: "datastore1", + ResourcePool: "pool1", + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // VM Configuration Tests. + Entry("With custom VM configuration", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + NumCPUs: 8, + NumCoresPerSocket: 4, + MemoryMiB: 16384, + DiskGiB: 120, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Secret Tests. + Entry("With custom credentials secret", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + CredentialsSecret: &corev1.LocalObjectReference{ + Name: "custom-vsphere-credentials", + }, + }), + }), + infra: infra, + expectedErrors: []string{ + "spec.providerSpec.value.credentialsSecret: Invalid value: \"custom-vsphere-credentials\": credentialsSecret does not match the default of \"vsphere-cloud-credentials\", credentials must be configured at the cluster level", + }, + expectedWarnings: []string{}, + }), + + Entry("With user data secret", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + UserDataSecret: &corev1.LocalObjectReference{ + Name: "worker-user-data", + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Network Configuration Tests. + Entry("With network devices", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Network: mapiv1beta1.NetworkSpec{ + Devices: []mapiv1beta1.NetworkDeviceSpec{ + {NetworkName: "network1"}, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With multiple network devices", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Network: mapiv1beta1.NetworkSpec{ + Devices: []mapiv1beta1.NetworkDeviceSpec{ + {NetworkName: "network1"}, + {NetworkName: "network2"}, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + // Comprehensive Test. + Entry("With comprehensive configuration", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + CloneMode: mapiv1beta1.FullClone, + Snapshot: "snapshot-1", + NumCPUs: 8, + NumCoresPerSocket: 4, + MemoryMiB: 16384, + DiskGiB: 120, + TagIDs: []string{ + "urn:vmomi:InventoryServiceTag:5736bf56-49f5-4667-b38c-b97e09dc9578:GLOBAL", + }, + Workspace: &mapiv1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + Folder: "/vm/folder", + Datastore: "datastore1", + ResourcePool: "pool1", + }, + Network: mapiv1beta1.NetworkSpec{ + Devices: []mapiv1beta1.NetworkDeviceSpec{ + {NetworkName: "network1"}, + }, + }, + DataDisks: []mapiv1beta1.VSphereDisk{ + {Name: "disk1", SizeGiB: 100, ProvisioningMode: mapiv1beta1.ProvisioningModeThin}, + }, + UserDataSecret: &corev1.LocalObjectReference{ + Name: "worker-user-data", + }, + CredentialsSecret: &corev1.LocalObjectReference{ + Name: "vsphere-cloud-credentials", + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With VMGroup in workspace", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Workspace: &mapiv1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + VMGroup: "vm-group-1", + }, + }), + }), + infra: infra, + expectedErrors: []string{ + "spec.providerSpec.value.workspace.vmGroup: Invalid value: \"vm-group-1\": vmGroup is not supported in Cluster API", + }, + expectedWarnings: []string{}, + }), + ) + + var _ = DescribeTable("mapi2capi VSphere convert MAPI MachineSet", + func(in vsphereMAPI2CAPIMachinesetConversionInput) { + _, _, warns, err := FromVSphereMachineSetAndInfra(in.machineSetBuilder.Build(), in.infra).ToMachineSetAndMachineTemplate() + Expect(err).To(matchers.ConsistOfMatchErrorSubstrings(in.expectedErrors), "should match expected errors while converting a VSphere MAPI MachineSet to CAPI") + Expect(warns).To(matchers.ConsistOfSubstrings(in.expectedWarnings), "should match expected warnings while converting a VSphere MAPI MachineSet to CAPI") + }, + + Entry("With a Base configuration", vsphereMAPI2CAPIMachinesetConversionInput{ + machineSetBuilder: vsphereMAPIMachineSetBase, + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With tags", vsphereMAPI2CAPIMachinesetConversionInput{ + machineSetBuilder: vsphereMAPIMachineSetBase.WithProviderSpecBuilder( + vsphereBaseProviderSpec.WithTags([]string{ + "urn:vmomi:InventoryServiceTag:5736bf56-49f5-4667-b38c-b97e09dc9578:GLOBAL", + }), + ), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With data disks", vsphereMAPI2CAPIMachinesetConversionInput{ + machineSetBuilder: machinebuilder.MachineSet().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + DataDisks: []mapiv1beta1.VSphereDisk{ + {Name: "disk1", SizeGiB: 100, ProvisioningMode: mapiv1beta1.ProvisioningModeThin}, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With workspace configuration", vsphereMAPI2CAPIMachinesetConversionInput{ + machineSetBuilder: machinebuilder.MachineSet().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Workspace: &mapiv1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + Folder: "/vm/folder", + Datastore: "datastore1", + ResourcePool: "pool1", + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With custom VM configuration", vsphereMAPI2CAPIMachinesetConversionInput{ + machineSetBuilder: machinebuilder.MachineSet().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + NumCPUs: 8, + NumCoresPerSocket: 4, + MemoryMiB: 16384, + DiskGiB: 120, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With empty infrastructure name", vsphereMAPI2CAPIMachinesetConversionInput{ + machineSetBuilder: vsphereMAPIMachineSetBase, + infra: &configv1.Infrastructure{ + Status: configv1.InfrastructureStatus{InfrastructureName: ""}, + }, + expectedErrors: []string{ + // Two errors: one from Machine conversion, one from MachineSet conversion + "infrastructure.status.infrastructureName: Invalid value: \"\": infrastructure cannot be nil and infrastructure.Status.InfrastructureName cannot be empty", + "infrastructure.status.infrastructureName: Invalid value: \"\": infrastructure cannot be nil and infrastructure.Status.InfrastructureName cannot be empty", + }, + expectedWarnings: []string{}, + }), + ) +}) From 96a17716b75f35cd2640432b491d7533c5267fda Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Thu, 5 Feb 2026 13:41:54 -0600 Subject: [PATCH 2/8] Add v1beta2 condition support for infrastructure providers Add functions to handle v1beta2 conditions (used by vSphere provider) while maintaining backward compatibility with v1beta1 conditions (used by AWS, Azure, GCP providers). --- .../machine_migration_controller.go | 2 +- pkg/util/conditions.go | 86 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/pkg/controllers/machinemigration/machine_migration_controller.go b/pkg/controllers/machinemigration/machine_migration_controller.go index 686bc84c6..b3a194a10 100644 --- a/pkg/controllers/machinemigration/machine_migration_controller.go +++ b/pkg/controllers/machinemigration/machine_migration_controller.go @@ -232,7 +232,7 @@ func (r *MachineMigrationReconciler) isOldAuthoritativeResourcePaused(ctx contex return false, fmt.Errorf("failed to get Cluster API infra machine: %w", err) } - infraMachinePausedConditionStatus, err := util.GetConditionStatus(infraMachine, clusterv1.PausedCondition) + infraMachinePausedConditionStatus, err := util.GetConditionStatusFromInfraObject(infraMachine, clusterv1.PausedCondition) if err != nil { return false, fmt.Errorf("unable to get paused condition for %s/%s: %w", infraMachine.GetNamespace(), infraMachine.GetName(), err) } diff --git a/pkg/util/conditions.go b/pkg/util/conditions.go index db01c4150..204650d34 100644 --- a/pkg/util/conditions.go +++ b/pkg/util/conditions.go @@ -33,20 +33,20 @@ import ( ) var ( - errStatusNotMap = errors.New("unable to assert status structure to map") - errStatusConditionsNotInterface = errors.New("unable to assert status.condition structure to interface") + errStatusNotMap = errors.New("unable to assert status structure to map") + errStatusConditionsNotInterface = errors.New("unable to assert status.condition structure to interface") + errStatusV1Beta2NotFound = errors.New("unable to find status.v1beta2 field") + errStatusV1Beta2ConditionsNotInterface = errors.New("unable to assert status.v1beta2.conditions structure to interface") ) // GetCondition retrieves a specific condition from a client.Object. func GetCondition(obj client.Object, conditionType string) (*metav1.Condition, error) { - // Convert client.Object to unstructured.Unstructured unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) if err != nil { return nil, fmt.Errorf("failed to convert object to unstructured: %w", err) } unstructuredObj := &unstructured.Unstructured{Object: unstructuredMap} - data := unstructuredObj.UnstructuredContent() status, ok := data["status"].(map[string]interface{}) @@ -59,6 +59,40 @@ func GetCondition(obj client.Object, conditionType string) (*metav1.Condition, e return nil, errStatusConditionsNotInterface } + return getConditionFromList(conditions, conditionType) +} + +// GetV1Beta2Condition retrieves a specific v1beta2 condition from a client.Object. +// V1Beta2 conditions are stored at status.v1beta2.conditions instead of status.conditions. +func GetV1Beta2Condition(obj client.Object, conditionType string) (*metav1.Condition, error) { + unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + return nil, fmt.Errorf("failed to convert object to unstructured: %w", err) + } + + unstructuredObj := &unstructured.Unstructured{Object: unstructuredMap} + data := unstructuredObj.UnstructuredContent() + + status, ok := data["status"].(map[string]interface{}) + if !ok { + return nil, errStatusNotMap + } + + v1beta2, ok := status["v1beta2"].(map[string]interface{}) + if !ok { + return nil, errStatusV1Beta2NotFound + } + + conditions, ok := v1beta2["conditions"].([]interface{}) + if !ok { + return nil, errStatusV1Beta2ConditionsNotInterface + } + + return getConditionFromList(conditions, conditionType) +} + +// getConditionFromList finds a specific condition in a conditions list. +func getConditionFromList(conditions []interface{}, conditionType string) (*metav1.Condition, error) { for _, c := range conditions { condMap, ok := c.(map[string]interface{}) if !ok { @@ -98,6 +132,50 @@ func GetConditionStatus(obj client.Object, conditionType string) (corev1.Conditi return corev1.ConditionStatus(cond.Status), nil } +// GetV1Beta2ConditionStatus returns the status for the v1beta2 condition. +func GetV1Beta2ConditionStatus(obj client.Object, conditionType string) (corev1.ConditionStatus, error) { + cond, err := GetV1Beta2Condition(obj, conditionType) + if err != nil { + return corev1.ConditionUnknown, fmt.Errorf("unable to get v1beta2 condition %q for the object: %w", conditionType, err) + } + + if cond == nil { + return corev1.ConditionUnknown, nil + } + + return corev1.ConditionStatus(cond.Status), nil +} + +// GetConditionStatusFromInfraObject tries to get condition status from either v1beta2 or v1beta1 conditions. +// It first tries v1beta2 (for providers like vSphere) then falls back to v1beta1 (for AWS, Azure, GCP, etc.). +// This is useful for infrastructure provider objects where the condition location varies by provider. +// If the condition is not found in either location, it returns ConditionUnknown instead of an error. +func GetConditionStatusFromInfraObject(obj client.Object, conditionType string) (corev1.ConditionStatus, error) { + // Try v1beta2 first (for providers like vSphere) + status, err := GetV1Beta2ConditionStatus(obj, conditionType) + if status != corev1.ConditionUnknown { + return status, err + } + + if err != nil && !errors.Is(err, errStatusV1Beta2NotFound) { + return status, err + } + + status, err = GetConditionStatus(obj, conditionType) + if status != corev1.ConditionUnknown { + return status, err + } + + // Both returned Unknown. If the errors are expected (missing fields), return Unknown with no error + // to avoid reconcile failures for providers that simply omit the condition. + // Otherwise, return the real error. + if err != nil && !errors.Is(err, errStatusConditionsNotInterface) { + return corev1.ConditionUnknown, err + } + + return corev1.ConditionUnknown, nil +} + func getString(data map[string]interface{}, key string) string { if val, ok := data[key].(string); ok { return val From 5b378eec4ff41aafe14e50c6f05c5bbadc7c1d3f Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Mon, 16 Feb 2026 13:15:41 -0800 Subject: [PATCH 3/8] Add feature gate check for vSphere migration support Gate vSphere migration functionality behind theClusterAPIMachineManagementVSphere feature gate. The migrationcontrollers for vSphere will now only start when both the generalMachineAPIMigration feature gate and the vSphere-specificClusterAPIMachineManagementVSphere feature gate are enabled. --- cmd/machine-api-migration/main.go | 38 ++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/cmd/machine-api-migration/main.go b/cmd/machine-api-migration/main.go index f66829407..f495c31c9 100644 --- a/cmd/machine-api-migration/main.go +++ b/cmd/machine-api-migration/main.go @@ -93,7 +93,8 @@ func main() { os.Exit(1) } - if err := checkFeatureGates(ctx, log, mgr); err != nil { + currentFeatureGates, err := setupFeatureGates(ctx, log, mgr) + if err != nil { log.Error(err, "unable to check feature gates") os.Exit(1) } @@ -115,7 +116,7 @@ func main() { os.Exit(1) } - checkPlatformSupported(ctx, log, platform) + checkPlatformSupported(ctx, log, platform, currentFeatureGates) for name, controller := range getControllers(operatorConfig, platform, infra, infraTypes) { if err := controller.SetupWithManager(mgr); err != nil { @@ -132,15 +133,15 @@ func main() { } } -func checkFeatureGates(ctx context.Context, log logr.Logger, mgr ctrl.Manager) error { - featureGateAccessor, err := getFeatureGates(ctx, mgr) +func setupFeatureGates(ctx context.Context, log logr.Logger, mgr ctrl.Manager) (featuregates.FeatureGate, error) { + featureGateAccessor, err := getFeatureGatesAccessor(ctx, mgr) if err != nil { - return fmt.Errorf("unable to get feature gates: %w", err) + return nil, fmt.Errorf("unable to get feature gates: %w", err) } currentFeatureGates, err := featureGateAccessor.CurrentFeatureGates() if err != nil { - return fmt.Errorf("unable to get current feature gates: %w", err) + return nil, fmt.Errorf("unable to get current feature gates: %w", err) } if !currentFeatureGates.Enabled(features.FeatureGateMachineAPIMigration) { @@ -148,14 +149,29 @@ func checkFeatureGates(ctx context.Context, log logr.Logger, mgr ctrl.Manager) e exitAfterTerminationSignal(ctx) } - return nil + checkMachineAPIMigrationEnabled(ctx, log, currentFeatureGates) + + return currentFeatureGates, nil +} + +func checkMachineAPIMigrationEnabled(ctx context.Context, log logr.Logger, currentFeatureGates featuregates.FeatureGate) { + if !currentFeatureGates.Enabled(features.FeatureGateMachineAPIMigration) { + log.Info("MachineAPIMigration feature gate is not enabled, nothing to do. Waiting for termination signal.") + exitAfterTerminationSignal(ctx) + } } -func checkPlatformSupported(ctx context.Context, log logr.Logger, platform configv1.PlatformType) { +func checkPlatformSupported(ctx context.Context, log logr.Logger, platform configv1.PlatformType, currentFeatureGates featuregates.FeatureGate) { switch platform { - case configv1.AWSPlatformType, configv1.OpenStackPlatformType, configv1.VSpherePlatformType: - klog.Infof("MachineAPIMigration: starting %s controllers", platform) + case configv1.AWSPlatformType, configv1.OpenStackPlatformType: + log.Info("starting controllers", "platform", platform) + case configv1.VSpherePlatformType: + if !currentFeatureGates.Enabled(features.FeatureGateMachineAPIMigrationVSphere) { + log.Info("MachineAPIMigrationVSphere feature gate is not enabled for vSphere platform. Waiting for termination signal.") + exitAfterTerminationSignal(ctx) + } + log.Info("MachineAPIMigration: starting %s controllers", platform) default: log.Info("MachineAPIMigration not implemented for platform, nothing to do. Waiting for termination signal.", "platform", platform) exitAfterTerminationSignal(ctx) @@ -215,7 +231,7 @@ func exitAfterTerminationSignal(ctx context.Context) { // getFeatureGates is used to fetch the current feature gates from the cluster. // We use this to check if the machine api migration is actually enabled or not. -func getFeatureGates(ctx context.Context, mgr ctrl.Manager) (featuregates.FeatureGateAccess, error) { +func getFeatureGatesAccessor(ctx context.Context, mgr ctrl.Manager) (featuregates.FeatureGateAccess, error) { desiredVersion := util.GetReleaseVersion() missingVersion := "0.0.1-snapshot" From 8785df61ad54a3e24d7b67e6c3a455015eb259eb Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Thu, 2 Apr 2026 16:24:38 -0700 Subject: [PATCH 4/8] Add ValidatingAdmissionPolicy for vSphere unsupported fields Implements admission-time validation to block unsupported vSphere fieldsin VSphereMachine and VSphereMachineTemplate resources, providing clear error messages before conversion attempts. --- admission-policies/vsphere/kustomization.yaml | 5 + .../unsupported-vsphere-spec-fields.yaml | 213 ++++++++++++++++++ 2 files changed, 218 insertions(+) create mode 100644 admission-policies/vsphere/kustomization.yaml create mode 100644 admission-policies/vsphere/unsupported-vsphere-spec-fields.yaml diff --git a/admission-policies/vsphere/kustomization.yaml b/admission-policies/vsphere/kustomization.yaml new file mode 100644 index 000000000..673e8575a --- /dev/null +++ b/admission-policies/vsphere/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: + - unsupported-vsphere-spec-fields.yaml diff --git a/admission-policies/vsphere/unsupported-vsphere-spec-fields.yaml b/admission-policies/vsphere/unsupported-vsphere-spec-fields.yaml new file mode 100644 index 000000000..5b2ba400c --- /dev/null +++ b/admission-policies/vsphere/unsupported-vsphere-spec-fields.yaml @@ -0,0 +1,213 @@ +# ValidatingAdmissionPolicy for blocking unsupported vSphere fields +# +# API Version: v1beta1 +# This policy targets cluster-api-provider-vsphere v1beta1 API. +# When upgrading to v1beta2, add these additional fields to block: +# - nestedHV +# - ftEncryptionMode +# - migrateEncryption +# - cryptoKeyID +# - cryptoProfile +# +# Fields Currently Allowed (conversion supports them): +# - template, cloneMode, snapshot +# - numCPUs, numCoresPerSocket, memoryMiB, diskGiB +# - server, datacenter, folder, datastore, resourcePool +# - tagIDs, dataDisks +# - network.devices[*].networkName +# - network.devices[*].gateway4 (supports IPv6 addresses too) +# - network.devices[*].ipAddrs (supports IPv4 and IPv6) +# - network.devices[*].nameservers (supports IPv4 and IPv6) +# - network.devices[*].addressesFromPools (IPAM) +# +# Fields Warned for Future Enhancement (when MAPI support is added): +# - storagePolicyName (medium priority) +# - hardwareVersion (low priority) +# - pciDevices (low priority) +# - network.devices[*].macAddr (low priority - CAPV supports it but not in MAPI yet) +# +# See docs/vsphere-field-conversion-support.md for complete field documentation. +# +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: "openshift-cluster-api-unsupported-vsphere-spec-fields" +spec: + policyName: "openshift-cluster-api-unsupported-vsphere-spec-fields" + validationActions: [Deny] + matchResources: + namespaceSelector: + matchExpressions: + - key: kubernetes.io/metadata.name + operator: In + values: + - openshift-cluster-api +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: "openshift-cluster-api-unsupported-vsphere-spec-fields-warning" +spec: + policyName: "openshift-cluster-api-unsupported-vsphere-spec-fields-warning" + validationActions: [Warn] + matchResources: + namespaceSelector: + matchExpressions: + - key: kubernetes.io/metadata.name + operator: In + values: + - openshift-cluster-api +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: "openshift-cluster-api-unsupported-vsphere-spec-fields" +spec: + failurePolicy: Fail + matchConstraints: + resourceRules: + - apiGroups: ["infrastructure.cluster.x-k8s.io"] + apiVersions: ["v1beta1"] + operations: ["CREATE", "UPDATE"] + resources: ["vspheremachines", "vspheremachinetemplates"] + variables: + - name: machineSpec + expression: "object.kind == 'VSphereMachine' ? object.spec : object.spec.template.spec" + - name: specPath + expression: "object.kind == 'VSphereMachine' ? 'spec' : 'spec.template.spec'" + validations: + # Cluster-level configuration + - expression: "!has(variables.machineSpec.thumbprint) || variables.machineSpec.thumbprint == ''" + messageExpression: "variables.specPath + '.thumbprint is not supported - TLS validation is handled at cluster level'" + + # CAPI-only runtime configuration + - expression: "!has(variables.machineSpec.customVMXKeys) || variables.machineSpec.customVMXKeys.size() == 0" + messageExpression: "variables.specPath + '.customVMXKeys is not supported in OpenShift'" + + # Resource management (not implemented in CAPV) + - expression: "!has(variables.machineSpec.resources)" + messageExpression: "variables.specPath + '.resources is not supported'" + + # Lifecycle management (not in MAPI) + - expression: "!has(variables.machineSpec.guestSoftPowerOffTimeout)" + messageExpression: "variables.specPath + '.guestSoftPowerOffTimeout is not supported'" + + - expression: "!has(variables.machineSpec.namingStrategy)" + messageExpression: "variables.specPath + '.namingStrategy is not supported'" + + # Power off mode - only allow hard or unset + - expression: "!has(variables.machineSpec.powerOffMode) || variables.machineSpec.powerOffMode == '' || variables.machineSpec.powerOffMode == 'hard'" + messageExpression: "variables.specPath + '.powerOffMode must be \"hard\" or unset - soft power-off is not supported'" + + # Operating system type (not used by CAPV) + - expression: "!has(variables.machineSpec.os) || variables.machineSpec.os == ''" + messageExpression: "variables.specPath + '.os is not supported'" + + # Network-level fields + - expression: "!has(variables.machineSpec.network.routes) || variables.machineSpec.network.routes.size() == 0" + messageExpression: "variables.specPath + '.network.routes is not supported'" + + - expression: "!has(variables.machineSpec.network.preferredAPIServerCIDR) || variables.machineSpec.network.preferredAPIServerCIDR == ''" + messageExpression: "variables.specPath + '.network.preferredAPIServerCIDR is not supported'" + + # Network device fields - iterate over all devices and check unsupported fields + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.deviceName) || device.deviceName == '' + ) + messageExpression: "variables.specPath + '.network.devices[*].deviceName is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.dhcp6) + ) + messageExpression: "variables.specPath + '.network.devices[*].dhcp6 is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.gateway6) || device.gateway6 == '' + ) + messageExpression: "variables.specPath + '.network.devices[*].gateway6 is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.mtu) + ) + messageExpression: "variables.specPath + '.network.devices[*].mtu is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.routes) || device.routes.size() == 0 + ) + messageExpression: "variables.specPath + '.network.devices[*].routes is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.searchDomains) || device.searchDomains.size() == 0 + ) + messageExpression: "variables.specPath + '.network.devices[*].searchDomains is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.dhcp4Overrides) + ) + messageExpression: "variables.specPath + '.network.devices[*].dhcp4Overrides is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.dhcp6Overrides) + ) + messageExpression: "variables.specPath + '.network.devices[*].dhcp6Overrides is not supported'" + + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.skipIPAllocation) + ) + messageExpression: "variables.specPath + '.network.devices[*].skipIPAllocation is not supported'" +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: "openshift-cluster-api-unsupported-vsphere-spec-fields-warning" +spec: + failurePolicy: Fail + matchConstraints: + resourceRules: + - apiGroups: ["infrastructure.cluster.x-k8s.io"] + apiVersions: ["v1beta1"] + operations: ["CREATE", "UPDATE"] + resources: ["vspheremachines", "vspheremachinetemplates"] + variables: + - name: machineSpec + expression: "object.kind == 'VSphereMachine' ? object.spec : object.spec.template.spec" + - name: specPath + expression: "object.kind == 'VSphereMachine' ? 'spec' : 'spec.template.spec'" + validations: + # Storage policy (not yet supported - future enhancement) + - expression: "!has(variables.machineSpec.storagePolicyName) || variables.machineSpec.storagePolicyName == ''" + messageExpression: "variables.specPath + '.storagePolicyName is not yet supported but planned for future release'" + + # PCI devices (not yet supported - future enhancement) + - expression: "!has(variables.machineSpec.pciDevices) || variables.machineSpec.pciDevices.size() == 0" + messageExpression: "variables.specPath + '.pciDevices is not yet supported but planned for future release'" + + # Hardware version (not yet supported - future enhancement) + - expression: "!has(variables.machineSpec.hardwareVersion) || variables.machineSpec.hardwareVersion == ''" + messageExpression: "variables.specPath + '.hardwareVersion is not yet supported but planned for future release'" + + # MAC address (not yet supported - future enhancement) + - expression: >- + !has(variables.machineSpec.network.devices) || + variables.machineSpec.network.devices.all(device, + !has(device.macAddr) || device.macAddr == '' + ) + messageExpression: "variables.specPath + '.network.devices[*].macAddr is not yet supported but planned for future release'" From 05296f9c9a6aeb3239c68762e5236d717ddfb175 Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Mon, 6 Apr 2026 11:37:39 -0700 Subject: [PATCH 5/8] Convert vSphere ProviderStatus during CAPI to MAPI conversion Align vSphere conversion with AWS implementation by converting and setting ProviderStatus during CAPI to MAPI conversion to ensure proper status synchronization between the two APIs. --- .../machine_sync_capi2mapi_infrastructure.go | 30 ++++ pkg/conversion/capi2mapi/vsphere.go | 62 +++++++- pkg/conversion/capi2mapi/vsphere_test.go | 141 ++++++++++++++++++ pkg/conversion/mapi2capi/vsphere.go | 14 ++ 4 files changed, 243 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/machinesync/machine_sync_capi2mapi_infrastructure.go b/pkg/controllers/machinesync/machine_sync_capi2mapi_infrastructure.go index 6d8ea2ea9..1467020b3 100644 --- a/pkg/controllers/machinesync/machine_sync_capi2mapi_infrastructure.go +++ b/pkg/controllers/machinesync/machine_sync_capi2mapi_infrastructure.go @@ -24,9 +24,12 @@ import ( "github.com/openshift/cluster-capi-operator/pkg/conversion/capi2mapi" "github.com/openshift/cluster-capi-operator/pkg/conversion/mapi2capi" "github.com/openshift/cluster-capi-operator/pkg/util" + "k8s.io/utils/ptr" ) // setChangedMAPIMachineProviderStatusFields unmarshals the existing and converted ProviderStatus, copies over the fields and marshals it back to the existingMAPIMachine. +// +//nolint:funlen func setChangedMAPIMachineProviderStatusFields(platform configv1.PlatformType, existingMAPIMachine, convertedMAPIMachine *mapiv1beta1.Machine) error { var newProviderStatus interface{} @@ -48,6 +51,33 @@ func setChangedMAPIMachineProviderStatusFields(platform configv1.PlatformType, e convertedStatus.Conditions = existingStatus.Conditions + newProviderStatus = convertedStatus + case configv1.VSpherePlatformType: + existingStatus, err := mapi2capi.VSphereProviderStatusFromRawExtension(existingMAPIMachine.Status.ProviderStatus) + if err != nil { + return fmt.Errorf("unable to convert RawExtension to VSphere ProviderStatus: %w", err) + } + + convertedStatus, err := mapi2capi.VSphereProviderStatusFromRawExtension(convertedMAPIMachine.Status.ProviderStatus) + if err != nil { + return fmt.Errorf("unable to convert RawExtension to VSphere ProviderStatus: %w", err) + } + + for i := range convertedStatus.Conditions { + existingStatus.Conditions = util.SetMAPIProviderCondition(existingStatus.Conditions, &convertedStatus.Conditions[i]) + } + + convertedStatus.Conditions = existingStatus.Conditions + + // CAPI→MAPI conversion does not set instanceId or taskRef; preserve actuator-populated values. + if ptr.Deref(convertedStatus.InstanceID, "") == "" { + convertedStatus.InstanceID = existingStatus.InstanceID + } + + if convertedStatus.TaskRef == "" { + convertedStatus.TaskRef = existingStatus.TaskRef + } + newProviderStatus = convertedStatus case configv1.OpenStackPlatformType: // TODO(openstack): implement diff --git a/pkg/conversion/capi2mapi/vsphere.go b/pkg/conversion/capi2mapi/vsphere.go index e52db5d82..42cdffeae 100644 --- a/pkg/conversion/capi2mapi/vsphere.go +++ b/pkg/conversion/capi2mapi/vsphere.go @@ -24,6 +24,7 @@ import ( "github.com/openshift/cluster-capi-operator/pkg/conversion/consts" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" @@ -115,10 +116,13 @@ func (m machineAndVSphereMachineAndVSphereCluster) ToMachine() (*mapiv1beta1.Mac } mapiMachine.Spec.ProviderSpec.Value = vsphereSpecRawExt - // Note: ProviderStatus is not set during conversion, similar to OpenStack and PowerVS providers. - // During the migration period when MAPI is authoritative, the MAPI controller manages the status. - // When CAPI becomes authoritative, the ProviderStatus will remain stale but unused, as status - // will be tracked in the CAPI VSphereMachine status instead. + + vsphereStatusRawExt, errRaw := RawExtensionFromInterface(m.toProviderStatus()) + if errRaw != nil { + return nil, nil, fmt.Errorf("unable to convert vSphere providerStatus to raw extension: %w", errRaw) + } + + mapiMachine.Status.ProviderStatus = vsphereStatusRawExt if len(errs) > 0 { return nil, warning, errs.ToAggregate() @@ -127,6 +131,13 @@ func (m machineAndVSphereMachineAndVSphereCluster) ToMachine() (*mapiv1beta1.Mac return mapiMachine, warning, nil } +func (m machineAndVSphereMachineAndVSphereCluster) toProviderStatus() *mapiv1beta1.VSphereMachineProviderStatus { + return &mapiv1beta1.VSphereMachineProviderStatus{ + InstanceState: ptr.To(m.getInstanceState()), + Conditions: convertCAPVMachineConditionsToMAPIVSphereMachineProviderConditions(m.vSphereMachine), + } +} + // ToMachineSet converts a capi2mapi MachineSetAndVSphereMachineTemplate into a MAPI MachineSet. func (m machineSetAndVSphereMachineTemplateAndVSphereCluster) ToMachineSet() (*mapiv1beta1.MachineSet, []string, error) { if m.machineSet == nil || m.vSphereMachineTemplate == nil || m.vSphereCluster == nil || m.machineAndVSphereMachineAndVSphereCluster == nil { @@ -389,6 +400,49 @@ func (m machineAndVSphereMachineAndVSphereCluster) getInstanceState() string { return "not-ready" } +func convertCAPVMachineConditionsToMAPIVSphereMachineProviderConditions(vSphereMachine *vspherev1.VSphereMachine) []metav1.Condition { + message := "" + + // Try to extract a meaningful message from v1beta2 conditions + if vSphereMachine.Status.V1Beta2 != nil { + // Prefer VirtualMachineProvisioned condition as it has more detailed status + if cond := meta.FindStatusCondition(vSphereMachine.Status.V1Beta2.Conditions, vspherev1.VSphereMachineVirtualMachineProvisionedV1Beta2Condition); cond != nil && cond.Message != "" { + message = cond.Message + } else if cond := meta.FindStatusCondition(vSphereMachine.Status.V1Beta2.Conditions, vspherev1.VSphereMachineReadyV1Beta2Condition); cond != nil && cond.Message != "" { + // Fall back to Ready condition if VirtualMachineProvisioned doesn't have a message + message = cond.Message + } + } + + if vSphereMachine.Status.Ready { + // Set conditionSuccess + if message == "" { + message = "Machine successfully created" + } + + return []metav1.Condition{{ + Type: string(mapiv1beta1.MachineCreation), + Status: metav1.ConditionTrue, + Reason: mapiv1beta1.MachineCreationSucceededConditionReason, + Message: message, + // LastTransitionTime will be set by the condition utilities. + }} + } + + // Set conditionFailed + if message == "" { + message = "See VSphereMachine conditions." + } + + return []metav1.Condition{{ + Type: string(mapiv1beta1.MachineCreation), + Status: metav1.ConditionFalse, + Reason: mapiv1beta1.MachineCreationFailedConditionReason, + Message: message, + // LastTransitionTime will be set by the condition utilities. + }} +} + // convertCAPVCloneModeToMAPI converts CAPV CloneMode to MAPI CloneMode. func convertCAPVCloneModeToMAPI(fldPath *field.Path, capvMode vspherev1.CloneMode) (mapiv1beta1.CloneMode, *field.Error) { switch capvMode { diff --git a/pkg/conversion/capi2mapi/vsphere_test.go b/pkg/conversion/capi2mapi/vsphere_test.go index 108f59bb2..724a177f9 100644 --- a/pkg/conversion/capi2mapi/vsphere_test.go +++ b/pkg/conversion/capi2mapi/vsphere_test.go @@ -16,11 +16,15 @@ limitations under the License. package capi2mapi import ( + "encoding/json" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + mapiv1beta1 "github.com/openshift/api/machine/v1beta1" "github.com/openshift/cluster-capi-operator/pkg/conversion/test/matchers" "k8s.io/utils/ptr" vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -397,4 +401,141 @@ var _ = Describe("capi2mapi vSphere conversion", func() { expectedWarnings: []string{}, }), ) + + // ProviderStatus on the MAPI Machine is populated during CAPI→MAPI conversion (see ToMachine / toProviderStatus). + // Full infra status round-trip requires MAPI→CAPI to read ProviderStatus into VSphereMachine.Status (AWS does this; + // vSphere does not yet). These tests lock in the CAPI→MAPI propagation contract. + DescribeTable("ToMachine sets status.providerStatus from VSphereMachine.Status", + func(vmStatus vspherev1.VSphereMachineStatus, wantInstanceState string, wantCreationStatus metav1.ConditionStatus, wantMessage string) { + vm := vsphereCAPIVSphereMachineBase.DeepCopy() + vm.Status = vmStatus + + mapiMachine, warns, err := FromMachineAndVSphereMachineAndVSphereCluster(vsphereCAPIMachineBase, vm, vsphereCAPIVSphereClusterBase).ToMachine() + Expect(err).NotTo(HaveOccurred()) + Expect(warns).To(BeEmpty()) + Expect(mapiMachine.Status.ProviderStatus).NotTo(BeNil()) + + var got mapiv1beta1.VSphereMachineProviderStatus + Expect(json.Unmarshal(mapiMachine.Status.ProviderStatus.Raw, &got)).To(Succeed()) + Expect(ptr.Deref(got.InstanceState, "")).To(Equal(wantInstanceState)) + Expect(got.Conditions).To(HaveLen(1)) + Expect(got.Conditions[0].Type).To(Equal(string(mapiv1beta1.MachineCreation))) + Expect(got.Conditions[0].Status).To(Equal(wantCreationStatus)) + Expect(got.Conditions[0].Message).To(Equal(wantMessage)) + + if wantCreationStatus == metav1.ConditionTrue { + Expect(got.Conditions[0].Reason).To(Equal(mapiv1beta1.MachineCreationSucceededConditionReason)) + } else { + Expect(got.Conditions[0].Reason).To(Equal(mapiv1beta1.MachineCreationFailedConditionReason)) + } + }, + Entry("no addresses: empty instance state, creation not succeeded", + vspherev1.VSphereMachineStatus{Ready: false, Addresses: nil}, + "", + metav1.ConditionFalse, + "See VSphereMachine conditions.", + ), + Entry("provisioned but not ready: not-ready, creation not succeeded", + vspherev1.VSphereMachineStatus{ + Ready: false, + Addresses: []clusterv1beta1.MachineAddress{ + {Type: clusterv1beta1.MachineHostName, Address: "vm-1"}, + }, + }, + "not-ready", + metav1.ConditionFalse, + "See VSphereMachine conditions.", + ), + Entry("provisioned and ready: ready, creation succeeded", + vspherev1.VSphereMachineStatus{ + Ready: true, + Addresses: []clusterv1beta1.MachineAddress{ + {Type: clusterv1beta1.MachineInternalIP, Address: "10.0.0.1"}, + }, + }, + "ready", + metav1.ConditionTrue, + "Machine successfully created", + ), + Entry("ready with v1beta2 VirtualMachineProvisioned message", + vspherev1.VSphereMachineStatus{ + Ready: true, + Addresses: []clusterv1beta1.MachineAddress{ + {Type: clusterv1beta1.MachineInternalIP, Address: "10.0.0.1"}, + }, + V1Beta2: &vspherev1.VSphereMachineV1Beta2Status{ + Conditions: []metav1.Condition{ + { + Type: vspherev1.VSphereMachineVirtualMachineProvisionedV1Beta2Condition, + Status: metav1.ConditionTrue, + Message: "Virtual machine is provisioned", + }, + }, + }, + }, + "ready", + metav1.ConditionTrue, + "Virtual machine is provisioned", + ), + Entry("ready with v1beta2 Ready message (VirtualMachineProvisioned fallback)", + vspherev1.VSphereMachineStatus{ + Ready: true, + Addresses: []clusterv1beta1.MachineAddress{ + {Type: clusterv1beta1.MachineInternalIP, Address: "10.0.0.1"}, + }, + V1Beta2: &vspherev1.VSphereMachineV1Beta2Status{ + Conditions: []metav1.Condition{ + { + Type: vspherev1.VSphereMachineReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Message: "Machine is ready", + }, + }, + }, + }, + "ready", + metav1.ConditionTrue, + "Machine is ready", + ), + Entry("not ready with v1beta2 VirtualMachineProvisioned error message", + vspherev1.VSphereMachineStatus{ + Ready: false, + Addresses: []clusterv1beta1.MachineAddress{ + {Type: clusterv1beta1.MachineHostName, Address: "vm-1"}, + }, + V1Beta2: &vspherev1.VSphereMachineV1Beta2Status{ + Conditions: []metav1.Condition{ + { + Type: vspherev1.VSphereMachineVirtualMachineProvisionedV1Beta2Condition, + Status: metav1.ConditionFalse, + Message: "VM configuration failed: network error", + }, + }, + }, + }, + "not-ready", + metav1.ConditionFalse, + "VM configuration failed: network error", + ), + Entry("not ready with v1beta2 Ready error message (fallback)", + vspherev1.VSphereMachineStatus{ + Ready: false, + Addresses: []clusterv1beta1.MachineAddress{ + {Type: clusterv1beta1.MachineHostName, Address: "vm-1"}, + }, + V1Beta2: &vspherev1.VSphereMachineV1Beta2Status{ + Conditions: []metav1.Condition{ + { + Type: vspherev1.VSphereMachineReadyV1Beta2Condition, + Status: metav1.ConditionFalse, + Message: "Waiting for cluster infrastructure", + }, + }, + }, + }, + "not-ready", + metav1.ConditionFalse, + "Waiting for cluster infrastructure", + ), + ) }) diff --git a/pkg/conversion/mapi2capi/vsphere.go b/pkg/conversion/mapi2capi/vsphere.go index ecb538b3d..8099161c3 100644 --- a/pkg/conversion/mapi2capi/vsphere.go +++ b/pkg/conversion/mapi2capi/vsphere.go @@ -321,6 +321,20 @@ func vSphereProviderSpecFromRawExtension(rawExtension *runtime.RawExtension) (ma return spec, nil } +// VSphereProviderStatusFromRawExtension unmarshals a raw extension into a VSphereMachineProviderStatus. +func VSphereProviderStatusFromRawExtension(rawExtension *runtime.RawExtension) (mapiv1beta1.VSphereMachineProviderStatus, error) { + if rawExtension == nil { + return mapiv1beta1.VSphereMachineProviderStatus{}, nil + } + + status := mapiv1beta1.VSphereMachineProviderStatus{} + if err := yaml.Unmarshal(rawExtension.Raw, &status); err != nil { + return mapiv1beta1.VSphereMachineProviderStatus{}, fmt.Errorf("error unmarshalling providerStatus %q: %w", string(rawExtension.Raw), err) + } + + return status, nil +} + // vSphereMachineToVSphereMachineTemplate converts a VSphereMachine to a VSphereMachineTemplate. func vSphereMachineToVSphereMachineTemplate(vSphereMachine *vspherev1.VSphereMachine, name string, namespace string) (*vspherev1.VSphereMachineTemplate, error) { nameWithHash, err := util.GenerateInfraMachineTemplateNameWithSpecHash(name, vSphereMachine.Spec) From b0f95cbb54c4e258b7d720c1b4ca4def744f48b4 Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Mon, 20 Apr 2026 13:07:03 -0700 Subject: [PATCH 6/8] squash! Enable vSphere in MAPI<->CAPI conversion framework --- pkg/conversion/mapi2capi/vsphere.go | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/pkg/conversion/mapi2capi/vsphere.go b/pkg/conversion/mapi2capi/vsphere.go index 8099161c3..f7f392853 100644 --- a/pkg/conversion/mapi2capi/vsphere.go +++ b/pkg/conversion/mapi2capi/vsphere.go @@ -227,7 +227,6 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe capiCloneMode := convertVSphereCloneModeMAPIToCAPI(providerSpec.CloneMode) spec := vspherev1.VSphereMachineSpec{ - PowerOffMode: vspherev1.VirtualMachinePowerOpModeHard, VirtualMachineCloneSpec: vspherev1.VirtualMachineCloneSpec{ Template: providerSpec.Template, CloneMode: capiCloneMode, @@ -239,11 +238,11 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe TagIDs: providerSpec.TagIDs, Network: capiNetworkSpec, DataDisks: capiDataDisks, - // Server - Set below from Workspace. - // Datacenter - Set below from Workspace. - // Folder - Set below from Workspace. - // Datastore - Set below from Workspace. - // ResourcePool - Set below from Workspace. + // Server - Set below from Workspace if present; otherwise inherited from cluster-level VSphereCluster or failure domain. + // Datacenter - Set below from Workspace if present; otherwise inherited from failure domain. + // Folder - Set below from Workspace if present; otherwise inherited from failure domain. + // Datastore - Set below from Workspace if present; otherwise inherited from failure domain. + // ResourcePool - Set below from Workspace if present; otherwise inherited from failure domain. // Thumbprint - Not supported in MAPI. // StoragePolicyName - Not supported in MAPI. // Resources - Not supported in MAPI. @@ -255,18 +254,33 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe }, // ProviderID - Set at a higher level in ToMachine(). // FailureDomain - Set at a higher level from machine.Spec.FailureDomain. + // PowerOffMode - Not set; CAPV defaults to "hard" which matches MAPI behavior. MAPI has no PowerOffMode field. // GuestSoftPowerOffTimeout - Not supported in MAPI. // NamingStrategy - Not supported in MAPI. } // Set workspace fields if available if providerSpec.Workspace != nil { + // Validate that workspace has required fields populated. + // CPMS-style empty workspace (for failure domain-based topology) is not yet implemented. + // Implementation would require creating VSphereDeploymentZone resources from Infrastructure.FailureDomains. + // For now, machines must have workspace.server and workspace.datacenter populated. + if providerSpec.Workspace.Server == "" { + errs = append(errs, field.Required(fldPath.Child("workspace", "server"), "workspace.server is required. Control Plane Machine Set (CPMS) with failure domain-based topology is not yet implemented for vSphere")) + } + if providerSpec.Workspace.Datacenter == "" { + errs = append(errs, field.Required(fldPath.Child("workspace", "datacenter"), "workspace.datacenter is required. Control Plane Machine Set (CPMS) with failure domain-based topology is not yet implemented for vSphere")) + } + spec.Server = providerSpec.Workspace.Server spec.Datacenter = providerSpec.Workspace.Datacenter spec.Folder = providerSpec.Workspace.Folder spec.Datastore = providerSpec.Workspace.Datastore spec.ResourcePool = providerSpec.Workspace.ResourcePool // VMGroup - MAPI-specific field for vm-host group based zoning, not supported in CAPV. + } else { + // Workspace is nil - this pattern is used by CPMS with failure domains, which is not yet implemented. + errs = append(errs, field.Required(fldPath.Child("workspace"), "workspace is required. Control Plane Machine Set (CPMS) with failure domain-based topology is not yet implemented for vSphere")) } // Unused fields - Below this line are fields not used from the MAPI VSphereMachineProviderSpec. @@ -283,7 +297,6 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe // Only take action when a non-default credentials secret is being used in MAPI. // If the user is using the default, then their CAPV secret will already be configured and no action is necessary. if providerSpec.CredentialsSecret != nil && - providerSpec.CredentialsSecret.Name != "" && providerSpec.CredentialsSecret.Name != DefaultVSphereCredentialsSecretName { // Not convertable; need custom credential configuration errs = append(errs, field.Invalid(fldPath.Child("credentialsSecret"), providerSpec.CredentialsSecret.Name, fmt.Sprintf("credentialsSecret does not match the default of %q, credentials must be configured at the cluster level", DefaultVSphereCredentialsSecretName))) From c3f73902615a65fd0b6d7f372390084996c55711 Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Mon, 20 Apr 2026 13:19:25 -0700 Subject: [PATCH 7/8] squash! Convert vSphere ProviderStatus during CAPI to MAPI conversion --- pkg/conversion/capi2mapi/vsphere.go | 140 +++++++++++------- pkg/conversion/capi2mapi/vsphere_test.go | 8 +- pkg/conversion/consts/consts.go | 3 + pkg/conversion/mapi2capi/vsphere.go | 100 ++++++++++--- pkg/conversion/mapi2capi/vsphere_fuzz_test.go | 62 +++++++- pkg/conversion/mapi2capi/vsphere_test.go | 68 ++++++++- 6 files changed, 302 insertions(+), 79 deletions(-) diff --git a/pkg/conversion/capi2mapi/vsphere.go b/pkg/conversion/capi2mapi/vsphere.go index 42cdffeae..8af93a511 100644 --- a/pkg/conversion/capi2mapi/vsphere.go +++ b/pkg/conversion/capi2mapi/vsphere.go @@ -42,9 +42,20 @@ const ( errUnsupportedCAPVProvisioningMode = "unable to convert provisioning mode, unknown value" errUnsupportedCAPVCloneMode = "unable to convert clone mode, unknown value" vsphereCredentialsSecretName = "vsphere-cloud-credentials" //#nosec G101 -- False positive, not actually a credential. - ) +// ipamKindToResource maps IPAM API group and Kind names (singular) to resource names (plural). +// This is the reverse mapping of ipamResourceToKind (from mapi2capi), used for CAPI→MAPI conversion. +// +//nolint:gochecknoglobals // Lookup table for IPAM Kind name to resource name conversion +var ipamKindToResource = map[string]map[string]string{ + "ipam.cluster.x-k8s.io": { + "InClusterIPPool": "inclusterippools", + "GlobalInClusterIPPool": "globalinclusterippools", + }, + // Additional IPAM providers can be added here as needed +} + // machineAndVSphereMachineAndVSphereCluster stores the details of a Cluster API Machine and VSphereMachine and VSphereCluster. type machineAndVSphereMachineAndVSphereCluster struct { machine *clusterv1.Machine @@ -108,7 +119,6 @@ func (m machineAndVSphereMachineAndVSphereCluster) ToMachine() (*mapiv1beta1.Mac } additionalMachineAPIMetadataLabels, additionalMachineAPIMetadataAnnotations := m.buildAdditionalMetadata() - mapiMachine, err := fromCAPIMachineToMAPIMachine(m.machine, additionalMachineAPIMetadataLabels, additionalMachineAPIMetadataAnnotations) if err != nil { @@ -148,21 +158,18 @@ func (m machineSetAndVSphereMachineTemplateAndVSphereCluster) ToMachineSet() (*m // Run the full ToMachine conversion so that we can check for // any Machine level conversion errs in the spec translation. - mapiMachine, warn, err := m.ToMachine() + mapiMachine, warning, err := m.ToMachine() if err != nil { errs = append(errs, err) } - warnings := make([]string, 0, len(warn)) - warnings = append(warnings, warn...) - mapiMachineSet, err := fromCAPIMachineSetToMAPIMachineSet(m.machineSet) if err != nil { errs = append(errs, err) } if len(errs) > 0 { - return nil, warnings, utilerrors.NewAggregate(errs) + return nil, warning, utilerrors.NewAggregate(errs) } mapiMachineSet.Spec.Template.Spec = mapiMachine.Spec @@ -174,7 +181,7 @@ func (m machineSetAndVSphereMachineTemplateAndVSphereCluster) ToMachineSet() (*m mapiMachineSet.Spec.Template.ObjectMeta.Annotations = mapiMachine.ObjectMeta.Annotations //nolint:staticcheck // ObjectMeta is a field, not embedded mapiMachineSet.Spec.Template.ObjectMeta.Labels = mapiMachine.ObjectMeta.Labels //nolint:staticcheck // ObjectMeta is a field, not embedded - return mapiMachineSet, warnings, nil + return mapiMachineSet, warning, nil } // toProviderSpec converts a capi2mapi MachineAndVSphereMachineTemplateAndVSphereCluster into a MAPI VSphereMachineProviderSpec. @@ -198,7 +205,7 @@ func (m machineAndVSphereMachineAndVSphereCluster) toProviderSpec() (*mapiv1beta } // Convert data disks - // AdditionalDisksGiB is deprecated in CAPV in favor of DataDisks. + // AdditionalDisksGiB is deprecated in CoAPV in favor of DataDisks. // If AdditionalDisksGiB is set, convert it to DataDisks format first. allDataDisks := m.vSphereMachine.Spec.DataDisks if len(m.vSphereMachine.Spec.AdditionalDisksGiB) > 0 { @@ -364,17 +371,27 @@ func (m machineAndVSphereMachineAndVSphereCluster) buildAdditionalMetadata() (ma } } + //nolint:nestif // Nested logic is acceptable for conditionally building metadata maps if !m.excludeMachineAPILabelsAndAnnotations { - if additionalMachineAPIMetadataLabels == nil { - additionalMachineAPIMetadataLabels = make(map[string]string) - } // For vSphere, we use template name as instance type and datacenter as region - additionalMachineAPIMetadataLabels[consts.MAPIMachineMetadataLabelInstanceType] = m.vSphereMachine.Spec.Template - additionalMachineAPIMetadataLabels[consts.MAPIMachineMetadataLabelRegion] = m.vSphereMachine.Spec.Datacenter + // Only add these labels if they have non-empty values + if m.vSphereMachine.Spec.Template != "" || m.vSphereMachine.Spec.Datacenter != "" { + if additionalMachineAPIMetadataLabels == nil { + additionalMachineAPIMetadataLabels = make(map[string]string) + } - // Get instance state from VM status - use empty string if VM is not yet provisioned - instanceState := m.getInstanceState() + if m.vSphereMachine.Spec.Template != "" { + additionalMachineAPIMetadataLabels[consts.MAPIMachineMetadataLabelInstanceType] = m.vSphereMachine.Spec.Template + } + if m.vSphereMachine.Spec.Datacenter != "" { + additionalMachineAPIMetadataLabels[consts.MAPIMachineMetadataLabelRegion] = m.vSphereMachine.Spec.Datacenter + } + } + + // Get instance state from VM status and set annotation. + // Always set the annotation to match AWS behavior. + instanceState := m.getInstanceState() additionalMachineAPIMetadataAnnotations = map[string]string{ consts.MAPIMachineMetadataAnnotationInstanceState: instanceState, } @@ -384,20 +401,14 @@ func (m machineAndVSphereMachineAndVSphereCluster) buildAdditionalMetadata() (ma } // getInstanceState determines the instance state from the VSphereMachine status. -// Returns empty string if VM is not yet provisioned, "ready" if provisioned and ready, -// or "not-ready" if provisioned but not ready. -// This matches behavior of other providers (AWS, OpenStack, PowerVS). +// Returns "ready" if Ready is true, otherwise empty string. +// This matches AWS's pattern where instance state is derived from a single boolean field. func (m machineAndVSphereMachineAndVSphereCluster) getInstanceState() string { - // We check if addresses are set to determine if the VM has been provisioned - if len(m.vSphereMachine.Status.Addresses) == 0 { - return "" - } - if m.vSphereMachine.Status.Ready { - return "ready" + return consts.VSphereInstanceStateReady } - return "not-ready" + return "" } func convertCAPVMachineConditionsToMAPIVSphereMachineProviderConditions(vSphereMachine *vspherev1.VSphereMachine) []metav1.Condition { @@ -472,40 +483,65 @@ func convertCAPVNetworkSpecToMAPI(_ *field.Path, capvNetwork vspherev1.NetworkSp if len(capvNetwork.Devices) > 0 { devices = make([]mapiv1beta1.NetworkDeviceSpec, len(capvNetwork.Devices)) for i, device := range capvNetwork.Devices { - devices[i] = mapiv1beta1.NetworkDeviceSpec{ - NetworkName: device.NetworkName, - Gateway: device.Gateway4, // Map IPv4 gateway - IPAddrs: device.IPAddrs, - Nameservers: device.Nameservers, - } + devices[i], warnings = convertCAPVNetworkDevice(device, i, warnings) + } + } - // Convert AddressesFromPools - if len(device.AddressesFromPools) > 0 { - addressesFromPools := make([]mapiv1beta1.AddressesFromPool, len(device.AddressesFromPools)) - for j, pool := range device.AddressesFromPools { - addressesFromPools[j] = mapiv1beta1.AddressesFromPool{ - Group: ptr.Deref(pool.APIGroup, ""), - Resource: pool.Kind, // This might need adjustment based on actual mapping - Name: pool.Name, - } - } + return mapiv1beta1.NetworkSpec{ + Devices: devices, + }, warnings, errs +} - devices[i].AddressesFromPools = addressesFromPools - } +// convertCAPVNetworkDevice converts a single CAPV NetworkDeviceSpec to MAPI. +func convertCAPVNetworkDevice(device vspherev1.NetworkDeviceSpec, index int, warnings []string) (mapiv1beta1.NetworkDeviceSpec, []string) { + mapiDevice := mapiv1beta1.NetworkDeviceSpec{ + NetworkName: device.NetworkName, + Gateway: device.Gateway4, // Map IPv4 gateway + IPAddrs: device.IPAddrs, + Nameservers: device.Nameservers, + } + + mapiDevice.AddressesFromPools = convertCAPVAddressesFromPools(device.AddressesFromPools) + + // Note: DHCP settings are not directly represented in MAPI NetworkDeviceSpec + // The presence of DHCP4/DHCP6 in CAPV is inferred from the absence of static IPs + if device.DHCP4 || device.DHCP6 { + if len(device.IPAddrs) > 0 { + warnings = append(warnings, fmt.Sprintf("device %d has both DHCP and static IPs configured, MAPI will use static IPs", index)) + } + } - // Note: DHCP settings are not directly represented in MAPI NetworkDeviceSpec - // The presence of DHCP4/DHCP6 in CAPV is inferred from the absence of static IPs - if device.DHCP4 || device.DHCP6 { - if len(device.IPAddrs) > 0 { - warnings = append(warnings, fmt.Sprintf("device %d has both DHCP and static IPs configured, MAPI will use static IPs", i)) + return mapiDevice, warnings +} + +// convertCAPVAddressesFromPools converts CAPV AddressesFromPools to MAPI. +func convertCAPVAddressesFromPools(capvPools []corev1.TypedLocalObjectReference) []mapiv1beta1.AddressesFromPool { + if len(capvPools) == 0 { + return nil + } + + mapiPools := make([]mapiv1beta1.AddressesFromPool, len(capvPools)) + for i, pool := range capvPools { + group := ptr.Deref(pool.APIGroup, "") + resource := pool.Kind // default to Kind if no mapping found + + // Convert Kind name to resource name (e.g., "InClusterIPPool" → "inclusterippools") + if group != "" { + if groupMappings, ok := ipamKindToResource[group]; ok { + if mappedResource, ok := groupMappings[pool.Kind]; ok { + resource = mappedResource } } } + + mapiPools[i] = mapiv1beta1.AddressesFromPool{ + Group: group, + Resource: resource, + Name: pool.Name, + } } - return mapiv1beta1.NetworkSpec{ - Devices: devices, - }, warnings, errs + return mapiPools } // convertCAPVDataDisksToMAPI converts CAPV DataDisks to MAPI DataDisks. diff --git a/pkg/conversion/capi2mapi/vsphere_test.go b/pkg/conversion/capi2mapi/vsphere_test.go index 724a177f9..c2d599512 100644 --- a/pkg/conversion/capi2mapi/vsphere_test.go +++ b/pkg/conversion/capi2mapi/vsphere_test.go @@ -435,14 +435,14 @@ var _ = Describe("capi2mapi vSphere conversion", func() { metav1.ConditionFalse, "See VSphereMachine conditions.", ), - Entry("provisioned but not ready: not-ready, creation not succeeded", + Entry("provisioned but not ready: empty instance state, creation not succeeded", vspherev1.VSphereMachineStatus{ Ready: false, Addresses: []clusterv1beta1.MachineAddress{ {Type: clusterv1beta1.MachineHostName, Address: "vm-1"}, }, }, - "not-ready", + "", metav1.ConditionFalse, "See VSphereMachine conditions.", ), @@ -513,7 +513,7 @@ var _ = Describe("capi2mapi vSphere conversion", func() { }, }, }, - "not-ready", + "", metav1.ConditionFalse, "VM configuration failed: network error", ), @@ -533,7 +533,7 @@ var _ = Describe("capi2mapi vSphere conversion", func() { }, }, }, - "not-ready", + "", metav1.ConditionFalse, "Waiting for cluster infrastructure", ), diff --git a/pkg/conversion/consts/consts.go b/pkg/conversion/consts/consts.go index 531f66476..1c280171b 100644 --- a/pkg/conversion/consts/consts.go +++ b/pkg/conversion/consts/consts.go @@ -27,4 +27,7 @@ const ( // MAPIMachineMetadataAnnotationInstanceState is the annotation for the instance state of the machine is used as column for kubectl. MAPIMachineMetadataAnnotationInstanceState = "machine.openshift.io/instance-state" + + // VSphereInstanceStateReady indicates the vSphere instance is ready. + VSphereInstanceStateReady = "ready" ) diff --git a/pkg/conversion/mapi2capi/vsphere.go b/pkg/conversion/mapi2capi/vsphere.go index f7f392853..75f35e33a 100644 --- a/pkg/conversion/mapi2capi/vsphere.go +++ b/pkg/conversion/mapi2capi/vsphere.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,6 +43,18 @@ const ( vSphereMachineTemplateKind = "VSphereMachineTemplate" ) +// ipamResourceToKind maps IPAM API group and resource names (plural) to Kind names (singular). +// MAPI uses resource names while CAPI requires Kind names. +// +//nolint:gochecknoglobals // Lookup table for IPAM resource name to Kind name conversion +var ipamResourceToKind = map[string]map[string]string{ + "ipam.cluster.x-k8s.io": { + "inclusterippools": "InClusterIPPool", + "globalinclusterippools": "GlobalInClusterIPPool", + }, + // Additional IPAM providers can be added here as needed +} + // vSphereMachineAndInfra stores the details of a Machine API VSphereMachine and Infra. type vSphereMachineAndInfra struct { machine *mapiv1beta1.Machine @@ -254,33 +267,22 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe }, // ProviderID - Set at a higher level in ToMachine(). // FailureDomain - Set at a higher level from machine.Spec.FailureDomain. - // PowerOffMode - Not set; CAPV defaults to "hard" which matches MAPI behavior. MAPI has no PowerOffMode field. + // PowerOffMode must be set explicitly to match CAPV's default behavior. If not set, CAPV's webhook + // defaults it to "hard", causing the differ to detect a spec change and trigger an infinite + // delete/recreate loop. MAPI has no PowerOffMode field, so "hard" matches MAPI's power-off behavior. + PowerOffMode: vspherev1.VirtualMachinePowerOpModeHard, // GuestSoftPowerOffTimeout - Not supported in MAPI. // NamingStrategy - Not supported in MAPI. } // Set workspace fields if available if providerSpec.Workspace != nil { - // Validate that workspace has required fields populated. - // CPMS-style empty workspace (for failure domain-based topology) is not yet implemented. - // Implementation would require creating VSphereDeploymentZone resources from Infrastructure.FailureDomains. - // For now, machines must have workspace.server and workspace.datacenter populated. - if providerSpec.Workspace.Server == "" { - errs = append(errs, field.Required(fldPath.Child("workspace", "server"), "workspace.server is required. Control Plane Machine Set (CPMS) with failure domain-based topology is not yet implemented for vSphere")) - } - if providerSpec.Workspace.Datacenter == "" { - errs = append(errs, field.Required(fldPath.Child("workspace", "datacenter"), "workspace.datacenter is required. Control Plane Machine Set (CPMS) with failure domain-based topology is not yet implemented for vSphere")) - } - spec.Server = providerSpec.Workspace.Server spec.Datacenter = providerSpec.Workspace.Datacenter spec.Folder = providerSpec.Workspace.Folder spec.Datastore = providerSpec.Workspace.Datastore spec.ResourcePool = providerSpec.Workspace.ResourcePool // VMGroup - MAPI-specific field for vm-host group based zoning, not supported in CAPV. - } else { - // Workspace is nil - this pattern is used by CPMS with failure domains, which is not yet implemented. - errs = append(errs, field.Required(fldPath.Child("workspace"), "workspace is required. Control Plane Machine Set (CPMS) with failure domain-based topology is not yet implemented for vSphere")) } // Unused fields - Below this line are fields not used from the MAPI VSphereMachineProviderSpec. @@ -307,6 +309,11 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe errs = append(errs, field.Invalid(fldPath.Child("workspace", "vmGroup"), providerSpec.Workspace.VMGroup, "vmGroup is not supported in Cluster API")) } + capvMachineStatus, statusErrs := convertMAPIMachineStatusToVSphereMachineStatus(v.machine) + if len(statusErrs) > 0 { + errs = append(errs, statusErrs...) + } + return &vspherev1.VSphereMachine{ TypeMeta: metav1.TypeMeta{ APIVersion: vspherev1.GroupVersion.String(), @@ -316,7 +323,8 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe Name: v.machine.Name, Namespace: capiNamespace, }, - Spec: spec, + Spec: spec, + Status: capvMachineStatus, }, warnings, errs } @@ -397,13 +405,38 @@ func convertVSphereNetworkSpecMAPIToCAPI(fldPath *field.Path, mapiNetwork mapiv1 Nameservers: device.Nameservers, } - // Convert AddressesFromPools + // Convert AddressesFromPools: MAPI uses plural resource names, CAPI uses singular Kind names + //nolint:nestif // Nested complexity is acceptable for IPAM resource name mapping logic if len(device.AddressesFromPools) > 0 { addressesFromPools := make([]corev1.TypedLocalObjectReference, len(device.AddressesFromPools)) for j, pool := range device.AddressesFromPools { + kind := pool.Resource // fallback to resource name if mapping not found + + // Skip empty APIGroup + if pool.Group == "" { + kind = pool.Resource + } else if groupMappings, ok := ipamResourceToKind[pool.Group]; ok { + // Known IPAM group - look up the Kind + if mappedKind, ok := groupMappings[pool.Resource]; ok { + kind = mappedKind + } else { + // Known IPAM group but unknown resource - warn but continue + warnings = append(warnings, + fmt.Sprintf("Unknown IPAM resource %q for group %q - using %q as Kind. "+ + "This may not work. If you need this resource, please report it.", + pool.Resource, pool.Group, pool.Resource)) + } + } else { + // Unknown IPAM group - warn but continue with resource name as Kind + warnings = append(warnings, + fmt.Sprintf("Unknown IPAM group %q - using resource name %q as Kind. "+ + "This may not work correctly. Known IPAM groups: ipam.cluster.x-k8s.io", + pool.Group, pool.Resource)) + } + addressesFromPools[j] = corev1.TypedLocalObjectReference{ APIGroup: &pool.Group, - Kind: pool.Resource, // This might need adjustment based on actual mapping + Kind: kind, Name: pool.Name, } } @@ -470,3 +503,34 @@ func convertVSphereCloneModeMAPIToCAPI(mapiMode mapiv1beta1.CloneMode) vspherev1 return vspherev1.FullClone } } + +// convertMAPIMachineStatusToVSphereMachineStatus converts MAPI Machine status to CAPV VSphereMachine status. +func convertMAPIMachineStatusToVSphereMachineStatus(mapiMachine *mapiv1beta1.Machine) (vspherev1.VSphereMachineStatus, field.ErrorList) { + var errs field.ErrorList + + mapiProviderStatus, err := VSphereProviderStatusFromRawExtension(mapiMachine.Status.ProviderStatus) + if err != nil { + return vspherev1.VSphereMachineStatus{}, append(errs, field.InternalError(field.NewPath("status", "providerStatus"), err)) + } + + addresses, addressesErr := convertMAPIMachineAddressesToCAPIV1Beta1(mapiMachine.Status.Addresses) + if len(addressesErr) > 0 { + errs = append(errs, addressesErr...) + } + + // CAPV sets Ready to true if InstanceState is "ready". Otherwise false. + // This matches AWS's pattern of using ProviderStatus.InstanceState as the source of truth. + ready := ptr.Deref(mapiProviderStatus.InstanceState, "") == consts.VSphereInstanceStateReady + + s := vspherev1.VSphereMachineStatus{ + Ready: ready, + Addresses: addresses, + // Network: MAPI doesn't track network status, CAPV controller will populate this. + // Conditions are not directly convertible - MAPI uses metav1.Condition while CAPV uses clusterv1beta1.Conditions. + // The CAPV controller will recreate conditions based on the machine state. + // FailureReason: Not set here because we already set it on the Cluster API Machine from .Status.ErrorReason + // FailureMessage: Not set here because we already set it on the Cluster API Machine from .Status.ErrorMessage + } + + return s, errs +} diff --git a/pkg/conversion/mapi2capi/vsphere_fuzz_test.go b/pkg/conversion/mapi2capi/vsphere_fuzz_test.go index 89171d79c..6e3c91718 100644 --- a/pkg/conversion/mapi2capi/vsphere_fuzz_test.go +++ b/pkg/conversion/mapi2capi/vsphere_fuzz_test.go @@ -26,12 +26,14 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/utils/ptr" vspherev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/randfill" "github.com/openshift/cluster-capi-operator/pkg/conversion/capi2mapi" + "github.com/openshift/cluster-capi-operator/pkg/conversion/consts" "github.com/openshift/cluster-capi-operator/pkg/conversion/mapi2capi" conversiontest "github.com/openshift/cluster-capi-operator/pkg/conversion/test/fuzz" ) @@ -72,7 +74,7 @@ var _ = Describe("vSphere Fuzz (mapi2capi)", func() { mapi2capi.FromVSphereMachineAndInfra, fromMachineAndVSphereMachineAndVSphereCluster, conversiontest.ObjectMetaFuzzerFuncs(mapiNamespace), - conversiontest.MAPIMachineFuzzerFuncs(&mapiv1beta1.VSphereMachineProviderSpec{}, nil, vsphereProviderIDFuzzer), + conversiontest.MAPIMachineFuzzerFuncs(&mapiv1beta1.VSphereMachineProviderSpec{}, &mapiv1beta1.VSphereMachineProviderStatus{}, vsphereProviderIDFuzzer), f.FuzzerFuncsMachine, ) }) @@ -97,7 +99,7 @@ var _ = Describe("vSphere Fuzz (mapi2capi)", func() { mapi2capi.FromVSphereMachineSetAndInfra, fromMachineSetAndVSphereMachineTemplateAndVSphereCluster, conversiontest.ObjectMetaFuzzerFuncs(mapiNamespace), - conversiontest.MAPIMachineFuzzerFuncs(&mapiv1beta1.VSphereMachineProviderSpec{}, nil, vsphereProviderIDFuzzer), + conversiontest.MAPIMachineFuzzerFuncs(&mapiv1beta1.VSphereMachineProviderSpec{}, &mapiv1beta1.VSphereMachineProviderStatus{}, vsphereProviderIDFuzzer), conversiontest.MAPIMachineSetFuzzerFuncs(), f.FuzzerFuncsMachineSet, ) @@ -110,6 +112,7 @@ func vsphereProviderIDFuzzer(c randfill.Continue) string { type vsphereProviderFuzzer struct { conversiontest.MAPIMachineFuzzer + InstanceState string } func (f *vsphereProviderFuzzer) fuzzProviderSpec(providerSpec *mapiv1beta1.VSphereMachineProviderSpec, c randfill.Continue) { @@ -153,6 +156,41 @@ func (f *vsphereProviderFuzzer) fuzzProviderSpec(providerSpec *mapiv1beta1.VSphe } } +func (f *vsphereProviderFuzzer) fuzzProviderStatus(providerStatus *mapiv1beta1.VSphereMachineProviderStatus, c randfill.Continue) { + c.FillNoCustom(providerStatus) + + // InstanceState can only be "ready" or empty to match getInstanceState() logic. + // This matches how CAPV VSphereMachine.Status.Ready is a simple boolean. + // Save the value so FuzzMachine can sync it to annotations. + switch c.Int31n(2) { + case 0: + f.InstanceState = "ready" + case 1: + f.InstanceState = "" + } + + if f.InstanceState != "" { + providerStatus.InstanceState = ptr.To(f.InstanceState) + } else { + providerStatus.InstanceState = nil + } +} + +func (f *vsphereProviderFuzzer) FuzzMachine(m *mapiv1beta1.Machine, c randfill.Continue) { + // Call parent fuzzer first + f.MAPIMachineFuzzer.FuzzMachine(m, c) + + // Sync ProviderStatus.InstanceState to annotation for roundtrip fidelity. + // In production, these are always in sync (controllers set both). + if f.InstanceState != "" { + if m.Annotations == nil { + m.Annotations = map[string]string{} + } + + m.Annotations[consts.MAPIMachineMetadataAnnotationInstanceState] = f.InstanceState + } +} + func (f *vsphereProviderFuzzer) FuzzerFuncsMachineSet(codecs runtimeserializer.CodecFactory) []interface{} { return []interface{}{ func(cloneMode *mapiv1beta1.CloneMode, c randfill.Continue) { @@ -180,6 +218,25 @@ func (f *vsphereProviderFuzzer) FuzzerFuncsMachineSet(codecs runtimeserializer.C *provisioningMode = "" } }, + func(pool *mapiv1beta1.AddressesFromPool, c randfill.Continue) { + c.FillNoCustom(pool) + + // Constrain IPAM group and resource to known valid values to avoid warnings. + // The conversion code only recognizes "ipam.cluster.x-k8s.io" or empty. + // Resource names are lowercase plural (e.g., "inclusterippools"), not Kind names. + switch c.Int31n(3) { + case 0: + pool.Group = "ipam.cluster.x-k8s.io" + pool.Resource = "inclusterippools" + case 1: + pool.Group = "ipam.cluster.x-k8s.io" + pool.Resource = "globalinclusterippools" + case 2: + // Empty group - no conversion, no warning + pool.Group = "" + pool.Resource = "" + } + }, f.fuzzProviderSpec, } } @@ -187,6 +244,7 @@ func (f *vsphereProviderFuzzer) FuzzerFuncsMachineSet(codecs runtimeserializer.C func (f *vsphereProviderFuzzer) FuzzerFuncsMachine(codecs runtimeserializer.CodecFactory) []interface{} { return append( f.FuzzerFuncsMachineSet(codecs), + f.fuzzProviderStatus, f.FuzzMachine, ) } diff --git a/pkg/conversion/mapi2capi/vsphere_test.go b/pkg/conversion/mapi2capi/vsphere_test.go index f5404e810..fc4b70d1e 100644 --- a/pkg/conversion/mapi2capi/vsphere_test.go +++ b/pkg/conversion/mapi2capi/vsphere_test.go @@ -102,9 +102,11 @@ var _ = Describe("mapi2capi VSphere conversion", func() { machineBuilder: vsphereMAPIMachineBase.WithProviderSpecBuilder( vsphereBaseProviderSpec.WithIPPool(), ), - infra: infra, - expectedErrors: []string{}, - expectedWarnings: []string{}, + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{ + "Unknown IPAM group \"test\" - using resource name \"IPpool\" as Kind", + }, }), // Error Cases. @@ -365,6 +367,66 @@ var _ = Describe("mapi2capi VSphere conversion", func() { expectedWarnings: []string{}, }), + Entry("With AddressesFromPools (known IPAM)", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Workspace: &mapiv1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + }, + Network: mapiv1beta1.NetworkSpec{ + Devices: []mapiv1beta1.NetworkDeviceSpec{ + { + NetworkName: "network1", + AddressesFromPools: []mapiv1beta1.AddressesFromPool{ + { + Group: "ipam.cluster.x-k8s.io", + Resource: "inclusterippools", + Name: "test-pool", + }, + }, + }, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{}, + }), + + Entry("With AddressesFromPools (unknown IPAM group)", vsphereMAPI2CAPIConversionInput{ + machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ + Value: mustConvertVSphereProviderSpecToRawExtension(&mapiv1beta1.VSphereMachineProviderSpec{ + Template: "test-template", + Workspace: &mapiv1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + }, + Network: mapiv1beta1.NetworkSpec{ + Devices: []mapiv1beta1.NetworkDeviceSpec{ + { + NetworkName: "network1", + AddressesFromPools: []mapiv1beta1.AddressesFromPool{ + { + Group: "custom.ipam.io", + Resource: "custompools", + Name: "my-pool", + }, + }, + }, + }, + }, + }), + }), + infra: infra, + expectedErrors: []string{}, + expectedWarnings: []string{ + "Unknown IPAM group \"custom.ipam.io\" - using resource name \"custompools\" as Kind", + }, + }), + // Comprehensive Test. Entry("With comprehensive configuration", vsphereMAPI2CAPIConversionInput{ machineBuilder: machinebuilder.Machine().WithProviderSpec(mapiv1beta1.ProviderSpec{ From 4ae9066c1896e6ccf495ce0c38fba11fb5f1fbf5 Mon Sep 17 00:00:00 2001 From: tzivkovi Date: Tue, 9 Jun 2026 13:10:54 -0700 Subject: [PATCH 8/8] squash! Enable vSphere in MAPI<->CAPI conversion framework Address review feedback from damdo: - Refactor condition utilities to be version-agnostic: replace hardcoded v1beta2 functions with a dynamic approach using an optional version parameter and auto-discovery of versioned condition paths, supporting v1beta2, v1beta3, and future API versions. - Remove ObjectMeta (Labels/Annotations) copying from FromVSphereMachineSetAndInfra to match the pattern used by other providers. - Clarify VSphereInstanceStateReady is a MAPI-only concept used in both conversion directions. - Add comprehensive tests for condition utilities. Co-Authored-By: Claude Sonnet 4.5 --- pkg/conversion/consts/consts.go | 4 +- pkg/conversion/mapi2capi/vsphere.go | 13 +- pkg/util/conditions.go | 129 +++++------- pkg/util/conditions_test.go | 316 ++++++++++++++++++++++++++++ 4 files changed, 382 insertions(+), 80 deletions(-) diff --git a/pkg/conversion/consts/consts.go b/pkg/conversion/consts/consts.go index 1c280171b..2a706737a 100644 --- a/pkg/conversion/consts/consts.go +++ b/pkg/conversion/consts/consts.go @@ -28,6 +28,8 @@ const ( // MAPIMachineMetadataAnnotationInstanceState is the annotation for the instance state of the machine is used as column for kubectl. MAPIMachineMetadataAnnotationInstanceState = "machine.openshift.io/instance-state" - // VSphereInstanceStateReady indicates the vSphere instance is ready. + // VSphereInstanceStateReady is the MAPI ProviderStatus.InstanceState value indicating the vSphere instance is ready. + // CAPV has no InstanceState field; this is used in both conversion directions to translate between + // MAPI's string-based InstanceState and CAPV's boolean Ready field. VSphereInstanceStateReady = "ready" ) diff --git a/pkg/conversion/mapi2capi/vsphere.go b/pkg/conversion/mapi2capi/vsphere.go index 75f35e33a..8cf4890dc 100644 --- a/pkg/conversion/mapi2capi/vsphere.go +++ b/pkg/conversion/mapi2capi/vsphere.go @@ -80,10 +80,6 @@ func FromVSphereMachineSetAndInfra(m *mapiv1beta1.MachineSet, i *configv1.Infras infrastructure: i, vSphereMachineAndInfra: &vSphereMachineAndInfra{ machine: &mapiv1beta1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: m.Spec.Template.ObjectMeta.Labels, //nolint:staticcheck // ObjectMeta is a field, not embedded - Annotations: m.Spec.Template.ObjectMeta.Annotations, //nolint:staticcheck // ObjectMeta is a field, not embedded - }, Spec: m.Spec.Template.Spec, }, infrastructure: i, @@ -201,6 +197,12 @@ func (v *vSphereMachineSetAndInfra) ToMachineSetAndMachineTemplate() (*clusterv1 capiMachineSet.Spec.Template.Spec.InfrastructureRef.Kind = vSphereMachineTemplateKind capiMachineSet.Spec.Template.Spec.InfrastructureRef.Name = capvMachineTemplate.Name + // vSphere stores FailureDomain in template labels (machine.openshift.io/zone), not in ProviderSpec. + // Read it directly from the MachineSet template since the synthetic machine has no ObjectMeta. + if zone, ok := v.machineSet.Spec.Template.ObjectMeta.Labels[consts.MAPIMachineMetadataLabelZone]; ok && zone != "" { //nolint:staticcheck // ObjectMeta is a field, not embedded + capiMachineSet.Spec.Template.Spec.FailureDomain = zone + } + if v.infrastructure == nil || v.infrastructure.Status.InfrastructureName == "" { errors = append(errors, field.Invalid(field.NewPath("infrastructure", "status", "infrastructureName"), v.infrastructure.Status.InfrastructureName, "infrastructure cannot be nil and infrastructure.Status.InfrastructureName cannot be empty")) } else { @@ -272,7 +274,8 @@ func (v *vSphereMachineAndInfra) toVSphereMachine(providerSpec mapiv1beta1.VSphe // delete/recreate loop. MAPI has no PowerOffMode field, so "hard" matches MAPI's power-off behavior. PowerOffMode: vspherev1.VirtualMachinePowerOpModeHard, // GuestSoftPowerOffTimeout - Not supported in MAPI. - // NamingStrategy - Not supported in MAPI. + // NamingStrategy - Not supported in MAPI. Labels: m.Spec.Template.ObjectMeta.Labels, //nolint:staticcheck // ObjectMeta is a field, not embedded + // Annotations: m.Spec.Template.ObjectMeta.Annotations, //nolint:staticcheck // ObjectMeta is a field, not embedded } // Set workspace fields if available diff --git a/pkg/util/conditions.go b/pkg/util/conditions.go index 204650d34..a978e26ac 100644 --- a/pkg/util/conditions.go +++ b/pkg/util/conditions.go @@ -18,6 +18,7 @@ package util import ( "errors" "fmt" + "regexp" "time" mapiv1beta1 "github.com/openshift/api/machine/v1beta1" @@ -32,63 +33,37 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -var ( - errStatusNotMap = errors.New("unable to assert status structure to map") - errStatusConditionsNotInterface = errors.New("unable to assert status.condition structure to interface") - errStatusV1Beta2NotFound = errors.New("unable to find status.v1beta2 field") - errStatusV1Beta2ConditionsNotInterface = errors.New("unable to assert status.v1beta2.conditions structure to interface") -) +var errConditionsPathNotFound = errors.New("conditions path not found") + +// kubeVersionRegex matches Kubernetes API versions: v1, v2, v1alpha1, v1beta1, v1beta2, etc. +var kubeVersionRegex = regexp.MustCompile(`^v\d+(?:(?:alpha|beta)\d+)?$`) // GetCondition retrieves a specific condition from a client.Object. -func GetCondition(obj client.Object, conditionType string) (*metav1.Condition, error) { +// It accepts an optional version string (e.g., "v1beta2") to look under status..conditions. +// If no version is provided, it defaults to status.conditions. +func GetCondition(obj client.Object, conditionType string, version ...string) (*metav1.Condition, error) { unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) if err != nil { return nil, fmt.Errorf("failed to convert object to unstructured: %w", err) } - unstructuredObj := &unstructured.Unstructured{Object: unstructuredMap} - data := unstructuredObj.UnstructuredContent() - - status, ok := data["status"].(map[string]interface{}) - if !ok { - return nil, errStatusNotMap - } - - conditions, ok := status["conditions"].([]interface{}) - if !ok { - return nil, errStatusConditionsNotInterface + path := []string{"status"} + if len(version) > 0 && version[0] != "" { + path = append(path, version[0]) } - return getConditionFromList(conditions, conditionType) -} + path = append(path, "conditions") -// GetV1Beta2Condition retrieves a specific v1beta2 condition from a client.Object. -// V1Beta2 conditions are stored at status.v1beta2.conditions instead of status.conditions. -func GetV1Beta2Condition(obj client.Object, conditionType string) (*metav1.Condition, error) { - unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + conds, found, err := unstructured.NestedSlice(unstructuredMap, path...) if err != nil { - return nil, fmt.Errorf("failed to convert object to unstructured: %w", err) + return nil, fmt.Errorf("failed to parse conditions field at %v: %w", path, err) } - unstructuredObj := &unstructured.Unstructured{Object: unstructuredMap} - data := unstructuredObj.UnstructuredContent() - - status, ok := data["status"].(map[string]interface{}) - if !ok { - return nil, errStatusNotMap - } - - v1beta2, ok := status["v1beta2"].(map[string]interface{}) - if !ok { - return nil, errStatusV1Beta2NotFound + if !found { + return nil, fmt.Errorf("%w: %v", errConditionsPathNotFound, path) } - conditions, ok := v1beta2["conditions"].([]interface{}) - if !ok { - return nil, errStatusV1Beta2ConditionsNotInterface - } - - return getConditionFromList(conditions, conditionType) + return getConditionFromList(conds, conditionType) } // getConditionFromList finds a specific condition in a conditions list. @@ -118,9 +93,10 @@ func getConditionFromList(conditions []interface{}, conditionType string) (*meta return nil, nil //nolint:nilnil } -// GetConditionStatus returns the status for the condition. -func GetConditionStatus(obj client.Object, conditionType string) (corev1.ConditionStatus, error) { - cond, err := GetCondition(obj, conditionType) +// GetConditionStatus returns the status for the specified condition type. +// It supports the same optional versioning as GetCondition. +func GetConditionStatus(obj client.Object, conditionType string, version ...string) (corev1.ConditionStatus, error) { + cond, err := GetCondition(obj, conditionType, version...) if err != nil { return corev1.ConditionUnknown, fmt.Errorf("unable to get condition %q for the object: %w", conditionType, err) } @@ -132,48 +108,53 @@ func GetConditionStatus(obj client.Object, conditionType string) (corev1.Conditi return corev1.ConditionStatus(cond.Status), nil } -// GetV1Beta2ConditionStatus returns the status for the v1beta2 condition. -func GetV1Beta2ConditionStatus(obj client.Object, conditionType string) (corev1.ConditionStatus, error) { - cond, err := GetV1Beta2Condition(obj, conditionType) +// GetConditionStatusFromInfraObject discovers a versioned condition path from the object's status +// and tries it first, falling back to root status.conditions. +// Returns the first non-Unknown status found, or ConditionUnknown if not found anywhere. +func GetConditionStatusFromInfraObject(obj client.Object, conditionType string) (corev1.ConditionStatus, error) { + unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) if err != nil { - return corev1.ConditionUnknown, fmt.Errorf("unable to get v1beta2 condition %q for the object: %w", conditionType, err) + return corev1.ConditionUnknown, fmt.Errorf("failed to convert object to unstructured: %w", err) } - if cond == nil { + statusMap, ok := unstructuredMap["status"].(map[string]interface{}) + if !ok { return corev1.ConditionUnknown, nil } - return corev1.ConditionStatus(cond.Status), nil -} - -// GetConditionStatusFromInfraObject tries to get condition status from either v1beta2 or v1beta1 conditions. -// It first tries v1beta2 (for providers like vSphere) then falls back to v1beta1 (for AWS, Azure, GCP, etc.). -// This is useful for infrastructure provider objects where the condition location varies by provider. -// If the condition is not found in either location, it returns ConditionUnknown instead of an error. -func GetConditionStatusFromInfraObject(obj client.Object, conditionType string) (corev1.ConditionStatus, error) { - // Try v1beta2 first (for providers like vSphere) - status, err := GetV1Beta2ConditionStatus(obj, conditionType) - if status != corev1.ConditionUnknown { - return status, err + if version := findVersionedConditionPath(statusMap); version != "" { + return GetConditionStatus(obj, conditionType, version) } - if err != nil && !errors.Is(err, errStatusV1Beta2NotFound) { - return status, err + status, err := GetConditionStatus(obj, conditionType) + if err != nil && errors.Is(err, errConditionsPathNotFound) { + return corev1.ConditionUnknown, nil } - status, err = GetConditionStatus(obj, conditionType) - if status != corev1.ConditionUnknown { - return status, err - } + return status, err +} + +func isKubeVersion(s string) bool { + return kubeVersionRegex.MatchString(s) +} + +func findVersionedConditionPath(statusMap map[string]interface{}) string { + for key, val := range statusMap { + if !isKubeVersion(key) { + continue + } + + sub, ok := val.(map[string]interface{}) + if !ok { + continue + } - // Both returned Unknown. If the errors are expected (missing fields), return Unknown with no error - // to avoid reconcile failures for providers that simply omit the condition. - // Otherwise, return the real error. - if err != nil && !errors.Is(err, errStatusConditionsNotInterface) { - return corev1.ConditionUnknown, err + if _, has := sub["conditions"]; has { + return key + } } - return corev1.ConditionUnknown, nil + return "" } func getString(data map[string]interface{}, key string) string { diff --git a/pkg/util/conditions_test.go b/pkg/util/conditions_test.go index b3465108a..13f94d40c 100644 --- a/pkg/util/conditions_test.go +++ b/pkg/util/conditions_test.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func TestSetMAPICondition(t *testing.T) { @@ -249,6 +250,321 @@ func TestSetMAPICondition(t *testing.T) { } } +func TestGetCondition(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + conditionType string + version []string + wantStatus corev1.ConditionStatus + wantErr bool + }{ + { + name: "root condition found", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Ready", "status": "True"}, + }, + }, + }, + }, + conditionType: "Ready", + version: nil, + wantStatus: corev1.ConditionTrue, + }, + { + name: "empty version defaults to root conditions", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Ready", "status": "False"}, + }, + }, + }, + }, + conditionType: "Ready", + version: []string{}, + wantStatus: corev1.ConditionFalse, + }, + { + name: "versioned v1beta2 condition found", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "v1beta2": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Available", "status": "False"}, + }, + }, + }, + }, + }, + conditionType: "Available", + version: []string{"v1beta2"}, + wantStatus: corev1.ConditionFalse, + }, + { + name: "arbitrary future version condition found", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "v2": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Reconciled", "status": "True"}, + }, + }, + }, + }, + }, + conditionType: "Reconciled", + version: []string{"v2"}, + wantStatus: corev1.ConditionTrue, + }, + { + name: "root conditions missing entirely", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{}, + }, + }, + conditionType: "Ready", + version: nil, + wantErr: true, + }, + { + name: "versioned path missing entirely", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{}, + }, + }, + conditionType: "Ready", + version: []string{"v1beta2"}, + wantErr: true, + }, + { + name: "condition type not found in list", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Progressing", "status": "True"}, + }, + }, + }, + }, + conditionType: "Ready", + version: nil, + wantStatus: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + cond, err := GetCondition(tt.obj, tt.conditionType, tt.version...) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).NotTo(HaveOccurred()) + + if tt.wantStatus != "" { + g.Expect(cond).NotTo(BeNil()) + g.Expect(corev1.ConditionStatus(cond.Status)).To(Equal(tt.wantStatus)) + } + }) + } +} + +func TestGetConditionStatus(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "v1beta2": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Ready", "status": "True"}, + }, + }, + }, + }, + } + + t.Run("versioned condition found", func(t *testing.T) { + g := NewWithT(t) + status, err := GetConditionStatus(obj, "Ready", "v1beta2") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(status).To(Equal(corev1.ConditionTrue)) + }) + + t.Run("missing condition returns Unknown", func(t *testing.T) { + g := NewWithT(t) + status, err := GetConditionStatus(obj, "MissingCondition", "v1beta2") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(status).To(Equal(corev1.ConditionUnknown)) + }) + + t.Run("non-existent version returns error", func(t *testing.T) { + g := NewWithT(t) + status, err := GetConditionStatus(obj, "Ready", "non-existent-version") + g.Expect(err).To(HaveOccurred()) + g.Expect(status).To(Equal(corev1.ConditionUnknown)) + }) +} + +func TestIsKubeVersion(t *testing.T) { + t.Run("valid kube versions", func(t *testing.T) { + g := NewWithT(t) + for _, v := range []string{"v1", "v2", "v1alpha1", "v1beta1", "v1beta2", "v1beta3", "v2beta1", "v10alpha3"} { + g.Expect(isKubeVersion(v)).To(BeTrue(), "expected %q to be a valid kube version", v) + } + }) + + t.Run("invalid kube versions", func(t *testing.T) { + g := NewWithT(t) + for _, v := range []string{"", "foo", "conditions", "v", "beta1", "v1gamma1", "1beta1", "v1beta", "v1BETA1"} { + g.Expect(isKubeVersion(v)).To(BeFalse(), "expected %q to NOT be a valid kube version", v) + } + }) +} + +func TestFindVersionedConditionPath(t *testing.T) { + tests := []struct { + name string + statusMap map[string]interface{} + want string + }{ + { + name: "finds v1beta2", + statusMap: map[string]interface{}{ + "v1beta2": map[string]interface{}{ + "conditions": []interface{}{}, + }, + }, + want: "v1beta2", + }, + { + name: "finds v1 GA version", + statusMap: map[string]interface{}{ + "v1": map[string]interface{}{ + "conditions": []interface{}{}, + }, + }, + want: "v1", + }, + { + name: "ignores non-version keys", + statusMap: map[string]interface{}{ + "observedGeneration": int64(1), + "foo": map[string]interface{}{ + "conditions": []interface{}{}, + }, + }, + want: "", + }, + { + name: "ignores version key without conditions", + statusMap: map[string]interface{}{ + "v1beta2": map[string]interface{}{ + "ready": true, + }, + }, + want: "", + }, + { + name: "empty status map", + statusMap: map[string]interface{}{}, + want: "", + }, + { + name: "ignores root conditions (not a versioned sub-map)", + statusMap: map[string]interface{}{ + "conditions": []interface{}{}, + }, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(findVersionedConditionPath(tt.statusMap)).To(Equal(tt.want)) + }) + } +} + +func TestGetConditionStatusFromInfraObject(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + condType string + wantStatus corev1.ConditionStatus + }{ + { + name: "finds versioned condition", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "v1beta2": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Ready", "status": "True"}, + }, + }, + }, + }, + }, + condType: "Ready", + wantStatus: corev1.ConditionTrue, + }, + { + name: "falls back to root conditions", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Ready", "status": "True"}, + }, + }, + }, + }, + condType: "Ready", + wantStatus: corev1.ConditionTrue, + }, + { + name: "empty status returns Unknown", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "status": map[string]interface{}{}, + }, + }, + condType: "Ready", + wantStatus: corev1.ConditionUnknown, + }, + { + name: "no status at all returns Unknown", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{}, + }, + condType: "Ready", + wantStatus: corev1.ConditionUnknown, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + status, err := GetConditionStatusFromInfraObject(tt.obj, tt.condType) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(status).To(Equal(tt.wantStatus)) + }) + } +} + func TestSetMAPIProviderCondition(t *testing.T) { t1 := time.Now().UTC().Truncate(time.Second).Add(-1 * time.Hour)