From 0038e5b963b07aad3bad91ee34a34e01d644adc0 Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Wed, 24 Jul 2024 09:41:47 -0400 Subject: [PATCH 1/2] Make annotation in pod sync only happen for local pods Otherwise there is a performance/start up hit in large clusters where we potentially try to annotate all pods in a cluster on each node. Signed-off-by: Tim Rozet --- go-controller/pkg/ovn/pods.go | 57 ++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/go-controller/pkg/ovn/pods.go b/go-controller/pkg/ovn/pods.go index 26d96af1e79..e153ac39867 100644 --- a/go-controller/pkg/ovn/pods.go +++ b/go-controller/pkg/ovn/pods.go @@ -97,38 +97,41 @@ func (oc *DefaultNetworkController) syncPods(pods []interface{}) error { expectedLogicalPorts[expectedLogicalPortName] = true } - // delete the outdated hybrid overlay subnet route if it exists - newRoutes := []util.PodRoute{} - // HO is IPv4 only - ipv4Subnets := util.MatchAllIPNetFamily(false, oc.lsManager.GetSwitchSubnets(pod.Spec.NodeName)) - for _, route := range annotations.Routes { - if !util.IsNodeHybridOverlayIfAddr(route.NextHop, ipv4Subnets) { - newRoutes = append(newRoutes, route) + // only update annotations for pods belonging to my zone + if oc.isPodScheduledinLocalZone(pod) { + // delete the outdated hybrid overlay subnet route if it exists + newRoutes := []util.PodRoute{} + // HO is IPv4 only + ipv4Subnets := util.MatchAllIPNetFamily(false, oc.lsManager.GetSwitchSubnets(pod.Spec.NodeName)) + for _, route := range annotations.Routes { + if !util.IsNodeHybridOverlayIfAddr(route.NextHop, ipv4Subnets) { + newRoutes = append(newRoutes, route) + } } - } - syncPodAnnotations := false + syncPodAnnotations := false - // checking the length because cannot compare the slices directly and if routes are removed - // the length will be different - if len(annotations.Routes) != len(newRoutes) { - annotations.Routes = newRoutes - syncPodAnnotations = true - } - // the ovn pod annotation role field is mandatory for default pod network, update old pods - // it it's missing - if util.IsNetworkSegmentationSupportEnabled() { - if annotations.Role == "" { - // Hardcode directly to primary since we don't support primary role networks at namespaces with - // pods in it, do not make sense to call GetNetworkRole here. - annotations.Role = types.NetworkRolePrimary + // checking the length because cannot compare the slices directly and if routes are removed + // the length will be different + if len(annotations.Routes) != len(newRoutes) { + annotations.Routes = newRoutes syncPodAnnotations = true } - } - if syncPodAnnotations { - err = oc.updatePodAnnotationWithRetry(pod, annotations, ovntypes.DefaultNetworkName) - if err != nil { - return fmt.Errorf("failed to set annotation on pod %s: %v", pod.Name, err) + // the ovn pod annotation role field is mandatory for default pod network, update old pods + // it it's missing + if util.IsNetworkSegmentationSupportEnabled() { + if annotations.Role == "" { + // Hardcode directly to primary since we don't support primary role networks at namespaces with + // pods in it, do not make sense to call GetNetworkRole here. + annotations.Role = types.NetworkRolePrimary + syncPodAnnotations = true + } + } + if syncPodAnnotations { + err = oc.updatePodAnnotationWithRetry(pod, annotations, ovntypes.DefaultNetworkName) + if err != nil { + return fmt.Errorf("failed to set annotation on pod %s: %v", pod.Name, err) + } } } } From c53542d4d29a4533d0ee3be484fcddf6c5632ffc Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Wed, 24 Jul 2024 11:58:01 -0400 Subject: [PATCH 2/2] Ensure config is reset before node ip handler tests There was a panic seen in https://github.com/ovn-org/ovn-kubernetes/actions/runs/10077834705/job/27861284436?pr=4560 Due to the watchFactory starting and trying to list NADs would end up with a NPE, as the client in node for unit test not having NAD support. The suspect is that global config is not reset during these tests, and UDN was enabled. This patch resets global config before the test runs to ensure UDN is disabled as these tests do not require it. Signed-off-by: Tim Rozet --- go-controller/pkg/node/node_ip_handler_linux_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go-controller/pkg/node/node_ip_handler_linux_test.go b/go-controller/pkg/node/node_ip_handler_linux_test.go index 152377d2963..8227872fee4 100644 --- a/go-controller/pkg/node/node_ip_handler_linux_test.go +++ b/go-controller/pkg/node/node_ip_handler_linux_test.go @@ -187,6 +187,8 @@ var _ = Describe("Node IP Handler tests", func() { Output: dummyBrInternalIPv4, }) Expect(util.SetExec(fexec)).ShouldNot(HaveOccurred()) + // Restore global default values before each testcase + Expect(config.PrepareTestConfig()).To(Succeed()) config.IPv4Mode = true config.IPv6Mode = true tc = configureKubeOVNContextWithNs(nodeName)