diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index af1f9e7057..e89da0bf7e 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -20,6 +20,7 @@ import ( var ( ErrStoreEmpty = errors.New("empty endpoint state store") ErrParsePodIPFailed = errors.New("failed to parse pod's ip") + ErrNoNCs = errors.New("No NCs found in the CNS internal state") ) // requestIPConfigHandlerHelper validates the request, assigns IPs, and returns a response @@ -88,13 +89,13 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r return } - // This method can only return 1 IP. If we have more than one NC then we expect to need to return one IP per NC - if len(service.state.ContainerStatus) > 1 { + // This method can only return EXACTLY 1 IP. If we have more than one NC then we expect to need to return one IP per NC + if len(service.state.ContainerStatus) != 1 { // we send a response back saying that this API won't be able to return the amount of IPs needed to fulfill the request reserveResp := &cns.IPConfigResponse{ Response: cns.Response{ ReturnCode: types.InvalidRequest, - Message: fmt.Sprintf("Called API that can only return 1 IP when expecting %d", len(service.state.ContainerStatus)), + Message: fmt.Sprintf("Expected 1 NC when calling this API but found %d NCs", len(service.state.ContainerStatus)), }, } w.Header().Set(cnsReturnCode, reserveResp.Response.ReturnCode.String()) @@ -128,6 +129,21 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r return } + // Checks to make sure we return exactly 1 IP + // If IPAM assigned more than 1 IP then we need to raise an error since this API can only return one IP and IPAM may have assigned more than one + if len(ipConfigsResp.PodIPInfo) != 1 { + // we send a response back saying that this API won't be able to return the amount of IPs needed to fulfill the request + reserveResp := &cns.IPConfigResponse{ + Response: cns.Response{ + ReturnCode: types.UnexpectedError, + Message: fmt.Sprintf("request returned incorrect number of IPs. Expected 1 and returned %d", len(ipConfigsResp.PodIPInfo)), + }, + } + w.Header().Set(cnsReturnCode, reserveResp.Response.ReturnCode.String()) + err = service.Listener.Encode(w, &reserveResp) + logger.ResponseEx(service.Name+operationName, ipconfigRequest, reserveResp, reserveResp.Response.ReturnCode, err) + return + } // As this API is expected to return IPConfigResponse, generate it from the IPConfigsResponse returned above. reserveResp := &cns.IPConfigResponse{ Response: ipConfigsResp.Response, @@ -273,12 +289,12 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r return } - // check to make sure there aren't multiple NCs - if len(service.state.ContainerStatus) > 1 { + // check to make sure there is only one NC + if len(service.state.ContainerStatus) != 1 { reserveResp := &cns.IPConfigResponse{ Response: cns.Response{ ReturnCode: types.InvalidRequest, - Message: fmt.Sprintf("Called API that can only return 1 IP when expecting %d", len(service.state.ContainerStatus)), + Message: fmt.Sprintf("Expected 1 NC when calling this API but found %d NCs", len(service.state.ContainerStatus)), }, } w.Header().Set(cnsReturnCode, reserveResp.Response.ReturnCode.String()) @@ -661,7 +677,15 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.PodInfo) ([]cns. // Assigns a pod with all IPs desired func (service *HTTPRestService) AssignDesiredIPConfigs(podInfo cns.PodInfo, desiredIPAddresses []string) ([]cns.PodIpInfo, error) { + // Gets the number of NCs which will determine the number of IPs given to a pod + numOfNCs := len(service.state.ContainerStatus) + // checks to make sure we have NCs before trying to get IPs + if numOfNCs == 0 { + return nil, ErrNoNCs + } + // Sets the number of desired IPs equal to the number of desired IPs passed in numDesiredIPAddresses := len(desiredIPAddresses) + // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs podIPInfo := make([]cns.PodIpInfo, numDesiredIPAddresses) // creating a map for the loop to check to see if the IP in the pool is one of the desired IPs desiredIPMap := make(map[string]struct{}) @@ -756,12 +780,16 @@ func (service *HTTPRestService) AssignDesiredIPConfigs(podInfo cns.PodInfo, desi // Assigns an available IP from each NC on the NNC. If there is one NC then we expect to only have one IP return // In the case of dualstack we would expect to have one IPv6 from one NC and one IPv4 from a second NC func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([]cns.PodIpInfo, error) { + // Gets the number of NCs which will determine the number of IPs given to a pod + numOfNCs := len(service.state.ContainerStatus) + // if there are no NCs on the NNC there will be no IPs in the pool so return error + if numOfNCs == 0 { + return nil, ErrNoNCs + } service.Lock() defer service.Unlock() - // Sets the number of IPs needed equal to the number of NCs so that we can get one IP per NC - numIPsNeeded := len(service.state.ContainerStatus) // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs - podIPInfo := make([]cns.PodIpInfo, numIPsNeeded) + podIPInfo := make([]cns.PodIpInfo, numOfNCs) // This map is used to store whether or not we have found an available IP from an NC when looping through the pool ipsToAssign := make(map[string]cns.IPConfigurationStatus) @@ -777,13 +805,13 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ } ipsToAssign[ipState.NCID] = ipState // Once one IP per container is found break out of the loop and stop searching - if len(ipsToAssign) == numIPsNeeded { + if len(ipsToAssign) == numOfNCs { break } } // Checks to make sure we found one IP for each NC - if len(ipsToAssign) != numIPsNeeded { + if len(ipsToAssign) != numOfNCs { //nolint:goerr113 // return error return podIPInfo, fmt.Errorf("not enough IPs available, waiting on Azure CNS to allocate more") } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index d35b7d3b56..b552b15774 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -1360,6 +1360,40 @@ func IPAMMarkExistingIPConfigAsPending(t *testing.T, ncStates []ncState) { } } +func TestIPAMFailToRequestIPsWithNoNCsSpecificIP(t *testing.T) { + svc := getTestService() + req := cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + req.DesiredIPAddresses = make([]string, 1) + req.DesiredIPAddresses[0] = testIP1 + + _, err := requestIPConfigsHelper(svc, req) + if err == nil { + t.Fatalf("Expected error. Should not be able to request IPs when there are no NCs") + } + assert.ErrorIs(t, err, ErrNoNCs) +} + +func TestIPAMFailToRequestIPsWithNoNCsAnyIP(t *testing.T) { + svc := getTestService() + req := cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + + _, err := requestIPConfigsHelper(svc, req) + if err == nil { + t.Fatalf("Expected error. Should not be able to request IPs when there are no NCs") + } + assert.ErrorIs(t, err, ErrNoNCs) +} + func TestIPAMReleaseOneIPWhenExpectedToHaveTwo(t *testing.T) { svc := getTestService()