diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 093dbb1f90..b2185096c2 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -127,7 +127,7 @@ type IPConfiguration struct { // SecondaryIPConfig contains IP info of SecondaryIP type SecondaryIPConfig struct { - IPSubnet IPSubnet + IPAddress string } // IPSubnet contains ip subnet. diff --git a/cns/cnsclient/cnsclient_test.go b/cns/cnsclient/cnsclient_test.go index 2b7747f1af..4d7607b490 100644 --- a/cns/cnsclient/cnsclient_test.go +++ b/cns/cnsclient/cnsclient_test.go @@ -46,10 +46,7 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { for _, secIpAddress := range secondaryIps { secIpConfig := cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: secIpAddress, - PrefixLength: 32, - }, + IPAddress: secIpAddress, } ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go index 0b435079b8..6a98def3e6 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller_test.go @@ -366,17 +366,11 @@ func TestUpdateSpecOnNonExistingNodeNetConfig(t *testing.T) { logger.InitLogger("Azure CNS RequestController", 0, 0, "") ipConfig1 := cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.0.1", - PrefixLength: 32, - }, + IPAddress: "10.0.0.1", } ipConfig2 := cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.0.2", - PrefixLength: 32, - }, + IPAddress: "10.0.0.2", } secondaryIPConfigs := map[string]cns.SecondaryIPConfig{ @@ -421,17 +415,11 @@ func TestUpdateSpecOnExistingNodeNetConfig(t *testing.T) { logger.InitLogger("Azure CNS RequestController", 0, 0, "") ipConfig1 := cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.0.1", - PrefixLength: 32, - }, + IPAddress: "10.0.0.1", } ipConfig2 := cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.0.2", - PrefixLength: 32, - }, + IPAddress: "10.0.0.2", } secondaryIPConfigs := map[string]cns.SecondaryIPConfig{ diff --git a/cns/requestcontroller/kubecontroller/crdtranslator.go b/cns/requestcontroller/kubecontroller/crdtranslator.go index ba969cf3f3..e3ff1ca526 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator.go @@ -37,7 +37,8 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.NetworkContainerid = nc.ID ncRequest.NetworkContainerType = cns.Docker - // Convert "10.0.0.1/32" into "10.0.0.1" and 32 + // Convert "10.0.0.1/32" into "10.0.0.1" and prefix length + // Todo, this will be changed soon and only ipaddress will be passed if ip, ipNet, err = net.ParseCIDR(nc.PrimaryIP); err != nil { return ncRequest, err } @@ -49,16 +50,12 @@ func CRDStatusToNCRequest(crdStatus nnc.NodeNetworkConfigStatus) (cns.CreateNetw ncRequest.IPConfiguration.GatewayIPAddress = nc.DefaultGateway for _, ipAssignment = range nc.IPAssignments { - if ip, ipNet, err = net.ParseCIDR(ipAssignment.IP); err != nil { + if ip, _, err = net.ParseCIDR(ipAssignment.IP); err != nil { return ncRequest, err } - _, bits = ipNet.Mask.Size() - - ipSubnet.IPAddress = ip.String() - ipSubnet.PrefixLength = uint8(bits) secondaryIPConfig = cns.SecondaryIPConfig{ - IPSubnet: ipSubnet, + IPAddress: ip.String(), } ncRequest.SecondaryIPConfigs[ipAssignment.Name] = secondaryIPConfig } diff --git a/cns/requestcontroller/kubecontroller/crdtranslator_test.go b/cns/requestcontroller/kubecontroller/crdtranslator_test.go index 6e16fc2503..97277f833e 100644 --- a/cns/requestcontroller/kubecontroller/crdtranslator_test.go +++ b/cns/requestcontroller/kubecontroller/crdtranslator_test.go @@ -195,12 +195,8 @@ func TestStatusToNCRequestSuccess(t *testing.T) { t.Fatalf("Expected there to be a secondary ip with the key %v but found nothing", allocatedUUID) } - if secondaryIP.IPSubnet.IPAddress != ipCIDRString { - t.Fatalf("Expected %v as the secondary IP config but got %v", ipCIDRString, secondaryIP.IPSubnet.IPAddress) - } - - if secondaryIP.IPSubnet.PrefixLength != ipCIDRMaskLength { - t.Fatalf("Expected %v as the prefix length for the secondary IP config but got %v", ipCIDRMaskLength, secondaryIP.IPSubnet.PrefixLength) + if secondaryIP.IPAddress != ipCIDRString { + t.Fatalf("Expected %v as the secondary IP config but got %v", ipCIDRString, secondaryIP.IPAddress) } } @@ -233,10 +229,7 @@ func TestSecondaryIPsToCRDSpecSuccess(t *testing.T) { secondaryIPs = map[string]cns.SecondaryIPConfig{ allocatedUUID: { - IPSubnet: cns.IPSubnet{ - IPAddress: ipCIDRString, - PrefixLength: ipCIDRMaskLength, - }, + IPAddress: ipCIDRString, }, } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index fc20ce940a..de9a689a57 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -167,12 +167,12 @@ func (service *HTTPRestService) ReconcileNCState(ncRequest *cns.CreateNetworkCon // now parse the secondaryIP list, if it exists in PodInfo list, then allocate that ip for _, secIpConfig := range ncRequest.SecondaryIPConfigs { - if podInfo, exists := podInfoByIp[secIpConfig.IPSubnet.IPAddress]; exists { + if podInfo, exists := podInfoByIp[secIpConfig.IPAddress]; exists { log.Logf("SecondaryIP %+v is allocated to Pod. %+v, ncId: %s", secIpConfig, podInfo, ncRequest.NetworkContainerid) desiredIPConfig := cns.IPSubnet{ - IPAddress: secIpConfig.IPSubnet.IPAddress, - PrefixLength: secIpConfig.IPSubnet.PrefixLength, + IPAddress: secIpConfig.IPAddress, + PrefixLength: 32, //todo: remove PrefixLenght in } kubernetesPodInfo := cns.KubernetesPodInfo{ @@ -220,11 +220,10 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req cns.C } // Validate SecondaryIPConfig - for ipId, secIpconfig := range req.SecondaryIPConfigs { + for _, secIpconfig := range req.SecondaryIPConfigs { // Validate Ipconfig - err := validateIPSubnet(secIpconfig.IPSubnet) - if err != nil { - logger.Errorf("[Azure CNS] Error. SecondaryIpConfig, Id:%s is invalid, SecondaryIPConfig: %v, ncId: %s", ipId, secIpconfig, req.NetworkContainerid) + if secIpconfig.IPAddress == "" { + logger.Errorf("Failed to add IPConfig to state: %+v, empty IPSubnet.IPAddress", secIpconfig) return InvalidSecondaryIPConfig } } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 4114a46851..0ab0eb0aad 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -58,7 +58,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, 32) + secIpConfig := newSecondaryIPConfig(ipaddress) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -95,7 +95,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, 32) + secIpConfig := newSecondaryIPConfig(ipaddress) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -136,7 +136,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { var startingIndex = 6 for i := 0; i < secondaryIpCount; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress, 32) + secIpConfig := newSecondaryIPConfig(ipaddress) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -148,7 +148,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int) { fmt.Println("Validate Scaleup") for i := 0; i < secondaryIpCount; i++ { ipaddress := "10.0.0." + strconv.Itoa(startingIndex) - secIpConfig := newSecondaryIPConfig(ipaddress, 32) + secIpConfig := newSecondaryIPConfig(ipaddress) ipId := uuid.New() secondaryIPConfigs[ipId.String()] = secIpConfig startingIndex++ @@ -216,8 +216,8 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) if secondaryIpConfig, ok := req.SecondaryIPConfigs[ipid]; !ok { t.Fatalf("PodIpConfigState has stale ipId: %s, config: %+v", ipid, ipStatus) } else { - if ipStatus.IPSubnet != secondaryIpConfig.IPSubnet { - t.Fatalf("IPId: %s IPSubnet doesnt match: expected %+v, actual: %+v", ipid, secondaryIpConfig.IPSubnet, ipStatus.IPSubnet) + if ipStatus.IPAddress != secondaryIpConfig.IPAddress { + t.Fatalf("IPId: %s IPSubnet doesnt match: expected %+v, actual: %+v", ipid, secondaryIpConfig.IPAddress, ipStatus.IPAddress) } // Validate IP state @@ -239,12 +239,12 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest) t.Fatalf("IPId: %s State is not Available, ipStatus: %+v", ipid, ipStatus) } - alreadyValidated[ipid] = ipStatus.IPSubnet.IPAddress + alreadyValidated[ipid] = ipStatus.IPAddress } } else { // if ipaddress is not same, then fail - if ipaddress != ipStatus.IPSubnet.IPAddress { - t.Fatalf("Added the same IP guid :%s with different ipaddress, expected:%s, actual %s", ipid, ipStatus.IPSubnet.IPAddress, ipaddress) + if ipaddress != ipStatus.IPAddress { + t.Fatalf("Added the same IP guid :%s with different ipaddress, expected:%s, actual %s", ipid, ipStatus.IPAddress, ipaddress) } } } @@ -298,7 +298,7 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon } // Validate if IPAddress matches - if ipConfigstate.IPSubnet.IPAddress != ipaddress { + if ipConfigstate.IPAddress != ipaddress { t.Fatalf("IpAddress %s is not same, for Pod: %+v, actual ipState: %+v", ipaddress, podInfo, ipConfigstate) } @@ -319,7 +319,7 @@ func validateNCStateAfterReconcile(t *testing.T, ncRequest *cns.CreateNetworkCon // validate rest of Secondary IPs in Available state if ncRequest != nil { for secIpId, secIpConfig := range ncRequest.SecondaryIPConfigs { - if _, exists := expectedAllocatedPods[secIpConfig.IPSubnet.IPAddress]; exists { + if _, exists := expectedAllocatedPods[secIpConfig.IPAddress]; exists { continue } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 4ae8eebb45..91839a12a6 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -169,8 +169,7 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInf ipID := service.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] if ipID != "" { if ipState, isExist := service.PodIPConfigState[ipID]; isExist { - ipConfiguration.IPSubnet = ipState.IPSubnet - err := service.populateIpConfigInfoFromNCUntransacted(ipState.NCID, &ipConfiguration) + err := service.populateIpConfigInfoUntransacted(ipState, &ipConfiguration) return ipConfiguration, isExist, err } @@ -188,7 +187,7 @@ func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPo found := false for _, ipState := range service.PodIPConfigState { - if ipState.IPSubnet.IPAddress == desiredIPAddress { + if ipState.IPAddress == desiredIPAddress { if ipState.State == cns.Allocated { // This IP has already been allocated, if it is allocated to same pod, then return the same // IPconfiguration @@ -207,13 +206,8 @@ func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPo } if found { - err := service.populateIpConfigInfoFromNCUntransacted(ipState.NCID, &ipConfiguration) - if err != nil { - return ipConfiguration, err - } - - ipConfiguration.IPSubnet = ipState.IPSubnet - return ipConfiguration, nil + err := service.populateIpConfigInfoUntransacted(ipState, &ipConfiguration) + return ipConfiguration, err } } } @@ -228,10 +222,10 @@ func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.Kuberne for _, ipState := range service.PodIPConfigState { if ipState.State == cns.Available { - service.setIPConfigAsAllocated(ipState, podInfo, orchestratorContext) - - ipConfiguration.IPSubnet = ipState.IPSubnet - err := service.populateIpConfigInfoFromNCUntransacted(ipState.NCID, &ipConfiguration) + err := service.populateIpConfigInfoUntransacted(ipState, &ipConfiguration) + if err == nil { + service.setIPConfigAsAllocated(ipState, podInfo, orchestratorContext) + } return ipConfiguration, err } } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index a88792dfbd..c14470950e 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -47,23 +47,20 @@ func getTestService() *HTTPRestService { return svc } -func newSecondaryIPConfig(ipAddress string, prefixLength uint8) cns.SecondaryIPConfig { +func newSecondaryIPConfig(ipAddress string) cns.SecondaryIPConfig { return cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: ipAddress, - PrefixLength: prefixLength, - }, + IPAddress: ipAddress, } } func NewPodState(ipaddress string, prefixLength uint8, id, ncid, state string) ipConfigurationStatus { - ipconfig := newSecondaryIPConfig(ipaddress, prefixLength) + ipconfig := newSecondaryIPConfig(ipaddress) return ipConfigurationStatus{ - IPSubnet: ipconfig.IPSubnet, - ID: id, - NCID: ncid, - State: state, + IPAddress: ipconfig.IPAddress, + ID: id, + NCID: ncid, + State: state, } } @@ -88,6 +85,7 @@ func requestIpAddressAndGetState(t *testing.T, req cns.GetIPConfigRequest) (ipCo if reflect.DeepEqual(ipConfig.GatewayIPAddress, gatewayIp) != true { t.Fatalf("Gateway is not added as expected ipConfig %+v, expected GatewayIp: %+v", ipConfig, gatewayIp) } + // retrieve podinfo from orchestrator context if err := json.Unmarshal(req.OrchestratorContext, &podInfo); err != nil { return ipState, err @@ -100,10 +98,10 @@ func requestIpAddressAndGetState(t *testing.T, req cns.GetIPConfigRequest) (ipCo } func NewPodStateWithOrchestratorContext(ipaddress string, prefixLength uint8, id, ncid, state string, orchestratorContext cns.KubernetesPodInfo) (ipConfigurationStatus, error) { - ipconfig := newSecondaryIPConfig(ipaddress, prefixLength) + ipconfig := newSecondaryIPConfig(ipaddress) b, err := json.Marshal(orchestratorContext) return ipConfigurationStatus{ - IPSubnet: ipconfig.IPSubnet, + IPAddress: ipconfig.IPAddress, ID: id, NCID: ncid, State: state, @@ -117,10 +115,7 @@ func UpdatePodIpConfigState(t *testing.T, svc *HTTPRestService, ipconfigs map[st secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) for _, ipconfig := range ipconfigs { secIpConfig := cns.SecondaryIPConfig{ - IPSubnet: cns.IPSubnet{ - IPAddress: ipconfig.IPSubnet.IPAddress, - PrefixLength: ipconfig.IPSubnet.PrefixLength, - }, + IPAddress: ipconfig.IPAddress, } ipId := ipconfig.ID @@ -406,7 +401,7 @@ func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { desiredState, _ := NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Allocated, testPod1Info) // want first available Pod IP State - desiredState.IPSubnet = desiredIPConfig + desiredState.IPAddress = desiredIPConfig.IPAddress desiredState.OrchestratorContext = b if reflect.DeepEqual(desiredState, actualstate) != true { @@ -489,7 +484,10 @@ func TestAvailableIPConfigs(t *testing.T) { req := cns.GetIPConfigRequest{} b, _ := json.Marshal(testPod1Info) req.OrchestratorContext = b - req.DesiredIPConfig = state1.IPSubnet + req.DesiredIPConfig = cns.IPSubnet{ + IPAddress: state1.IPAddress, + PrefixLength: 32, + } _, err := requestIpAddressAndGetState(t, req) if err != nil { diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index b1070ed188..062750e902 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -59,7 +59,7 @@ type allocatedIPCount struct { type ipConfigurationStatus struct { NCID string ID string //uuid - IPSubnet cns.IPSubnet + IPAddress string State string OrchestratorContext json.RawMessage } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 836cab42dc..706c68652a 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -241,7 +241,7 @@ func (service *HTTPRestService) updateIpConfigsStateUntransacted(req cns.CreateN return 0, "" } -// addIPConfigStateUntransacted adds the IPConfis to the PodIpConfigState map with Available state +// 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(ncId string, ipconfigs map[string]cns.SecondaryIPConfig) { @@ -256,7 +256,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, ipconf ipconfigStatus := ipConfigurationStatus{ NCID: ncId, ID: ipId, - IPSubnet: ipconfig.IPSubnet, + IPAddress: ipconfig.IPAddress, State: cns.Available, OrchestratorContext: nil, } @@ -640,19 +640,24 @@ func (service *HTTPRestService) validateIpConfigRequest(ipConfigRequest cns.GetI return Success, "" } -func (service *HTTPRestService) populateIpConfigInfoFromNCUntransacted(ncId string, ipConfiguration *cns.IPConfiguration) error { +func (service *HTTPRestService) populateIpConfigInfoUntransacted(ipConfigStatus ipConfigurationStatus, ipConfiguration *cns.IPConfiguration) error { var ( ncStatus containerstatus exists bool ) - if ncStatus, exists = service.state.ContainerStatus[ncId]; !exists { - return fmt.Errorf("Failed to get NC Configuration for NcId: %s", ncId) + if ncStatus, exists = service.state.ContainerStatus[ipConfigStatus.NCID]; !exists { + return fmt.Errorf("Failed to get NC Configuration for NcId: %s", ipConfigStatus.NCID) } ipConfiguration.DNSServers = ncStatus.CreateNetworkContainerRequest.IPConfiguration.DNSServers ipConfiguration.GatewayIPAddress = ncStatus.CreateNetworkContainerRequest.IPConfiguration.GatewayIPAddress + // TODO: This will be changed soon, and prefix length will be set as Subnetprefix length + ipConfiguration.IPSubnet = cns.IPSubnet{ + IPAddress: ipConfigStatus.IPAddress, + PrefixLength: 32, + } return nil }