diff --git a/apis/v1beta1/vspheremachine_types.go b/apis/v1beta1/vspheremachine_types.go index cc6d31d1aa..cbdd5ff7ac 100644 --- a/apis/v1beta1/vspheremachine_types.go +++ b/apis/v1beta1/vspheremachine_types.go @@ -81,6 +81,10 @@ const ( // Note: This reason is used only in supervisor mode. VSphereMachineVirtualMachinePoweringOnV1Beta2Reason = "PoweringOn" + // VSphereMachineVirtualMachineWaitingForVirtualMachineGroupV1Beta2Reason surfaces that the VirtualMachine + // is waiting for its corresponding VirtualMachineGroup to be created and to include this VM as a member. + VSphereMachineVirtualMachineWaitingForVirtualMachineGroupV1Beta2Reason = "WaitingForVirtualMachineGroup" + // VSphereMachineVirtualMachineWaitingForNetworkAddressV1Beta2Reason surfaces when the VirtualMachine that is controlled // by the VSphereMachine waiting for the machine network settings to be reported after machine being powered on. VSphereMachineVirtualMachineWaitingForNetworkAddressV1Beta2Reason = "WaitingForNetworkAddress" diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 401dd765e5..102217c078 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -21,7 +21,7 @@ spec: - "--diagnostics-address=${CAPI_DIAGNOSTICS_ADDRESS:=:8443}" - "--insecure-diagnostics=${CAPI_INSECURE_DIAGNOSTICS:=false}" - --v=4 - - "--feature-gates=MultiNetworks=${EXP_MULTI_NETWORKS:=false},NodeAntiAffinity=${EXP_NODE_ANTI_AFFINITY:=false},NamespaceScopedZones=${EXP_NAMESPACE_SCOPED_ZONES:=false},PriorityQueue=${EXP_PRIORITY_QUEUE:=false}" + - "--feature-gates=MultiNetworks=${EXP_MULTI_NETWORKS:=false},NodeAntiAffinity=${EXP_NODE_ANTI_AFFINITY:=false},NamespaceScopedZones=${EXP_NAMESPACE_SCOPED_ZONES:=false},NodeAutoPlacement=${EXP_NODE_AUTO_PLACEMENT:=false},PriorityQueue=${EXP_PRIORITY_QUEUE:=false}" image: controller:latest imagePullPolicy: IfNotPresent name: manager diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index ff4613da71..d3963fb5bf 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -249,6 +249,8 @@ rules: - apiGroups: - vmoperator.vmware.com resources: + - virtualmachinegroups + - virtualmachinegroups/status - virtualmachineimages - virtualmachineimages/status - virtualmachines diff --git a/controllers/vmware/controllers_suite_test.go b/controllers/vmware/controllers_suite_test.go index 87d99112e0..128ee2086d 100644 --- a/controllers/vmware/controllers_suite_test.go +++ b/controllers/vmware/controllers_suite_test.go @@ -26,6 +26,7 @@ import ( . "github.com/onsi/ginkgo/v2" "github.com/onsi/ginkgo/v2/types" . "github.com/onsi/gomega" + vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -71,6 +72,7 @@ func setup(ctx context.Context) (*helpers.TestEnvironment, clustercache.ClusterC utilruntime.Must(infrav1.AddToScheme(scheme.Scheme)) utilruntime.Must(clusterv1.AddToScheme(scheme.Scheme)) utilruntime.Must(vmwarev1.AddToScheme(scheme.Scheme)) + utilruntime.Must(vmoprv1.AddToScheme(scheme.Scheme)) testEnv := helpers.NewTestEnvironment(ctx) diff --git a/controllers/vmware/virtualmachinegroup_controller.go b/controllers/vmware/virtualmachinegroup_controller.go new file mode 100644 index 0000000000..22767f12f3 --- /dev/null +++ b/controllers/vmware/virtualmachinegroup_controller.go @@ -0,0 +1,101 @@ +/* +Copyright 2025 The Kubernetes Authors. + +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 vmware + +import ( + "context" + + vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + apitypes "k8s.io/apimachinery/pkg/types" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/util/predicates" + ctrl "sigs.k8s.io/controller-runtime" + ctrlbldr "sigs.k8s.io/controller-runtime/pkg/builder" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context" +) + +// AddVirtualMachineGroupControllerToManager adds the VirtualMachineGroup controller to the provided manager. +func AddVirtualMachineGroupControllerToManager(ctx context.Context, controllerManagerCtx *capvcontext.ControllerManagerContext, mgr manager.Manager, options controller.Options) error { + predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "virtualmachinegroup") + + reconciler := &VirtualMachineGroupReconciler{ + Client: controllerManagerCtx.Client, + Recorder: mgr.GetEventRecorderFor("virtualmachinegroup-controller"), + } + + builder := ctrl.NewControllerManagedBy(mgr). + For(&clusterv1.Cluster{}). + WithOptions(options). + // Set the controller's name explicitly to virtualmachinegroup. + Named("virtualmachinegroup"). + Watches( + &vmoprv1.VirtualMachineGroup{}, + handler.EnqueueRequestForOwner(mgr.GetScheme(), reconciler.Client.RESTMapper(), &clusterv1.Cluster{}), + ctrlbldr.WithPredicates(predicates.ResourceIsChanged(mgr.GetScheme(), predicateLog)), + ). + Watches( + &vmwarev1.VSphereMachine{}, + handler.EnqueueRequestsFromMapFunc(reconciler.VSphereMachineToCluster), + ctrlbldr.WithPredicates( + predicate.Funcs{ + UpdateFunc: func(event.UpdateEvent) bool { return false }, + CreateFunc: func(e event.CreateEvent) bool { + // Only handle VSphereMachine which belongs to a MachineDeployment + _, found := e.Object.GetLabels()[clusterv1.MachineDeploymentNameLabel] + return found + }, + DeleteFunc: func(e event.DeleteEvent) bool { + // Only handle VSphereMachine which belongs to a MachineDeployment + _, found := e.Object.GetLabels()[clusterv1.MachineDeploymentNameLabel] + return found + }, + GenericFunc: func(event.GenericEvent) bool { return false }, + }), + ). + WithEventFilter(predicates.ResourceHasFilterLabel(mgr.GetScheme(), predicateLog, controllerManagerCtx.WatchFilterValue)) + + return builder.Complete(reconciler) +} + +// VSphereMachineToCluster maps VSphereMachine events to Cluster reconcile requests. +func (r *VirtualMachineGroupReconciler) VSphereMachineToCluster(_ context.Context, a ctrlclient.Object) []reconcile.Request { + vSphereMachine, ok := a.(*vmwarev1.VSphereMachine) + if !ok { + return nil + } + + clusterName, ok := vSphereMachine.Labels[clusterv1.ClusterNameLabel] + if !ok || clusterName == "" { + return nil + } + + return []reconcile.Request{{ + NamespacedName: apitypes.NamespacedName{ + Namespace: vSphereMachine.Namespace, + Name: clusterName, + }, + }} +} diff --git a/controllers/vmware/virtualmachinegroup_reconciler.go b/controllers/vmware/virtualmachinegroup_reconciler.go new file mode 100644 index 0000000000..3e02cc5f21 --- /dev/null +++ b/controllers/vmware/virtualmachinegroup_reconciler.go @@ -0,0 +1,552 @@ +/* +Copyright 2025 The Kubernetes Authors. + +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 vmware contains the VirtualMachineGroup Reconciler. +package vmware + +import ( + "context" + "fmt" + "strings" + + "github.com/pkg/errors" + vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "golang.org/x/exp/slices" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/util/conditions" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + infrautilv1 "sigs.k8s.io/cluster-api-provider-vsphere/pkg/util" +) + +const ( + // ZoneAnnotationPrefix is the prefix used for placement decision annotations which will be set on VirtualMachineGroup. + ZoneAnnotationPrefix = "zone.vmware.infrastructure.cluster.x-k8s.io" +) + +// VirtualMachineGroupReconciler reconciles VirtualMachineGroup. +type VirtualMachineGroupReconciler struct { + Client client.Client + Recorder record.EventRecorder +} + +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters/status,verbs=get +// +kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinegroups,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinegroups/status,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=vmware.infrastructure.cluster.x-k8s.io,resources=vspheremachines,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;list;watch + +// This controller is introduced to coordinate the creation and maintenance of +// the VirtualMachineGroup (VMG) object with respect to the worker VSphereMachines in the Cluster. +// +// - Batch Coordination: Gating the initial creation of the VMG until for the first time all the +// MachineDeployment replicas will have a corresponding VSphereMachine. +// Once this condition is met, the VirtualMachineGroup is created considering +// the initial set of machines for the initial placement decision. +// When the VirtualMachineGroup reports the placement decision, then finally +// creation of VirtualMachines is unblocked. +// +// - Placement Persistence: Persisting the MachineDeployment-to-Zone mapping (placement decision) as a +// metadata annotation on the VMG object. The same decision must be respected also for placement +// of machines created after initial placement. +// +// - Membership Maintenance: Dynamically updating the VMG's member list to reflect the current +// state of VMs belonging to MachineDeployments (handling scale-up/down events). + +func (r *VirtualMachineGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + + // Fetch the Cluster instance. + cluster := &clusterv1.Cluster{} + if err := r.Client.Get(ctx, req.NamespacedName, cluster); err != nil { + if apierrors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } + + // Note: VirtualMachineGroup is going to have same name and namespace of the cluster. + // Using cluster here, because VirtualMachineGroup is created only after initial placement completes. + log = log.WithValues("VirtualMachineGroup", klog.KObj(cluster)) + ctx = ctrl.LoggerInto(ctx, log) + + // If Cluster is deleted, just return as VirtualMachineGroup will be GCed and no extra processing needed. + if !cluster.DeletionTimestamp.IsZero() { + return reconcile.Result{}, nil + } + + // If ControlPlane haven't initialized, requeue it since CAPV will only start to reconcile VSphereMachines of + // MachineDeployment after ControlPlane is initialized. + if !conditions.IsTrue(cluster, clusterv1.ClusterControlPlaneInitializedCondition) { + return reconcile.Result{}, nil + } + + return r.createOrUpdateVirtualMachineGroup(ctx, cluster) +} + +// createOrUpdateVirtualMachineGroup Create or Update VirtualMachineGroup. +func (r *VirtualMachineGroupReconciler) createOrUpdateVirtualMachineGroup(ctx context.Context, cluster *clusterv1.Cluster) (reconcile.Result, error) { + log := ctrl.LoggerFrom(ctx) + + // Get current VSphereMachines of all MachineDeployments. + currentVSphereMachines, err := getCurrentVSphereMachines(ctx, r.Client, cluster.Namespace, cluster.Name) + if err != nil { + return reconcile.Result{}, err + } + + vmg := &vmoprv1.VirtualMachineGroup{} + key := &client.ObjectKey{ + Namespace: cluster.Namespace, + Name: cluster.Name, + } + + if err := r.Client.Get(ctx, *key, vmg); err != nil { + if !apierrors.IsNotFound(err) { + return reconcile.Result{}, errors.Wrapf(err, "failed to get VirtualMachineGroup %s", klog.KObj(vmg)) + } + + // If the VirtualMachineGroup does not exist yet, + // calculate expected VSphereMachine count of all MachineDeployments. + expectedVSphereMachineCount, err := getExpectedVSphereMachineCount(ctx, r.Client, cluster) + if err != nil { + return reconcile.Result{}, errors.Wrapf(err, "failed to get expected Machines of all MachineDeployment, Cluster %s", klog.KObj(cluster)) + } + + // Since CAPV retrieves placement decisions from the VirtualMachineGroup to guide + // day-2 worker VM placement. At least one VM is expected for each MachineDeployment. + // If no worker of MachineDeployment is defined,the controller + // interprets this as an intentional configuration, just logs the observation and no-op. + if expectedVSphereMachineCount == 0 { + log.Info("Found 0 desired VSphereMachine of MachineDeployment, stop reconcile") + return reconcile.Result{}, nil + } + + // Wait for all intended VSphereMachines corresponding to MachineDeployment to exist only during initial Cluster creation. + // For day-2, VirtualMachineGroup exists and should not run into here wait for VSphereMachines. + currentVSphereMachineCount := int32(len(currentVSphereMachines)) + if currentVSphereMachineCount != expectedVSphereMachineCount { + log.Info("Waiting for expected VSphereMachines required for the initial placement call", "Expected:", expectedVSphereMachineCount, + "Current:", currentVSphereMachineCount, "Cluster", klog.KObj(cluster)) + return reconcile.Result{}, nil + } + + vmg = &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: key.Name, + Namespace: key.Namespace, + }, + } + } + + // Generate VM names according to the naming strategy set on the VSphereMachine. + vmNames := make([]string, 0, len(currentVSphereMachines)) + for _, machine := range currentVSphereMachines { + name, err := GenerateVirtualMachineName(machine.Name, machine.Spec.NamingStrategy) + if err != nil { + return reconcile.Result{}, err + } + vmNames = append(vmNames, name) + } + // Sort the VM names alphabetically for consistent ordering + slices.Sort(vmNames) + + members := make([]vmoprv1.GroupMember, 0, len(currentVSphereMachines)) + for _, name := range vmNames { + members = append(members, vmoprv1.GroupMember{ + Name: name, + Kind: "VirtualMachine", + }) + } + + // Get all the names of MachineDeployments of the Cluster. + machineDeployments := &clusterv1.MachineDeploymentList{} + if err := r.Client.List(ctx, machineDeployments, + client.InNamespace(cluster.Namespace), + client.MatchingLabels{clusterv1.ClusterNameLabel: cluster.Name}); err != nil { + return reconcile.Result{}, err + } + mdNames := []string{} + for _, md := range machineDeployments.Items { + // Skip MachineDeployment marked for removal. + if !md.DeletionTimestamp.IsZero() { + mdNames = append(mdNames, md.Name) + } + } + + // Use CreateOrPatch to create or update the VirtualMachineGroup. + _, err = controllerutil.CreateOrPatch(ctx, r.Client, vmg, func() error { + return r.reconcileVirtualMachineGroup(ctx, vmg, cluster, members, mdNames) + }) + + return reconcile.Result{}, err +} + +// reconcileVirtualMachineGroup mutates the VirtualMachineGroup object to reflect the necessary spec and metadata changes. +func (r *VirtualMachineGroupReconciler) reconcileVirtualMachineGroup(ctx context.Context, vmg *vmoprv1.VirtualMachineGroup, cluster *clusterv1.Cluster, members []vmoprv1.GroupMember, mdNames []string) error { + // Set the desired labels + if vmg.Labels == nil { + vmg.Labels = make(map[string]string) + } + // Always ensure cluster name label is set + vmg.Labels[clusterv1.ClusterNameLabel] = cluster.Name + + if vmg.Annotations == nil { + vmg.Annotations = make(map[string]string) + } + + // Add per-md-zone label for day-2 operations once placement of a VM belongs to MachineDeployment is done. + // Do not update per-md-zone label once set, as placement decision should not change without user explicitly + // set failureDomain. + if err := generateVirtualMachineGroupAnnotations(ctx, r.Client, vmg, mdNames); err != nil { + return err + } + + // Member Update: + // The VirtualMachineGroup's BootOrder.Members list, is only allowed to be set or added + // during two phases to maintain control over VM placement: + // + // 1. Initial Creation: When the VirtualMachineGroup object does not yet exist. + // 2. Post-Placement: After the VirtualMachineGroup exists AND is marked Ready which means all members are placed successfully, + // and critically, all MachineDeployments have a corresponding zone placement annotation recorded on the VMG. + // + // For member removal, this is always allowed since it doesn't impact ongoing placement or rely on the placement annotation. + // + // This prevents member updates that could lead to new VMs being created + // without necessary zone labels, resulting in undesired placement, such as VM within a MachineDeployment but are + // placed to different Zones. + + isMemberUpdateAllowed, err := isMemberUpdateAllowed(ctx, r.Client, members, vmg) + if err != nil { + return err + } + + if isMemberUpdateAllowed { + vmg.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{ + { + Members: members, + }, + } + } + + // Set the owner reference + if err := controllerutil.SetControllerReference(cluster, vmg, r.Client.Scheme()); err != nil { + return errors.Wrapf(err, "failed to mark Cluster %s as owner of VirtualMachineGroup %s", klog.KObj(cluster), klog.KObj(vmg)) + } + + return nil +} + +// isMemberUpdateAllowed determines if the BootOrder.Members field can be safely updated on the VirtualMachineGroup. +// It allows updates only during initial creation or after all member placement are completed successfully. +func isMemberUpdateAllowed(ctx context.Context, kubeClient client.Client, targetMember []vmoprv1.GroupMember, vmg *vmoprv1.VirtualMachineGroup) (bool, error) { + logger := log.FromContext(ctx) + key := client.ObjectKey{ + Namespace: vmg.Namespace, + Name: vmg.Name, + } + + // Retrieve the current VirtualMachineGroup state + currentVMG := &vmoprv1.VirtualMachineGroup{} + if err := kubeClient.Get(ctx, key, currentVMG); err != nil { + if apierrors.IsNotFound(err) { + // If VirtualMachineGroup is not found, allow member update as it should be in initial creation phase. + logger.V(5).Info("VirtualMachineGroup not found, allowing member update for initial creation.") + return true, nil + } + return false, errors.Wrapf(err, "failed to get VirtualMachineGroup %s/%s", vmg.Namespace, vmg.Name) + } + // Copy retrieved data back to the input pointer for consistency + *vmg = *currentVMG + + // Get current member names from VirtualMachineGroup Spec.BootOrder + currentMemberNames := make(map[string]struct{}) + if len(vmg.Spec.BootOrder) > 0 { + for _, m := range vmg.Spec.BootOrder[0].Members { + currentMemberNames[m.Name] = struct{}{} + } + } + + // 1. If removing members, allow immediately since it doesn't impact placement or placement annotation set. + if len(targetMember) < len(currentMemberNames) { + logger.V(5).Info("Scaling down detected (fewer target members), allowing member update.") + return true, nil + } + + // 2. If adding members, continue following checks. + var newMembers []vmoprv1.GroupMember + for _, m := range targetMember { + if _, exists := currentMemberNames[m.Name]; !exists { + newMembers = append(newMembers, m) + } + } + + // 3. Check newly added members for Machine.Spec.FailureDomain via VSphereMachine.If a member belongs to a Machine + // which has failureDomain specified, allow it since it will skip the placement + // process. If not, continue to check if the belonging MachineDeployment has got placement annotation. + for _, newMember := range newMembers { + vsphereMachineKey := types.NamespacedName{ + Namespace: vmg.Namespace, + Name: newMember.Name, // Member Name is the VSphereMachine Name. + } + vsphereMachine := &vmwarev1.VSphereMachine{} + if err := kubeClient.Get(ctx, vsphereMachineKey, vsphereMachine); err != nil { + if apierrors.IsNotFound(err) { + logger.V(5).Info("VSphereMachine for new member not found, temporarily blocking update.", "VSphereMachineName", newMember.Name) + return false, nil + } + return false, errors.Wrapf(err, "failed to get VSphereMachine %s", klog.KRef(newMember.Name, vmg.Namespace)) + } + + var machineOwnerName string + for _, owner := range vsphereMachine.OwnerReferences { + if owner.Kind == "Machine" { + machineOwnerName = owner.Name + break + } + } + + if machineOwnerName == "" { + // VSphereMachine found but owner Machine reference is missing + logger.V(5).Info("VSphereMachine found but owner Machine reference is missing, temporarily blocking update.", "VSphereMachineName", newMember.Name) + return false, nil + } + + machineKey := types.NamespacedName{ + Namespace: vmg.Namespace, + Name: machineOwnerName, + } + machine := &clusterv1.Machine{} + + if err := kubeClient.Get(ctx, machineKey, machine); err != nil { + if apierrors.IsNotFound(err) { + logger.V(5).Info("CAPI Machine not found via owner reference, temporarily blocking update.", "Machine", klog.KRef(machineOwnerName, vmg.Namespace)) + return false, nil + } + return false, errors.Wrapf(err, "failed to get CAPI Machine %s", klog.KRef(machineOwnerName, vmg.Namespace)) + } + + // If FailureDomain is set on CAPI Machine, placement process will be skipped. Allow update. + fd := machine.Spec.FailureDomain + if fd != "" { + logger.V(5).Info("New member's Machine has FailureDomain specified. Allowing VMG update for this member.") + continue + } + + // If FailureDomain is NOT set. Requires placement or placement Annotation. Fall through to full VMG Annotation check. + logger.V(5).Info("New member's CAPI Machine lacks FailureDomain. Falling through to full VMG Ready and Annotation check.", "MachineName", machineOwnerName) + + // If no Placement Annotations, skip member update and wait for it. + annotations := vmg.GetAnnotations() + if len(annotations) == 0 { + return false, nil + } + + mdLabelName := vsphereMachine.Labels[clusterv1.MachineDeploymentNameLabel] + + annotationKey := fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdLabelName) + + if _, found := annotations[annotationKey]; !found { + logger.V(5).Info("Required placement annotation is missing.", + "Member", newMember, "Annotation", annotationKey) + return false, nil + } + + logger.V(5).Info("New member requires placement annotation and it is present. Allowing this member.", "Member", newMember) + } + + logger.V(5).Info("Either no new members, or all newly added members existed or have satisfied placement requirements, allowing update.") + return true, nil +} + +// getExpectedVSphereMachineCount get expected total count of Machines belonging to the Cluster. +func getExpectedVSphereMachineCount(ctx context.Context, kubeClient client.Client, cluster *clusterv1.Cluster) (int32, error) { + var mdList clusterv1.MachineDeploymentList + if err := kubeClient.List( + ctx, + &mdList, + client.InNamespace(cluster.Namespace), + client.MatchingLabels{clusterv1.ClusterNameLabel: cluster.Name}, + ); err != nil { + return 0, errors.Wrap(err, "failed to list MachineDeployments") + } + + var total int32 + for _, md := range mdList.Items { + // Skip MachineDeployment marked for removal + if md.DeletionTimestamp.IsZero() && md.Spec.Replicas != nil { + total += *md.Spec.Replicas + } + } + + return total, nil +} + +// getCurrentVSphereMachines returns the list of VSphereMachines belonging to the Cluster’s MachineDeployments. +// VSphereMachines marked for removal are excluded from the result. +func getCurrentVSphereMachines(ctx context.Context, kubeClient client.Client, clusterNamespace, clusterName string) ([]vmwarev1.VSphereMachine, error) { + // List VSphereMachine objects + var vsMachineList vmwarev1.VSphereMachineList + if err := kubeClient.List(ctx, &vsMachineList, + client.InNamespace(clusterNamespace), + client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName}, + client.HasLabels{clusterv1.MachineDeploymentNameLabel}, + ); err != nil { + return nil, errors.Wrapf(err, "failed to list VSphereMachines of Cluster %s", klog.KRef(clusterNamespace, clusterName)) + } + + var result []vmwarev1.VSphereMachine + for _, vs := range vsMachineList.Items { + if vs.DeletionTimestamp.IsZero() { + result = append(result, vs) + } + } + return result, nil +} + +// generateVirtualMachineGroupAnnotations checks the VMG status for placed members, verifies their ownership +// by fetching the corresponding VSphereMachine, and extracts the zone information to persist it +// as an annotation on the VMG object for Day-2 operations. It will also clean up +// any existing placement annotations that correspond to MachineDeployments that no longer exist. +// +// The function attempts to find at least one successfully placed VM (VirtualMachineGroupMemberConditionPlacementReady==True) +// for each MachineDeployment and records its zone. Once a zone is recorded for an MD, subsequent VMs +// belonging to that same MD are skipped. +func generateVirtualMachineGroupAnnotations(ctx context.Context, kubeClient client.Client, vmg *vmoprv1.VirtualMachineGroup, machineDeployments []string) error { + log := ctrl.LoggerFrom(ctx) + log.V(5).Info(fmt.Sprintf("Generating annotations for VirtualMachineGroup %s/%s", vmg.Name, vmg.Namespace)) + + if vmg.Annotations == nil { + vmg.Annotations = make(map[string]string) + } + annotations := vmg.Annotations + + // If a MachineDeployment has been deleted, its corresponding placement annotation + // on the VirtualMachineGroup should also be removed to avoid configuration drift. + activeMDs := make(map[string]bool) + for _, md := range machineDeployments { + activeMDs[md] = true + } + + // Iterate over existing VirtualMachineGroup annotations and delete those that are stale. + for key := range annotations { + if !strings.HasPrefix(key, ZoneAnnotationPrefix+"/") { + // Skip non-placement annotations + continue + } + + mdName := strings.TrimPrefix(key, ZoneAnnotationPrefix+"/") + + // If the MD name is NOT in the list of currently active MDs, delete the annotation. + if found := activeMDs[mdName]; !found { + log.Info(fmt.Sprintf("Cleaning up stale placement annotation for none-existed MachineDeployment %s", mdName)) + delete(annotations, key) + } + } + + // Pre-computation: Convert the list of valid MachineDeployment names into a set. + mdNames := sets.New(machineDeployments...) + + // Iterate through the VMG's members in Status. + for _, member := range vmg.Status.Members { + ns := vmg.Namespace + + // Skip it if member's VirtualMachineGroupMemberConditionPlacementReady is still not true. + if !conditions.IsTrue(&member, vmoprv1.VirtualMachineGroupMemberConditionPlacementReady) { + continue + } + + // Get VSphereMachine which share the same Name of the member Name and get the MachineDeployment Name it belonged to. + vsmKey := types.NamespacedName{ + Name: member.Name, + Namespace: vmg.Namespace, + } + vsm := &vmwarev1.VSphereMachine{} + if err := kubeClient.Get(ctx, vsmKey, vsm); err != nil { + if apierrors.IsNotFound(err) { + log.Info(fmt.Sprintf("VSphereMachine %s/%s by member Name %s is not found, skipping it", member.Name, ns, member.Name)) + continue + } + return errors.Wrapf(err, "failed to get VSphereMachine %s/%s", member.Name, ns) + } + + mdName, found := vsm.Labels[clusterv1.MachineDeploymentNameLabel] + if !found { + log.Info(fmt.Sprintf("Failed to get MachineDeployment label from VSphereMachine %s/%s, skipping it", member.Name, ns)) + continue + } + + // If we already found placement for this MachineDeployment, continue and move to next member. + if _, found := annotations[fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName)]; found { + continue + } + + // Check if this VM belongs to any of our target MachineDeployments. + if !mdNames.Has(mdName) { + log.V(5).Info("Skipping member as its MachineDeployment name is not in the known list.", + "VMName", member.Name, "MDName", mdName) + continue + } + + // Get the VM placement information by member status. + // VMs that have undergone placement do not have Placement info set, skip. + if member.Placement == nil { + log.V(5).Info(fmt.Sprintf("VM %s in VMG %s/%s has no placement info. Placement is nil", member.Name, vmg.Name, ns)) + continue + } + + // Skip to next member if Zone is empty. + zone := member.Placement.Zone + if zone == "" { + log.V(5).Info(fmt.Sprintf("VM %s in VMG %s/%s has no placement info. Zone is empty", member.Name, "VMG", ns)) + continue + } + + log.V(5).Info(fmt.Sprintf("VM %s in VMG %s/%s has been placed in zone %s", member.Name, ns, vmg.Name, zone)) + annotations[fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName)] = zone + } + + return nil +} + +// GenerateVirtualMachineName generates the name of a VirtualMachine based on the naming strategy. +// Duplicated this logic from pkg/services/vmoperator/vmopmachine.go. +func GenerateVirtualMachineName(machineName string, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) (string, error) { + // Per default the name of the VirtualMachine should be equal to the Machine name (this is the same as "{{ .machine.name }}") + if namingStrategy == nil || namingStrategy.Template == nil { + // Note: No need to trim to max length in this case as valid Machine names will also be valid VirtualMachine names. + return machineName, nil + } + + name, err := infrautilv1.GenerateMachineNameFromTemplate(machineName, namingStrategy.Template) + if err != nil { + return "", errors.Wrapf(err, "failed to generate name for VirtualMachine %s", machineName) + } + + return name, nil +} diff --git a/controllers/vmware/virtualmachinegroup_reconciler_test.go b/controllers/vmware/virtualmachinegroup_reconciler_test.go new file mode 100644 index 0000000000..d8bed93b50 --- /dev/null +++ b/controllers/vmware/virtualmachinegroup_reconciler_test.go @@ -0,0 +1,929 @@ +/* +Copyright 2025 The Kubernetes Authors. + +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 vmware + +import ( + "context" + "fmt" + "sort" + "testing" + "time" + + . "github.com/onsi/gomega" + vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + "k8s.io/utils/ptr" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/util/conditions" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" +) + +const ( + clusterName = "test-cluster" + otherClusterName = "other-cluster" + clusterNamespace = "test-ns" + mdName1 = "md-worker-a" + mdName2 = "md-worker-b" + mdNameStale = "md-stale-c" + zoneA = "zone-a" + zoneB = "zone-b" + vmgName = "test-vmg" + vmgNamespace = "test-vmg-ns" + memberName1 = "vm-01" + memberName2 = "vm-02" + memberKind = "VirtualMachine" + failureDomainA = "zone-1" +) + +func TestIsMemberUpdateAllowed(t *testing.T) { + ctx := context.Background() + + baseVMG := &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{Name: vmgName, Namespace: vmgNamespace}, + Status: vmoprv1.VirtualMachineGroupStatus{}, + Spec: vmoprv1.VirtualMachineGroupSpec{}, + } + + member := func(name string) vmoprv1.GroupMember { return vmoprv1.GroupMember{Name: name} } + + // CAPI Machine helpers + makeCAPIMachine := func(name, namespace string, fd *string) *clusterv1.Machine { + m := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + } + if fd != nil { + m.Spec = clusterv1.MachineSpec{FailureDomain: *fd} + } + return m + } + makeUnplacedCAPIMachine := func(name, namespace string) *clusterv1.Machine { + return makeCAPIMachine(name, namespace, nil) + } + + // VSphereMachine helpers + makeVSphereMachineOwned := func(vmName, vmgNamespace, ownerMachineName, mdName string) *vmwarev1.VSphereMachine { + return &vmwarev1.VSphereMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmName, + Namespace: vmgNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Machine", + Name: ownerMachineName, + UID: types.UID(ownerMachineName + "-uid"), + }, + }, + Labels: map[string]string{fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName): "zone-1"}, + }, + } + } + makeVSphereMachineNoOwner := func(vmName, ns string) *vmwarev1.VSphereMachine { + return &vmwarev1.VSphereMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmName, + Namespace: ns, + OwnerReferences: []metav1.OwnerReference{}, + }, + } + } + + tests := []struct { + name string + targetMember []vmoprv1.GroupMember + vmgInput *vmoprv1.VirtualMachineGroup + mdNames []string + existingObjects []runtime.Object + wantAllowed bool + wantErr bool + }{ + { + name: "Allow member update if VirtualMachineGroup not existed", + targetMember: []vmoprv1.GroupMember{member(memberName1)}, + vmgInput: &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{Name: vmgName, Namespace: vmgNamespace}, + }, + mdNames: []string{mdName1}, + existingObjects: nil, + wantAllowed: true, + wantErr: false, + }, + { + name: "Allow member update if it is removing", + targetMember: []vmoprv1.GroupMember{}, + vmgInput: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{ + {Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }}}} + return v + }(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{ + {Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }}}} + return []runtime.Object{v} + }(), + wantAllowed: true, + wantErr: false, + }, + { + name: "Allow member update when VMG Ready and All Annotations Present", + targetMember: []vmoprv1.GroupMember{member(memberName1)}, + vmgInput: baseVMG.DeepCopy(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + conditions.Set(v, metav1.Condition{ + Type: vmoprv1.ReadyConditionType, + Status: metav1.ConditionTrue}) + v.Annotations = map[string]string{fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName1): zoneA} + + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + + return []runtime.Object{v} + }(), + wantAllowed: true, + wantErr: false, + }, + { + name: "Skip member update if new member VSphereMachine Not Found", + targetMember: []vmoprv1.GroupMember{member(memberName1), member(memberName2)}, // vm-02 is new + vmgInput: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return v + }(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + // vm-02 VSphereMachine is missing + return []runtime.Object{v, makeVSphereMachineOwned(memberName1, vmgNamespace, "m-01", mdName1), makeCAPIMachine("m-01", vmgNamespace, ptr.To(failureDomainA))} + }(), + wantAllowed: false, + wantErr: false, + }, + { + name: "Skip member update if VSphereMachine found but CAPI Machine missing", + targetMember: []vmoprv1.GroupMember{member(memberName1), member(memberName2)}, // vm-02 is new + vmgInput: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return v + }(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + // vm-02 VSphereMachine exists but has no owner ref + return []runtime.Object{v, makeVSphereMachineOwned(memberName1, vmgNamespace, "m-01", mdName1), makeCAPIMachine("m-01", vmgNamespace, ptr.To(failureDomainA)), makeVSphereMachineNoOwner(memberName2, vmgNamespace)} + }(), + wantAllowed: false, + wantErr: false, + }, + { + name: "Allow member update if all new members have Machine FailureDomain specified", + targetMember: []vmoprv1.GroupMember{member(memberName1), member(memberName2)}, // vm-02 is new + vmgInput: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return v + }(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + // m-02 (owner of vm-02) has FailureDomain set -> Allowed + return []runtime.Object{ + v, + makeVSphereMachineOwned(memberName1, vmgNamespace, "m-01", mdName1), makeCAPIMachine("m-01", vmgNamespace, nil), + makeVSphereMachineOwned(memberName2, vmgNamespace, "m-02", mdName2), makeCAPIMachine("m-02", vmgNamespace, ptr.To(failureDomainA)), + } + }(), + wantAllowed: true, // Allowed because new members don't require VMO placement + wantErr: false, + }, + { + name: "Allow member update if no new member", + targetMember: []vmoprv1.GroupMember{member(memberName1)}, // No new members + vmgInput: baseVMG.DeepCopy(), + mdNames: []string{mdName1}, // Expects mdName1 annotation + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + // Annotation for mdName1 is missing + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return []runtime.Object{v} + }(), + wantAllowed: true, + wantErr: false, + }, + { + name: "Skip member update if new member Machine requires placement annotation", + targetMember: []vmoprv1.GroupMember{member(memberName1), member(memberName2)}, // vm-02 is new and requires placement + vmgInput: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return v + }(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + // m-02 lacks FailureDomain and new Member requires placement annotation + return []runtime.Object{ + v, + makeVSphereMachineOwned(memberName1, vmgNamespace, "m-01", mdName1), makeCAPIMachine("m-01", vmgNamespace, ptr.To(failureDomainA)), + makeVSphereMachineOwned(memberName2, vmgNamespace, "m-02", mdName2), makeUnplacedCAPIMachine("m-02", vmgNamespace), + } + }(), + wantAllowed: false, + wantErr: false, + }, + { + name: "Allow new member Machine since required placement annotation exists", + targetMember: []vmoprv1.GroupMember{member(memberName1), member(memberName2)}, // vm-02 is new and requires placement + vmgInput: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return v + }(), + mdNames: []string{mdName1}, + existingObjects: func() []runtime.Object { + v := baseVMG.DeepCopy() + v.Annotations = map[string]string{fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName1): zoneA} + v.Spec.BootOrder = []vmoprv1.VirtualMachineGroupBootOrderGroup{{Members: []vmoprv1.GroupMember{ + { + Name: memberName1, + Kind: memberKind, + }, + }}} + return []runtime.Object{ + v, + makeVSphereMachineOwned(memberName1, vmgNamespace, "m-01", mdName1), makeCAPIMachine("m-01", vmgNamespace, ptr.To(failureDomainA)), + makeVSphereMachineOwned(memberName2, vmgNamespace, "m-02", mdName2), makeUnplacedCAPIMachine("m-02", vmgNamespace), + } + }(), + wantAllowed: true, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kubeClient := fake.NewClientBuilder().WithRuntimeObjects(tt.existingObjects...).Build() + + vmgInput := tt.vmgInput.DeepCopy() + + gotAllowed, err := isMemberUpdateAllowed(ctx, kubeClient, tt.targetMember, vmgInput) + + if (err != nil) != tt.wantErr { + t.Fatalf("isMemberUpdateAllowed() error = %v, wantErr %v", err, tt.wantErr) + } + + if gotAllowed != tt.wantAllowed { + t.Errorf("isMemberUpdateAllowed() gotAllowed = %t, wantAllowed %t", gotAllowed, tt.wantAllowed) + } + }) + } +} + +func TestGetExpectedVSphereMachineCount(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + scheme := runtime.NewScheme() + g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) + + targetCluster := newTestCluster(clusterName, clusterNamespace) + + mdA := newMachineDeployment("md-a", clusterName, clusterNamespace, true, ptr.To(int32(3))) + mdB := newMachineDeployment("md-b", clusterName, clusterNamespace, true, ptr.To(int32(5))) + mdCNil := newMachineDeployment("md-c-nil", clusterName, clusterNamespace, false, nil) + mdDZero := newMachineDeployment("md-d-zero", clusterName, clusterNamespace, true, ptr.To(int32(0))) + // Create an MD for a different cluster (should be filtered) + mdOtherCluster := newMachineDeployment("md-other", otherClusterName, clusterNamespace, true, ptr.To(int32(5))) + + tests := []struct { + name string + initialObjects []client.Object + expectedTotal int32 + wantErr bool + }{ + { + name: "Sum of two MDs", + initialObjects: []client.Object{mdA, mdB}, + expectedTotal: 8, + wantErr: false, + }, + { + name: "Should get count when MDs include nil and zero replicas", + initialObjects: []client.Object{mdA, mdB, mdCNil, mdDZero}, + expectedTotal: 8, + wantErr: false, + }, + { + name: "Should filters out MDs from other clusters", + initialObjects: []client.Object{mdA, mdB, mdOtherCluster}, + expectedTotal: 8, + wantErr: false, + }, + { + name: "Should succeed when no MachineDeployments found", + initialObjects: []client.Object{}, + expectedTotal: 0, + wantErr: false, + }, + } + + for _, tt := range tests { + // Looks odd, but need to reinitialize test variable + tt := tt + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialObjects...).Build() + total, err := getExpectedVSphereMachineCount(ctx, fakeClient, targetCluster) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(total).To(Equal(tt.expectedTotal)) + } + }) + } +} + +func TestGetCurrentVSphereMachines(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + scheme := runtime.NewScheme() + g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed()) + + // VSphereMachine names are based on CAPI Machine names, but we use fake name here. + vsmName1 := fmt.Sprintf("%s-%s", mdName1, "vsm-1") + vsmName2 := fmt.Sprintf("%s-%s", mdName2, "vsm-2") + vsm1 := newVSphereMachine(vsmName1, mdName1, false, false, nil) + vsm2 := newVSphereMachine(vsmName2, mdName2, false, false, nil) + vsmDeleting := newVSphereMachine("vsm-3", mdName1, false, true, nil) // Deleting + vsmControlPlane := newVSphereMachine("vsm-cp", "not-md", true, false, nil) + + tests := []struct { + name string + objects []client.Object + want int + }{ + { + name: "Should filtered out deleting VSphereMachines", + objects: []client.Object{ + vsm1, + vsm2, + vsmDeleting, + vsmControlPlane, + }, + want: 2, + }, + { + name: "Want no Error if no VSphereMachines found", + objects: []client.Object{}, + want: 0, + }, + } + + for _, tt := range tests { + // Looks odd, but need to reinitialize test variable + tt := tt + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build() + got, err := getCurrentVSphereMachines(ctx, fakeClient, clusterNamespace, clusterName) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(HaveLen(tt.want)) + + // Check that the correct Machines are present + if tt.want > 0 { + names := make([]string, len(got)) + for i, vsm := range got { + names[i] = vsm.Name + } + sort.Strings(names) + g.Expect(names).To(Equal([]string{vsmName1, vsmName2})) + } + }) + } +} +func TestGenerateVirtualMachineGroupAnnotations(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + scheme := runtime.NewScheme() + g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed()) + + baseVMG := &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: clusterNamespace, + Annotations: make(map[string]string), + }, + } + + // VSphereMachines corresponding to the VMG members + vsmName1 := fmt.Sprintf("%s-%s", mdName1, "vsm-1") + vsmName2 := fmt.Sprintf("%s-%s", mdName2, "vsm-2") + vsm1 := newVSphereMachine(vsmName1, mdName1, false, false, nil) + vsm2 := newVSphereMachine(vsmName2, mdName2, false, false, nil) + vsmMissingLabel := newVSphereMachine("vsm-nolabel", mdName2, false, false, nil) + vsmMissingLabel.Labels = nil // Explicitly remove labels for test case + + tests := []struct { + name string + vmg *vmoprv1.VirtualMachineGroup + machineDeployments []string + initialClientObjects []client.Object + expectedAnnotations map[string]string + wantErr bool + }{ + { + name: "Placement found for two distinct MDs", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus(vsmName1, "VirtualMachine", true, true, zoneA), + newVMGMemberStatus(vsmName2, "VirtualMachine", true, true, zoneB), + }, + } + return v + }(), + machineDeployments: []string{mdName1, mdName2}, + initialClientObjects: []client.Object{vsm1, vsm2}, + expectedAnnotations: map[string]string{ + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName1): zoneA, + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName2): zoneB, + }, + wantErr: false, + }, + { + name: "Skip as placement already exists in VMG Annotations", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Annotations = map[string]string{fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName1): zoneA} + v.Status.Members = []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus(vsmName1, "VirtualMachine", true, true, zoneB), + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{vsm1}, + // Should retain existing zone-a + expectedAnnotations: map[string]string{ + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName1): zoneA, + }, + wantErr: false, + }, + { + name: "Skip if VSphereMachine Missing MachineDeployment Label", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus("vsm-nolabel", "VirtualMachine", true, true, zoneA), + }, + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{vsmMissingLabel}, + expectedAnnotations: map[string]string{}, + wantErr: false, + }, + { + name: "Skip if VSphereMachine is Not Found in API", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus("non-existent-vm", "VirtualMachine", true, true, zoneA), + }, + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{vsm1}, + expectedAnnotations: map[string]string{}, + wantErr: false, + }, + { + name: "Skip if placement is nil", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus(vsmName1, "VirtualMachine", true, false, zoneA), + }, + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{vsm1}, + expectedAnnotations: map[string]string{}, + wantErr: false, + }, + { + name: "Skip if Zone is empty string", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus(vsmName1, "VirtualMachine", true, true, ""), + }, + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{vsm1}, + expectedAnnotations: map[string]string{}, + wantErr: false, + }, + { + name: "Deletes stale annotation for none-existed MD", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + // This MD (mdNameStale) is NOT in the machineDeployments list below. + v.SetAnnotations(map[string]string{ + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdNameStale): zoneA, + "other/annotation": "keep-me", + }) + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{}, + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{}, + expectedAnnotations: map[string]string{ + "other/annotation": "keep-me", + }, + wantErr: false, + }, + { + name: "Cleans stale and adds new annotations", + vmg: func() *vmoprv1.VirtualMachineGroup { + v := baseVMG.DeepCopy() + // Stale annotation to be deleted + v.SetAnnotations(map[string]string{ + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdNameStale): zoneB, + }) + v.Status = vmoprv1.VirtualMachineGroupStatus{ + Members: []vmoprv1.VirtualMachineGroupMemberStatus{ + newVMGMemberStatus(vsmName1, "VirtualMachine", true, true, zoneA), + }, + } + return v + }(), + machineDeployments: []string{mdName1}, + initialClientObjects: []client.Object{vsm1}, + expectedAnnotations: map[string]string{ + // Stale annotation for mdNameStale should be gone + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName1): zoneA, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + // Looks odd, but need to reinitialize test variable + tt := tt + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialClientObjects...).Build() + err := generateVirtualMachineGroupAnnotations(ctx, fakeClient, tt.vmg, tt.machineDeployments) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(tt.vmg.Annotations).To(Equal(tt.expectedAnnotations)) + } + }) + } +} + +func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + scheme := runtime.NewScheme() + g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) + g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed()) + g.Expect(vmoprv1.AddToScheme(scheme)).To(Succeed()) + + // Initial objects for the successful VMG creation path (Expected: 1, Current: 1) + cluster := newCluster(clusterName, clusterNamespace, true, 1, 0) + vsm1 := newVSphereMachine("vsm-1", mdName1, false, false, nil) + md1 := newMachineDeployment(mdName1, clusterName, clusterNamespace, true, ptr.To(int32(1))) + + tests := []struct { + name string + initialObjects []client.Object + expectedResult reconcile.Result + checkVMGExists bool + }{ + { + name: "Should Exit if Cluster Not Found", + initialObjects: []client.Object{}, + expectedResult: reconcile.Result{}, + checkVMGExists: false, + }, + { + name: "Should Exit if Cluster Deletion Timestamp Set", + initialObjects: []client.Object{ + func() client.Object { + c := cluster.DeepCopy() + c.Finalizers = []string{"test.finalizer.cluster"} + c.DeletionTimestamp = &metav1.Time{Time: time.Now()} + return c + }(), + }, + expectedResult: reconcile.Result{}, + checkVMGExists: false, + }, + { + name: "Should Requeue if ControlPlane Not Initialized", + initialObjects: []client.Object{ + newCluster(clusterName, clusterNamespace, false, 1, 0), + }, + expectedResult: reconcile.Result{}, + checkVMGExists: false, + }, + { + name: "Should Requeue if VMG Not Found", + initialObjects: []client.Object{ + cluster.DeepCopy(), + md1.DeepCopy(), + }, + expectedResult: reconcile.Result{}, + checkVMGExists: false, + }, + { + name: "Should Succeed to create VMG", + initialObjects: []client.Object{ + cluster.DeepCopy(), + md1.DeepCopy(), + vsm1.DeepCopy(), + }, + expectedResult: reconcile.Result{}, + checkVMGExists: true, + }, + { + name: "Should Succeed if VMG is already existed", + initialObjects: []client.Object{ + cluster.DeepCopy(), + md1.DeepCopy(), + vsm1.DeepCopy(), + &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: clusterNamespace, + Labels: map[string]string{clusterv1.ClusterNameLabel: cluster.Name}, + }, + Spec: vmoprv1.VirtualMachineGroupSpec{ + BootOrder: []vmoprv1.VirtualMachineGroupBootOrderGroup{ + { + Members: []vmoprv1.GroupMember{ + { + Name: vsm1.Name, + Kind: "VSphereMachine", + }, + }, + }, + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + // Looks odd, but need to reinitialize test variable + tt := tt + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialObjects...).Build() + reconciler := &VirtualMachineGroupReconciler{ + Client: fakeClient, + Recorder: record.NewFakeRecorder(1), + } + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: clusterName, Namespace: clusterNamespace}} + + result, err := reconciler.Reconcile(ctx, req) + + g.Expect(err).NotTo(HaveOccurred(), "Reconcile should not return an error") + g.Expect(result).To(Equal(tt.expectedResult)) + + vmg := &vmoprv1.VirtualMachineGroup{} + vmgKey := types.NamespacedName{Name: clusterName, Namespace: clusterNamespace} + err = fakeClient.Get(ctx, vmgKey, vmg) + + if tt.checkVMGExists { + g.Expect(err).NotTo(HaveOccurred(), "VMG should exist") + // Check that the core fields were set by the MutateFn + g.Expect(vmg.Labels).To(HaveKeyWithValue(clusterv1.ClusterNameLabel, clusterName)) + g.Expect(vmg.Spec.BootOrder).To(HaveLen(1)) + expected, err := getExpectedVSphereMachineCount(ctx, fakeClient, tt.initialObjects[0].(*clusterv1.Cluster)) + g.Expect(err).NotTo(HaveOccurred(), "Should get expected Machines") + g.Expect(vmg.Spec.BootOrder[0].Members).To(HaveLen(int(expected))) + + // VMG members should match the VSphereMachine name + g.Expect(vmg.Spec.BootOrder[0].Members[0].Name).To(Equal("vsm-1")) + } + }) + } +} + +// Helper function to create a basic Cluster object. +func newCluster(name, namespace string, initialized bool, replicasMD1, replicasMD2 int32) *clusterv1.Cluster { + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{clusterv1.ClusterNameLabel: name}, + }, + Spec: clusterv1.ClusterSpec{ + Topology: clusterv1.Topology{ + Workers: clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + {Name: mdName1, Replicas: &replicasMD1}, + {Name: mdName2, Replicas: &replicasMD2}, + }, + }, + }, + }, + } + if initialized { + conditions.Set(cluster, metav1.Condition{ + Type: clusterv1.ClusterControlPlaneInitializedCondition, + Status: metav1.ConditionTrue, + }) + } + return cluster +} + +// Helper function to create a VSphereMachine (worker, owned by a CAPI Machine). +func newVSphereMachine(name, mdName string, isCP, deleted bool, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) *vmwarev1.VSphereMachine { + vsm := &vmwarev1.VSphereMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: clusterNamespace, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: clusterName, + }, + }, + Spec: vmwarev1.VSphereMachineSpec{ + NamingStrategy: namingStrategy, + }, + } + if !isCP { + vsm.Labels[clusterv1.MachineDeploymentNameLabel] = mdName + } else { + vsm.Labels[clusterv1.MachineControlPlaneLabel] = "true" + } + if deleted { + vsm.Finalizers = []string{"test.finalizer.0"} + vsm.DeletionTimestamp = &metav1.Time{Time: time.Now()} + } + return vsm +} + +// Helper function to create a VMG member status with placement info. +func newVMGMemberStatus(name, kind string, isPlacementReady, placement bool, zone string) vmoprv1.VirtualMachineGroupMemberStatus { + memberStatus := vmoprv1.VirtualMachineGroupMemberStatus{ + Name: name, + Kind: kind, + } + + if isPlacementReady { + conditions.Set(&memberStatus, metav1.Condition{ + Type: vmoprv1.VirtualMachineGroupMemberConditionPlacementReady, + Status: metav1.ConditionTrue, + }) + } + + if placement { + memberStatus.Placement = &vmoprv1.VirtualMachinePlacementStatus{Zone: zone} + } + + return memberStatus +} + +// Helper function to create a MachineDeployment object. +func newMachineDeployment(name, clusterName, clusterNS string, isReplicaSet bool, replicas *int32) *clusterv1.MachineDeployment { + md := &clusterv1.MachineDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: clusterNS, + Labels: map[string]string{clusterv1.ClusterNameLabel: clusterName}, + }, + } + + if isReplicaSet { + md.Spec = clusterv1.MachineDeploymentSpec{ + Replicas: replicas, + } + } + + return md +} + +// Helper function to create a basic Cluster object used as input. +func newTestCluster(name, namespace string) *clusterv1.Cluster { + return &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } +} diff --git a/feature/feature.go b/feature/feature.go index a233d351c7..1799aaeb68 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -44,6 +44,11 @@ const ( // alpha: v1.11 NamespaceScopedZones featuregate.Feature = "NamespaceScopedZones" + // NodeAutoPlacement is a feature gate for the NodeAutoPlacement functionality for supervisor. + // + // alpha: v1.15 + NodeAutoPlacement featuregate.Feature = "NodeAutoPlacement" + // PriorityQueue is a feature gate that controls if the controller uses the controller-runtime PriorityQueue // instead of the default queue implementation. // @@ -61,6 +66,7 @@ var defaultCAPVFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ // Every feature should be initiated here: NodeAntiAffinity: {Default: false, PreRelease: featuregate.Alpha}, NamespaceScopedZones: {Default: false, PreRelease: featuregate.Alpha}, + NodeAutoPlacement: {Default: false, PreRelease: featuregate.Alpha}, PriorityQueue: {Default: false, PreRelease: featuregate.Alpha}, MultiNetworks: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/go.mod b/go.mod index 44f52fa9ea..df97e28e79 100644 --- a/go.mod +++ b/go.mod @@ -4,16 +4,16 @@ go 1.24.0 replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.11.0-rc.0.0.20250905091528-eb4e38c46ff6 -replace github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels => github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels v0.0.0-20240404200847-de75746a9505 +replace github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels => github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels v1.9.1-0.20251003150112-9b458d311c4c // The version of vm-operator should be kept in sync with the manifests at: config/deployments/integration-tests -replace github.com/vmware-tanzu/vm-operator/api => github.com/vmware-tanzu/vm-operator/api v1.8.6 +replace github.com/vmware-tanzu/vm-operator/api => github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c require ( github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d // The version of vm-operator should be kept in sync with the manifests at: config/deployments/integration-tests - github.com/vmware-tanzu/vm-operator/api v1.8.6 + github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505 github.com/vmware/govmomi v0.52.0 ) diff --git a/go.sum b/go.sum index 47a16466b0..34bb470a23 100644 --- a/go.sum +++ b/go.sum @@ -243,8 +243,8 @@ github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d h1:c github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d/go.mod h1:JbFOh22iDsT5BowJe0GgpMI5e2/S7cWaJlv9LdURVQM= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d h1:z9lrzKVtNlujduv9BilzPxuge/LE2F0N1ms3TP4JZvw= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d/go.mod h1:Q4JzNkNMvjo7pXtlB5/R3oME4Nhah7fAObWgghVmtxk= -github.com/vmware-tanzu/vm-operator/api v1.8.6 h1:NIndORjcnSmIlQsCMIewpIwg/ocRVDh2lYjOroTVLrU= -github.com/vmware-tanzu/vm-operator/api v1.8.6/go.mod h1:HHA2SNI9B5Yqtyp5t+Gt9WTWBi/fIkM6+MukDDSf11A= +github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c h1:XISTT0dw/XwMlyyiOPHPsXCxfI1Ro2Zuozi6eIacXGo= +github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c/go.mod h1:nWTPpxfe4gHuuYuFcrs86+NMxfkqPk3a3IlvI8TCWak= github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505 h1:y4wXx1FUFqqSgJ/xUOEM1DLS2Uu0KaeLADWpzpioGTU= github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505/go.mod h1:5rqRJ9zGR+KnKbkGx373WgN8xJpvAj99kHnfoDYRO5I= github.com/vmware/govmomi v0.52.0 h1:JyxQ1IQdllrY7PJbv2am9mRsv3p9xWlIQ66bv+XnyLw= diff --git a/internal/test/helpers/envtest.go b/internal/test/helpers/envtest.go index 41341b70cb..0acbcd68eb 100644 --- a/internal/test/helpers/envtest.go +++ b/internal/test/helpers/envtest.go @@ -29,6 +29,7 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/pkg/errors" + vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" "github.com/vmware/govmomi/simulator" "golang.org/x/tools/go/packages" admissionv1 "k8s.io/api/admissionregistration/v1" @@ -89,6 +90,7 @@ func init() { utilruntime.Must(admissionv1.AddToScheme(scheme)) utilruntime.Must(clusterv1.AddToScheme(scheme)) utilruntime.Must(infrav1.AddToScheme(scheme)) + utilruntime.Must(vmoprv1.AddToScheme(scheme)) // Get the root of the current file to use in CRD paths. _, filename, _, ok := goruntime.Caller(0) diff --git a/main.go b/main.go index b92f48d25a..6d6ea9e011 100644 --- a/main.go +++ b/main.go @@ -94,6 +94,7 @@ var ( vSphereVMConcurrency int vSphereClusterIdentityConcurrency int vSphereDeploymentZoneConcurrency int + virtualMachineGroupConcurrency int skipCRDMigrationPhases []string managerOptions = capiflags.ManagerOptions{} @@ -141,6 +142,9 @@ func InitFlags(fs *pflag.FlagSet) { fs.IntVar(&vSphereDeploymentZoneConcurrency, "vspheredeploymentzone-concurrency", 10, "Number of vSphere deployment zones to process simultaneously") + fs.IntVar(&virtualMachineGroupConcurrency, "virtualmachinegroup-concurrency", 10, + "Number of virtual machine group to process simultaneously") + fs.StringVar( &managerOpts.PodName, "pod-name", @@ -482,6 +486,12 @@ func setupSupervisorControllers(ctx context.Context, controllerCtx *capvcontext. return err } + if feature.Gates.Enabled(feature.NamespaceScopedZones) && feature.Gates.Enabled(feature.NodeAutoPlacement) { + if err := vmware.AddVirtualMachineGroupControllerToManager(ctx, controllerCtx, mgr, concurrency(virtualMachineGroupConcurrency)); err != nil { + return err + } + } + return vmware.AddServiceDiscoveryControllerToManager(ctx, controllerCtx, mgr, clusterCache, concurrency(serviceDiscoveryConcurrency)) } diff --git a/packaging/go.sum b/packaging/go.sum index 14a389257b..0659c3663f 100644 --- a/packaging/go.sum +++ b/packaging/go.sum @@ -135,8 +135,8 @@ github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d h1:c github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d/go.mod h1:JbFOh22iDsT5BowJe0GgpMI5e2/S7cWaJlv9LdURVQM= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d h1:z9lrzKVtNlujduv9BilzPxuge/LE2F0N1ms3TP4JZvw= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d/go.mod h1:Q4JzNkNMvjo7pXtlB5/R3oME4Nhah7fAObWgghVmtxk= -github.com/vmware-tanzu/vm-operator/api v1.8.6 h1:NIndORjcnSmIlQsCMIewpIwg/ocRVDh2lYjOroTVLrU= -github.com/vmware-tanzu/vm-operator/api v1.8.6/go.mod h1:HHA2SNI9B5Yqtyp5t+Gt9WTWBi/fIkM6+MukDDSf11A= +github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c h1:XISTT0dw/XwMlyyiOPHPsXCxfI1Ro2Zuozi6eIacXGo= +github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c/go.mod h1:nWTPpxfe4gHuuYuFcrs86+NMxfkqPk3a3IlvI8TCWak= github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505 h1:y4wXx1FUFqqSgJ/xUOEM1DLS2Uu0KaeLADWpzpioGTU= github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505/go.mod h1:5rqRJ9zGR+KnKbkGx373WgN8xJpvAj99kHnfoDYRO5I= github.com/vmware/govmomi v0.52.0 h1:JyxQ1IQdllrY7PJbv2am9mRsv3p9xWlIQ66bv+XnyLw= diff --git a/pkg/services/network/netop_provider.go b/pkg/services/network/netop_provider.go index fa1c1860fa..e13de3bd4d 100644 --- a/pkg/services/network/netop_provider.go +++ b/pkg/services/network/netop_provider.go @@ -136,7 +136,7 @@ func (np *netopNetworkProvider) ConfigureVirtualMachine(ctx context.Context, clu // Set the VM primary interface vm.Spec.Network.Interfaces = append(vm.Spec.Network.Interfaces, vmoprv1.VirtualMachineNetworkInterfaceSpec{ Name: PrimaryInterfaceName, - Network: vmoprv1common.PartialObjectRef{ + Network: &vmoprv1common.PartialObjectRef{ TypeMeta: metav1.TypeMeta{ Kind: NetworkGVKNetOperator.Kind, APIVersion: NetworkGVKNetOperator.GroupVersion().String(), diff --git a/pkg/services/network/nsxt_provider.go b/pkg/services/network/nsxt_provider.go index 96a0450bb7..90885cb568 100644 --- a/pkg/services/network/nsxt_provider.go +++ b/pkg/services/network/nsxt_provider.go @@ -223,7 +223,7 @@ func (np *nsxtNetworkProvider) ConfigureVirtualMachine(_ context.Context, cluste } vm.Spec.Network.Interfaces = append(vm.Spec.Network.Interfaces, vmoprv1.VirtualMachineNetworkInterfaceSpec{ Name: fmt.Sprintf("eth%d", len(vm.Spec.Network.Interfaces)), - Network: vmoprv1common.PartialObjectRef{ + Network: &vmoprv1common.PartialObjectRef{ TypeMeta: metav1.TypeMeta{ Kind: NetworkGVKNSXT.Kind, APIVersion: NetworkGVKNSXT.GroupVersion().String(), diff --git a/pkg/services/network/nsxt_vpc_provider.go b/pkg/services/network/nsxt_vpc_provider.go index 0c3533a37c..9b2c8defa0 100644 --- a/pkg/services/network/nsxt_vpc_provider.go +++ b/pkg/services/network/nsxt_vpc_provider.go @@ -224,7 +224,7 @@ func (vp *nsxtVPCNetworkProvider) ConfigureVirtualMachine(_ context.Context, clu networkName := clusterCtx.VSphereCluster.Name vm.Spec.Network.Interfaces = append(vm.Spec.Network.Interfaces, vmoprv1.VirtualMachineNetworkInterfaceSpec{ Name: PrimaryInterfaceName, - Network: vmoprv1common.PartialObjectRef{ + Network: &vmoprv1common.PartialObjectRef{ TypeMeta: metav1.TypeMeta{ Kind: NetworkGVKNSXTVPCSubnetSet.Kind, APIVersion: NetworkGVKNSXTVPCSubnetSet.GroupVersion().String(), @@ -243,7 +243,7 @@ func (vp *nsxtVPCNetworkProvider) ConfigureVirtualMachine(_ context.Context, clu } vmInterface := vmoprv1.VirtualMachineNetworkInterfaceSpec{ Name: PrimaryInterfaceName, - Network: vmoprv1common.PartialObjectRef{ + Network: &vmoprv1common.PartialObjectRef{ TypeMeta: metav1.TypeMeta{ Kind: primary.Network.Kind, APIVersion: primary.Network.APIVersion, @@ -281,7 +281,7 @@ func setVMSecondaryInterfaces(machine *vmwarev1.VSphereMachine, vm *vmoprv1.Virt } vmInterface := vmoprv1.VirtualMachineNetworkInterfaceSpec{ Name: secondaryInterface.Name, - Network: vmoprv1common.PartialObjectRef{ + Network: &vmoprv1common.PartialObjectRef{ TypeMeta: metav1.TypeMeta{ Kind: secondaryInterface.Network.Kind, APIVersion: secondaryInterface.Network.APIVersion, diff --git a/pkg/services/vmoperator/constants.go b/pkg/services/vmoperator/constants.go index 011082a06c..37ca556fc6 100644 --- a/pkg/services/vmoperator/constants.go +++ b/pkg/services/vmoperator/constants.go @@ -18,8 +18,6 @@ limitations under the License. package vmoperator const ( - kubeTopologyZoneLabelKey = "topology.kubernetes.io/zone" - // ControlPlaneVMClusterModuleGroupName is the name used for the control plane Cluster Module. ControlPlaneVMClusterModuleGroupName = "control-plane-group" // ClusterModuleNameAnnotationKey is key for the Cluster Module annotation. diff --git a/pkg/services/vmoperator/control_plane_endpoint.go b/pkg/services/vmoperator/control_plane_endpoint.go index e0070188e3..3b500711d7 100644 --- a/pkg/services/vmoperator/control_plane_endpoint.go +++ b/pkg/services/vmoperator/control_plane_endpoint.go @@ -189,7 +189,7 @@ func newVirtualMachineService(ctx *vmware.ClusterContext) *vmoprv1.VirtualMachin Namespace: ctx.Cluster.Namespace, }, TypeMeta: metav1.TypeMeta{ - APIVersion: vmoprv1.SchemeGroupVersion.String(), + APIVersion: vmoprv1.GroupVersion.String(), Kind: "VirtualMachineService", }, } diff --git a/pkg/services/vmoperator/vmopmachine.go b/pkg/services/vmoperator/vmopmachine.go index 840b166406..98b1a2e308 100644 --- a/pkg/services/vmoperator/vmopmachine.go +++ b/pkg/services/vmoperator/vmopmachine.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "sort" "github.com/pkg/errors" vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" @@ -30,6 +31,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apitypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/klog/v2" "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" @@ -41,11 +43,17 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/feature" capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/vmware" infrautilv1 "sigs.k8s.io/cluster-api-provider-vsphere/pkg/util" ) +const ( + // ZoneAnnotationPrefix is the prefix used for placement decision annotations which will be set on VirtualMachineGroup. + ZoneAnnotationPrefix = "zone.vmware.infrastructure.cluster.x-k8s.io" +) + // VmopMachineService reconciles VM Operator VM. type VmopMachineService struct { Client client.Client @@ -163,6 +171,13 @@ func (v *VmopMachineService) SyncFailureReason(_ context.Context, machineCtx cap return supervisorMachineCtx.VSphereMachine.Status.FailureReason != nil || supervisorMachineCtx.VSphereMachine.Status.FailureMessage != nil, nil } +// affinityInfo is an internal struct used to store information about VM affinity. +type affinityInfo struct { + affinitySpec vmoprv1.AffinitySpec + vmGroupName string + failureDomain string +} + // ReconcileNormal reconciles create and update events for VM Operator VMs. func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx capvcontext.MachineContext) (bool, error) { log := ctrl.LoggerFrom(ctx) @@ -171,10 +186,6 @@ func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx cap return false, errors.New("received unexpected SupervisorMachineContext type") } - if supervisorMachineCtx.Machine.Spec.FailureDomain != "" { - supervisorMachineCtx.VSphereMachine.Spec.FailureDomain = ptr.To(supervisorMachineCtx.Machine.Spec.FailureDomain) - } - // If debug logging is enabled, report the number of vms in the cluster before and after the reconcile if log.V(5).Enabled() { vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx) @@ -188,6 +199,120 @@ func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx cap // Set the VM state. Will get reset throughout the reconcile supervisorMachineCtx.VSphereMachine.Status.VMStatus = vmwarev1.VirtualMachineStatePending + var affInfo *affinityInfo + if feature.Gates.Enabled(feature.NodeAutoPlacement) && + !infrautilv1.IsControlPlaneMachine(machineCtx.GetVSphereMachine()) { + vmOperatorVMGroup := &vmoprv1.VirtualMachineGroup{} + key := client.ObjectKey{ + Namespace: supervisorMachineCtx.Cluster.Namespace, + Name: supervisorMachineCtx.Cluster.Name, + } + err := v.Client.Get(ctx, key, vmOperatorVMGroup) + if err != nil { + if !apierrors.IsNotFound(err) { + return false, err + } + + v1beta2conditions.Set(supervisorMachineCtx.VSphereMachine, metav1.Condition{ + Type: infrav1.VSphereMachineVirtualMachineProvisionedV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereMachineVirtualMachineWaitingForVirtualMachineGroupV1Beta2Reason, + }) + log.V(4).Info(fmt.Sprintf("Waiting for VirtualMachineGroup %s, requeueing", key.Name), "VirtualMachineGroup", klog.KRef(key.Namespace, key.Name)) + return true, nil + } + + // Proceed only if the machine is a member of the VirtualMachineGroup. + isMember, err := v.checkVirtualMachineGroupMembership(vmOperatorVMGroup, supervisorMachineCtx) + if err != nil { + return true, errors.Wrapf(err, "%s", fmt.Sprintf("failed to check if VirtualMachine %s is a member of VirtualMachineGroup %s", supervisorMachineCtx.VSphereMachine.Name, klog.KObj(vmOperatorVMGroup))) + } + if !isMember { + v1beta2conditions.Set(supervisorMachineCtx.VSphereMachine, metav1.Condition{ + Type: infrav1.VSphereMachineVirtualMachineProvisionedV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: infrav1.VSphereMachineVirtualMachineWaitingForVirtualMachineGroupV1Beta2Reason, + }) + log.V(4).Info(fmt.Sprintf("Waiting for VirtualMachineGroup %s membership, requeueing", key.Name), "VirtualMachineGroup", klog.KRef(key.Namespace, key.Name)) + return true, nil + } + + affInfo = &affinityInfo{ + vmGroupName: vmOperatorVMGroup.Name, + } + + // Set the zone label using the annotation of the per-md zone mapping from VirtualMachineGroup. + // This is for new VMs created during day-2 operations when Node Auto Placement is enabled. + mdName := supervisorMachineCtx.Machine.Labels[clusterv1.MachineDeploymentNameLabel] + if fd, ok := vmOperatorVMGroup.Annotations[fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, mdName)]; ok && fd != "" { + affInfo.failureDomain = fd + } + + // Fetch machine deployments without explicit failureDomain specified + // to use when setting the anti-affinity rules. + machineDeployments := &clusterv1.MachineDeploymentList{} + if err := v.Client.List(ctx, machineDeployments, + client.InNamespace(supervisorMachineCtx.Cluster.Namespace), + client.MatchingLabels{clusterv1.ClusterNameLabel: supervisorMachineCtx.Cluster.Name}); err != nil { + return false, err + } + othermMDNames := []string{} + for _, machineDeployment := range machineDeployments.Items { + if machineDeployment.Spec.Template.Spec.FailureDomain == "" && machineDeployment.Name != mdName { + othermMDNames = append(othermMDNames, machineDeployment.Name) + } + } + sort.Strings(othermMDNames) + + affInfo.affinitySpec = vmoprv1.AffinitySpec{ + VMAffinity: &vmoprv1.VMAffinitySpec{ + RequiredDuringSchedulingPreferredDuringExecution: []vmoprv1.VMAffinityTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + TopologyKey: corev1.LabelTopologyZone, + }, + }, + }, + VMAntiAffinity: &vmoprv1.VMAntiAffinitySpec{ + PreferredDuringSchedulingPreferredDuringExecution: []vmoprv1.VMAffinityTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + TopologyKey: corev1.LabelHostname, + }, + }, + }, + } + if len(othermMDNames) > 0 { + affInfo.affinitySpec.VMAntiAffinity.PreferredDuringSchedulingPreferredDuringExecution = append( + affInfo.affinitySpec.VMAntiAffinity.PreferredDuringSchedulingPreferredDuringExecution, + vmoprv1.VMAffinityTerm{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: clusterv1.MachineDeploymentNameLabel, + Operator: metav1.LabelSelectorOpIn, + Values: othermMDNames, + }, + }, + }, + TopologyKey: corev1.LabelTopologyZone, + }, + ) + } + } + + if supervisorMachineCtx.Machine.Spec.FailureDomain != "" { + supervisorMachineCtx.VSphereMachine.Spec.FailureDomain = ptr.To(supervisorMachineCtx.Machine.Spec.FailureDomain) + } + // Check for the presence of an existing object vmOperatorVM := &vmoprv1.VirtualMachine{} key, err := virtualMachineObjectKey(supervisorMachineCtx.Machine.Name, supervisorMachineCtx.Machine.Namespace, supervisorMachineCtx.VSphereMachine.Spec.NamingStrategy) @@ -208,7 +333,7 @@ func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx cap } // Reconcile the VM Operator VirtualMachine. - if err := v.reconcileVMOperatorVM(ctx, supervisorMachineCtx, vmOperatorVM); err != nil { + if err := v.reconcileVMOperatorVM(ctx, supervisorMachineCtx, vmOperatorVM, affInfo); err != nil { v1beta1conditions.MarkFalse(supervisorMachineCtx.VSphereMachine, infrav1.VMProvisionedCondition, vmwarev1.VMCreationFailedReason, clusterv1beta1.ConditionSeverityWarning, "failed to create or update VirtualMachine: %v", err) v1beta2conditions.Set(supervisorMachineCtx.VSphereMachine, metav1.Condition{ @@ -378,7 +503,7 @@ func (v *VmopMachineService) GetHostInfo(ctx context.Context, machineCtx capvcon return vmOperatorVM.Status.Host, nil } -func (v *VmopMachineService) reconcileVMOperatorVM(ctx context.Context, supervisorMachineCtx *vmware.SupervisorMachineContext, vmOperatorVM *vmoprv1.VirtualMachine) error { +func (v *VmopMachineService) reconcileVMOperatorVM(ctx context.Context, supervisorMachineCtx *vmware.SupervisorMachineContext, vmOperatorVM *vmoprv1.VirtualMachine, affinityInfo *affinityInfo) error { // All Machine resources should define the version of Kubernetes to use. if supervisorMachineCtx.Machine.Spec.Version == "" { return errors.Errorf( @@ -472,7 +597,7 @@ func (v *VmopMachineService) reconcileVMOperatorVM(ctx context.Context, supervis } // Assign the VM's labels. - vmOperatorVM.Labels = getVMLabels(supervisorMachineCtx, vmOperatorVM.Labels) + vmOperatorVM.Labels = getVMLabels(supervisorMachineCtx, vmOperatorVM.Labels, affinityInfo) addResourcePolicyAnnotations(supervisorMachineCtx, vmOperatorVM) @@ -494,6 +619,23 @@ func (v *VmopMachineService) reconcileVMOperatorVM(ctx context.Context, supervis vmOperatorVM = typedModified } + // Set VM Affinity rules and GroupName. + // The Affinity rules set in Spec.Affinity primarily take effect only during the + // initial placement. + // These rules DO NOT impact new VMs created after initial placement, such as scaling up, + // because placement relies on information derived from + // VirtualMachineGroup annotations. This ensures all the VMs + // for a MachineDeployment are placed in the same failureDomain. + // Note: no matter of the different placement behaviour, we are setting affinity rules on all machines for consistency. + if affinityInfo != nil { + if vmOperatorVM.Spec.Affinity == nil { + vmOperatorVM.Spec.Affinity = &affinityInfo.affinitySpec + } + if vmOperatorVM.Spec.GroupName == "" { + vmOperatorVM.Spec.GroupName = affinityInfo.vmGroupName + } + } + // Make sure the VSphereMachine owns the VM Operator VirtualMachine. if err := ctrlutil.SetControllerReference(supervisorMachineCtx.VSphereMachine, vmOperatorVM, v.Client.Scheme()); err != nil { return errors.Wrapf(err, "failed to mark %s %s/%s as owner of %s %s/%s", @@ -731,11 +873,15 @@ func (v *VmopMachineService) addVolumes(ctx context.Context, supervisorMachineCt // which is required when the cluster has multiple (3) zones. // Single zone clusters (legacy/default) do not support zonal storage and must not // have the zone annotation set. + // + // However, with Node Auto Placement enabled, failureDomain is optional and CAPV no longer + // sets PVC annotations when creating worker VMs. PVC placement now follows the StorageClass behavior (Immediate or WaitForFirstConsumer). + // Control Plane VMs will still have failureDomain set, and we will set PVC annotation. zonal := len(supervisorMachineCtx.VSphereCluster.Status.FailureDomains) > 1 if zone := supervisorMachineCtx.VSphereMachine.Spec.FailureDomain; zonal && zone != nil { topology := []map[string]string{ - {kubeTopologyZoneLabelKey: *zone}, + {corev1.LabelTopologyZone: *zone}, } b, err := json.Marshal(topology) if err != nil { @@ -777,7 +923,7 @@ func (v *VmopMachineService) addVolumes(ctx context.Context, supervisorMachineCt } // getVMLabels returns the labels applied to a VirtualMachine. -func getVMLabels(supervisorMachineCtx *vmware.SupervisorMachineContext, vmLabels map[string]string) map[string]string { +func getVMLabels(supervisorMachineCtx *vmware.SupervisorMachineContext, vmLabels map[string]string, affinityInfo *affinityInfo) map[string]string { if vmLabels == nil { vmLabels = map[string]string{} } @@ -789,9 +935,14 @@ func getVMLabels(supervisorMachineCtx *vmware.SupervisorMachineContext, vmLabels vmLabels[k] = v } - // Get the labels that determine the VM's placement inside of a stretched - // cluster. - topologyLabels := getTopologyLabels(supervisorMachineCtx) + // Set the labels that determine the VM's placement. + // Note: if the failureDomain is not set, auto placement will happen according to affinity rules on VM during initial Cluster creation. + // For VM created during day-2 operation like scaling up, we should expect the failureDomain to be always set. + var failureDomain string + if affinityInfo != nil && affinityInfo.failureDomain != "" { + failureDomain = affinityInfo.failureDomain + } + topologyLabels := getTopologyLabels(supervisorMachineCtx, failureDomain) for k, v := range topologyLabels { vmLabels[k] = v } @@ -800,6 +951,11 @@ func getVMLabels(supervisorMachineCtx *vmware.SupervisorMachineContext, vmLabels // resources associated with the target cluster. vmLabels[clusterv1.ClusterNameLabel] = supervisorMachineCtx.GetClusterContext().Cluster.Name + // Ensure the VM has the machine deployment name label + if !infrautilv1.IsControlPlaneMachine(supervisorMachineCtx.Machine) { + vmLabels[clusterv1.MachineDeploymentNameLabel] = supervisorMachineCtx.Machine.Labels[clusterv1.MachineDeploymentNameLabel] + } + return vmLabels } @@ -809,10 +965,17 @@ func getVMLabels(supervisorMachineCtx *vmware.SupervisorMachineContext, vmLabels // // and thus the code is optimized as such. However, in the future // this function may return a more diverse topology. -func getTopologyLabels(supervisorMachineCtx *vmware.SupervisorMachineContext) map[string]string { +func getTopologyLabels(supervisorMachineCtx *vmware.SupervisorMachineContext, failureDomain string) map[string]string { + // This is for explicit placement. if fd := supervisorMachineCtx.VSphereMachine.Spec.FailureDomain; fd != nil && *fd != "" { return map[string]string{ - kubeTopologyZoneLabelKey: *fd, + corev1.LabelTopologyZone: *fd, + } + } + // This is for automatic placement. + if failureDomain != "" { + return map[string]string{ + corev1.LabelTopologyZone: failureDomain, } } return nil @@ -823,3 +986,20 @@ func getTopologyLabels(supervisorMachineCtx *vmware.SupervisorMachineContext) ma func getMachineDeploymentNameForCluster(cluster *clusterv1.Cluster) string { return fmt.Sprintf("%s-workers-0", cluster.Name) } + +// checkVirtualMachineGroupMembership checks if the machine is in the first boot order group +// and performs logic if a match is found. +func (v *VmopMachineService) checkVirtualMachineGroupMembership(vmOperatorVMGroup *vmoprv1.VirtualMachineGroup, supervisorMachineCtx *vmware.SupervisorMachineContext) (bool, error) { + if len(vmOperatorVMGroup.Spec.BootOrder) > 0 { + for _, member := range vmOperatorVMGroup.Spec.BootOrder[0].Members { + virtualMachineName, err := GenerateVirtualMachineName(supervisorMachineCtx.Machine.Name, supervisorMachineCtx.VSphereMachine.Spec.NamingStrategy) + if err != nil { + return false, err + } + if member.Name == virtualMachineName { + return true, nil + } + } + } + return false, nil +} diff --git a/pkg/services/vmoperator/vmopmachine_test.go b/pkg/services/vmoperator/vmopmachine_test.go index aa91556341..f2f80789ad 100644 --- a/pkg/services/vmoperator/vmopmachine_test.go +++ b/pkg/services/vmoperator/vmopmachine_test.go @@ -18,6 +18,8 @@ package vmoperator import ( "context" + "fmt" + "slices" "testing" "time" @@ -32,6 +34,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" @@ -40,6 +43,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/feature" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/fake" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/vmware" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/services/network" @@ -65,6 +69,49 @@ func updateReconciledVMStatus(ctx context.Context, vmService VmopMachineService, Expect(err).ShouldNot(HaveOccurred()) } +func verifyVMAffinityRules(vmopVM *vmoprv1.VirtualMachine, machineDeploymentName string) { + Expect(vmopVM.Spec.Affinity.VMAffinity).ShouldNot(BeNil()) + Expect(vmopVM.Spec.Affinity.VMAffinity.RequiredDuringSchedulingPreferredDuringExecution).To(HaveLen(1)) + + vmAffinityTerm := vmopVM.Spec.Affinity.VMAffinity.RequiredDuringSchedulingPreferredDuringExecution[0] + Expect(vmAffinityTerm.LabelSelector.MatchLabels).To(HaveKeyWithValue(clusterv1.MachineDeploymentNameLabel, machineDeploymentName)) + Expect(vmAffinityTerm.TopologyKey).To(Equal(corev1.LabelTopologyZone)) +} + +func verifyVMAntiAffinityRules(vmopVM *vmoprv1.VirtualMachine, machineDeploymentName string, extraMDs ...string) { + Expect(vmopVM.Spec.Affinity.VMAntiAffinity).ShouldNot(BeNil()) + + expectedNumAntiAffinityTerms := 1 + if len(extraMDs) > 0 { + expectedNumAntiAffinityTerms = 2 + } + + antiAffinityTerms := vmopVM.Spec.Affinity.VMAntiAffinity.PreferredDuringSchedulingPreferredDuringExecution + Expect(antiAffinityTerms).To(HaveLen(expectedNumAntiAffinityTerms)) + + // First anti-affinity constraint - same machine deployment, different hosts + antiAffinityTerm1 := antiAffinityTerms[0] + Expect(antiAffinityTerm1.LabelSelector.MatchLabels).To(HaveKeyWithValue(clusterv1.MachineDeploymentNameLabel, machineDeploymentName)) + Expect(antiAffinityTerm1.TopologyKey).To(Equal(corev1.LabelHostname)) + + // Second anti-affinity term - different machine deployments + if len(extraMDs) > 0 { + isSortedAlphabetically := func(actual []string) (bool, error) { + return slices.IsSorted(actual), nil + } + antiAffinityTerm2 := antiAffinityTerms[1] + Expect(antiAffinityTerm2.LabelSelector.MatchExpressions).To(HaveLen(1)) + Expect(antiAffinityTerm2.LabelSelector.MatchExpressions[0].Key).To(Equal(clusterv1.MachineDeploymentNameLabel)) + Expect(antiAffinityTerm2.LabelSelector.MatchExpressions[0].Operator).To(Equal(metav1.LabelSelectorOpIn)) + + Expect(antiAffinityTerm2.LabelSelector.MatchExpressions[0].Values).To(HaveLen(len(extraMDs))) + Expect(antiAffinityTerm2.LabelSelector.MatchExpressions[0].Values).To( + WithTransform(isSortedAlphabetically, BeTrue()), + "Expected extra machine deployments to be sorted alphabetically", + ) + } +} + const ( machineName = "test-machine" clusterName = "test-cluster" @@ -81,6 +128,32 @@ const ( clusterNameLabel = clusterv1.ClusterNameLabel ) +func createMachineDeployment(name, namespace, clusterName, failureDomain string) *clusterv1.MachineDeployment { + md := &clusterv1.MachineDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: clusterName, + }, + }, + Spec: clusterv1.MachineDeploymentSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + // FailureDomain will be set conditionally below + }, + }, + }, + } + + // Only set failure domain if it's provided and not empty + if failureDomain != "" { + md.Spec.Template.Spec.FailureDomain = failureDomain + } + + return md +} + var _ = Describe("VirtualMachine tests", func() { var ( @@ -655,6 +728,347 @@ var _ = Describe("VirtualMachine tests", func() { Expect(vmopVM.Spec.Volumes[i]).To(BeEquivalentTo(vmVolume)) } }) + + Context("With node auto placement feature gate enabled", func() { + BeforeEach(func() { + t := GinkgoT() + featuregatetesting.SetFeatureGateDuringTest(t, feature.Gates, feature.NodeAutoPlacement, true) + }) + + // control plane machine is the machine with the control plane label set + Specify("Reconcile valid control plane Machine", func() { + // Control plane machines should not have auto placement logic applied + expectReconcileError = false + expectVMOpVM = true + expectedImageName = imageName + expectedRequeue = true + + // Provide valid bootstrap data + By("bootstrap data is created") + secretName := machine.GetName() + "-data" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: machine.GetNamespace(), + }, + Data: map[string][]byte{ + "value": []byte(bootstrapData), + }, + } + Expect(vmService.Client.Create(ctx, secret)).To(Succeed()) + + machine.Spec.Bootstrap.DataSecretName = &secretName + expectedConditions = append(expectedConditions, clusterv1beta1.Condition{ + Type: infrav1.VMProvisionedCondition, + Status: corev1.ConditionFalse, + Reason: vmwarev1.VMProvisionStartedReason, + Message: "", + }) + + By("VirtualMachine is created") + requeue, err = vmService.ReconcileNormal(ctx, supervisorMachineContext) + verifyOutput(supervisorMachineContext) + + By("Verify that control plane machine does not have affinity spec set") + vmopVM = getReconciledVM(ctx, vmService, supervisorMachineContext) + Expect(vmopVM).ShouldNot(BeNil()) + Expect(vmopVM.Spec.Affinity).To(BeNil()) + + By("Verify that control plane machine has correct labels") + Expect(vmopVM.Labels[nodeSelectorKey]).To(Equal(roleControlPlane)) + + By("Verify that machine-deployment label is not set for control plane") + Expect(vmopVM.Labels).ToNot(HaveKey(clusterv1.MachineDeploymentNameLabel)) + }) + + Context("For worker machine", func() { + var ( + machineDeploymentName string + vmGroup *vmoprv1.VirtualMachineGroup + ) + + BeforeEach(func() { + // Create a worker machine (no control plane label) + machineDeploymentName = "test-md" + workerMachineName := "test-worker-machine" + machine = util.CreateMachine(workerMachineName, clusterName, k8sVersion, false) + machine.Labels[clusterv1.MachineDeploymentNameLabel] = machineDeploymentName + + vsphereMachine = util.CreateVSphereMachine(workerMachineName, clusterName, className, imageName, storageClass, false) + + clusterContext, controllerManagerContext := util.CreateClusterContext(cluster, vsphereCluster) + supervisorMachineContext = util.CreateMachineContext(clusterContext, machine, vsphereMachine) + supervisorMachineContext.ControllerManagerContext = controllerManagerContext + + // Create a VirtualMachineGroup for the cluster + vmGroup = &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: corev1.NamespaceDefault, + }, + Spec: vmoprv1.VirtualMachineGroupSpec{ + BootOrder: []vmoprv1.VirtualMachineGroupBootOrderGroup{ + { + Members: []vmoprv1.GroupMember{ + { + Name: workerMachineName, + Kind: "VirtualMachine", + }, + }, + }, + }, + }, + } + Expect(vmService.Client.Create(ctx, vmGroup)).To(Succeed()) + + // Create a MachineDeployment for the worker + machineDeployment := createMachineDeployment(machineDeploymentName, corev1.NamespaceDefault, clusterName, "") + Expect(vmService.Client.Create(ctx, machineDeployment)).To(Succeed()) + }) + + Specify("Requeue valid Machine but not a member of the VirtualMachineGroup yet", func() { + machineDeploymentNotMemberName := "test-md-not-member" + workerMachineNotMember := "test-worker-machine-not-member" + machineNotMember := util.CreateMachine(workerMachineNotMember, clusterName, k8sVersion, false) + machineNotMember.Labels[clusterv1.MachineDeploymentNameLabel] = machineDeploymentNotMemberName + + vsphereMachineNotMember := util.CreateVSphereMachine(workerMachineNotMember, clusterName, className, imageName, storageClass, false) + + clusterContext, controllerManagerContext := util.CreateClusterContext(cluster, vsphereCluster) + supervisorMachineContext = util.CreateMachineContext(clusterContext, machineNotMember, vsphereMachineNotMember) + supervisorMachineContext.ControllerManagerContext = controllerManagerContext + + // Create a MachineDeployment for the worker + machineDeploymentNotMember := createMachineDeployment(machineDeploymentNotMemberName, corev1.NamespaceDefault, clusterName, "") + Expect(vmService.Client.Create(ctx, machineDeploymentNotMember)).To(Succeed()) + + expectReconcileError = false + expectVMOpVM = false + expectedImageName = imageName + expectedRequeue = true + + // Provide valid bootstrap data + By("bootstrap data is created") + secretName := machineNotMember.GetName() + "-data" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: machineNotMember.GetNamespace(), + }, + Data: map[string][]byte{ + "value": []byte(bootstrapData), + }, + } + Expect(vmService.Client.Create(ctx, secret)).To(Succeed()) + + machineNotMember.Spec.Bootstrap.DataSecretName = &secretName + + By("VirtualMachine is not created") + requeue, err = vmService.ReconcileNormal(ctx, supervisorMachineContext) + Expect(err).ShouldNot(HaveOccurred()) + Expect(requeue).Should(BeTrue()) + }) + + Specify("Reconcile valid Machine with no failure domain set", func() { + expectReconcileError = false + expectVMOpVM = true + expectedImageName = imageName + expectedRequeue = true + + // Provide valid bootstrap data + By("bootstrap data is created") + secretName := machine.GetName() + "-data" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: machine.GetNamespace(), + }, + Data: map[string][]byte{ + "value": []byte(bootstrapData), + }, + } + Expect(vmService.Client.Create(ctx, secret)).To(Succeed()) + + machine.Spec.Bootstrap.DataSecretName = &secretName + + By("VirtualMachine is created") + requeue, err = vmService.ReconcileNormal(ctx, supervisorMachineContext) + Expect(err).ShouldNot(HaveOccurred()) + Expect(requeue).Should(BeTrue()) + + By("Verify that worker machine has affinity spec set") + vmopVM = getReconciledVM(ctx, vmService, supervisorMachineContext) + Expect(vmopVM).ShouldNot(BeNil()) + Expect(vmopVM.Spec.Affinity).ShouldNot(BeNil()) + + By("Verify VM affinity rules are set correctly") + verifyVMAffinityRules(vmopVM, machineDeploymentName) + + By("Verify VM anti-affinity rules are set correctly") + verifyVMAntiAffinityRules(vmopVM, machineDeploymentName) + + By("Verify that worker machine has machine deployment label set") + Expect(vmopVM.Labels[clusterv1.MachineDeploymentNameLabel]).To(Equal(machineDeploymentName)) + + By("Verify that GroupName is set from VirtualMachineGroup") + Expect(vmopVM.Spec.GroupName).To(Equal(clusterName)) + }) + + Specify("Reconcile machine with failure domain set", func() { + expectReconcileError = false + expectVMOpVM = true + expectedImageName = imageName + expectedRequeue = true + + failureDomainName := "zone-1" + machineDeploymentName := "test-md-with-fd" + workerMachineName := "test-worker-machine-with-fd" + fdClusterName := "test-cluster-fd" + + // Create a separate cluster for this test to avoid VirtualMachineGroup conflicts + fdCluster := util.CreateCluster(fdClusterName) + fdVSphereCluster := util.CreateVSphereCluster(fdClusterName) + fdVSphereCluster.Status.ResourcePolicyName = resourcePolicyName + + // Create a worker machine with failure domain + machine = util.CreateMachine(workerMachineName, fdClusterName, k8sVersion, false) + machine.Labels[clusterv1.MachineDeploymentNameLabel] = machineDeploymentName + machine.Spec.FailureDomain = failureDomainName + + vsphereMachine = util.CreateVSphereMachine(workerMachineName, fdClusterName, className, imageName, storageClass, false) + + fdClusterContext, fdControllerManagerContext := util.CreateClusterContext(fdCluster, fdVSphereCluster) + supervisorMachineContext = util.CreateMachineContext(fdClusterContext, machine, vsphereMachine) + supervisorMachineContext.ControllerManagerContext = fdControllerManagerContext + + // Create a VirtualMachineGroup for the cluster with per-md zone annotation + vmGroup := &vmoprv1.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: fdClusterName, + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{ + fmt.Sprintf("%s/%s", ZoneAnnotationPrefix, machineDeploymentName): failureDomainName, + }, + }, + Spec: vmoprv1.VirtualMachineGroupSpec{ + BootOrder: []vmoprv1.VirtualMachineGroupBootOrderGroup{ + { + Members: []vmoprv1.GroupMember{ + { + Name: workerMachineName, + Kind: "VirtualMachine", + }, + }, + }, + }, + }, + } + Expect(vmService.Client.Create(ctx, vmGroup)).To(Succeed()) + + // Create a MachineDeployment for the worker with no explicit failure domain + machineDeployment := createMachineDeployment(machineDeploymentName, corev1.NamespaceDefault, fdClusterName, "") + Expect(vmService.Client.Create(ctx, machineDeployment)).To(Succeed()) + + // Provide valid bootstrap data + By("bootstrap data is created") + secretName := machine.GetName() + "-data" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: machine.GetNamespace(), + }, + Data: map[string][]byte{ + "value": []byte(bootstrapData), + }, + } + Expect(vmService.Client.Create(ctx, secret)).To(Succeed()) + + machine.Spec.Bootstrap.DataSecretName = &secretName + + By("VirtualMachine is created with auto placement and failure domain") + requeue, err = vmService.ReconcileNormal(ctx, supervisorMachineContext) + Expect(err).ShouldNot(HaveOccurred()) + Expect(requeue).Should(BeTrue()) + + By("Verify that worker machine has affinity spec set") + vmopVM = getReconciledVM(ctx, vmService, supervisorMachineContext) + Expect(vmopVM).ShouldNot(BeNil()) + Expect(vmopVM.Spec.Affinity).ShouldNot(BeNil()) + + By("Verify VM affinity rules are set correctly") + verifyVMAffinityRules(vmopVM, machineDeploymentName) + + By("Verify VM anti-affinity rules are set correctly") + verifyVMAntiAffinityRules(vmopVM, machineDeploymentName) + + By("Verify that worker machine has correct labels including topology") + Expect(vmopVM.Labels[clusterv1.MachineDeploymentNameLabel]).To(Equal(machineDeploymentName)) + Expect(vmopVM.Labels[corev1.LabelTopologyZone]).To(Equal(failureDomainName)) + + By("Verify that GroupName is set from VirtualMachineGroup") + Expect(vmopVM.Spec.GroupName).To(Equal(fdClusterName)) + }) + + Context("For multiple machine deployments", func() { + const ( + otherMdName1 = "other-md-1" + otherMdName2 = "other-md-2" + ) + + BeforeEach(func() { + otherMd1 := createMachineDeployment(otherMdName1, corev1.NamespaceDefault, clusterName, "") + Expect(vmService.Client.Create(ctx, otherMd1)).To(Succeed()) + + otherMd2 := createMachineDeployment(otherMdName2, corev1.NamespaceDefault, clusterName, "") + Expect(vmService.Client.Create(ctx, otherMd2)).To(Succeed()) + + // Create a MachineDeployment with failure domain + otherMdWithFd := createMachineDeployment("other-md-with-fd", corev1.NamespaceDefault, clusterName, "zone-1") + Expect(vmService.Client.Create(ctx, otherMdWithFd)).To(Succeed()) + }) + + Specify("Reconcile valid machine with additional anti-affinity term added", func() { + expectReconcileError = false + expectVMOpVM = true + expectedImageName = imageName + expectedRequeue = true + + // Provide valid bootstrap data + By("bootstrap data is created") + secretName := machine.GetName() + "-data" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: machine.GetNamespace(), + }, + Data: map[string][]byte{ + "value": []byte(bootstrapData), + }, + } + Expect(vmService.Client.Create(ctx, secret)).To(Succeed()) + + machine.Spec.Bootstrap.DataSecretName = &secretName + + By("VirtualMachine is created") + requeue, err = vmService.ReconcileNormal(ctx, supervisorMachineContext) + Expect(err).ShouldNot(HaveOccurred()) + Expect(requeue).Should(BeTrue()) + + By("Verify that worker machine has affinity spec set") + vmopVM = getReconciledVM(ctx, vmService, supervisorMachineContext) + Expect(vmopVM).ShouldNot(BeNil()) + Expect(vmopVM.Spec.Affinity).ShouldNot(BeNil()) + + By("Verify VM affinity rules are set correctly") + verifyVMAffinityRules(vmopVM, machineDeploymentName) + + By("Verify VM anti-affinity rules are set correctly") + verifyVMAntiAffinityRules(vmopVM, machineDeploymentName, otherMdName1, otherMdName2) + }) + }) + }) + + }) }) Context("Delete tests", func() { diff --git a/test/framework/vmoperator/vmoperator.go b/test/framework/vmoperator/vmoperator.go index c80ec76545..2c1e367b01 100644 --- a/test/framework/vmoperator/vmoperator.go +++ b/test/framework/vmoperator/vmoperator.go @@ -534,7 +534,7 @@ func ReconcileDependencies(ctx context.Context, c client.Client, dependenciesCon Namespace: config.Namespace, }, Spec: vmoprv1.VirtualMachineImageSpec{ - ProviderRef: vmoprv1common.LocalObjectRef{ + ProviderRef: &vmoprv1common.LocalObjectRef{ Kind: "ContentLibraryItem", }, }, diff --git a/test/go.mod b/test/go.mod index bcab8743c0..db1b6ea8b6 100644 --- a/test/go.mod +++ b/test/go.mod @@ -8,15 +8,15 @@ replace sigs.k8s.io/cluster-api/test => sigs.k8s.io/cluster-api/test v1.11.0-rc. replace sigs.k8s.io/cluster-api-provider-vsphere => ../ -replace github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels => github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels v0.0.0-20240404200847-de75746a9505 +replace github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels => github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels v1.9.1-0.20251003150112-9b458d311c4c -// The version of vm-operator should be kept in sync with the manifests at: config/deployments/integration-testsz -replace github.com/vmware-tanzu/vm-operator/api => github.com/vmware-tanzu/vm-operator/api v1.8.6 +// The version of vm-operator should be kept in sync with the manifests at: config/deployments/integration-tests +replace github.com/vmware-tanzu/vm-operator/api => github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c require ( github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d // The version of vm-operator should be kept in sync with the manifests at: config/deployments/integration-tests - github.com/vmware-tanzu/vm-operator/api v1.8.6 + github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c github.com/vmware/govmomi v0.52.0 ) diff --git a/test/go.sum b/test/go.sum index 8ac8dfd79b..e5e682ab61 100644 --- a/test/go.sum +++ b/test/go.sum @@ -360,8 +360,8 @@ github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d h1:c github.com/vmware-tanzu/net-operator-api v0.0.0-20240326163340-1f32d6bf7f9d/go.mod h1:JbFOh22iDsT5BowJe0GgpMI5e2/S7cWaJlv9LdURVQM= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d h1:z9lrzKVtNlujduv9BilzPxuge/LE2F0N1ms3TP4JZvw= github.com/vmware-tanzu/nsx-operator/pkg/apis v0.0.0-20241112044858-9da8637c1b0d/go.mod h1:Q4JzNkNMvjo7pXtlB5/R3oME4Nhah7fAObWgghVmtxk= -github.com/vmware-tanzu/vm-operator/api v1.8.6 h1:NIndORjcnSmIlQsCMIewpIwg/ocRVDh2lYjOroTVLrU= -github.com/vmware-tanzu/vm-operator/api v1.8.6/go.mod h1:HHA2SNI9B5Yqtyp5t+Gt9WTWBi/fIkM6+MukDDSf11A= +github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c h1:XISTT0dw/XwMlyyiOPHPsXCxfI1Ro2Zuozi6eIacXGo= +github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20251003150112-9b458d311c4c/go.mod h1:nWTPpxfe4gHuuYuFcrs86+NMxfkqPk3a3IlvI8TCWak= github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505 h1:y4wXx1FUFqqSgJ/xUOEM1DLS2Uu0KaeLADWpzpioGTU= github.com/vmware-tanzu/vm-operator/external/ncp v0.0.0-20240404200847-de75746a9505/go.mod h1:5rqRJ9zGR+KnKbkGx373WgN8xJpvAj99kHnfoDYRO5I= github.com/vmware/govmomi v0.52.0 h1:JyxQ1IQdllrY7PJbv2am9mRsv3p9xWlIQ66bv+XnyLw=