From bed5d59aaf42f84c285df3d41f0992e836c300af Mon Sep 17 00:00:00 2001 From: Riya Jain Date: Thu, 21 Nov 2024 09:45:34 -0800 Subject: [PATCH 1/3] modified unit tests --- .../v2/networkPolicyController_test.go | 156 ++++++++++++++---- 1 file changed, 127 insertions(+), 29 deletions(-) diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go index 1c531e7dea..03d4b8987f 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics/promutil" "github.com/Azure/azure-container-networking/npm/pkg/dataplane" dpmocks "github.com/Azure/azure-container-networking/npm/pkg/dataplane/mocks" + "github.com/Azure/azure-container-networking/npm/util" gomock "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -302,24 +303,71 @@ func TestAddMultipleNetworkPolicies(t *testing.T) { dp := dpmocks.NewMockGenericDataplane(ctrl) f.newNetPolController(stopCh, dp, false) + var testCases []expectedNetPolValues + + if util.IsWindowsDP() { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) + addNetPol(f, netPolObj1) + addNetPol(f, netPolObj2) + + // already exists (will be a no-op) + addNetPol(f, netPolObj1) + // named ports are not allowed on windows + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 0, 0, 0}}, + } + } else { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) + addNetPol(f, netPolObj1) + addNetPol(f, netPolObj2) + + // already exists (will be a no-op) + addNetPol(f, netPolObj1) + testCases = []expectedNetPolValues{ + {2, 0, netPolPromVals{2, 2, 0, 0}}, + } + } + + checkNetPolTestResult("TestAddMultipleNetPols", f, testCases) +} + +func TestAddNetworkPolicy(t *testing.T) { + netPolObj := createNetPol() + + f := newNetPolFixture(t) + f.netPolLister = append(f.netPolLister, netPolObj) + f.kubeobjects = append(f.kubeobjects, netPolObj) + stopCh := make(chan struct{}) + defer close(stopCh) + ctrl := gomock.NewController(t) + defer ctrl.Finish() - dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) + dp := dpmocks.NewMockGenericDataplane(ctrl) + f.newNetPolController(stopCh, dp, false) - addNetPol(f, netPolObj1) - addNetPol(f, netPolObj2) + var testCases []expectedNetPolValues - // already exists (will be a no-op) - addNetPol(f, netPolObj1) + if util.IsWindowsDP() { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) + addNetPol(f, netPolObj) + // named ports are not allowed on windows + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 0, 0, 0}}, + } + } else { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) - testCases := []expectedNetPolValues{ - {2, 0, netPolPromVals{2, 2, 0, 0}}, + addNetPol(f, netPolObj) + testCases = []expectedNetPolValues{ + {1, 0, netPolPromVals{1, 1, 0, 0}}, + } } - checkNetPolTestResult("TestAddMultipleNetPols", f, testCases) + checkNetPolTestResult("TestAddNetPol", f, testCases) } -func TestAddNetworkPolicy(t *testing.T) { +func TestAddNetworkPolicyWithNumericPort(t *testing.T) { netPolObj := createNetPol() - + netPolObj.Spec.Egress[0].Ports[0].Port = &intstr.IntOrString{IntVal: 8000} f := newNetPolFixture(t) f.netPolLister = append(f.netPolLister, netPolObj) f.kubeobjects = append(f.kubeobjects, netPolObj) @@ -331,10 +379,12 @@ func TestAddNetworkPolicy(t *testing.T) { dp := dpmocks.NewMockGenericDataplane(ctrl) f.newNetPolController(stopCh, dp, false) + var testCases []expectedNetPolValues + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) addNetPol(f, netPolObj) - testCases := []expectedNetPolValues{ + testCases = []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 0, 0}}, } @@ -403,12 +453,24 @@ func TestDeleteNetworkPolicy(t *testing.T) { dp := dpmocks.NewMockGenericDataplane(ctrl) f.newNetPolController(stopCh, dp, false) - dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) - dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) + var testCases []expectedNetPolValues - deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) - testCases := []expectedNetPolValues{ - {0, 0, netPolPromVals{0, 1, 0, 1}}, + if util.IsWindowsDP() { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) + dp.EXPECT().RemovePolicy(gomock.Any()).Times(0) + + deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 0, 0, 0}}, + } + } else { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) + dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) + + deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 1, 0, 1}}, + } } checkNetPolTestResult("TestDelNetPol", f, testCases) } @@ -454,12 +516,24 @@ func TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy(t *testing.T) dp := dpmocks.NewMockGenericDataplane(ctrl) f.newNetPolController(stopCh, dp, false) - dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) - dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) + var testCases []expectedNetPolValues - deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) - testCases := []expectedNetPolValues{ - {0, 0, netPolPromVals{0, 1, 0, 1}}, + if util.IsWindowsDP() { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) + dp.EXPECT().RemovePolicy(gomock.Any()).Times(0) + + deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 0, 0, 0}}, + } + } else { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) + dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) + + deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 1, 0, 1}}, + } } checkNetPolTestResult("TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy", f, testCases) } @@ -484,11 +558,22 @@ func TestUpdateNetworkPolicy(t *testing.T) { // oldNetPolObj.ResourceVersion value is "0" newRV, _ := strconv.Atoi(oldNetPolObj.ResourceVersion) newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) - dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) + var testCases []expectedNetPolValues - updateNetPol(t, f, oldNetPolObj, newNetPolObj) - testCases := []expectedNetPolValues{ - {1, 0, netPolPromVals{1, 1, 0, 0}}, + if util.IsWindowsDP() { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) + + updateNetPol(t, f, oldNetPolObj, newNetPolObj) + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 0, 0, 0}}, + } + } else { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) + + updateNetPol(t, f, oldNetPolObj, newNetPolObj) + testCases = []expectedNetPolValues{ + {1, 0, netPolPromVals{1, 1, 0, 0}}, + } } checkNetPolTestResult("TestUpdateNetPol", f, testCases) } @@ -518,12 +603,25 @@ func TestLabelUpdateNetworkPolicy(t *testing.T) { // oldNetPolObj.ResourceVersion value is "0" newRV, _ := strconv.Atoi(oldNetPolObj.ResourceVersion) newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) - dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) + var testCases []expectedNetPolValues - testCases := []expectedNetPolValues{ - {1, 0, netPolPromVals{1, 1, 1, 0}}, + if util.IsWindowsDP() { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) + + updateNetPol(t, f, oldNetPolObj, newNetPolObj) + + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 0, 0, 0}}, + } + } else { + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) + + updateNetPol(t, f, oldNetPolObj, newNetPolObj) + + testCases = []expectedNetPolValues{ + {1, 0, netPolPromVals{1, 1, 1, 0}}, + } } checkNetPolTestResult("TestUpdateNetPol", f, testCases) } From 24626be51af358a513871985bb4d84ae62fea2b3 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Thu, 21 Nov 2024 10:52:49 -0800 Subject: [PATCH 2/3] refactored code --- .../v2/networkPolicyController_test.go | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go index 03d4b8987f..d14f6f67f1 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go @@ -307,26 +307,22 @@ func TestAddMultipleNetworkPolicies(t *testing.T) { if util.IsWindowsDP() { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) - addNetPol(f, netPolObj1) - addNetPol(f, netPolObj2) - - // already exists (will be a no-op) - addNetPol(f, netPolObj1) // named ports are not allowed on windows testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 0, 0, 0}}, } } else { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) - addNetPol(f, netPolObj1) - addNetPol(f, netPolObj2) - // already exists (will be a no-op) - addNetPol(f, netPolObj1) testCases = []expectedNetPolValues{ {2, 0, netPolPromVals{2, 2, 0, 0}}, } } + addNetPol(f, netPolObj1) + addNetPol(f, netPolObj2) + + // already exists (will be a no-op) + addNetPol(f, netPolObj1) checkNetPolTestResult("TestAddMultipleNetPols", f, testCases) } @@ -349,7 +345,6 @@ func TestAddNetworkPolicy(t *testing.T) { if util.IsWindowsDP() { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) - addNetPol(f, netPolObj) // named ports are not allowed on windows testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 0, 0, 0}}, @@ -357,11 +352,12 @@ func TestAddNetworkPolicy(t *testing.T) { } else { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) - addNetPol(f, netPolObj) testCases = []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 0, 0}}, } } + addNetPol(f, netPolObj) + checkNetPolTestResult("TestAddNetPol", f, testCases) } @@ -459,7 +455,6 @@ func TestDeleteNetworkPolicy(t *testing.T) { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) dp.EXPECT().RemovePolicy(gomock.Any()).Times(0) - deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 0, 0, 0}}, } @@ -467,11 +462,11 @@ func TestDeleteNetworkPolicy(t *testing.T) { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) - deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 1, 0, 1}}, } } + deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) checkNetPolTestResult("TestDelNetPol", f, testCases) } @@ -522,7 +517,6 @@ func TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy(t *testing.T) dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) dp.EXPECT().RemovePolicy(gomock.Any()).Times(0) - deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 0, 0, 0}}, } @@ -530,11 +524,12 @@ func TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy(t *testing.T) dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) - deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 1, 0, 1}}, } } + deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) + checkNetPolTestResult("TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy", f, testCases) } @@ -563,18 +558,18 @@ func TestUpdateNetworkPolicy(t *testing.T) { if util.IsWindowsDP() { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 0, 0, 0}}, } } else { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) testCases = []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 0, 0}}, } } + updateNetPol(t, f, oldNetPolObj, newNetPolObj) + checkNetPolTestResult("TestUpdateNetPol", f, testCases) } @@ -609,19 +604,17 @@ func TestLabelUpdateNetworkPolicy(t *testing.T) { if util.IsWindowsDP() { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(0) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) - testCases = []expectedNetPolValues{ {0, 0, netPolPromVals{0, 0, 0, 0}}, } } else { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) - testCases = []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 1, 0}}, } } + updateNetPol(t, f, oldNetPolObj, newNetPolObj) + checkNetPolTestResult("TestUpdateNetPol", f, testCases) } From 9e3d51741efbe98218d67e08d1605288ee657c77 Mon Sep 17 00:00:00 2001 From: rejain456 Date: Mon, 6 Jan 2025 13:34:40 -0800 Subject: [PATCH 3/3] fixed unit tests --- npm/pkg/dataplane/dataplane_windows_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index bfd2dd11f3..20b1d1bb37 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -42,14 +42,14 @@ func TestMetrics(t *testing.T) { count, err = metrics.TotalListEndpointsLatencyCalls() require.Nil(t, err, "failed to get metric") - require.Equal(t, 1, count, "should have listed endpoints once") + require.Equal(t, 2, count, "should have listed endpoints twice") err = dp.refreshPodEndpoints() require.Nil(t, err, "failed to refresh pod endpoints") count, err = metrics.TotalListEndpointsLatencyCalls() require.Nil(t, err, "failed to get metric") - require.Equal(t, 2, count, "should have listed endpoints twice") + require.Equal(t, 4, count, "should have listed endpoints four times") count, err = metrics.TotalListEndpointsFailures() require.Nil(t, err, "failed to get metric")