Skip to content

SPLAT-2410: vsphere conversion #352

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

SPLAT-2410: vsphere conversion #352

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions cmd/machine-api-migration/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ import (
"os"
"time"

"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/v1beta1"

configv1 "github.com/openshift/api/config/v1"
mapiv1alpha1 "github.com/openshift/api/machine/v1alpha1"
mapiv1beta1 "github.com/openshift/api/machine/v1beta1"
Expand All @@ -32,14 +38,7 @@ import (
"github.com/openshift/cluster-capi-operator/pkg/controllers/machinesetsync"
"github.com/openshift/cluster-capi-operator/pkg/controllers/machinesync"
"github.com/openshift/cluster-capi-operator/pkg/util"
"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"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"

"github.com/openshift/api/features"
featuregates "github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand All @@ -52,6 +51,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"

"github.com/openshift/api/features"
featuregates "github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/events"
)

var (
Expand All @@ -70,6 +73,7 @@ func initScheme(scheme *runtime.Scheme) {
utilruntime.Must(awsv1.AddToScheme(scheme))
utilruntime.Must(openstackv1.AddToScheme(scheme))
utilruntime.Must(clusterv1.AddToScheme(scheme))
utilruntime.Must(vspherev1.AddToScheme(scheme))
}

//nolint:funlen
Expand Down Expand Up @@ -196,7 +200,7 @@ func main() {

// Currently we only plan to support AWS, so all others are a noop until they're implemented.
switch provider {
case configv1.AWSPlatformType, configv1.OpenStackPlatformType:
case configv1.AWSPlatformType, configv1.OpenStackPlatformType, configv1.VSpherePlatformType:
klog.Infof("MachineAPIMigration: starting %s controllers", provider)

default:
Expand Down
5 changes: 5 additions & 0 deletions pkg/controllers/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,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"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -46,6 +47,8 @@ func InitInfraMachineAndInfraClusterFromProvider(platform configv1.PlatformType)
return &openstackv1.OpenStackMachine{}, &openstackv1.OpenStackCluster{}, nil
case configv1.PowerVSPlatformType:
return &ibmpowervsv1.IBMPowerVSMachine{}, &ibmpowervsv1.IBMPowerVSCluster{}, nil
case configv1.VSpherePlatformType:
return &vspherev1.VSphereMachine{}, &vspherev1.VSphereCluster{}, nil
default:
return nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform)
}
Expand All @@ -63,6 +66,8 @@ func InitInfraMachineTemplateAndInfraClusterFromProvider(platform configv1.Platf
return &openstackv1.OpenStackMachineTemplate{}, &openstackv1.OpenStackCluster{}, nil
case configv1.PowerVSPlatformType:
return &ibmpowervsv1.IBMPowerVSMachineTemplate{}, &ibmpowervsv1.IBMPowerVSCluster{}, nil
case configv1.VSpherePlatformType:
return &vspherev1.VSphereMachineTemplate{}, &vspherev1.VSphereCluster{}, nil
default:
return nil, nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform)
}
Expand Down
50 changes: 50 additions & 0 deletions pkg/controllers/machinesetsync/machineset_sync_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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/v1beta1"
"sigs.k8s.io/cluster-api/util/annotations"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -80,6 +81,9 @@ var (
// errAssertingCAPIOpenStackMachineTemplate is returned when we encounter an issue asserting a client.Object into a OpenStackMachineTemplate.
errAssertingCAPIOpenStackMachineTemplate = errors.New("error asserting the CAPI OpenStackMachineTemplate object")

// errAssertingCAPIVSphereMachineTemplate is returned when we encounter an issue asserting a client.Object into a VSphereMachineTemplate.
errAssertingCAPIVSphereMachineTemplate = errors.New("error asserting the CAPI VSphereMachineTemplate object")

// errUnsuportedOwnerKindForConversion is returned when the owner kind is not supported for conversion.
errUnsuportedOwnerKindForConversion = errors.New("unsupported owner kind for conversion")

Expand Down Expand Up @@ -381,6 +385,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)
}
}
Comment on lines +388 to +393
Copy link

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix address-of-loop-variable bug when collecting outdated templates (also affects other providers).

Using &template from a range loop appends the same pointer repeatedly. Iterate by index and take the address of the slice element. Apply this across all provider cases in this function.

Apply:

 case *awsv1.AWSMachineTemplateList:
-    for _, template := range list.Items {
-        if template.GetName() != newInfraMachineTemplateName {
-            outdatedTemplates = append(outdatedTemplates, &template)
-        }
-    }
+    for i := range list.Items {
+        tmpl := &list.Items[i]
+        if tmpl.GetName() != newInfraMachineTemplateName {
+            outdatedTemplates = append(outdatedTemplates, tmpl)
+        }
+    }
 case *ibmpowervsv1.IBMPowerVSMachineTemplateList:
-    for _, template := range list.Items {
-        if template.GetName() != newInfraMachineTemplateName {
-            outdatedTemplates = append(outdatedTemplates, &template)
-        }
-    }
+    for i := range list.Items {
+        tmpl := &list.Items[i]
+        if tmpl.GetName() != newInfraMachineTemplateName {
+            outdatedTemplates = append(outdatedTemplates, tmpl)
+        }
+    }
 case *openstackv1.OpenStackMachineTemplateList:
-    for _, template := range list.Items {
-        if template.GetName() != newInfraMachineTemplateName {
-            outdatedTemplates = append(outdatedTemplates, &template)
-        }
-    }
+    for i := range list.Items {
+        tmpl := &list.Items[i]
+        if tmpl.GetName() != newInfraMachineTemplateName {
+            outdatedTemplates = append(outdatedTemplates, tmpl)
+        }
+    }
 case *vspherev1.VSphereMachineTemplateList:
-    for _, template := range list.Items {
-        if template.GetName() != newInfraMachineTemplateName {
-            outdatedTemplates = append(outdatedTemplates, &template)
-        }
-    }
+    for i := range list.Items {
+        tmpl := &list.Items[i]
+        if tmpl.GetName() != newInfraMachineTemplateName {
+            outdatedTemplates = append(outdatedTemplates, tmpl)
+        }
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case *vspherev1.VSphereMachineTemplateList:
for _, template := range list.Items {
if template.GetName() != newInfraMachineTemplateName {
outdatedTemplates = append(outdatedTemplates, &template)
}
}
case *vspherev1.VSphereMachineTemplateList:
for i := range list.Items {
tmpl := &list.Items[i]
if tmpl.GetName() != newInfraMachineTemplateName {
outdatedTemplates = append(outdatedTemplates, tmpl)
}
}
🤖 Prompt for AI Agents 
In pkg/controllers/machinesetsync/machineset_sync_controller.go around lines 388
to 393, the code takes the address of the range loop variable (e.g. &template)
which results in appending the same pointer repeatedly; change the loops to
iterate by index and append the address of the slice element (e.g.
&list.Items[i]) instead, and apply this index-based addressing fix to every
provider case in this function that currently uses &loopVar in a range loop to
collect outdated templates.

default:
return nil, fmt.Errorf("%w: got unknown type %T", errUnexpectedInfraMachineTemplateListType, list)
}
Expand Down Expand Up @@ -714,6 +724,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)
}
Expand All @@ -728,6 +752,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)
}
Expand Down Expand Up @@ -1071,6 +1097,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)
}
Expand Down Expand Up @@ -1146,6 +1174,28 @@ func compareCAPIInfraMachineTemplates(platform configv1.PlatformType, infraMachi
diff[".metadata"] = diffObjectMeta
}

return diff, nil
case configv1.VSpherePlatformType:
typedInfraMachineTemplate1, ok := infraMachineTemplate1.(*vspherev1.VSphereMachineTemplate)
if !ok {
return nil, errAssertingCAPIVSphereMachineTemplate
}

typedinfraMachineTemplate2, ok := infraMachineTemplate2.(*vspherev1.VSphereMachineTemplate)
if !ok {
return nil, errAssertingCAPIVSphereMachineTemplate
}

diff := make(map[string]any)

if diffSpec := deep.Equal(typedInfraMachineTemplate1.Spec, typedinfraMachineTemplate2.Spec); len(diffSpec) > 0 {
diff[".spec"] = diffSpec
}

if diffObjectMeta := util.ObjectMetaEqual(typedInfraMachineTemplate1.ObjectMeta, typedinfraMachineTemplate2.ObjectMeta); len(diffObjectMeta) > 0 {
diff[".metadata"] = diffObjectMeta
}

return diff, nil
default:
return nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform)
Expand Down
39 changes: 39 additions & 0 deletions pkg/controllers/machinesync/machine_sync_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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/v1beta1"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/labels/format"
Expand Down Expand Up @@ -99,6 +100,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.
Expand Down Expand Up @@ -562,6 +566,8 @@ func (r *MachineSyncReconciler) convertMAPIToCAPIMachine(mapiMachine *machinev1b
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)
}
Expand Down Expand Up @@ -593,6 +599,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)
}
Expand Down Expand Up @@ -1362,6 +1380,27 @@ func compareCAPIInfraMachines(platform configv1.PlatformType, infraMachine1, inf
diff[".metadata"] = diffMetadata
}

return diff, nil
case configv1.VSpherePlatformType:
typedInfraMachine1, ok := infraMachine1.(*vspherev1.VSphereMachine)
if !ok {
return nil, errAssertingCAPIVSphereMachine
}

typedinfraMachine2, ok := infraMachine2.(*vspherev1.VSphereMachine)
if !ok {
return nil, errAssertingCAPIVSphereMachine
}

diff := make(map[string]any)
if diffSpec := deep.Equal(typedInfraMachine1.Spec, typedinfraMachine2.Spec); len(diffSpec) > 0 {
diff[".spec"] = diffSpec
}

if diffMetadata := util.ObjectMetaEqual(typedInfraMachine1.ObjectMeta, typedinfraMachine2.ObjectMeta); len(diffMetadata) > 0 {
diff[".metadata"] = diffMetadata
}

return diff, nil
default:
return nil, fmt.Errorf("%w: %s", errPlatformNotSupported, platform)
Expand Down
Loading