Reconcile namespace for network change

Since CanServeNamespace filters out namespace events for namespaces unknown
to be served by this primary network, we need to reconcile namespaces once
the network is reconfigured to serve a namespace.
Hence this commit reconciles those namespaces and also reconciles each network
policy if it contains only peer namespace selector.

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

Author: pperiyasamy

go-controller/pkg/ovn/base_network_controller.go

@@ -191,8 +191,7 @@ type BaseNetworkController struct {
 
 func (oc *BaseNetworkController) reconcile(netInfo util.NetInfo, setNodeFailed func(string)) error {
 	// gather some information first
-	var err error
-	var retryNodes []*corev1.Node
+	var reconcileNodes []string
 	oc.localZoneNodes.Range(func(key, _ any) bool {
 		nodeName := key.(string)
 		wasAdvertised := util.IsPodNetworkAdvertisedAtNode(oc, nodeName)
@@ -201,41 +200,57 @@ func (oc *BaseNetworkController) reconcile(netInfo util.NetInfo, setNodeFailed f
 			// noop
 			return true
 		}
-		var node *corev1.Node
-		node, err = oc.watchFactory.GetNode(nodeName)
-		if err != nil {
-			return false
-		}
-		retryNodes = append(retryNodes, node)
+		reconcileNodes = append(reconcileNodes, nodeName)
 		return true
 	})
-	if err != nil {
-		return fmt.Errorf("failed to reconcile network %s: %w", oc.GetNetworkName(), err)
-	}
 	reconcileRoutes := oc.routeImportManager != nil && oc.routeImportManager.NeedsReconciliation(netInfo)
 	reconcilePendingPods := !oc.IsDefault() && !oc.ReconcilableNetInfo.EqualNADs(netInfo.GetNADs()...)
+	reconcileNamespaces := sets.NewString()
+	if oc.IsPrimaryNetwork() {
+		// since CanServeNamespace filters out namespace events for namespaces unknown
+		// to be served by this primary network, we need to reconcile namespaces once
+		// the network is reconfigured to serve a namespace.
+		reconcileNamespaces = sets.NewString(netInfo.GetNADNamespaces()...).Difference(
+			sets.NewString(oc.GetNADNamespaces()...))
+	}
 
 	// set the new NetInfo, point of no return
-	err = util.ReconcileNetInfo(oc.ReconcilableNetInfo, netInfo)
+	err := util.ReconcileNetInfo(oc.ReconcilableNetInfo, netInfo)
 	if err != nil {
 		return fmt.Errorf("failed to reconcile network information for network %s: %v", oc.GetNetworkName(), err)
 	}
 
+	oc.doReconcile(reconcileRoutes, reconcilePendingPods, reconcileNodes, setNodeFailed, reconcileNamespaces.List())
+
+	return nil
+}
+
+// doReconcile performs the reconciliation after the controller NetInfo has already being
+// updated with the changes. What needs to be reconciled should already be known and
+// provided on the arguments of the method. This method returns no error and logs them
+// instead since once the controller NetInfo has been updated there is no point in retrying.
+func (oc *BaseNetworkController) doReconcile(reconcileRoutes, reconcilePendingPods bool,
+	reconcileNodes []string, setNodeFailed func(string), reconcileNamespaces []string) {
 	if reconcileRoutes {
-		err = oc.routeImportManager.ReconcileNetwork(oc.GetNetworkName())
+		err := oc.routeImportManager.ReconcileNetwork(oc.GetNetworkName())
 		if err != nil {
 			klog.Errorf("Failed to reconcile network %s on route import controller: %v", oc.GetNetworkName(), err)
 		}
 	}
 
-	for _, node := range retryNodes {
-		setNodeFailed(node.Name)
+	for _, nodeName := range reconcileNodes {
+		setNodeFailed(nodeName)
+		node, err := oc.watchFactory.GetNode(nodeName)
+		if err != nil {
+			klog.Infof("Failed to get node %s for reconciling network %s: %v", nodeName, oc.GetNetworkName(), err)
+			continue
+		}
 		err = oc.retryNodes.AddRetryObjWithAddNoBackoff(node)
 		if err != nil {
-			klog.Errorf("Failed to retry node %s for network %s: %v", node.Name, oc.GetNetworkName(), err)
+			klog.Errorf("Failed to retry node %s for network %s: %v", nodeName, oc.GetNetworkName(), err)
 		}
 	}
-	if len(retryNodes) > 0 {
+	if len(reconcileNodes) > 0 {
 		oc.retryNodes.RequestRetryObjs()
 	}
 
@@ -245,7 +260,28 @@ func (oc *BaseNetworkController) reconcile(netInfo util.NetInfo, setNodeFailed f
 		}
 	}
 
-	return nil
+	namespaceAdded := false
+	for _, ns := range reconcileNamespaces {
+		namespace, err := oc.watchFactory.GetNamespace(ns)
+		if err != nil {
+			klog.Infof("Failed to get namespace %s for reconciling network %s: %v", ns, oc.GetNetworkName(), err)
+			continue
+		}
+		err = oc.retryNamespaces.AddRetryObjWithAddNoBackoff(namespace)
+		if err != nil {
+			klog.Infof("Failed to retry namespace %s for network %s: %v", ns, oc.GetNetworkName(), err)
+			continue
+		}
+		namespaceAdded = true
+	}
+	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

@@ -10,6 +10,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	knet "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 	utilnet "k8s.io/utils/net"
@@ -23,6 +24,7 @@ import (
 	libovsdbutil "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdb/util"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/metrics"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/retry"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 	utilerrors "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util/errors"
@@ -162,6 +164,8 @@ type networkPolicy struct {
 	localPodHandler *factory.Handler
 	// peer namespace handlers
 	nsHandlerList []*factory.Handler
+	// peer namespace reconcilers
+	reconcilePeerNamespaces []*reconcilePeerNamespaces
 	// peerAddressSets stores PodSelectorAddressSet keys for peers that this network policy was successfully added to.
 	// Required for cleanup.
 	peerAddressSets []string
@@ -186,17 +190,23 @@ type networkPolicy struct {
 	cancelableContext *util.CancelableContext
 }
 
+type reconcilePeerNamespaces struct {
+	retryNamespaces   *retry.RetryFramework
+	namespaceSelector *metav1.LabelSelector
+}
+
 func NewNetworkPolicy(policy *knet.NetworkPolicy) *networkPolicy {
 	policyTypeIngress, policyTypeEgress := getPolicyType(policy)
 	np := &networkPolicy{
-		name:            policy.Name,
-		namespace:       policy.Namespace,
-		ingressPolicies: make([]*gressPolicy, 0),
-		egressPolicies:  make([]*gressPolicy, 0),
-		isIngress:       policyTypeIngress,
-		isEgress:        policyTypeEgress,
-		nsHandlerList:   make([]*factory.Handler, 0),
-		localPods:       sync.Map{},
+		name:                    policy.Name,
+		namespace:               policy.Namespace,
+		ingressPolicies:         make([]*gressPolicy, 0),
+		egressPolicies:          make([]*gressPolicy, 0),
+		isIngress:               policyTypeIngress,
+		isEgress:                policyTypeEgress,
+		nsHandlerList:           make([]*factory.Handler, 0),
+		reconcilePeerNamespaces: make([]*reconcilePeerNamespaces, 0),
+		localPods:               sync.Map{},
 	}
 	return np
 }
@@ -1490,6 +1500,63 @@ func (bnc *BaseNetworkController) peerNamespaceUpdate(np *networkPolicy, gp *gre
 	return err
 }
 
+// requeuePeerNamespaces 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 {
+	npKeys := bnc.networkPolicies.GetKeys()
+	var errors []error
+	for _, npKey := range npKeys {
+		err := bnc.networkPolicies.DoWithLock(npKey, func(npKey string) error {
+			np, ok := bnc.networkPolicies.Load(npKey)
+			if !ok {
+				return nil
+			}
+			np.RLock()
+			defer np.RUnlock()
+			var errors []error
+			for _, reconcilePeerNamespace := range np.reconcilePeerNamespaces {
+				namespaceAdded := false
+				for _, ns := range namespaces {
+					namespace, err := bnc.watchFactory.GetNamespace(ns)
+					if err != nil {
+						errors = append(errors, fmt.Errorf("failed to retrieve peer namespace %s for network policy %s on network %s: %w",
+							ns, npKey, bnc.GetNetworkName(), err))
+						continue
+					}
+					namespaceLabels := labels.Set(namespace.Labels)
+					peerNamespaceSelector, err := metav1.LabelSelectorAsSelector(reconcilePeerNamespace.namespaceSelector)
+					if err != nil {
+						errors = append(errors, fmt.Errorf("failed to parse peer namespace %s selector for network policy %s on network %s: %w",
+							ns, npKey, bnc.GetNetworkName(), err))
+						continue
+					}
+					// Filter out namespace when it's labels not matching with network policy peer namespace
+					// selector.
+					if !peerNamespaceSelector.Matches(namespaceLabels) {
+						continue
+					}
+					err = reconcilePeerNamespace.retryNamespaces.AddRetryObjWithAddNoBackoff(namespace)
+					if err != nil {
+						errors = append(errors, fmt.Errorf("failed to retry peer namespace %s for network policy %s on network %s: %w",
+							ns, npKey, bnc.GetNetworkName(), err))
+						continue
+					}
+					namespaceAdded = true
+				}
+				if namespaceAdded {
+					reconcilePeerNamespace.retryNamespaces.RequestRetryObjs()
+				}
+			}
+			return utilerrors.Join(errors...)
+		})
+		if err != nil {
+			errors = append(errors, fmt.Errorf("failed to retry peer namespaces for network policy %s on network %s: %w",
+				npKey, bnc.GetNetworkName(), err))
+		}
+	}
+	return utilerrors.Join(errors...)
+}
+
 // addPeerNamespaceHandler starts a watcher for PeerNamespaceSelectorType.
 // Sync function and Add event for every existing namespace will be executed sequentially first, and an error will be
 // returned if something fails.
@@ -1522,7 +1589,17 @@ func (bnc *BaseNetworkController) addPeerNamespaceHandler(
 		klog.Errorf("WatchResource failed for addPeerNamespaceHandler: %v", err)
 		return err
 	}
-
+	// Add peer namespace retry framework object into np.retryPeerNamespaces list so that
+	// when 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.
+	if util.IsNetworkSegmentationSupportEnabled() && bnc.IsPrimaryNetwork() {
+		np.Lock()
+		np.reconcilePeerNamespaces = append(np.reconcilePeerNamespaces,
+			&reconcilePeerNamespaces{retryNamespaces: retryPeerNamespaces,
+				namespaceSelector: namespaceSelector})
+		np.Unlock()
+	}
 	np.nsHandlerList = append(np.nsHandlerList, namespaceHandler)
 	return nil
 }
@@ -1540,6 +1617,7 @@ func (bnc *BaseNetworkController) shutdownHandlers(np *networkPolicy) {
 	for _, handler := range np.nsHandlerList {
 		bnc.watchFactory.RemoveNamespaceHandler(handler)
 	}
+	np.reconcilePeerNamespaces = make([]*reconcilePeerNamespaces, 0)
 	np.nsHandlerList = make([]*factory.Handler, 0)
 }
 

go-controller/pkg/ovn/gress_policy.go

@@ -209,6 +209,10 @@ func (gp *gressPolicy) addNamespaceAddressSet(name string, asf addressset.Addres
 		return false, fmt.Errorf("cannot add peer namespace %s: failed to get address set: %v", name, err)
 	}
 	v4HashName, v6HashName := as.GetASHashNames()
+	if v4HashName == "" && v6HashName == "" {
+		// This would happen when a namespace is not yet reconciled with UDN network.
+		return false, fmt.Errorf("cannot add peer namespace %s: address set has empty hashed name", name)
+	}
 	v4HashName = "$" + v4HashName
 	v6HashName = "$" + v6HashName
 
@@ -234,6 +238,9 @@ func (gp *gressPolicy) addNamespaceAddressSet(name string, asf addressset.Addres
 func (gp *gressPolicy) delNamespaceAddressSet(name string) bool {
 	dbIDs := getNamespaceAddrSetDbIDs(name, gp.controllerName)
 	v4HashName, v6HashName := addressset.GetHashNamesForAS(dbIDs)
+	if v4HashName == "" && v6HashName == "" {
+		return false
+	}
 	v4HashName = "$" + v4HashName
 	v6HashName = "$" + v6HashName