From d40c276217461d952054c2d7ceaba057c7234cfa Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 12 Dec 2023 15:58:07 -0800 Subject: [PATCH 01/25] changes for adding swiftv2 SF config --- cni/network/network.go | 5 +- cns/NetworkContainerContract.go | 8 +- cns/configuration/configuration.go | 2 + cns/restserver/ipam.go | 123 ++++++++++++++++++++++++++++- cns/restserver/restserver.go | 1 + cns/restserver/util.go | 18 +++++ cns/service/main.go | 4 +- 7 files changed, 153 insertions(+), 8 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 2381b8ffc4..d7c6dec53e 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -608,7 +608,8 @@ func (plugin *NetPlugin) createNetworkInternal( ipamAddResult.hostSubnetPrefix.IP = ipamAddResult.hostSubnetPrefix.IP.Mask(ipamAddResult.hostSubnetPrefix.Mask) ipamAddConfig.nwCfg.IPAM.Subnet = ipamAddResult.hostSubnetPrefix.String() // Find the master interface. - masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.hostSubnetPrefix) + //masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.hostSubnetPrefix) + masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.secondaryInterfacesInfo[0].IPConfigs[0].Address) if masterIfName == "" { err := plugin.Errorf("Failed to find the master interface") return nwInfo, err @@ -616,7 +617,7 @@ func (plugin *NetPlugin) createNetworkInternal( logger.Info("Found master interface", zap.String("ifname", masterIfName)) // Add the master as an external interface. - err := plugin.nm.AddExternalInterface(masterIfName, ipamAddResult.hostSubnetPrefix.String()) + err := plugin.nm.AddExternalInterface(masterIfName, ipamAddResult.secondaryInterfacesInfo[0].IPConfigs[0].Address.String()) if err != nil { err = plugin.Errorf("Failed to add external interface: %v", err) return nwInfo, err diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index be33c105e3..fc7fe16f1a 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -457,6 +457,7 @@ type PodIpInfo struct { PodIPConfig IPSubnet NetworkContainerPrimaryIPConfig IPConfiguration HostPrimaryIPInfo HostIPInfo + HostSecondaryIPInfo HostIPInfo // NICType defines whether NIC is InfraNIC or DelegatedVMNIC or BackendNIC NICType NICType InterfaceName string @@ -469,9 +470,10 @@ type PodIpInfo struct { } type HostIPInfo struct { - Gateway string - PrimaryIP string - Subnet string + Gateway string + PrimaryIP string + SecondaryIP string + Subnet string } type IPConfigRequest struct { diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 005ed3f83b..91942ce47e 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -57,6 +57,8 @@ type CNSConfig struct { AsyncPodDeletePath string } +type EnableSwiftV2Mode string + type TelemetrySettings struct { // Flag to disable the telemetry. DisableAll bool diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index c9f4d059cf..49b9a30f77 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -4,10 +4,14 @@ package restserver import ( + "bytes" "context" + "encoding/json" "fmt" + "github.com/Azure/azure-container-networking/cns/configuration" "net" "net/http" + "net/http/httptest" "strconv" "strings" @@ -20,6 +24,11 @@ import ( "github.com/pkg/errors" ) +const ( + contentTypeJSON = "application/json" + headerContentType = "Content-Type" +) + var ( ErrStoreEmpty = errors.New("empty endpoint state store") ErrParsePodIPFailed = errors.New("failed to parse pod's ip") @@ -30,6 +39,12 @@ var ( // requestIPConfigHandlerHelper validates the request, assign IPs and return the IPConfigs func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + cmdLineConfigPath := common.GetArg(common.OptCNSConfigPath).(string) + cnsconfig, err := configuration.ReadConfig(cmdLineConfigPath) + var swiftv2sf bool + if cnsconfig.EnableSwiftV2 == configuration.EnableSFSwiftV2 { + swiftv2sf = true + } // For SWIFT v2 scenario, the validator function will also modify the ipconfigsRequest. podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) if returnCode != types.Success { @@ -43,8 +58,12 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context // record a pod requesting an IP service.podsPendingIPAssignment.Push(podInfo.Key()) + var podIPInfo []cns.PodIpInfo + // request IPs from pod state only if scenario for swiftv2 is non-Standalone i.e. AKS + if swiftv2sf { + podIPInfo, err = requestIPConfigsHelper(service, ipconfigsRequest) + } - podIPInfo, err := requestIPConfigsHelper(service, ipconfigsRequest) if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -77,6 +96,55 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context } } + // Check if request is for pod with secondary interface(s) + if podInfo.SecondaryInterfacesExist() || swiftv2sf { + // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here + var SWIFTv2PodIPInfo cns.PodIpInfo + if swiftv2sf { + SWIFTv2PodIPInfo, err = service.getIPConfig(podInfo) + } else { + SWIFTv2PodIPInfo, err = service.SWIFTv2Middleware.GetIPConfig(ctx, podInfo) + } + if err != nil { + defer func() { + logger.Errorf("failed to get SWIFTv2 IP config : %v. Releasing default IP config...", err) + _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) + if err != nil { + logger.Errorf("failed to release default IP config : %v", err) + } + }() + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, ipconfigsRequest), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", ipconfigsRequest) + } + podIPInfo = append(podIPInfo, SWIFTv2PodIPInfo) + // Setting up routes for SWIFTv2 scenario + for i := range podIPInfo { + ipInfo := &podIPInfo[i] + err := service.SWIFTv2Middleware.SetRoutes(ipInfo) + if err != nil { + defer func() { //nolint:gocritic + logger.Errorf("failed to set routes for SWIFTv2 IP config : %v. Releasing default IP config...", err) + _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) + if err != nil { + logger.Errorf("failed to release default IP config : %v", err) + } + }() + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.UnexpectedError, + Message: fmt.Sprintf("failed to set SWIFTv2 routes : %v", err), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to set SWIFTv2 routes : %v", ipconfigsRequest) + } + } + } + return &cns.IPConfigsResponse{ Response: cns.Response{ ReturnCode: types.Success, @@ -85,6 +153,59 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }, nil } +func (service *HTTPRestService) getIPConfig(podInfo cns.PodInfo) (cns.PodIpInfo, error) { + var body bytes.Buffer + var resp cns.GetNetworkContainerResponse + podInfoBytes, err := json.Marshal(podInfo) + if err != nil { + return cns.PodIpInfo{}, errors.Wrap(err, "failed to marshal podinfo from request") + } + payload := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} + + if err := json.NewEncoder(&body).Encode(payload); err != nil { + return cns.PodIpInfo{}, errors.Wrap(err, "failed to encode GetNetworkContainerRequest OrchestratorContext") + } + + req, err := http.NewRequest(http.MethodPost, cns.GetNetworkContainerByOrchestratorContext, &body) + if err != nil { + return cns.PodIpInfo{}, errors.Wrap(err, "failed to build request GetNetworkContainerByOrchestratorContext") + } + req.Header.Set(headerContentType, contentTypeJSON) + w := httptest.NewRecorder() + service.getNetworkContainerByOrchestratorContext(w, req) + + err = json.NewDecoder(w.Body).Decode(&resp) + if err != nil { + return cns.PodIpInfo{}, fmt.Errorf("decoding response: %w", err) + } + + // Check if the ncstate/ipconfig ready. If one of the fields is empty, return error + if resp.IPConfiguration.IPSubnet.IPAddress == "" || resp.NetworkInterfaceInfo.MACAddress == "" || resp.NetworkContainerID == "" || resp.IPConfiguration.GatewayIPAddress == "" { + return cns.PodIpInfo{}, errors.New("ipconfig is empty for given nc") + } + logger.Printf("[SWIFTv2SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) + + hostInterface, err := service.getSecondaryHostInterface(context.TODO()) + logger.Printf("secondary hostInterface is %+v", hostInterface) + if err != nil { + return cns.PodIpInfo{}, err + } + podIPInfo := cns.PodIpInfo{ + PodIPConfig: resp.IPConfiguration.IPSubnet, + MacAddress: resp.NetworkInterfaceInfo.MACAddress, + NICType: resp.NetworkInterfaceInfo.NICType, + SkipDefaultRoutes: false, + HostSecondaryIPInfo: cns.HostIPInfo{ + Gateway: hostInterface.Gateway, + SecondaryIP: hostInterface.SecondaryIPs[0], + Subnet: hostInterface.Subnet, + }, + HostPrimaryIPInfo: cns.HostIPInfo{Subnet: hostInterface.Subnet}, + NetworkContainerPrimaryIPConfig: resp.IPConfiguration, + } + return podIPInfo, nil +} + // requestIPConfigHandler requests an IPConfig from the CNS state func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r *http.Request) { var ipconfigRequest cns.IPConfigRequest diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index da13d0c9b5..1e79b593ae 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -148,6 +148,7 @@ type httpRestServiceState struct { TimeStamp time.Time joinedNetworks map[string]struct{} primaryInterface *wireserver.InterfaceInfo + secondaryInterface *wireserver.InterfaceInfo } type networkInfo struct { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index b65e440a54..2bdef96585 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -790,6 +790,24 @@ func (service *HTTPRestService) getPrimaryHostInterface(ctx context.Context) (*w return service.state.primaryInterface, nil } +func (service *HTTPRestService) getSecondaryHostInterface(ctx context.Context) (*wireserver.InterfaceInfo, error) { + if service.state.secondaryInterface == nil { + res, err := service.wscli.GetInterfaces(ctx) + logger.Printf("secondary interface res is %+v", res) + if err != nil { + return nil, errors.Wrap(err, "failed to get interfaces from IMDS") + } + secondary, err := wireserver.GetSecondaryInterfaceFromResult(res) + logger.Printf("secondary is %+v", secondary) + if err != nil { + return nil, errors.Wrap(err, "failed to get secondary interface from IMDS response") + } + service.state.secondaryInterface = secondary + } + logger.Printf("service.state.secondaryInterface is %+v", service.state.secondaryInterface) + return service.state.secondaryInterface, nil +} + //nolint:gocritic // ignore hugeParam pls func (service *HTTPRestService) populateIPConfigInfoUntransacted(ipConfigStatus cns.IPConfigurationStatus, podIPInfo *cns.PodIpInfo) error { ncStatus, exists := service.state.ContainerStatus[ipConfigStatus.NCID] diff --git a/cns/service/main.go b/cns/service/main.go index 94a8cfd878..5dfc9e60b1 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1213,7 +1213,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn // check the Node labels for Swift V2 if _, ok := node.Labels[configuration.LabelNodeSwiftV2]; ok { - cnsconfig.EnableSwiftV2 = true + cnsconfig.EnableSwiftV2 = configuration.EnableK8SwiftV2 cnsconfig.WatchPods = true // TODO(rbtr): create the NodeInfo for Swift V2 // register the noop mtpnc reconciler to populate the cache @@ -1376,7 +1376,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } } - if cnsconfig.EnableSwiftV2 { + if cnsconfig.EnableSwiftV2 == configuration.EnableK8SwiftV2 { if err := mtpncctrl.SetupWithManager(manager); err != nil { return errors.Wrapf(err, "failed to setup mtpnc reconciler with manager") } From 1bcce579ebfc99cd636248990799fe5a4ec28926 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Fri, 15 Dec 2023 00:10:32 -0800 Subject: [PATCH 02/25] refactor: getSecondaryInterfaceInfo --- cns/restserver/ipam.go | 12 +++++------ cns/restserver/util.go | 9 ++++---- cns/wireserver/net.go | 48 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 49b9a30f77..1ffa6a60e3 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -45,7 +45,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context if cnsconfig.EnableSwiftV2 == configuration.EnableSFSwiftV2 { swiftv2sf = true } - // For SWIFT v2 scenario, the validator function will also modify the ipconfigsRequest. + // For only SWIFTV2-AKS scenario, the validator function will also modify the ipconfigsRequest. podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ @@ -101,7 +101,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here var SWIFTv2PodIPInfo cns.PodIpInfo if swiftv2sf { - SWIFTv2PodIPInfo, err = service.getIPConfig(podInfo) + SWIFTv2PodIPInfo, err = service.getIPConfigforSwiftV2SF(podInfo) } else { SWIFTv2PodIPInfo, err = service.SWIFTv2Middleware.GetIPConfig(ctx, podInfo) } @@ -153,7 +153,8 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }, nil } -func (service *HTTPRestService) getIPConfig(podInfo cns.PodInfo) (cns.PodIpInfo, error) { +// getIPConfig PodIpInfo for swiftv2 SF scenario, gets info from NC goal state +func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cns.PodIpInfo, error) { var body bytes.Buffer var resp cns.GetNetworkContainerResponse podInfoBytes, err := json.Marshal(podInfo) @@ -183,10 +184,9 @@ func (service *HTTPRestService) getIPConfig(podInfo cns.PodInfo) (cns.PodIpInfo, if resp.IPConfiguration.IPSubnet.IPAddress == "" || resp.NetworkInterfaceInfo.MACAddress == "" || resp.NetworkContainerID == "" || resp.IPConfiguration.GatewayIPAddress == "" { return cns.PodIpInfo{}, errors.New("ipconfig is empty for given nc") } - logger.Printf("[SWIFTv2SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) + logger.Printf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) - hostInterface, err := service.getSecondaryHostInterface(context.TODO()) - logger.Printf("secondary hostInterface is %+v", hostInterface) + hostInterface, err := service.getSecondaryHostInterface(context.TODO(), resp.NetworkInterfaceInfo.MACAddress) if err != nil { return cns.PodIpInfo{}, err } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 2bdef96585..a21b91ff1b 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -790,21 +790,20 @@ func (service *HTTPRestService) getPrimaryHostInterface(ctx context.Context) (*w return service.state.primaryInterface, nil } -func (service *HTTPRestService) getSecondaryHostInterface(ctx context.Context) (*wireserver.InterfaceInfo, error) { +func (service *HTTPRestService) getSecondaryHostInterface(ctx context.Context, macAddress string) (*wireserver.InterfaceInfo, error) { if service.state.secondaryInterface == nil { res, err := service.wscli.GetInterfaces(ctx) - logger.Printf("secondary interface res is %+v", res) + if err != nil { return nil, errors.Wrap(err, "failed to get interfaces from IMDS") } - secondary, err := wireserver.GetSecondaryInterfaceFromResult(res) - logger.Printf("secondary is %+v", secondary) + secondary, err := wireserver.GetSecondaryInterfaceFromResult(res, macAddress) + if err != nil { return nil, errors.Wrap(err, "failed to get secondary interface from IMDS response") } service.state.secondaryInterface = secondary } - logger.Printf("service.state.secondaryInterface is %+v", service.state.secondaryInterface) return service.state.secondaryInterface, nil } diff --git a/cns/wireserver/net.go b/cns/wireserver/net.go index ed0791d0af..2fb287a994 100644 --- a/cns/wireserver/net.go +++ b/cns/wireserver/net.go @@ -2,13 +2,17 @@ package wireserver import ( "net" + "regexp" + "strings" "github.com/pkg/errors" ) var ( - // ErrNoPrimaryInterface indicates the wireserver respnose does not have a primary interface indicated. + // ErrNoPrimaryInterface indicates the wireserver response does not have a primary interface indicated. ErrNoPrimaryInterface = errors.New("no primary interface found") + // ErrNoSecondaryInterface indicates the wireserver response does not have a secondary interface + ErrNoSecondaryInterface = errors.New("no secondary interface found") // ErrInsufficientAddressSpace indicates that the CIDR space is too small to include a gateway IP; it is 1 IP. ErrInsufficientAddressSpace = errors.New("insufficient address space to generate gateway IP") ) @@ -49,6 +53,48 @@ func GetPrimaryInterfaceFromResult(res *GetInterfacesResult) (*InterfaceInfo, er return nil, ErrNoPrimaryInterface } +// Gets secondary interface details for swiftv2 secondary nics scenario +func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string) (*InterfaceInfo, error) { + for _, i := range res.Interface { + // skip if primary + if i.IsPrimary { + continue + } + + // skip if no subnets + if len(i.IPSubnet) == 0 { + continue + } + + newMacAddress := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(macAddress, "") + if i.MacAddress == strings.ToUpper(newMacAddress) { + // get the second subnet + s := i.IPSubnet[0] + gw, err := calculateGatewayIP(s.Prefix) + if err != nil { + return nil, err + } + + secondaryIP := "" + for _, ip := range s.IPAddress { + if !ip.IsPrimary { + secondaryIP = ip.Address + } + } + secondaryIPs := []string{} + secondaryIPs = append(secondaryIPs, secondaryIP) + + return &InterfaceInfo{ + Subnet: s.Prefix, + IsPrimary: false, + Gateway: gw.String(), + SecondaryIPs: secondaryIPs, + }, nil + } + } + return nil, ErrNoSecondaryInterface +} + // calculateGatewayIP parses the passed CIDR string and returns the first IP in the range. func calculateGatewayIP(cidr string) (net.IP, error) { _, subnet, err := net.ParseCIDR(cidr) From be6bf9d6b66d8497d5e5bb6e502916df110ab167 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Fri, 15 Dec 2023 13:01:52 -0800 Subject: [PATCH 03/25] checks for sf orch --- cns/restserver/internalapi.go | 2 +- cns/restserver/ipam.go | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 904cc33974..bde4be6c82 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -521,7 +521,7 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. // For now only RequestController uses this API which will be initialized only for AKS scenario. // Validate ContainerType is set as Docker - if service.state.OrchestratorType != cns.KubernetesCRD && service.state.OrchestratorType != cns.Kubernetes { + if service.state.OrchestratorType != cns.KubernetesCRD && service.state.OrchestratorType != cns.Kubernetes && service.state.OrchestratorType != cns.ServiceFabric { logger.Errorf("[Azure CNS] Error. Unsupported OrchestratorType: %s", service.state.OrchestratorType) return types.UnsupportedOrchestratorType } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 1ffa6a60e3..e74d6e20d7 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -60,7 +60,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context service.podsPendingIPAssignment.Push(podInfo.Key()) var podIPInfo []cns.PodIpInfo // request IPs from pod state only if scenario for swiftv2 is non-Standalone i.e. AKS - if swiftv2sf { + if !swiftv2sf { podIPInfo, err = requestIPConfigsHelper(service, ipconfigsRequest) } @@ -186,7 +186,12 @@ func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cn } logger.Printf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) - hostInterface, err := service.getSecondaryHostInterface(context.TODO(), resp.NetworkInterfaceInfo.MACAddress) + hostPrimaryInterface, err := service.getPrimaryHostInterface(context.TODO()) + if err != nil { + return cns.PodIpInfo{}, err + } + + hostSecondaryInterface, err := service.getSecondaryHostInterface(context.TODO(), resp.NetworkInterfaceInfo.MACAddress) if err != nil { return cns.PodIpInfo{}, err } @@ -195,12 +200,16 @@ func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cn MacAddress: resp.NetworkInterfaceInfo.MACAddress, NICType: resp.NetworkInterfaceInfo.NICType, SkipDefaultRoutes: false, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: hostPrimaryInterface.Gateway, + PrimaryIP: hostPrimaryInterface.PrimaryIP, + Subnet: hostPrimaryInterface.Subnet, + }, HostSecondaryIPInfo: cns.HostIPInfo{ - Gateway: hostInterface.Gateway, - SecondaryIP: hostInterface.SecondaryIPs[0], - Subnet: hostInterface.Subnet, + Gateway: hostSecondaryInterface.Gateway, + SecondaryIP: hostSecondaryInterface.SecondaryIPs[0], + Subnet: hostSecondaryInterface.Subnet, }, - HostPrimaryIPInfo: cns.HostIPInfo{Subnet: hostInterface.Subnet}, NetworkContainerPrimaryIPConfig: resp.IPConfiguration, } return podIPInfo, nil From e41c827e4c1c7d36f05b98b6aa84b611c1647b46 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Sun, 17 Dec 2023 20:58:05 -0800 Subject: [PATCH 04/25] Add hnsclient new mode & NCID in PodIpInfo --- cns/hnsclient/hnsclient_windows.go | 7 +++--- cns/restserver/ipam.go | 40 ++++++++++++++++++++++++------ cns/wireserver/net.go | 9 +++---- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/cns/hnsclient/hnsclient_windows.go b/cns/hnsclient/hnsclient_windows.go index cc24b7917a..64c958dd36 100644 --- a/cns/hnsclient/hnsclient_windows.go +++ b/cns/hnsclient/hnsclient_windows.go @@ -27,8 +27,9 @@ const ( ExtHnsNetworkGwAddress = "192.168.255.1" // HNS network types - hnsL2Bridge = "l2bridge" - hnsL2Tunnel = "l2tunnel" + hnsL2Bridge = "l2bridge" + hnsL2Tunnel = "l2tunnel" + hnsTransparent = "transparent" // hcnSchemaVersionMajor indicates major version number for hcn schema hcnSchemaVersionMajor = 2 @@ -137,7 +138,7 @@ func CreateDefaultExtNetwork(networkType string) error { return nil } - if networkType != hnsL2Bridge && networkType != hnsL2Tunnel { + if networkType != hnsL2Bridge && networkType != hnsL2Tunnel && networkType != hnsTransparent { return fmt.Errorf("Invalid hns network type %s", networkType) } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index e74d6e20d7..871a490d02 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -96,15 +96,10 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context } } - // Check if request is for pod with secondary interface(s) - if podInfo.SecondaryInterfacesExist() || swiftv2sf { + // Follow AKS swiftv2 podinfo path if secondary interface(s) exist + if podInfo.SecondaryInterfacesExist() && !swiftv2sf { // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here - var SWIFTv2PodIPInfo cns.PodIpInfo - if swiftv2sf { - SWIFTv2PodIPInfo, err = service.getIPConfigforSwiftV2SF(podInfo) - } else { - SWIFTv2PodIPInfo, err = service.SWIFTv2Middleware.GetIPConfig(ctx, podInfo) - } + SWIFTv2PodIPInfo, err := service.SWIFTv2Middleware.GetIPConfig(ctx, podInfo) if err != nil { defer func() { logger.Errorf("failed to get SWIFTv2 IP config : %v. Releasing default IP config...", err) @@ -144,6 +139,35 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context } } } + // Check if request is for pod with secondary interface(s) + if swiftv2sf { + // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here + SWIFTv2PodIPInfo, err := service.getIPConfigforSwiftV2SF(podInfo) + if err != nil { + defer func() { + logger.Errorf("failed to get SWIFTv2 IP config : %v. Releasing default IP config...", err) + _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) + if err != nil { + logger.Errorf("failed to release default IP config : %v", err) + } + }() + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, ipconfigsRequest), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", ipconfigsRequest) + } + podIPInfo = append(podIPInfo, SWIFTv2PodIPInfo) + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.UnexpectedError, + Message: fmt.Sprintf("failed to set SWIFTv2 routes : %v", err), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to set SWIFTv2 routes : %v", ipconfigsRequest) + } return &cns.IPConfigsResponse{ Response: cns.Response{ diff --git a/cns/wireserver/net.go b/cns/wireserver/net.go index 2fb287a994..ebe4d8dd55 100644 --- a/cns/wireserver/net.go +++ b/cns/wireserver/net.go @@ -2,7 +2,6 @@ package wireserver import ( "net" - "regexp" "strings" "github.com/pkg/errors" @@ -11,8 +10,8 @@ import ( var ( // ErrNoPrimaryInterface indicates the wireserver response does not have a primary interface indicated. ErrNoPrimaryInterface = errors.New("no primary interface found") - // ErrNoSecondaryInterface indicates the wireserver response does not have a secondary interface - ErrNoSecondaryInterface = errors.New("no secondary interface found") + // ErrNoSecondaryInterfaces indicates the wireserver response does not have secondary interfaces on the node + ErrNoSecondaryInterfaces = errors.New("no secondary interfaces found") // ErrInsufficientAddressSpace indicates that the CIDR space is too small to include a gateway IP; it is 1 IP. ErrInsufficientAddressSpace = errors.New("insufficient address space to generate gateway IP") ) @@ -66,7 +65,7 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string continue } - newMacAddress := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(macAddress, "") + newMacAddress := strings.ReplaceAll(macAddress, "-", "") if i.MacAddress == strings.ToUpper(newMacAddress) { // get the second subnet s := i.IPSubnet[0] @@ -92,7 +91,7 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string }, nil } } - return nil, ErrNoSecondaryInterface + return nil, ErrNoSecondaryInterfaces } // calculateGatewayIP parses the passed CIDR string and returns the first IP in the range. From 78d60b3afe4d868c7b4e8d474ee26625d40895c9 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 19 Dec 2023 13:38:52 -0800 Subject: [PATCH 05/25] undoing cni part changes --- cni/network/network.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index d7c6dec53e..2381b8ffc4 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -608,8 +608,7 @@ func (plugin *NetPlugin) createNetworkInternal( ipamAddResult.hostSubnetPrefix.IP = ipamAddResult.hostSubnetPrefix.IP.Mask(ipamAddResult.hostSubnetPrefix.Mask) ipamAddConfig.nwCfg.IPAM.Subnet = ipamAddResult.hostSubnetPrefix.String() // Find the master interface. - //masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.hostSubnetPrefix) - masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.secondaryInterfacesInfo[0].IPConfigs[0].Address) + masterIfName := plugin.findMasterInterface(ipamAddConfig.nwCfg, &ipamAddResult.hostSubnetPrefix) if masterIfName == "" { err := plugin.Errorf("Failed to find the master interface") return nwInfo, err @@ -617,7 +616,7 @@ func (plugin *NetPlugin) createNetworkInternal( logger.Info("Found master interface", zap.String("ifname", masterIfName)) // Add the master as an external interface. - err := plugin.nm.AddExternalInterface(masterIfName, ipamAddResult.secondaryInterfacesInfo[0].IPConfigs[0].Address.String()) + err := plugin.nm.AddExternalInterface(masterIfName, ipamAddResult.hostSubnetPrefix.String()) if err != nil { err = plugin.Errorf("Failed to add external interface: %v", err) return nwInfo, err From d9dde58b3597644ef06ef7196b23a5688b20fdb8 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 19 Dec 2023 13:38:52 -0800 Subject: [PATCH 06/25] undoing cni part changes --- cns/wireserver/net.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cns/wireserver/net.go b/cns/wireserver/net.go index ebe4d8dd55..d881bd8780 100644 --- a/cns/wireserver/net.go +++ b/cns/wireserver/net.go @@ -1,7 +1,9 @@ package wireserver import ( + "github.com/Azure/azure-container-networking/cns/logger" "net" + "regexp" "strings" "github.com/pkg/errors" @@ -65,7 +67,8 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string continue } - newMacAddress := strings.ReplaceAll(macAddress, "-", "") + logger.Printf("i.MacAddress is %s", strings.Split(i.MacAddress, ":")) + newMacAddress := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(macAddress, "") if i.MacAddress == strings.ToUpper(newMacAddress) { // get the second subnet s := i.IPSubnet[0] From a9cb9fa387c2675e0da095c6314f4fb75bfc6948 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 2 Jan 2024 23:15:36 -0800 Subject: [PATCH 07/25] fix: reordering code --- cns/restserver/ipam.go | 8 ------ cns/restserver/util.go | 37 ++++++++++++++++--------- cns/wireserver/client.go | 60 +++++++++++++++++++++++++++++----------- 3 files changed, 68 insertions(+), 37 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 871a490d02..2e32d6d75c 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -160,13 +160,6 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", ipconfigsRequest) } podIPInfo = append(podIPInfo, SWIFTv2PodIPInfo) - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: types.UnexpectedError, - Message: fmt.Sprintf("failed to set SWIFTv2 routes : %v", err), - }, - PodIPInfo: []cns.PodIpInfo{}, - }, errors.Wrapf(err, "failed to set SWIFTv2 routes : %v", ipconfigsRequest) } return &cns.IPConfigsResponse{ @@ -209,7 +202,6 @@ func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cn return cns.PodIpInfo{}, errors.New("ipconfig is empty for given nc") } logger.Printf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) - hostPrimaryInterface, err := service.getPrimaryHostInterface(context.TODO()) if err != nil { return cns.PodIpInfo{}, err diff --git a/cns/restserver/util.go b/cns/restserver/util.go index a21b91ff1b..55ab6cdc0a 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -791,20 +791,31 @@ func (service *HTTPRestService) getPrimaryHostInterface(ctx context.Context) (*w } func (service *HTTPRestService) getSecondaryHostInterface(ctx context.Context, macAddress string) (*wireserver.InterfaceInfo, error) { - if service.state.secondaryInterface == nil { - res, err := service.wscli.GetInterfaces(ctx) - - if err != nil { - return nil, errors.Wrap(err, "failed to get interfaces from IMDS") - } - secondary, err := wireserver.GetSecondaryInterfaceFromResult(res, macAddress) - - if err != nil { - return nil, errors.Wrap(err, "failed to get secondary interface from IMDS response") - } - service.state.secondaryInterface = secondary + // todo: remove this temp hack to test e2e code, wireserver isnt responding on node + secondaryIPs := []string{} + secondaryIPs = append(secondaryIPs, "10.0.0.5") + secondNIC := wireserver.InterfaceInfo{ + Subnet: "10.0.0.0/24", + Gateway: "10.0.0.1", + IsPrimary: false, + PrimaryIP: "", + SecondaryIPs: secondaryIPs, } - return service.state.secondaryInterface, nil + return &secondNIC, nil + //if service.state.secondaryInterface == nil { + // res, err := service.wscli.GetInterfaces(ctx) + // + // if err != nil { + // return nil, errors.Wrap(err, "failed to get interfaces from IMDS") + // } + // secondary, err := wireserver.GetSecondaryInterfaceFromResult(res, macAddress) + // + // if err != nil { + // return nil, errors.Wrap(err, "failed to get secondary interface from IMDS response") + // } + // service.state.secondaryInterface = secondary + //} + //return service.state.secondaryInterface, nil } //nolint:gocritic // ignore hugeParam pls diff --git a/cns/wireserver/client.go b/cns/wireserver/client.go index 417e60ef6f..8fdae84f63 100644 --- a/cns/wireserver/client.go +++ b/cns/wireserver/client.go @@ -4,11 +4,10 @@ import ( "bytes" "context" "encoding/xml" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/pkg/errors" "io" "net/http" - "net/url" - - "github.com/pkg/errors" ) const ( @@ -40,21 +39,50 @@ func (c *Client) hostport() string { } // GetInterfaces queries interfaces from the wireserver. -func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error) { - c.Logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") +//func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error) { +// c.Logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") +// +// q := &url.Values{} +// q.Add("comp", "nmagent") +// q.Add("type", "getinterfaceinfov1") +// +// reqURL := &url.URL{ +// Scheme: "http", +// Host: c.hostport(), +// Path: "/machine/plugins", +// RawQuery: q.Encode(), +// } +// +// req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL.String(), http.NoBody) +// if err != nil { +// return nil, errors.Wrap(err, "failed to construct request") +// } +// resp, err := c.HTTPClient.Do(req) +// if err != nil { +// return nil, errors.Wrap(err, "failed to execute request") +// } +// defer resp.Body.Close() +// b, err := io.ReadAll(resp.Body) +// if err != nil { +// return nil, errors.Wrap(err, "failed to read response body") +// } +// +// c.Logger.Printf("[Azure CNS] Response received from NMAgent for get interface details: %s", string(b)) +// +// var res GetInterfacesResult +// if err := xml.NewDecoder(bytes.NewReader(b)).Decode(&res); err != nil { +// return nil, errors.Wrap(err, "failed to decode response body") +// } +// c.Logger.Printf("[km] GetInterfaces Result: %+v", res) +// return &res, nil +//} - q := &url.Values{} - q.Add("comp", "nmagent") - q.Add("type", "getinterfaceinfov1") +const hostQueryURL = "http://168.63.129.16/machine/plugins?comp=nmagent&type=getinterfaceinfov1" - reqURL := &url.URL{ - Scheme: "http", - Host: c.hostport(), - Path: "/machine/plugins", - RawQuery: q.Encode(), - } +func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error) { + logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") - req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL.String(), http.NoBody) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, hostQueryURL, nil) if err != nil { return nil, errors.Wrap(err, "failed to construct request") } @@ -68,7 +96,7 @@ func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error return nil, errors.Wrap(err, "failed to read response body") } - c.Logger.Printf("[Azure CNS] Response received from NMAgent for get interface details: %s", string(b)) + logger.Printf("[Azure CNS] Response received from NMAgent for get interface details: %s", string(b)) var res GetInterfacesResult if err := xml.NewDecoder(bytes.NewReader(b)).Decode(&res); err != nil { From 5426ed3be311d20f9fc7e81477c736fc50e8deb5 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Wed, 3 Jan 2024 12:05:01 -0800 Subject: [PATCH 08/25] fix: golint errors --- cns/restserver/ipam.go | 18 ++++++------ cns/restserver/util.go | 38 +++++++++--------------- cns/wireserver/client.go | 62 ++++++++++------------------------------ cns/wireserver/net.go | 12 ++++---- 4 files changed, 44 insertions(+), 86 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 2e32d6d75c..1feb9d4c6f 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/Azure/azure-container-networking/cns/configuration" "net" "net/http" "net/http/httptest" @@ -16,6 +15,7 @@ import ( "strings" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/filter" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" @@ -61,7 +61,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context var podIPInfo []cns.PodIpInfo // request IPs from pod state only if scenario for swiftv2 is non-Standalone i.e. AKS if !swiftv2sf { - podIPInfo, err = requestIPConfigsHelper(service, ipconfigsRequest) + podIPInfo, err = requestIPConfigsHelper(service, ipconfigsRequest) //nolint:contextcheck // appease linter } if err != nil { @@ -142,11 +142,11 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context // Check if request is for pod with secondary interface(s) if swiftv2sf { // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here - SWIFTv2PodIPInfo, err := service.getIPConfigforSwiftV2SF(podInfo) + SWIFTv2PodIPInfo, err := service.getIPConfigforSwiftV2SF(podInfo) //nolint:contextcheck // appease linter if err != nil { defer func() { logger.Errorf("failed to get SWIFTv2 IP config : %v. Releasing default IP config...", err) - _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) + _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) //nolint:contextcheck // appease linter if err != nil { logger.Errorf("failed to release default IP config : %v", err) } @@ -180,11 +180,11 @@ func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cn } payload := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} - if err := json.NewEncoder(&body).Encode(payload); err != nil { + if err = json.NewEncoder(&body).Encode(payload); err != nil { return cns.PodIpInfo{}, errors.Wrap(err, "failed to encode GetNetworkContainerRequest OrchestratorContext") } - req, err := http.NewRequest(http.MethodPost, cns.GetNetworkContainerByOrchestratorContext, &body) + req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, cns.GetNetworkContainerByOrchestratorContext, &body) if err != nil { return cns.PodIpInfo{}, errors.Wrap(err, "failed to build request GetNetworkContainerByOrchestratorContext") } @@ -199,9 +199,10 @@ func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cn // Check if the ncstate/ipconfig ready. If one of the fields is empty, return error if resp.IPConfiguration.IPSubnet.IPAddress == "" || resp.NetworkInterfaceInfo.MACAddress == "" || resp.NetworkContainerID == "" || resp.IPConfiguration.GatewayIPAddress == "" { - return cns.PodIpInfo{}, errors.New("ipconfig is empty for given nc") + return cns.PodIpInfo{}, fmt.Errorf("one of the fields for GetNCResponse is empty for given NC: %+v", resp) //nolint:goerr113 // return error } - logger.Printf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) + logger.Debugf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) + hostPrimaryInterface, err := service.getPrimaryHostInterface(context.TODO()) if err != nil { return cns.PodIpInfo{}, err @@ -211,6 +212,7 @@ func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cn if err != nil { return cns.PodIpInfo{}, err } + podIPInfo := cns.PodIpInfo{ PodIPConfig: resp.IPConfiguration.IPSubnet, MacAddress: resp.NetworkInterfaceInfo.MACAddress, diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 55ab6cdc0a..6063ed16dc 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -790,32 +790,22 @@ func (service *HTTPRestService) getPrimaryHostInterface(ctx context.Context) (*w return service.state.primaryInterface, nil } +// getSecondaryHostInterface returns the cached InterfaceInfo, if available, otherwise +// queries the IMDS to get the secondary interface info and caches it in the server-state before returning the result. func (service *HTTPRestService) getSecondaryHostInterface(ctx context.Context, macAddress string) (*wireserver.InterfaceInfo, error) { - // todo: remove this temp hack to test e2e code, wireserver isnt responding on node - secondaryIPs := []string{} - secondaryIPs = append(secondaryIPs, "10.0.0.5") - secondNIC := wireserver.InterfaceInfo{ - Subnet: "10.0.0.0/24", - Gateway: "10.0.0.1", - IsPrimary: false, - PrimaryIP: "", - SecondaryIPs: secondaryIPs, + if service.state.secondaryInterface == nil { + res, err := service.wscli.GetInterfaces(ctx) + if err != nil { + return nil, errors.Wrap(err, "failed to get interfaces from wireserver client") + } + secondary, err := wireserver.GetSecondaryInterfaceFromResult(res, macAddress) + if err != nil { + return nil, errors.Wrap(err, "failed to get secondary interface from wireserver client") + } + + service.state.secondaryInterface = secondary } - return &secondNIC, nil - //if service.state.secondaryInterface == nil { - // res, err := service.wscli.GetInterfaces(ctx) - // - // if err != nil { - // return nil, errors.Wrap(err, "failed to get interfaces from IMDS") - // } - // secondary, err := wireserver.GetSecondaryInterfaceFromResult(res, macAddress) - // - // if err != nil { - // return nil, errors.Wrap(err, "failed to get secondary interface from IMDS response") - // } - // service.state.secondaryInterface = secondary - //} - //return service.state.secondaryInterface, nil + return service.state.secondaryInterface, nil } //nolint:gocritic // ignore hugeParam pls diff --git a/cns/wireserver/client.go b/cns/wireserver/client.go index 8fdae84f63..4ae375898c 100644 --- a/cns/wireserver/client.go +++ b/cns/wireserver/client.go @@ -4,14 +4,11 @@ import ( "bytes" "context" "encoding/xml" - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/pkg/errors" "io" "net/http" -) + "net/url" -const ( - WireserverIP = "168.63.129.16" + "github.com/pkg/errors" ) type GetNetworkContainerOpts struct { @@ -39,50 +36,21 @@ func (c *Client) hostport() string { } // GetInterfaces queries interfaces from the wireserver. -//func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error) { -// c.Logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") -// -// q := &url.Values{} -// q.Add("comp", "nmagent") -// q.Add("type", "getinterfaceinfov1") -// -// reqURL := &url.URL{ -// Scheme: "http", -// Host: c.hostport(), -// Path: "/machine/plugins", -// RawQuery: q.Encode(), -// } -// -// req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL.String(), http.NoBody) -// if err != nil { -// return nil, errors.Wrap(err, "failed to construct request") -// } -// resp, err := c.HTTPClient.Do(req) -// if err != nil { -// return nil, errors.Wrap(err, "failed to execute request") -// } -// defer resp.Body.Close() -// b, err := io.ReadAll(resp.Body) -// if err != nil { -// return nil, errors.Wrap(err, "failed to read response body") -// } -// -// c.Logger.Printf("[Azure CNS] Response received from NMAgent for get interface details: %s", string(b)) -// -// var res GetInterfacesResult -// if err := xml.NewDecoder(bytes.NewReader(b)).Decode(&res); err != nil { -// return nil, errors.Wrap(err, "failed to decode response body") -// } -// c.Logger.Printf("[km] GetInterfaces Result: %+v", res) -// return &res, nil -//} +func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error) { + c.Logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") -const hostQueryURL = "http://168.63.129.16/machine/plugins?comp=nmagent&type=getinterfaceinfov1" + q := &url.Values{} + q.Add("comp", "nmagent") + q.Add("type", "getinterfaceinfov1") -func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error) { - logger.Printf("[Azure CNS] GetPrimaryInterfaceInfoFromHost") + reqURL := &url.URL{ + Scheme: "http", + Host: c.hostport(), + Path: "/machine/plugins", + RawQuery: q.Encode(), + } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, hostQueryURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL.String(), http.NoBody) if err != nil { return nil, errors.Wrap(err, "failed to construct request") } @@ -96,7 +64,7 @@ func (c *Client) GetInterfaces(ctx context.Context) (*GetInterfacesResult, error return nil, errors.Wrap(err, "failed to read response body") } - logger.Printf("[Azure CNS] Response received from NMAgent for get interface details: %s", string(b)) + c.Logger.Printf("[Azure CNS] Response received from NMAgent for get interface details: %s", string(b)) var res GetInterfacesResult if err := xml.NewDecoder(bytes.NewReader(b)).Decode(&res); err != nil { diff --git a/cns/wireserver/net.go b/cns/wireserver/net.go index d881bd8780..7310afce75 100644 --- a/cns/wireserver/net.go +++ b/cns/wireserver/net.go @@ -1,7 +1,6 @@ package wireserver import ( - "github.com/Azure/azure-container-networking/cns/logger" "net" "regexp" "strings" @@ -12,8 +11,8 @@ import ( var ( // ErrNoPrimaryInterface indicates the wireserver response does not have a primary interface indicated. ErrNoPrimaryInterface = errors.New("no primary interface found") - // ErrNoSecondaryInterfaces indicates the wireserver response does not have secondary interfaces on the node - ErrNoSecondaryInterfaces = errors.New("no secondary interfaces found") + // ErrNoSecondaryInterface indicates the wireserver response does not have secondary interface on the node + ErrNoSecondaryInterface = errors.New("no secondary interface found") // ErrInsufficientAddressSpace indicates that the CIDR space is too small to include a gateway IP; it is 1 IP. ErrInsufficientAddressSpace = errors.New("insufficient address space to generate gateway IP") ) @@ -67,9 +66,8 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string continue } - logger.Printf("i.MacAddress is %s", strings.Split(i.MacAddress, ":")) newMacAddress := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(macAddress, "") - if i.MacAddress == strings.ToUpper(newMacAddress) { + if strings.EqualFold(i.MacAddress, newMacAddress) { // get the second subnet s := i.IPSubnet[0] gw, err := calculateGatewayIP(s.Prefix) @@ -83,7 +81,7 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string secondaryIP = ip.Address } } - secondaryIPs := []string{} + var secondaryIPs []string secondaryIPs = append(secondaryIPs, secondaryIP) return &InterfaceInfo{ @@ -94,7 +92,7 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string }, nil } } - return nil, ErrNoSecondaryInterfaces + return nil, ErrNoSecondaryInterface } // calculateGatewayIP parses the passed CIDR string and returns the first IP in the range. From d485ebcefb7e5fd78b0a2846ea01e291cfb57feb Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 4 Jan 2024 00:38:06 -0800 Subject: [PATCH 09/25] fix: add new macaddressEqualCheck func --- cns/restserver/ipam.go | 1 + cns/wireserver/net.go | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 1feb9d4c6f..cd30045c78 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -39,6 +39,7 @@ var ( // requestIPConfigHandlerHelper validates the request, assign IPs and return the IPConfigs func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + // fetch config to check swiftV2ServiceFabric scenario & fetch podIPInfo from GetNetworkContainer API if swiftv2SF cmdLineConfigPath := common.GetArg(common.OptCNSConfigPath).(string) cnsconfig, err := configuration.ReadConfig(cmdLineConfigPath) var swiftv2sf bool diff --git a/cns/wireserver/net.go b/cns/wireserver/net.go index 7310afce75..5fa71b52da 100644 --- a/cns/wireserver/net.go +++ b/cns/wireserver/net.go @@ -2,7 +2,6 @@ package wireserver import ( "net" - "regexp" "strings" "github.com/pkg/errors" @@ -66,8 +65,7 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string continue } - newMacAddress := regexp.MustCompile(`[^a-zA-Z0-9 ]+`).ReplaceAllString(macAddress, "") - if strings.EqualFold(i.MacAddress, newMacAddress) { + if macAddressesEqual(i.MacAddress, macAddress) { // get the second subnet s := i.IPSubnet[0] gw, err := calculateGatewayIP(s.Prefix) @@ -125,3 +123,10 @@ func calculateGatewayIP(cidr string) (net.IP, error) { } return gw, nil } + +func macAddressesEqual(macAddress1, macAddress2 string) bool { + macAddress1 = strings.ToLower(strings.ReplaceAll(macAddress1, ":", "")) + macAddress2 = strings.ToLower(strings.ReplaceAll(macAddress2, ":", "")) + + return macAddress1 == macAddress2 +} From d0edcb2715b4ce9d322ad24197a03947d3e18e49 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 23 Jan 2024 11:52:16 -0800 Subject: [PATCH 10/25] feat: adding swiftv2-SF middleware, refactor --- cns/NetworkContainerContract.go | 13 ++- cns/middlewares/SFSwiftV2.go | 125 +++++++++++++++++++++ cns/restserver/ipam.go | 189 ++++++-------------------------- cns/restserver/util.go | 4 +- cns/service/main.go | 5 +- 5 files changed, 170 insertions(+), 166 deletions(-) create mode 100644 cns/middlewares/SFSwiftV2.go diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index fc7fe16f1a..801b1210f1 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -486,12 +486,13 @@ type IPConfigRequest struct { // Same as IPConfigRequest except that DesiredIPAddresses is passed in as a slice type IPConfigsRequest struct { - DesiredIPAddresses []string `json:"desiredIPAddresses"` - PodInterfaceID string `json:"podInterfaceID"` - InfraContainerID string `json:"infraContainerID"` - OrchestratorContext json.RawMessage `json:"orchestratorContext"` - Ifname string `json:"ifname"` // Used by delegated IPAM - SecondaryInterfacesExist bool `json:"secondaryInterfacesExist"` // will be set by SWIFT v2 validator func + DesiredIPAddresses []string `json:"desiredIPAddresses"` + PodInterfaceID string `json:"podInterfaceID"` + InfraContainerID string `json:"infraContainerID"` + OrchestratorContext json.RawMessage `json:"orchestratorContext"` + Ifname string `json:"ifname"` // Used by delegated IPAM + SecondaryInterfacesExist bool `json:"secondaryInterfacesExist"` // will be set by SWIFT v2 validator func + AddInterfacesDataToResponse bool `json:"addInterfacesDataToResponse"` // used by SF Swiftv2 middleware to externally add interfaces using restserver methods } // IPConfigResponse is used in CNS IPAM mode as a response to CNI ADD diff --git a/cns/middlewares/SFSwiftV2.go b/cns/middlewares/SFSwiftV2.go new file mode 100644 index 0000000000..aee1ed00c0 --- /dev/null +++ b/cns/middlewares/SFSwiftV2.go @@ -0,0 +1,125 @@ +package middlewares + +import ( + "context" + "fmt" + "github.com/Azure/azure-container-networking/cns" + cnsclient "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/types" + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + "time" +) + +const ( + contentTypeJSON = "application/json" + headerContentType = "Content-Type" + cnsReqTimeout = 15 * time.Second +) + +type SFSWIFTv2Middleware struct { + Cli client.Client +} + +// Verify interface compliance at compile time +var _ cns.IPConfigsHandlerMiddleware = (*SFSWIFTv2Middleware)(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. +func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { + return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, respCode, message := m.validateIPConfigsRequest(ctx, &req) + + if respCode != types.Success { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: respCode, + Message: message, + }, + }, errors.New("failed to validate ip configs request") + } + ipConfigsResp, err := defaultHandler(ctx, req) + // If the pod is not v2, return the response from the handler + if !req.SecondaryInterfacesExist { + return ipConfigsResp, err + } + // 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 + if err != nil { + _, err = failureHandler(ctx, req) + if err != nil { + logger.Errorf("failed to release default IP config : %v", err) + } + } + }() + if err != nil { + return ipConfigsResp, err + } + SWIFTv2PodIPInfo, err := m.getIPConfig(ctx, podInfo) + if err != nil { + return &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.FailedToAllocateIPConfig, + Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, req), + }, + PodIPInfo: []cns.PodIpInfo{}, + }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", req) + } + ipConfigsResp.PodIPInfo = append(ipConfigsResp.PodIPInfo, SWIFTv2PodIPInfo) + return ipConfigsResp, nil + } +} + +// validateIPConfigsRequest validates if pod orchestrator context is unmarshalled & sets secondary interfaces true +// nolint +func (m *SFSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string) { + // Retrieve the pod from the cluster + podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) + if err != nil { + errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req) + return nil, types.UnexpectedError, errBuf.Error() + } + logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name()) + + req.SecondaryInterfacesExist = true + // swiftv2 SF scenario for windows requires host interface info to be populated + // which can be done only by restserver service function, hence setting this flag to do so in ipam.go + req.AddInterfacesDataToResponse = true + + logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) + // retrieve podinfo from orchestrator context + return podInfo, types.Success, "" +} + +// getIPConfig returns the pod's SWIFT V2 IP configuration. +func (m *SFSWIFTv2Middleware) getIPConfig(ctx context.Context, podInfo cns.PodInfo) (cns.PodIpInfo, error) { + orchestratorContext, err := podInfo.OrchestratorContext() + if err != nil { + return cns.PodIpInfo{}, fmt.Errorf("error getting orchestrator context from PodInfo %v", err) + } + cnsclient, err := cnsclient.New("", cnsReqTimeout) + if err != nil { + return cns.PodIpInfo{}, fmt.Errorf("error initializing cnsclient %v", err) + } + resp, err := cnsclient.GetNetworkContainer(ctx, orchestratorContext) + if err != nil { + return cns.PodIpInfo{}, fmt.Errorf("error getNetworkContainerByOrchestrator Context %v", err) + } + + // Check if the ncstate/ipconfig ready. If one of the fields is empty, return error + if resp.IPConfiguration.IPSubnet.IPAddress == "" || resp.NetworkInterfaceInfo.MACAddress == "" || resp.NetworkContainerID == "" || resp.IPConfiguration.GatewayIPAddress == "" { + return cns.PodIpInfo{}, fmt.Errorf("one of the fields for GetNCResponse is empty for given NC: %+v", resp) //nolint:goerr113 // return error + } + logger.Debugf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) + + podIPInfo := cns.PodIpInfo{ + PodIPConfig: resp.IPConfiguration.IPSubnet, + MacAddress: resp.NetworkInterfaceInfo.MACAddress, + NICType: resp.NetworkInterfaceInfo.NICType, + SkipDefaultRoutes: false, + NetworkContainerPrimaryIPConfig: resp.IPConfiguration, + } + return podIPInfo, nil +} diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index cd30045c78..625915a4c2 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -4,18 +4,14 @@ package restserver import ( - "bytes" "context" - "encoding/json" "fmt" "net" "net/http" - "net/http/httptest" "strconv" "strings" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/filter" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" @@ -24,11 +20,6 @@ import ( "github.com/pkg/errors" ) -const ( - contentTypeJSON = "application/json" - headerContentType = "Content-Type" -) - var ( ErrStoreEmpty = errors.New("empty endpoint state store") ErrParsePodIPFailed = errors.New("failed to parse pod's ip") @@ -39,14 +30,7 @@ var ( // requestIPConfigHandlerHelper validates the request, assign IPs and return the IPConfigs func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - // fetch config to check swiftV2ServiceFabric scenario & fetch podIPInfo from GetNetworkContainer API if swiftv2SF - cmdLineConfigPath := common.GetArg(common.OptCNSConfigPath).(string) - cnsconfig, err := configuration.ReadConfig(cmdLineConfigPath) - var swiftv2sf bool - if cnsconfig.EnableSwiftV2 == configuration.EnableSFSwiftV2 { - swiftv2sf = true - } - // For only SWIFTV2-AKS scenario, the validator function will also modify the ipconfigsRequest. + // For SWIFT v2 scenario, the validator function will also modify the ipconfigsRequest. podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ @@ -56,15 +40,9 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }, }, errors.New("failed to validate ip config request") } - // record a pod requesting an IP service.podsPendingIPAssignment.Push(podInfo.Key()) - var podIPInfo []cns.PodIpInfo - // request IPs from pod state only if scenario for swiftv2 is non-Standalone i.e. AKS - if !swiftv2sf { - podIPInfo, err = requestIPConfigsHelper(service, ipconfigsRequest) //nolint:contextcheck // appease linter - } - + podIPInfo, err := requestIPConfigsHelper(service, ipconfigsRequest) if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -74,7 +52,6 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context PodIPInfo: podIPInfo, }, err } - // record a pod assigned an IP defer func() { // observe IP assignment wait time @@ -82,7 +59,6 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context ipAssignmentLatency.Observe(since.Seconds()) } }() - // Check if http rest service managed endpoint state is set if service.Options[common.OptManageEndpointState] == true { err = service.updateEndpointState(ipconfigsRequest, podInfo, podIPInfo) @@ -96,73 +72,6 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }, err } } - - // Follow AKS swiftv2 podinfo path if secondary interface(s) exist - if podInfo.SecondaryInterfacesExist() && !swiftv2sf { - // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here - SWIFTv2PodIPInfo, err := service.SWIFTv2Middleware.GetIPConfig(ctx, podInfo) - if err != nil { - defer func() { - logger.Errorf("failed to get SWIFTv2 IP config : %v. Releasing default IP config...", err) - _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) - if err != nil { - logger.Errorf("failed to release default IP config : %v", err) - } - }() - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: types.FailedToAllocateIPConfig, - Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, ipconfigsRequest), - }, - PodIPInfo: []cns.PodIpInfo{}, - }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", ipconfigsRequest) - } - podIPInfo = append(podIPInfo, SWIFTv2PodIPInfo) - // Setting up routes for SWIFTv2 scenario - for i := range podIPInfo { - ipInfo := &podIPInfo[i] - err := service.SWIFTv2Middleware.SetRoutes(ipInfo) - if err != nil { - defer func() { //nolint:gocritic - logger.Errorf("failed to set routes for SWIFTv2 IP config : %v. Releasing default IP config...", err) - _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) - if err != nil { - logger.Errorf("failed to release default IP config : %v", err) - } - }() - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: types.UnexpectedError, - Message: fmt.Sprintf("failed to set SWIFTv2 routes : %v", err), - }, - PodIPInfo: []cns.PodIpInfo{}, - }, errors.Wrapf(err, "failed to set SWIFTv2 routes : %v", ipconfigsRequest) - } - } - } - // Check if request is for pod with secondary interface(s) - if swiftv2sf { - // In the future, if we have multiple scenario with secondary interfaces, we can add a switch case here - SWIFTv2PodIPInfo, err := service.getIPConfigforSwiftV2SF(podInfo) //nolint:contextcheck // appease linter - if err != nil { - defer func() { - logger.Errorf("failed to get SWIFTv2 IP config : %v. Releasing default IP config...", err) - _, err = service.releaseIPConfigHandlerHelper(ctx, ipconfigsRequest) //nolint:contextcheck // appease linter - if err != nil { - logger.Errorf("failed to release default IP config : %v", err) - } - }() - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: types.FailedToAllocateIPConfig, - Message: fmt.Sprintf("AllocateIPConfig failed: %v, IP config request is %v", err, ipconfigsRequest), - }, - PodIPInfo: []cns.PodIpInfo{}, - }, errors.Wrapf(err, "failed to get SWIFTv2 IP config : %v", ipconfigsRequest) - } - podIPInfo = append(podIPInfo, SWIFTv2PodIPInfo) - } - return &cns.IPConfigsResponse{ Response: cns.Response{ ReturnCode: types.Success, @@ -171,69 +80,6 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }, nil } -// getIPConfig PodIpInfo for swiftv2 SF scenario, gets info from NC goal state -func (service *HTTPRestService) getIPConfigforSwiftV2SF(podInfo cns.PodInfo) (cns.PodIpInfo, error) { - var body bytes.Buffer - var resp cns.GetNetworkContainerResponse - podInfoBytes, err := json.Marshal(podInfo) - if err != nil { - return cns.PodIpInfo{}, errors.Wrap(err, "failed to marshal podinfo from request") - } - payload := &cns.GetNetworkContainerRequest{OrchestratorContext: podInfoBytes} - - if err = json.NewEncoder(&body).Encode(payload); err != nil { - return cns.PodIpInfo{}, errors.Wrap(err, "failed to encode GetNetworkContainerRequest OrchestratorContext") - } - - req, err := http.NewRequestWithContext(context.TODO(), http.MethodPost, cns.GetNetworkContainerByOrchestratorContext, &body) - if err != nil { - return cns.PodIpInfo{}, errors.Wrap(err, "failed to build request GetNetworkContainerByOrchestratorContext") - } - req.Header.Set(headerContentType, contentTypeJSON) - w := httptest.NewRecorder() - service.getNetworkContainerByOrchestratorContext(w, req) - - err = json.NewDecoder(w.Body).Decode(&resp) - if err != nil { - return cns.PodIpInfo{}, fmt.Errorf("decoding response: %w", err) - } - - // Check if the ncstate/ipconfig ready. If one of the fields is empty, return error - if resp.IPConfiguration.IPSubnet.IPAddress == "" || resp.NetworkInterfaceInfo.MACAddress == "" || resp.NetworkContainerID == "" || resp.IPConfiguration.GatewayIPAddress == "" { - return cns.PodIpInfo{}, fmt.Errorf("one of the fields for GetNCResponse is empty for given NC: %+v", resp) //nolint:goerr113 // return error - } - logger.Debugf("[SWIFTv2-SF] NetworkContainerResponse for pod %s is : %+v", podInfo.Name(), resp) - - hostPrimaryInterface, err := service.getPrimaryHostInterface(context.TODO()) - if err != nil { - return cns.PodIpInfo{}, err - } - - hostSecondaryInterface, err := service.getSecondaryHostInterface(context.TODO(), resp.NetworkInterfaceInfo.MACAddress) - if err != nil { - return cns.PodIpInfo{}, err - } - - podIPInfo := cns.PodIpInfo{ - PodIPConfig: resp.IPConfiguration.IPSubnet, - MacAddress: resp.NetworkInterfaceInfo.MACAddress, - NICType: resp.NetworkInterfaceInfo.NICType, - SkipDefaultRoutes: false, - HostPrimaryIPInfo: cns.HostIPInfo{ - Gateway: hostPrimaryInterface.Gateway, - PrimaryIP: hostPrimaryInterface.PrimaryIP, - Subnet: hostPrimaryInterface.Subnet, - }, - HostSecondaryIPInfo: cns.HostIPInfo{ - Gateway: hostSecondaryInterface.Gateway, - SecondaryIP: hostSecondaryInterface.SecondaryIPs[0], - Subnet: hostSecondaryInterface.Subnet, - }, - NetworkContainerPrimaryIPConfig: resp.IPConfiguration, - } - return podIPInfo, nil -} - // requestIPConfigHandler requests an IPConfig from the CNS state func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r *http.Request) { var ipconfigRequest cns.IPConfigRequest @@ -325,6 +171,9 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r // Wrap the default datapath handlers with the middleware wrappedHandler := service.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(service.requestIPConfigHandlerHelper, service.releaseIPConfigHandlerHelper) ipConfigsResp, err = wrappedHandler(r.Context(), ipconfigsRequest) + if ipconfigsRequest.AddInterfacesDataToResponse { + ipConfigsResp, err = service.updatePodInfoWithInterfaces(ipConfigsResp) + } } else { ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) // nolint:contextcheck // appease linter } @@ -341,6 +190,34 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name+operationName, ipconfigsRequest, ipConfigsResp, ipConfigsResp.Response.ReturnCode, err) } +func (service *HTTPRestService) updatePodInfoWithInterfaces(ipconfigResponse *cns.IPConfigsResponse) (*cns.IPConfigsResponse, error) { + for _, podIpInfo := range ipconfigResponse.PodIPInfo { + hostPrimaryInterface, err := service.getPrimaryHostInterface(context.TODO()) + if err != nil { + return &cns.IPConfigsResponse{}, err + } + + hostSecondaryInterface, err := service.getSecondaryHostInterface(context.TODO(), podIpInfo.MacAddress) + if err != nil { + return &cns.IPConfigsResponse{}, err + } + + podIpInfo.HostPrimaryIPInfo = + cns.HostIPInfo{ + Gateway: hostPrimaryInterface.Gateway, + PrimaryIP: hostPrimaryInterface.PrimaryIP, + Subnet: hostPrimaryInterface.Subnet, + } + podIpInfo.HostSecondaryIPInfo = + cns.HostIPInfo{ + Gateway: hostSecondaryInterface.Gateway, + SecondaryIP: hostSecondaryInterface.SecondaryIPs[0], + Subnet: hostSecondaryInterface.Subnet, + } + } + + return ipconfigResponse, nil +} func (service *HTTPRestService) updateEndpointState(ipconfigsRequest cns.IPConfigsRequest, podInfo cns.PodInfo, podIPInfo []cns.PodIpInfo) error { if service.EndpointStateStore == nil { return ErrStoreEmpty diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 6063ed16dc..6c90000587 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -756,12 +756,12 @@ func (service *HTTPRestService) SendNCSnapShotPeriodically(ctx context.Context, } func (service *HTTPRestService) validateIPConfigsRequest(ctx context.Context, ipConfigsRequest cns.IPConfigsRequest) (cns.PodInfo, types.ResponseCode, string) { - if service.state.OrchestratorType != cns.KubernetesCRD && service.state.OrchestratorType != cns.Kubernetes { + if service.state.OrchestratorType != cns.KubernetesCRD && service.state.OrchestratorType != cns.Kubernetes && service.state.OrchestratorType != cns.ServiceFabric { return nil, types.UnsupportedOrchestratorType, "ReleaseIPConfig API supported only for kubernetes orchestrator" } if ipConfigsRequest.OrchestratorContext == nil { - return nil, types.EmptyOrchestratorContext, fmt.Sprintf("OrchastratorContext is not set in the req: %+v", ipConfigsRequest) + return nil, types.EmptyOrchestratorContext, fmt.Sprintf("OrchestratorContext is not set in the req: %+v", ipConfigsRequest) } // retrieve podinfo from orchestrator context diff --git a/cns/service/main.go b/cns/service/main.go index 5dfc9e60b1..67f0e8da59 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1213,7 +1213,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn // check the Node labels for Swift V2 if _, ok := node.Labels[configuration.LabelNodeSwiftV2]; ok { - cnsconfig.EnableSwiftV2 = configuration.EnableK8SwiftV2 + cnsconfig.EnableSwiftV2 = true cnsconfig.WatchPods = true // TODO(rbtr): create the NodeInfo for Swift V2 // register the noop mtpnc reconciler to populate the cache @@ -1376,7 +1376,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } } - if cnsconfig.EnableSwiftV2 == configuration.EnableK8SwiftV2 { + if cnsconfig.EnableSwiftV2 { if err := mtpncctrl.SetupWithManager(manager); err != nil { return errors.Wrapf(err, "failed to setup mtpnc reconciler with manager") } @@ -1387,6 +1387,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn case configuration.K8sSWIFTV2: swiftV2Middleware = &middlewares.K8sSWIFTv2Middleware{Cli: manager.GetClient()} case configuration.SFSWIFTV2: + swiftV2Middleware = &middlewares.SFSWIFTv2Middleware{Cli: manager.GetClient()} default: // default to K8s middleware for now, in a later changes we where start to pass in // SWIFT v2 mode in CNS config, this should throw an error if the mode is not set. From 91bc874e76878c453ae5248dc6ed1b6a2e284177 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Wed, 24 Jan 2024 22:56:04 -0800 Subject: [PATCH 11/25] review: address comments --- cns/configuration/configuration.go | 2 -- cns/middlewares/SFSwiftV2.go | 2 +- cns/restserver/ipam.go | 8 ++++---- cns/wireserver/net.go | 1 + 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 91942ce47e..005ed3f83b 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -57,8 +57,6 @@ type CNSConfig struct { AsyncPodDeletePath string } -type EnableSwiftV2Mode string - type TelemetrySettings struct { // Flag to disable the telemetry. DisableAll bool diff --git a/cns/middlewares/SFSwiftV2.go b/cns/middlewares/SFSwiftV2.go index aee1ed00c0..425c9380e9 100644 --- a/cns/middlewares/SFSwiftV2.go +++ b/cns/middlewares/SFSwiftV2.go @@ -25,7 +25,7 @@ type SFSWIFTv2Middleware struct { // Verify interface compliance at compile time var _ cns.IPConfigsHandlerMiddleware = (*SFSWIFTv2Middleware)(nil) -// IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request +// IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP config requests for SF standalone scenario. This function wraps the default SWIFT request // and release IP configs handlers. func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 625915a4c2..96b43f3952 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -172,7 +172,7 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r wrappedHandler := service.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(service.requestIPConfigHandlerHelper, service.releaseIPConfigHandlerHelper) ipConfigsResp, err = wrappedHandler(r.Context(), ipconfigsRequest) if ipconfigsRequest.AddInterfacesDataToResponse { - ipConfigsResp, err = service.updatePodInfoWithInterfaces(ipConfigsResp) + ipConfigsResp, err = service.updatePodInfoWithInterfaces(r.Context(), ipConfigsResp) } } else { ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) // nolint:contextcheck // appease linter @@ -190,14 +190,14 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r logger.ResponseEx(service.Name+operationName, ipconfigsRequest, ipConfigsResp, ipConfigsResp.Response.ReturnCode, err) } -func (service *HTTPRestService) updatePodInfoWithInterfaces(ipconfigResponse *cns.IPConfigsResponse) (*cns.IPConfigsResponse, error) { +func (service *HTTPRestService) updatePodInfoWithInterfaces(ctx context.Context, ipconfigResponse *cns.IPConfigsResponse) (*cns.IPConfigsResponse, error) { for _, podIpInfo := range ipconfigResponse.PodIPInfo { - hostPrimaryInterface, err := service.getPrimaryHostInterface(context.TODO()) + hostPrimaryInterface, err := service.getPrimaryHostInterface(ctx) if err != nil { return &cns.IPConfigsResponse{}, err } - hostSecondaryInterface, err := service.getSecondaryHostInterface(context.TODO(), podIpInfo.MacAddress) + hostSecondaryInterface, err := service.getSecondaryHostInterface(ctx, podIpInfo.MacAddress) if err != nil { return &cns.IPConfigsResponse{}, err } diff --git a/cns/wireserver/net.go b/cns/wireserver/net.go index 5fa71b52da..cb7c0cd3eb 100644 --- a/cns/wireserver/net.go +++ b/cns/wireserver/net.go @@ -77,6 +77,7 @@ func GetSecondaryInterfaceFromResult(res *GetInterfacesResult, macAddress string for _, ip := range s.IPAddress { if !ip.IsPrimary { secondaryIP = ip.Address + break } } var secondaryIPs []string From 8776afe4205973398ff7cd11ce107bd7a4bb871c Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 25 Jan 2024 00:47:39 -0800 Subject: [PATCH 12/25] review: address comments-2 --- cns/api.go | 2 +- cns/middlewares/SFSwiftV2.go | 14 +++++++------- cns/middlewares/k8sSwiftV2.go | 4 ++-- cns/middlewares/k8sSwiftV2_test.go | 14 +++++++------- cns/restserver/ipam.go | 22 +++++++++++----------- cns/restserver/ipam_test.go | 8 ++++---- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/cns/api.go b/cns/api.go index 9252825c25..fadd69151c 100644 --- a/cns/api.go +++ b/cns/api.go @@ -53,7 +53,7 @@ type HTTPService interface { } // IPConfigsHandlerFunc -type IPConfigsHandlerFunc func(context.Context, IPConfigsRequest) (*IPConfigsResponse, error) +type IPConfigsHandlerFunc func(context.Context, *IPConfigsRequest) (*IPConfigsResponse, error) // IPConfigsHandlerMiddleware type IPConfigsHandlerMiddleware interface { diff --git a/cns/middlewares/SFSwiftV2.go b/cns/middlewares/SFSwiftV2.go index 425c9380e9..9543da47c0 100644 --- a/cns/middlewares/SFSwiftV2.go +++ b/cns/middlewares/SFSwiftV2.go @@ -28,8 +28,8 @@ var _ cns.IPConfigsHandlerMiddleware = (*SFSWIFTv2Middleware)(nil) // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP config requests for SF standalone scenario. This function wraps the default SWIFT request // and release IP configs handlers. func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { - return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message := m.validateIPConfigsRequest(ctx, &req) + return func(ctx context.Context, req *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, respCode, message := m.validateIPConfigsRequest(ctx, req) if respCode != types.Success { return &cns.IPConfigsResponse{ @@ -97,15 +97,15 @@ func (m *SFSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req func (m *SFSWIFTv2Middleware) getIPConfig(ctx context.Context, podInfo cns.PodInfo) (cns.PodIpInfo, error) { orchestratorContext, err := podInfo.OrchestratorContext() if err != nil { - return cns.PodIpInfo{}, fmt.Errorf("error getting orchestrator context from PodInfo %v", err) + return cns.PodIpInfo{}, fmt.Errorf("error getting orchestrator context from PodInfo %w", err) } - cnsclient, err := cnsclient.New("", cnsReqTimeout) + cnsClient, err := cnsclient.New("", cnsReqTimeout) if err != nil { - return cns.PodIpInfo{}, fmt.Errorf("error initializing cnsclient %v", err) + return cns.PodIpInfo{}, fmt.Errorf("error initializing cnsclient %w", err) } - resp, err := cnsclient.GetNetworkContainer(ctx, orchestratorContext) + resp, err := cnsClient.GetNetworkContainer(ctx, orchestratorContext) if err != nil { - return cns.PodIpInfo{}, fmt.Errorf("error getNetworkContainerByOrchestrator Context %v", err) + return cns.PodIpInfo{}, fmt.Errorf("error getNetworkContainerByOrchestrator Context %w", err) } // Check if the ncstate/ipconfig ready. If one of the fields is empty, return error diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/middlewares/k8sSwiftV2.go index a9721c3995..423bd0c16a 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/middlewares/k8sSwiftV2.go @@ -40,8 +40,8 @@ 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. func (m *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { - return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message := m.validateIPConfigsRequest(ctx, &req) + return func(ctx context.Context, req *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, respCode, message := m.validateIPConfigsRequest(ctx, req) if respCode != types.Success { return &cns.IPConfigsResponse{ diff --git a/cns/middlewares/k8sSwiftV2_test.go b/cns/middlewares/k8sSwiftV2_test.go index 856c4cd521..b30c7865e2 100644 --- a/cns/middlewares/k8sSwiftV2_test.go +++ b/cns/middlewares/k8sSwiftV2_test.go @@ -37,7 +37,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { t.Setenv(configuration.EnvPodCIDRs, "10.0.1.10/24,16A0:0010:AB00:001E::2/32") t.Setenv(configuration.EnvServiceCIDRs, "10.0.0.0/16,16A0:0010:AB00:0000::/32") t.Setenv(configuration.EnvInfraVNETCIDRs, "10.240.0.1/16,16A0:0020:AB00:0000::/32") - defaultHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + defaultHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return &cns.IPConfigsResponse{ PodIPInfo: []cns.PodIpInfo{ { @@ -57,7 +57,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { }, }, nil } - failureHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + failureHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return nil, nil } wrappedHandler := middleware.IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler) @@ -67,7 +67,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { } b, _ := testPod1Info.OrchestratorContext() happyReq.OrchestratorContext = b - resp, err := wrappedHandler(context.TODO(), happyReq) + resp, err := wrappedHandler(context.TODO(), &happyReq) assert.Equal(t, err, nil) assert.Equal(t, resp.PodIPInfo[2].PodIPConfig.IPAddress, "192.168.0.1") assert.Equal(t, resp.PodIPInfo[2].MacAddress, "00:00:00:00:00:00") @@ -75,7 +75,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} - defaultHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + defaultHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return &cns.IPConfigsResponse{ PodIPInfo: []cns.PodIpInfo{ { @@ -95,7 +95,7 @@ func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { }, }, nil } - failureHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + failureHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return nil, nil } wrappedHandler := middleware.IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler) @@ -106,7 +106,7 @@ func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { } b, _ := testPod4Info.OrchestratorContext() failReq.OrchestratorContext = b - resp, _ := wrappedHandler(context.TODO(), failReq) + resp, _ := wrappedHandler(context.TODO(), &failReq) assert.Equal(t, resp.Response.Message, errMTPNCNotReady.Error()) // Failed to set routes @@ -116,7 +116,7 @@ func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { } b, _ = testPod1Info.OrchestratorContext() failReq.OrchestratorContext = b - _, err := wrappedHandler(context.TODO(), failReq) + _, err := wrappedHandler(context.TODO(), &failReq) assert.ErrorContains(t, err, "failed to set routes for pod") } diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 96b43f3952..9869c210ae 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -29,9 +29,9 @@ var ( ) // requestIPConfigHandlerHelper validates the request, assign IPs and return the IPConfigs -func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { +func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { // For SWIFT v2 scenario, the validator function will also modify the ipconfigsRequest. - podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) + podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, *ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -42,7 +42,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context } // record a pod requesting an IP service.podsPendingIPAssignment.Push(podInfo.Key()) - podIPInfo, err := requestIPConfigsHelper(service, ipconfigsRequest) + podIPInfo, err := requestIPConfigsHelper(service, *ipconfigsRequest) if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -61,7 +61,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }() // Check if http rest service managed endpoint state is set if service.Options[common.OptManageEndpointState] == true { - err = service.updateEndpointState(ipconfigsRequest, podInfo, podIPInfo) + err = service.updateEndpointState(*ipconfigsRequest, podInfo, podIPInfo) if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -118,7 +118,7 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r } } - ipConfigsResp, errResp := service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) //nolint:contextcheck // appease linter + ipConfigsResp, errResp := service.requestIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) //nolint:contextcheck // appease linter if errResp != nil { // As this API is expected to return IPConfigResponse, generate it from the IPConfigsResponse returned above reserveResp := &cns.IPConfigResponse{ @@ -170,12 +170,12 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r if service.IPConfigsHandlerMiddleware != nil { // Wrap the default datapath handlers with the middleware wrappedHandler := service.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(service.requestIPConfigHandlerHelper, service.releaseIPConfigHandlerHelper) - ipConfigsResp, err = wrappedHandler(r.Context(), ipconfigsRequest) + ipConfigsResp, err = wrappedHandler(r.Context(), &ipconfigsRequest) if ipconfigsRequest.AddInterfacesDataToResponse { ipConfigsResp, err = service.updatePodInfoWithInterfaces(r.Context(), ipConfigsResp) } } else { - ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) // nolint:contextcheck // appease linter + ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) // nolint:contextcheck // appease linter } if err != nil { @@ -281,8 +281,8 @@ func (service *HTTPRestService) updateEndpointState(ipconfigsRequest cns.IPConfi } // releaseIPConfigHandlerHelper validates the request and removes the endpoint associated with the pod -func (service *HTTPRestService) releaseIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) +func (service *HTTPRestService) releaseIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, *ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -362,7 +362,7 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r Ifname: ipconfigRequest.Ifname, } - resp, err := service.releaseIPConfigHandlerHelper(r.Context(), ipconfigsRequest) + resp, err := service.releaseIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) if err != nil { w.Header().Set(cnsReturnCode, resp.Response.ReturnCode.String()) err = service.Listener.Encode(w, &resp) @@ -391,7 +391,7 @@ func (service *HTTPRestService) releaseIPConfigsHandler(w http.ResponseWriter, r return } - resp, err := service.releaseIPConfigHandlerHelper(r.Context(), ipconfigsRequest) + resp, err := service.releaseIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) if err != nil { w.Header().Set(cnsReturnCode, resp.Response.ReturnCode.String()) err = service.Listener.Encode(w, &resp) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 1c545bc5da..04a4cea078 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -1579,7 +1579,7 @@ func TestIPAMReleaseSWIFTV2PodIPSuccess(t *testing.T) { req.DesiredIPAddresses[0] = testIP1 req.DesiredIPAddresses[1] = testIP1v6 // Requesting release ip config for SWIFT V2 pod when mtpnc is not ready, should be a no-op - _, err := svc.releaseIPConfigHandlerHelper(context.TODO(), req) + _, err := svc.releaseIPConfigHandlerHelper(context.TODO(), &req) if err != nil { t.Fatalf("Expected not to fail when requesting to release SWIFT V2 pod due to MTPNC not ready") } @@ -1631,7 +1631,7 @@ func TestIPAMGetK8sSWIFTv2IPSuccess(t *testing.T) { req.DesiredIPAddresses[1] = testIP1v6 wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) - resp, err := wrappedHandler(context.TODO(), req) + resp, err := wrappedHandler(context.TODO(), &req) if err != nil { t.Fatalf("Expected to not fail requesting IPs: %+v", err) } @@ -1687,7 +1687,7 @@ func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { req.DesiredIPAddresses[0] = testIP1 req.DesiredIPAddresses[1] = testIP1v6 wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) - _, err := wrappedHandler(context.TODO(), req) + _, err := wrappedHandler(context.TODO(), &req) if err == nil { t.Fatalf("Expected failing requesting IPs due to MTPNC not ready") } @@ -1707,7 +1707,7 @@ func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { req.DesiredIPAddresses[0] = testIP1 req.DesiredIPAddresses[1] = testIP1v6 - _, err = wrappedHandler(context.TODO(), req) + _, err = wrappedHandler(context.TODO(), &req) if err == nil { t.Fatalf("Expected failing requesting IPs due to not able to set routes") } From f457f6ad935f6fd8b2b74a50fe960c4dc2dd2da2 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 25 Jan 2024 14:04:50 -0800 Subject: [PATCH 13/25] feat: change middleware initialization at right cns channel mode --- cns/middlewares/SFSwiftV2.go | 2 -- cns/service/main.go | 24 ++++++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/cns/middlewares/SFSwiftV2.go b/cns/middlewares/SFSwiftV2.go index 9543da47c0..22ac14b2df 100644 --- a/cns/middlewares/SFSwiftV2.go +++ b/cns/middlewares/SFSwiftV2.go @@ -8,7 +8,6 @@ import ( "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/pkg/errors" - "sigs.k8s.io/controller-runtime/pkg/client" "time" ) @@ -19,7 +18,6 @@ const ( ) type SFSWIFTv2Middleware struct { - Cli client.Client } // Verify interface compliance at compile time diff --git a/cns/service/main.go b/cns/service/main.go index 67f0e8da59..df98da2041 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -789,6 +789,13 @@ func main() { if platform.HasMellanoxAdapter() { go platform.MonitorAndSetMellanoxRegKeyPriorityVLANTag(rootCtx, cnsconfig.MellanoxMonitorIntervalSecs) } + // if swiftv2 scenario is enabled, we need to initialize the Service Fabric (standalone) swiftv2 middleware to process IP configs requests + if cnsconfig.EnableSwiftV2 { + if cnsconfig.SWIFTV2Mode == configuration.SFSWIFTV2 { + swiftV2Middleware := &middlewares.SFSWIFTv2Middleware{} + httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) + } + } } // Initialze state in if CNS is running in CRD mode @@ -1381,19 +1388,12 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn return errors.Wrapf(err, "failed to setup mtpnc reconciler with manager") } // if SWIFT v2 is enabled on CNS, attach multitenant middleware to rest service - // switch here for different type of swift v2 middleware (k8s or SF) - var swiftV2Middleware cns.IPConfigsHandlerMiddleware - switch cnsconfig.SWIFTV2Mode { - case configuration.K8sSWIFTV2: - swiftV2Middleware = &middlewares.K8sSWIFTv2Middleware{Cli: manager.GetClient()} - case configuration.SFSWIFTV2: - swiftV2Middleware = &middlewares.SFSWIFTv2Middleware{Cli: manager.GetClient()} - default: - // default to K8s middleware for now, in a later changes we where start to pass in - // SWIFT v2 mode in CNS config, this should throw an error if the mode is not set. - swiftV2Middleware = &middlewares.K8sSWIFTv2Middleware{Cli: manager.GetClient()} + // here for AKS(K8s) swiftv2 middleware to process IP configs requests + if cnsconfig.SWIFTV2Mode == configuration.K8sSWIFTV2 { + swiftV2Middleware := &middlewares.K8sSWIFTv2Middleware{Cli: manager.GetClient()} + httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) } - httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) + } // start the pool Monitor before the Reconciler, since it needs to be ready to receive an From a027e77e7fc1b3cf6a7891043f3cd9e48920a354 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 30 Jan 2024 12:58:23 -0800 Subject: [PATCH 14/25] test:UTs to validate IP configs request, refactor cnsclient import, api.go new packages, remove cnsclient dependency on restserver package --- cns/api.go | 31 +++ cns/client/client.go | 9 +- cns/client/client_test.go | 12 +- cns/{middlewares => restserver}/SFSwiftV2.go | 33 ++- cns/restserver/SFSwiftV2_test.go | 52 +++++ cns/restserver/ipam.go | 5 +- cns/restserver/ipam_test.go | 199 +++++++++++++++++- cns/{middlewares => restserver}/k8sSwiftV2.go | 2 +- .../k8sSwiftV2_test.go | 18 +- cns/restserver/restserver.go | 12 -- cns/service/main.go | 23 +- 11 files changed, 308 insertions(+), 88 deletions(-) rename cns/{middlewares => restserver}/SFSwiftV2.go (85%) create mode 100644 cns/restserver/SFSwiftV2_test.go rename cns/{middlewares => restserver}/k8sSwiftV2.go (99%) rename cns/{middlewares => restserver}/k8sSwiftV2_test.go (93%) diff --git a/cns/api.go b/cns/api.go index fadd69151c..e2bc534874 100644 --- a/cns/api.go +++ b/cns/api.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "net" "time" "github.com/Azure/azure-container-networking/cns/common" @@ -363,3 +364,33 @@ type EndpointRequest struct { HnsEndpointID string `json:"hnsEndpointID"` HostVethName string `json:"hostVethName"` } + +// GetEndpointResponse describes response from the The GetEndpoint API. +type GetEndpointResponse struct { + Response Response `json:"response"` + EndpointInfo EndpointInfo `json:"endpointInfo"` +} + +type EndpointInfo struct { + PodName string + PodNamespace string + IfnameToIPMap map[string]*IPInfo // key : interface name, value : IPInfo + HnsEndpointID string + HostVethName string +} +type IPInfo struct { + IPv4 []net.IPNet + IPv6 []net.IPNet +} + +type GetHTTPServiceDataResponse struct { + HTTPRestServiceData HTTPRestServiceData + Response Response +} + +// HTTPRestServiceData represents in-memory CNS data in the debug API paths. +type HTTPRestServiceData struct { + PodIPIDByPodInterfaceKey map[string][]string // PodInterfaceId is key and value is slice of Pod IP uuids. + PodIPConfigState map[string]IPConfigurationStatus // secondaryipid(uuid) is key + IPAMPoolMonitor IpamPoolMonitorStateSnapshot +} diff --git a/cns/client/client.go b/cns/client/client.go index 2ee524cfb9..35cc03603b 100644 --- a/cns/client/client.go +++ b/cns/client/client.go @@ -11,7 +11,6 @@ import ( "time" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/cns/types" "github.com/pkg/errors" ) @@ -563,7 +562,7 @@ func (c *Client) GetPodOrchestratorContext(ctx context.Context) (map[string][]st } // GetHTTPServiceData gets all public in-memory struct details for debugging purpose -func (c *Client) GetHTTPServiceData(ctx context.Context) (*restserver.GetHTTPServiceDataResponse, error) { +func (c *Client) GetHTTPServiceData(ctx context.Context) (*cns.GetHTTPServiceDataResponse, error) { u := c.routes[cns.PathDebugRestData] req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) if err != nil { @@ -583,7 +582,7 @@ func (c *Client) GetHTTPServiceData(ctx context.Context) (*restserver.GetHTTPSer if res.StatusCode != http.StatusOK { return nil, errors.Errorf("http response %d", res.StatusCode) } - var resp restserver.GetHTTPServiceDataResponse + var resp cns.GetHTTPServiceDataResponse err = json.NewDecoder(bytes.NewReader(b)).Decode(&resp) if err != nil { return nil, errors.Wrap(err, "failed to decode GetHTTPServiceDataResponse") @@ -1024,7 +1023,7 @@ func (c *Client) GetHomeAz(ctx context.Context) (*cns.GetHomeAzResponse, error) } // GetEndpoint calls the EndpointHandlerAPI in CNS to retrieve the state of a given EndpointID -func (c *Client) GetEndpoint(ctx context.Context, endpointID string) (*restserver.GetEndpointResponse, error) { +func (c *Client) GetEndpoint(ctx context.Context, endpointID string) (*cns.GetEndpointResponse, error) { // build the request u := c.routes[cns.EndpointAPI] uString := u.String() + endpointID @@ -1044,7 +1043,7 @@ func (c *Client) GetEndpoint(ctx context.Context, endpointID string) (*restserve return nil, errors.Errorf("http response %d", res.StatusCode) } - var response restserver.GetEndpointResponse + var response cns.GetEndpointResponse err = json.NewDecoder(res.Body).Decode(&response) if err != nil { return nil, errors.Wrap(err, "failed to decode GetEndpointResponse") diff --git a/cns/client/client_test.go b/cns/client/client_test.go index 871773eded..0b64b8fcd0 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -2359,7 +2359,7 @@ func TestGetHTTPServiceData(t *testing.T) { ctx context.Context mockdo *mockdo routes map[string]url.URL - want *restserver.GetHTTPServiceDataResponse + want *cns.GetHTTPServiceDataResponse wantErr bool }{ { @@ -2367,11 +2367,11 @@ func TestGetHTTPServiceData(t *testing.T) { ctx: context.TODO(), mockdo: &mockdo{ errToReturn: nil, - objToReturn: &restserver.GetHTTPServiceDataResponse{}, + objToReturn: &cns.GetHTTPServiceDataResponse{}, httpStatusCodeToReturn: http.StatusOK, }, routes: emptyRoutes, - want: &restserver.GetHTTPServiceDataResponse{}, + want: &cns.GetHTTPServiceDataResponse{}, wantErr: false, }, { @@ -2391,7 +2391,7 @@ func TestGetHTTPServiceData(t *testing.T) { ctx: context.TODO(), mockdo: &mockdo{ errToReturn: nil, - objToReturn: []restserver.GetHTTPServiceDataResponse{}, + objToReturn: []cns.GetHTTPServiceDataResponse{}, httpStatusCodeToReturn: http.StatusOK, }, routes: emptyRoutes, @@ -2415,8 +2415,8 @@ func TestGetHTTPServiceData(t *testing.T) { ctx: context.TODO(), mockdo: &mockdo{ errToReturn: nil, - objToReturn: &restserver.GetHTTPServiceDataResponse{ - Response: restserver.Response{ + objToReturn: &cns.GetHTTPServiceDataResponse{ + Response: cns.Response{ ReturnCode: types.UnsupportedNetworkType, }, }, diff --git a/cns/middlewares/SFSwiftV2.go b/cns/restserver/SFSwiftV2.go similarity index 85% rename from cns/middlewares/SFSwiftV2.go rename to cns/restserver/SFSwiftV2.go index 22ac14b2df..6350a7266f 100644 --- a/cns/middlewares/SFSwiftV2.go +++ b/cns/restserver/SFSwiftV2.go @@ -1,28 +1,19 @@ -package middlewares +package restserver import ( "context" "fmt" "github.com/Azure/azure-container-networking/cns" - cnsclient "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/cns/client" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/pkg/errors" - "time" -) - -const ( - contentTypeJSON = "application/json" - headerContentType = "Content-Type" - cnsReqTimeout = 15 * time.Second ) type SFSWIFTv2Middleware struct { + CnsClient *client.Client } -// Verify interface compliance at compile time -var _ cns.IPConfigsHandlerMiddleware = (*SFSWIFTv2Middleware)(nil) - // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP config requests for SF standalone scenario. This function wraps the default SWIFT request // and release IP configs handlers. func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { @@ -37,11 +28,14 @@ func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, fai }, }, errors.New("failed to validate ip configs request") } - ipConfigsResp, err := defaultHandler(ctx, req) - // If the pod is not v2, return the response from the handler - if !req.SecondaryInterfacesExist { - return ipConfigsResp, err + podInfo, err := cns.NewPodInfoFromIPConfigsRequest(*req) + ipConfigsResp := &cns.IPConfigsResponse{ + Response: cns.Response{ + ReturnCode: types.Success, + }, + PodIPInfo: []cns.PodIpInfo{}, } + // 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 @@ -97,11 +91,8 @@ func (m *SFSWIFTv2Middleware) getIPConfig(ctx context.Context, podInfo cns.PodIn if err != nil { return cns.PodIpInfo{}, fmt.Errorf("error getting orchestrator context from PodInfo %w", err) } - cnsClient, err := cnsclient.New("", cnsReqTimeout) - if err != nil { - return cns.PodIpInfo{}, fmt.Errorf("error initializing cnsclient %w", err) - } - resp, err := cnsClient.GetNetworkContainer(ctx, orchestratorContext) + // call getNC via CNSClient + resp, err := m.CnsClient.GetNetworkContainer(ctx, orchestratorContext) if err != nil { return cns.PodIpInfo{}, fmt.Errorf("error getNetworkContainerByOrchestrator Context %w", err) } diff --git a/cns/restserver/SFSwiftV2_test.go b/cns/restserver/SFSwiftV2_test.go new file mode 100644 index 0000000000..980e0a2961 --- /dev/null +++ b/cns/restserver/SFSwiftV2_test.go @@ -0,0 +1,52 @@ +package restserver + +import ( + "context" + "github.com/Azure/azure-container-networking/cns" + cnsclient "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/types" + "gotest.tools/v3/assert" + "testing" + "time" +) + +func TestValidateSFIPConfigsRequestSuccess(t *testing.T) { + cnsClient, err := cnsclient.New("", 15*time.Second) + if err != nil { + logger.Errorf("Failed to init cnsclient, err:%v.\n", err) + return + } + middleware := SFSWIFTv2Middleware{CnsClient: cnsClient} + + happyReq := &cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + happyReq.OrchestratorContext = b + happyReq.SecondaryInterfacesExist = false + + _, respCode, err1 := middleware.validateIPConfigsRequest(context.TODO(), happyReq) + assert.Equal(t, err1, "") + assert.Equal(t, respCode, types.Success) + assert.Equal(t, happyReq.SecondaryInterfacesExist, true) + assert.Equal(t, happyReq.AddInterfacesDataToResponse, true) +} + +func TestValidateSFIPConfigsRequestFailure(t *testing.T) { + cnsClient, err := cnsclient.New("", 15*time.Second) + if err != nil { + logger.Errorf("Failed to init cnsclient, err:%v.\n", err) + return + } + middleware := SFSWIFTv2Middleware{CnsClient: cnsClient} + // Fail to unmarshal pod info test + failReq := &cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + failReq.OrchestratorContext = []byte("invalid") + _, respCode, _ := middleware.validateIPConfigsRequest(context.TODO(), failReq) + assert.Equal(t, respCode, types.UnexpectedError) +} diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 9869c210ae..b172b0ebd0 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -30,7 +30,6 @@ var ( // requestIPConfigHandlerHelper validates the request, assign IPs and return the IPConfigs func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - // For SWIFT v2 scenario, the validator function will also modify the ipconfigsRequest. podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, *ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ @@ -538,8 +537,8 @@ func (service *HTTPRestService) handleDebugRestData(w http.ResponseWriter, r *ht http.Error(w, "not ready", http.StatusServiceUnavailable) return } - resp := GetHTTPServiceDataResponse{ - HTTPRestServiceData: HTTPRestServiceData{ + resp := cns.GetHTTPServiceDataResponse{ + HTTPRestServiceData: cns.HTTPRestServiceData{ PodIPIDByPodInterfaceKey: service.PodIPIDByPodInterfaceKey, PodIPConfigState: service.PodIPConfigState, IPAMPoolMonitor: service.IPAMPoolMonitor.GetStateSnapshot(), diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 04a4cea078..3d6724374a 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -6,27 +6,27 @@ package restserver import ( "context" "fmt" - "net" - "net/netip" - "strconv" - "testing" - "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/fakes" - "github.com/Azure/azure-container-networking/cns/middlewares" "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "net" + "net/netip" + "strconv" + "testing" ) var ( testNCID = "06867cf3-332d-409d-8819-ed70d2c116b0" + testNCID2 = "131b26ff-11d2-45ab-9eef-c8a041c380dd" testNCIDv6 = "a69b9217-3d89-4b73-a052-1e8baa453cb0" + testMAC = "00:22:c4:26:64:d2" ipPrefixBitsv4 = uint8(24) ipPrefixBitsv6 = uint8(120) @@ -75,6 +75,16 @@ func getTestService() *HTTPRestService { return httpsvc } +func getTestServiceSF() *HTTPRestService { + var config common.ServiceConfig + httpsvc, _ := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, store.NewMockStore(""), nil, nil) + svc = httpsvc + httpsvc.IPAMPoolMonitor = &fakes.MonitorFake{} + setOrchestratorTypeInternal(cns.ServiceFabric) + + return httpsvc +} + func newSecondaryIPConfig(ipAddress string, ncVersion int) cns.SecondaryIPConfig { return cns.SecondaryIPConfig{ IPAddress: ipAddress, @@ -1485,7 +1495,7 @@ func TestIPAMFailToReleasePartialIPsInPool(t *testing.T) { if err != nil { t.Fatalf("Expected to not fail adding empty NC to state: %+v", err) } - // remove the IP from the from the ipconfig map so that it throws an error when trying to release one of the IPs + // remove the IP from the ipconfig map so that it throws an error when trying to release one of the IPs delete(svc.PodIPConfigState, testStatev6.ID) err = svc.releaseIPConfigs(testPod1Info) @@ -1515,7 +1525,7 @@ func TestIPAMFailToRequestPartialIPsInPool(t *testing.T) { if err != nil { t.Fatalf("Expected to not fail adding empty NC to state: %+v", err) } - // remove the IP from the from the ipconfig map so that it throws an error when trying to release one of the IPs + // remove the IP from the ipconfig map so that it throws an error when trying to release one of the IPs delete(svc.PodIPConfigState, testStatev6.ID) req := cns.IPConfigsRequest{ @@ -1536,7 +1546,7 @@ func TestIPAMFailToRequestPartialIPsInPool(t *testing.T) { func TestIPAMReleaseSWIFTV2PodIPSuccess(t *testing.T) { svc := getTestService() - middleware := middlewares.K8sSWIFTv2Middleware{Cli: mock.NewClient()} + middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} svc.AttachIPConfigsHandlerMiddleware(&middleware) t.Setenv(configuration.EnvPodCIDRs, "10.0.1.10/24") @@ -1587,7 +1597,7 @@ func TestIPAMReleaseSWIFTV2PodIPSuccess(t *testing.T) { func TestIPAMGetK8sSWIFTv2IPSuccess(t *testing.T) { svc := getTestService() - middleware := middlewares.K8sSWIFTv2Middleware{Cli: mock.NewClient()} + middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} svc.AttachIPConfigsHandlerMiddleware(&middleware) t.Setenv(configuration.EnvPodCIDRs, "10.0.1.10/24") @@ -1650,7 +1660,7 @@ func TestIPAMGetK8sSWIFTv2IPSuccess(t *testing.T) { func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { svc := getTestService() - middleware := middlewares.K8sSWIFTv2Middleware{Cli: mock.NewClient()} + middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} svc.AttachIPConfigsHandlerMiddleware(&middleware) ncStates := []ncState{ { @@ -1717,3 +1727,170 @@ func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { t.Fatal("Expected available ips to be 2 since we expect the IP to not be assigned") } } + +// +//func TestIPAMGetSFSWIFTv2IPSuccess(t *testing.T) { +// req := cns.IPConfigsRequest{ +// PodInterfaceID: testPod1Info.InterfaceID(), +// InfraContainerID: testPod1Info.InfraContainerID(), +// } +// b, _ := testPod1Info.OrchestratorContext() +// req.OrchestratorContext = b +// +// svc := getTestServiceSF() +// ncResponse := cns.GetNetworkContainerResponse{ +// NetworkContainerID: testNCID, +// PrimaryInterfaceIdentifier: "10.0.0.1", +// LocalIPConfiguration: cns.IPConfiguration{ +// IPSubnet: cns.IPSubnet{ +// IPAddress: "10.0.0.5", +// PrefixLength: 16, +// }, +// GatewayIPAddress: "5.5.5.5", +// }, +// IPConfiguration: cns.IPConfiguration{ +// IPSubnet: cns.IPSubnet{ +// IPAddress: testIP2, +// PrefixLength: 16, +// }, +// DNSServers: nil, +// GatewayIPAddress: "10.1.0.1", +// }, +// NetworkInterfaceInfo: cns.NetworkInterfaceInfo{NICType: cns.DelegatedVMNIC, MACAddress: testMAC}, +// Routes: []cns.Route{ +// { +// IPAddress: "10.1.0.0/16", +// GatewayIPAddress: "10.1.0.1", +// }, +// }, +// } +// cx := ClientMockCNSSF{getNetworkContainerConfiguration: getNetworkContainerConfigurationHandler{ +// orchestratorContext: b, +// returnResponse: &ncResponse, +// err: nil, +// }} +// //cnsClient, err := cnsclient.New("", 15*time.Second) +// //if err != nil { +// // logger.Errorf("Failed to init cnsclient, err:%v.\n", err) +// // return +// //} +// middleware := SFSWIFTv2Middleware{CnsCli: &cx} +// +// svc.AttachIPConfigsHandlerMiddleware(&middleware) +// +// // prepare cns state file population +// //svc.state.OrchestratorType = cns.ServiceFabric +// //svc.state.ContainerStatus = make(map[string]containerstatus) +// //svc.state.ContainerStatus[testNCID] = containerstatus{ +// // ID: testNCID, +// // VMVersion: "0", +// // HostVersion: "0", +// // CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ +// // NetworkContainerid: testNCID, +// // PrimaryInterfaceIdentifier: testIP1, +// // IPConfiguration: cns.IPConfiguration{ +// // IPSubnet: cns.IPSubnet{ +// // IPAddress: testIP2, +// // PrefixLength: 24, +// // }, +// // }, +// // OrchestratorContext: b, +// // NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ +// // NICType: cns.DelegatedVMNIC, +// // MACAddress: testMAC, +// // }, +// // NetworkContainerType: cns.AzureContainerInstance, +// // }, +// //} +// +// wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) +// resp, err := wrappedHandler(context.TODO(), &req) +// if err != nil { +// t.Fatalf("Expected to not fail requesting IPs: %+v", err) +// } +// podIPInfo := resp.PodIPInfo +// +// // Asserting that SWIFT v2 IP is returned +// assert.Equal(t, testIP2, podIPInfo[0].PodIPConfig.IPAddress) +// assert.Equal(t, testMAC, podIPInfo[0].MacAddress) +// assert.Equal(t, cns.DelegatedVMNIC, podIPInfo[0].NICType) +//} +// +//func TestIPAMGetSFSWIFTv2IPError(t *testing.T) { +// req := cns.IPConfigsRequest{ +// PodInterfaceID: testPod1Info.InterfaceID(), +// InfraContainerID: testPod1Info.InfraContainerID(), +// } +// b, _ := testPod1Info.OrchestratorContext() +// req.OrchestratorContext = b +// +// svc := getTestServiceSF() +// ncResponse := cns.GetNetworkContainerResponse{ +// NetworkContainerID: testNCID, +// PrimaryInterfaceIdentifier: "10.0.0.1", +// LocalIPConfiguration: cns.IPConfiguration{ +// IPSubnet: cns.IPSubnet{ +// IPAddress: "10.0.0.5", +// PrefixLength: 16, +// }, +// GatewayIPAddress: "5.5.5.5", +// }, +// IPConfiguration: cns.IPConfiguration{ +// IPSubnet: cns.IPSubnet{ +// IPAddress: testIP2, +// PrefixLength: 16, +// }, +// DNSServers: nil, +// GatewayIPAddress: "10.1.0.1", +// }, +// NetworkInterfaceInfo: cns.NetworkInterfaceInfo{NICType: cns.DelegatedVMNIC, MACAddress: ""}, +// Routes: []cns.Route{ +// { +// IPAddress: "10.1.0.0/16", +// GatewayIPAddress: "10.1.0.1", +// }, +// }, +// } +// cx := ClientMockCNSSF{getNetworkContainerConfiguration: getNetworkContainerConfigurationHandler{ +// orchestratorContext: b, +// returnResponse: &ncResponse, +// err: nil, +// }} +// middleware := SFSWIFTv2Middleware{CnsCli: &cx} +// +// svc.AttachIPConfigsHandlerMiddleware(&middleware) +// +// // prepare cns state file population +// svc.state.OrchestratorType = cns.ServiceFabric +// svc.state.ContainerStatus = make(map[string]containerstatus) +// svc.state.ContainerStatus[testNCID] = containerstatus{ +// ID: testNCID, +// VMVersion: "0", +// HostVersion: "0", +// CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ +// NetworkContainerid: testNCID, +// PrimaryInterfaceIdentifier: testIP1, +// IPConfiguration: cns.IPConfiguration{ +// IPSubnet: cns.IPSubnet{ +// IPAddress: testIP2, +// PrefixLength: 24, +// }, +// }, +// OrchestratorContext: b, +// NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ +// NICType: cns.DelegatedVMNIC, +// MACAddress: testMAC, +// }, +// NetworkContainerType: cns.AzureContainerInstance, +// }, +// } +// +// wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) +// _, err := wrappedHandler(context.TODO(), &req) +// if err != nil { +// t.Logf("Expected to fail requesting IPs: %+v", err) +// } +// if err == nil { +// t.Fatalf("Expected fail requesting IPs due to empty MAC") +// } +//} diff --git a/cns/middlewares/k8sSwiftV2.go b/cns/restserver/k8sSwiftV2.go similarity index 99% rename from cns/middlewares/k8sSwiftV2.go rename to cns/restserver/k8sSwiftV2.go index 423bd0c16a..4d19c69df8 100644 --- a/cns/middlewares/k8sSwiftV2.go +++ b/cns/restserver/k8sSwiftV2.go @@ -1,4 +1,4 @@ -package middlewares +package restserver import ( "context" diff --git a/cns/middlewares/k8sSwiftV2_test.go b/cns/restserver/k8sSwiftV2_test.go similarity index 93% rename from cns/middlewares/k8sSwiftV2_test.go rename to cns/restserver/k8sSwiftV2_test.go index b30c7865e2..6dbadd02cc 100644 --- a/cns/middlewares/k8sSwiftV2_test.go +++ b/cns/restserver/k8sSwiftV2_test.go @@ -1,4 +1,4 @@ -package middlewares +package restserver import ( "context" @@ -7,31 +7,15 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/configuration" - "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/cns/types" "gotest.tools/v3/assert" ) var ( - testPod1GUID = "898fb8f1-f93e-4c96-9c31-6b89098949a3" - testPod1Info = cns.NewPodInfo("898fb8-eth0", testPod1GUID, "testpod1", "testpod1namespace") - - testPod2GUID = "b21e1ee1-fb7e-4e6d-8c68-22ee5049944e" - testPod2Info = cns.NewPodInfo("b21e1e-eth0", testPod2GUID, "testpod2", "testpod2namespace") - - testPod3GUID = "718e04ac-5a13-4dce-84b3-040accaa9b41" - testPod3Info = cns.NewPodInfo("718e04-eth0", testPod3GUID, "testpod3", "testpod3namespace") - - testPod4GUID = "b21e1ee1-fb7e-4e6d-8c68-22ee5049944e" testPod4Info = cns.NewPodInfo("b21e1e-eth0", testPod4GUID, "testpod4", "testpod4namespace") ) -func TestMain(m *testing.M) { - logger.InitLogger("testlogs", 0, 0, "./") - m.Run() -} - func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} t.Setenv(configuration.EnvPodCIDRs, "10.0.1.10/24,16A0:0010:AB00:001E::2/32") diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 1e79b593ae..6e1f72da73 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -103,18 +103,6 @@ type IPInfo struct { IPv6 []net.IPNet } -type GetHTTPServiceDataResponse struct { - HTTPRestServiceData HTTPRestServiceData - Response Response -} - -// HTTPRestServiceData represents in-memory CNS data in the debug API paths. -type HTTPRestServiceData struct { - PodIPIDByPodInterfaceKey map[string][]string // PodInterfaceId is key and value is slice of Pod IP uuids. - PodIPConfigState map[string]cns.IPConfigurationStatus // secondaryipid(uuid) is key - IPAMPoolMonitor cns.IpamPoolMonitorStateSnapshot -} - type Response struct { ReturnCode types.ResponseCode Message string diff --git a/cns/service/main.go b/cns/service/main.go index df98da2041..1f1a68c0da 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -37,7 +37,6 @@ import ( nncctrl "github.com/Azure/azure-container-networking/cns/kubecontroller/nodenetworkconfig" podctrl "github.com/Azure/azure-container-networking/cns/kubecontroller/pod" "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/middlewares" "github.com/Azure/azure-container-networking/cns/multitenantcontroller" "github.com/Azure/azure-container-networking/cns/multitenantcontroller/multitenantoperator" "github.com/Azure/azure-container-networking/cns/restserver" @@ -790,11 +789,14 @@ func main() { go platform.MonitorAndSetMellanoxRegKeyPriorityVLANTag(rootCtx, cnsconfig.MellanoxMonitorIntervalSecs) } // if swiftv2 scenario is enabled, we need to initialize the Service Fabric (standalone) swiftv2 middleware to process IP configs requests - if cnsconfig.EnableSwiftV2 { - if cnsconfig.SWIFTV2Mode == configuration.SFSWIFTV2 { - swiftV2Middleware := &middlewares.SFSWIFTv2Middleware{} - httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) + if cnsconfig.SWIFTV2Mode == configuration.SFSWIFTV2 { + cnsClient, err := cnsclient.New("", cnsReqTimeout) + if err != nil { + logger.Errorf("Failed to init cnsclient, err:%v.\n", err) + return } + swiftV2Middleware := &restserver.SFSWIFTv2Middleware{CnsClient: cnsClient} + httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) } } @@ -1220,7 +1222,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn // check the Node labels for Swift V2 if _, ok := node.Labels[configuration.LabelNodeSwiftV2]; ok { - cnsconfig.EnableSwiftV2 = true + cnsconfig.SWIFTV2Mode = configuration.K8sSWIFTV2 cnsconfig.WatchPods = true // TODO(rbtr): create the NodeInfo for Swift V2 // register the noop mtpnc reconciler to populate the cache @@ -1383,17 +1385,14 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } } - if cnsconfig.EnableSwiftV2 { + if cnsconfig.SWIFTV2Mode == configuration.K8sSWIFTV2 { if err := mtpncctrl.SetupWithManager(manager); err != nil { return errors.Wrapf(err, "failed to setup mtpnc reconciler with manager") } // if SWIFT v2 is enabled on CNS, attach multitenant middleware to rest service // here for AKS(K8s) swiftv2 middleware to process IP configs requests - if cnsconfig.SWIFTV2Mode == configuration.K8sSWIFTV2 { - swiftV2Middleware := &middlewares.K8sSWIFTv2Middleware{Cli: manager.GetClient()} - httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) - } - + swiftV2Middleware := &restserver.K8sSWIFTv2Middleware{Cli: manager.GetClient()} + httpRestService.AttachIPConfigsHandlerMiddleware(swiftV2Middleware) } // start the pool Monitor before the Reconciler, since it needs to be ready to receive an From f2a2405fa35562bf6c3aff5a1ba368a9cd1e1412 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 1 Feb 2024 15:30:35 -0800 Subject: [PATCH 15/25] fix: updatepodinfowithinterfaces & linters fix --- cns/restserver/SFSwiftV2.go | 5 +++-- cns/restserver/ipam.go | 8 +++++++- cns/restserver/ipam_test.go | 12 ++++++------ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/cns/restserver/SFSwiftV2.go b/cns/restserver/SFSwiftV2.go index 6350a7266f..1c261498ff 100644 --- a/cns/restserver/SFSwiftV2.go +++ b/cns/restserver/SFSwiftV2.go @@ -3,6 +3,7 @@ package restserver import ( "context" "fmt" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/client" "github.com/Azure/azure-container-networking/cns/logger" @@ -16,9 +17,9 @@ type SFSWIFTv2Middleware struct { // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP config requests for SF standalone scenario. This function wraps the default SWIFT request // and release IP configs handlers. -func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { +func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(_, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message := m.validateIPConfigsRequest(ctx, req) + _, respCode, message := m.validateIPConfigsRequest(ctx, req) if respCode != types.Success { return &cns.IPConfigsResponse{ diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index b172b0ebd0..2781df83af 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -190,7 +190,9 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r } func (service *HTTPRestService) updatePodInfoWithInterfaces(ctx context.Context, ipconfigResponse *cns.IPConfigsResponse) (*cns.IPConfigsResponse, error) { + var podIpInfoList []cns.PodIpInfo for _, podIpInfo := range ipconfigResponse.PodIPInfo { + // populating podIpInfo with primary & secondary interface info & updating IpConfigsResponse hostPrimaryInterface, err := service.getPrimaryHostInterface(ctx) if err != nil { return &cns.IPConfigsResponse{}, err @@ -213,10 +215,14 @@ func (service *HTTPRestService) updatePodInfoWithInterfaces(ctx context.Context, SecondaryIP: hostSecondaryInterface.SecondaryIPs[0], Subnet: hostSecondaryInterface.Subnet, } - } + podIpInfoList = append(podIpInfoList, podIpInfo) + + } + ipconfigResponse.PodIPInfo = podIpInfoList return ipconfigResponse, nil } + func (service *HTTPRestService) updateEndpointState(ipconfigsRequest cns.IPConfigsRequest, podInfo cns.PodInfo, podIPInfo []cns.PodIpInfo) error { if service.EndpointStateStore == nil { return ErrStoreEmpty diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 3d6724374a..50d4f1551f 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -6,6 +6,12 @@ package restserver import ( "context" "fmt" + + "net" + "net/netip" + "strconv" + "testing" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/configuration" @@ -16,17 +22,11 @@ import ( "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" "github.com/stretchr/testify/assert" - "net" - "net/netip" - "strconv" - "testing" ) var ( testNCID = "06867cf3-332d-409d-8819-ed70d2c116b0" - testNCID2 = "131b26ff-11d2-45ab-9eef-c8a041c380dd" testNCIDv6 = "a69b9217-3d89-4b73-a052-1e8baa453cb0" - testMAC = "00:22:c4:26:64:d2" ipPrefixBitsv4 = uint8(24) ipPrefixBitsv6 = uint8(120) From 91638bfdb3390312dd1421bc0548cb8a7e92adf3 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Fri, 2 Feb 2024 00:34:22 -0800 Subject: [PATCH 16/25] fix: linter errors --- cns/restserver/SFSwiftV2_test.go | 5 +++-- cns/restserver/ipam.go | 33 ++++++++++++++++---------------- cns/restserver/ipam_test.go | 10 ---------- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/cns/restserver/SFSwiftV2_test.go b/cns/restserver/SFSwiftV2_test.go index 980e0a2961..36f5d0e70e 100644 --- a/cns/restserver/SFSwiftV2_test.go +++ b/cns/restserver/SFSwiftV2_test.go @@ -2,13 +2,14 @@ package restserver import ( "context" + "testing" + "time" + "github.com/Azure/azure-container-networking/cns" cnsclient "github.com/Azure/azure-container-networking/cns/client" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "gotest.tools/v3/assert" - "testing" - "time" ) func TestValidateSFIPConfigsRequestSuccess(t *testing.T) { diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 2781df83af..efbcc9f0d7 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -190,36 +190,35 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r } func (service *HTTPRestService) updatePodInfoWithInterfaces(ctx context.Context, ipconfigResponse *cns.IPConfigsResponse) (*cns.IPConfigsResponse, error) { - var podIpInfoList []cns.PodIpInfo - for _, podIpInfo := range ipconfigResponse.PodIPInfo { + podIPInfoList := make([]cns.PodIpInfo, 0, len(ipconfigResponse.PodIPInfo)) + for i := range ipconfigResponse.PodIPInfo { // populating podIpInfo with primary & secondary interface info & updating IpConfigsResponse hostPrimaryInterface, err := service.getPrimaryHostInterface(ctx) if err != nil { return &cns.IPConfigsResponse{}, err } - hostSecondaryInterface, err := service.getSecondaryHostInterface(ctx, podIpInfo.MacAddress) + hostSecondaryInterface, err := service.getSecondaryHostInterface(ctx, ipconfigResponse.PodIPInfo[i].MacAddress) if err != nil { return &cns.IPConfigsResponse{}, err } - podIpInfo.HostPrimaryIPInfo = - cns.HostIPInfo{ - Gateway: hostPrimaryInterface.Gateway, - PrimaryIP: hostPrimaryInterface.PrimaryIP, - Subnet: hostPrimaryInterface.Subnet, - } - podIpInfo.HostSecondaryIPInfo = - cns.HostIPInfo{ - Gateway: hostSecondaryInterface.Gateway, - SecondaryIP: hostSecondaryInterface.SecondaryIPs[0], - Subnet: hostSecondaryInterface.Subnet, - } + ipconfigResponse.PodIPInfo[i].HostPrimaryIPInfo = cns.HostIPInfo{ + Gateway: hostPrimaryInterface.Gateway, + PrimaryIP: hostPrimaryInterface.PrimaryIP, + Subnet: hostPrimaryInterface.Subnet, + } + + ipconfigResponse.PodIPInfo[i].HostSecondaryIPInfo = cns.HostIPInfo{ + Gateway: hostSecondaryInterface.Gateway, + SecondaryIP: hostSecondaryInterface.SecondaryIPs[0], + Subnet: hostSecondaryInterface.Subnet, + } - podIpInfoList = append(podIpInfoList, podIpInfo) + podIPInfoList = append(podIPInfoList, ipconfigResponse.PodIPInfo[i]) } - ipconfigResponse.PodIPInfo = podIpInfoList + ipconfigResponse.PodIPInfo = podIPInfoList return ipconfigResponse, nil } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 50d4f1551f..a5636e992c 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -75,16 +75,6 @@ func getTestService() *HTTPRestService { return httpsvc } -func getTestServiceSF() *HTTPRestService { - var config common.ServiceConfig - httpsvc, _ := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, store.NewMockStore(""), nil, nil) - svc = httpsvc - httpsvc.IPAMPoolMonitor = &fakes.MonitorFake{} - setOrchestratorTypeInternal(cns.ServiceFabric) - - return httpsvc -} - func newSecondaryIPConfig(ipAddress string, ncVersion int) cns.SecondaryIPConfig { return cns.SecondaryIPConfig{ IPAddress: ipAddress, From d8e2e72c85df6da2fd7948712cd438d238fc86dc Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Mon, 5 Feb 2024 00:55:04 -0800 Subject: [PATCH 17/25] fix: linter errors context check & govet --- cns/restserver/ipam.go | 2 +- cns/service/main.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index efbcc9f0d7..669a04721d 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -41,7 +41,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context } // record a pod requesting an IP service.podsPendingIPAssignment.Push(podInfo.Key()) - podIPInfo, err := requestIPConfigsHelper(service, *ipconfigsRequest) + podIPInfo, err := requestIPConfigsHelper(service, *ipconfigsRequest) //nolint:contextcheck // to refactor later if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ diff --git a/cns/service/main.go b/cns/service/main.go index 1f1a68c0da..c226ca320e 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -790,7 +790,7 @@ func main() { } // if swiftv2 scenario is enabled, we need to initialize the Service Fabric (standalone) swiftv2 middleware to process IP configs requests if cnsconfig.SWIFTV2Mode == configuration.SFSWIFTV2 { - cnsClient, err := cnsclient.New("", cnsReqTimeout) + cnsClient, err := cnsclient.New("", cnsReqTimeout) //nolint:govet // shadow ok as function returns in above errs if err != nil { logger.Errorf("Failed to init cnsclient, err:%v.\n", err) return From c44f71d2aaefe25bfa6dd49911af01842e5264dd Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Mon, 5 Feb 2024 16:43:11 -0800 Subject: [PATCH 18/25] removed validateIpConfigs, removed failureHandler, removed swiftv2sf_test file, removed addInterfaces config field --- cns/NetworkContainerContract.go | 13 +-- cns/restserver/SFSwiftV2.go | 54 ++------- cns/restserver/SFSwiftV2_test.go | 53 --------- cns/restserver/ipam.go | 3 +- cns/restserver/ipam_test.go | 193 +++++-------------------------- 5 files changed, 44 insertions(+), 272 deletions(-) delete mode 100644 cns/restserver/SFSwiftV2_test.go diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 801b1210f1..fc7fe16f1a 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -486,13 +486,12 @@ type IPConfigRequest struct { // Same as IPConfigRequest except that DesiredIPAddresses is passed in as a slice type IPConfigsRequest struct { - DesiredIPAddresses []string `json:"desiredIPAddresses"` - PodInterfaceID string `json:"podInterfaceID"` - InfraContainerID string `json:"infraContainerID"` - OrchestratorContext json.RawMessage `json:"orchestratorContext"` - Ifname string `json:"ifname"` // Used by delegated IPAM - SecondaryInterfacesExist bool `json:"secondaryInterfacesExist"` // will be set by SWIFT v2 validator func - AddInterfacesDataToResponse bool `json:"addInterfacesDataToResponse"` // used by SF Swiftv2 middleware to externally add interfaces using restserver methods + DesiredIPAddresses []string `json:"desiredIPAddresses"` + PodInterfaceID string `json:"podInterfaceID"` + InfraContainerID string `json:"infraContainerID"` + OrchestratorContext json.RawMessage `json:"orchestratorContext"` + Ifname string `json:"ifname"` // Used by delegated IPAM + SecondaryInterfacesExist bool `json:"secondaryInterfacesExist"` // will be set by SWIFT v2 validator func } // IPConfigResponse is used in CNS IPAM mode as a response to CNI ADD diff --git a/cns/restserver/SFSwiftV2.go b/cns/restserver/SFSwiftV2.go index 1c261498ff..ce75497871 100644 --- a/cns/restserver/SFSwiftV2.go +++ b/cns/restserver/SFSwiftV2.go @@ -17,18 +17,9 @@ type SFSWIFTv2Middleware struct { // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP config requests for SF standalone scenario. This function wraps the default SWIFT request // and release IP configs handlers. -func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(_, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { +func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(_, _ cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { return func(ctx context.Context, req *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - _, respCode, message := m.validateIPConfigsRequest(ctx, req) - - if respCode != types.Success { - return &cns.IPConfigsResponse{ - Response: cns.Response{ - ReturnCode: respCode, - Message: message, - }, - }, errors.New("failed to validate ip configs request") - } + // unmarshal & retrieve podInfo from OrchestratorContext podInfo, err := cns.NewPodInfoFromIPConfigsRequest(*req) ipConfigsResp := &cns.IPConfigsResponse{ Response: cns.Response{ @@ -36,20 +27,16 @@ func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(_, failureHandler c }, PodIPInfo: []cns.PodIpInfo{}, } - - // 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 - if err != nil { - _, err = failureHandler(ctx, req) - if err != nil { - logger.Errorf("failed to release default IP config : %v", err) - } - } - }() if err != nil { - return ipConfigsResp, err + ipConfigsResp.Response.ReturnCode = types.UnexpectedError + return ipConfigsResp, errors.Wrapf(err, "Failed to receive PodInfo after unmarshalling from IPConfigsRequest %v", req) } + + // SwiftV2-SF will always request for secondaryInterfaces for a pod + req.SecondaryInterfacesExist = true + logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) + + // get the IPConfig for swiftv2 SF scenario by calling into cns getNC api SWIFTv2PodIPInfo, err := m.getIPConfig(ctx, podInfo) if err != nil { return &cns.IPConfigsResponse{ @@ -65,27 +52,6 @@ func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(_, failureHandler c } } -// validateIPConfigsRequest validates if pod orchestrator context is unmarshalled & sets secondary interfaces true -// nolint -func (m *SFSWIFTv2Middleware) validateIPConfigsRequest(ctx context.Context, req *cns.IPConfigsRequest) (podInfo cns.PodInfo, respCode types.ResponseCode, message string) { - // Retrieve the pod from the cluster - podInfo, err := cns.UnmarshalPodInfo(req.OrchestratorContext) - if err != nil { - errBuf := errors.Wrapf(err, "failed to unmarshalling pod info from ipconfigs request %+v", req) - return nil, types.UnexpectedError, errBuf.Error() - } - logger.Printf("[SWIFTv2Middleware] validate ipconfigs request for pod %s", podInfo.Name()) - - req.SecondaryInterfacesExist = true - // swiftv2 SF scenario for windows requires host interface info to be populated - // which can be done only by restserver service function, hence setting this flag to do so in ipam.go - req.AddInterfacesDataToResponse = true - - logger.Printf("[SWIFTv2Middleware] pod %s has secondary interface : %v", podInfo.Name(), req.SecondaryInterfacesExist) - // retrieve podinfo from orchestrator context - return podInfo, types.Success, "" -} - // getIPConfig returns the pod's SWIFT V2 IP configuration. func (m *SFSWIFTv2Middleware) getIPConfig(ctx context.Context, podInfo cns.PodInfo) (cns.PodIpInfo, error) { orchestratorContext, err := podInfo.OrchestratorContext() diff --git a/cns/restserver/SFSwiftV2_test.go b/cns/restserver/SFSwiftV2_test.go deleted file mode 100644 index 36f5d0e70e..0000000000 --- a/cns/restserver/SFSwiftV2_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package restserver - -import ( - "context" - "testing" - "time" - - "github.com/Azure/azure-container-networking/cns" - cnsclient "github.com/Azure/azure-container-networking/cns/client" - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/types" - "gotest.tools/v3/assert" -) - -func TestValidateSFIPConfigsRequestSuccess(t *testing.T) { - cnsClient, err := cnsclient.New("", 15*time.Second) - if err != nil { - logger.Errorf("Failed to init cnsclient, err:%v.\n", err) - return - } - middleware := SFSWIFTv2Middleware{CnsClient: cnsClient} - - happyReq := &cns.IPConfigsRequest{ - PodInterfaceID: testPod1Info.InterfaceID(), - InfraContainerID: testPod1Info.InfraContainerID(), - } - b, _ := testPod1Info.OrchestratorContext() - happyReq.OrchestratorContext = b - happyReq.SecondaryInterfacesExist = false - - _, respCode, err1 := middleware.validateIPConfigsRequest(context.TODO(), happyReq) - assert.Equal(t, err1, "") - assert.Equal(t, respCode, types.Success) - assert.Equal(t, happyReq.SecondaryInterfacesExist, true) - assert.Equal(t, happyReq.AddInterfacesDataToResponse, true) -} - -func TestValidateSFIPConfigsRequestFailure(t *testing.T) { - cnsClient, err := cnsclient.New("", 15*time.Second) - if err != nil { - logger.Errorf("Failed to init cnsclient, err:%v.\n", err) - return - } - middleware := SFSWIFTv2Middleware{CnsClient: cnsClient} - // Fail to unmarshal pod info test - failReq := &cns.IPConfigsRequest{ - PodInterfaceID: testPod1Info.InterfaceID(), - InfraContainerID: testPod1Info.InfraContainerID(), - } - failReq.OrchestratorContext = []byte("invalid") - _, respCode, _ := middleware.validateIPConfigsRequest(context.TODO(), failReq) - assert.Equal(t, respCode, types.UnexpectedError) -} diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 669a04721d..6791723288 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "reflect" "strconv" "strings" @@ -170,7 +171,7 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r // Wrap the default datapath handlers with the middleware wrappedHandler := service.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(service.requestIPConfigHandlerHelper, service.releaseIPConfigHandlerHelper) ipConfigsResp, err = wrappedHandler(r.Context(), &ipconfigsRequest) - if ipconfigsRequest.AddInterfacesDataToResponse { + if reflect.DeepEqual(ipConfigsResp.PodIPInfo[0].HostPrimaryIPInfo, cns.HostIPInfo{}) || reflect.DeepEqual(ipConfigsResp.PodIPInfo[0].HostSecondaryIPInfo, cns.HostIPInfo{}) { ipConfigsResp, err = service.updatePodInfoWithInterfaces(r.Context(), ipConfigsResp) } } else { diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index a5636e992c..30f3189ebf 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -6,16 +6,18 @@ package restserver import ( "context" "fmt" - "net" "net/netip" "strconv" "testing" + "time" "github.com/Azure/azure-container-networking/cns" + cnsclient "github.com/Azure/azure-container-networking/cns/client" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/middlewares/mock" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" @@ -1718,169 +1720,26 @@ func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { } } -// -//func TestIPAMGetSFSWIFTv2IPSuccess(t *testing.T) { -// req := cns.IPConfigsRequest{ -// PodInterfaceID: testPod1Info.InterfaceID(), -// InfraContainerID: testPod1Info.InfraContainerID(), -// } -// b, _ := testPod1Info.OrchestratorContext() -// req.OrchestratorContext = b -// -// svc := getTestServiceSF() -// ncResponse := cns.GetNetworkContainerResponse{ -// NetworkContainerID: testNCID, -// PrimaryInterfaceIdentifier: "10.0.0.1", -// LocalIPConfiguration: cns.IPConfiguration{ -// IPSubnet: cns.IPSubnet{ -// IPAddress: "10.0.0.5", -// PrefixLength: 16, -// }, -// GatewayIPAddress: "5.5.5.5", -// }, -// IPConfiguration: cns.IPConfiguration{ -// IPSubnet: cns.IPSubnet{ -// IPAddress: testIP2, -// PrefixLength: 16, -// }, -// DNSServers: nil, -// GatewayIPAddress: "10.1.0.1", -// }, -// NetworkInterfaceInfo: cns.NetworkInterfaceInfo{NICType: cns.DelegatedVMNIC, MACAddress: testMAC}, -// Routes: []cns.Route{ -// { -// IPAddress: "10.1.0.0/16", -// GatewayIPAddress: "10.1.0.1", -// }, -// }, -// } -// cx := ClientMockCNSSF{getNetworkContainerConfiguration: getNetworkContainerConfigurationHandler{ -// orchestratorContext: b, -// returnResponse: &ncResponse, -// err: nil, -// }} -// //cnsClient, err := cnsclient.New("", 15*time.Second) -// //if err != nil { -// // logger.Errorf("Failed to init cnsclient, err:%v.\n", err) -// // return -// //} -// middleware := SFSWIFTv2Middleware{CnsCli: &cx} -// -// svc.AttachIPConfigsHandlerMiddleware(&middleware) -// -// // prepare cns state file population -// //svc.state.OrchestratorType = cns.ServiceFabric -// //svc.state.ContainerStatus = make(map[string]containerstatus) -// //svc.state.ContainerStatus[testNCID] = containerstatus{ -// // ID: testNCID, -// // VMVersion: "0", -// // HostVersion: "0", -// // CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ -// // NetworkContainerid: testNCID, -// // PrimaryInterfaceIdentifier: testIP1, -// // IPConfiguration: cns.IPConfiguration{ -// // IPSubnet: cns.IPSubnet{ -// // IPAddress: testIP2, -// // PrefixLength: 24, -// // }, -// // }, -// // OrchestratorContext: b, -// // NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ -// // NICType: cns.DelegatedVMNIC, -// // MACAddress: testMAC, -// // }, -// // NetworkContainerType: cns.AzureContainerInstance, -// // }, -// //} -// -// wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) -// resp, err := wrappedHandler(context.TODO(), &req) -// if err != nil { -// t.Fatalf("Expected to not fail requesting IPs: %+v", err) -// } -// podIPInfo := resp.PodIPInfo -// -// // Asserting that SWIFT v2 IP is returned -// assert.Equal(t, testIP2, podIPInfo[0].PodIPConfig.IPAddress) -// assert.Equal(t, testMAC, podIPInfo[0].MacAddress) -// assert.Equal(t, cns.DelegatedVMNIC, podIPInfo[0].NICType) -//} -// -//func TestIPAMGetSFSWIFTv2IPError(t *testing.T) { -// req := cns.IPConfigsRequest{ -// PodInterfaceID: testPod1Info.InterfaceID(), -// InfraContainerID: testPod1Info.InfraContainerID(), -// } -// b, _ := testPod1Info.OrchestratorContext() -// req.OrchestratorContext = b -// -// svc := getTestServiceSF() -// ncResponse := cns.GetNetworkContainerResponse{ -// NetworkContainerID: testNCID, -// PrimaryInterfaceIdentifier: "10.0.0.1", -// LocalIPConfiguration: cns.IPConfiguration{ -// IPSubnet: cns.IPSubnet{ -// IPAddress: "10.0.0.5", -// PrefixLength: 16, -// }, -// GatewayIPAddress: "5.5.5.5", -// }, -// IPConfiguration: cns.IPConfiguration{ -// IPSubnet: cns.IPSubnet{ -// IPAddress: testIP2, -// PrefixLength: 16, -// }, -// DNSServers: nil, -// GatewayIPAddress: "10.1.0.1", -// }, -// NetworkInterfaceInfo: cns.NetworkInterfaceInfo{NICType: cns.DelegatedVMNIC, MACAddress: ""}, -// Routes: []cns.Route{ -// { -// IPAddress: "10.1.0.0/16", -// GatewayIPAddress: "10.1.0.1", -// }, -// }, -// } -// cx := ClientMockCNSSF{getNetworkContainerConfiguration: getNetworkContainerConfigurationHandler{ -// orchestratorContext: b, -// returnResponse: &ncResponse, -// err: nil, -// }} -// middleware := SFSWIFTv2Middleware{CnsCli: &cx} -// -// svc.AttachIPConfigsHandlerMiddleware(&middleware) -// -// // prepare cns state file population -// svc.state.OrchestratorType = cns.ServiceFabric -// svc.state.ContainerStatus = make(map[string]containerstatus) -// svc.state.ContainerStatus[testNCID] = containerstatus{ -// ID: testNCID, -// VMVersion: "0", -// HostVersion: "0", -// CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ -// NetworkContainerid: testNCID, -// PrimaryInterfaceIdentifier: testIP1, -// IPConfiguration: cns.IPConfiguration{ -// IPSubnet: cns.IPSubnet{ -// IPAddress: testIP2, -// PrefixLength: 24, -// }, -// }, -// OrchestratorContext: b, -// NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ -// NICType: cns.DelegatedVMNIC, -// MACAddress: testMAC, -// }, -// NetworkContainerType: cns.AzureContainerInstance, -// }, -// } -// -// wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) -// _, err := wrappedHandler(context.TODO(), &req) -// if err != nil { -// t.Logf("Expected to fail requesting IPs: %+v", err) -// } -// if err == nil { -// t.Fatalf("Expected fail requesting IPs due to empty MAC") -// } -//} +func TestValidateSFIPConfigsRequestFailure(t *testing.T) { + svc := getTestService() + cnsClient, err := cnsclient.New("", 15*time.Second) + if err != nil { + logger.Errorf("Failed to init cnsclient, err:%v.\n", err) + return + } + middleware := SFSWIFTv2Middleware{CnsClient: cnsClient} + svc.AttachIPConfigsHandlerMiddleware(&middleware) + + // Fail to unmarshal pod info test + failReq := &cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + failReq.OrchestratorContext = []byte("invalid") + + wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) + resp, err := wrappedHandler(context.TODO(), failReq) + + assert.NotEmpty(t, err) + assert.Equal(t, types.UnexpectedError, resp.Response.ReturnCode) +} From 9038e8d7da6ced9c3db4515b3b89bcf76639079a Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 6 Feb 2024 13:04:24 -0800 Subject: [PATCH 19/25] test: move out UTs to restserver package, avoid import cycle --- cns/client/client_test.go | 313 ---------------------------- cns/restserver/cnsclient_test.go | 337 +++++++++++++++++++++++++++++++ 2 files changed, 337 insertions(+), 313 deletions(-) create mode 100644 cns/restserver/cnsclient_test.go diff --git a/cns/client/client_test.go b/cns/client/client_test.go index 0b64b8fcd0..0c19c32282 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -7,33 +7,22 @@ import ( "errors" "fmt" "io" - "net" "net/http" "net/url" "os" "sort" - "strconv" "testing" "time" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/common" - "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/logger" - "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/Azure/azure-container-networking/log" "github.com/google/go-cmp/cmp" - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( - primaryIp = "10.0.0.5" - gatewayIp = "10.0.0.1" subnetPrfixLength = 24 dockerContainerType = cns.Docker releasePercent = 150 @@ -43,8 +32,6 @@ const ( ) var ( - svc *restserver.HTTPRestService - dnsServers = []string{"8.8.8.8", "8.8.4.4"} errBadRequest = errors.New("bad request") ) @@ -64,89 +51,7 @@ func (m *mockdo) Do(req *http.Request) (*http.Response, error) { }, m.errToReturn } -func addTestStateToRestServer(t *testing.T, secondaryIps []string) { - var ipConfig cns.IPConfiguration - ipConfig.DNSServers = dnsServers - ipConfig.GatewayIPAddress = gatewayIp - var ipSubnet cns.IPSubnet - ipSubnet.IPAddress = primaryIp - ipSubnet.PrefixLength = subnetPrfixLength - ipConfig.IPSubnet = ipSubnet - secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) - - for _, secIpAddress := range secondaryIps { - secIpConfig := cns.SecondaryIPConfig{ - IPAddress: secIpAddress, - NCVersion: -1, - } - ipId := uuid.New() - secondaryIPConfigs[ipId.String()] = secIpConfig - } - - req := &cns.CreateNetworkContainerRequest{ - NetworkContainerType: dockerContainerType, - NetworkContainerid: "testNcId1", - IPConfiguration: ipConfig, - SecondaryIPConfigs: secondaryIPConfigs, - // Set it as -1 to be same as default host version. - // It will allow secondary IPs status to be set as available. - Version: "-1", - } - - returnCode := svc.CreateOrUpdateNetworkContainerInternal(req) - if returnCode != 0 { - t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) - } - - _ = svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ - Spec: v1alpha.NodeNetworkConfigSpec{ - RequestedIPCount: 16, - IPsNotInUse: []string{"abc"}, - }, - Status: v1alpha.NodeNetworkConfigStatus{ - Scaler: v1alpha.Scaler{ - BatchSize: batchSize, - ReleaseThresholdPercent: releasePercent, - RequestThresholdPercent: requestPercent, - MaxIPCount: 250, - }, - NetworkContainers: []v1alpha.NetworkContainer{ - {}, - }, - }, - }) -} - -func getIPNetFromResponse(resp *cns.IPConfigResponse) (net.IPNet, error) { - var ( - resultIPnet net.IPNet - err error - ) - - // set result ipconfig from CNS Response Body - prefix := strconv.Itoa(int(resp.PodIpInfo.PodIPConfig.PrefixLength)) - ip, ipnet, err := net.ParseCIDR(resp.PodIpInfo.PodIPConfig.IPAddress + "/" + prefix) - if err != nil { - return resultIPnet, err - } - - // construct ipnet for result - resultIPnet = net.IPNet{ - IP: ip, - Mask: ipnet.Mask, - } - return resultIPnet, err -} - func TestMain(m *testing.M) { - var ( - info = &cns.SetOrchestratorTypeRequest{ - OrchestratorType: cns.KubernetesCRD, - } - body bytes.Buffer - res *http.Response - ) - tmpFileState, err := os.CreateTemp(os.TempDir(), "cns-*.json") tmpLogDir, err := os.MkdirTemp("", "cns-") fmt.Printf("logdir: %+v", tmpLogDir) @@ -167,229 +72,11 @@ func TestMain(m *testing.M) { } logger.InitLogger(logName, 0, 0, tmpLogDir+"/") - config := common.ServiceConfig{} - - httpRestService, err := restserver.NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, nil, nil, nil) - svc = httpRestService - httpRestService.Name = "cns-test-server" - fakeNNC := v1alpha.NodeNetworkConfig{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{}, - Spec: v1alpha.NodeNetworkConfigSpec{ - RequestedIPCount: 16, - IPsNotInUse: []string{"abc"}, - }, - Status: v1alpha.NodeNetworkConfigStatus{ - Scaler: v1alpha.Scaler{ - BatchSize: 10, - ReleaseThresholdPercent: 150, - RequestThresholdPercent: 50, - MaxIPCount: 250, - }, - NetworkContainers: []v1alpha.NetworkContainer{ - { - ID: "nc1", - PrimaryIP: "10.0.0.11", - SubnetName: "sub1", - IPAssignments: []v1alpha.IPAssignment{ - { - Name: "ip1", - IP: "10.0.0.10", - }, - }, - DefaultGateway: "10.0.0.1", - SubnetAddressSpace: "10.0.0.0/24", - Version: 2, - }, - }, - }, - } - httpRestService.IPAMPoolMonitor = &fakes.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} - - if err != nil { - logger.Errorf("Failed to create CNS object, err:%v.\n", err) - return - } - - if httpRestService != nil { - err = httpRestService.Init(&config) - if err != nil { - logger.Errorf("Failed to initialize HttpService, err:%v.\n", err) - return - } - - err = httpRestService.Start(&config) - if err != nil { - logger.Errorf("Failed to start HttpService, err:%v.\n", err) - return - } - } - - if err := json.NewEncoder(&body).Encode(info); err != nil { - log.Errorf("encoding json failed with %v", err) - return - } - - httpc := &http.Client{} - url := defaultBaseURL + cns.SetOrchestratorType - - res, err = httpc.Post(url, "application/json", &body) - if err != nil { - fmt.Println(err) - } - fmt.Println(res) exitCode := m.Run() os.Exit(exitCode) } -func TestCNSClientRequestAndRelease(t *testing.T) { - podName := "testpodname" - podNamespace := "testpodnamespace" - desiredIpAddress := "10.0.0.5" - ip := net.ParseIP(desiredIpAddress) - _, ipnet, _ := net.ParseCIDR("10.0.0.5/24") - desired := net.IPNet{ - IP: ip, - Mask: ipnet.Mask, - } - - secondaryIps := make([]string, 0) - secondaryIps = append(secondaryIps, desiredIpAddress) - cnsClient, _ := New("", 2*time.Hour) - - addTestStateToRestServer(t, secondaryIps) - - podInfo := cns.KubernetesPodInfo{PodName: podName, PodNamespace: podNamespace} - orchestratorContext, err := json.Marshal(podInfo) - assert.NoError(t, err) - - // no IP reservation found with that context, expect no failure. - err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Release ip idempotent call failed") - - // request IP address - resp, err := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "get IP from CNS failed") - - podIPInfo := resp.PodIpInfo - assert.Equal(t, primaryIp, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, "PrimaryIP is not added as epected ipConfig") - assert.EqualValues(t, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.PrefixLength, subnetPrfixLength, "Primary IP Prefix length is not added as expected ipConfig") - - // validate DnsServer and Gateway Ip as the same configured for Primary IP - assert.Equal(t, dnsServers, podIPInfo.NetworkContainerPrimaryIPConfig.DNSServers, "DnsServer is not added as expected ipConfig") - assert.Equal(t, gatewayIp, podIPInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress, "Gateway is not added as expected ipConfig") - - resultIPnet, err := getIPNetFromResponse(resp) - - assert.Equal(t, desired, resultIPnet, "Desired result not matching actual result") - - // checking for assigned IP address and pod context printing before ReleaseIPAddress is called - ipaddresses, err := cnsClient.GetIPAddressesMatchingStates(context.TODO(), types.Assigned) - assert.NoError(t, err, "Get assigned IP addresses failed") - - assert.Len(t, ipaddresses, 1, "Number of available IP addresses expected to be 1") - assert.Equal(t, desiredIpAddress, ipaddresses[0].IPAddress, "Available IP address does not match expected, address state") - assert.Equal(t, types.Assigned, ipaddresses[0].GetState(), "Available IP address does not match expected, address state") - - t.Log(ipaddresses) - - addresses := make([]string, len(ipaddresses)) - for i := range ipaddresses { - addresses[i] = ipaddresses[i].IPAddress - } - - // release requested IP address, expect success - err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{DesiredIPAddress: ipaddresses[0].IPAddress, OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") -} - -func TestCNSClientPodContextApi(t *testing.T) { - podName := "testpodname" - podNamespace := "testpodnamespace" - desiredIpAddress := "10.0.0.5" - - secondaryIps := []string{desiredIpAddress} - cnsClient, _ := New("", 2*time.Second) - - addTestStateToRestServer(t, secondaryIps) - - podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podName, podNamespace) - orchestratorContext, err := json.Marshal(podInfo) - assert.NoError(t, err) - - // request IP address - _, err = cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "get IP from CNS failed") - - // test for pod ip by orch context map - podcontext, err := cnsClient.GetPodOrchestratorContext(context.TODO()) - assert.NoError(t, err, "Get pod ip by orchestrator context failed") - assert.GreaterOrEqual(t, len(podcontext), 1, "Expected at least 1 entry in map for podcontext") - - t.Log(podcontext) - - // release requested IP address, expect success - err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") -} - -func TestCNSClientDebugAPI(t *testing.T) { - podName := "testpodname" - podNamespace := "testpodnamespace" - desiredIpAddress := "10.0.0.5" - - secondaryIps := []string{desiredIpAddress} - cnsClient, _ := New("", 2*time.Hour) - - addTestStateToRestServer(t, secondaryIps) - - podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podName, podNamespace) - orchestratorContext, err := json.Marshal(podInfo) - assert.NoError(t, err) - - // request IP address - _, err1 := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err1, "get IP from CNS failed") - - // test for debug api/cmd to get inmemory data from HTTPRestService - inmemory, err := cnsClient.GetHTTPServiceData(context.TODO()) - assert.NoError(t, err, "Get in-memory http REST Struct failed") - - assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey), 1, "OrchestratorContext map is expected but not returned") - - // testing Pod IP Configuration Status values set for test - podConfig := inmemory.HTTPRestServiceData.PodIPConfigState - for _, v := range podConfig { - assert.Equal(t, "10.0.0.5", v.IPAddress, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) - assert.Equal(t, types.Assigned, v.GetState(), "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) - assert.Equal(t, "testNcId1", v.NCID, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) - } - assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPConfigState), 1, "PodIpConfigState with at least 1 entry expected") - - testIpamPoolMonitor := inmemory.HTTPRestServiceData.IPAMPoolMonitor - assert.EqualValues(t, 5, testIpamPoolMonitor.MinimumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") - assert.EqualValues(t, 15, testIpamPoolMonitor.MaximumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") - assert.EqualValues(t, 13, testIpamPoolMonitor.UpdatingIpsNotInUseCount, "IPAMPoolMonitor state is not reflecting the initial set values") - - // check for cached NNC Spec struct values - assert.EqualValues(t, 16, testIpamPoolMonitor.CachedNNC.Spec.RequestedIPCount, "IPAMPoolMonitor cached NNC Spec is not reflecting the initial set values") - assert.Len(t, testIpamPoolMonitor.CachedNNC.Spec.IPsNotInUse, 1, "IPAMPoolMonitor cached NNC Spec is not reflecting the initial set values") - - // check for cached NNC Status struct values - assert.EqualValues(t, 10, testIpamPoolMonitor.CachedNNC.Status.Scaler.BatchSize, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") - assert.EqualValues(t, 150, testIpamPoolMonitor.CachedNNC.Status.Scaler.ReleaseThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") - assert.EqualValues(t, 50, testIpamPoolMonitor.CachedNNC.Status.Scaler.RequestThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") - assert.Len(t, testIpamPoolMonitor.CachedNNC.Status.NetworkContainers, 1, "Expected only one Network Container in the list") - - t.Logf("In-memory Data: ") - for i := range inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey { - t.Logf("PodIPIDByOrchestratorContext: %+v", inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey[i]) - } - t.Logf("PodIPConfigState: %+v", inmemory.HTTPRestServiceData.PodIPConfigState) - t.Logf("IPAMPoolMonitor: %+v", inmemory.HTTPRestServiceData.IPAMPoolMonitor) -} - func TestNew(t *testing.T) { fqdnBaseURL := "http://testinstance.centraluseuap.cloudapp.azure.com" fqdnWithPortBaseURL := fqdnBaseURL + ":10090" diff --git a/cns/restserver/cnsclient_test.go b/cns/restserver/cnsclient_test.go new file mode 100644 index 0000000000..fbd55aa86e --- /dev/null +++ b/cns/restserver/cnsclient_test.go @@ -0,0 +1,337 @@ +package restserver + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + + "github.com/Azure/azure-container-networking/cns/common" + "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/log" + + "net" + "net/http" + "strconv" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/cns/types" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" +) + +const ( + primaryIp = "10.0.0.5" + gatewayIp = "10.0.0.1" + defaultBaseURL = "http://localhost:10090" + podnametest = "testpodname" + podnamespacetest = "testpodnamespace" +) + +var ( + dnsServers = []string{"8.8.8.8", "8.8.4.4"} +) + +func setUpRestserver() { + var ( + info = &cns.SetOrchestratorTypeRequest{ + OrchestratorType: cns.KubernetesCRD, + } + body bytes.Buffer + res *http.Response + ) + + config := common.ServiceConfig{} + + httpRestService, err := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, nil, nil, nil) + svc = httpRestService + httpRestService.Name = "cns-test-server" + fakeNNC := v1alpha.NodeNetworkConfig{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: 16, + IPsNotInUse: []string{"abc"}, + }, + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: 10, + ReleaseThresholdPercent: 150, + RequestThresholdPercent: 50, + MaxIPCount: 250, + }, + NetworkContainers: []v1alpha.NetworkContainer{ + { + ID: "nc1", + PrimaryIP: "10.0.0.11", + SubnetName: "sub1", + IPAssignments: []v1alpha.IPAssignment{ + { + Name: "ip1", + IP: "10.0.0.10", + }, + }, + DefaultGateway: "10.0.0.1", + SubnetAddressSpace: "10.0.0.0/24", + Version: 2, + }, + }, + }, + } + httpRestService.IPAMPoolMonitor = &fakes.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} + + if err != nil { + logger.Errorf("Failed to create CNS object, err:%v.\n", err) + return + } + + if httpRestService != nil { + err = httpRestService.Init(&config) + if err != nil { + logger.Errorf("Failed to initialize HttpService, err:%v.\n", err) + return + } + + err = httpRestService.Start(&config) + if err != nil { + logger.Errorf("Failed to start HttpService, err:%v.\n", err) + return + } + } + + if err := json.NewEncoder(&body).Encode(info); err != nil { + log.Errorf("encoding json failed with %v", err) + return + } + + httpc := &http.Client{} + url := defaultBaseURL + cns.SetOrchestratorType + + res, err = httpc.Post(url, "application/json", &body) + if err != nil { + fmt.Println(err) + } + fmt.Println(res) +} + +func TestCNSClientRequestAndRelease(t *testing.T) { + desiredIPAddress := "10.0.0.5" + ip := net.ParseIP(desiredIPAddress) + _, ipnet, _ := net.ParseCIDR("10.0.0.5/24") + desired := net.IPNet{ + IP: ip, + Mask: ipnet.Mask, + } + + secondaryIps := make([]string, 0) + secondaryIps = append(secondaryIps, desiredIPAddress) + cnsClient, _ := client.New("", 2*time.Hour) + setUpRestserver() + addTestStateToRestServer(t, secondaryIps) + + podInfo := cns.KubernetesPodInfo{PodName: podnametest, PodNamespace: podnamespacetest} + orchestratorContext, err := json.Marshal(podInfo) + assert.NoError(t, err) + + // no IP reservation found with that context, expect no failure. + err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + assert.NoError(t, err, "Release ip idempotent call failed") + + // request IP address + resp, err := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + assert.NoError(t, err, "get IP from CNS failed") + + podIPInfo := resp.PodIpInfo + assert.Equal(t, primaryIp, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, "PrimaryIP is not added as epected ipConfig") + assert.EqualValues(t, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.PrefixLength, subnetPrfixLength, "Primary IP Prefix length is not added as expected ipConfig") + + // validate DnsServer and Gateway Ip as the same configured for Primary IP + assert.Equal(t, dnsServers, podIPInfo.NetworkContainerPrimaryIPConfig.DNSServers, "DnsServer is not added as expected ipConfig") + assert.Equal(t, gatewayIp, podIPInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress, "Gateway is not added as expected ipConfig") + + resultIPnet, err := getIPNetFromResponse(resp) + + assert.Equal(t, desired, resultIPnet, "Desired result not matching actual result") + + // checking for assigned IP address and pod context printing before ReleaseIPAddress is called + ipaddresses, err := cnsClient.GetIPAddressesMatchingStates(context.TODO(), types.Assigned) + assert.NoError(t, err, "Get assigned IP addresses failed") + + assert.Len(t, ipaddresses, 1, "Number of available IP addresses expected to be 1") + assert.Equal(t, desiredIPAddress, ipaddresses[0].IPAddress, "Available IP address does not match expected, address state") + assert.Equal(t, types.Assigned, ipaddresses[0].GetState(), "Available IP address does not match expected, address state") + + t.Log(ipaddresses) + + addresses := make([]string, len(ipaddresses)) + for i := range ipaddresses { + addresses[i] = ipaddresses[i].IPAddress + } + + // release requested IP address, expect success + err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{DesiredIPAddress: ipaddresses[0].IPAddress, OrchestratorContext: orchestratorContext}) + assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") +} + +func TestCNSClientPodContextApi(t *testing.T) { + desiredIPAddress := "10.0.0.5" + + secondaryIps := []string{desiredIPAddress} + cnsClient, _ := client.New("", 2*time.Second) + + addTestStateToRestServer(t, secondaryIps) + + podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) + orchestratorContext, err := json.Marshal(podInfo) + assert.NoError(t, err) + + // request IP address + _, err = cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + assert.NoError(t, err, "get IP from CNS failed") + + // test for pod ip by orch context map + podcontext, err := cnsClient.GetPodOrchestratorContext(context.TODO()) + assert.NoError(t, err, "Get pod ip by orchestrator context failed") + assert.GreaterOrEqual(t, len(podcontext), 1, "Expected at least 1 entry in map for podcontext") + + t.Log(podcontext) + + // release requested IP address, expect success + err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") +} + +func TestCNSClientDebugAPI(t *testing.T) { + desiredIPAddress := "10.0.0.5" + + secondaryIPs := []string{desiredIPAddress} + cnsClient, _ := client.New("", 2*time.Hour) + + addTestStateToRestServer(t, secondaryIPs) + + podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) + orchestratorContext, err := json.Marshal(podInfo) + assert.NoError(t, err) + + // request IP address + _, err1 := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + assert.NoError(t, err1, "get IP from CNS failed") + + // test for debug api/cmd to get inmemory data from HTTPRestService + inmemory, err := cnsClient.GetHTTPServiceData(context.TODO()) + assert.NoError(t, err, "Get in-memory http REST Struct failed") + + assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey), 1, "OrchestratorContext map is expected but not returned") + + // testing Pod IP Configuration Status values set for test + podConfig := inmemory.HTTPRestServiceData.PodIPConfigState + for _, v := range podConfig { + assert.Equal(t, "10.0.0.5", v.IPAddress, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) + assert.Equal(t, types.Assigned, v.GetState(), "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) + assert.Equal(t, "testNcId1", v.NCID, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) + } + assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPConfigState), 1, "PodIpConfigState with at least 1 entry expected") + + testIpamPoolMonitor := inmemory.HTTPRestServiceData.IPAMPoolMonitor + assert.EqualValues(t, 5, testIpamPoolMonitor.MinimumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") + assert.EqualValues(t, 15, testIpamPoolMonitor.MaximumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") + assert.EqualValues(t, 13, testIpamPoolMonitor.UpdatingIpsNotInUseCount, "IPAMPoolMonitor state is not reflecting the initial set values") + + // check for cached NNC Spec struct values + assert.EqualValues(t, 16, testIpamPoolMonitor.CachedNNC.Spec.RequestedIPCount, "IPAMPoolMonitor cached NNC Spec is not reflecting the initial set values") + assert.Len(t, testIpamPoolMonitor.CachedNNC.Spec.IPsNotInUse, 1, "IPAMPoolMonitor cached NNC Spec is not reflecting the initial set values") + + // check for cached NNC Status struct values + assert.EqualValues(t, 10, testIpamPoolMonitor.CachedNNC.Status.Scaler.BatchSize, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") + assert.EqualValues(t, 150, testIpamPoolMonitor.CachedNNC.Status.Scaler.ReleaseThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") + assert.EqualValues(t, 50, testIpamPoolMonitor.CachedNNC.Status.Scaler.RequestThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") + assert.Len(t, testIpamPoolMonitor.CachedNNC.Status.NetworkContainers, 1, "Expected only one Network Container in the list") + + t.Logf("In-memory Data: ") + for i := range inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey { + t.Logf("PodIPIDByOrchestratorContext: %+v", inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey[i]) + } + t.Logf("PodIPConfigState: %+v", inmemory.HTTPRestServiceData.PodIPConfigState) + t.Logf("IPAMPoolMonitor: %+v", inmemory.HTTPRestServiceData.IPAMPoolMonitor) +} + +func addTestStateToRestServer(t *testing.T, secondaryIps []string) { + var ipConfig cns.IPConfiguration + ipConfig.DNSServers = dnsServers + ipConfig.GatewayIPAddress = gatewayIp + var ipSubnet cns.IPSubnet + ipSubnet.IPAddress = primaryIp + ipSubnet.PrefixLength = subnetPrfixLength + ipConfig.IPSubnet = ipSubnet + secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) + + for _, secIpAddress := range secondaryIps { + secIpConfig := cns.SecondaryIPConfig{ + IPAddress: secIpAddress, + NCVersion: -1, + } + ipId := uuid.New() + secondaryIPConfigs[ipId.String()] = secIpConfig + } + + req := &cns.CreateNetworkContainerRequest{ + NetworkContainerType: dockerContainerType, + NetworkContainerid: "testNcId1", + IPConfiguration: ipConfig, + SecondaryIPConfigs: secondaryIPConfigs, + // Set it as -1 to be same as default host version. + // It will allow secondary IPs status to be set as available. + Version: "-1", + } + + returnCode := svc.CreateOrUpdateNetworkContainerInternal(req) + if returnCode != 0 { + t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) + } + + _ = svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: 16, + IPsNotInUse: []string{"abc"}, + }, + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + MaxIPCount: 250, + }, + NetworkContainers: []v1alpha.NetworkContainer{ + {}, + }, + }, + }) +} + +func getIPNetFromResponse(resp *cns.IPConfigResponse) (net.IPNet, error) { + var ( + resultIPnet net.IPNet + err error + ) + + // set result ipconfig from CNS Response Body + prefix := strconv.Itoa(int(resp.PodIpInfo.PodIPConfig.PrefixLength)) + ip, ipnet, err := net.ParseCIDR(resp.PodIpInfo.PodIPConfig.IPAddress + "/" + prefix) + if err != nil { + return resultIPnet, err + } + + // construct ipnet for result + resultIPnet = net.IPNet{ + IP: ip, + Mask: ipnet.Mask, + } + return resultIPnet, err +} From 1df0fa85777d9fbcc138255ce2bb722bf5b9e6b2 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Tue, 6 Feb 2024 20:04:10 -0800 Subject: [PATCH 20/25] test: modify tests --- cns/restserver/cnsclient_test.go | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/cns/restserver/cnsclient_test.go b/cns/restserver/cnsclient_test.go index fbd55aa86e..1227d90190 100644 --- a/cns/restserver/cnsclient_test.go +++ b/cns/restserver/cnsclient_test.go @@ -92,25 +92,13 @@ func setUpRestserver() { return } - if httpRestService != nil { - err = httpRestService.Init(&config) - if err != nil { - logger.Errorf("Failed to initialize HttpService, err:%v.\n", err) - return - } - - err = httpRestService.Start(&config) - if err != nil { - logger.Errorf("Failed to start HttpService, err:%v.\n", err) - return - } - } - if err := json.NewEncoder(&body).Encode(info); err != nil { log.Errorf("encoding json failed with %v", err) return } + svc.state.OrchestratorType = cns.KubernetesCRD + svc.IPAMPoolMonitor = &fakes.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} httpc := &http.Client{} url := defaultBaseURL + cns.SetOrchestratorType @@ -141,8 +129,8 @@ func TestCNSClientRequestAndRelease(t *testing.T) { assert.NoError(t, err) // no IP reservation found with that context, expect no failure. - err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Release ip idempotent call failed") + //err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + //assert.NoError(t, err, "Release ip idempotent call failed") // request IP address resp, err := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) @@ -182,10 +170,10 @@ func TestCNSClientRequestAndRelease(t *testing.T) { func TestCNSClientPodContextApi(t *testing.T) { desiredIPAddress := "10.0.0.5" - secondaryIps := []string{desiredIPAddress} cnsClient, _ := client.New("", 2*time.Second) + setUpRestserver() addTestStateToRestServer(t, secondaryIps) podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) @@ -214,6 +202,7 @@ func TestCNSClientDebugAPI(t *testing.T) { secondaryIPs := []string{desiredIPAddress} cnsClient, _ := client.New("", 2*time.Hour) + setUpRestserver() addTestStateToRestServer(t, secondaryIPs) podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) From 50927282437f6d1421c573b72c148ae8463e7817 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Wed, 7 Feb 2024 14:31:56 -0800 Subject: [PATCH 21/25] test: fix constant values used in UT --- cns/client/client_test.go | 9 --- cns/restserver/cnsclient_test.go | 95 ++++++++++++-------------------- 2 files changed, 34 insertions(+), 70 deletions(-) diff --git a/cns/client/client_test.go b/cns/client/client_test.go index 0c19c32282..e7265a5483 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -22,15 +22,6 @@ import ( "github.com/stretchr/testify/require" ) -const ( - subnetPrfixLength = 24 - dockerContainerType = cns.Docker - releasePercent = 150 - requestPercent = 50 - batchSize = 10 - initPoolSize = 10 -) - var ( errBadRequest = errors.New("bad request") ) diff --git a/cns/restserver/cnsclient_test.go b/cns/restserver/cnsclient_test.go index 1227d90190..75e8896951 100644 --- a/cns/restserver/cnsclient_test.go +++ b/cns/restserver/cnsclient_test.go @@ -1,57 +1,37 @@ package restserver import ( - "bytes" "context" "encoding/json" - "fmt" - - "github.com/Azure/azure-container-networking/cns/common" - "github.com/Azure/azure-container-networking/cns/fakes" - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/log" - "net" - "net/http" "strconv" "testing" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/client" + "github.com/Azure/azure-container-networking/cns/common" + "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( - primaryIp = "10.0.0.5" - gatewayIp = "10.0.0.1" - defaultBaseURL = "http://localhost:10090" podnametest = "testpodname" podnamespacetest = "testpodnamespace" + ncid = "440515e4-bf48-4997-8e4c-d650d29b462d" ) -var ( - dnsServers = []string{"8.8.8.8", "8.8.4.4"} -) +var dnsServers = []string{"8.8.8.8", "8.8.4.4"} func setUpRestserver() { - var ( - info = &cns.SetOrchestratorTypeRequest{ - OrchestratorType: cns.KubernetesCRD, - } - body bytes.Buffer - res *http.Response - ) - config := common.ServiceConfig{} httpRestService, err := NewHTTPRestService(&config, &fakes.WireserverClientFake{}, &fakes.WireserverProxyFake{}, &fakes.NMAgentClientFake{}, nil, nil, nil) - svc = httpRestService httpRestService.Name = "cns-test-server" fakeNNC := v1alpha.NodeNetworkConfig{ TypeMeta: metav1.TypeMeta{}, @@ -92,25 +72,14 @@ func setUpRestserver() { return } - if err := json.NewEncoder(&body).Encode(info); err != nil { - log.Errorf("encoding json failed with %v", err) - return - } - + svc = httpRestService + svc = service.(*HTTPRestService) svc.state.OrchestratorType = cns.KubernetesCRD svc.IPAMPoolMonitor = &fakes.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} - httpc := &http.Client{} - url := defaultBaseURL + cns.SetOrchestratorType - - res, err = httpc.Post(url, "application/json", &body) - if err != nil { - fmt.Println(err) - } - fmt.Println(res) } func TestCNSClientRequestAndRelease(t *testing.T) { - desiredIPAddress := "10.0.0.5" + desiredIPAddress := primaryIP ip := net.ParseIP(desiredIPAddress) _, ipnet, _ := net.ParseCIDR("10.0.0.5/24") desired := net.IPNet{ @@ -124,27 +93,28 @@ func TestCNSClientRequestAndRelease(t *testing.T) { setUpRestserver() addTestStateToRestServer(t, secondaryIps) - podInfo := cns.KubernetesPodInfo{PodName: podnametest, PodNamespace: podnamespacetest} + podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) orchestratorContext, err := json.Marshal(podInfo) assert.NoError(t, err) // no IP reservation found with that context, expect no failure. - //err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - //assert.NoError(t, err, "Release ip idempotent call failed") + err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) + assert.NoError(t, err, "Release ip idempotent call failed") // request IP address resp, err := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) assert.NoError(t, err, "get IP from CNS failed") podIPInfo := resp.PodIpInfo - assert.Equal(t, primaryIp, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, "PrimaryIP is not added as epected ipConfig") + assert.Equal(t, primaryIP, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, "PrimaryIP is not added as epected ipConfig") assert.EqualValues(t, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.PrefixLength, subnetPrfixLength, "Primary IP Prefix length is not added as expected ipConfig") // validate DnsServer and Gateway Ip as the same configured for Primary IP assert.Equal(t, dnsServers, podIPInfo.NetworkContainerPrimaryIPConfig.DNSServers, "DnsServer is not added as expected ipConfig") - assert.Equal(t, gatewayIp, podIPInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress, "Gateway is not added as expected ipConfig") + assert.Equal(t, gatewayIP, podIPInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress, "Gateway is not added as expected ipConfig") resultIPnet, err := getIPNetFromResponse(resp) + assert.NoError(t, err, "getIPNetFromResponse failed") assert.Equal(t, desired, resultIPnet, "Desired result not matching actual result") @@ -166,10 +136,11 @@ func TestCNSClientRequestAndRelease(t *testing.T) { // release requested IP address, expect success err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{DesiredIPAddress: ipaddresses[0].IPAddress, OrchestratorContext: orchestratorContext}) assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") + service.Stop() } func TestCNSClientPodContextApi(t *testing.T) { - desiredIPAddress := "10.0.0.5" + desiredIPAddress := primaryIP secondaryIps := []string{desiredIPAddress} cnsClient, _ := client.New("", 2*time.Second) @@ -194,10 +165,11 @@ func TestCNSClientPodContextApi(t *testing.T) { // release requested IP address, expect success err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") + service.Stop() } func TestCNSClientDebugAPI(t *testing.T) { - desiredIPAddress := "10.0.0.5" + desiredIPAddress := primaryIP secondaryIPs := []string{desiredIPAddress} cnsClient, _ := client.New("", 2*time.Hour) @@ -222,9 +194,9 @@ func TestCNSClientDebugAPI(t *testing.T) { // testing Pod IP Configuration Status values set for test podConfig := inmemory.HTTPRestServiceData.PodIPConfigState for _, v := range podConfig { - assert.Equal(t, "10.0.0.5", v.IPAddress, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) + assert.Equal(t, primaryIP, v.IPAddress, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) assert.Equal(t, types.Assigned, v.GetState(), "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) - assert.Equal(t, "testNcId1", v.NCID, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) + assert.Equal(t, ncid, v.NCID, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) } assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPConfigState), 1, "PodIpConfigState with at least 1 entry expected") @@ -249,30 +221,31 @@ func TestCNSClientDebugAPI(t *testing.T) { } t.Logf("PodIPConfigState: %+v", inmemory.HTTPRestServiceData.PodIPConfigState) t.Logf("IPAMPoolMonitor: %+v", inmemory.HTTPRestServiceData.IPAMPoolMonitor) + service.Stop() } func addTestStateToRestServer(t *testing.T, secondaryIps []string) { var ipConfig cns.IPConfiguration ipConfig.DNSServers = dnsServers - ipConfig.GatewayIPAddress = gatewayIp + ipConfig.GatewayIPAddress = gatewayIP var ipSubnet cns.IPSubnet - ipSubnet.IPAddress = primaryIp + ipSubnet.IPAddress = primaryIP ipSubnet.PrefixLength = subnetPrfixLength ipConfig.IPSubnet = ipSubnet secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig) - for _, secIpAddress := range secondaryIps { - secIpConfig := cns.SecondaryIPConfig{ - IPAddress: secIpAddress, + for _, secIPAddress := range secondaryIps { + secIPConfig := cns.SecondaryIPConfig{ + IPAddress: secIPAddress, NCVersion: -1, } - ipId := uuid.New() - secondaryIPConfigs[ipId.String()] = secIpConfig + ipID := uuid.New() + secondaryIPConfigs[ipID.String()] = secIPConfig } req := &cns.CreateNetworkContainerRequest{ NetworkContainerType: dockerContainerType, - NetworkContainerid: "testNcId1", + NetworkContainerid: ncid, IPConfiguration: ipConfig, SecondaryIPConfigs: secondaryIPConfigs, // Set it as -1 to be same as default host version. @@ -293,8 +266,8 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { Status: v1alpha.NodeNetworkConfigStatus{ Scaler: v1alpha.Scaler{ BatchSize: batchSize, - ReleaseThresholdPercent: releasePercent, - RequestThresholdPercent: requestPercent, + ReleaseThresholdPercent: 150, + RequestThresholdPercent: 50, MaxIPCount: 250, }, NetworkContainers: []v1alpha.NetworkContainer{ @@ -314,7 +287,7 @@ func getIPNetFromResponse(resp *cns.IPConfigResponse) (net.IPNet, error) { prefix := strconv.Itoa(int(resp.PodIpInfo.PodIPConfig.PrefixLength)) ip, ipnet, err := net.ParseCIDR(resp.PodIpInfo.PodIPConfig.IPAddress + "/" + prefix) if err != nil { - return resultIPnet, err + return resultIPnet, err //nolint:wrapcheck // we don't need error wrapping for tests } // construct ipnet for result @@ -322,5 +295,5 @@ func getIPNetFromResponse(resp *cns.IPConfigResponse) (net.IPNet, error) { IP: ip, Mask: ipnet.Mask, } - return resultIPnet, err + return resultIPnet, err //nolint:wrapcheck // we don't need error wrapping for tests } From b13f1e28c3155b302247b211a4b73abcec07a0c9 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Wed, 7 Feb 2024 15:47:17 -0800 Subject: [PATCH 22/25] test: fix linter errors --- cns/api.go | 12 ++++++----- cns/restserver/cnsclient_test.go | 35 ++++++++++++++++---------------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/cns/api.go b/cns/api.go index e2bc534874..e8422e7413 100644 --- a/cns/api.go +++ b/cns/api.go @@ -384,13 +384,15 @@ type IPInfo struct { } type GetHTTPServiceDataResponse struct { - HTTPRestServiceData HTTPRestServiceData - Response Response + HTTPRestServiceData HTTPRestServiceData `json:"HTTPRestServiceData"` + Response Response `json:"response"` } // HTTPRestServiceData represents in-memory CNS data in the debug API paths. +// PodInterfaceId is key and value is slice of Pod IP uuids in PodIPIDByPodInterfaceKey +// secondaryipid(uuid) is key for PodIPConfigState type HTTPRestServiceData struct { - PodIPIDByPodInterfaceKey map[string][]string // PodInterfaceId is key and value is slice of Pod IP uuids. - PodIPConfigState map[string]IPConfigurationStatus // secondaryipid(uuid) is key - IPAMPoolMonitor IpamPoolMonitorStateSnapshot + PodIPIDByPodInterfaceKey map[string][]string `json:"podIPIDByPodInterfaceKey"` + PodIPConfigState map[string]IPConfigurationStatus `json:"podIpConfigState"` + IPAMPoolMonitor IpamPoolMonitorStateSnapshot `json:"ipamPoolMonitor"` } diff --git a/cns/restserver/cnsclient_test.go b/cns/restserver/cnsclient_test.go index 75e8896951..10190f887c 100644 --- a/cns/restserver/cnsclient_test.go +++ b/cns/restserver/cnsclient_test.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -95,15 +96,15 @@ func TestCNSClientRequestAndRelease(t *testing.T) { podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) orchestratorContext, err := json.Marshal(podInfo) - assert.NoError(t, err) + require.NoError(t, err) // no IP reservation found with that context, expect no failure. err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Release ip idempotent call failed") + require.NoError(t, err, "Release ip idempotent call failed") // request IP address resp, err := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "get IP from CNS failed") + require.NoError(t, err, "get IP from CNS failed") podIPInfo := resp.PodIpInfo assert.Equal(t, primaryIP, podIPInfo.NetworkContainerPrimaryIPConfig.IPSubnet.IPAddress, "PrimaryIP is not added as epected ipConfig") @@ -114,13 +115,13 @@ func TestCNSClientRequestAndRelease(t *testing.T) { assert.Equal(t, gatewayIP, podIPInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress, "Gateway is not added as expected ipConfig") resultIPnet, err := getIPNetFromResponse(resp) - assert.NoError(t, err, "getIPNetFromResponse failed") + require.NoError(t, err, "getIPNetFromResponse failed") assert.Equal(t, desired, resultIPnet, "Desired result not matching actual result") // checking for assigned IP address and pod context printing before ReleaseIPAddress is called ipaddresses, err := cnsClient.GetIPAddressesMatchingStates(context.TODO(), types.Assigned) - assert.NoError(t, err, "Get assigned IP addresses failed") + require.NoError(t, err, "Get assigned IP addresses failed") assert.Len(t, ipaddresses, 1, "Number of available IP addresses expected to be 1") assert.Equal(t, desiredIPAddress, ipaddresses[0].IPAddress, "Available IP address does not match expected, address state") @@ -135,7 +136,7 @@ func TestCNSClientRequestAndRelease(t *testing.T) { // release requested IP address, expect success err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{DesiredIPAddress: ipaddresses[0].IPAddress, OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") + require.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") service.Stop() } @@ -149,22 +150,21 @@ func TestCNSClientPodContextApi(t *testing.T) { podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) orchestratorContext, err := json.Marshal(podInfo) - assert.NoError(t, err) + require.NoError(t, err) // request IP address _, err = cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "get IP from CNS failed") + require.NoError(t, err, "get IP from CNS failed") // test for pod ip by orch context map podcontext, err := cnsClient.GetPodOrchestratorContext(context.TODO()) - assert.NoError(t, err, "Get pod ip by orchestrator context failed") - assert.GreaterOrEqual(t, len(podcontext), 1, "Expected at least 1 entry in map for podcontext") - + require.NoError(t, err, "Get pod ip by orchestrator context failed") + assert.NotEmpty(t, podcontext, "Expected at least 1 entry in map for podcontext") t.Log(podcontext) // release requested IP address, expect success err = cnsClient.ReleaseIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") + require.NoError(t, err, "Expected to not fail when releasing IP reservation found with context") service.Stop() } @@ -179,17 +179,16 @@ func TestCNSClientDebugAPI(t *testing.T) { podInfo := cns.NewPodInfo("some-guid-1", "abc-eth0", podnametest, podnamespacetest) orchestratorContext, err := json.Marshal(podInfo) - assert.NoError(t, err) + require.NoError(t, err) // request IP address _, err1 := cnsClient.RequestIPAddress(context.TODO(), cns.IPConfigRequest{OrchestratorContext: orchestratorContext}) - assert.NoError(t, err1, "get IP from CNS failed") + require.NoError(t, err1, "get IP from CNS failed") // test for debug api/cmd to get inmemory data from HTTPRestService inmemory, err := cnsClient.GetHTTPServiceData(context.TODO()) - assert.NoError(t, err, "Get in-memory http REST Struct failed") - - assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey), 1, "OrchestratorContext map is expected but not returned") + require.NoError(t, err, "Get in-memory http REST Struct failed") + assert.NotEmpty(t, inmemory.HTTPRestServiceData.PodIPIDByPodInterfaceKey, "OrchestratorContext map is expected but not returned") // testing Pod IP Configuration Status values set for test podConfig := inmemory.HTTPRestServiceData.PodIPConfigState @@ -198,7 +197,7 @@ func TestCNSClientDebugAPI(t *testing.T) { assert.Equal(t, types.Assigned, v.GetState(), "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) assert.Equal(t, ncid, v.NCID, "Not the expected set values for testing IPConfigurationStatus, %+v", podConfig) } - assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPConfigState), 1, "PodIpConfigState with at least 1 entry expected") + assert.NotEmpty(t, inmemory.HTTPRestServiceData.PodIPConfigState, "PodIpConfigState with at least 1 entry expected") testIpamPoolMonitor := inmemory.HTTPRestServiceData.IPAMPoolMonitor assert.EqualValues(t, 5, testIpamPoolMonitor.MinimumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") From ebf6640cde6fcdc2a3ec8c329a589b1f74c457f2 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 8 Feb 2024 11:32:18 -0800 Subject: [PATCH 23/25] remove enableswiftv2, add new field in podipinfo, remove ipconfigsrequest passed as a pointer --- cns/NetworkContainerContract.go | 2 ++ cns/api.go | 2 +- cns/configuration/configuration.go | 1 - cns/restserver/SFSwiftV2.go | 4 ++-- cns/restserver/ipam.go | 25 ++++++++++++------------- cns/restserver/ipam_test.go | 10 +++++----- cns/restserver/k8sSwiftV2.go | 4 ++-- cns/restserver/k8sSwiftV2_test.go | 14 +++++++------- 8 files changed, 31 insertions(+), 31 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index fc7fe16f1a..9462ca3053 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -467,6 +467,8 @@ type PodIpInfo struct { SkipDefaultRoutes bool // Routes to configure on interface Routes []Route + // AddInterfacesDataToPodInfo is set to true for SF SwiftV2 scenario + AddInterfacesDataToPodInfo bool } type HostIPInfo struct { diff --git a/cns/api.go b/cns/api.go index e8422e7413..c1a22f4457 100644 --- a/cns/api.go +++ b/cns/api.go @@ -54,7 +54,7 @@ type HTTPService interface { } // IPConfigsHandlerFunc -type IPConfigsHandlerFunc func(context.Context, *IPConfigsRequest) (*IPConfigsResponse, error) +type IPConfigsHandlerFunc func(context.Context, IPConfigsRequest) (*IPConfigsResponse, error) // IPConfigsHandlerMiddleware type IPConfigsHandlerMiddleware interface { diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 005ed3f83b..631db7b143 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -29,7 +29,6 @@ type CNSConfig struct { ChannelMode string EnablePprof bool EnableSubnetScarcity bool - EnableSwiftV2 bool SWIFTV2Mode SWIFTV2Mode InitializeFromCNI bool ManagedSettings ManagedSettings diff --git a/cns/restserver/SFSwiftV2.go b/cns/restserver/SFSwiftV2.go index ce75497871..d7746c7138 100644 --- a/cns/restserver/SFSwiftV2.go +++ b/cns/restserver/SFSwiftV2.go @@ -18,9 +18,9 @@ type SFSWIFTv2Middleware struct { // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP config requests for SF standalone scenario. This function wraps the default SWIFT request // and release IP configs handlers. func (m *SFSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(_, _ cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { - return func(ctx context.Context, req *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { // unmarshal & retrieve podInfo from OrchestratorContext - podInfo, err := cns.NewPodInfoFromIPConfigsRequest(*req) + podInfo, err := cns.NewPodInfoFromIPConfigsRequest(req) ipConfigsResp := &cns.IPConfigsResponse{ Response: cns.Response{ ReturnCode: types.Success, diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 6791723288..6887f244b1 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -8,7 +8,6 @@ import ( "fmt" "net" "net/http" - "reflect" "strconv" "strings" @@ -30,8 +29,8 @@ var ( ) // requestIPConfigHandlerHelper validates the request, assign IPs and return the IPConfigs -func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, *ipconfigsRequest) +func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -42,7 +41,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context } // record a pod requesting an IP service.podsPendingIPAssignment.Push(podInfo.Key()) - podIPInfo, err := requestIPConfigsHelper(service, *ipconfigsRequest) //nolint:contextcheck // to refactor later + podIPInfo, err := requestIPConfigsHelper(service, ipconfigsRequest) //nolint:contextcheck // to refactor later if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -61,7 +60,7 @@ func (service *HTTPRestService) requestIPConfigHandlerHelper(ctx context.Context }() // Check if http rest service managed endpoint state is set if service.Options[common.OptManageEndpointState] == true { - err = service.updateEndpointState(*ipconfigsRequest, podInfo, podIPInfo) + err = service.updateEndpointState(ipconfigsRequest, podInfo, podIPInfo) if err != nil { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -118,7 +117,7 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r } } - ipConfigsResp, errResp := service.requestIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) //nolint:contextcheck // appease linter + ipConfigsResp, errResp := service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) //nolint:contextcheck // appease linter if errResp != nil { // As this API is expected to return IPConfigResponse, generate it from the IPConfigsResponse returned above reserveResp := &cns.IPConfigResponse{ @@ -170,12 +169,12 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r if service.IPConfigsHandlerMiddleware != nil { // Wrap the default datapath handlers with the middleware wrappedHandler := service.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(service.requestIPConfigHandlerHelper, service.releaseIPConfigHandlerHelper) - ipConfigsResp, err = wrappedHandler(r.Context(), &ipconfigsRequest) - if reflect.DeepEqual(ipConfigsResp.PodIPInfo[0].HostPrimaryIPInfo, cns.HostIPInfo{}) || reflect.DeepEqual(ipConfigsResp.PodIPInfo[0].HostSecondaryIPInfo, cns.HostIPInfo{}) { + ipConfigsResp, err = wrappedHandler(r.Context(), ipconfigsRequest) + if ipConfigsResp.PodIPInfo[0].AddInterfacesDataToPodInfo { ipConfigsResp, err = service.updatePodInfoWithInterfaces(r.Context(), ipConfigsResp) } } else { - ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) // nolint:contextcheck // appease linter + ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) // nolint:contextcheck // appease linter } if err != nil { @@ -286,8 +285,8 @@ func (service *HTTPRestService) updateEndpointState(ipconfigsRequest cns.IPConfi } // releaseIPConfigHandlerHelper validates the request and removes the endpoint associated with the pod -func (service *HTTPRestService) releaseIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, *ipconfigsRequest) +func (service *HTTPRestService) releaseIPConfigHandlerHelper(ctx context.Context, ipconfigsRequest cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, returnCode, returnMessage := service.validateIPConfigsRequest(ctx, ipconfigsRequest) if returnCode != types.Success { return &cns.IPConfigsResponse{ Response: cns.Response{ @@ -367,7 +366,7 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r Ifname: ipconfigRequest.Ifname, } - resp, err := service.releaseIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) + resp, err := service.releaseIPConfigHandlerHelper(r.Context(), ipconfigsRequest) if err != nil { w.Header().Set(cnsReturnCode, resp.Response.ReturnCode.String()) err = service.Listener.Encode(w, &resp) @@ -396,7 +395,7 @@ func (service *HTTPRestService) releaseIPConfigsHandler(w http.ResponseWriter, r return } - resp, err := service.releaseIPConfigHandlerHelper(r.Context(), &ipconfigsRequest) + resp, err := service.releaseIPConfigHandlerHelper(r.Context(), ipconfigsRequest) if err != nil { w.Header().Set(cnsReturnCode, resp.Response.ReturnCode.String()) err = service.Listener.Encode(w, &resp) diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 30f3189ebf..6b1f3234f8 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -1581,7 +1581,7 @@ func TestIPAMReleaseSWIFTV2PodIPSuccess(t *testing.T) { req.DesiredIPAddresses[0] = testIP1 req.DesiredIPAddresses[1] = testIP1v6 // Requesting release ip config for SWIFT V2 pod when mtpnc is not ready, should be a no-op - _, err := svc.releaseIPConfigHandlerHelper(context.TODO(), &req) + _, err := svc.releaseIPConfigHandlerHelper(context.TODO(), req) if err != nil { t.Fatalf("Expected not to fail when requesting to release SWIFT V2 pod due to MTPNC not ready") } @@ -1633,7 +1633,7 @@ func TestIPAMGetK8sSWIFTv2IPSuccess(t *testing.T) { req.DesiredIPAddresses[1] = testIP1v6 wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) - resp, err := wrappedHandler(context.TODO(), &req) + resp, err := wrappedHandler(context.TODO(), req) if err != nil { t.Fatalf("Expected to not fail requesting IPs: %+v", err) } @@ -1689,7 +1689,7 @@ func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { req.DesiredIPAddresses[0] = testIP1 req.DesiredIPAddresses[1] = testIP1v6 wrappedHandler := svc.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(svc.requestIPConfigHandlerHelper, svc.releaseIPConfigHandlerHelper) - _, err := wrappedHandler(context.TODO(), &req) + _, err := wrappedHandler(context.TODO(), req) if err == nil { t.Fatalf("Expected failing requesting IPs due to MTPNC not ready") } @@ -1709,7 +1709,7 @@ func TestIPAMGetK8sSWIFTv2IPFailure(t *testing.T) { req.DesiredIPAddresses[0] = testIP1 req.DesiredIPAddresses[1] = testIP1v6 - _, err = wrappedHandler(context.TODO(), &req) + _, err = wrappedHandler(context.TODO(), req) if err == nil { t.Fatalf("Expected failing requesting IPs due to not able to set routes") } @@ -1731,7 +1731,7 @@ func TestValidateSFIPConfigsRequestFailure(t *testing.T) { svc.AttachIPConfigsHandlerMiddleware(&middleware) // Fail to unmarshal pod info test - failReq := &cns.IPConfigsRequest{ + failReq := cns.IPConfigsRequest{ PodInterfaceID: testPod1Info.InterfaceID(), InfraContainerID: testPod1Info.InfraContainerID(), } diff --git a/cns/restserver/k8sSwiftV2.go b/cns/restserver/k8sSwiftV2.go index 4d19c69df8..ed11020b88 100644 --- a/cns/restserver/k8sSwiftV2.go +++ b/cns/restserver/k8sSwiftV2.go @@ -40,8 +40,8 @@ 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. func (m *K8sSWIFTv2Middleware) IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler cns.IPConfigsHandlerFunc) cns.IPConfigsHandlerFunc { - return func(ctx context.Context, req *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { - podInfo, respCode, message := m.validateIPConfigsRequest(ctx, req) + return func(ctx context.Context, req cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + podInfo, respCode, message := m.validateIPConfigsRequest(ctx, &req) if respCode != types.Success { return &cns.IPConfigsResponse{ diff --git a/cns/restserver/k8sSwiftV2_test.go b/cns/restserver/k8sSwiftV2_test.go index 6dbadd02cc..70d4e0359d 100644 --- a/cns/restserver/k8sSwiftV2_test.go +++ b/cns/restserver/k8sSwiftV2_test.go @@ -21,7 +21,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { t.Setenv(configuration.EnvPodCIDRs, "10.0.1.10/24,16A0:0010:AB00:001E::2/32") t.Setenv(configuration.EnvServiceCIDRs, "10.0.0.0/16,16A0:0010:AB00:0000::/32") t.Setenv(configuration.EnvInfraVNETCIDRs, "10.240.0.1/16,16A0:0020:AB00:0000::/32") - defaultHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + defaultHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return &cns.IPConfigsResponse{ PodIPInfo: []cns.PodIpInfo{ { @@ -41,7 +41,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { }, }, nil } - failureHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + failureHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return nil, nil } wrappedHandler := middleware.IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler) @@ -51,7 +51,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { } b, _ := testPod1Info.OrchestratorContext() happyReq.OrchestratorContext = b - resp, err := wrappedHandler(context.TODO(), &happyReq) + resp, err := wrappedHandler(context.TODO(), happyReq) assert.Equal(t, err, nil) assert.Equal(t, resp.PodIPInfo[2].PodIPConfig.IPAddress, "192.168.0.1") assert.Equal(t, resp.PodIPInfo[2].MacAddress, "00:00:00:00:00:00") @@ -59,7 +59,7 @@ func TestIPConfigsRequestHandlerWrapperSuccess(t *testing.T) { func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()} - defaultHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + defaultHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return &cns.IPConfigsResponse{ PodIPInfo: []cns.PodIpInfo{ { @@ -79,7 +79,7 @@ func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { }, }, nil } - failureHandler := func(context.Context, *cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { + failureHandler := func(context.Context, cns.IPConfigsRequest) (*cns.IPConfigsResponse, error) { return nil, nil } wrappedHandler := middleware.IPConfigsRequestHandlerWrapper(defaultHandler, failureHandler) @@ -90,7 +90,7 @@ func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { } b, _ := testPod4Info.OrchestratorContext() failReq.OrchestratorContext = b - resp, _ := wrappedHandler(context.TODO(), &failReq) + resp, _ := wrappedHandler(context.TODO(), failReq) assert.Equal(t, resp.Response.Message, errMTPNCNotReady.Error()) // Failed to set routes @@ -100,7 +100,7 @@ func TestIPConfigsRequestHandlerWrapperFailure(t *testing.T) { } b, _ = testPod1Info.OrchestratorContext() failReq.OrchestratorContext = b - _, err := wrappedHandler(context.TODO(), &failReq) + _, err := wrappedHandler(context.TODO(), failReq) assert.ErrorContains(t, err, "failed to set routes for pod") } From 6513227f50fc0a3ac9003bab69cf4d57c6d8c0e6 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 8 Feb 2024 12:03:50 -0800 Subject: [PATCH 24/25] fix: address comment --- cns/restserver/ipam.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 6887f244b1..2b5800ec8b 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -170,9 +170,6 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r // Wrap the default datapath handlers with the middleware wrappedHandler := service.IPConfigsHandlerMiddleware.IPConfigsRequestHandlerWrapper(service.requestIPConfigHandlerHelper, service.releaseIPConfigHandlerHelper) ipConfigsResp, err = wrappedHandler(r.Context(), ipconfigsRequest) - if ipConfigsResp.PodIPInfo[0].AddInterfacesDataToPodInfo { - ipConfigsResp, err = service.updatePodInfoWithInterfaces(r.Context(), ipConfigsResp) - } } else { ipConfigsResp, err = service.requestIPConfigHandlerHelper(r.Context(), ipconfigsRequest) // nolint:contextcheck // appease linter } @@ -184,6 +181,16 @@ func (service *HTTPRestService) requestIPConfigsHandler(w http.ResponseWriter, r return } + if ipConfigsResp.PodIPInfo[0].AddInterfacesDataToPodInfo { + ipConfigsResp, err = service.updatePodInfoWithInterfaces(r.Context(), ipConfigsResp) + if err != nil { + w.Header().Set(cnsReturnCode, ipConfigsResp.Response.ReturnCode.String()) + err = service.Listener.Encode(w, &ipConfigsResp) + logger.ResponseEx(service.Name+operationName, ipconfigsRequest, ipConfigsResp, ipConfigsResp.Response.ReturnCode, err) + return + } + } + w.Header().Set(cnsReturnCode, ipConfigsResp.Response.ReturnCode.String()) err = service.Listener.Encode(w, &ipConfigsResp) logger.ResponseEx(service.Name+operationName, ipconfigsRequest, ipConfigsResp, ipConfigsResp.Response.ReturnCode, err) From a0c96663072eb9e521d82a53e20841bf474aad49 Mon Sep 17 00:00:00 2001 From: Kshitija Murudi Date: Thu, 8 Feb 2024 12:21:17 -0800 Subject: [PATCH 25/25] fix: remove enableswiftv2, replace by swiftv2mode --- cns/configuration/configuration.go | 3 +-- cns/restserver/SFSwiftV2.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/configuration/configuration.go b/cns/configuration/configuration.go index 2bcb345862..ccc2466d81 100644 --- a/cns/configuration/configuration.go +++ b/cns/configuration/configuration.go @@ -36,7 +36,6 @@ type CNSConfig struct { EnableIPAMv2 bool EnablePprof bool EnableSubnetScarcity bool - SWIFTV2Mode SWIFTV2Mode InitializeFromCNI bool KeyVaultSettings KeyVaultSettings MSISettings MSISettings @@ -220,5 +219,5 @@ func SetCNSConfigDefaults(config *CNSConfig) { if config.AsyncPodDeletePath == "" { config.AsyncPodDeletePath = "/var/run/azure-vnet/deleteIDs" } - config.WatchPods = config.EnableIPAMv2 || config.EnableSwiftV2 + config.WatchPods = config.EnableIPAMv2 || config.SWIFTV2Mode == K8sSWIFTV2 } diff --git a/cns/restserver/SFSwiftV2.go b/cns/restserver/SFSwiftV2.go index d7746c7138..d66ef93806 100644 --- a/cns/restserver/SFSwiftV2.go +++ b/cns/restserver/SFSwiftV2.go @@ -76,6 +76,7 @@ func (m *SFSWIFTv2Middleware) getIPConfig(ctx context.Context, podInfo cns.PodIn NICType: resp.NetworkInterfaceInfo.NICType, SkipDefaultRoutes: false, NetworkContainerPrimaryIPConfig: resp.IPConfiguration, + AddInterfacesDataToPodInfo: true, } return podIPInfo, nil }