RouteAdvertisements: update API to use universal NetworkSelector

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>

Author: jcaamano

dist/templates/k8s.ovn.org_routeadvertisements.yaml.j2

@@ -86,9 +86,8 @@ kind: RouteAdvertisements
 metadata:
   name: default
 spec:
-  networkSelector:
-    matchLabels:
-      k8s.ovn.org/default-network: ""
+  networkSelectors:
+    - networkSelectionType: DefaultNetwork
   advertisements:
     - "PodNetwork"
 {%- endif %}

dist/templates/ovn-setup.yaml.j2

@@ -38,6 +38,8 @@ import (
 	raapply "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/routeadvertisements/v1/apis/applyconfiguration/routeadvertisements/v1"
 	raclientset "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/routeadvertisements/v1/apis/clientset/versioned"
 	ralisters "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/routeadvertisements/v1/apis/listers/routeadvertisements/v1"
+	apitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/types"
+	userdefinednetworkv1 "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/userdefinednetwork/v1"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/networkmanager"
@@ -51,8 +53,9 @@ const (
 )
 
 var (
-	errConfig  = errors.New("configuration error")
-	errPending = errors.New("configuration pending")
+	errConfig      = errors.New("configuration error")
+	errPending     = errors.New("configuration pending")
+	cudnController = userdefinednetworkv1.SchemeGroupVersion.WithKind("ClusterUserDefinedNetwork")
 )
 
 // Controller reconciles RouteAdvertisements
@@ -332,20 +335,7 @@ func (c *Controller) generateFRRConfigurations(ra *ratypes.RouteAdvertisements)
 
 	// if we are matching on the well known default network label, create an
 	// internal nad for it if it doesn't exist
-	if matchesDefaultNetworkLabel(ra.Spec.NetworkSelector) {
-		_, err := c.getOrCreateDefaultNetworkNAD()
-		if err != nil {
-			return nil, nil, fmt.Errorf("failed to get/create default network NAD: %w", err)
-		}
-	}
-
-	// gather selected networks
-	var nads []*nadtypes.NetworkAttachmentDefinition
-	nadSelector, err := metav1.LabelSelectorAsSelector(&ra.Spec.NetworkSelector)
-	if err != nil {
-		return nil, nil, err
-	}
-	nads, err = c.nadLister.List(nadSelector)
+	nads, err := c.getSelectedNADs(ra.Spec.NetworkSelectors)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -994,6 +984,44 @@ func (c *Controller) updateRAStatus(ra *ratypes.RouteAdvertisements, hadUpdates
 	return nil
 }
 
+func (c *Controller) getSelectedNADs(networkSelectors apitypes.NetworkSelectors) ([]*nadtypes.NetworkAttachmentDefinition, error) {
+	var selected []*nadtypes.NetworkAttachmentDefinition
+	for _, networkSelector := range networkSelectors {
+		switch networkSelector.NetworkSelectionType {
+		case apitypes.DefaultNetwork:
+			// if we are selecting the default networkdefault network label,
+			// make sure a NAD exists for it
+			nad, err := c.getOrCreateDefaultNetworkNAD()
+			if err != nil {
+				return nil, fmt.Errorf("failed to get/create default network NAD: %w", err)
+			}
+			selected = append(selected, nad)
+		case apitypes.ClusterUserDefinedNetworks:
+			nadSelector, err := metav1.LabelSelectorAsSelector(&networkSelector.ClusterUserDefinedNetworkSelector.NetworkSelector)
+			if err != nil {
+				return nil, err
+			}
+			nads, err := c.nadLister.List(nadSelector)
+			if err != nil {
+				return nil, err
+			}
+			for _, nad := range nads {
+				// check this NAD is controlled by a CUDN
+				controller := metav1.GetControllerOfNoCopy(nad)
+				isCUDN := controller != nil && controller.Kind == cudnController.Kind && controller.APIVersion == cudnController.GroupVersion().String()
+				if !isCUDN {
+					continue
+				}
+				selected = append(selected, nad)
+			}
+		default:
+			return nil, fmt.Errorf("%w: unsupported network selection type %s", errConfig, networkSelector.NetworkSelectionType)
+		}
+	}
+
+	return selected, nil
+}
+
 // getOrCreateDefaultNetworkNAD ensure that a well-known NAD exists for the
 // default network in ovn-k namespace.
 func (c *Controller) getOrCreateDefaultNetworkNAD() (*nadtypes.NetworkAttachmentDefinition, error) {
@@ -1010,7 +1038,6 @@ func (c *Controller) getOrCreateDefaultNetworkNAD() (*nadtypes.NetworkAttachment
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      types.DefaultNetworkName,
 				Namespace: config.Kubernetes.OVNConfigNamespace,
-				Labels:    map[string]string{types.DefaultNetworkLabelSelector: ""},
 			},
 			Spec: nadtypes.NetworkAttachmentDefinitionSpec{
 				Config: fmt.Sprintf("{\"cniVersion\": \"0.4.0\", \"name\": \"ovn-kubernetes\", \"type\": \"%s\"}", config.CNI.Plugin),
@@ -1236,16 +1263,3 @@ func (c *Controller) reconcileEgressIPs(string) error {
 
 	return nil
 }
-
-func matchesDefaultNetworkLabel(selector metav1.LabelSelector) bool {
-	_, matchesLabel := selector.MatchLabels[types.DefaultNetworkLabelSelector]
-	if matchesLabel {
-		return true
-	}
-	for _, expr := range selector.MatchExpressions {
-		if expr.Key == types.DefaultNetworkLabelSelector {
-			return true
-		}
-	}
-	return false
-}

go-controller/pkg/clustermanager/routeadvertisements/controller.go

@@ -28,6 +28,8 @@ import (
 	controllerutil "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/controller"
 	eiptypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/egressip/v1"
 	ratypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/routeadvertisements/v1"
+	apitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/types"
+	userdefinednetworkv1 "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/userdefinednetwork/v1"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/networkmanager"
 	ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
@@ -42,6 +44,7 @@ type testRA struct {
 	NetworkSelector          map[string]string
 	NodeSelector             map[string]string
 	FRRConfigurationSelector map[string]string
+	SelectsDefault           bool
 	AdvertisePods            bool
 	AdvertiseEgressIPs       bool
 }
@@ -63,9 +66,19 @@ func (tra testRA) RouteAdvertisements() *ratypes.RouteAdvertisements {
 		ra.Spec.Advertisements = append(ra.Spec.Advertisements, ratypes.EgressIP)
 	}
 	if tra.NetworkSelector != nil {
-		ra.Spec.NetworkSelector = metav1.LabelSelector{
-			MatchLabels: tra.NetworkSelector,
-		}
+		ra.Spec.NetworkSelectors = append(ra.Spec.NetworkSelectors, apitypes.NetworkSelector{
+			NetworkSelectionType: apitypes.ClusterUserDefinedNetworks,
+			ClusterUserDefinedNetworkSelector: &apitypes.ClusterUserDefinedNetworkSelector{
+				NetworkSelector: metav1.LabelSelector{
+					MatchLabels: tra.NetworkSelector,
+				},
+			},
+		})
+	}
+	if tra.SelectsDefault {
+		ra.Spec.NetworkSelectors = append(ra.Spec.NetworkSelectors, apitypes.NetworkSelector{
+			NetworkSelectionType: apitypes.DefaultNetwork,
+		})
 	}
 	if tra.NodeSelector != nil {
 		ra.Spec.NodeSelector = metav1.LabelSelector{
@@ -291,6 +304,13 @@ func (tn testNAD) NAD() *nadtypes.NetworkAttachmentDefinition {
 			Annotations: tn.Annotations,
 		},
 	}
+	if strings.HasPrefix(tn.Network, types.CUDNPrefix) {
+		ownerRef := *metav1.NewControllerRef(
+			&metav1.ObjectMeta{Name: tn.Network},
+			userdefinednetworkv1.SchemeGroupVersion.WithKind("ClusterUserDefinedNetwork"),
+		)
+		nad.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ownerRef}
+	}
 	topology := tn.Topology
 	if topology == "" {
 		topology = "layer3"
@@ -378,7 +398,7 @@ func TestController_reconcile(t *testing.T) {
 	}{
 		{
 			name: "reconciles pod+eip RouteAdvertisement for a single FRR config, node and default network and target VRF",
-			ra:   &testRA{Name: "ra", AdvertisePods: true, AdvertiseEgressIPs: true},
+			ra:   &testRA{Name: "ra", AdvertisePods: true, AdvertiseEgressIPs: true, SelectsDefault: true},
 			frrConfigs: []*testFRRConfig{
 				{
 					Name:      "frrConfig",
@@ -409,7 +429,7 @@ func TestController_reconcile(t *testing.T) {
 		},
 		{
 			name: "reconciles dual-stack pod+eip RouteAdvertisement for a single FRR config, node and default network and target VRF",
-			ra:   &testRA{Name: "ra", AdvertisePods: true, AdvertiseEgressIPs: true},
+			ra:   &testRA{Name: "ra", AdvertisePods: true, AdvertiseEgressIPs: true, SelectsDefault: true},
 			frrConfigs: []*testFRRConfig{
 				{
 					Name:      "frrConfig",
@@ -455,8 +475,8 @@ func TestController_reconcile(t *testing.T) {
 				},
 			},
 			nads: []*testNAD{
-				{Name: "red", Namespace: "red", Network: util.GenerateCUDNNetworkName("red"), Topology: "layer3", Subnet: "1.2.0.0/16", Labels: map[string]string{"selected": "true"}},
-				{Name: "blue", Namespace: "blue", Network: util.GenerateCUDNNetworkName("blue"), Topology: "layer3", Subnet: "1.3.0.0/16", Labels: map[string]string{"selected": "true"}},
+				{Name: "red", Namespace: "red", Network: "cluster_udn_red", Topology: "layer3", Subnet: "1.2.0.0/16", Labels: map[string]string{"selected": "true"}},
+				{Name: "blue", Namespace: "blue", Network: "cluster_udn_blue", Topology: "layer3", Subnet: "1.3.0.0/16", Labels: map[string]string{"selected": "true"}},
 			},
 			nodes:                []*testNode{{Name: "node", SubnetsAnnotation: "{\"default\":\"1.1.0.0/24\", \"cluster_udn_red\":\"1.2.0.0/24\", \"cluster_udn_blue\":\"1.3.0.0/24\"}"}},
 			eips:                 []*testEIP{{Name: "eip", EIPs: map[string]string{"node": "1.0.1.1"}}},
@@ -493,11 +513,11 @@ func TestController_reconcile(t *testing.T) {
 			},
 			nads: []*testNAD{
 				{Name: "default", Namespace: "ovn-kubernetes", Network: "default"},
-				{Name: "red", Namespace: "red", Network: "cluster.udn.red", Topology: "layer3", Subnet: "1.2.0.0/16"},
-				{Name: "blue", Namespace: "blue", Network: "cluster.udn.blue", Topology: "layer3", Subnet: "1.3.0.0/16", Labels: map[string]string{"selected": "true"}},
+				{Name: "red", Namespace: "red", Network: "cluster_udn_red", Topology: "layer3", Subnet: "1.2.0.0/16"},
+				{Name: "blue", Namespace: "blue", Network: "cluster_udn_blue", Topology: "layer3", Subnet: "1.3.0.0/16", Labels: map[string]string{"selected": "true"}},
 			},
 			nodes: []*testNode{
-				{Name: "node", SubnetsAnnotation: "{\"default\":\"1.1.1.0/24\", \"cluster.udn.red\":\"1.2.1.0/24\", \"cluster.udn.blue\":\"1.3.1.0/24\"}"},
+				{Name: "node", SubnetsAnnotation: "{\"default\":\"1.1.1.0/24\", \"cluster_udn_red\":\"1.2.1.0/24\", \"cluster_udn_blue\":\"1.3.1.0/24\"}"},
 			},
 			namespaces: []*testNamespace{
 				{Name: "default", Labels: map[string]string{"selected": "default"}},
@@ -526,7 +546,7 @@ func TestController_reconcile(t *testing.T) {
 		},
 		{
 			name: "reconciles a RouteAdvertisement updating the generated FRRConfigurations if needed",
-			ra:   &testRA{Name: "ra", AdvertisePods: true, AdvertiseEgressIPs: true},
+			ra:   &testRA{Name: "ra", AdvertisePods: true, AdvertiseEgressIPs: true, SelectsDefault: true},
 			frrConfigs: []*testFRRConfig{
 				{
 					Name:      "frrConfig",
@@ -594,11 +614,12 @@ func TestController_reconcile(t *testing.T) {
 				TargetVRF:                "auto",
 				FRRConfigurationSelector: map[string]string{"selected": "true"},
 				NetworkSelector:          map[string]string{"selected": "true"},
+				SelectsDefault:           true,
 			},
 			nads: []*testNAD{
 				{Name: "default", Namespace: "ovn-kubernetes", Network: "default", Labels: map[string]string{"selected": "true"}},
-				{Name: "red", Namespace: "red", Network: util.GenerateCUDNNetworkName("red"), Topology: "layer3", Subnet: "1.2.0.0/16", Labels: map[string]string{"selected": "true"}},
-				{Name: "blue", Namespace: "blue", Network: util.GenerateCUDNNetworkName("blue"), Topology: "layer3"}, // not selected
+				{Name: "red", Namespace: "red", Network: "cluster_udn_red", Topology: "layer3", Subnet: "1.2.0.0/16", Labels: map[string]string{"selected": "true"}},
+				{Name: "blue", Namespace: "blue", Network: "cluster_udn_blue", Topology: "layer3"}, // not selected
 			},
 			frrConfigs: []*testFRRConfig{
 				{
@@ -708,6 +729,15 @@ func TestController_reconcile(t *testing.T) {
 			reconcile:            "ra",
 			expectAcceptedStatus: metav1.ConditionFalse,
 		},
+		{
+			name: "fails to reconcile an non-cluster UDN",
+			ra:   &testRA{Name: "ra", AdvertisePods: true, NetworkSelector: map[string]string{"selected": "true"}},
+			nads: []*testNAD{
+				{Name: "red", Namespace: "red", Network: "red", Topology: "layer3", Subnet: "1.2.0.0/16", Labels: map[string]string{"selected": "true"}},
+			},
+			reconcile:            "ra",
+			expectAcceptedStatus: metav1.ConditionFalse,
+		},
 		{
 			name:                 "fails to reconcile pod network if node selector is not empty",
 			ra:                   &testRA{Name: "ra", AdvertisePods: true, NodeSelector: map[string]string{"selected": "true"}},
@@ -794,8 +824,8 @@ func TestController_reconcile(t *testing.T) {
 			name: "fails to reconcile if not all VRFs were matched with 'auto' target VRF",
 			ra:   &testRA{Name: "ra", TargetVRF: "auto", AdvertisePods: true, NetworkSelector: map[string]string{"selected": "true"}},
 			nads: []*testNAD{
-				{Name: "red", Namespace: "red", Network: "red", Topology: "layer3", Labels: map[string]string{"selected": "true"}},
-				{Name: "blue", Namespace: "blue", Network: "blue", Topology: "layer3", Labels: map[string]string{"selected": "true"}},
+				{Name: "red", Namespace: "red", Network: "cluster_udn_red", Topology: "layer3", Labels: map[string]string{"selected": "true"}},
+				{Name: "blue", Namespace: "blue", Network: "cluster_udn_blue", Topology: "layer3", Labels: map[string]string{"selected": "true"}},
 			},
 			frrConfigs: []*testFRRConfig{
 				{
@@ -808,7 +838,7 @@ func TestController_reconcile(t *testing.T) {
 					},
 				},
 			},
-			nodes:                []*testNode{{Name: "node", SubnetsAnnotation: "{\"red\":\"1.1.0.0/24\", \"blue\":\"1.2.0.0/24\"}"}},
+			nodes:                []*testNode{{Name: "node", SubnetsAnnotation: "{\"cluster_udn_red\":\"1.1.0.0/24\", \"cluster_udn_blue\":\"1.2.0.0/24\"}"}},
 			reconcile:            "ra",
 			expectAcceptedStatus: metav1.ConditionFalse,
 		},

go-controller/pkg/clustermanager/routeadvertisements/controller_test.go

@@ -2,6 +2,8 @@ package v1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/types"
 )
 
 // +genclient
@@ -24,14 +26,16 @@ type RouteAdvertisements struct {
 
 // RouteAdvertisementsSpec defines the desired state of RouteAdvertisements
 // +kubebuilder:validation:XValidation:rule="!has(self.nodeSelector) || !('PodNetwork' in self.advertisements)",message="If 'PodNetwork' is selected for advertisement, a 'nodeSelector' can't be specified as it needs to be advertised on all nodes"
+// +kubebuilder:validation:XValidation:rule="!self.networkSelectors.exists(i, i.networkSelectionType != 'DefaultNetwork' && i.networkSelectionType != 'ClusterUserDefinedNetworks')",message="Only DefaultNetwork or ClusterUserDefinedNetworks can be selected"
 type RouteAdvertisementsSpec struct {
 	// targetVRF determines which VRF the routes should be advertised in.
 	// +kubebuilder:validation:Optional
 	TargetVRF string `json:"targetVRF,omitempty"`
 
-	// networkSelector determines which network routes should be advertised. To
-	// select the default network, match on label 'k8s.ovn.org/default-network'.
-	NetworkSelector metav1.LabelSelector `json:"networkSelector,omitempty"`
+	// networkSelectors determines which network routes should be advertised.
+	// Only ClusterUserDefinedNetworks and the default network can be selected.
+	// +kubebuilder:validation:Required
+	NetworkSelectors types.NetworkSelectors `json:"networkSelectors"`
 
 	// nodeSelector limits the advertisements to selected nodes.
 	// When omitted, all nodes are selected.

go-controller/pkg/crd/routeadvertisements/v1/apis/applyconfiguration/routeadvertisements/v1/routeadvertisementsspec.go

@@ -157,9 +157,6 @@ const (
 	// OVN-K8S annotation & taint constants
 	OvnK8sPrefix = "k8s.ovn.org"
 
-	// DefaultNetworkLabelSelector is the label that needs to be matched on a
-	// selector to select the default network
-	DefaultNetworkLabelSelector = OvnK8sPrefix + "/default-network"
 	// OvnNetworkNameAnnotation is the name of the network annotated on the NAD
 	// by cluster manager nad controller
 	OvnNetworkNameAnnotation = OvnK8sPrefix + "/network-name"

go-controller/pkg/crd/routeadvertisements/v1/types.go

@@ -466,9 +466,12 @@ kind: RouteAdvertisements
 metadata:
   name: ` + name + `
 spec:
-  networkSelector:
-    matchLabels:
-      k8s.ovn.org/bgp-network: ""
+  networkSelectors:
+    - networkSelectionType: ClusterUserDefinedNetworks
+      clusterUserDefinedNetworkSelector:
+        networkSelector:
+          matchLabels:
+            k8s.ovn.org/bgp-network: ""
   advertisements:
     - "PodNetwork"`
 }