Use namespace reconcilation loop for syncing network policies

This commit makes network reconcilation loop to sync only namespace
object and network policies sync to happen from namespace reconcilation
loop.

Signed-off-by: Periyasamy Palanisamy <[email protected]>

Author: pperiyasamy

go-controller/pkg/ovn/base_network_controller.go

@@ -260,6 +260,10 @@ func (oc *BaseNetworkController) doReconcile(reconcileRoutes, reconcilePendingPo
 		}
 	}
 
+	// reconciles namespaces that were added to the network, this will trigger namespace add event and
+	// network controller creates the address set for the namespace.
+	// To update gress policy ACLs with peer namespace address set, invoke requeuePeerNamespace method after
+	// address set is created for the namespace.
 	namespaceAdded := false
 	for _, ns := range reconcileNamespaces {
 		namespace, err := oc.watchFactory.GetNamespace(ns)
@@ -277,11 +281,6 @@ func (oc *BaseNetworkController) doReconcile(reconcileRoutes, reconcilePendingPo
 	if namespaceAdded {
 		oc.retryNamespaces.RequestRetryObjs()
 	}
-
-	err := oc.requeuePeerNamespaces(reconcileNamespaces)
-	if err != nil {
-		klog.Infof("Failed to retry network policy peer namespaces for network %s: %v", oc.GetNetworkName(), err)
-	}
 }
 
 // BaseSecondaryNetworkController structure holds per-network fields and network specific

go-controller/pkg/ovn/base_network_controller_policy.go

@@ -1496,20 +1496,10 @@ func (bnc *BaseNetworkController) peerNamespaceUpdate(np *networkPolicy, gp *gre
 	return err
 }
 
-// requeuePeerNamespaces enqueues the namespace into network policy peer namespace
+// requeuePeerNamespace enqueues the namespace into network policy peer namespace
 // retry framework object(s) which need to be retried immediately with add event.
-func (bnc *BaseNetworkController) requeuePeerNamespaces(namespaces []string) error {
+func (bnc *BaseNetworkController) requeuePeerNamespace(namespace *corev1.Namespace) error {
 	var errors []error
-	var peerNamespaces []*corev1.Namespace
-	for _, ns := range namespaces {
-		namespace, err := bnc.watchFactory.GetNamespace(ns)
-		if err != nil {
-			errors = append(errors, fmt.Errorf("failed to retrieve namespace %s for reconciling network %s: %w",
-				ns, bnc.GetNetworkName(), err))
-			continue
-		}
-		peerNamespaces = append(peerNamespaces, namespace)
-	}
 	npKeys := bnc.networkPolicies.GetKeys()
 	for _, npKey := range npKeys {
 		err := bnc.networkPolicies.DoWithLock(npKey, func(npKey string) error {
@@ -1519,26 +1509,23 @@ func (bnc *BaseNetworkController) requeuePeerNamespaces(namespaces []string) err
 			}
 			np.RLock()
 			defer np.RUnlock()
+			if np.deleted {
+				return nil
+			}
 			var errors []error
 			for _, reconcilePeerNamespace := range np.reconcilePeerNamespaces {
-				namespaceAdded := false
-				for _, namespace := range peerNamespaces {
-					// Filter out namespace when it's labels not matching with network policy peer namespace
-					// selector.
-					if !reconcilePeerNamespace.handler.FilterFunc(namespace) {
-						continue
-					}
-					err := reconcilePeerNamespace.retryFramework.AddRetryObjWithAddNoBackoff(namespace)
-					if err != nil {
-						errors = append(errors, fmt.Errorf("failed to retry peer namespace %s for network policy %s on network %s: %w",
-							namespace.Name, npKey, bnc.GetNetworkName(), err))
-						continue
-					}
-					namespaceAdded = true
+				// Filter out namespace when it's labels not matching with network policy peer namespace
+				// selector.
+				if !reconcilePeerNamespace.handler.FilterFunc(namespace) {
+					continue
 				}
-				if namespaceAdded {
-					reconcilePeerNamespace.retryFramework.RequestRetryObjs()
+				err := reconcilePeerNamespace.retryFramework.AddRetryObjWithAddNoBackoff(namespace)
+				if err != nil {
+					errors = append(errors, fmt.Errorf("failed to retry peer namespace %s for network policy %s on network %s: %w",
+						namespace.Name, npKey, bnc.GetNetworkName(), err))
+					continue
 				}
+				reconcilePeerNamespace.retryFramework.RequestRetryObjs()
 			}
 			return utilerrors.Join(errors...)
 		})
@@ -1587,11 +1574,13 @@ func (bnc *BaseNetworkController) addPeerNamespaceHandler(
 	// a new peer namespace is newly created later under UDN network, it gets reconciled and
 	// address set is created for the namespace. so we must reconcile it for network policy
 	// as well to update gress policy ACL with matching peer namespace address set.
-	np.Lock()
-	np.reconcilePeerNamespaces = append(np.reconcilePeerNamespaces,
-		&peerNamespacesRetry{retryFramework: retryPeerNamespaces,
-			handler: namespaceHandler})
-	np.Unlock()
+	if bnc.IsPrimaryNetwork() {
+		np.Lock()
+		np.reconcilePeerNamespaces = append(np.reconcilePeerNamespaces,
+			&peerNamespacesRetry{retryFramework: retryPeerNamespaces,
+				handler: namespaceHandler})
+		np.Unlock()
+	}
 
 	return nil
 }

go-controller/pkg/ovn/base_network_controller_secondary.go

@@ -679,7 +679,15 @@ func (bsnc *BaseSecondaryNetworkController) AddNamespaceForSecondaryNetwork(ns *
 	if err != nil {
 		return fmt.Errorf("failed to ensure namespace locked: %v", err)
 	}
-	defer nsUnlock()
+	nsUnlock()
+	// Enqueue the UDN namespace into network policy controller if it needs to be
+	// processed by network policy peer namespace handlers.
+	if bsnc.IsPrimaryNetwork() {
+		err = bsnc.requeuePeerNamespace(ns)
+		if err != nil {
+			return fmt.Errorf("failed to requeue peer namespace %s: %v", ns.Name, err)
+		}
+	}
 	return nil
 }
 

test/e2e/network_segmentation_policy.go

@@ -298,6 +298,8 @@ var _ = ginkgo.Describe("Network Segmentation: Network Policies", feature.Networ
 					return updatedPod.Status.Phase
 				}, 1*time.Minute, 6*time.Second).Should(gomega.Equal(v1.PodPending))
 
+				// The pod won't run and the namespace address set won't be created until the NAD for the network is added
+				// to the namespace and we test here that once that happens the policy is reconciled to account for it.
 				ginkgo.By("creating NAD for red and orange namespaces and check pod moves into running state")
 				for _, namespace := range []string{namespaceRed, namespaceOrange} {
 					ginkgo.By("creating the attachment configuration for " + netConfName + " in namespace " + namespace)