Merge pull request #4653 from kyrtapz/l2_pod_svc

UDN: Basic layer2 service support
ovn-org · Sep 20, 2024 · 9a25bc3 · 9a25bc3
2 parents 312a5cb + 760d9e2
commit 9a25bc3
Show file tree

Hide file tree

Showing 21 changed files with 1,329 additions and 753 deletions.
diff --git a/go-controller/pkg/libovsdb/ops/router.go b/go-controller/pkg/libovsdb/ops/router.go
@@ -8,9 +8,10 @@ import (
 
 	libovsdbclient "github.com/ovn-org/libovsdb/client"
 	libovsdb "github.com/ovn-org/libovsdb/ovsdb"
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb"
-	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // ROUTER OPs
@@ -975,10 +976,14 @@ func BuildDNATAndSNATWithMatch(
 		match)
 }
 
-// isEquivalentNAT if it has same uuid. Otherwise, check if types match.
-// ExternalIP must be unique amonst non-SNATs;
-// LogicalIP must be unique amonst SNATs;
-// If provided, LogicalPort is expected to match;
+// isEquivalentNAT checks if the `searched` NAT is equivalent to `existing`.
+// Returns true if the UUID is set in `searched` and matches the UUID of `existing`.
+// Otherwise, perform the following checks:
+//   - Compare the Type and Match fields.
+//   - Compare ExternalIP if it is set in `searched`.
+//   - Compare LogicalIP if the Type in `searched` is SNAT.
+//   - Compare LogicalPort if it is set in `searched`.
+//   - Ensure that all ExternalIDs of `searched` exist and have the same value in `existing`.
 func isEquivalentNAT(existing *nbdb.NAT, searched *nbdb.NAT) bool {
 	// Simple case: uuid was provided.
 	if searched.UUID != "" && existing.UUID == searched.UUID {
@@ -989,6 +994,10 @@ func isEquivalentNAT(existing *nbdb.NAT, searched *nbdb.NAT) bool {
 		return false
 	}
 
+	if searched.Match != existing.Match {
+		return false
+	}
+
 	// Compre externalIP if its not empty.
 	if searched.ExternalIP != "" && searched.ExternalIP != existing.ExternalIP {
 		return false

diff --git a/go-controller/pkg/network-controller-manager/network_controller_manager.go b/go-controller/pkg/network-controller-manager/network_controller_manager.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/containernetworking/cni/pkg/types"
 	libovsdbclient "github.com/ovn-org/libovsdb/client"
+
 	ovncnitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory"
@@ -66,7 +67,7 @@ func (cm *NetworkControllerManager) NewNetworkController(nInfo util.NetInfo) (na
 	case ovntypes.Layer3Topology:
 		return ovn.NewSecondaryLayer3NetworkController(cnci, nInfo, cm.nadController)
 	case ovntypes.Layer2Topology:
-		return ovn.NewSecondaryLayer2NetworkController(cnci, nInfo, cm.nadController), nil
+		return ovn.NewSecondaryLayer2NetworkController(cnci, nInfo, cm.nadController)
 	case ovntypes.LocalnetTopology:
 		return ovn.NewSecondaryLocalnetNetworkController(cnci, nInfo, cm.nadController), nil
 	}
@@ -84,7 +85,7 @@ func (cm *NetworkControllerManager) newDummyNetworkController(topoType, netName
 	case ovntypes.Layer3Topology:
 		return ovn.NewSecondaryLayer3NetworkController(cnci, netInfo, cm.nadController)
 	case ovntypes.Layer2Topology:
-		return ovn.NewSecondaryLayer2NetworkController(cnci, netInfo, cm.nadController), nil
+		return ovn.NewSecondaryLayer2NetworkController(cnci, netInfo, cm.nadController)
 	case ovntypes.LocalnetTopology:
 		return ovn.NewSecondaryLocalnetNetworkController(cnci, netInfo, cm.nadController), nil
 	}

diff --git a/go-controller/pkg/ovn/base_network_controller_secondary.go b/go-controller/pkg/ovn/base_network_controller_secondary.go
@@ -13,6 +13,7 @@ import (
 
 	libovsdbclient "github.com/ovn-org/libovsdb/client"
 	"github.com/ovn-org/libovsdb/ovsdb"
+
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/generator/udn"
@@ -405,8 +406,8 @@ func (bsnc *BaseSecondaryNetworkController) addPerPodSNATOps(pod *kapi.Pod, podI
 	if err != nil {
 		return nil, fmt.Errorf("failed to get masquerade IPs, network %s (%d): %v", bsnc.GetNetworkName(), networkID, err)
 	}
-	ops, err := addOrUpdatePodSNATOps(bsnc.nbClient, bsnc.GetNetworkScopedGWRouterName(pod.Spec.NodeName),
-		masqIPs, podIPs, nil)
+
+	ops, err := addOrUpdatePodSNATOps(bsnc.nbClient, bsnc.GetNetworkScopedGWRouterName(pod.Spec.NodeName), masqIPs, podIPs, bsnc.NetInfo.GetNetworkScopedClusterSubnetSNATMatch(pod.Spec.NodeName), nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct SNAT pods for pod %s/%s which is part of network %s, err: %v",
 			pod.Namespace, pod.Name, bsnc.GetNetworkName(), err)
@@ -459,7 +460,6 @@ func (bsnc *BaseSecondaryNetworkController) removePodForSecondaryNetwork(pod *ka
 	if err != nil {
 		return fmt.Errorf("failed looking for the active network at namespace '%s': %w", pod.Namespace, err)
 	}
-
 	for nadName := range podNetworks {
 		if !bsnc.HasNAD(nadName) {
 			continue
@@ -477,6 +477,15 @@ func (bsnc *BaseSecondaryNetworkController) removePodForSecondaryNetwork(pod *ka
 			return err
 		}
 
+		// Cleanup the SNAT entries before checking whether this controller handled the IP allocation
+		if util.IsNetworkSegmentationSupportEnabled() && bsnc.IsPrimaryNetwork() && config.Gateway.DisableSNATMultipleGWs {
+			// we need to delete per-pod SNATs for UDN networks
+			if err := bsnc.delPerPodSNAT(pod, nadName); err != nil {
+				return fmt.Errorf("failed to delete SNAT for pod %s/%s which is part of network %s, err: %v",
+					pod.Namespace, pod.Name, bsnc.GetNetworkName(), err)
+			}
+		}
+
 		// do not release IP address if this controller does not handle IP allocation
 		if !bsnc.allocatesPodAnnotation() {
 			continue
@@ -526,13 +535,6 @@ func (bsnc *BaseSecondaryNetworkController) removePodForSecondaryNetwork(pod *ka
 
 		bsnc.forgetPodReleasedBeforeStartup(string(pod.UID), nadName)
 
-		if util.IsNetworkSegmentationSupportEnabled() && bsnc.IsPrimaryNetwork() && config.Gateway.DisableSNATMultipleGWs {
-			// we need to delete per-pod SNATs for UDN networks
-			if err := bsnc.delPerPodSNAT(pod, nadName); err != nil {
-				return fmt.Errorf("failed to delete SNAT for pod %s/%s which is part of network %s, err: %v",
-					pod.Namespace, pod.Name, bsnc.GetNetworkName(), err)
-			}
-		}
 	}
 	return nil
 }
@@ -556,7 +558,7 @@ func (bsnc *BaseSecondaryNetworkController) delPerPodSNAT(pod *kapi.Pod, nadName
 	if err != nil {
 		return fmt.Errorf("failed to fetch annotations for pod %s/%s in network %s; err: %v", pod.Namespace, pod.Name, bsnc.GetNetworkName(), err)
 	}
-	ops, err := deletePodSNATOps(bsnc.nbClient, nil, bsnc.GetNetworkScopedGWRouterName(pod.Spec.NodeName), masqIPs, podNetAnnotation.IPs)
+	ops, err := deletePodSNATOps(bsnc.nbClient, nil, bsnc.GetNetworkScopedGWRouterName(pod.Spec.NodeName), masqIPs, podNetAnnotation.IPs, bsnc.GetNetworkScopedClusterSubnetSNATMatch(pod.Spec.NodeName))
 	if err != nil {
 		return fmt.Errorf("failed to construct SNAT pods for pod %s/%s which is part of network %s, err: %v",
 			pod.Namespace, pod.Name, bsnc.GetNetworkName(), err)

diff --git a/go-controller/pkg/ovn/base_secondary_layer2_network_controller.go b/go-controller/pkg/ovn/base_secondary_layer2_network_controller.go
@@ -110,7 +110,7 @@ func (oc *BaseSecondaryLayer2NetworkController) run() error {
 }
 
 func (oc *BaseSecondaryLayer2NetworkController) initializeLogicalSwitch(switchName string, clusterSubnets []config.CIDRNetworkEntry,
-	excludeSubnets []*net.IPNet) (*nbdb.LogicalSwitch, error) {
+	excludeSubnets []*net.IPNet, clusterLoadBalancerGroupUUID, switchLoadBalancerGroupUUID string) (*nbdb.LogicalSwitch, error) {
 	logicalSwitch := nbdb.LogicalSwitch{
 		Name:        switchName,
 		ExternalIDs: util.GenerateExternalIDsForSwitchOrRouter(oc.NetInfo),
@@ -134,6 +134,10 @@ func (oc *BaseSecondaryLayer2NetworkController) initializeLogicalSwitch(switchNa
 		}
 	}
 
+	if clusterLoadBalancerGroupUUID != "" && switchLoadBalancerGroupUUID != "" {
+		logicalSwitch.LoadBalancerGroup = []string{clusterLoadBalancerGroupUUID, switchLoadBalancerGroupUUID}
+	}
+
 	err := libovsdbops.CreateOrUpdateLogicalSwitch(oc.nbClient, &logicalSwitch)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create logical switch %+v: %v", logicalSwitch, err)

diff --git a/go-controller/pkg/ovn/controller/services/lb_config_test.go b/go-controller/pkg/ovn/controller/services/lb_config_test.go
@@ -6,11 +6,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	globalconfig "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	kubetest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
-	"github.com/stretchr/testify/assert"
 
 	v1 "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1"
@@ -1220,9 +1221,11 @@ func Test_buildClusterLBs(t *testing.T) {
 	defaultOpts := LBOpts{Reject: true}
 
 	globalconfig.IPv4Mode = true
-	UDNNetInfo, err := getSampleUDNNetInfo(namespace)
+	l3UDN, err := getSampleUDNNetInfo(namespace, "layer3")
+	assert.Equal(t, err, nil)
+	l2UDN, err := getSampleUDNNetInfo(namespace, "layer2")
 	assert.Equal(t, err, nil)
-	UDNGroups := []string{UDNNetInfo.GetNetworkScopedLoadBalancerGroupName(types.ClusterLBGroupName)}
+	udnNets := []util.NetInfo{l3UDN, l2UDN}
 
 	tc := []struct {
 		name      string
@@ -1443,14 +1446,18 @@ func Test_buildClusterLBs(t *testing.T) {
 			assert.Equal(t, tt.expected, actual)
 
 			// UDN
-			UDNExternalIDs := loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), UDNNetInfo.GetNetworkName())
-			for idx := range tt.expected {
-				tt.expected[idx].ExternalIDs = UDNExternalIDs
-				tt.expected[idx].Groups = UDNGroups
-				tt.expected[idx].Name = UDNNetInfo.GetNetworkScopedLoadBalancerName(tt.expected[idx].Name)
+			for _, udn := range udnNets {
+				UDNExternalIDs := loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), udn.GetNetworkName())
+				expected := make([]LB, len(tt.expected))
+				copy(expected, tt.expected)
+				for idx := range tt.expected {
+					expected[idx].ExternalIDs = UDNExternalIDs
+					expected[idx].Groups = []string{udn.GetNetworkScopedLoadBalancerGroupName(types.ClusterLBGroupName)}
+					expected[idx].Name = udn.GetNetworkScopedLoadBalancerName(tt.expected[idx].Name)
+				}
+				actual = buildClusterLBs(tt.service, tt.configs, tt.nodeInfos, true, udn)
+				assert.Equal(t, expected, actual)
 			}
-			actual = buildClusterLBs(tt.service, tt.configs, tt.nodeInfos, true, UDNNetInfo)
-			assert.Equal(t, tt.expected, actual)
 		})
 	}
 }
@@ -1479,8 +1486,11 @@ func Test_buildPerNodeLBs(t *testing.T) {
 	name := "foo"
 	namespace := "testns"
 
-	UDNNetInfo, err := getSampleUDNNetInfo(namespace)
-	assert.Equal(t, nil, err)
+	l3UDN, err := getSampleUDNNetInfo(namespace, "layer3")
+	assert.Equal(t, err, nil)
+	l2UDN, err := getSampleUDNNetInfo(namespace, "layer2")
+	assert.Equal(t, err, nil)
+	udnNetworks := []util.NetInfo{l3UDN, l2UDN}
 
 	defaultService := &v1.Service{
 		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
@@ -3100,8 +3110,6 @@ func Test_buildPerNodeLBs(t *testing.T) {
 		},
 	}
 
-	UDNExternalIDs := loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), UDNNetInfo.GetNetworkName())
-
 	// v4
 	for i, tt := range tc {
 		t.Run(fmt.Sprintf("%d_%s", i, tt.name), func(t *testing.T) {
@@ -3112,13 +3120,16 @@ func Test_buildPerNodeLBs(t *testing.T) {
 				assert.Equal(t, tt.expectedShared, actual, "shared gateway mode not as expected")
 
 				// UDN
-				for idx := range tt.expectedShared {
-					tt.expectedShared[idx].ExternalIDs = UDNExternalIDs
-					tt.expectedShared[idx].Name = UDNNetInfo.GetNetworkScopedLoadBalancerName(tt.expectedShared[idx].Name)
-
+				for _, udn := range udnNetworks {
+					expectedShared := make([]LB, len(tt.expectedShared))
+					copy(expectedShared, tt.expectedShared)
+					for idx := range tt.expectedShared {
+						expectedShared[idx].ExternalIDs = loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), udn.GetNetworkName())
+						expectedShared[idx].Name = udn.GetNetworkScopedLoadBalancerName(tt.expectedShared[idx].Name)
+					}
+					actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodes, udn)
+					assert.Equal(t, expectedShared, actual, "shared gateway mode not as expected")
 				}
-				actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodes, UDNNetInfo)
-				assert.Equal(t, tt.expectedShared, actual, "shared gateway mode not as expected")
 			}
 
 			if tt.expectedLocal != nil {
@@ -3129,13 +3140,16 @@ func Test_buildPerNodeLBs(t *testing.T) {
 				assert.Equal(t, tt.expectedLocal, actual, "local gateway mode not as expected")
 
 				// UDN
-				for idx := range tt.expectedLocal {
-					tt.expectedLocal[idx].ExternalIDs = UDNExternalIDs
-					tt.expectedLocal[idx].Name = UDNNetInfo.GetNetworkScopedLoadBalancerName(tt.expectedLocal[idx].Name)
-
+				for _, udn := range udnNetworks {
+					expectedLocal := make([]LB, len(tt.expectedLocal))
+					copy(expectedLocal, tt.expectedLocal)
+					for idx := range tt.expectedLocal {
+						expectedLocal[idx].ExternalIDs = loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), udn.GetNetworkName())
+						expectedLocal[idx].Name = udn.GetNetworkScopedLoadBalancerName(tt.expectedLocal[idx].Name)
+					}
+					actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodes, udn)
+					assert.Equal(t, expectedLocal, actual, "local gateway mode not as expected")
 				}
-				actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodes, UDNNetInfo)
-				assert.Equal(t, tt.expectedLocal, actual, "local gateway mode not as expected")
 			}
 
 		})
@@ -3154,13 +3168,16 @@ func Test_buildPerNodeLBs(t *testing.T) {
 				assert.Equal(t, tt.expectedShared, actual, "shared gateway mode not as expected")
 
 				// UDN
-				for idx := range tt.expectedShared {
-					tt.expectedShared[idx].ExternalIDs = UDNExternalIDs
-					tt.expectedShared[idx].Name = UDNNetInfo.GetNetworkScopedLoadBalancerName(tt.expectedShared[idx].Name)
-
+				for _, udn := range udnNetworks {
+					expectedShared := make([]LB, len(tt.expectedShared))
+					copy(expectedShared, tt.expectedShared)
+					for idx := range tt.expectedShared {
+						expectedShared[idx].ExternalIDs = loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), udn.GetNetworkName())
+						expectedShared[idx].Name = udn.GetNetworkScopedLoadBalancerName(tt.expectedShared[idx].Name)
+					}
+					actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodesV6, udn)
+					assert.Equal(t, expectedShared, actual, "shared gateway mode not as expected for UDN")
 				}
-				actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodesV6, UDNNetInfo)
-				assert.Equal(t, tt.expectedShared, actual, "shared gateway mode not as expected for UDN")
 			}
 
 			if tt.expectedLocal != nil {
@@ -3171,13 +3188,16 @@ func Test_buildPerNodeLBs(t *testing.T) {
 				assert.Equal(t, tt.expectedLocal, actual, "local gateway mode not as expected")
 
 				// UDN
-				for idx := range tt.expectedLocal {
-					tt.expectedLocal[idx].ExternalIDs = UDNExternalIDs
-					tt.expectedLocal[idx].Name = UDNNetInfo.GetNetworkScopedLoadBalancerName(tt.expectedLocal[idx].Name)
-
+				for _, udn := range udnNetworks {
+					expectedLocal := make([]LB, len(tt.expectedLocal))
+					copy(expectedLocal, tt.expectedLocal)
+					for idx := range tt.expectedLocal {
+						expectedLocal[idx].ExternalIDs = loadBalancerExternalIDsForNetwork(namespacedServiceName(namespace, name), udn.GetNetworkName())
+						expectedLocal[idx].Name = udn.GetNetworkScopedLoadBalancerName(tt.expectedLocal[idx].Name)
+					}
+					actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodesV6, udn)
+					assert.Equal(t, expectedLocal, actual, "local gateway mode not as expected for UDN")
 				}
-				actual = buildPerNodeLBs(tt.service, tt.configs, defaultNodesV6, UDNNetInfo)
-				assert.Equal(t, tt.expectedLocal, actual, "local gateway mode not as expected for UDN")
 			}
 
 		})

diff --git a/go-controller/pkg/ovn/controller/services/node_tracker.go b/go-controller/pkg/ovn/controller/services/node_tracker.go
@@ -209,7 +209,15 @@ func (nt *nodeTracker) removeNode(nodeName string) {
 // The gateway router will exist sometime after the L3Gateway annotation is set.
 func (nt *nodeTracker) updateNode(node *v1.Node) {
 	klog.V(2).Infof("Processing possible switch / router updates for node %s", node.Name)
-	hsn, err := util.ParseNodeHostSubnetAnnotation(node, nt.netInfo.GetNetworkName())
+	var hsn []*net.IPNet
+	var err error
+	if nt.netInfo.TopologyType() == types.Layer2Topology {
+		for _, subnet := range nt.netInfo.Subnets() {
+			hsn = append(hsn, subnet.CIDR)
+		}
+	} else {
+		hsn, err = util.ParseNodeHostSubnetAnnotation(node, nt.netInfo.GetNetworkName())
+	}
 	if err != nil || hsn == nil || util.NoHostSubnet(node) {
 		// usually normal; means the node's gateway hasn't been initialized yet
 		klog.Infof("Node %s has invalid / no HostSubnet annotations (probably waiting on initialization), or it's a hybrid overlay node: %v", node.Name, err)