Skip to content

Commit 779157b

Browse files
committed
Fix UT errors
Signed-off-by: Gong Zhang <[email protected]>
1 parent 8cc8e8a commit 779157b

File tree

7 files changed

+142
-62
lines changed

7 files changed

+142
-62
lines changed

controllers/vmware/virtualmachinegroup_controller.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ import (
2121

2222
vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
2323
apitypes "k8s.io/apimachinery/pkg/types"
24-
vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
25-
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
2624
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
2725
"sigs.k8s.io/cluster-api/util/predicates"
2826
ctrl "sigs.k8s.io/controller-runtime"
@@ -34,6 +32,9 @@ import (
3432
"sigs.k8s.io/controller-runtime/pkg/manager"
3533
"sigs.k8s.io/controller-runtime/pkg/predicate"
3634
"sigs.k8s.io/controller-runtime/pkg/reconcile"
35+
36+
vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
37+
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
3738
)
3839

3940
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch
@@ -77,7 +78,7 @@ func AddVirtualMachineGroupControllerToManager(ctx context.Context, controllerMa
7778
handler.EnqueueRequestsFromMapFunc(reconciler.VSphereMachineToVirtualMachineGroup),
7879
ctrlbldr.WithPredicates(
7980
predicate.Funcs{
80-
UpdateFunc: func(e event.UpdateEvent) bool { return false },
81+
UpdateFunc: func(event.UpdateEvent) bool { return false },
8182
CreateFunc: func(event.CreateEvent) bool { return true },
8283
DeleteFunc: func(event.DeleteEvent) bool { return true },
8384
GenericFunc: func(event.GenericEvent) bool { return false },
@@ -89,7 +90,7 @@ func AddVirtualMachineGroupControllerToManager(ctx context.Context, controllerMa
8990
}
9091

9192
// ClusterToVirtualMachineGroup maps Cluster events to VirtualMachineGroup reconcile requests.
92-
func (r *VirtualMachineGroupReconciler) ClusterToVirtualMachineGroup(ctx context.Context, a ctrlclient.Object) []reconcile.Request {
93+
func (r *VirtualMachineGroupReconciler) ClusterToVirtualMachineGroup(_ context.Context, a ctrlclient.Object) []reconcile.Request {
9394
cluster, ok := a.(*clusterv1.Cluster)
9495
if !ok {
9596
return nil

controllers/vmware/virtualmachinegroup_reconciler.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
"k8s.io/client-go/tools/record"
3232
"k8s.io/klog/v2"
33+
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
3334
"sigs.k8s.io/cluster-api/util/conditions"
3435
ctrl "sigs.k8s.io/controller-runtime"
3536
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -38,7 +39,6 @@ import (
3839

3940
vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
4041
infrautilv1 "sigs.k8s.io/cluster-api-provider-vsphere/pkg/util"
41-
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
4242
)
4343

4444
const (
@@ -72,6 +72,7 @@ func (r *VirtualMachineGroupReconciler) Reconcile(ctx context.Context, req ctrl.
7272
}
7373

7474
log = log.WithValues("Cluster", klog.KObj(cluster))
75+
7576
// If Cluster is deleted, just return as VirtualMachineGroup will be GCed and no extra processing needed.
7677
if !cluster.DeletionTimestamp.IsZero() {
7778
return reconcile.Result{}, nil
@@ -86,10 +87,9 @@ func (r *VirtualMachineGroupReconciler) Reconcile(ctx context.Context, req ctrl.
8687

8788
// Continue with the main logic.
8889
return r.createOrUpdateVMG(ctx, cluster)
89-
9090
}
9191

92-
// createOrUpdateVMG Create or Update VirtualMachineGroup
92+
// createOrUpdateVMG Create or Update VirtualMachineGroup.
9393
func (r *VirtualMachineGroupReconciler) createOrUpdateVMG(ctx context.Context, cluster *clusterv1.Cluster) (_ reconcile.Result, reterr error) {
9494
log := ctrl.LoggerFrom(ctx)
9595

@@ -228,8 +228,7 @@ func (r *VirtualMachineGroupReconciler) createOrUpdateVMG(ctx context.Context, c
228228
return reconcile.Result{}, err
229229
}
230230

231-
// getExpectedVSphereMachines returns the total number of replicas across all
232-
// MachineDeployments belonging to the Cluster
231+
// MachineDeployments belonging to the Cluster.
233232
func getExpectedVSphereMachines(ctx context.Context, kubeClient client.Client, cluster *clusterv1.Cluster) (int32, error) {
234233
var mdList clusterv1.MachineDeploymentList
235234
if err := kubeClient.List(
@@ -329,8 +328,8 @@ func GenerateVMGPlacementAnnotations(ctx context.Context, vmg *vmoprv1.VirtualMa
329328
return annotations, nil
330329
}
331330

332-
// Duplicated this logic from pkg/services/vmoperator/vmopmachine.go
333331
// GenerateVirtualMachineName generates the name of a VirtualMachine based on the naming strategy.
332+
// Duplicated this logic from pkg/services/vmoperator/vmopmachine.go.
334333
func GenerateVirtualMachineName(machineName string, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) (string, error) {
335334
// Per default the name of the VirtualMachine should be equal to the Machine name (this is the same as "{{ .machine.name }}")
336335
if namingStrategy == nil || namingStrategy.Template == nil {

controllers/vmware/virtualmachinegroup_reconciler_test.go

Lines changed: 127 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
117
package vmware
218

319
import (
@@ -14,19 +30,20 @@ import (
1430
"k8s.io/apimachinery/pkg/runtime"
1531
"k8s.io/apimachinery/pkg/types"
1632
"k8s.io/client-go/tools/record"
17-
vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
33+
"k8s.io/utils/ptr"
1834
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
1935
"sigs.k8s.io/cluster-api/util/conditions"
2036
ctrl "sigs.k8s.io/controller-runtime"
2137
"sigs.k8s.io/controller-runtime/pkg/client"
2238
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2339
"sigs.k8s.io/controller-runtime/pkg/reconcile"
24-
)
2540

26-
var s = runtime.NewScheme()
41+
vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
42+
)
2743

2844
const (
2945
clusterName = "test-cluster"
46+
otherClusterName = "other-cluster"
3047
clusterNamespace = "test-ns"
3148
mdName1 = "md-worker-a"
3249
mdName2 = "md-worker-b"
@@ -38,36 +55,70 @@ func TestGetExpectedVSphereMachines(t *testing.T) {
3855
g := NewWithT(t)
3956
ctx := context.Background()
4057

58+
// Replica helper values
59+
r3 := int32(3)
60+
r5 := int32(5)
61+
r0 := int32(0)
62+
63+
// Cluster object used as input parameter
64+
targetCluster := newTestCluster(clusterName, clusterNamespace)
65+
66+
// Create MDs for the target cluster
67+
mdA := newMachineDeployment("md-a", clusterName, clusterNamespace, &r3)
68+
mdB := newMachineDeployment("md-b", clusterName, clusterNamespace, &r5)
69+
mdCNil := newMachineDeployment("md-c-nil", clusterName, clusterNamespace, nil)
70+
mdDZero := newMachineDeployment("md-d-zero", clusterName, clusterNamespace, &r0)
71+
72+
// Create an MD for a different cluster (should be filtered)
73+
mdOtherCluster := newMachineDeployment("md-other", otherClusterName, clusterNamespace, &r5)
74+
4175
tests := []struct {
42-
name string
43-
cluster *clusterv1.Cluster
44-
expected int32
76+
name string
77+
initialObjects []client.Object
78+
expectedTotal int32
79+
wantErr bool
4580
}{
4681
{
47-
name: "Defined topology with replicas",
48-
cluster: newCluster(clusterName, clusterNamespace, true, 3, 2),
49-
expected: 5,
82+
name: "Success: Sum of two MDs",
83+
initialObjects: []client.Object{mdA, mdB},
84+
expectedTotal: 8,
85+
wantErr: false,
86+
},
87+
{
88+
name: "Success: Includes nil and zero replicas",
89+
initialObjects: []client.Object{mdA, mdB, mdCNil, mdDZero},
90+
expectedTotal: 8,
91+
wantErr: false,
5092
},
5193
{
52-
name: "Defined topology with zero replicas",
53-
cluster: newCluster(clusterName, clusterNamespace, true, 0, 0),
54-
expected: 0,
94+
name: "Success: Filters MDs from other clusters",
95+
initialObjects: []client.Object{mdA, mdB, mdOtherCluster},
96+
expectedTotal: 8,
97+
wantErr: false,
5598
},
5699
{
57-
name: "Undefined topology",
58-
cluster: func() *clusterv1.Cluster {
59-
c := newCluster(clusterName, clusterNamespace, true, 1, 1)
60-
c.Spec.Topology = clusterv1.Topology{}
61-
return c
62-
}(),
63-
expected: 0,
100+
name: "Success: No MachineDeployments found",
101+
initialObjects: []client.Object{},
102+
expectedTotal: 0,
103+
wantErr: false,
64104
},
65105
}
66106

67107
for _, tt := range tests {
68-
fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(tt.cluster).Build()
69-
t.Run(tt.name, func(t *testing.T) {
70-
g.Expect(getExpectedVSphereMachines(ctx, fakeClient, tt.cluster)).To(Equal(tt.expected))
108+
// Looks odd, but need to reinitialize test variable
109+
tt := tt
110+
t.Run(tt.name, func(_ *testing.T) {
111+
scheme := runtime.NewScheme()
112+
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
113+
114+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialObjects...).Build()
115+
total, err := getExpectedVSphereMachines(ctx, fakeClient, targetCluster)
116+
if tt.wantErr {
117+
g.Expect(err).To(HaveOccurred())
118+
} else {
119+
g.Expect(err).NotTo(HaveOccurred())
120+
g.Expect(total).To(Equal(tt.expectedTotal))
121+
}
71122
})
72123
}
73124
}
@@ -76,12 +127,14 @@ func TestGetCurrentVSphereMachines(t *testing.T) {
76127
g := NewWithT(t)
77128
ctx := context.Background()
78129

79-
// VM names are based on CAPI Machine names, not VSphereMachine names, but we use VSphereMachine here.
80-
vsm1 := newVSphereMachine("vsm-1", mdName1, false, nil)
81-
vsm2 := newVSphereMachine("vsm-2", mdName2, false, nil)
82-
vsmDeleting := newVSphereMachine("vsm-3", mdName1, true, nil) // Deleting
83-
vsmControlPlane := newVSphereMachine("vsm-cp", "cp-md", false, nil)
84-
vsmControlPlane.Labels[clusterv1.MachineControlPlaneLabel] = "true"
130+
scheme := runtime.NewScheme()
131+
g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed())
132+
133+
// VSphereMachine names are based on CAPI Machine names, but we use fake name here.
134+
vsm1 := newVSphereMachine("vsm-1", mdName1, false, false, nil)
135+
vsm2 := newVSphereMachine("vsm-2", mdName2, false, false, nil)
136+
vsmDeleting := newVSphereMachine("vsm-3", mdName1, false, true, nil) // Deleting
137+
vsmControlPlane := newVSphereMachine("vsm-cp", "not-md", true, false, nil)
85138

86139
tests := []struct {
87140
name string
@@ -96,7 +149,7 @@ func TestGetCurrentVSphereMachines(t *testing.T) {
96149
vsmDeleting,
97150
vsmControlPlane,
98151
},
99-
want: 2, // Should exclude vsm-3 (deleting) and vsm-cp (control plane VSphereMachine)
152+
want: 2,
100153
},
101154
{
102155
name: "No VSphereMachines found",
@@ -106,11 +159,13 @@ func TestGetCurrentVSphereMachines(t *testing.T) {
106159
}
107160

108161
for _, tt := range tests {
109-
t.Run(tt.name, func(t *testing.T) {
110-
fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(tt.objects...).Build()
162+
// Looks odd, but need to reinitialize test variable
163+
tt := tt
164+
t.Run(tt.name, func(_ *testing.T) {
165+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build()
111166
got, err := getCurrentVSphereMachines(ctx, fakeClient, clusterNamespace, clusterName)
112167
g.Expect(err).NotTo(HaveOccurred())
113-
g.Expect(len(got)).To(Equal(tt.want))
168+
g.Expect(got).To(HaveLen(tt.want))
114169

115170
// Check that the correct Machines are present
116171
if tt.want > 0 {
@@ -221,7 +276,9 @@ func TestGenerateVMGPlacementAnnotations(t *testing.T) {
221276
}
222277

223278
for _, tt := range tests {
224-
t.Run(tt.name, func(t *testing.T) {
279+
// Looks odd, but need to reinitialize test variable
280+
tt := tt
281+
t.Run(tt.name, func(_ *testing.T) {
225282
ctx := ctrl.LoggerInto(context.Background(), ctrl.LoggerFrom(context.Background()))
226283

227284
got, err := GenerateVMGPlacementAnnotations(ctx, tt.vmg, tt.machineDeployments)
@@ -240,10 +297,14 @@ func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) {
240297
g := NewWithT(t)
241298
ctx := context.Background()
242299

300+
scheme := runtime.NewScheme()
301+
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
302+
g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed())
303+
243304
// Initial objects for the successful VMG creation path (Expected: 1, Current: 1)
244305
cluster := newCluster(clusterName, clusterNamespace, true, 1, 0)
245-
vsm1 := newVSphereMachine("vsm-1", mdName1, false, nil)
246-
md1 := newMachineDeployment(mdName1)
306+
vsm1 := newVSphereMachine("vsm-1", mdName1, false, false, nil)
307+
md1 := newMachineDeployment(mdName1, clusterName, clusterNamespace, ptr.To(int32(1)))
247308

248309
tests := []struct {
249310
name string
@@ -312,8 +373,10 @@ func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) {
312373
}
313374

314375
for _, tt := range tests {
315-
t.Run(tt.name, func(t *testing.T) {
316-
fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(tt.initialObjects...).Build()
376+
// Looks odd, but need to reinitialize test variable
377+
tt := tt
378+
t.Run(tt.name, func(_ *testing.T) {
379+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialObjects...).Build()
317380
reconciler := &VirtualMachineGroupReconciler{
318381
Client: fakeClient,
319382
Recorder: record.NewFakeRecorder(1),
@@ -347,7 +410,7 @@ func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) {
347410
}
348411
}
349412

350-
// Helper function to create a basic Cluster object
413+
// Helper function to create a basic Cluster object.
351414
func newCluster(name, namespace string, initialized bool, replicasMD1, replicasMD2 int32) *clusterv1.Cluster {
352415
cluster := &clusterv1.Cluster{
353416
ObjectMeta: metav1.ObjectMeta{
@@ -375,28 +438,33 @@ func newCluster(name, namespace string, initialized bool, replicasMD1, replicasM
375438
return cluster
376439
}
377440

378-
// Helper function to create a VSphereMachine (worker, owned by a CAPI Machine)
379-
func newVSphereMachine(name, mdName string, deleted bool, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) *vmwarev1.VSphereMachine {
441+
// Helper function to create a VSphereMachine (worker, owned by a CAPI Machine).
442+
func newVSphereMachine(name, mdName string, isCP, deleted bool, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) *vmwarev1.VSphereMachine {
380443
vsm := &vmwarev1.VSphereMachine{
381444
ObjectMeta: metav1.ObjectMeta{
382445
Name: name,
383446
Namespace: clusterNamespace,
384447
Labels: map[string]string{
385-
clusterv1.ClusterNameLabel: clusterName,
386-
clusterv1.MachineDeploymentNameLabel: mdName,
448+
clusterv1.ClusterNameLabel: clusterName,
387449
},
388450
},
389451
Spec: vmwarev1.VSphereMachineSpec{
390452
NamingStrategy: namingStrategy,
391453
},
392454
}
455+
if !isCP {
456+
vsm.Labels[clusterv1.MachineDeploymentNameLabel] = mdName
457+
} else {
458+
vsm.Labels[clusterv1.MachineControlPlaneLabel] = "true"
459+
}
393460
if deleted {
461+
vsm.Finalizers = []string{"test.finalizer.0"}
394462
vsm.DeletionTimestamp = &metav1.Time{Time: time.Now()}
395463
}
396464
return vsm
397465
}
398466

399-
// Helper function to create a VMG member status with placement info
467+
// Helper function to create a VMG member status with placement info.
400468
func newVMGMemberStatus(name, kind string, isPlacementReady bool, zone string) vmoprv1.VirtualMachineGroupMemberStatus {
401469
memberStatus := vmoprv1.VirtualMachineGroupMemberStatus{
402470
Name: name,
@@ -413,13 +481,26 @@ func newVMGMemberStatus(name, kind string, isPlacementReady bool, zone string) v
413481
return memberStatus
414482
}
415483

416-
// Helper function to create a MachineDeployment (for listing MD names)
417-
func newMachineDeployment(name string) *clusterv1.MachineDeployment {
484+
// Helper function to create a MachineDeployment object.
485+
func newMachineDeployment(name, clusterName, clusterNS string, replicas *int32) *clusterv1.MachineDeployment {
418486
return &clusterv1.MachineDeployment{
419487
ObjectMeta: metav1.ObjectMeta{
420488
Name: name,
421-
Namespace: clusterNamespace,
489+
Namespace: clusterNS,
422490
Labels: map[string]string{clusterv1.ClusterNameLabel: clusterName},
423491
},
492+
Spec: clusterv1.MachineDeploymentSpec{
493+
Replicas: replicas,
494+
},
495+
}
496+
}
497+
498+
// Helper function to create a basic Cluster object used as input.
499+
func newTestCluster(name, namespace string) *clusterv1.Cluster {
500+
return &clusterv1.Cluster{
501+
ObjectMeta: metav1.ObjectMeta{
502+
Name: name,
503+
Namespace: namespace,
504+
},
424505
}
425506
}

0 commit comments

Comments
 (0)