Skip to content

Commit dc441f6

Browse files
committed
Address review comments
- Refine VMG controller watch - Handle race conditions in VMG controller by gating member update - Refine data struct for VM Affinity config - Refine UT - Refine naming, logging, godoc - Miscellaneous Signed-off-by: Gong Zhang <[email protected]>
1 parent e3078c9 commit dc441f6

File tree

6 files changed

+734
-209
lines changed

6 files changed

+734
-209
lines changed

config/rbac/role.yaml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ rules:
250250
- vmoperator.vmware.com
251251
resources:
252252
- virtualmachinegroups
253+
- virtualmachinegroups/status
253254
- virtualmachineimages
254255
- virtualmachineimages/status
255256
- virtualmachines
@@ -265,12 +266,6 @@ rules:
265266
- patch
266267
- update
267268
- watch
268-
- apiGroups:
269-
- vmoperator.vmware.com
270-
resources:
271-
- virtualmachinegroups/status
272-
verbs:
273-
- get
274269
- apiGroups:
275270
- vmware.com
276271
resources:

controllers/vmware/virtualmachinegroup_controller.go

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,6 @@ import (
3737
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
3838
)
3939

40-
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch
41-
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters/status,verbs=get
42-
// +kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinegroups,verbs=get;list;watch;create;update;patch;delete
43-
// +kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinegroups/status,verbs=get
44-
// +kubebuilder:rbac:groups=vmware.infrastructure.cluster.x-k8s.io,resources=vspheremachines,verbs=get;list;watch
45-
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch
46-
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;list;watch
47-
4840
// AddVirtualMachineGroupControllerToManager adds the VirtualMachineGroup controller to the provided manager.
4941
func AddVirtualMachineGroupControllerToManager(ctx context.Context, controllerManagerCtx *capvcontext.ControllerManagerContext, mgr manager.Manager, options controller.Options) error {
5042
predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "virtualmachinegroup")
@@ -62,15 +54,24 @@ func AddVirtualMachineGroupControllerToManager(ctx context.Context, controllerMa
6254
Watches(
6355
&vmoprv1.VirtualMachineGroup{},
6456
handler.EnqueueRequestForOwner(mgr.GetScheme(), reconciler.Client.RESTMapper(), &clusterv1.Cluster{}),
57+
ctrlbldr.WithPredicates(predicates.ResourceIsChanged(mgr.GetScheme(), predicateLog)),
6558
).
6659
Watches(
6760
&vmwarev1.VSphereMachine{},
6861
handler.EnqueueRequestsFromMapFunc(reconciler.VSphereMachineToCluster),
6962
ctrlbldr.WithPredicates(
7063
predicate.Funcs{
71-
UpdateFunc: func(event.UpdateEvent) bool { return false },
72-
CreateFunc: func(event.CreateEvent) bool { return true },
73-
DeleteFunc: func(event.DeleteEvent) bool { return true },
64+
UpdateFunc: func(event.UpdateEvent) bool { return false },
65+
CreateFunc: func(e event.CreateEvent) bool {
66+
// Only handle VSphereMachine which belongs to a MachineDeployment
67+
_, found := e.Object.GetLabels()[clusterv1.MachineDeploymentNameLabel]
68+
return found
69+
},
70+
DeleteFunc: func(e event.DeleteEvent) bool {
71+
// Only handle VSphereMachine which belongs to a MachineDeployment
72+
_, found := e.Object.GetLabels()[clusterv1.MachineDeploymentNameLabel]
73+
return found
74+
},
7475
GenericFunc: func(event.GenericEvent) bool { return false },
7576
}),
7677
).
@@ -80,9 +81,7 @@ func AddVirtualMachineGroupControllerToManager(ctx context.Context, controllerMa
8081
}
8182

8283
// VSphereMachineToCluster maps VSphereMachine events to Cluster reconcile requests.
83-
// This handler only processes VSphereMachine objects for Day-2 operations when VMG could be found, ensuring
84-
// VMG member list in sync with VSphereMachines. If no corresponding VMG is found, this is a no-op.
85-
func (r *VirtualMachineGroupReconciler) VSphereMachineToCluster(ctx context.Context, a ctrlclient.Object) []reconcile.Request {
84+
func (r *VirtualMachineGroupReconciler) VSphereMachineToCluster(_ context.Context, a ctrlclient.Object) []reconcile.Request {
8685
vSphereMachine, ok := a.(*vmwarev1.VSphereMachine)
8786
if !ok {
8887
return nil
@@ -93,17 +92,10 @@ func (r *VirtualMachineGroupReconciler) VSphereMachineToCluster(ctx context.Cont
9392
return nil
9493
}
9594

96-
vmg := &vmoprv1.VirtualMachineGroup{}
97-
err := r.Client.Get(ctx, apitypes.NamespacedName{Namespace: vSphereMachine.Namespace, Name: clusterName}, vmg)
98-
99-
if err != nil {
100-
return nil
101-
}
102-
10395
return []reconcile.Request{{
10496
NamespacedName: apitypes.NamespacedName{
105-
Namespace: vmg.Namespace,
106-
Name: vmg.Name,
97+
Namespace: vSphereMachine.Namespace,
98+
Name: clusterName,
10799
},
108100
}}
109101
}

0 commit comments

Comments
 (0)