From 0b3872b07b983f148f5d6da008af279ca4c1a0a7 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 20 Oct 2020 23:54:40 -0700 Subject: [PATCH 01/11] Add nc version in SecondaryIPConfig stucture. --- cns/NetworkContainerContract.go | 1 + cns/requestcontroller/kubecontroller/crdtranslator.go | 4 +++- cns/restserver/internalapi_test.go | 5 +++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 4f0940e9a6..2faa3787aa 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -131,6 +131,7 @@ type IPConfiguration struct { // SecondaryIPConfig contains IP info of SecondaryIP type SecondaryIPConfig struct { IPAddress string + NCVersion int } // IPSubnet contains ip subnet. diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index aa8c9e89cd..ef09782b6c 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -3,6 +3,7 @@ package kubecontroller import ( "fmt" "net" + "strconv" "github.com/Azure/azure-container-networking/cns" nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" @@ -56,9 +57,10 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw if ip = net.ParseIP(ipAssignment.IP); ip == nil { return ncRequest, fmt.Errorf("Invalid SecondaryIP %s:", ipAssignment.IP) } - + ncVersion, _ := strconv.Atoi(ncRequest.Version) secondaryIPConfig = cns.SecondaryIPConfig{ IPAddress: ip.String(), + NCVersion: ncVersion, } ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 923cc46032..3e6c5faa0c 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -342,7 +342,12 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon // validate rest of Secondary IPs in Available state if ncRequest != nil { + ncRequestVersion, _ := strconv.Atoi(ncRequest.Version) for secIpId, secIpConfig := range ncRequest.SecondaryIPConfigs { + if secIpConfig.NCVersion != ncRequestVersion { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, they are not equal", + ncRequestVersion, secIpConfig.IPAddress, secIpConfig.NCVersion) + } if _, exists := expectedAllocatedPods[secIpConfig.IPAddress]; exists { continue } From 7df229fec47c52330b4e8443419e0d27c49aaf6d Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 21 Oct 2020 10:32:02 -0700 Subject: [PATCH 02/11] Only update IP version when IP changes. --- .../kubecontroller/crdtranslator.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index ef09782b6c..4c4df578cd 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -57,12 +57,16 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw if ip = net.ParseIP(ipAssignment.IP); ip == nil { return ncRequest, fmt.Errorf("Invalid SecondaryIP %s:", ipAssignment.IP) } - ncVersion, _ := strconv.Atoi(ncRequest.Version) - secondaryIPConfig = cns.SecondaryIPConfig{ - IPAddress: ip.String(), - NCVersion: ncVersion, + + ipConfig, ok := ncRequest.SecondaryIPConfigs[ipAssignment.Name] + if !ok || (ok && ipConfig.IPAddress != ip.String()) { + ncVersion, _ := strconv.Atoi(ncRequest.Version) + secondaryIPConfig = cns.SecondaryIPConfig{ + IPAddress: ip.String(), + NCVersion: ncVersion, + } + ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig } - ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig } } From 3cdb96216b83e4ceddf19054220ab0271c222cba Mon Sep 17 00:00:00 2001 From: Shufang Date: Fri, 23 Oct 2020 00:50:17 -0700 Subject: [PATCH 03/11] Add unit test to secondary IP NC version update. --- .../kubecontroller/crdtranslator.go | 14 ++-- .../kubecontroller/crdtranslator_test.go | 6 ++ cns/restserver/internalapi_test.go | 72 ++++++++++++++++--- cns/restserver/ipam_test.go | 59 +++++++-------- cns/restserver/util.go | 19 +++-- 5 files changed, 116 insertions(+), 54 deletions(-) diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index 4c4df578cd..89de61fb51 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -52,21 +52,17 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ipSubnet.PrefixLength = uint8(size) ncRequest.IPConfiguration.IPSubnet = ipSubnet ncRequest.IPConfiguration.GatewayIPAddress = nc.DefaultGateway + ncVersion, _ := strconv.Atoi(ncRequest.Version) for _, ipAssignment = range nc.IPAssignments { if ip = net.ParseIP(ipAssignment.IP); ip == nil { return ncRequest, fmt.Errorf("Invalid SecondaryIP %s:", ipAssignment.IP) } - - ipConfig, ok := ncRequest.SecondaryIPConfigs[ipAssignment.Name] - if !ok || (ok && ipConfig.IPAddress != ip.String()) { - ncVersion, _ := strconv.Atoi(ncRequest.Version) - secondaryIPConfig = cns.SecondaryIPConfig{ - IPAddress: ip.String(), - NCVersion: ncVersion, - } - ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig + secondaryIPConfig = cns.SecondaryIPConfig{ + IPAddress: ip.String(), + NCVersion: ncVersion, } + ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig } } diff --git a/cns/requestcontroller/kubecontroller/crdtranslator_test.go b/cns/requestcontroller/kubecontroller/crdtranslator_test.go index 3153680915..e34053950e 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator_test.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator_test.go @@ -1,6 +1,7 @@ package kubecontroller import ( + "strconv" "testing" "github.com/Azure/azure-container-networking/cns" @@ -235,4 +236,9 @@ func TestStatusToNCRequestSuccess(t *testing.T) { if secondaryIP.IPAddress != testSecIp1 { t.Fatalf("Expected %v as the secondary IP config but got %v", testSecIp1, secondaryIP.IPAddress) } + + ncVersionInInt, _ := strconv.Atoi(version) + if secondaryIP.NCVersion != ncVersionInInt { + t.Fatalf("Expected %d as the secondary IP config NC version but got %v", ncVersionInInt, secondaryIP.NCVersion) + } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 3e6c5faa0c..77f5bd3810 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -48,6 +48,26 @@ func TestCreateOrUpdateNCWithLargerVersionComparedToNMAgent(t *testing.T) { validateCreateOrUpdateNCInternal(t, 2, "1") } +func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { + restartService() + + setEnv(t) + setOrchestratorTypeInternal(cns.KubernetesCRD) + // NC version set as 0 which is the default initial value. + ncVersion := 0 + secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) + ncID := "testNc1" + addSecondaryIPsToConfig("10.0.0.16", ncVersion, secondaryIPConfigs) + req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) + validateSecondaryIPsNCVersion(t, req) + + // now Validate Update, add more secondaryIpConfig and it should handle the update + ncVersion++ + addSecondaryIPsToConfig("10.0.0.17", ncVersion, secondaryIPConfigs) + req = createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) + validateSecondaryIPsNCVersion(t, req) +} + func TestReconcileNCWithEmptyState(t *testing.T) { restartService() setEnv(t) @@ -73,7 +93,7 @@ func TestReconcileNCWithExistingState(t *testing.T) { var startingIndex = 6 for i := 0; i < 4; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress) + secIpConfig := newSecondaryIPConfig(ipaddress, 0) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -110,7 +130,7 @@ func TestReconcileNCWithSystemPods(t *testing.T) { var startingIndex = 6 for i := 0; i < 4; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress) + secIpConfig := newSecondaryIPConfig(ipaddress, 0) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -151,7 +171,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVers var startingIndex = 6 for i := 0; i < secondaryIpCount; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress) + secIpConfig := newSecondaryIPConfig(ipaddress, 0) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -163,7 +183,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVers fmt.Println("Validate Scaleup") for i := 0; i < secondaryIpCount; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress) + secIpConfig := newSecondaryIPConfig(ipaddress, 1) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -289,9 +309,12 @@ func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConf Version: ncVersion, } + ncVersionInInt, _ := strconv.Atoi(ncVersion) req.SecondaryIPConfigs = make(map[string]cns.SecondaryIPConfig) for k, v := range secondaryIps { req.SecondaryIPConfigs[k] = v + ipconfig, _ := req.SecondaryIPConfigs[k] + ipconfig.NCVersion = ncVersionInInt } fmt.Printf("NC Request %+v", req) @@ -342,12 +365,7 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon // validate rest of Secondary IPs in Available state if ncRequest != nil { - ncRequestVersion, _ := strconv.Atoi(ncRequest.Version) for secIpId, secIpConfig := range ncRequest.SecondaryIPConfigs { - if secIpConfig.NCVersion != ncRequestVersion { - t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, they are not equal", - ncRequestVersion, secIpConfig.IPAddress, secIpConfig.NCVersion) - } if _, exists := expectedAllocatedPods[secIpConfig.IPAddress]; exists { continue } @@ -364,6 +382,42 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon } } +func addSecondaryIPsToConfig(ipaddress string, ncVersion int, secondaryIPConfigs map[string]cns.SecondaryIPConfig) { + secIPConfig := newSecondaryIPConfig(ipaddress, ncVersion) + ipID := uuid.New() + secondaryIPConfigs[ipID.String()] = secIPConfig +} + +func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncID, ncVersion string) cns.CreateNetworkContainerRequest { + req := generateNetworkContainerRequest(secondaryIPConfigs, ncID, ncVersion) + returnCode := svc.CreateOrUpdateNetworkContainerInternal(req, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + if returnCode != 0 { + t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) + } + return req +} + +func validateSecondaryIPsNCVersion(t *testing.T, req cns.CreateNetworkContainerRequest) { + containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] + // Validate secondary IPs' NC version has been updated by NC request + ncVersion, _ := strconv.Atoi(containerStatus.VMVersion) + for _, secIPConfig := range containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs { + if secIPConfig.IPAddress == "10.0.0.16" { + // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 + if secIPConfig.NCVersion != 0 { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) + } + } + if secIPConfig.IPAddress == "10.0.0.17" { + if secIPConfig.NCVersion != 1 { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 1", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) + } + } + } +} + func restartService() { fmt.Println("Restart Service") diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index f1a280eea3..e7fe083372 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -49,14 +49,15 @@ func getTestService() *HTTPRestService { return svc } -func newSecondaryIPConfig(ipAddress string) cns.SecondaryIPConfig { +func newSecondaryIPConfig(ipAddress string, ncVersion int) cns.SecondaryIPConfig { return cns.SecondaryIPConfig{ IPAddress: ipAddress, + NCVersion: ncVersion, } } -func NewPodState(ipaddress string, prefixLength uint8, id, ncid, state string) cns.IPConfigurationStatus { - ipconfig := newSecondaryIPConfig(ipaddress) +func NewPodState(ipaddress string, prefixLength uint8, id, ncid, state string, ncVersion int) cns.IPConfigurationStatus { + ipconfig := newSecondaryIPConfig(ipaddress, ncVersion) return cns.IPConfigurationStatus{ IPAddress: ipconfig.IPAddress, @@ -119,8 +120,8 @@ func requestIpAddressAndGetState(t *testing.T, req cns.GetIPConfigRequest) (cns. return ipState, err } -func NewPodStateWithOrchestratorContext(ipaddress string, prefixLength uint8, id, ncid, state string, orchestratorContext cns.KubernetesPodInfo) (cns.IPConfigurationStatus, error) { - ipconfig := newSecondaryIPConfig(ipaddress) +func NewPodStateWithOrchestratorContext(ipaddress, id, ncid, state string, prefixLength uint8, ncVersion int, orchestratorContext cns.KubernetesPodInfo) (cns.IPConfigurationStatus, error) { + ipconfig := newSecondaryIPConfig(ipaddress, ncVersion) b, err := json.Marshal(orchestratorContext) return cns.IPConfigurationStatus{ IPAddress: ipconfig.IPAddress, @@ -166,7 +167,7 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st func TestIPAMGetAvailableIPConfig(t *testing.T) { svc := getTestService() - testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available) + testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ testState.ID: testState, } @@ -181,7 +182,7 @@ func TestIPAMGetAvailableIPConfig(t *testing.T) { t.Fatal("Expected IP retrieval to be nil") } - desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated) + desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, 0) desiredState.OrchestratorContext = b if reflect.DeepEqual(desiredState, actualstate) != true { @@ -195,8 +196,8 @@ func TestIPAMGetNextAvailableIPConfig(t *testing.T) { // Add already allocated pod ip to state svc.PodIPIDByOrchestratorContext[testPod1Info.GetOrchestratorContextKey()] = testPod1GUID - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) - state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) + state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, @@ -216,7 +217,7 @@ func TestIPAMGetNextAvailableIPConfig(t *testing.T) { t.Fatalf("Expected IP retrieval to be nil: %+v", err) } // want second available Pod IP State as first has been allocated - desiredState, _ := NewPodStateWithOrchestratorContext(testIP2, 24, testPod2GUID, testNCID, cns.Allocated, testPod2Info) + desiredState, _ := NewPodStateWithOrchestratorContext(testIP2, testPod2GUID, testNCID, cns.Allocated, 24, 0, testPod2Info) if reflect.DeepEqual(desiredState, actualstate) != true { t.Fatalf("Desired state not matching actual state, expected: %+v, actual: %+v", desiredState, actualstate) @@ -227,7 +228,7 @@ func TestIPAMGetAlreadyAllocatedIPConfigForSamePod(t *testing.T) { svc := getTestService() // Add Allocated Pod IP to state - testState, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + testState, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) ipconfigs := map[string]cns.IPConfigurationStatus{ testState.ID: testState, } @@ -245,7 +246,7 @@ func TestIPAMGetAlreadyAllocatedIPConfigForSamePod(t *testing.T) { t.Fatalf("Expected not error: %+v", err) } - desiredState, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + desiredState, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) if reflect.DeepEqual(desiredState, actualstate) != true { t.Fatalf("Desired state not matching actual state, expected: %+v, actual: %+v", desiredState, actualstate) @@ -256,7 +257,7 @@ func TestIPAMAttemptToRequestIPNotFoundInPool(t *testing.T) { svc := getTestService() // Add Available Pod IP to state - testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available) + testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ testState.ID: testState, } @@ -281,7 +282,7 @@ func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { svc := getTestService() // Add Available Pod IP to state - testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available) + testState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ testState.ID: testState, } @@ -301,7 +302,7 @@ func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { t.Fatalf("Expected IP retrieval to be nil: %+v", err) } - desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated) + desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, 0) desiredState.OrchestratorContext = b if reflect.DeepEqual(desiredState, actualstate) != true { @@ -313,7 +314,7 @@ func TestIPAMFailToGetDesiredIPConfigWithAlreadyAllocatedSpecfiedIP(t *testing.T svc := getTestService() // set state as already allocated - testState, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + testState, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) ipconfigs := map[string]cns.IPConfigurationStatus{ testState.ID: testState, } @@ -338,8 +339,8 @@ func TestIPAMFailToGetIPWhenAllIPsAreAllocated(t *testing.T) { svc := getTestService() // set state as already allocated - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) - state2, _ := NewPodStateWithOrchestratorContext(testIP2, 24, testPod2GUID, testNCID, cns.Allocated, testPod2Info) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) + state2, _ := NewPodStateWithOrchestratorContext(testIP2, testPod2GUID, testNCID, cns.Allocated, 24, 0, testPod2Info) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, @@ -369,7 +370,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { svc := getTestService() // set state as already allocated - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, } @@ -409,7 +410,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { t.Fatalf("Expected IP retrieval to be nil: %+v", err) } - desiredState, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + desiredState, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) // want first available Pod IP State desiredState.IPAddress = desiredIpAddress desiredState.OrchestratorContext = b @@ -422,7 +423,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { func TestIPAMReleaseIPIdempotency(t *testing.T) { svc := getTestService() // set state as already allocated - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, } @@ -448,7 +449,7 @@ func TestIPAMReleaseIPIdempotency(t *testing.T) { func TestIPAMAllocateIPIdempotency(t *testing.T) { svc := getTestService() // set state as already allocated - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, @@ -468,9 +469,9 @@ func TestIPAMAllocateIPIdempotency(t *testing.T) { func TestAvailableIPConfigs(t *testing.T) { svc := getTestService() - state1 := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available) - state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available) - state3 := NewPodState(testIP3, 24, testPod3GUID, testNCID, cns.Available) + state1 := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Available, 0) + state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) + state3 := NewPodState(testIP3, 24, testPod3GUID, testNCID, cns.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, @@ -505,7 +506,7 @@ func TestAvailableIPConfigs(t *testing.T) { availableIps = svc.GetAvailableIPConfigs() validateIpState(t, availableIps, desiredAvailableIps) - desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated) + desiredState := NewPodState(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, 0) desiredState.OrchestratorContext = b desiredAllocatedIpConfigs[desiredState.ID] = desiredState allocatedIps = svc.GetAllocatedIPConfigs() @@ -537,7 +538,7 @@ func validateIpState(t *testing.T, actualIps []cns.IPConfigurationStatus, expect func TestIPAMMarkIPCountAsPending(t *testing.T) { svc := getTestService() // set state as already allocated - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Available, testPod1Info) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Available, 24, 0, testPod1Info) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, } @@ -580,8 +581,8 @@ func TestIPAMMarkExistingIPConfigAsPending(t *testing.T) { // Add already allocated pod ip to state svc.PodIPIDByOrchestratorContext[testPod1Info.GetOrchestratorContextKey()] = testPod1GUID - state1, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) - state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available) + state1, _ := NewPodStateWithOrchestratorContext(testIP1, testPod1GUID, testNCID, cns.Allocated, 24, 0, testPod1Info) + state2 := NewPodState(testIP2, 24, testPod2GUID, testNCID, cns.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ state1.ID: state1, diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 645aa06d62..5366935eeb 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -237,9 +237,9 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN nmagentNCVersion = service.imdsClient.GetNetworkContainerInfoFromHostWithoutToken() if nmagentNCVersion >= newNCVersion { - service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs) + service.addIPConfigStateUntransacted(cns.Available, req.NetworkContainerid, newIPConfigs, existingSecondaryIPConfigs) } else { - service.addIPConfigStateUntransacted(cns.PendingProgramming, req.NetworkContainerid, newIPConfigs) + service.addIPConfigStateUntransacted(cns.PendingProgramming, req.NetworkContainerid, newIPConfigs, existingSecondaryIPConfigs) } return 0, "" @@ -248,23 +248,28 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncId string, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state - for ipId, ipconfig := range ipconfigs { - if _, exists := service.PodIPConfigState[ipId]; exists { + for ipID, ipconfig := range ipconfigs { + // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new NC version + // Set it back to previous NC version if IP already exist. + if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipID]; existsInPreviousIPConfig && existingIPConfig.IPAddress == ipconfig.IPAddress { + ipconfig.NCVersion = existingIPConfig.NCVersion + } + if _, exists := service.PodIPConfigState[ipID]; exists { continue } // add the new State ipconfigStatus := cns.IPConfigurationStatus{ NCID: ncId, - ID: ipId, + ID: ipID, IPAddress: ipconfig.IPAddress, State: newIPCNSStatus, OrchestratorContext: nil, } logger.Printf("[Azure-Cns] Add IP %s as %s", ipconfig.IPAddress, newIPCNSStatus) - service.PodIPConfigState[ipId] = ipconfigStatus + service.PodIPConfigState[ipID] = ipconfigStatus // Todo Update batch API and maintain the count } From 8a17ba28b298ede7307db8b9ed0281b871ca0301 Mon Sep 17 00:00:00 2001 From: Shufang Date: Fri, 23 Oct 2020 09:58:48 -0700 Subject: [PATCH 04/11] Simplify unit test. --- cns/restserver/internalapi_test.go | 44 ++++++++++++++++-------------- cns/restserver/util.go | 1 + 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 77f5bd3810..e59a0a7b04 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -57,13 +57,28 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { ncVersion := 0 secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) ncID := "testNc1" - addSecondaryIPsToConfig("10.0.0.16", ncVersion, secondaryIPConfigs) + + // Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0} + ipAddress := "10.0.0.16" + secIPConfig := newSecondaryIPConfig(ipAddress, ncVersion) + ipID := uuid.New() + secondaryIPConfigs[ipID.String()] = secIPConfig req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) validateSecondaryIPsNCVersion(t, req) - // now Validate Update, add more secondaryIpConfig and it should handle the update + // now Validate Update, simulate the CRD status where have 2 IP addresses as "10.0.0.16" and "10.0.0.17" with NC version 1 + // The secondaryIPConfigs build from CRD will be {[IPAddress:"10.0.0.16", NCVersion: 1,]; [IPAddress:"10.0.0.17", NCVersion: 1,]} + // However, when CNS saveNetworkContainerGoalState, since "10.0.0.16" already with NC version 0, it will set it back to 0. ncVersion++ - addSecondaryIPsToConfig("10.0.0.17", ncVersion, secondaryIPConfigs) + secIPConfig = newSecondaryIPConfig(ipAddress, ncVersion) + // "10.0.0.16" will be update to NC version 1 in CRD, reuse the same uuid with it. + secondaryIPConfigs[ipID.String()] = secIPConfig + + // Add {IPAddress:"10.0.0.17", NCVersion: 1} in secondaryIPConfig + ipAddress = "10.0.0.17" + secIPConfig = newSecondaryIPConfig(ipAddress, ncVersion) + ipID = uuid.New() + secondaryIPConfigs[ipID.String()] = secIPConfig req = createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) validateSecondaryIPsNCVersion(t, req) } @@ -382,12 +397,6 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon } } -func addSecondaryIPsToConfig(ipaddress string, ncVersion int, secondaryIPConfigs map[string]cns.SecondaryIPConfig) { - secIPConfig := newSecondaryIPConfig(ipaddress, ncVersion) - ipID := uuid.New() - secondaryIPConfigs[ipID.String()] = secIPConfig -} - func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncID, ncVersion string) cns.CreateNetworkContainerRequest { req := generateNetworkContainerRequest(secondaryIPConfigs, ncID, ncVersion) returnCode := svc.CreateOrUpdateNetworkContainerInternal(req, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) @@ -402,18 +411,11 @@ func validateSecondaryIPsNCVersion(t *testing.T, req cns.CreateNetworkContainerR // Validate secondary IPs' NC version has been updated by NC request ncVersion, _ := strconv.Atoi(containerStatus.VMVersion) for _, secIPConfig := range containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs { - if secIPConfig.IPAddress == "10.0.0.16" { - // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 - if secIPConfig.NCVersion != 0 { - t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", - ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) - } - } - if secIPConfig.IPAddress == "10.0.0.17" { - if secIPConfig.NCVersion != 1 { - t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 1", - ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) - } + // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 + if (secIPConfig.IPAddress == "10.0.0.16" && secIPConfig.NCVersion != 0) || + (secIPConfig.IPAddress == "10.0.0.17" && secIPConfig.NCVersion != 1) { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) } } } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 5366935eeb..f66b61518d 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -255,6 +255,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncI // Set it back to previous NC version if IP already exist. if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipID]; existsInPreviousIPConfig && existingIPConfig.IPAddress == ipconfig.IPAddress { ipconfig.NCVersion = existingIPConfig.NCVersion + ipconfigs[ipID] = ipconfig } if _, exists := service.PodIPConfigState[ipID]; exists { continue From b7ae041357aef58e3852d021cf00ba4e6050c8be Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 27 Oct 2020 00:09:55 -0700 Subject: [PATCH 05/11] Add more comments. --- cns/NetworkContainerContract.go | 1 + cns/restserver/internalapi_test.go | 10 +++++----- cns/restserver/util.go | 13 +++++++------ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 2faa3787aa..465b1c0963 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -131,6 +131,7 @@ type IPConfiguration struct { // SecondaryIPConfig contains IP info of SecondaryIP type SecondaryIPConfig struct { IPAddress string + // NCVesion will help in determining whether IP is in pending programming or available when reconciling. NCVersion int } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index e59a0a7b04..8af223824a 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -61,8 +61,8 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { // Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0} ipAddress := "10.0.0.16" secIPConfig := newSecondaryIPConfig(ipAddress, ncVersion) - ipID := uuid.New() - secondaryIPConfigs[ipID.String()] = secIPConfig + ipId := uuid.New() + secondaryIPConfigs[ipId.String()] = secIPConfig req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) validateSecondaryIPsNCVersion(t, req) @@ -72,13 +72,13 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { ncVersion++ secIPConfig = newSecondaryIPConfig(ipAddress, ncVersion) // "10.0.0.16" will be update to NC version 1 in CRD, reuse the same uuid with it. - secondaryIPConfigs[ipID.String()] = secIPConfig + secondaryIPConfigs[ipId.String()] = secIPConfig // Add {IPAddress:"10.0.0.17", NCVersion: 1} in secondaryIPConfig ipAddress = "10.0.0.17" secIPConfig = newSecondaryIPConfig(ipAddress, ncVersion) - ipID = uuid.New() - secondaryIPConfigs[ipID.String()] = secIPConfig + ipId = uuid.New() + secondaryIPConfigs[ipId.String()] = secIPConfig req = createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) validateSecondaryIPsNCVersion(t, req) } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index f66b61518d..cdc13e1467 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -250,27 +250,28 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN // acquire/release the service lock. func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncId string, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state - for ipID, ipconfig := range ipconfigs { + for ipId, ipconfig := range ipconfigs { // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new NC version // Set it back to previous NC version if IP already exist. - if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipID]; existsInPreviousIPConfig && existingIPConfig.IPAddress == ipconfig.IPAddress { + if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig && existingIPConfig.IPAddress == ipconfig.IPAddress { ipconfig.NCVersion = existingIPConfig.NCVersion - ipconfigs[ipID] = ipconfig + ipconfigs[ipId] = ipconfig + logger.Printf("[Azure-Cns] Set IP %s version from %d back to %d", ipconfig.IPAddress, ipconfig.NCVersion, existingIPConfig.NCVersion) } - if _, exists := service.PodIPConfigState[ipID]; exists { + if _, exists := service.PodIPConfigState[ipId]; exists { continue } // add the new State ipconfigStatus := cns.IPConfigurationStatus{ NCID: ncId, - ID: ipID, + ID: ipId, IPAddress: ipconfig.IPAddress, State: newIPCNSStatus, OrchestratorContext: nil, } logger.Printf("[Azure-Cns] Add IP %s as %s", ipconfig.IPAddress, newIPCNSStatus) - service.PodIPConfigState[ipID] = ipconfigStatus + service.PodIPConfigState[ipId] = ipconfigStatus // Todo Update batch API and maintain the count } From 8eecb898782e6e4f7141b49fecae1557c90cc0e8 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 27 Oct 2020 10:06:21 -0700 Subject: [PATCH 06/11] Update comments. --- cns/restserver/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index cdc13e1467..2865770ba8 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -256,8 +256,8 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncI if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig && existingIPConfig.IPAddress == ipconfig.IPAddress { ipconfig.NCVersion = existingIPConfig.NCVersion ipconfigs[ipId] = ipconfig - logger.Printf("[Azure-Cns] Set IP %s version from %d back to %d", ipconfig.IPAddress, ipconfig.NCVersion, existingIPConfig.NCVersion) } + logger.Printf("[Azure-Cns] Set IP %s version to %d", ipconfig.IPAddress, ipconfig.NCVersion) if _, exists := service.PodIPConfigState[ipId]; exists { continue } From b90580378c94de2494614d33e2b53e51ae5c889e Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 27 Oct 2020 23:35:14 -0700 Subject: [PATCH 07/11] Add nil check to string to int change. --- .../kubecontroller/crdtranslator.go | 5 ++++- cns/restserver/internalapi_test.go | 22 +++++++++++++++++-- cns/restserver/util.go | 2 +- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index 89de61fb51..cb0a79d62e 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -52,7 +52,10 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ipSubnet.PrefixLength = uint8(size) ncRequest.IPConfiguration.IPSubnet = ipSubnet ncRequest.IPConfiguration.GatewayIPAddress = nc.DefaultGateway - ncVersion, _ := strconv.Atoi(ncRequest.Version) + var ncVersion int + if ncVersion, err = strconv.Atoi(ncRequest.Version); err != nil { + return ncRequest, fmt.Errorf("Invalid ncRequest.Version is %s in CRD, err:%s", ncRequest.Version, err) + } for _, ipAssignment = range nc.IPAssignments { if ip = net.ParseIP(ipAssignment.IP); ip == nil { diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 8af223824a..db5a65214d 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -56,6 +56,7 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { // NC version set as 0 which is the default initial value. ncVersion := 0 secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) + receivedSecondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) ncID := "testNc1" // Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0} @@ -64,7 +65,16 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIPConfig req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) - validateSecondaryIPsNCVersion(t, req) + containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] + // Validate secondary IPs' NC version has been updated by NC request + receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + for _, secIPConfig := range receivedSecondaryIPConfigs { + // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 + if secIPConfig.IPAddress != "10.0.0.16" && secIPConfig.NCVersion != 0 { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) + } + } // now Validate Update, simulate the CRD status where have 2 IP addresses as "10.0.0.16" and "10.0.0.17" with NC version 1 // The secondaryIPConfigs build from CRD will be {[IPAddress:"10.0.0.16", NCVersion: 1,]; [IPAddress:"10.0.0.17", NCVersion: 1,]} @@ -80,7 +90,15 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { ipId = uuid.New() secondaryIPConfigs[ipId.String()] = secIPConfig req = createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) - validateSecondaryIPsNCVersion(t, req) + + for _, secIPConfig := range receivedSecondaryIPConfigs { + // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 + if (secIPConfig.IPAddress == "10.0.0.16" && secIPConfig.NCVersion != 0) || + (secIPConfig.IPAddress == "10.0.0.17" && secIPConfig.NCVersion != 1) { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) + } + } } func TestReconcileNCWithEmptyState(t *testing.T) { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 2865770ba8..2b635680e6 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -253,7 +253,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(newIPCNSStatus, ncI for ipId, ipconfig := range ipconfigs { // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new NC version // Set it back to previous NC version if IP already exist. - if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig && existingIPConfig.IPAddress == ipconfig.IPAddress { + if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig { ipconfig.NCVersion = existingIPConfig.NCVersion ipconfigs[ipId] = ipconfig } From 108ea56544a14b457e32478c46705cae7bcb83a8 Mon Sep 17 00:00:00 2001 From: Shufang Date: Tue, 27 Oct 2020 23:56:57 -0700 Subject: [PATCH 08/11] Remove unnecessary unit test validation. --- cns/restserver/internalapi_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index db5a65214d..4df3049236 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -424,20 +424,6 @@ func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.Seconda return req } -func validateSecondaryIPsNCVersion(t *testing.T, req cns.CreateNetworkContainerRequest) { - containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] - // Validate secondary IPs' NC version has been updated by NC request - ncVersion, _ := strconv.Atoi(containerStatus.VMVersion) - for _, secIPConfig := range containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs { - // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 - if (secIPConfig.IPAddress == "10.0.0.16" && secIPConfig.NCVersion != 0) || - (secIPConfig.IPAddress == "10.0.0.17" && secIPConfig.NCVersion != 1) { - t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", - ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) - } - } -} - func restartService() { fmt.Println("Restart Service") From e4705be814857eab3808eadd94ae81af5621988b Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 28 Oct 2020 00:12:00 -0700 Subject: [PATCH 09/11] Add nc version to TestInitRequestController. --- .../kubecontroller/crdrequestcontroller_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go index 0a0dcb3de2..1d451320ca 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go @@ -592,6 +592,7 @@ func TestInitRequestController(t *testing.T) { }, }, SubnetAddressSpace: subnetRange, + Version: "1", }, }, }, From 5567aa533267ef4af98fdb26d2ce2c7d0649d7d8 Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 28 Oct 2020 08:50:13 -0700 Subject: [PATCH 10/11] Add more comments in test. --- cns/restserver/internalapi_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 4df3049236..14e4f440dd 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -70,7 +70,7 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs for _, secIPConfig := range receivedSecondaryIPConfigs { // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 - if secIPConfig.IPAddress != "10.0.0.16" && secIPConfig.NCVersion != 0 { + if secIPConfig.IPAddress != "10.0.0.16" || secIPConfig.NCVersion != 0 { t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) } @@ -90,7 +90,8 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { ipId = uuid.New() secondaryIPConfigs[ipId.String()] = secIPConfig req = createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) - + // Validate secondary IPs' NC version has been updated by NC request + receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs for _, secIPConfig := range receivedSecondaryIPConfigs { // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 if (secIPConfig.IPAddress == "10.0.0.16" && secIPConfig.NCVersion != 0) || From 72b1e4b420a2562a6f713cd884b95f782ce84859 Mon Sep 17 00:00:00 2001 From: Shufang Date: Wed, 28 Oct 2020 10:47:54 -0700 Subject: [PATCH 11/11] Update TestCreateAndUpdateNCWithSecondaryIPNCVersion. --- cns/restserver/internalapi_test.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 14e4f440dd..92df0fea6a 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -50,13 +50,11 @@ func TestCreateOrUpdateNCWithLargerVersionComparedToNMAgent(t *testing.T) { func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { restartService() - setEnv(t) setOrchestratorTypeInternal(cns.KubernetesCRD) // NC version set as 0 which is the default initial value. ncVersion := 0 secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) - receivedSecondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) ncID := "testNc1" // Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0} @@ -67,9 +65,11 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) containerStatus := svc.state.ContainerStatus[req.NetworkContainerid] // Validate secondary IPs' NC version has been updated by NC request - receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + receivedSecondaryIPConfigs := containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + if len(receivedSecondaryIPConfigs) != 1 { + t.Fatalf("receivedSecondaryIPConfigs lenth must be 1, but recieved %d", len(receivedSecondaryIPConfigs)) + } for _, secIPConfig := range receivedSecondaryIPConfigs { - // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 if secIPConfig.IPAddress != "10.0.0.16" || secIPConfig.NCVersion != 0 { t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) @@ -91,12 +91,26 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) { secondaryIPConfigs[ipId.String()] = secIPConfig req = createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion)) // Validate secondary IPs' NC version has been updated by NC request + containerStatus = svc.state.ContainerStatus[req.NetworkContainerid] receivedSecondaryIPConfigs = containerStatus.CreateNetworkContainerRequest.SecondaryIPConfigs + if len(receivedSecondaryIPConfigs) != 2 { + t.Fatalf("receivedSecondaryIPConfigs must be 2, but received %d", len(receivedSecondaryIPConfigs)) + } for _, secIPConfig := range receivedSecondaryIPConfigs { - // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 - if (secIPConfig.IPAddress == "10.0.0.16" && secIPConfig.NCVersion != 0) || - (secIPConfig.IPAddress == "10.0.0.17" && secIPConfig.NCVersion != 1) { - t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", + switch secIPConfig.IPAddress { + case "10.0.0.16": + // Though "10.0.0.16" IP exists in NC version 1, secodanry IP still keep its original NC version 0 + if secIPConfig.NCVersion != 0 { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 0", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) + } + case "10.0.0.17": + if secIPConfig.NCVersion != 1 { + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d, expected nc version is 1", + ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) + } + default: + t.Fatalf("nc request version is %d, secondary ip %s nc version is %d should not exist in receivedSecondaryIPConfigs map", ncVersion, secIPConfig.IPAddress, secIPConfig.NCVersion) } }