Fix util.PodNadNames for primary networks

The NAD	name retrieval in the util.PodNadNames function is currently based on
NetworkSelectionElement which doesn't work for primary networks. So enhancing
this method to work for primary networks as well. It would benefit in getting
podIPs and NAD names associated with pod's user defined primary network.

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

go-controller/pkg/ovn/base_network_controller.go

@@ -705,30 +705,12 @@ func (bnc *BaseNetworkController) doesNetworkRequireIPAM() bool {
 	return util.DoesNetworkRequireIPAM(bnc.NetInfo)
 }
 
-func (bnc *BaseNetworkController) getPodNADNames(pod *kapi.Pod) ([]string, error) {
+func (bnc *BaseNetworkController) getPodNADNames(pod *kapi.Pod) []string {
 	if !bnc.IsSecondary() {
-		return []string{types.DefaultNetworkName}, nil
-	}
-	// util.PodNadNames is based on the network selection element so it wouldn't work for primary
-	// network. So retrieve NAD name from the network which is serving pod's namespace.
-	if bnc.IsPrimaryNetwork() {
-		// We couldn't rely on bnc.NetInfo because this may span across different namespaces.
-		// Hence retrieve active network object from pod namespace and then NADs from this object.
-		namespacePrimaryNetwork, err := bnc.getActiveNetworkForNamespace(pod.Namespace)
-		if err != nil {
-			return nil, err
-		}
-		if namespacePrimaryNetwork.GetNetworkName() != bnc.NetInfo.GetNetworkName() {
-			return nil, fmt.Errorf("pod %s/%s active network %s mismatch", pod.Namespace, pod.Name, namespacePrimaryNetwork.GetNetworkName())
-		}
-		nadList := namespacePrimaryNetwork.GetNADs()
-		if len(nadList) != 1 {
-			return nil, fmt.Errorf("expected one NAD in %s network, got: %d: %v", namespacePrimaryNetwork.GetNetworkName(), len(nadList), nadList)
-		}
-		return nadList, nil
+		return []string{types.DefaultNetworkName}
 	}
 	podNadNames, _ := util.PodNadNames(pod, bnc.NetInfo)
-	return podNadNames, nil
+	return podNadNames
 }
 
 func (bnc *BaseNetworkController) getClusterPortGroupDbIDs(base string) *libovsdbops.DbObjectIDs {

go-controller/pkg/ovn/base_network_controller_policy.go

@@ -597,10 +597,7 @@ func (bnc *BaseNetworkController) getNewLocalPolicyPorts(np *networkPolicy,
 			continue
 		}
 
-		nadNames, err := bnc.getPodNADNames(pod)
-		if err != nil {
-			errs = append(errs, fmt.Errorf("unable to get NAD names for pod %s/%s: %v", pod.Namespace, pod.Name, err))
-		}
+		nadNames := bnc.getPodNADNames(pod)
 		for _, nadName := range nadNames {
 			logicalPortName := bnc.GetLogicalPortName(pod, nadName)
 			if _, ok := np.localPods.Load(logicalPortName); ok {
@@ -649,11 +646,7 @@ func (bnc *BaseNetworkController) getExistingLocalPolicyPorts(np *networkPolicy,
 	for _, obj := range objs {
 		pod := obj.(*kapi.Pod)
 
-		var nadNames []string
-		nadNames, err = bnc.getPodNADNames(pod)
-		if err != nil {
-			return
-		}
+		nadNames := bnc.getPodNADNames(pod)
 		for _, nadName := range nadNames {
 			logicalPortName := bnc.GetLogicalPortName(pod, nadName)
 			loadedPortUUID, ok := np.localPods.Load(logicalPortName)

go-controller/pkg/util/pod_annotation.go

@@ -11,6 +11,7 @@ import (
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
+	"k8s.io/client-go/tools/cache"
 
 	v1 "k8s.io/api/core/v1"
 	listers "k8s.io/client-go/listers/core/v1"
@@ -204,7 +205,7 @@ func UnmarshalPodAnnotation(annotations map[string]string, nadName string) (*Pod
 
 	tempA, ok := podNetworks[nadName]
 	if !ok {
-		return nil, newAnnotationNotSetError("no ovn pod annotation for network %s: %q",
+		return nil, fmt.Errorf("no ovn pod annotation for network %s: %q",
 			nadName, ovnAnnotation)
 	}
 
@@ -303,38 +304,14 @@ func GetPodCIDRsWithFullMask(pod *v1.Pod, nInfo NetInfo) ([]*net.IPNet, error) {
 	return ips, nil
 }
 
-// GetPodIPsOfNetwork returns the pod's IP addresses.
-// If the network is default type, then retrieve it from Pod Status IPs.
-// If the network is secondary and also used as primary network for the pod, then retrieve
-// IPs directly from "k8s.ovn.org/pod-networks" pod annotation.
-// If the network is only secondary, then fetch pod NAD name using NetworkSelectionElement
-// and then retrieve IPs from k8s.ovn.org/pod-networks annotation for the network.
-// This function is intended to also return IPs for HostNetwork and other non-OVN-IPAM-ed pods.
+// GetPodIPsOfNetwork returns the pod's IP addresses, first from the OVN annotation
+// and then falling back to the Pod Status IPs. This function is intended to
+// also return IPs for HostNetwork and other non-OVN-IPAM-ed pods.
 func GetPodIPsOfNetwork(pod *v1.Pod, nInfo NetInfo) ([]net.IP, error) {
-	if !nInfo.IsSecondary() || PodWantsHostNetwork(pod) {
-		return DefaultNetworkPodIPs(pod)
-	}
-	if nInfo.IsPrimaryNetwork() {
-		// The nInfo might belongs to a network which may be serving multiple namespaces.
-		// Hence find out the NAD associated with pod namespace and retrieve IPs from it.
-		nadList := nInfo.GetNADs()
-		for _, nad := range nadList {
-			userNet, err := UnmarshalPodAnnotation(pod.Annotations, nad)
-			if err != nil && IsAnnotationNotSetError(err) {
-				// NAD belongs to same network, but serving different namespace. so skip processing it.
-				continue
-			} else if err != nil {
-				return nil, err
-			}
-			ips := make([]net.IP, 0, len(userNet.IPs))
-			for _, netIP := range userNet.IPs {
-				ips = append(ips, netIP.IP)
-			}
-			return ips, nil
-		}
-		return []net.IP{}, nil
+	if nInfo.IsSecondary() {
+		return SecondaryNetworkPodIPs(pod, nInfo)
 	}
-	return SecondaryNetworkPodIPs(pod, nInfo)
+	return DefaultNetworkPodIPs(pod)
 }
 
 func DefaultNetworkPodIPs(pod *v1.Pod) ([]net.IP, error) {
@@ -381,6 +358,15 @@ func SecondaryNetworkPodIPs(pod *v1.Pod, networkInfo NetInfo) ([]net.IP, error)
 }
 
 func PodNadNames(pod *v1.Pod, netinfo NetInfo) ([]string, error) {
+	// When netInfo is a primary network, then retrieve NAD name from netinfo.GetNADs() which
+	// is serving pod's namespace.
+	if netinfo.IsPrimaryNetwork() {
+		nadName, err := GetPrimaryNetworkNADNameForNamespaceFromNetInfo(pod.Namespace, netinfo)
+		if err != nil {
+			return nil, err
+		}
+		return []string{nadName}, nil
+	}
 	on, networkMap, err := GetPodNADToNetworkMapping(pod, netinfo)
 	// skip pods that are not on this network
 	if err != nil {
@@ -395,6 +381,20 @@ func PodNadNames(pod *v1.Pod, netinfo NetInfo) ([]string, error) {
 	return nadNames, nil
 }
 
+func GetPrimaryNetworkNADNameForNamespaceFromNetInfo(namespace string, netinfo NetInfo) (string, error) {
+	for _, nadName := range netinfo.GetNADs() {
+		ns, _, err := cache.SplitMetaNamespaceKey(nadName)
+		if err != nil {
+			return "", fmt.Errorf("error parsing nad name %s from network %s: %v", nadName, netinfo.GetNetworkName(), err)
+		}
+		if ns != namespace {
+			continue
+		}
+		return nadName, nil
+	}
+	return "", fmt.Errorf("nad name not found on namespace %s for network %s", namespace, netinfo.GetNetworkName())
+}
+
 func getAnnotatedPodIPs(pod *v1.Pod, nadName string) []net.IP {
 	var ips []net.IP
 	annotation, _ := UnmarshalPodAnnotation(pod.Annotations, nadName)