From db4166eba78a3eb45e66f916903fc416ffaa67c4 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Thu, 17 Oct 2024 21:38:09 -0700 Subject: [PATCH 01/25] feat: support for cilium + nodesubnet --- .pipelines/pipeline.yaml | 16 +++ .../cilium-nodesubnet-e2e-job-template.yaml | 2 +- .../cilium-nodesubnet-e2e-step-template.yaml | 1 + cns/fakes/nmagentclientfake.go | 11 +- cns/restserver/nodesubnet.go | 56 ++++++++++ cns/restserver/restserver.go | 4 + cns/service/main.go | 103 ++++++++++++------ hack/aks/Makefile | 35 +++--- .../cilium-nodesubnet/ipconfigupdate.go | 12 +- .../ip-masq-agent/config-custom.yaml | 1 + .../ip-masq-agent/config-reconcile.yaml | 1 + 11 files changed, 183 insertions(+), 59 deletions(-) create mode 100644 cns/restserver/nodesubnet.go diff --git a/.pipelines/pipeline.yaml b/.pipelines/pipeline.yaml index 6631f80f02..6fa30d2c87 100644 --- a/.pipelines/pipeline.yaml +++ b/.pipelines/pipeline.yaml @@ -261,6 +261,17 @@ stages: k8sVersion: "" dependsOn: "containerize" + # Cilium Nodesubnet E2E tests + - template: singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml + parameters: + name: "cilium_nodesubnet_e2e" + displayName: Cilium NodeSubnet + clusterType: nodesubnet-byocni-nokubeproxy-up + clusterName: "cilndsubnete2e" + vmSize: Standard_B2s + k8sVersion: "" + dependsOn: "containerize" + # Cilium Overlay E2E tests - template: singletenancy/cilium-overlay/cilium-overlay-e2e-job-template.yaml parameters: @@ -405,6 +416,7 @@ stages: - azure_overlay_stateless_e2e - aks_swift_e2e - cilium_e2e + - cilium_nodesubnet_e2e - cilium_overlay_e2e - cilium_h_overlay_e2e - aks_ubuntu_22_linux_e2e @@ -425,6 +437,10 @@ stages: cilium_e2e: name: cilium_e2e clusterName: "ciliume2e" + region: $(REGION_AKS_CLUSTER_TEST) + cilium_nodesubnet_e2e: + name: cilium_nodesubnet_e2e + clusterName: "cilndsubnete2e" region: $(REGION_AKS_CLUSTER_TEST) cilium_overlay_e2e: name: cilium_overlay_e2e diff --git a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml index 416be76c01..694d0b4e94 100644 --- a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml +++ b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml @@ -71,9 +71,9 @@ stages: os: ${{ parameters.os }} datapath: true dns: true + cni: cilium portforward: true service: true - hostport: true dependsOn: ${{ parameters.name }} - job: failedE2ELogs diff --git a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml index 835a8dd1fd..3e3e889eac 100644 --- a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml +++ b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml @@ -57,6 +57,7 @@ steps: pwd kubectl cluster-info kubectl get po -owide -A + CILIUM_VERSION_TAG=${CILIUM_NODE_SUBNET_VERSION} echo "install Cilium ${CILIUM_VERSION_TAG}" export DIR=${CILIUM_VERSION_TAG%.*} echo "installing files from ${DIR}" diff --git a/cns/fakes/nmagentclientfake.go b/cns/fakes/nmagentclientfake.go index 67988d98e6..a5359ca006 100644 --- a/cns/fakes/nmagentclientfake.go +++ b/cns/fakes/nmagentclientfake.go @@ -14,9 +14,10 @@ import ( // NMAgentClientFake can be used to query to VM Host info. type NMAgentClientFake struct { - SupportedAPIsF func(context.Context) ([]string, error) - GetNCVersionListF func(context.Context) (nmagent.NCVersionList, error) - GetHomeAzF func(context.Context) (nmagent.AzResponse, error) + SupportedAPIsF func(context.Context) ([]string, error) + GetNCVersionListF func(context.Context) (nmagent.NCVersionList, error) + GetHomeAzF func(context.Context) (nmagent.AzResponse, error) + GetInterfaceIPInfoF func(ctx context.Context) (nmagent.Interfaces, error) } func (n *NMAgentClientFake) SupportedAPIs(ctx context.Context) ([]string, error) { @@ -30,3 +31,7 @@ func (n *NMAgentClientFake) GetNCVersionList(ctx context.Context) (nmagent.NCVer func (n *NMAgentClientFake) GetHomeAz(ctx context.Context) (nmagent.AzResponse, error) { return n.GetHomeAzF(ctx) } + +func (n *NMAgentClientFake) GetInterfaceIPInfo(ctx context.Context) (nmagent.Interfaces, error) { + return n.GetInterfaceIPInfoF(ctx) +} diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go new file mode 100644 index 0000000000..ffbf21917b --- /dev/null +++ b/cns/restserver/nodesubnet.go @@ -0,0 +1,56 @@ +package restserver + +import ( + "context" + "net/netip" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" + nodesubnet "github.com/Azure/azure-container-networking/cns/nodesubnet" + "github.com/Azure/azure-container-networking/cns/types" + errors "github.com/pkg/errors" +) + +var _ nodesubnet.IPConsumer = &HTTPRestService{} + +// Implement the UpdateIPsForNodeSubnet method for HTTPRestService +func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr) error { + secondaryIPStrs := make([]string, len(secondaryIPs)) + for i, ip := range secondaryIPs { + secondaryIPStrs[i] = ip.String() + } + + networkContainerRequest := nodesubnet.CreateNodeSubnetNCRequest(secondaryIPStrs) + + code, msg := service.saveNetworkContainerGoalState(*networkContainerRequest) + if code != types.Success { + logger.Debugf("Error in processing IP change") + return errors.Errorf("failed to save fetched ips. code: %d, message %s", code, msg) + } else { + logger.Debugf("IP change processed successfully") + } + + // saved NC successfully, generate conflist to indicate CNS is ready + go service.MustGenerateCNIConflistOnce() + return nil +} + +func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInfoByIPProvider cns.PodInfoByIPProvider) error { + // Set orchestrator type + orchestrator := cns.SetOrchestratorTypeRequest{ + OrchestratorType: cns.KubernetesCRD, + } + service.SetNodeOrchestrator(&orchestrator) + service.nodesubnetIPFetcher = nodesubnet.NewIPFetcher(service.nma, service, 0, 0, logger.Log) + if podInfoByIPProvider == nil { + logger.Printf("PodInfoByIPProvider is nil, this usually means no saved endpoint state. Skipping reconciliation") + } else if _, err := nodesubnet.ReconcileInitialCNSState(ctx, service, podInfoByIPProvider); err != nil { + return errors.Wrap(err, "reconcile initial CNS state") + } + + return nil +} + +func (service *HTTPRestService) StartNodeSubnet(ctx context.Context) { + service.nodesubnetIPFetcher.Start(ctx) +} diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index c74580f29c..9ad6a7a2fc 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -13,11 +13,13 @@ import ( "github.com/Azure/azure-container-networking/cns/dockerclient" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/networkcontainers" + nodesubnet "github.com/Azure/azure-container-networking/cns/nodesubnet" "github.com/Azure/azure-container-networking/cns/routes" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/cns/types/bounded" "github.com/Azure/azure-container-networking/cns/wireserver" acn "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/nmagent" nma "github.com/Azure/azure-container-networking/nmagent" "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" @@ -40,6 +42,7 @@ type nmagentClient interface { SupportedAPIs(context.Context) ([]string, error) GetNCVersionList(context.Context) (nma.NCVersionList, error) GetHomeAz(context.Context) (nma.AzResponse, error) + GetInterfaceIPInfo(ctx context.Context) (nmagent.Interfaces, error) } type wireserverProxy interface { @@ -76,6 +79,7 @@ type HTTPRestService struct { IPConfigsHandlerMiddleware cns.IPConfigsHandlerMiddleware PnpIDByMacAddress map[string]string imdsClient imdsClient + nodesubnetIPFetcher *nodesubnet.IPFetcher } type CNIConflistGenerator interface { diff --git a/cns/service/main.go b/cns/service/main.go index 5219ed25de..66bd058f8d 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -661,6 +661,8 @@ func main() { } if isManaged, ok := acn.GetArg(acn.OptManaged).(bool); ok && isManaged { config.ChannelMode = cns.Managed + } else if cnsconfig.ChannelMode == cns.AzureHost { + config.ChannelMode = cns.AzureHost } homeAzMonitor := restserver.NewHomeAzMonitor(nmaClient, time.Duration(cnsconfig.AZRSettings.PopulateHomeAzCacheRetryIntervalSecs)*time.Second) @@ -736,7 +738,6 @@ func main() { } imdsClient := imds.NewClient() - httpRemoteRestService, err := restserver.NewHTTPRestService(&config, wsclient, &wsProxy, nmaClient, endpointStateStore, conflistGenerator, homeAzMonitor, imdsClient) if err != nil { @@ -871,6 +872,26 @@ func main() { } } + if config.ChannelMode == cns.AzureHost { + if !cnsconfig.ManageEndpointState { + logger.Errorf("[Azure CNS] ManageEndpointState must be set to true for AzureHost mode") + return + } else { + // If cns manageendpointstate is true, then cns maintains its own state and reconciles from it. + // in this case, cns maintains state with containerid as key and so in-memory cache can lookup + // and update based on container id. + cns.GlobalPodInfoScheme = cns.InfraIDPodInfoScheme + } + + podInfoByIPProvider, err := getPodInfoByIPProvider(rootCtx, cnsconfig, httpRemoteRestService, nil, "") + if err != nil { + logger.Errorf("[Azure CNS] Failed to get PodInfoByIPProvider: %v", err) + return + } + + httpRemoteRestService.InitializeNodeSubnet(rootCtx, podInfoByIPProvider) + } + // Initialize multi-tenant controller if the CNS is running in MultiTenantCRD mode. // It must be started before we start HTTPRemoteRestService. if config.ChannelMode == cns.MultiTenantCRD { @@ -909,6 +930,7 @@ func main() { } // if user provides cns url by -c option, then only start HTTP remote server using this url + logger.Printf("[Azure CNS] Start HTTP Remote server") if httpRemoteRestService != nil { if cnsconfig.EnablePprof { @@ -1019,6 +1041,10 @@ func main() { }(privateEndpoint, infravnet, nodeID) } + if config.ChannelMode == cns.AzureHost { + httpRemoteRestService.StartNodeSubnet(rootCtx) + } + // mark the service as "ready" close(readyCh) // block until process exiting @@ -1249,40 +1275,11 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } } - var podInfoByIPProvider cns.PodInfoByIPProvider - switch { - case cnsconfig.ManageEndpointState: - logger.Printf("Initializing from self managed endpoint store") - podInfoByIPProvider, err = cnireconciler.NewCNSPodInfoProvider(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server - if err != nil { - if errors.Is(err, store.ErrKeyNotFound) { - logger.Printf("[Azure CNS] No endpoint state found, skipping initializing CNS state") - } else { - return errors.Wrap(err, "failed to create CNS PodInfoProvider") - } - } - case cnsconfig.InitializeFromCNI: - logger.Printf("Initializing from CNI") - podInfoByIPProvider, err = cnireconciler.NewCNIPodInfoProvider() - if err != nil { - return errors.Wrap(err, "failed to create CNI PodInfoProvider") - } - default: - logger.Printf("Initializing from Kubernetes") - podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { - pods, err := clientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{ //nolint:govet // ignore err shadow - FieldSelector: "spec.nodeName=" + nodeName, - }) - if err != nil { - return nil, errors.Wrap(err, "failed to list Pods for PodInfoProvider") - } - podInfo, err := cns.KubePodsToPodInfoByIP(pods.Items) - if err != nil { - return nil, errors.Wrap(err, "failed to convert Pods to PodInfoByIP") - } - return podInfo, nil - }) + podInfoByIPProvider, err := getPodInfoByIPProvider(ctx, cnsconfig, httpRestServiceImplementation, clientset, nodeName) + if err != nil { + return errors.Wrap(err, "failed to initialize ip state") } + // create scoped kube clients. directcli, err := client.New(kubeConfig, client.Options{Scheme: nodenetworkconfig.Scheme}) if err != nil { @@ -1505,6 +1502,44 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn return nil } +func getPodInfoByIPProvider(ctx context.Context, cnsconfig *configuration.CNSConfig, httpRestServiceImplementation *restserver.HTTPRestService, clientset *kubernetes.Clientset, nodeName string) (podInfoByIPProvider cns.PodInfoByIPProvider, err error) { + switch { + case cnsconfig.ManageEndpointState: + logger.Printf("Initializing from self managed endpoint store") + podInfoByIPProvider, err = cnireconciler.NewCNSPodInfoProvider(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server + if err != nil { + if errors.Is(err, store.ErrKeyNotFound) { + logger.Printf("[Azure CNS] No endpoint state found, skipping initializing CNS state") + } else { + return podInfoByIPProvider, errors.Wrap(err, "failed to create CNS PodInfoProvider") + } + } + case cnsconfig.InitializeFromCNI: + logger.Printf("Initializing from CNI") + podInfoByIPProvider, err = cnireconciler.NewCNIPodInfoProvider() + if err != nil { + return podInfoByIPProvider, errors.Wrap(err, "failed to create CNI PodInfoProvider") + } + default: + logger.Printf("Initializing from Kubernetes") + podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) { + pods, err := clientset.CoreV1().Pods("").List(ctx, metav1.ListOptions{ //nolint:govet // ignore err shadow + FieldSelector: "spec.nodeName=" + nodeName, + }) + if err != nil { + return nil, errors.Wrap(err, "failed to list Pods for PodInfoProvider") + } + podInfo, err := cns.KubePodsToPodInfoByIP(pods.Items) + if err != nil { + return nil, errors.Wrap(err, "failed to convert Pods to PodInfoByIP") + } + return podInfo, nil + }) + } + + return podInfoByIPProvider, nil +} + // createOrUpdateNodeInfoCRD polls imds to learn the VM Unique ID and then creates or updates the NodeInfo CRD // with that vm unique ID func createOrUpdateNodeInfoCRD(ctx context.Context, restConfig *rest.Config, node *corev1.Node) error { diff --git a/hack/aks/Makefile b/hack/aks/Makefile index 6aedf52c01..cdf6928ee3 100644 --- a/hack/aks/Makefile +++ b/hack/aks/Makefile @@ -8,17 +8,18 @@ AZIMG = mcr.microsoft.com/azure-cli AZCLI ?= docker run --rm -v $(AZCFG):/root/.azure -v $(KUBECFG):/root/.kube -v $(SSH):/root/.ssh -v $(PWD):/root/tmpsrc $(AZIMG) az # overrideable defaults -AUTOUPGRADE ?= patch -K8S_VER ?= 1.30 -NODE_COUNT ?= 2 -NODE_COUNT_WIN ?= $(NODE_COUNT) -NODEUPGRADE ?= NodeImage -OS ?= linux # Used to signify if you want to bring up a windows nodePool on byocni clusters -OS_SKU ?= Ubuntu -OS_SKU_WIN ?= Windows2022 -REGION ?= westus2 -VM_SIZE ?= Standard_B2s -VM_SIZE_WIN ?= Standard_B2s +AUTOUPGRADE ?= patch +K8S_VER ?= 1.30 +NODE_COUNT ?= 2 +NODE_COUNT_WIN ?= $(NODE_COUNT) +NODEUPGRADE ?= NodeImage +OS ?= linux # Used to signify if you want to bring up a windows nodePool on byocni clusters +OS_SKU ?= Ubuntu +OS_SKU_WIN ?= Windows2022 +REGION ?= westus2 +VM_SIZE ?= Standard_B2s +VM_SIZE_WIN ?= Standard_B2s +KUBE_PROXY_JSON_PATH ?= ./kube-proxy.json # overrideable variables SUB ?= $(AZURE_SUBSCRIPTION) @@ -103,13 +104,13 @@ nodesubnet-byocni-nokubeproxy-up: rg-up overlay-net-up ## Brings up an NodeSubne --kubernetes-version $(K8S_VER) \ --node-count $(NODE_COUNT) \ --node-vm-size $(VM_SIZE) \ - --load-balancer-sku basic \ + --load-balancer-sku standard \ --max-pods 250 \ --network-plugin none \ --vnet-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/nodenet \ --os-sku $(OS_SKU) \ --no-ssh-key \ - --kube-proxy-config ./kube-proxy.json \ + --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ --yes @$(MAKE) set-kubeconf @@ -146,7 +147,7 @@ overlay-byocni-nokubeproxy-up: rg-up overlay-net-up ## Brings up an Overlay BYO --pod-cidr 192.168.0.0/16 \ --vnet-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/nodenet \ --no-ssh-key \ - --kube-proxy-config ./kube-proxy.json \ + --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ --yes @$(MAKE) set-kubeconf @@ -215,7 +216,7 @@ swift-byocni-nokubeproxy-up: rg-up swift-net-up ## Bring up a SWIFT BYO CNI clus --pod-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/podnet \ --no-ssh-key \ --os-sku $(OS_SKU) \ - --kube-proxy-config ./kube-proxy.json \ + --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ --yes @$(MAKE) set-kubeconf @@ -305,7 +306,7 @@ vnetscale-swift-byocni-nokubeproxy-up: rg-up vnetscale-swift-net-up ## Bring up --pod-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/podnet \ --no-ssh-key \ --os-sku $(OS_SKU) \ - --kube-proxy-config ./kube-proxy.json \ + --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ --yes @$(MAKE) set-kubeconf @@ -434,7 +435,7 @@ dualstack-byocni-nokubeproxy-up: rg-up overlay-net-up ## Brings up a Dualstack o --ip-families ipv4,ipv6 \ --aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/AzureOverlayDualStackPreview \ --no-ssh-key \ - --kube-proxy-config ./kube-proxy.json \ + --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ --yes @$(MAKE) set-kubeconf diff --git a/test/integration/cilium-nodesubnet/ipconfigupdate.go b/test/integration/cilium-nodesubnet/ipconfigupdate.go index 2a8fa8a84c..bb7cf81d07 100644 --- a/test/integration/cilium-nodesubnet/ipconfigupdate.go +++ b/test/integration/cilium-nodesubnet/ipconfigupdate.go @@ -13,13 +13,13 @@ import ( ) func runCommand(command string) (string, error) { - cmd := exec.Command("bash", "-c", command) var out bytes.Buffer var stderr bytes.Buffer - cmd.Stdout = &out - cmd.Stderr = &stderr var err error for i := 0; i < 3; i++ { + cmd := exec.Command("bash", "-c", command) + cmd.Stdout = &out + cmd.Stderr = &stderr err = cmd.Run() if err == nil { break @@ -27,7 +27,7 @@ func runCommand(command string) (string, error) { } if err != nil { - return "", errors.Wrap(err, "command failed") + return "", errors.Wrap(err, fmt.Sprintf("command %s failed ", command)) } return out.String(), nil @@ -104,8 +104,12 @@ func main() { for i := 2; i <= secondaryConfigCount+1; i++ { ipConfig := make(map[string]interface{}) for k, v := range primaryIPConfig { + if k == "loadBalancerBackendAddressPools" { + continue + } ipConfig[k] = v } + ipConfigName := fmt.Sprintf("ipconfig%d", i) if !contains(usedIPConfigNames, ipConfigName) { ipConfig["name"] = ipConfigName diff --git a/test/integration/manifests/ip-masq-agent/config-custom.yaml b/test/integration/manifests/ip-masq-agent/config-custom.yaml index 4bdc6cc5ee..3ace541311 100644 --- a/test/integration/manifests/ip-masq-agent/config-custom.yaml +++ b/test/integration/manifests/ip-masq-agent/config-custom.yaml @@ -13,5 +13,6 @@ data: - 192.168.0.0/16 - 100.64.0.0/10 - 10.244.0.0/16 + - 10.10.0.0/16 masqLinkLocal: false masqLinkLocalIPv6: true diff --git a/test/integration/manifests/ip-masq-agent/config-reconcile.yaml b/test/integration/manifests/ip-masq-agent/config-reconcile.yaml index 0f715267d8..67944cd917 100644 --- a/test/integration/manifests/ip-masq-agent/config-reconcile.yaml +++ b/test/integration/manifests/ip-masq-agent/config-reconcile.yaml @@ -11,4 +11,5 @@ data: - 192.168.0.0/16 - 100.64.0.0/10 - 10.244.0.0/16 + - 10.10.0.0/16 masqLinkLocal: true From 9cc4ec676c28bd01687e24f7954d4486aee87b0b Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Fri, 18 Oct 2024 16:08:05 -0700 Subject: [PATCH 02/25] fix: make linter happy --- cns/restserver/nodesubnet.go | 4 ++-- cns/restserver/restserver.go | 3 +-- cns/service/main.go | 9 +++++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index ffbf21917b..928f759bb9 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -26,10 +26,10 @@ func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr if code != types.Success { logger.Debugf("Error in processing IP change") return errors.Errorf("failed to save fetched ips. code: %d, message %s", code, msg) - } else { - logger.Debugf("IP change processed successfully") } + logger.Debugf("IP change processed successfully") + // saved NC successfully, generate conflist to indicate CNS is ready go service.MustGenerateCNIConflistOnce() return nil diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 9ad6a7a2fc..ec9e6382d8 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -19,7 +19,6 @@ import ( "github.com/Azure/azure-container-networking/cns/types/bounded" "github.com/Azure/azure-container-networking/cns/wireserver" acn "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/nmagent" nma "github.com/Azure/azure-container-networking/nmagent" "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" @@ -42,7 +41,7 @@ type nmagentClient interface { SupportedAPIs(context.Context) ([]string, error) GetNCVersionList(context.Context) (nma.NCVersionList, error) GetHomeAz(context.Context) (nma.AzResponse, error) - GetInterfaceIPInfo(ctx context.Context) (nmagent.Interfaces, error) + GetInterfaceIPInfo(ctx context.Context) (nma.Interfaces, error) } type wireserverProxy interface { diff --git a/cns/service/main.go b/cns/service/main.go index 66bd058f8d..3fb0211e64 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -883,13 +883,18 @@ func main() { cns.GlobalPodInfoScheme = cns.InfraIDPodInfoScheme } - podInfoByIPProvider, err := getPodInfoByIPProvider(rootCtx, cnsconfig, httpRemoteRestService, nil, "") + var podInfoByIPProvider cns.PodInfoByIPProvider + podInfoByIPProvider, err = getPodInfoByIPProvider(rootCtx, cnsconfig, httpRemoteRestService, nil, "") if err != nil { logger.Errorf("[Azure CNS] Failed to get PodInfoByIPProvider: %v", err) return } - httpRemoteRestService.InitializeNodeSubnet(rootCtx, podInfoByIPProvider) + err = httpRemoteRestService.InitializeNodeSubnet(rootCtx, podInfoByIPProvider) + if err != nil { + logger.Errorf("[Azure CNS] Failed to initialize node subnet: %v", err) + return + } } // Initialize multi-tenant controller if the CNS is running in MultiTenantCRD mode. From 107ebce64c040c8fe39f6c3b21e5db80a8234214 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Fri, 18 Oct 2024 16:12:37 -0700 Subject: [PATCH 03/25] fix: make linter happy --- cns/service/main.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index 3fb0211e64..503b166a33 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -876,13 +876,13 @@ func main() { if !cnsconfig.ManageEndpointState { logger.Errorf("[Azure CNS] ManageEndpointState must be set to true for AzureHost mode") return - } else { - // If cns manageendpointstate is true, then cns maintains its own state and reconciles from it. - // in this case, cns maintains state with containerid as key and so in-memory cache can lookup - // and update based on container id. - cns.GlobalPodInfoScheme = cns.InfraIDPodInfoScheme } + // If cns manageendpointstate is true, then cns maintains its own state and reconciles from it. + // in this case, cns maintains state with containerid as key and so in-memory cache can lookup + // and update based on container id. + cns.GlobalPodInfoScheme = cns.InfraIDPodInfoScheme + var podInfoByIPProvider cns.PodInfoByIPProvider podInfoByIPProvider, err = getPodInfoByIPProvider(rootCtx, cnsconfig, httpRemoteRestService, nil, "") if err != nil { From 07a8575fd89e5c43272144b369946ecf55da29e1 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Fri, 18 Oct 2024 16:29:57 -0700 Subject: [PATCH 04/25] fix: make linter happy --- cns/service/main.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cns/service/main.go b/cns/service/main.go index 503b166a33..062bcf365a 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1507,7 +1507,13 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn return nil } -func getPodInfoByIPProvider(ctx context.Context, cnsconfig *configuration.CNSConfig, httpRestServiceImplementation *restserver.HTTPRestService, clientset *kubernetes.Clientset, nodeName string) (podInfoByIPProvider cns.PodInfoByIPProvider, err error) { +func getPodInfoByIPProvider( + ctx context.Context, + cnsconfig *configuration.CNSConfig, + httpRestServiceImplementation *restserver.HTTPRestService, + clientset *kubernetes.Clientset, + nodeName string, +) (podInfoByIPProvider cns.PodInfoByIPProvider, err error) { switch { case cnsconfig.ManageEndpointState: logger.Printf("Initializing from self managed endpoint store") From c59f2dccfb7cc47b0e747f19fa91a281bddbff3c Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Mon, 21 Oct 2024 11:24:28 -0700 Subject: [PATCH 05/25] test: add test for nodesubnet --- cns/restserver/nodesubnet.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 928f759bb9..310718f6f6 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -41,12 +41,14 @@ func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInf OrchestratorType: cns.KubernetesCRD, } service.SetNodeOrchestrator(&orchestrator) - service.nodesubnetIPFetcher = nodesubnet.NewIPFetcher(service.nma, service, 0, 0, logger.Log) + if podInfoByIPProvider == nil { logger.Printf("PodInfoByIPProvider is nil, this usually means no saved endpoint state. Skipping reconciliation") } else if _, err := nodesubnet.ReconcileInitialCNSState(ctx, service, podInfoByIPProvider); err != nil { return errors.Wrap(err, "reconcile initial CNS state") } + // statefile (if any) is reconciled. Fetch secondary IPs and signal readiness + service.nodesubnetIPFetcher = nodesubnet.NewIPFetcher(service.nma, service, 0, 0, logger.Log) return nil } From fbd60a505e822d37e0a1743beddefa9eba9362b5 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Mon, 21 Oct 2024 11:26:01 -0700 Subject: [PATCH 06/25] chore: add missing files --- cns/restserver/helper_for_nodesubnet_test.go | 80 ++++++++++++++++++++ cns/restserver/nodesubnet_test.go | 78 +++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 cns/restserver/helper_for_nodesubnet_test.go create mode 100644 cns/restserver/nodesubnet_test.go diff --git a/cns/restserver/helper_for_nodesubnet_test.go b/cns/restserver/helper_for_nodesubnet_test.go new file mode 100644 index 0000000000..2acb96d775 --- /dev/null +++ b/cns/restserver/helper_for_nodesubnet_test.go @@ -0,0 +1,80 @@ +package restserver + +import ( + "context" + "net/netip" + + "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/nodesubnet" + acn "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/nmagent" + "github.com/Azure/azure-container-networking/store" +) + +func GetRestServiceObjectForNodeSubnetTest(generator CNIConflistGenerator) *HTTPRestService { + config := &common.ServiceConfig{ + Name: "test", + Version: "1.0", + ChannelMode: "AzureHost", + Store: store.NewMockStore("test"), + } + interfaces := nmagent.Interfaces{ + Entries: []nmagent.Interface{ + { + MacAddress: nmagent.MACAddress{0x00, 0x0D, 0x3A, 0xF9, 0xDC, 0xA6}, + IsPrimary: true, + InterfaceSubnets: []nmagent.InterfaceSubnet{ + { + Prefix: "10.240.0.0/16", + IPAddress: []nmagent.NodeIP{ + { + Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 240, 0, 5})), + IsPrimary: true, + }, + { + Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 240, 0, 6})), + IsPrimary: false, + }, + }, + }, + }, + }, + }, + } + + svc, err := cns.NewService(config.Name, config.Version, config.ChannelMode, config.Store) + if err != nil { + return nil + } + + svc.SetOption(acn.OptCnsURL, "") + svc.SetOption(acn.OptCnsPort, "") + err = svc.Initialize(config) + if err != nil { + return nil + } + + return &HTTPRestService{ + Service: svc, + cniConflistGenerator: generator, + state: &httpRestServiceState{}, + PodIPConfigState: make(map[string]cns.IPConfigurationStatus), + nma: &fakes.NMAgentClientFake{ + GetInterfaceIPInfoF: func(_ context.Context) (nmagent.Interfaces, error) { + return interfaces, nil + }, + }, + } +} + +// SetCNIConflistGenerator sets the CNIConflistGenerator for the HTTPRestService. +func (service *HTTPRestService) SetCNIConflistGenerator(generator CNIConflistGenerator) { + service.cniConflistGenerator = generator +} + +// GetNodesubnetIPFetcher gets the nodesubnet.IPFetcher from the HTTPRestService. +func (service *HTTPRestService) GetNodesubnetIPFetcher() *nodesubnet.IPFetcher { + return service.nodesubnetIPFetcher +} diff --git a/cns/restserver/nodesubnet_test.go b/cns/restserver/nodesubnet_test.go new file mode 100644 index 0000000000..9bd4f133cf --- /dev/null +++ b/cns/restserver/nodesubnet_test.go @@ -0,0 +1,78 @@ +package restserver_test + +import ( + "context" + "testing" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/restserver" +) + +// Mock implementation of PodInfoByIPProvider +type MockPodInfoByIPProvider struct{} + +func (m *MockPodInfoByIPProvider) PodInfoByIP() (res map[string]cns.PodInfo, err error) { + return res, nil +} + +// Mock implementation of CNIConflistGenerator +type MockCNIConflistGenerator struct { + GenerateCalled chan bool +} + +func (m *MockCNIConflistGenerator) Generate() error { + m.GenerateCalled <- true + return nil +} + +func (m *MockCNIConflistGenerator) Close() error { + // Implement the Close method logic here if needed + return nil +} + +func TestNodeSubnet(t *testing.T) { + mockPodInfoProvider := new(MockPodInfoByIPProvider) + + // Create a real HTTPRestService object + mockCNIConflistGenerator := &MockCNIConflistGenerator{ + GenerateCalled: make(chan bool), + } + service := restserver.GetRestServiceObjectForNodeSubnetTest(mockCNIConflistGenerator) + defer service.Service.Uninitialize() + + ctx, cancel := testContext(t) + defer cancel() + + err := service.InitializeNodeSubnet(ctx, mockPodInfoProvider) + service.StartNodeSubnet(ctx) + + if err != nil { + t.Errorf("InitializeNodeSubnet returned an error: %v", err) + } + + if service.GetNodesubnetIPFetcher() == nil { + t.Error("NodeSubnetIPFetcher is not initialized") + } + + select { + case <-ctx.Done(): + t.Error("Test context was canceled before conflist generation") + return + case <-mockCNIConflistGenerator.GenerateCalled: + break + } +} + +// testContext creates a context from the provided testing.T that will be +// canceled if the test suite is terminated. +func testContext(t *testing.T) (context.Context, context.CancelFunc) { + if deadline, ok := t.Deadline(); ok { + return context.WithDeadline(context.Background(), deadline) + } + return context.WithCancel(context.Background()) +} + +func init() { + logger.InitLogger("testlogs", 0, 0, "./") +} From 4ee1a9025c5247f6f7cfd47b47036ab73fc0b909 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Mon, 21 Oct 2024 15:21:19 -0700 Subject: [PATCH 07/25] nicer comment --- cns/restserver/nodesubnet.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 310718f6f6..54962233f0 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -47,7 +47,8 @@ func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInf } else if _, err := nodesubnet.ReconcileInitialCNSState(ctx, service, podInfoByIPProvider); err != nil { return errors.Wrap(err, "reconcile initial CNS state") } - // statefile (if any) is reconciled. Fetch secondary IPs and signal readiness + // statefile (if any) is reconciled. Initalize the IP fetcher. Start the IP fetcher only after the service is started, + // and any pending async delete operations are completed. service.nodesubnetIPFetcher = nodesubnet.NewIPFetcher(service.nma, service, 0, 0, logger.Log) return nil From f94cb77e05905f606fdb51b4e0f0abf85250b099 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Mon, 21 Oct 2024 16:15:51 -0700 Subject: [PATCH 08/25] chore: fix comment typo --- cns/restserver/nodesubnet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 54962233f0..92221c8fa9 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -47,7 +47,7 @@ func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInf } else if _, err := nodesubnet.ReconcileInitialCNSState(ctx, service, podInfoByIPProvider); err != nil { return errors.Wrap(err, "reconcile initial CNS state") } - // statefile (if any) is reconciled. Initalize the IP fetcher. Start the IP fetcher only after the service is started, + // statefile (if any) is reconciled. Initialize the IP fetcher. Start the IP fetcher only after the service is started, // and any pending async delete operations are completed. service.nodesubnetIPFetcher = nodesubnet.NewIPFetcher(service.nma, service, 0, 0, logger.Log) From a1c4bfeb673ff15cb65721250c4825b8ccf11b02 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu <6684582+santhoshmprabhu@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:13:45 -0700 Subject: [PATCH 09/25] fix: update cns/restserver/nodesubnet.go Co-authored-by: Timothy J. Raymond Signed-off-by: Santhosh Prabhu <6684582+santhoshmprabhu@users.noreply.github.com> --- cns/restserver/nodesubnet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 92221c8fa9..cba2e3ec63 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -8,7 +8,7 @@ import ( "github.com/Azure/azure-container-networking/cns/logger" nodesubnet "github.com/Azure/azure-container-networking/cns/nodesubnet" "github.com/Azure/azure-container-networking/cns/types" - errors "github.com/pkg/errors" + "github.com/pkg/errors" ) var _ nodesubnet.IPConsumer = &HTTPRestService{} From 2f15117c7d2039fbb4c29bd4bc48f30bd7a84d13 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu <6684582+santhoshmprabhu@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:21:41 -0700 Subject: [PATCH 10/25] fix: update cns/restserver/restserver.go Co-authored-by: Timothy J. Raymond Signed-off-by: Santhosh Prabhu <6684582+santhoshmprabhu@users.noreply.github.com> --- cns/restserver/restserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index ec9e6382d8..30a2b1e8d6 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -13,7 +13,7 @@ import ( "github.com/Azure/azure-container-networking/cns/dockerclient" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/networkcontainers" - nodesubnet "github.com/Azure/azure-container-networking/cns/nodesubnet" + "github.com/Azure/azure-container-networking/cns/nodesubnet" "github.com/Azure/azure-container-networking/cns/routes" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/cns/types/bounded" From 6e8bda077898b6df19198e6ed9df05d2d660d99b Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Tue, 22 Oct 2024 14:29:57 -0700 Subject: [PATCH 11/25] refactor: address comments --- cns/restserver/helper_for_nodesubnet_test.go | 5 ++++- cns/restserver/nodesubnet.go | 1 - cns/restserver/nodesubnet_test.go | 22 +++++++++----------- cns/service/main.go | 3 ++- hack/aks/README.md | 1 + 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/cns/restserver/helper_for_nodesubnet_test.go b/cns/restserver/helper_for_nodesubnet_test.go index 2acb96d775..3ec98b72f6 100644 --- a/cns/restserver/helper_for_nodesubnet_test.go +++ b/cns/restserver/helper_for_nodesubnet_test.go @@ -3,6 +3,7 @@ package restserver import ( "context" "net/netip" + "testing" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" @@ -13,7 +14,7 @@ import ( "github.com/Azure/azure-container-networking/store" ) -func GetRestServiceObjectForNodeSubnetTest(generator CNIConflistGenerator) *HTTPRestService { +func GetRestServiceObjectForNodeSubnetTest(t *testing.T, generator CNIConflistGenerator) *HTTPRestService { config := &common.ServiceConfig{ Name: "test", Version: "1.0", @@ -56,6 +57,8 @@ func GetRestServiceObjectForNodeSubnetTest(generator CNIConflistGenerator) *HTTP return nil } + t.Cleanup(func() { svc.Uninitialize() }) + return &HTTPRestService{ Service: svc, cniConflistGenerator: generator, diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 92221c8fa9..b2fd8c213c 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -24,7 +24,6 @@ func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr code, msg := service.saveNetworkContainerGoalState(*networkContainerRequest) if code != types.Success { - logger.Debugf("Error in processing IP change") return errors.Errorf("failed to save fetched ips. code: %d, message %s", code, msg) } diff --git a/cns/restserver/nodesubnet_test.go b/cns/restserver/nodesubnet_test.go index 9bd4f133cf..22261f6a60 100644 --- a/cns/restserver/nodesubnet_test.go +++ b/cns/restserver/nodesubnet_test.go @@ -18,11 +18,11 @@ func (m *MockPodInfoByIPProvider) PodInfoByIP() (res map[string]cns.PodInfo, err // Mock implementation of CNIConflistGenerator type MockCNIConflistGenerator struct { - GenerateCalled chan bool + GenerateCalled chan struct{} } func (m *MockCNIConflistGenerator) Generate() error { - m.GenerateCalled <- true + close(m.GenerateCalled) return nil } @@ -32,32 +32,30 @@ func (m *MockCNIConflistGenerator) Close() error { } func TestNodeSubnet(t *testing.T) { - mockPodInfoProvider := new(MockPodInfoByIPProvider) + mockPodInfoProvider := &MockPodInfoByIPProvider{} // Create a real HTTPRestService object mockCNIConflistGenerator := &MockCNIConflistGenerator{ - GenerateCalled: make(chan bool), + GenerateCalled: make(chan struct{}), } - service := restserver.GetRestServiceObjectForNodeSubnetTest(mockCNIConflistGenerator) - defer service.Service.Uninitialize() - + service := restserver.GetRestServiceObjectForNodeSubnetTest(t, mockCNIConflistGenerator) ctx, cancel := testContext(t) defer cancel() err := service.InitializeNodeSubnet(ctx, mockPodInfoProvider) service.StartNodeSubnet(ctx) - if err != nil { - t.Errorf("InitializeNodeSubnet returned an error: %v", err) - } - if service.GetNodesubnetIPFetcher() == nil { t.Error("NodeSubnetIPFetcher is not initialized") } + if err != nil { + t.Fatalf("InitializeNodeSubnet returned an error: %v", err) + } + select { case <-ctx.Done(): - t.Error("Test context was canceled before conflist generation") + t.Errorf("test context done - %s", ctx.Err()) return case <-mockCNIConflistGenerator.GenerateCalled: break diff --git a/cns/service/main.go b/cns/service/main.go index 062bcf365a..ed169e6986 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -872,9 +872,10 @@ func main() { } } + // AzureHost channelmode indeicates Nodesubnet. IPs are to be fetched from NMagent. if config.ChannelMode == cns.AzureHost { if !cnsconfig.ManageEndpointState { - logger.Errorf("[Azure CNS] ManageEndpointState must be set to true for AzureHost mode") + logger.Errorf("ManageEndpointState must be set to true for AzureHost mode") return } diff --git a/hack/aks/README.md b/hack/aks/README.md index 8e1febd0e4..0ff3ed4811 100644 --- a/hack/aks/README.md +++ b/hack/aks/README.md @@ -24,6 +24,7 @@ AKS Clusters byocni-up Alias to swift-byocni-up cilium-up Alias to swift-cilium-up up Alias to swift-up + nodesubnet-byocni-nokubeproxy-up Bring up a Nodesubnet BYO CNI cluster. Does not include secondary IP configs. overlay-byocni-up Bring up a Overlay BYO CNI cluster overlay-byocni-nokubeproxy-up Bring up a Overlay BYO CNI cluster without kube-proxy overlay-cilium-up Bring up a Overlay Cilium cluster From 301de9fbde4c86e449cffa0c019172a650302877 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Tue, 22 Oct 2024 16:59:12 -0700 Subject: [PATCH 12/25] fix: address comments --- cns/service/main.go | 2 -- test/integration/cilium-nodesubnet/ipconfigupdate.go | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index ed169e6986..810a23b78e 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -661,8 +661,6 @@ func main() { } if isManaged, ok := acn.GetArg(acn.OptManaged).(bool); ok && isManaged { config.ChannelMode = cns.Managed - } else if cnsconfig.ChannelMode == cns.AzureHost { - config.ChannelMode = cns.AzureHost } homeAzMonitor := restserver.NewHomeAzMonitor(nmaClient, time.Duration(cnsconfig.AZRSettings.PopulateHomeAzCacheRetryIntervalSecs)*time.Second) diff --git a/test/integration/cilium-nodesubnet/ipconfigupdate.go b/test/integration/cilium-nodesubnet/ipconfigupdate.go index bb7cf81d07..a1bed99d55 100644 --- a/test/integration/cilium-nodesubnet/ipconfigupdate.go +++ b/test/integration/cilium-nodesubnet/ipconfigupdate.go @@ -104,6 +104,8 @@ func main() { for i := 2; i <= secondaryConfigCount+1; i++ { ipConfig := make(map[string]interface{}) for k, v := range primaryIPConfig { + // only the primary config needs loadBalancerBackendAddressPools. Azure doesn't allow + // secondary IP configs to be associated with pools that are used for load balancing. if k == "loadBalancerBackendAddressPools" { continue } From c4d940846ae1e23c9977e5760c0a8ab68a1ae18b Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Wed, 23 Oct 2024 09:40:57 -0700 Subject: [PATCH 13/25] chore:comment cleanup --- test/integration/cilium-nodesubnet/ipconfigupdate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/cilium-nodesubnet/ipconfigupdate.go b/test/integration/cilium-nodesubnet/ipconfigupdate.go index a1bed99d55..6641b55314 100644 --- a/test/integration/cilium-nodesubnet/ipconfigupdate.go +++ b/test/integration/cilium-nodesubnet/ipconfigupdate.go @@ -105,7 +105,7 @@ func main() { ipConfig := make(map[string]interface{}) for k, v := range primaryIPConfig { // only the primary config needs loadBalancerBackendAddressPools. Azure doesn't allow - // secondary IP configs to be associated with pools that are used for load balancing. + // secondary IP configs to be associated load balancer backend pools. if k == "loadBalancerBackendAddressPools" { continue } From 2b210d63097748fe26118485a8b1b834a4ab035e Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Wed, 23 Oct 2024 11:10:21 -0700 Subject: [PATCH 14/25] fix: do not use bash in ip config update --- .../cilium-nodesubnet/ipconfigupdate.go | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/test/integration/cilium-nodesubnet/ipconfigupdate.go b/test/integration/cilium-nodesubnet/ipconfigupdate.go index 6641b55314..87c0fe16b6 100644 --- a/test/integration/cilium-nodesubnet/ipconfigupdate.go +++ b/test/integration/cilium-nodesubnet/ipconfigupdate.go @@ -12,12 +12,13 @@ import ( "github.com/pkg/errors" ) -func runCommand(command string) (string, error) { +func runAzCommand(params ...string) (string, error) { var out bytes.Buffer var stderr bytes.Buffer var err error + fmt.Println("Running Azure CLI command ", strings.Join(params, " ")) for i := 0; i < 3; i++ { - cmd := exec.Command("bash", "-c", command) + cmd := exec.Command("az", params...) cmd.Stdout = &out cmd.Stderr = &stderr err = cmd.Run() @@ -27,7 +28,7 @@ func runCommand(command string) (string, error) { } if err != nil { - return "", errors.Wrap(err, fmt.Sprintf("command %s failed ", command)) + return "", errors.Wrap(err, fmt.Sprintf("command failed "+stderr.String())) } return out.String(), nil @@ -60,16 +61,14 @@ func main() { os.Exit(1) } - command := fmt.Sprintf("az vmss list -g %s --query '[0].name' -o tsv", resourceGroup) - result, err := runCommand(command) + result, err := runAzCommand("vmss", "list", "-g", resourceGroup, "--query", "[0].name", "-o", "tsv") if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) } vmssName := strings.TrimSpace(result) - command = fmt.Sprintf("az vmss show -g %s -n %s", resourceGroup, vmssName) - result, err = runCommand(command) + result, err = runAzCommand("vmss", "show", "-g", resourceGroup, "-n", vmssName) if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) @@ -131,20 +130,13 @@ func main() { os.Exit(1) } - escapedNetworkProfileJSON := strings.ReplaceAll(string(networkProfileJSON), `\`, `\\`) - escapedNetworkProfileJSON = strings.ReplaceAll(escapedNetworkProfileJSON, `'`, `\'`) - - command = fmt.Sprintf("az vmss update -g %s -n %s --set virtualMachineProfile.networkProfile='%s'", resourceGroup, vmssName, escapedNetworkProfileJSON) - fmt.Println("Command to update VMSS: ", command) - _, err = runCommand(command) + _, err = runAzCommand("vmss", "update", "-g", resourceGroup, "-n", vmssName, "--set", fmt.Sprintf("virtualMachineProfile.networkProfile=%s", networkProfileJSON)) if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) } - command = fmt.Sprintf("az vmss update-instances -g %s -n %s --instance-ids '*'", resourceGroup, vmssName) - fmt.Println("Command to update VMSS instances: ", command) - _, err = runCommand(command) + _, err = runAzCommand("vmss", "update-instances", "-g", resourceGroup, "-n", vmssName, "--instance-ids", "*") if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) From a15ee32d41cd5b953d7e8f4320329c83d6745eb3 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Wed, 23 Oct 2024 11:18:14 -0700 Subject: [PATCH 15/25] fix: address comments --- cns/restserver/nodesubnet.go | 6 +++++- cns/service/main.go | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index ad8048e8fc..3d0a4f877f 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -34,8 +34,12 @@ func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr return nil } +// initialization steps for nodesubnet: +// 1. Set orchestrator type to KubernetesCRD +// 2. Reconcile initial CNS state from statefile +// 3. Create the IP fetcher func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInfoByIPProvider cns.PodInfoByIPProvider) error { - // Set orchestrator type + // set orchestrator type orchestrator := cns.SetOrchestratorTypeRequest{ OrchestratorType: cns.KubernetesCRD, } diff --git a/cns/service/main.go b/cns/service/main.go index 810a23b78e..4b5c769f97 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -870,7 +870,7 @@ func main() { } } - // AzureHost channelmode indeicates Nodesubnet. IPs are to be fetched from NMagent. + // AzureHost channelmode indicates Nodesubnet. IPs are to be fetched from NMagent. if config.ChannelMode == cns.AzureHost { if !cnsconfig.ManageEndpointState { logger.Errorf("ManageEndpointState must be set to true for AzureHost mode") @@ -1506,6 +1506,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn return nil } +// getPodInfoByIPProvider returns a PodInfoByIPProvider that reads endpoint state from the configured source func getPodInfoByIPProvider( ctx context.Context, cnsconfig *configuration.CNSConfig, From 30c7aa28f99025e5cff961fd43a17e547fd295f0 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Wed, 23 Oct 2024 11:51:33 -0700 Subject: [PATCH 16/25] fix: make linter happy --- test/integration/cilium-nodesubnet/ipconfigupdate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/cilium-nodesubnet/ipconfigupdate.go b/test/integration/cilium-nodesubnet/ipconfigupdate.go index 87c0fe16b6..4fe7656a1c 100644 --- a/test/integration/cilium-nodesubnet/ipconfigupdate.go +++ b/test/integration/cilium-nodesubnet/ipconfigupdate.go @@ -28,7 +28,7 @@ func runAzCommand(params ...string) (string, error) { } if err != nil { - return "", errors.Wrap(err, fmt.Sprintf("command failed "+stderr.String())) + return "", errors.Wrap(err, fmt.Sprintf("command failed %s", stderr.String())) } return out.String(), nil From 356dc69c492724a1c2e4eb05138d309436d77c6e Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Wed, 23 Oct 2024 12:02:58 -0700 Subject: [PATCH 17/25] chore: move pipeline changes out --- .pipelines/pipeline.yaml | 16 --------- .../cilium-nodesubnet-e2e-job-template.yaml | 2 +- .../cilium-nodesubnet-e2e-step-template.yaml | 1 - hack/aks/Makefile | 35 +++++++++---------- hack/aks/README.md | 1 - .../cilium-nodesubnet/ipconfigupdate.go | 34 +++++++++--------- .../ip-masq-agent/config-custom.yaml | 1 - .../ip-masq-agent/config-reconcile.yaml | 1 - 8 files changed, 36 insertions(+), 55 deletions(-) diff --git a/.pipelines/pipeline.yaml b/.pipelines/pipeline.yaml index 6fa30d2c87..6631f80f02 100644 --- a/.pipelines/pipeline.yaml +++ b/.pipelines/pipeline.yaml @@ -261,17 +261,6 @@ stages: k8sVersion: "" dependsOn: "containerize" - # Cilium Nodesubnet E2E tests - - template: singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml - parameters: - name: "cilium_nodesubnet_e2e" - displayName: Cilium NodeSubnet - clusterType: nodesubnet-byocni-nokubeproxy-up - clusterName: "cilndsubnete2e" - vmSize: Standard_B2s - k8sVersion: "" - dependsOn: "containerize" - # Cilium Overlay E2E tests - template: singletenancy/cilium-overlay/cilium-overlay-e2e-job-template.yaml parameters: @@ -416,7 +405,6 @@ stages: - azure_overlay_stateless_e2e - aks_swift_e2e - cilium_e2e - - cilium_nodesubnet_e2e - cilium_overlay_e2e - cilium_h_overlay_e2e - aks_ubuntu_22_linux_e2e @@ -437,10 +425,6 @@ stages: cilium_e2e: name: cilium_e2e clusterName: "ciliume2e" - region: $(REGION_AKS_CLUSTER_TEST) - cilium_nodesubnet_e2e: - name: cilium_nodesubnet_e2e - clusterName: "cilndsubnete2e" region: $(REGION_AKS_CLUSTER_TEST) cilium_overlay_e2e: name: cilium_overlay_e2e diff --git a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml index 694d0b4e94..416be76c01 100644 --- a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml +++ b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-job-template.yaml @@ -71,9 +71,9 @@ stages: os: ${{ parameters.os }} datapath: true dns: true - cni: cilium portforward: true service: true + hostport: true dependsOn: ${{ parameters.name }} - job: failedE2ELogs diff --git a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml index 3e3e889eac..835a8dd1fd 100644 --- a/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml +++ b/.pipelines/singletenancy/cilium-nodesubnet/cilium-nodesubnet-e2e-step-template.yaml @@ -57,7 +57,6 @@ steps: pwd kubectl cluster-info kubectl get po -owide -A - CILIUM_VERSION_TAG=${CILIUM_NODE_SUBNET_VERSION} echo "install Cilium ${CILIUM_VERSION_TAG}" export DIR=${CILIUM_VERSION_TAG%.*} echo "installing files from ${DIR}" diff --git a/hack/aks/Makefile b/hack/aks/Makefile index cdf6928ee3..6aedf52c01 100644 --- a/hack/aks/Makefile +++ b/hack/aks/Makefile @@ -8,18 +8,17 @@ AZIMG = mcr.microsoft.com/azure-cli AZCLI ?= docker run --rm -v $(AZCFG):/root/.azure -v $(KUBECFG):/root/.kube -v $(SSH):/root/.ssh -v $(PWD):/root/tmpsrc $(AZIMG) az # overrideable defaults -AUTOUPGRADE ?= patch -K8S_VER ?= 1.30 -NODE_COUNT ?= 2 -NODE_COUNT_WIN ?= $(NODE_COUNT) -NODEUPGRADE ?= NodeImage -OS ?= linux # Used to signify if you want to bring up a windows nodePool on byocni clusters -OS_SKU ?= Ubuntu -OS_SKU_WIN ?= Windows2022 -REGION ?= westus2 -VM_SIZE ?= Standard_B2s -VM_SIZE_WIN ?= Standard_B2s -KUBE_PROXY_JSON_PATH ?= ./kube-proxy.json +AUTOUPGRADE ?= patch +K8S_VER ?= 1.30 +NODE_COUNT ?= 2 +NODE_COUNT_WIN ?= $(NODE_COUNT) +NODEUPGRADE ?= NodeImage +OS ?= linux # Used to signify if you want to bring up a windows nodePool on byocni clusters +OS_SKU ?= Ubuntu +OS_SKU_WIN ?= Windows2022 +REGION ?= westus2 +VM_SIZE ?= Standard_B2s +VM_SIZE_WIN ?= Standard_B2s # overrideable variables SUB ?= $(AZURE_SUBSCRIPTION) @@ -104,13 +103,13 @@ nodesubnet-byocni-nokubeproxy-up: rg-up overlay-net-up ## Brings up an NodeSubne --kubernetes-version $(K8S_VER) \ --node-count $(NODE_COUNT) \ --node-vm-size $(VM_SIZE) \ - --load-balancer-sku standard \ + --load-balancer-sku basic \ --max-pods 250 \ --network-plugin none \ --vnet-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/nodenet \ --os-sku $(OS_SKU) \ --no-ssh-key \ - --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ + --kube-proxy-config ./kube-proxy.json \ --yes @$(MAKE) set-kubeconf @@ -147,7 +146,7 @@ overlay-byocni-nokubeproxy-up: rg-up overlay-net-up ## Brings up an Overlay BYO --pod-cidr 192.168.0.0/16 \ --vnet-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/nodenet \ --no-ssh-key \ - --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ + --kube-proxy-config ./kube-proxy.json \ --yes @$(MAKE) set-kubeconf @@ -216,7 +215,7 @@ swift-byocni-nokubeproxy-up: rg-up swift-net-up ## Bring up a SWIFT BYO CNI clus --pod-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/podnet \ --no-ssh-key \ --os-sku $(OS_SKU) \ - --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ + --kube-proxy-config ./kube-proxy.json \ --yes @$(MAKE) set-kubeconf @@ -306,7 +305,7 @@ vnetscale-swift-byocni-nokubeproxy-up: rg-up vnetscale-swift-net-up ## Bring up --pod-subnet-id /subscriptions/$(SUB)/resourceGroups/$(GROUP)/providers/Microsoft.Network/virtualNetworks/$(VNET)/subnets/podnet \ --no-ssh-key \ --os-sku $(OS_SKU) \ - --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ + --kube-proxy-config ./kube-proxy.json \ --yes @$(MAKE) set-kubeconf @@ -435,7 +434,7 @@ dualstack-byocni-nokubeproxy-up: rg-up overlay-net-up ## Brings up a Dualstack o --ip-families ipv4,ipv6 \ --aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/AzureOverlayDualStackPreview \ --no-ssh-key \ - --kube-proxy-config $(KUBE_PROXY_JSON_PATH) \ + --kube-proxy-config ./kube-proxy.json \ --yes @$(MAKE) set-kubeconf diff --git a/hack/aks/README.md b/hack/aks/README.md index 0ff3ed4811..8e1febd0e4 100644 --- a/hack/aks/README.md +++ b/hack/aks/README.md @@ -24,7 +24,6 @@ AKS Clusters byocni-up Alias to swift-byocni-up cilium-up Alias to swift-cilium-up up Alias to swift-up - nodesubnet-byocni-nokubeproxy-up Bring up a Nodesubnet BYO CNI cluster. Does not include secondary IP configs. overlay-byocni-up Bring up a Overlay BYO CNI cluster overlay-byocni-nokubeproxy-up Bring up a Overlay BYO CNI cluster without kube-proxy overlay-cilium-up Bring up a Overlay Cilium cluster diff --git a/test/integration/cilium-nodesubnet/ipconfigupdate.go b/test/integration/cilium-nodesubnet/ipconfigupdate.go index 4fe7656a1c..2a8fa8a84c 100644 --- a/test/integration/cilium-nodesubnet/ipconfigupdate.go +++ b/test/integration/cilium-nodesubnet/ipconfigupdate.go @@ -12,15 +12,14 @@ import ( "github.com/pkg/errors" ) -func runAzCommand(params ...string) (string, error) { +func runCommand(command string) (string, error) { + cmd := exec.Command("bash", "-c", command) var out bytes.Buffer var stderr bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &stderr var err error - fmt.Println("Running Azure CLI command ", strings.Join(params, " ")) for i := 0; i < 3; i++ { - cmd := exec.Command("az", params...) - cmd.Stdout = &out - cmd.Stderr = &stderr err = cmd.Run() if err == nil { break @@ -28,7 +27,7 @@ func runAzCommand(params ...string) (string, error) { } if err != nil { - return "", errors.Wrap(err, fmt.Sprintf("command failed %s", stderr.String())) + return "", errors.Wrap(err, "command failed") } return out.String(), nil @@ -61,14 +60,16 @@ func main() { os.Exit(1) } - result, err := runAzCommand("vmss", "list", "-g", resourceGroup, "--query", "[0].name", "-o", "tsv") + command := fmt.Sprintf("az vmss list -g %s --query '[0].name' -o tsv", resourceGroup) + result, err := runCommand(command) if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) } vmssName := strings.TrimSpace(result) - result, err = runAzCommand("vmss", "show", "-g", resourceGroup, "-n", vmssName) + command = fmt.Sprintf("az vmss show -g %s -n %s", resourceGroup, vmssName) + result, err = runCommand(command) if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) @@ -103,14 +104,8 @@ func main() { for i := 2; i <= secondaryConfigCount+1; i++ { ipConfig := make(map[string]interface{}) for k, v := range primaryIPConfig { - // only the primary config needs loadBalancerBackendAddressPools. Azure doesn't allow - // secondary IP configs to be associated load balancer backend pools. - if k == "loadBalancerBackendAddressPools" { - continue - } ipConfig[k] = v } - ipConfigName := fmt.Sprintf("ipconfig%d", i) if !contains(usedIPConfigNames, ipConfigName) { ipConfig["name"] = ipConfigName @@ -130,13 +125,20 @@ func main() { os.Exit(1) } - _, err = runAzCommand("vmss", "update", "-g", resourceGroup, "-n", vmssName, "--set", fmt.Sprintf("virtualMachineProfile.networkProfile=%s", networkProfileJSON)) + escapedNetworkProfileJSON := strings.ReplaceAll(string(networkProfileJSON), `\`, `\\`) + escapedNetworkProfileJSON = strings.ReplaceAll(escapedNetworkProfileJSON, `'`, `\'`) + + command = fmt.Sprintf("az vmss update -g %s -n %s --set virtualMachineProfile.networkProfile='%s'", resourceGroup, vmssName, escapedNetworkProfileJSON) + fmt.Println("Command to update VMSS: ", command) + _, err = runCommand(command) if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) } - _, err = runAzCommand("vmss", "update-instances", "-g", resourceGroup, "-n", vmssName, "--instance-ids", "*") + command = fmt.Sprintf("az vmss update-instances -g %s -n %s --instance-ids '*'", resourceGroup, vmssName) + fmt.Println("Command to update VMSS instances: ", command) + _, err = runCommand(command) if err != nil { fmt.Printf("Command failed with error: %s\n", err) os.Exit(1) diff --git a/test/integration/manifests/ip-masq-agent/config-custom.yaml b/test/integration/manifests/ip-masq-agent/config-custom.yaml index 3ace541311..4bdc6cc5ee 100644 --- a/test/integration/manifests/ip-masq-agent/config-custom.yaml +++ b/test/integration/manifests/ip-masq-agent/config-custom.yaml @@ -13,6 +13,5 @@ data: - 192.168.0.0/16 - 100.64.0.0/10 - 10.244.0.0/16 - - 10.10.0.0/16 masqLinkLocal: false masqLinkLocalIPv6: true diff --git a/test/integration/manifests/ip-masq-agent/config-reconcile.yaml b/test/integration/manifests/ip-masq-agent/config-reconcile.yaml index 67944cd917..0f715267d8 100644 --- a/test/integration/manifests/ip-masq-agent/config-reconcile.yaml +++ b/test/integration/manifests/ip-masq-agent/config-reconcile.yaml @@ -11,5 +11,4 @@ data: - 192.168.0.0/16 - 100.64.0.0/10 - 10.244.0.0/16 - - 10.10.0.0/16 masqLinkLocal: true From b3ac582e28eecd974deb7c50db8135c8cf7fdba5 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Thu, 24 Oct 2024 09:59:52 -0700 Subject: [PATCH 18/25] test: more elaborate test including checks on IP pool state --- cns/restserver/helper_for_nodesubnet_test.go | 29 ++++--- cns/restserver/nodesubnet_test.go | 87 +++++++++++++++++--- 2 files changed, 92 insertions(+), 24 deletions(-) diff --git a/cns/restserver/helper_for_nodesubnet_test.go b/cns/restserver/helper_for_nodesubnet_test.go index 3ec98b72f6..d23a417600 100644 --- a/cns/restserver/helper_for_nodesubnet_test.go +++ b/cns/restserver/helper_for_nodesubnet_test.go @@ -28,14 +28,22 @@ func GetRestServiceObjectForNodeSubnetTest(t *testing.T, generator CNIConflistGe IsPrimary: true, InterfaceSubnets: []nmagent.InterfaceSubnet{ { - Prefix: "10.240.0.0/16", + Prefix: "10.0.0.0/24", IPAddress: []nmagent.NodeIP{ { - Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 240, 0, 5})), + Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 0, 0, 4})), IsPrimary: true, }, { - Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 240, 0, 6})), + Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 0, 0, 52})), + IsPrimary: false, + }, + { + Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 0, 0, 63})), + IsPrimary: false, + }, + { + Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 0, 0, 45})), IsPrimary: false, }, }, @@ -60,23 +68,20 @@ func GetRestServiceObjectForNodeSubnetTest(t *testing.T, generator CNIConflistGe t.Cleanup(func() { svc.Uninitialize() }) return &HTTPRestService{ - Service: svc, - cniConflistGenerator: generator, - state: &httpRestServiceState{}, - PodIPConfigState: make(map[string]cns.IPConfigurationStatus), + Service: svc, + cniConflistGenerator: generator, + state: &httpRestServiceState{}, + PodIPConfigState: make(map[string]cns.IPConfigurationStatus), + PodIPIDByPodInterfaceKey: make(map[string][]string), nma: &fakes.NMAgentClientFake{ GetInterfaceIPInfoF: func(_ context.Context) (nmagent.Interfaces, error) { return interfaces, nil }, }, + wscli: &fakes.WireserverClientFake{}, } } -// SetCNIConflistGenerator sets the CNIConflistGenerator for the HTTPRestService. -func (service *HTTPRestService) SetCNIConflistGenerator(generator CNIConflistGenerator) { - service.cniConflistGenerator = generator -} - // GetNodesubnetIPFetcher gets the nodesubnet.IPFetcher from the HTTPRestService. func (service *HTTPRestService) GetNodesubnetIPFetcher() *nodesubnet.IPFetcher { return service.nodesubnetIPFetcher diff --git a/cns/restserver/nodesubnet_test.go b/cns/restserver/nodesubnet_test.go index 22261f6a60..7fe8d3f069 100644 --- a/cns/restserver/nodesubnet_test.go +++ b/cns/restserver/nodesubnet_test.go @@ -2,18 +2,54 @@ package restserver_test import ( "context" + "net" "testing" - "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/cnireconciler" "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/store" ) -// Mock implementation of PodInfoByIPProvider -type MockPodInfoByIPProvider struct{} +func getMockStore() store.KeyValueStore { + mockStore := store.NewMockStore("") + endpointState := map[string]*restserver.EndpointInfo{ + "12e65d89e58cb23c784e97840cf76866bfc9902089bdc8e87e9f64032e312b0b": { + PodName: "coredns-54b69f46b8-ldmwr", + PodNamespace: "kube-system", + IfnameToIPMap: map[string]*restserver.IPInfo{ + "eth0": { + IPv4: []net.IPNet{ + { + IP: net.IPv4(10, 0, 0, 52), + Mask: net.CIDRMask(24, 32), + }, + }, + }, + }, + }, + "1fc5176913a3a1a7facfb823dde3b4ded404041134fef4f4a0c8bba140fc0413": { + PodName: "load-test-7f7d49687d-wxc9p", + PodNamespace: "load-test", + IfnameToIPMap: map[string]*restserver.IPInfo{ + "eth0": { + IPv4: []net.IPNet{ + { + IP: net.IPv4(10, 0, 0, 63), + Mask: net.CIDRMask(24, 32), + }, + }, + }, + }, + }, + } -func (m *MockPodInfoByIPProvider) PodInfoByIP() (res map[string]cns.PodInfo, err error) { - return res, nil + err := mockStore.Write(restserver.EndpointStoreKey, endpointState) + if err != nil { + return nil + } + return mockStore } // Mock implementation of CNIConflistGenerator @@ -32,7 +68,10 @@ func (m *MockCNIConflistGenerator) Close() error { } func TestNodeSubnet(t *testing.T) { - mockPodInfoProvider := &MockPodInfoByIPProvider{} + podInfoByIPProvider, err := cnireconciler.NewCNSPodInfoProvider(getMockStore()) + if err != nil { + t.Fatalf("NewCNSPodInfoProvider returned an error: %v", err) + } // Create a real HTTPRestService object mockCNIConflistGenerator := &MockCNIConflistGenerator{ @@ -42,15 +81,22 @@ func TestNodeSubnet(t *testing.T) { ctx, cancel := testContext(t) defer cancel() - err := service.InitializeNodeSubnet(ctx, mockPodInfoProvider) - service.StartNodeSubnet(ctx) + err = service.InitializeNodeSubnet(ctx, podInfoByIPProvider) + if err != nil { + t.Fatalf("InitializeNodeSubnet returned an error: %v", err) + } - if service.GetNodesubnetIPFetcher() == nil { - t.Error("NodeSubnetIPFetcher is not initialized") + expectedIPs := map[string]types.IPState{ + "10.0.0.52": types.Assigned, + "10.0.0.63": types.Assigned, } - if err != nil { - t.Fatalf("InitializeNodeSubnet returned an error: %v", err) + checkIPassignment(t, service, expectedIPs) + + service.StartNodeSubnet(ctx) + + if service.GetNodesubnetIPFetcher() == nil { + t.Fatal("NodeSubnetIPFetcher is not initialized") } select { @@ -60,6 +106,23 @@ func TestNodeSubnet(t *testing.T) { case <-mockCNIConflistGenerator.GenerateCalled: break } + + expectedIPs["10.0.0.45"] = types.Available + checkIPassignment(t, service, expectedIPs) +} + +func checkIPassignment(t *testing.T, service *restserver.HTTPRestService, expectedIPs map[string]types.IPState) { + if len(service.PodIPConfigState) != len(expectedIPs) { + t.Fatalf("expected 2 entries in PodIPConfigState, got %d", len(service.PodIPConfigState)) + } + + for ip, config := range service.GetPodIPConfigState() { + if assignmentState, exists := expectedIPs[ip]; !exists { + t.Fatalf("unexpected IP %s in PodIPConfigState", ip) + } else if config.GetState() != assignmentState { + t.Fatalf("expected state 'Assigned' for IP %s, got %s", ip, config.GetState()) + } + } } // testContext creates a context from the provided testing.T that will be From 6385042f7974493ed28b8e3059a356e1e9f7b53d Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu <6684582+santhoshmprabhu@users.noreply.github.com> Date: Thu, 24 Oct 2024 12:35:44 -0700 Subject: [PATCH 19/25] fix: use comments suitable for documentation Co-authored-by: Timothy J. Raymond Signed-off-by: Santhosh Prabhu <6684582+santhoshmprabhu@users.noreply.github.com> --- cns/restserver/nodesubnet.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 3d0a4f877f..5277fa924d 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -34,10 +34,9 @@ func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr return nil } -// initialization steps for nodesubnet: -// 1. Set orchestrator type to KubernetesCRD -// 2. Reconcile initial CNS state from statefile -// 3. Create the IP fetcher +// InitializeNodeSubnet prepares CNS for serving NodeSubnet requests. +// It sets the orchestrator type to KubernetesCRD, reconciles the initial +// CNS state from the statefile, then creates an IP fetcher. func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInfoByIPProvider cns.PodInfoByIPProvider) error { // set orchestrator type orchestrator := cns.SetOrchestratorTypeRequest{ From d9b4384b77724700b8f6169ec38f2c77d0eaf94d Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Thu, 24 Oct 2024 13:03:40 -0700 Subject: [PATCH 20/25] chore: address comments --- cns/restserver/nodesubnet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 5277fa924d..67a822e914 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -13,7 +13,7 @@ import ( var _ nodesubnet.IPConsumer = &HTTPRestService{} -// Implement the UpdateIPsForNodeSubnet method for HTTPRestService +// UpdateIPsForNodeSubnet updates the IP pool of HTTPRestService with newly fetched secondary IPs func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr) error { secondaryIPStrs := make([]string, len(secondaryIPs)) for i, ip := range secondaryIPs { From 2a6881f6a03f374e7e338c97f705f1d937365f8a Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Thu, 24 Oct 2024 13:20:58 -0700 Subject: [PATCH 21/25] chore:make linter happy --- cns/restserver/nodesubnet_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cns/restserver/nodesubnet_test.go b/cns/restserver/nodesubnet_test.go index 7fe8d3f069..321c9fa5d2 100644 --- a/cns/restserver/nodesubnet_test.go +++ b/cns/restserver/nodesubnet_test.go @@ -116,7 +116,8 @@ func checkIPassignment(t *testing.T, service *restserver.HTTPRestService, expect t.Fatalf("expected 2 entries in PodIPConfigState, got %d", len(service.PodIPConfigState)) } - for ip, config := range service.GetPodIPConfigState() { + for ip := range service.GetPodIPConfigState() { + config := service.GetPodIPConfigState()[ip] if assignmentState, exists := expectedIPs[ip]; !exists { t.Fatalf("unexpected IP %s in PodIPConfigState", ip) } else if config.GetState() != assignmentState { From f8f724da40d7e280dd8133c5a281b745440cce8b Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Fri, 25 Oct 2024 12:22:23 -0700 Subject: [PATCH 22/25] fix: address comments --- cns/restserver/helper_for_nodesubnet_test.go | 1 + cns/restserver/nodesubnet.go | 2 ++ cns/restserver/nodesubnet_test.go | 6 +++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cns/restserver/helper_for_nodesubnet_test.go b/cns/restserver/helper_for_nodesubnet_test.go index d23a417600..a0ddb5001a 100644 --- a/cns/restserver/helper_for_nodesubnet_test.go +++ b/cns/restserver/helper_for_nodesubnet_test.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-container-networking/store" ) +// GetRestServiceObjectForNodeSubnetTest creates a new HTTPRestService object for use in nodesubnet unit tests. func GetRestServiceObjectForNodeSubnetTest(t *testing.T, generator CNIConflistGenerator) *HTTPRestService { config := &common.ServiceConfig{ Name: "test", diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index 67a822e914..e62bee9af9 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -56,6 +56,8 @@ func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInf return nil } +// StartNodeSubnet starts the IP fetcher for NodeSubnet. This will cause secondary IPs to be fetched periodically. +// After the first successful fetch, conflist will be generated to indicate CNS is ready. func (service *HTTPRestService) StartNodeSubnet(ctx context.Context) { service.nodesubnetIPFetcher.Start(ctx) } diff --git a/cns/restserver/nodesubnet_test.go b/cns/restserver/nodesubnet_test.go index 321c9fa5d2..6c5b5890cb 100644 --- a/cns/restserver/nodesubnet_test.go +++ b/cns/restserver/nodesubnet_test.go @@ -12,6 +12,7 @@ import ( "github.com/Azure/azure-container-networking/store" ) +// getMockStore creates a mock KeyValueStore with some endpoint state func getMockStore() store.KeyValueStore { mockStore := store.NewMockStore("") endpointState := map[string]*restserver.EndpointInfo{ @@ -67,13 +68,15 @@ func (m *MockCNIConflistGenerator) Close() error { return nil } +// TestNodeSubnet tests initialization of NodeSubnet with endpoint info, and verfies that +// the conflist is generated after fetching secondary IPs func TestNodeSubnet(t *testing.T) { podInfoByIPProvider, err := cnireconciler.NewCNSPodInfoProvider(getMockStore()) if err != nil { t.Fatalf("NewCNSPodInfoProvider returned an error: %v", err) } - // Create a real HTTPRestService object + // create a real HTTPRestService object mockCNIConflistGenerator := &MockCNIConflistGenerator{ GenerateCalled: make(chan struct{}), } @@ -111,6 +114,7 @@ func TestNodeSubnet(t *testing.T) { checkIPassignment(t, service, expectedIPs) } +// checkIPassignment checks whether the IP assignment state in the HTTPRestService object matches expectation func checkIPassignment(t *testing.T, service *restserver.HTTPRestService, expectedIPs map[string]types.IPState) { if len(service.PodIPConfigState) != len(expectedIPs) { t.Fatalf("expected 2 entries in PodIPConfigState, got %d", len(service.PodIPConfigState)) From a78a6b1d443fa06220a2a446099f2d5929b9c995 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Fri, 25 Oct 2024 12:23:08 -0700 Subject: [PATCH 23/25] chore: typo --- cns/restserver/helper_for_nodesubnet_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/helper_for_nodesubnet_test.go b/cns/restserver/helper_for_nodesubnet_test.go index a0ddb5001a..5719757429 100644 --- a/cns/restserver/helper_for_nodesubnet_test.go +++ b/cns/restserver/helper_for_nodesubnet_test.go @@ -83,7 +83,7 @@ func GetRestServiceObjectForNodeSubnetTest(t *testing.T, generator CNIConflistGe } } -// GetNodesubnetIPFetcher gets the nodesubnet.IPFetcher from the HTTPRestService. +// GetNodesubnetIPFetcher gets the nodesubnetIPFetcher from the HTTPRestService. func (service *HTTPRestService) GetNodesubnetIPFetcher() *nodesubnet.IPFetcher { return service.nodesubnetIPFetcher } From 800785e34b51bbc6105983850b2ef25bcec1bde7 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Tue, 29 Oct 2024 10:02:28 -0700 Subject: [PATCH 24/25] chore: address comments --- cns/restserver/nodesubnet.go | 5 +++-- cns/service/main.go | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index e62bee9af9..ed5b33e181 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -29,8 +29,9 @@ func (service *HTTPRestService) UpdateIPsForNodeSubnet(secondaryIPs []netip.Addr logger.Debugf("IP change processed successfully") - // saved NC successfully, generate conflist to indicate CNS is ready - go service.MustGenerateCNIConflistOnce() + // saved NC successfully. UpdateIPsForNodeSubnet is called only when IPs are fetched from NMAgent. + // We now have IPs to serve IPAM requests. Generate conflist to indicate CNS is ready + service.MustGenerateCNIConflistOnce() return nil } diff --git a/cns/service/main.go b/cns/service/main.go index 4b5c769f97..ba9de05cdc 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1046,6 +1046,8 @@ func main() { } if config.ChannelMode == cns.AzureHost { + // at this point, rest service is running, and any pending async deletes have been submitted to the rest + // service. We can now start serving new requests. httpRemoteRestService.StartNodeSubnet(rootCtx) } From f764c8a37375f6e8365a1b9a770946262f0869f9 Mon Sep 17 00:00:00 2001 From: Santhosh Prabhu Date: Wed, 30 Oct 2024 10:25:16 -0700 Subject: [PATCH 25/25] fix: update comments --- cns/restserver/nodesubnet.go | 2 +- cns/service/main.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cns/restserver/nodesubnet.go b/cns/restserver/nodesubnet.go index ed5b33e181..177d4266bc 100644 --- a/cns/restserver/nodesubnet.go +++ b/cns/restserver/nodesubnet.go @@ -51,7 +51,7 @@ func (service *HTTPRestService) InitializeNodeSubnet(ctx context.Context, podInf return errors.Wrap(err, "reconcile initial CNS state") } // statefile (if any) is reconciled. Initialize the IP fetcher. Start the IP fetcher only after the service is started, - // and any pending async delete operations are completed. + // because starting the IP fetcher will generate conflist, which should be done only once we are ready to respond to IPAM requests. service.nodesubnetIPFetcher = nodesubnet.NewIPFetcher(service.nma, service, 0, 0, logger.Log) return nil diff --git a/cns/service/main.go b/cns/service/main.go index ba9de05cdc..8073271a69 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1046,8 +1046,9 @@ func main() { } if config.ChannelMode == cns.AzureHost { - // at this point, rest service is running, and any pending async deletes have been submitted to the rest - // service. We can now start serving new requests. + // at this point, rest service is running. We can now start serving new requests. So call StartNodeSubnet, which + // will fetch secondary IPs and generate conflist. Do not move this all before rest service start - this will cause + // CNI to start sending requests, and if the service doesn't start successfully, the requests will fail. httpRemoteRestService.StartNodeSubnet(rootCtx) }