Skip to content

Commit 71919ae

Browse files
wenqiqwenqiq
authored
Improve unit-test for subnet service (vmware-tanzu#850)
Unit test coverage increased by 3%, reaching 73.07%. Signed-off-by: Wenqi Qiu <[email protected]>
1 parent 0cc163d commit 71919ae

File tree

6 files changed

+267
-92
lines changed

6 files changed

+267
-92
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ golangci-fix: $(GOLANGCI_LINT_BIN)
7979

8080
.PHONY: test
8181
test: manifests generate fmt vet envtest .coverage ## Run tests .
82-
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test -gcflags=all=-l $$(go list ./... | grep -v mock | grep -v e2e | grep -v hack) -v -coverprofile $(CURDIR)/.coverage/coverage-unit.out ## Prohibit inline optimization when using gomonkey
82+
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test -gcflags=all=-l -coverpkg=github.com/vmware-tanzu/nsx-operator/pkg/...,github.com/vmware-tanzu/nsx-operator/cmd/... -covermode=atomic $$(go list ./... | grep -v mock | grep -v e2e | grep -v hack) -v -coverprofile $(CURDIR)/.coverage/coverage-unit.out ## Prohibit inline optimization when using gomonkey
8383

8484
##@ Build
8585

‎codecov.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,9 @@ flag_management:
3333

3434
ignore:
3535
- "**/test/*.go"
36+
- "**/mock_*.go"
37+
- "**/*generate*.go"
38+
- "pkg/client"
39+
- "pkg/mock"
40+
- "**/pkg/client"
41+
- "**/*.pb.go"

‎pkg/controllers/subnetset/subnetset_controller_test.go‎

Lines changed: 83 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnet"
3737
"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnetport"
3838
"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc"
39+
"github.com/vmware-tanzu/nsx-operator/pkg/util"
3940
)
4041

4142
type fakeRecorder struct{}
@@ -49,6 +50,38 @@ func (recorder fakeRecorder) Eventf(object runtime.Object, eventtype, reason, me
4950
func (recorder fakeRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
5051
}
5152

53+
type fakeOrgRootClient struct {
54+
}
55+
56+
func (f fakeOrgRootClient) Get(basePathParam *string, filterParam *string, typeFilterParam *string) (model.OrgRoot, error) {
57+
return model.OrgRoot{}, nil
58+
}
59+
60+
func (f fakeOrgRootClient) Patch(orgRootParam model.OrgRoot, enforceRevisionCheckParam *bool) error {
61+
return errors.New("patch error")
62+
}
63+
64+
type fakeSubnetStatusClient struct {
65+
}
66+
67+
func (f fakeSubnetStatusClient) List(orgIdParam string, projectIdParam string, vpcIdParam string, subnetIdParam string) (model.VpcSubnetStatusListResult, error) {
68+
dhcpServerAddress := "1.1.1.1"
69+
ipAddressType := "fakeIpAddressType"
70+
networkAddress := "2.2.2.2"
71+
gatewayAddress := "3.3.3.3"
72+
return model.VpcSubnetStatusListResult{
73+
Results: []model.VpcSubnetStatus{
74+
{
75+
DhcpServerAddress: &gatewayAddress,
76+
GatewayAddress: &dhcpServerAddress,
77+
IpAddressType: &ipAddressType,
78+
NetworkAddress: &networkAddress,
79+
},
80+
},
81+
Status: nil,
82+
}, nil
83+
}
84+
5285
func createFakeSubnetSetReconciler(objs []client.Object) *SubnetSetReconciler {
5386
newScheme := runtime.NewScheme()
5487
utilruntime.Must(clientgoscheme.AddToScheme(newScheme))
@@ -62,10 +95,15 @@ func createFakeSubnetSetReconciler(objs []client.Object) *SubnetSetReconciler {
6295
}
6396
subnetService := &subnet.SubnetService{
6497
Service: common.Service{
65-
Client: fakeClient,
66-
NSXClient: &nsx.Client{},
67-
98+
Client: fakeClient,
99+
NSXClient: &nsx.Client{
100+
OrgRootClient: &fakeOrgRootClient{},
101+
SubnetStatusClient: &fakeSubnetStatusClient{},
102+
},
68103
NSXConfig: &config.NSXOperatorConfig{
104+
CoeConfig: &config.CoeConfig{
105+
Cluster: "clusterName",
106+
},
69107
NsxConfig: &config.NsxConfig{
70108
EnforcementPoint: "vmc-enforcementpoint",
71109
UseAVILoadBalancer: false,
@@ -97,6 +135,13 @@ func createFakeSubnetSetReconciler(objs []client.Object) *SubnetSetReconciler {
97135
func TestReconcile(t *testing.T) {
98136
subnetsetName := "test-subnetset"
99137
ns := "test-namespace"
138+
subnetSet := &v1alpha1.SubnetSet{
139+
ObjectMeta: metav1.ObjectMeta{
140+
Name: subnetsetName,
141+
Namespace: ns,
142+
},
143+
Spec: v1alpha1.SubnetSetSpec{},
144+
}
100145

101146
testCases := []struct {
102147
name string
@@ -124,9 +169,8 @@ func TestReconcile(t *testing.T) {
124169
},
125170
},
126171
{
127-
name: "Create a SubnetSet with error failed to generate SubnetSet tags",
128-
expectRes: ResultRequeue,
129-
expectErrStr: "failed to generate SubnetSet tags",
172+
name: "Create a SubnetSet",
173+
expectRes: ResultNormal,
130174
patches: func(r *SubnetSetReconciler) *gomonkey.Patches {
131175
vpcnetworkInfo := &common.VPCNetworkConfigInfo{DefaultSubnetSize: 32}
132176
patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo {
@@ -211,22 +255,26 @@ func TestReconcile(t *testing.T) {
211255
patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo {
212256
return vpcnetworkInfo
213257
})
214-
215258
patches.ApplyMethod(reflect.TypeOf(r.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet {
216259
id1 := "fake-id"
217-
path := "fake-path"
218-
vpcSubnet := model.VpcSubnet{Id: &id1, Path: &path}
219-
return []*model.VpcSubnet{
220-
&vpcSubnet,
221-
}
222-
})
223-
224-
tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}}
225-
patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag {
226-
return tags
260+
path := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-e2e_8f36f7fc-90cd-4e65-a816-daf3ecd6a0f9/subnets/fake-path"
261+
basicTags1 := util.BuildBasicTags("fakeClusterName", subnetSet, "")
262+
scopeNamespace := common.TagScopeNamespace
263+
basicTags1 = append(basicTags1, model.Tag{
264+
Scope: &scopeNamespace,
265+
Tag: &ns,
266+
})
267+
basicTags2 := util.BuildBasicTags("fakeClusterName", subnetSet, "")
268+
ns2 := "ns2"
269+
basicTags2 = append(basicTags2, model.Tag{
270+
Scope: &scopeNamespace,
271+
Tag: &ns2,
272+
})
273+
vpcSubnet1 := model.VpcSubnet{Id: &id1, Path: &path}
274+
vpcSubnet2 := model.VpcSubnet{Id: &id1, Path: &path, Tags: basicTags1}
275+
vpcSubnet3 := model.VpcSubnet{Id: &id1, Path: &path, Tags: basicTags2}
276+
return []*model.VpcSubnet{&vpcSubnet1, &vpcSubnet2, &vpcSubnet3}
227277
})
228-
229-
// UpdateSubnetSet
230278
patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "UpdateSubnetSet", func(_ *subnet.SubnetService, ns string, vpcSubnets []*model.VpcSubnet, tags []model.Tag, dhcpMode string) error {
231279
return nil
232280
})
@@ -239,18 +287,9 @@ func TestReconcile(t *testing.T) {
239287
ctx := context.TODO()
240288
req := ctrl.Request{NamespacedName: types.NamespacedName{Name: subnetsetName, Namespace: ns}}
241289

242-
subnetset := &v1alpha1.SubnetSet{
243-
ObjectMeta: metav1.ObjectMeta{
244-
Name: subnetsetName,
245-
Namespace: ns,
246-
},
247-
Spec: v1alpha1.SubnetSetSpec{},
248-
}
249-
namespace := &v12.Namespace{
250-
ObjectMeta: metav1.ObjectMeta{Name: ns, Namespace: ns},
251-
}
290+
namespace := &v12.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}}
252291

253-
r := createFakeSubnetSetReconciler([]client.Object{subnetset, namespace})
292+
r := createFakeSubnetSetReconciler([]client.Object{subnetSet, namespace})
254293
if testCase.patches != nil {
255294
patches := testCase.patches(r)
256295
defer patches.Reset()
@@ -260,6 +299,8 @@ func TestReconcile(t *testing.T) {
260299

261300
if testCase.expectErrStr != "" {
262301
assert.ErrorContains(t, err, testCase.expectErrStr)
302+
} else {
303+
assert.NoError(t, err)
263304
}
264305
assert.Equal(t, testCase.expectRes, res)
265306
})
@@ -295,7 +336,7 @@ func TestReconcile_DeleteSubnetSet(t *testing.T) {
295336
vpcSubnetSkip := model.VpcSubnet{Id: &id1, Path: &path, Tags: tags}
296337

297338
id2 := "fake-id-1"
298-
path2 := "fake-path-2"
339+
path2 := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-xxx/subnets/" + id2
299340
tagStale := []model.Tag{
300341
{Scope: common.String(common.TagScopeSubnetSetCRUID), Tag: common.String("fake-subnetSet-uid-stale")},
301342
{Scope: common.String(common.TagScopeSubnetSetCRName), Tag: common.String(subnetSetName)},
@@ -330,7 +371,7 @@ func TestReconcile_DeleteSubnetSet(t *testing.T) {
330371
vpcSubnetSkip := model.VpcSubnet{Id: &id1, Path: &path, Tags: tags}
331372

332373
id2 := "fake-id-1"
333-
path2 := "fake-path-2"
374+
path2 := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-xxx/subnets/fake-path-2"
334375
tagStale := []model.Tag{
335376
{Scope: common.String(common.TagScopeSubnetSetCRUID), Tag: common.String("fake-subnetSet-uid-stale")},
336377
{Scope: common.String(common.TagScopeSubnetSetCRName), Tag: common.String(subnetSetName)},
@@ -374,7 +415,7 @@ func TestReconcile_DeleteSubnetSet(t *testing.T) {
374415
vpcSubnetSkip := model.VpcSubnet{Id: &id1, Path: &path, Tags: tags}
375416

376417
id2 := "fake-id-1"
377-
path2 := "fake-path-2"
418+
path2 := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-xxx/subnets/fake-path-2"
378419
tagStale := []model.Tag{
379420
{Scope: common.String(common.TagScopeSubnetSetCRUID), Tag: common.String("fake-subnetSet-uid-stale")},
380421
{Scope: common.String(common.TagScopeSubnetSetCRName), Tag: common.String(subnetSetName)},
@@ -443,7 +484,7 @@ func TestReconcile_DeleteSubnetSet_WithFinalizer(t *testing.T) {
443484

444485
patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet {
445486
id1 := "fake-id"
446-
path := "fake-path"
487+
path := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-e2e_8f36f7fc-90cd-4e65-a816-daf3ecd6a0f9/subnets/" + id1
447488
vpcSubnet := model.VpcSubnet{Id: &id1, Path: &path}
448489
return []*model.VpcSubnet{
449490
&vpcSubnet,
@@ -533,10 +574,10 @@ func TestSubnetSetReconciler_CollectGarbage(t *testing.T) {
533574

534575
patches.ApplyMethod(reflect.TypeOf(r.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet {
535576
id1 := "fake-id"
536-
path := "fake-path"
537-
vpcSubnet := model.VpcSubnet{Id: &id1, Path: &path}
577+
path := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-e2e_8f36f7fc-90cd-4e65-a816-daf3ecd6a0f9/subnets/fake-path"
578+
vpcSubnet1 := model.VpcSubnet{Id: &id1, Path: &path}
538579
return []*model.VpcSubnet{
539-
&vpcSubnet,
580+
&vpcSubnet1,
540581
}
541582
})
542583
patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) {
@@ -553,10 +594,12 @@ func TestSubnetSetReconciler_CollectGarbage(t *testing.T) {
553594
// ListSubnetCreatedBySubnetSet
554595
patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnetSet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet {
555596
id1 := "fake-id"
556-
path := "fake-path"
557-
vpcSubnet := model.VpcSubnet{Id: &id1, Path: &path}
597+
path := "/orgs/default/projects/nsx_operator_e2e_test/vpcs/subnet-e2e_8f36f7fc-90cd-4e65-a816-daf3ecd6a0f9/subnets/fake-path"
598+
vpcSubnet1 := model.VpcSubnet{Id: &id1, Path: &path}
599+
invalidPath := "fakePath"
600+
vpcSubnet2 := model.VpcSubnet{Id: &id1, Path: &invalidPath}
558601
return []*model.VpcSubnet{
559-
&vpcSubnet,
602+
&vpcSubnet1, &vpcSubnet2,
560603
}
561604
})
562605

‎pkg/nsx/services/subnet/builder.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag, u
6060
var staticIpAllocation bool
6161
switch o := obj.(type) {
6262
case *v1alpha1.Subnet:
63-
staticIpAllocation = (o.Spec.SubnetDHCPConfig.Mode == "" || o.Spec.SubnetDHCPConfig.Mode == v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeDeactivated))
63+
staticIpAllocation = o.Spec.SubnetDHCPConfig.Mode == "" || o.Spec.SubnetDHCPConfig.Mode == v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeDeactivated)
6464
nsxSubnet = &model.VpcSubnet{
6565
Id: String(service.BuildSubnetID(o)),
6666
AccessMode: String(convertAccessMode(util.Capitalize(string(o.Spec.AccessMode)))),
@@ -80,7 +80,7 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag, u
8080
case *v1alpha1.SubnetSet:
8181
// The index is a random string with the length of 8 chars. It is the first 8 chars of the hash
8282
// value on a random UUID string.
83-
staticIpAllocation = (o.Spec.SubnetDHCPConfig.Mode == "" || o.Spec.SubnetDHCPConfig.Mode == v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeDeactivated))
83+
staticIpAllocation = o.Spec.SubnetDHCPConfig.Mode == "" || o.Spec.SubnetDHCPConfig.Mode == v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeDeactivated)
8484
index := util.GetRandomIndexString()
8585
nsxSubnet = &model.VpcSubnet{
8686
Id: String(service.buildSubnetSetID(o, index)),
@@ -124,7 +124,7 @@ func (service *SubnetService) buildDHCPConfig(enableDHCP bool) *model.VpcSubnetD
124124
}
125125

126126
func (service *SubnetService) buildSubnetDHCPConfig(mode string) *model.SubnetDhcpConfig {
127-
// Trasfer DHCPDeactivated to DHCP_DEACTIVATED
127+
// Transfer DHCPDeactivated to DHCP_DEACTIVATED
128128
nsxMode := strings.ToUpper(mode)
129129
nsxMode = nsxMode[:4] + "_" + nsxMode[4:]
130130

‎pkg/nsx/services/subnet/subnet.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ func (service *SubnetService) UpdateSubnetSet(ns string, vpcSubnets []*model.Vpc
411411
if dhcpMode == "" {
412412
dhcpMode = v1alpha1.DHCPConfigModeDeactivated
413413
}
414-
staticIpAllocation := (dhcpMode == v1alpha1.DHCPConfigModeDeactivated)
414+
staticIpAllocation := dhcpMode == v1alpha1.DHCPConfigModeDeactivated
415415
for i, vpcSubnet := range vpcSubnets {
416416
subnetSet := &v1alpha1.SubnetSet{}
417417
var name string

0 commit comments

Comments
 (0)