From e074bb37fd1c5aef6b0aa9ff9b15e2b4747afef7 Mon Sep 17 00:00:00 2001 From: ryandenney Date: Wed, 9 Oct 2024 11:56:31 -0400 Subject: [PATCH 01/10] initial TODOs for CNS --- cns/kubecontroller/nodenetworkconfig/reconciler.go | 1 + cns/middlewares/k8sSwiftV2.go | 5 +++++ cns/restserver/ipam.go | 5 +++++ cns/restserver/restserver.go | 2 ++ cns/service/main.go | 2 ++ 5 files changed, 15 insertions(+) diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler.go b/cns/kubecontroller/nodenetworkconfig/reconciler.go index 10b252b48a..81e28401fd 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler.go @@ -124,6 +124,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco ipAssignments += len(req.SecondaryIPConfigs) } + // TODO: Here we can assign a IPFamilies along with the allocated IPs // record assigned IPs metric allocatedIPs.Set(float64(ipAssignments)) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index a11290c205..3cfa9a5518 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -38,6 +38,7 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request // and release IP configs handlers. +// this will need to change to also work for a delegated NIC with multiple IPs func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { podInfo, respCode, message := k.validateIPConfigsRequest(ctx, &req) @@ -52,9 +53,11 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } ipConfigsResp, err := defaultHandler(ctx, req) // If the pod is not v2, return the response from the handler + // we need to add the secondary Interface. Our current POC cluster is returning here if !req.SecondaryInterfacesExist { return ipConfigsResp, err } + // TODO: the pod itself won't be "V2" as we aren't using multitenancy pods // If the pod is v2, get the infra IP configs from the handler first and then add the SWIFTv2 IP config defer func() { // Release the default IP config if there is an error @@ -100,6 +103,8 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } } +// TODO: we will not be using multitenant pods. Need to look into what labels we are currently seeing and maybe compare to Vanilla swiftv2 +// For our purposes we would skip over this logic or need to replace it with something to check the delegated NIC // validateIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. // nolint func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string) { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 883d9aaef3..30d364d16e 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -282,6 +282,7 @@ func (service *HTTPRestService) RequestIPConfigsHandler(w http.ResponseWriter, r } var ipConfigsResp *cns.IPConfigsResponse + // TODO: Decide what middleware option we need to singetenancy swift v2 // Check if IPConfigsHandlerMiddleware is set if service.IPConfigsHandlerMiddleware != nil { // Wrap the default datapath handlers with the middleware depending on middleware type @@ -990,8 +991,11 @@ func (service *HTTPRestService) AssignDesiredIPConfigs(podInfo cns.PodInfo, desi // 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 + // TODO: If we go with the One NC route then this logic would have to be updated + // Instead of using numOfNCs we would have a variable added to the HTTPRestService struct that determines IPFamilies and use the length of that numOfNCs := len(service.state.ContainerStatus) // if there are no NCs on the NNC there will be no IPs in the pool so return error + // We could still keep this check if numOfNCs == 0 { return nil, ErrNoNCs } @@ -1005,6 +1009,7 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // Searches for available IPs in the pool for _, ipState := range service.PodIPConfigState { // check if an IP from this NC is already set side for assignment. + // TODO: Instead of using ncAlreadyMarkedForAssignment it would be IPFamilyAlreadyMarkedForAssignment if _, ncAlreadyMarkedForAssignment := ipsToAssign[ipState.NCID]; ncAlreadyMarkedForAssignment { continue } diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 30a2b1e8d6..d43664d0ea 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -55,6 +55,8 @@ type imdsClient interface { } // HTTPRestService represents http listener for CNS - Container Networking Service. +// TODO: Add a new value for IPFamily +// If we add a new type of Middleware that will be reflected in the IPConfigsHandlerMiddleware value type HTTPRestService struct { *cns.Service dockerClient *dockerclient.Client diff --git a/cns/service/main.go b/cns/service/main.go index 53502b29fd..84643e1a94 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1404,6 +1404,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn // Start building the NNC Reconciler + // TODO: We need to return the IPFamilies from the reconciler so that they can be added to the Service struct // get CNS Node IP to compare NC Node IP with this Node IP to ensure NCs were created for this node nodeIP := configuration.NodeIP() nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP) @@ -1434,6 +1435,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } } + // TODO: If we need special middleware this is where we would be setting it if cnsconfig.EnableSwiftV2 { if err := mtpncctrl.SetupWithManager(manager); err != nil { return errors.Wrapf(err, "failed to setup mtpnc reconciler with manager") From ad0a7f4cb69cd2b72feb3ddd0d42f437acea920c Mon Sep 17 00:00:00 2001 From: ryandenney Date: Thu, 10 Oct 2024 11:58:06 -0400 Subject: [PATCH 02/10] more TODOs --- cns/kubecontroller/nodenetworkconfig/conversion_linux.go | 6 ++++++ cns/middlewares/k8sSwiftV2.go | 2 +- cns/middlewares/k8sSwiftV2_linux.go | 4 ++++ cns/middlewares/k8sSwiftV2_windows.go | 2 ++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index c89d41646b..8ffa3a9e98 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -18,17 +18,23 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. + // TODO: If we decide to look to not change the contract to add an IPFamilies flag and just detect it we would have to do it here + // By doing this we can also have it work for Dualstack Overlay + // To do it we would need to pass in a pointer to an IPFamilies slice to this function and add the family of the IPs added in + // Here we check the primary CIDR which is needed for overlay and VNET Block for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ IPAddress: addr.String(), NCVersion: int(nc.Version), } + } // Add IPs from CIDR block to the secondary IPConfigs if nc.Type == v1alpha.VNETBlock { for _, ipAssignment := range nc.IPAssignments { + // Here we would need to check all other assigned CIDR Blocks that aren't the primary. cidrPrefix, err := netip.ParsePrefix(ipAssignment.IP) if err != nil { return nil, errors.Wrapf(err, "invalid CIDR block: %s", ipAssignment.IP) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 3cfa9a5518..65f4fa0c82 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -38,7 +38,7 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request // and release IP configs handlers. -// this will need to change to also work for a delegated NIC with multiple IPs +// TODO: this will need to changed or add a new function to also work for a delegated NIC with multiple pods func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { podInfo, respCode, message := k.validateIPConfigsRequest(ctx, &req) diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index e9a93de0e2..8664657c4c 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -18,6 +18,9 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { var routes []cns.Route switch podIPInfo.NICType { + // TODO: We may want to create a new type of NIC. Currently we are using delegated NICs but this set routes method only + // takes in for the multitenant scenario. We should have a new case that behaves similarly to InfraNIC but for a delegated NIC + // Q: Does the code currently see the kube pods as Infra or Delegated? They currently use the delegated pod subnet for IPs case cns.DelegatedVMNIC: virtualGWRoute := cns.Route{ IPAddress: fmt.Sprintf("%s/%d", virtualGW, prefixLength), @@ -65,6 +68,7 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { return errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) } + //This function is called per IP so we shouldn't have to worry about adding both v4 and v6 at once if ip.Is4() { routes = append(routes, addRoutes(podCIDRsV4, overlayGatewayv4)...) routes = append(routes, addRoutes(serviceCIDRsV4, overlayGatewayv4)...) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index 2be2fbd1df..d4993eb46c 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -27,6 +27,8 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { // assignSubnetPrefixLengthFields will assign the subnet-prefix length to some fields of podipinfo // this is required for the windows scenario so that HNS programming is successful for pods +// TODO: This is being used for the delegated NIC for Windows solution. Once we are testing Windows we will +// Need to confirm that this assigns the whole pod subnet that we expect for both v4 and v6 func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(podIPInfo *cns.PodIpInfo, interfaceInfo v1alpha1.InterfaceInfo, ip string) error { // Parse MTPNC SubnetAddressSpace to get the subnet prefix length subnet, subnetPrefix, err := utils.ParseIPAndPrefix(interfaceInfo.SubnetAddressSpace) From 9740047ca36d18c4d907506dbf5f9818cafd723b Mon Sep 17 00:00:00 2001 From: ryandenney Date: Wed, 16 Oct 2024 11:58:26 -0400 Subject: [PATCH 03/10] initial changes --- cns/NetworkContainerContract.go | 2 ++ .../nodenetworkconfig/conversion_linux.go | 23 +++++++++--- .../nodenetworkconfig/reconciler.go | 1 - cns/restserver/const.go | 7 ++++ cns/restserver/ipam.go | 35 +++++++++---------- cns/restserver/restserver.go | 6 ++-- cns/restserver/util.go | 2 ++ 7 files changed, 49 insertions(+), 27 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 394f871f09..b07e41a011 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -7,6 +7,7 @@ import ( "strconv" "strings" + "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" @@ -127,6 +128,7 @@ type CreateNetworkContainerRequest struct { EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code + IPFamilies map[restserver.IPFamily]struct{} } func (req *CreateNetworkContainerRequest) Validate() error { diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 8ffa3a9e98..10f453fd0f 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -1,10 +1,12 @@ package nodenetworkconfig import ( + "fmt" "net/netip" "strconv" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -15,12 +17,11 @@ import ( //nolint:gocritic //ignore hugeparam func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) { secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} + ipFamilies := map[restserver.IPFamily]struct{}{} // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. - // TODO: If we decide to look to not change the contract to add an IPFamilies flag and just detect it we would have to do it here - // By doing this we can also have it work for Dualstack Overlay - // To do it we would need to pass in a pointer to an IPFamilies slice to this function and add the family of the IPs added in + // TODO: we need to pass in a pointer to an IPFamilies slice to this function and add the family of the IPs added in // Here we check the primary CIDR which is needed for overlay and VNET Block for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ @@ -30,6 +31,12 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre } + if primaryIPPrefix.Addr().Is4() { + ipFamilies[restserver.IPv4Family] = struct{}{} + } else { + ipFamilies[restserver.IPv6Family] = struct{}{} + } + // Add IPs from CIDR block to the secondary IPConfigs if nc.Type == v1alpha.VNETBlock { @@ -48,9 +55,16 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre NCVersion: int(nc.Version), } } + if cidrPrefix.Addr().Is4() { + ipFamilies[restserver.IPv4Family] = struct{}{} + } else { + ipFamilies[restserver.IPv6Family] = struct{}{} + } } } + fmt.Printf("IPFamilies on NC %+v and %+v", nc.ID, ipFamilies) + return &cns.CreateNetworkContainerRequest{ HostPrimaryIP: nc.NodeIP, SecondaryIPConfigs: secondaryIPConfigs, @@ -61,6 +75,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, }, - NCStatus: nc.Status, + NCStatus: nc.Status, + IPFamilies: ipFamilies, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler.go b/cns/kubecontroller/nodenetworkconfig/reconciler.go index 81e28401fd..10b252b48a 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler.go @@ -124,7 +124,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco ipAssignments += len(req.SecondaryIPConfigs) } - // TODO: Here we can assign a IPFamilies along with the allocated IPs // record assigned IPs metric allocatedIPs.Set(float64(ipAssignments)) diff --git a/cns/restserver/const.go b/cns/restserver/const.go index 0782755596..eff738423a 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -13,3 +13,10 @@ const ( dncApiVersion = "?api-version=2018-03-01" nmaAPICallTimeout = 2 * time.Second ) + +type IPFamily string + +const ( + IPv4Family IPFamily = "ipv4" + IPv6Family IPFamily = "ipv6" +) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 30d364d16e..13c11e8a00 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -990,48 +990,45 @@ 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 - // TODO: If we go with the One NC route then this logic would have to be updated - // Instead of using numOfNCs we would have a variable added to the HTTPRestService struct that determines IPFamilies and use the length of that - numOfNCs := len(service.state.ContainerStatus) - // if there are no NCs on the NNC there will be no IPs in the pool so return error - // We could still keep this check - if numOfNCs == 0 { + // Gets the number of IPFamilies expected to be returned. + numOfIPFamilies := len(service.IPFamilies) + + if numOfIPFamilies == 0 { + // TODO: replace with it's own error return nil, ErrNoNCs } service.Lock() defer service.Unlock() // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs - podIPInfo := make([]cns.PodIpInfo, numOfNCs) + podIPInfo := make([]cns.PodIpInfo, numOfIPFamilies) // 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) + ipsToAssign := make(map[IPFamily]cns.IPConfigurationStatus) // Searches for available IPs in the pool for _, ipState := range service.PodIPConfigState { - // check if an IP from this NC is already set side for assignment. - // TODO: Instead of using ncAlreadyMarkedForAssignment it would be IPFamilyAlreadyMarkedForAssignment - if _, ncAlreadyMarkedForAssignment := ipsToAssign[ipState.NCID]; ncAlreadyMarkedForAssignment { + // check if the IP with the same family type exists already + if _, IPFamilyAlreadyMarkedForAssignment := ipsToAssign[net.ParseIP(ipState.IPAddress).To4() == nil]; IPFamilyAlreadyMarkedForAssignment { continue } // Checks if the current IP is available if ipState.GetState() != types.Available { continue } - ipsToAssign[ipState.NCID] = ipState + ipsToAssign[net.ParseIP(ipState.IPAddress).To4() == nil] = ipState // Once one IP per container is found break out of the loop and stop searching - if len(ipsToAssign) == numOfNCs { + if len(ipsToAssign) == numOfIPFamilies { break } } // Checks to make sure we found one IP for each NC - if len(ipsToAssign) != numOfNCs { - for ncID := range service.state.ContainerStatus { - if _, found := ipsToAssign[ncID]; found { + if len(ipsToAssign) != numOfIPFamilies { + for _, ipFamily := range service.IPFamilies { + if _, found := ipsToAssign[ipFamily]; found { continue } - return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more with NC Status: %s", - ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) + return podIPInfo, errors.Errorf("not enough IPs available of type %s, waiting on Azure CNS to allocate more with NC Status: %s", + ipFamily, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } } diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index d43664d0ea..c271b669a0 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -54,9 +54,8 @@ type imdsClient interface { GetVMUniqueID(ctx context.Context) (string, error) } -// HTTPRestService represents http listener for CNS - Container Networking Service. -// TODO: Add a new value for IPFamily -// If we add a new type of Middleware that will be reflected in the IPConfigsHandlerMiddleware value +// HTTPRestService represents http listener for CNS - Container Networking Service.// TODO: Add a new value for IPFamily +// TODO: If we add a new type of Middleware that will be reflected in the IPConfigsHandlerMiddleware value type HTTPRestService struct { *cns.Service dockerClient *dockerclient.Client @@ -81,6 +80,7 @@ type HTTPRestService struct { PnpIDByMacAddress map[string]string imdsClient imdsClient nodesubnetIPFetcher *nodesubnet.IPFetcher + IPFamilies []IPFamily } type CNIConflistGenerator interface { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 43d1e1aef9..2bd198c34f 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -183,6 +183,8 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw VfpUpdateComplete: vfpUpdateComplete, } + service.IPFamilies = req.IPFamilies + switch req.NetworkContainerType { case cns.AzureContainerInstance: fallthrough From 25980f34fcb27845b0664fac44e95ae4f18c8f36 Mon Sep 17 00:00:00 2001 From: ryandenney Date: Thu, 17 Oct 2024 11:37:15 -0400 Subject: [PATCH 04/10] fixes overlay issue --- .../nodenetworkconfig/conversion_linux.go | 7 ++- cns/restserver/ipam.go | 45 ++++++++++++++----- cns/restserver/util.go | 2 - 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 10f453fd0f..03884de73b 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -21,8 +21,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. - // TODO: we need to pass in a pointer to an IPFamilies slice to this function and add the family of the IPs added in - // Here we check the primary CIDR which is needed for overlay and VNET Block for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ IPAddress: addr.String(), @@ -30,7 +28,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre } } - + // adds the IPFamily of the primary CIDR to the set if primaryIPPrefix.Addr().Is4() { ipFamilies[restserver.IPv4Family] = struct{}{} } else { @@ -55,6 +53,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre NCVersion: int(nc.Version), } } + // adds the IPFamily of the secondary CIDR to the set if cidrPrefix.Addr().Is4() { ipFamilies[restserver.IPv4Family] = struct{}{} } else { @@ -63,7 +62,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre } } - fmt.Printf("IPFamilies on NC %+v and %+v", nc.ID, ipFamilies) + fmt.Printf("IPFamilies found on NC %+v are %+v", nc.ID, ipFamilies) return &cns.CreateNetworkContainerRequest{ HostPrimaryIP: nc.NodeIP, diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 13c11e8a00..6b5ed9bc8e 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -25,6 +25,7 @@ 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") + ErrNoIPFamilies = errors.New("No IP Families found on NCs") ErrOptManageEndpointState = errors.New("CNS is not set to manage the endpoint state") ErrEndpointStateNotFound = errors.New("endpoint state could not be found in the statefile") ErrGetAllNCResponseEmpty = errors.New("failed to get NC responses from statefile") @@ -990,13 +991,25 @@ 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 IPFamilies expected to be returned. - numOfIPFamilies := len(service.IPFamilies) + // Gets the number of IPFamilies expected to be returned across all NCs + ipFamilies := map[IPFamily]struct{}{} - if numOfIPFamilies == 0 { - // TODO: replace with it's own error + // checks to make sure we have at least one NC + if len(service.state.ContainerStatus) == 0 { return nil, ErrNoNCs } + + for ncID := range service.state.ContainerStatus { + for ipFamily := range service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.IPFamilies { + ipFamilies[ipFamily] = struct{}{} + } + } + + numOfIPFamilies := len(ipFamilies) + if numOfIPFamilies == 0 { + return nil, ErrNoIPFamilies + } + service.Lock() defer service.Unlock() // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs @@ -1006,29 +1019,37 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // Searches for available IPs in the pool for _, ipState := range service.PodIPConfigState { + // get the IPFamily of the current ipState + var ipStateFamily IPFamily + if net.ParseIP(ipState.IPAddress).To4() != nil { + ipStateFamily = IPv4Family + } else { + ipStateFamily = IPv6Family + } + // check if the IP with the same family type exists already - if _, IPFamilyAlreadyMarkedForAssignment := ipsToAssign[net.ParseIP(ipState.IPAddress).To4() == nil]; IPFamilyAlreadyMarkedForAssignment { + if _, IPFamilyAlreadyMarkedForAssignment := ipsToAssign[ipStateFamily]; IPFamilyAlreadyMarkedForAssignment { continue } // Checks if the current IP is available if ipState.GetState() != types.Available { continue } - ipsToAssign[net.ParseIP(ipState.IPAddress).To4() == nil] = ipState - // Once one IP per container is found break out of the loop and stop searching + ipsToAssign[ipStateFamily] = ipState + // Once one IP per family is found break out of the loop and stop searching if len(ipsToAssign) == numOfIPFamilies { break } } - // Checks to make sure we found one IP for each NC + // Checks to make sure we found one IP for each IPFamily if len(ipsToAssign) != numOfIPFamilies { - for _, ipFamily := range service.IPFamilies { - if _, found := ipsToAssign[ipFamily]; found { + for ncID := range service.state.ContainerStatus { + if _, found := ipsToAssign[ncID]; found { continue } - return podIPInfo, errors.Errorf("not enough IPs available of type %s, waiting on Azure CNS to allocate more with NC Status: %s", - ipFamily, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) + return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more with NC Status: %s", + ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 2bd198c34f..43d1e1aef9 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -183,8 +183,6 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw VfpUpdateComplete: vfpUpdateComplete, } - service.IPFamilies = req.IPFamilies - switch req.NetworkContainerType { case cns.AzureContainerInstance: fallthrough From f62c6b9293192c759e4517bd43e8bf0c09b65151 Mon Sep 17 00:00:00 2001 From: ryandenney Date: Thu, 17 Oct 2024 11:42:05 -0400 Subject: [PATCH 05/10] cleaning up TODOs --- cns/restserver/ipusage.go | 58 ++++++++++++++++++++++++++++++++++++ cns/restserver/restserver.go | 2 +- 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 cns/restserver/ipusage.go diff --git a/cns/restserver/ipusage.go b/cns/restserver/ipusage.go new file mode 100644 index 0000000000..e2ae30382b --- /dev/null +++ b/cns/restserver/ipusage.go @@ -0,0 +1,58 @@ +package restserver + +import ( + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/types" +) + +type ipState struct { + // allocatedIPs are all the IPs given to CNS by DNC. + allocatedIPs int64 + // assignedIPs are the IPs CNS gives to Pods. + assignedIPs int64 + // availableIPs are the IPs in state "Available". + availableIPs int64 + // programmingIPs are the IPs in state "PendingProgramming". + programmingIPs int64 + // releasingIPs are the IPs in state "PendingReleasr". + releasingIPs int64 +} + +func (service *HTTPRestService) buildIPState() *ipState { + service.Lock() + defer service.Unlock() + + state := ipState{ + allocatedIPs: 0, + assignedIPs: 0, + availableIPs: 0, + programmingIPs: 0, + releasingIPs: 0, + } + + //nolint:gocritic // This has to iterate over the IP Config state to get the counts. + for _, ipConfig := range service.PodIPConfigState { + state.allocatedIPs++ + if ipConfig.GetState() == types.Assigned { + state.assignedIPs++ + } + if ipConfig.GetState() == types.Available { + state.availableIPs++ + } + if ipConfig.GetState() == types.PendingProgramming { + state.programmingIPs++ + } + if ipConfig.GetState() == types.PendingRelease { + state.releasingIPs++ + } + } + + logger.Printf("[IP Usage] Allocated IPs: %d, Assigned IPs: %d, Available IPs: %d, PendingProgramming IPs: %d, PendingRelease IPs: %d", + state.allocatedIPs, + state.assignedIPs, + state.availableIPs, + state.programmingIPs, + state.releasingIPs, + ) + return &state +} diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index c271b669a0..f130479c79 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -54,7 +54,7 @@ type imdsClient interface { GetVMUniqueID(ctx context.Context) (string, error) } -// HTTPRestService represents http listener for CNS - Container Networking Service.// TODO: Add a new value for IPFamily +// HTTPRestService represents http listener for CNS - Container Networking Service. // TODO: If we add a new type of Middleware that will be reflected in the IPConfigsHandlerMiddleware value type HTTPRestService struct { *cns.Service From 1fa64d3660ff68dac8c6cf0570445bb7f9e6c677 Mon Sep 17 00:00:00 2001 From: ryandenney Date: Mon, 11 Nov 2024 10:11:17 -0500 Subject: [PATCH 06/10] updating error check in ipam --- cns/restserver/ipam.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 6b5ed9bc8e..a1bd17a7cd 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -991,7 +991,7 @@ 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 IPFamilies expected to be returned across all NCs + // Map used to get the number of IPFamilies across all NCs ipFamilies := map[IPFamily]struct{}{} // checks to make sure we have at least one NC @@ -1045,11 +1045,13 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // Checks to make sure we found one IP for each IPFamily if len(ipsToAssign) != numOfIPFamilies { for ncID := range service.state.ContainerStatus { - if _, found := ipsToAssign[ncID]; found { - continue + for ipFamily := range service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.IPFamilies { + if _, found := ipsToAssign[ipFamily]; found { + continue + } + return podIPInfo, errors.Errorf("not enough IPs available of type %s for %s, waiting on Azure CNS to allocate more with NC Status: %s", + ipFamily, ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } - return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more with NC Status: %s", - ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } } From 7241b097a62e97cc1f955208bc682574a938ddad Mon Sep 17 00:00:00 2001 From: ryandenney Date: Tue, 12 Nov 2024 14:58:09 -0500 Subject: [PATCH 07/10] updates to IPFamily and dependencies --- cns/NetworkContainerContract.go | 10 ++++++++-- .../nodenetworkconfig/conversion_linux.go | 11 +++++------ cns/restserver/const.go | 7 ------- cns/restserver/ipam.go | 10 +++++----- cns/restserver/ipusage.go | 13 ------------- cns/restserver/restserver.go | 2 +- 6 files changed, 19 insertions(+), 34 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index b07e41a011..37cdaa71f7 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -7,7 +7,6 @@ import ( "strconv" "strings" - "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" @@ -128,7 +127,7 @@ type CreateNetworkContainerRequest struct { EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code - IPFamilies map[restserver.IPFamily]struct{} + IPFamilies map[IPFamily]struct{} } func (req *CreateNetworkContainerRequest) Validate() error { @@ -744,3 +743,10 @@ type NodeRegisterRequest struct { NumCores int NmAgentSupportedApis []string } + +type IPFamily string + +const ( + IPv4Family IPFamily = "ipv4" + IPv6Family IPFamily = "ipv6" +) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 03884de73b..4f430df655 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -6,7 +6,6 @@ import ( "strconv" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" ) @@ -17,7 +16,7 @@ import ( //nolint:gocritic //ignore hugeparam func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) { secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} - ipFamilies := map[restserver.IPFamily]struct{}{} + ipFamilies := map[cns.IPFamily]struct{}{} // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. @@ -30,9 +29,9 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre } // adds the IPFamily of the primary CIDR to the set if primaryIPPrefix.Addr().Is4() { - ipFamilies[restserver.IPv4Family] = struct{}{} + ipFamilies[cns.IPv4Family] = struct{}{} } else { - ipFamilies[restserver.IPv6Family] = struct{}{} + ipFamilies[cns.IPv6Family] = struct{}{} } // Add IPs from CIDR block to the secondary IPConfigs @@ -55,9 +54,9 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre } // adds the IPFamily of the secondary CIDR to the set if cidrPrefix.Addr().Is4() { - ipFamilies[restserver.IPv4Family] = struct{}{} + ipFamilies[cns.IPv4Family] = struct{}{} } else { - ipFamilies[restserver.IPv6Family] = struct{}{} + ipFamilies[cns.IPv6Family] = struct{}{} } } } diff --git a/cns/restserver/const.go b/cns/restserver/const.go index eff738423a..0782755596 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -13,10 +13,3 @@ const ( dncApiVersion = "?api-version=2018-03-01" nmaAPICallTimeout = 2 * time.Second ) - -type IPFamily string - -const ( - IPv4Family IPFamily = "ipv4" - IPv6Family IPFamily = "ipv6" -) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index a1bd17a7cd..ba70279a19 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -992,7 +992,7 @@ func (service *HTTPRestService) AssignDesiredIPConfigs(podInfo cns.PodInfo, desi // 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) { // Map used to get the number of IPFamilies across all NCs - ipFamilies := map[IPFamily]struct{}{} + ipFamilies := map[cns.IPFamily]struct{}{} // checks to make sure we have at least one NC if len(service.state.ContainerStatus) == 0 { @@ -1015,16 +1015,16 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs podIPInfo := make([]cns.PodIpInfo, numOfIPFamilies) // 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[IPFamily]cns.IPConfigurationStatus) + ipsToAssign := make(map[cns.IPFamily]cns.IPConfigurationStatus) // Searches for available IPs in the pool for _, ipState := range service.PodIPConfigState { // get the IPFamily of the current ipState - var ipStateFamily IPFamily + var ipStateFamily cns.IPFamily if net.ParseIP(ipState.IPAddress).To4() != nil { - ipStateFamily = IPv4Family + ipStateFamily = cns.IPv4Family } else { - ipStateFamily = IPv6Family + ipStateFamily = cns.IPv6Family } // check if the IP with the same family type exists already diff --git a/cns/restserver/ipusage.go b/cns/restserver/ipusage.go index e2ae30382b..ed87752104 100644 --- a/cns/restserver/ipusage.go +++ b/cns/restserver/ipusage.go @@ -5,19 +5,6 @@ import ( "github.com/Azure/azure-container-networking/cns/types" ) -type ipState struct { - // allocatedIPs are all the IPs given to CNS by DNC. - allocatedIPs int64 - // assignedIPs are the IPs CNS gives to Pods. - assignedIPs int64 - // availableIPs are the IPs in state "Available". - availableIPs int64 - // programmingIPs are the IPs in state "PendingProgramming". - programmingIPs int64 - // releasingIPs are the IPs in state "PendingReleasr". - releasingIPs int64 -} - func (service *HTTPRestService) buildIPState() *ipState { service.Lock() defer service.Unlock() diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index f130479c79..325b87b947 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -80,7 +80,7 @@ type HTTPRestService struct { PnpIDByMacAddress map[string]string imdsClient imdsClient nodesubnetIPFetcher *nodesubnet.IPFetcher - IPFamilies []IPFamily + IPFamilies []cns.IPFamily } type CNIConflistGenerator interface { From 56009593ce4dd2eb2a16949e697d359e5e7edfde Mon Sep 17 00:00:00 2001 From: ryandenney Date: Mon, 18 Nov 2024 11:45:30 -0500 Subject: [PATCH 08/10] removing todos --- cns/middlewares/k8sSwiftV2.go | 4 ---- cns/middlewares/k8sSwiftV2_linux.go | 4 ---- cns/middlewares/k8sSwiftV2_windows.go | 2 -- cns/restserver/ipam.go | 1 - cns/service/main.go | 2 -- 5 files changed, 13 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 65f4fa0c82..4a2c5838c6 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -38,7 +38,6 @@ var _ cns.IPConfigsHandlerMiddleware = (*K8sSWIFTv2Middleware)(nil) // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request // and release IP configs handlers. -// TODO: this will need to changed or add a new function to also work for a delegated NIC with multiple pods func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { podInfo, respCode, message := k.validateIPConfigsRequest(ctx, &req) @@ -53,11 +52,9 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } ipConfigsResp, err := defaultHandler(ctx, req) // If the pod is not v2, return the response from the handler - // we need to add the secondary Interface. Our current POC cluster is returning here if !req.SecondaryInterfacesExist { return ipConfigsResp, err } - // TODO: the pod itself won't be "V2" as we aren't using multitenancy pods // If the pod is v2, get the infra IP configs from the handler first and then add the SWIFTv2 IP config defer func() { // Release the default IP config if there is an error @@ -103,7 +100,6 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } } -// TODO: we will not be using multitenant pods. Need to look into what labels we are currently seeing and maybe compare to Vanilla swiftv2 // For our purposes we would skip over this logic or need to replace it with something to check the delegated NIC // validateIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. // nolint diff --git a/cns/middlewares/k8sSwiftV2_linux.go b/cns/middlewares/k8sSwiftV2_linux.go index 8664657c4c..e9a93de0e2 100644 --- a/cns/middlewares/k8sSwiftV2_linux.go +++ b/cns/middlewares/k8sSwiftV2_linux.go @@ -18,9 +18,6 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { var routes []cns.Route switch podIPInfo.NICType { - // TODO: We may want to create a new type of NIC. Currently we are using delegated NICs but this set routes method only - // takes in for the multitenant scenario. We should have a new case that behaves similarly to InfraNIC but for a delegated NIC - // Q: Does the code currently see the kube pods as Infra or Delegated? They currently use the delegated pod subnet for IPs case cns.DelegatedVMNIC: virtualGWRoute := cns.Route{ IPAddress: fmt.Sprintf("%s/%d", virtualGW, prefixLength), @@ -68,7 +65,6 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { return errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress) } - //This function is called per IP so we shouldn't have to worry about adding both v4 and v6 at once if ip.Is4() { routes = append(routes, addRoutes(podCIDRsV4, overlayGatewayv4)...) routes = append(routes, addRoutes(serviceCIDRsV4, overlayGatewayv4)...) diff --git a/cns/middlewares/k8sSwiftV2_windows.go b/cns/middlewares/k8sSwiftV2_windows.go index d4993eb46c..2be2fbd1df 100644 --- a/cns/middlewares/k8sSwiftV2_windows.go +++ b/cns/middlewares/k8sSwiftV2_windows.go @@ -27,8 +27,6 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error { // assignSubnetPrefixLengthFields will assign the subnet-prefix length to some fields of podipinfo // this is required for the windows scenario so that HNS programming is successful for pods -// TODO: This is being used for the delegated NIC for Windows solution. Once we are testing Windows we will -// Need to confirm that this assigns the whole pod subnet that we expect for both v4 and v6 func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(podIPInfo *cns.PodIpInfo, interfaceInfo v1alpha1.InterfaceInfo, ip string) error { // Parse MTPNC SubnetAddressSpace to get the subnet prefix length subnet, subnetPrefix, err := utils.ParseIPAndPrefix(interfaceInfo.SubnetAddressSpace) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index ba70279a19..48b9dc6579 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -283,7 +283,6 @@ func (service *HTTPRestService) RequestIPConfigsHandler(w http.ResponseWriter, r } var ipConfigsResp *cns.IPConfigsResponse - // TODO: Decide what middleware option we need to singetenancy swift v2 // Check if IPConfigsHandlerMiddleware is set if service.IPConfigsHandlerMiddleware != nil { // Wrap the default datapath handlers with the middleware depending on middleware type diff --git a/cns/service/main.go b/cns/service/main.go index 84643e1a94..53502b29fd 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1404,7 +1404,6 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn // Start building the NNC Reconciler - // TODO: We need to return the IPFamilies from the reconciler so that they can be added to the Service struct // get CNS Node IP to compare NC Node IP with this Node IP to ensure NCs were created for this node nodeIP := configuration.NodeIP() nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP) @@ -1435,7 +1434,6 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } } - // TODO: If we need special middleware this is where we would be setting it if cnsconfig.EnableSwiftV2 { if err := mtpncctrl.SetupWithManager(manager); err != nil { return errors.Wrapf(err, "failed to setup mtpnc reconciler with manager") From 8b9e153d9d2cc0ce7e6bcf86c801ad0f923a2f80 Mon Sep 17 00:00:00 2001 From: ryandenney Date: Mon, 18 Nov 2024 11:48:02 -0500 Subject: [PATCH 09/10] fixing comments --- cns/middlewares/k8sSwiftV2.go | 1 - cns/restserver/ipam.go | 2 ++ cns/restserver/restserver.go | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index 4a2c5838c6..a11290c205 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -100,7 +100,6 @@ func (k *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fa } } -// For our purposes we would skip over this logic or need to replace it with something to check the delegated NIC // validateIPConfigsRequest validates if pod is multitenant by checking the pod labels, used in SWIFT V2 AKS scenario. // nolint func (k *K8sSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string) { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 48b9dc6579..3873e82d96 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -998,12 +998,14 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ return nil, ErrNoNCs } + // Gets the IPFamilies from all NCs and stores them in a map. This will be ued to determine the amount of IPs to return for ncID := range service.state.ContainerStatus { for ipFamily := range service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.IPFamilies { ipFamilies[ipFamily] = struct{}{} } } + // Makes sure we have at least one IPFamily across all NCs numOfIPFamilies := len(ipFamilies) if numOfIPFamilies == 0 { return nil, ErrNoIPFamilies diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 325b87b947..917ded0f26 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -55,7 +55,6 @@ type imdsClient interface { } // HTTPRestService represents http listener for CNS - Container Networking Service. -// TODO: If we add a new type of Middleware that will be reflected in the IPConfigsHandlerMiddleware value type HTTPRestService struct { *cns.Service dockerClient *dockerclient.Client From 699592ee1d5db819441f9f9a60cc8d830e227509 Mon Sep 17 00:00:00 2001 From: ryandenney Date: Mon, 18 Nov 2024 11:50:21 -0500 Subject: [PATCH 10/10] remove old file --- cns/restserver/ipusage.go | 45 --------------------------------------- 1 file changed, 45 deletions(-) delete mode 100644 cns/restserver/ipusage.go diff --git a/cns/restserver/ipusage.go b/cns/restserver/ipusage.go deleted file mode 100644 index ed87752104..0000000000 --- a/cns/restserver/ipusage.go +++ /dev/null @@ -1,45 +0,0 @@ -package restserver - -import ( - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/types" -) - -func (service *HTTPRestService) buildIPState() *ipState { - service.Lock() - defer service.Unlock() - - state := ipState{ - allocatedIPs: 0, - assignedIPs: 0, - availableIPs: 0, - programmingIPs: 0, - releasingIPs: 0, - } - - //nolint:gocritic // This has to iterate over the IP Config state to get the counts. - for _, ipConfig := range service.PodIPConfigState { - state.allocatedIPs++ - if ipConfig.GetState() == types.Assigned { - state.assignedIPs++ - } - if ipConfig.GetState() == types.Available { - state.availableIPs++ - } - if ipConfig.GetState() == types.PendingProgramming { - state.programmingIPs++ - } - if ipConfig.GetState() == types.PendingRelease { - state.releasingIPs++ - } - } - - logger.Printf("[IP Usage] Allocated IPs: %d, Assigned IPs: %d, Available IPs: %d, PendingProgramming IPs: %d, PendingRelease IPs: %d", - state.allocatedIPs, - state.assignedIPs, - state.availableIPs, - state.programmingIPs, - state.releasingIPs, - ) - return &state -}