Skip to content

Commit 6ff7ae1

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

File tree

7 files changed

+141
-70
lines changed

7 files changed

+141
-70
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: 4 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 (
@@ -86,10 +86,9 @@ func (r *VirtualMachineGroupReconciler) Reconcile(ctx context.Context, req ctrl.
8686

8787
// Continue with the main logic.
8888
return r.createOrUpdateVMG(ctx, cluster)
89-
9089
}
9190

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

@@ -228,8 +227,7 @@ func (r *VirtualMachineGroupReconciler) createOrUpdateVMG(ctx context.Context, c
228227
return reconcile.Result{}, err
229228
}
230229

231-
// getExpectedVSphereMachines returns the total number of replicas across all
232-
// MachineDeployments belonging to the Cluster
230+
// MachineDeployments belonging to the Cluster.
233231
func getExpectedVSphereMachines(ctx context.Context, kubeClient client.Client, cluster *clusterv1.Cluster) (int32, error) {
234232
var mdList clusterv1.MachineDeploymentList
235233
if err := kubeClient.List(
@@ -329,8 +327,8 @@ func GenerateVMGPlacementAnnotations(ctx context.Context, vmg *vmoprv1.VirtualMa
329327
return annotations, nil
330328
}
331329

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

controllers/vmware/virtualmachinegroup_reconciler_test.go

Lines changed: 127 additions & 54 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,60 +30,117 @@ 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"
3350
zoneA = "zone-a"
3451
zoneB = "zone-b"
3552
)
3653

54+
// Helper function to create a MachineDeployment object.
55+
func newMachineDeployment(name, clusterName, clusterNS string, replicas *int32) *clusterv1.MachineDeployment {
56+
return &clusterv1.MachineDeployment{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: name,
59+
Namespace: clusterNS,
60+
Labels: map[string]string{clusterv1.ClusterNameLabel: clusterName},
61+
},
62+
Spec: clusterv1.MachineDeploymentSpec{
63+
Replicas: replicas,
64+
},
65+
}
66+
}
67+
68+
// Helper function to create a basic Cluster object used as input.
69+
func newTestCluster(name, namespace string) *clusterv1.Cluster {
70+
return &clusterv1.Cluster{
71+
ObjectMeta: metav1.ObjectMeta{
72+
Name: name,
73+
Namespace: namespace,
74+
},
75+
}
76+
}
77+
3778
func TestGetExpectedVSphereMachines(t *testing.T) {
3879
g := NewWithT(t)
3980
ctx := context.Background()
4081

82+
// Replica helper values
83+
r3 := int32(3)
84+
r5 := int32(5)
85+
r0 := int32(0)
86+
87+
// Cluster object used as input parameter
88+
targetCluster := newTestCluster(clusterName, clusterNamespace)
89+
90+
// Create MDs for the target cluster
91+
mdA := newMachineDeployment("md-a", clusterName, clusterNamespace, &r3)
92+
mdB := newMachineDeployment("md-b", clusterName, clusterNamespace, &r5)
93+
mdCNil := newMachineDeployment("md-c-nil", clusterName, clusterNamespace, nil)
94+
mdDZero := newMachineDeployment("md-d-zero", clusterName, clusterNamespace, &r0)
95+
96+
// Create an MD for a different cluster (should be filtered)
97+
mdOtherCluster := newMachineDeployment("md-other", otherClusterName, clusterNamespace, &r5)
98+
4199
tests := []struct {
42-
name string
43-
cluster *clusterv1.Cluster
44-
expected int32
100+
name string
101+
initialObjects []client.Object
102+
expectedTotal int32
103+
wantErr bool
45104
}{
46105
{
47-
name: "Defined topology with replicas",
48-
cluster: newCluster(clusterName, clusterNamespace, true, 3, 2),
49-
expected: 5,
106+
name: "Success: Sum of two MDs",
107+
initialObjects: []client.Object{mdA, mdB},
108+
expectedTotal: 8,
109+
wantErr: false,
50110
},
51111
{
52-
name: "Defined topology with zero replicas",
53-
cluster: newCluster(clusterName, clusterNamespace, true, 0, 0),
54-
expected: 0,
112+
name: "Success: Includes nil and zero replicas",
113+
initialObjects: []client.Object{mdA, mdB, mdCNil, mdDZero},
114+
expectedTotal: 8,
115+
wantErr: false,
55116
},
56117
{
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,
118+
name: "Success: Filters MDs from other clusters",
119+
initialObjects: []client.Object{mdA, mdB, mdOtherCluster},
120+
expectedTotal: 8,
121+
wantErr: false,
122+
},
123+
{
124+
name: "Success: No MachineDeployments found",
125+
initialObjects: []client.Object{},
126+
expectedTotal: 0,
127+
wantErr: false,
64128
},
65129
}
66130

67131
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))
132+
t.Run(tt.name, func(_ *testing.T) {
133+
scheme := runtime.NewScheme()
134+
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
135+
136+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialObjects...).Build()
137+
total, err := getExpectedVSphereMachines(ctx, fakeClient, targetCluster)
138+
if tt.wantErr {
139+
g.Expect(err).To(HaveOccurred())
140+
} else {
141+
g.Expect(err).NotTo(HaveOccurred())
142+
g.Expect(total).To(Equal(tt.expectedTotal))
143+
}
71144
})
72145
}
73146
}
@@ -76,12 +149,14 @@ func TestGetCurrentVSphereMachines(t *testing.T) {
76149
g := NewWithT(t)
77150
ctx := context.Background()
78151

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"
152+
scheme := runtime.NewScheme()
153+
g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed())
154+
155+
// VSphereMachine names are based on CAPI Machine names, but we use fake name here.
156+
vsm1 := newVSphereMachine("vsm-1", mdName1, false, false, nil)
157+
vsm2 := newVSphereMachine("vsm-2", mdName2, false, false, nil)
158+
vsmDeleting := newVSphereMachine("vsm-3", mdName1, false, true, nil) // Deleting
159+
vsmControlPlane := newVSphereMachine("vsm-cp", "not-md", true, false, nil)
85160

86161
tests := []struct {
87162
name string
@@ -96,7 +171,7 @@ func TestGetCurrentVSphereMachines(t *testing.T) {
96171
vsmDeleting,
97172
vsmControlPlane,
98173
},
99-
want: 2, // Should exclude vsm-3 (deleting) and vsm-cp (control plane VSphereMachine)
174+
want: 2,
100175
},
101176
{
102177
name: "No VSphereMachines found",
@@ -106,11 +181,11 @@ func TestGetCurrentVSphereMachines(t *testing.T) {
106181
}
107182

108183
for _, tt := range tests {
109-
t.Run(tt.name, func(t *testing.T) {
110-
fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(tt.objects...).Build()
184+
t.Run(tt.name, func(_ *testing.T) {
185+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build()
111186
got, err := getCurrentVSphereMachines(ctx, fakeClient, clusterNamespace, clusterName)
112187
g.Expect(err).NotTo(HaveOccurred())
113-
g.Expect(len(got)).To(Equal(tt.want))
188+
g.Expect(got).To(HaveLen(tt.want))
114189

115190
// Check that the correct Machines are present
116191
if tt.want > 0 {
@@ -221,7 +296,7 @@ func TestGenerateVMGPlacementAnnotations(t *testing.T) {
221296
}
222297

223298
for _, tt := range tests {
224-
t.Run(tt.name, func(t *testing.T) {
299+
t.Run(tt.name, func(_ *testing.T) {
225300
ctx := ctrl.LoggerInto(context.Background(), ctrl.LoggerFrom(context.Background()))
226301

227302
got, err := GenerateVMGPlacementAnnotations(ctx, tt.vmg, tt.machineDeployments)
@@ -240,10 +315,14 @@ func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) {
240315
g := NewWithT(t)
241316
ctx := context.Background()
242317

318+
scheme := runtime.NewScheme()
319+
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
320+
g.Expect(vmwarev1.AddToScheme(scheme)).To(Succeed())
321+
243322
// Initial objects for the successful VMG creation path (Expected: 1, Current: 1)
244323
cluster := newCluster(clusterName, clusterNamespace, true, 1, 0)
245-
vsm1 := newVSphereMachine("vsm-1", mdName1, false, nil)
246-
md1 := newMachineDeployment(mdName1)
324+
vsm1 := newVSphereMachine("vsm-1", mdName1, false, false, nil)
325+
md1 := newMachineDeployment(mdName1, clusterName, clusterNamespace, ptr.To(int32(1)))
247326

248327
tests := []struct {
249328
name string
@@ -312,8 +391,8 @@ func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) {
312391
}
313392

314393
for _, tt := range tests {
315-
t.Run(tt.name, func(t *testing.T) {
316-
fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(tt.initialObjects...).Build()
394+
t.Run(tt.name, func(_ *testing.T) {
395+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.initialObjects...).Build()
317396
reconciler := &VirtualMachineGroupReconciler{
318397
Client: fakeClient,
319398
Recorder: record.NewFakeRecorder(1),
@@ -347,7 +426,7 @@ func TestVirtualMachineGroupReconciler_ReconcileFlow(t *testing.T) {
347426
}
348427
}
349428

350-
// Helper function to create a basic Cluster object
429+
// Helper function to create a basic Cluster object.
351430
func newCluster(name, namespace string, initialized bool, replicasMD1, replicasMD2 int32) *clusterv1.Cluster {
352431
cluster := &clusterv1.Cluster{
353432
ObjectMeta: metav1.ObjectMeta{
@@ -375,28 +454,33 @@ func newCluster(name, namespace string, initialized bool, replicasMD1, replicasM
375454
return cluster
376455
}
377456

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 {
457+
// Helper function to create a VSphereMachine (worker, owned by a CAPI Machine).
458+
func newVSphereMachine(name, mdName string, isCP, deleted bool, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) *vmwarev1.VSphereMachine {
380459
vsm := &vmwarev1.VSphereMachine{
381460
ObjectMeta: metav1.ObjectMeta{
382461
Name: name,
383462
Namespace: clusterNamespace,
384463
Labels: map[string]string{
385-
clusterv1.ClusterNameLabel: clusterName,
386-
clusterv1.MachineDeploymentNameLabel: mdName,
464+
clusterv1.ClusterNameLabel: clusterName,
387465
},
388466
},
389467
Spec: vmwarev1.VSphereMachineSpec{
390468
NamingStrategy: namingStrategy,
391469
},
392470
}
471+
if !isCP {
472+
vsm.Labels[clusterv1.MachineDeploymentNameLabel] = mdName
473+
} else {
474+
vsm.Labels[clusterv1.MachineControlPlaneLabel] = "true"
475+
}
393476
if deleted {
477+
vsm.Finalizers = []string{"test.finalizer.0"}
394478
vsm.DeletionTimestamp = &metav1.Time{Time: time.Now()}
395479
}
396480
return vsm
397481
}
398482

399-
// Helper function to create a VMG member status with placement info
483+
// Helper function to create a VMG member status with placement info.
400484
func newVMGMemberStatus(name, kind string, isPlacementReady bool, zone string) vmoprv1.VirtualMachineGroupMemberStatus {
401485
memberStatus := vmoprv1.VirtualMachineGroupMemberStatus{
402486
Name: name,
@@ -412,14 +496,3 @@ func newVMGMemberStatus(name, kind string, isPlacementReady bool, zone string) v
412496
}
413497
return memberStatus
414498
}
415-
416-
// Helper function to create a MachineDeployment (for listing MD names)
417-
func newMachineDeployment(name string) *clusterv1.MachineDeployment {
418-
return &clusterv1.MachineDeployment{
419-
ObjectMeta: metav1.ObjectMeta{
420-
Name: name,
421-
Namespace: clusterNamespace,
422-
Labels: map[string]string{clusterv1.ClusterNameLabel: clusterName},
423-
},
424-
}
425-
}

controllers/vspherecluster_reconciler.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,6 @@ func (r *clusterReconciler) reconcileDeploymentZones(ctx context.Context, cluste
427427
failureDomains := clusterv1beta1.FailureDomains{}
428428
for _, zone := range deploymentZoneList.Items {
429429
if zone.Spec.Server != clusterCtx.VSphereCluster.Spec.Server {
430-
431430
continue
432431
}
433432

0 commit comments

Comments
 (0)