diff --git a/go-controller/pkg/libovsdb/libovsdb.go b/go-controller/pkg/libovsdb/libovsdb.go index cd7a2bce72..40bd1298fe 100644 --- a/go-controller/pkg/libovsdb/libovsdb.go +++ b/go-controller/pkg/libovsdb/libovsdb.go @@ -140,10 +140,6 @@ func NewSBClientWithConfig(cfg config.OvnAuthConfig, promRegistry prometheus.Reg enableMetricsOption := client.WithMetricsRegistryNamespaceSubsystem(promRegistry, "ovnkube", "master_libovsdb") - dbModel.SetIndexes(map[string][]model.ClientIndex{ - sbdb.EncapTable: {{Columns: []model.ColumnKey{{Column: "chassis_name"}}}}, - }) - c, err := newClient(cfg, dbModel, stopCh, enableMetricsOption) if err != nil { return nil, err diff --git a/go-controller/pkg/libovsdb/ops/chassis.go b/go-controller/pkg/libovsdb/ops/chassis.go index c1d67b614a..0196da3463 100644 --- a/go-controller/pkg/libovsdb/ops/chassis.go +++ b/go-controller/pkg/libovsdb/ops/chassis.go @@ -138,27 +138,33 @@ func DeleteChassisWithPredicate(sbClient libovsdbclient.Client, p chassisPredica } // CreateOrUpdateChassis creates or updates the chassis record along with the encap record -func CreateOrUpdateChassis(sbClient libovsdbclient.Client, chassis *sbdb.Chassis, encap *sbdb.Encap) error { +func CreateOrUpdateChassis(sbClient libovsdbclient.Client, chassis *sbdb.Chassis, encaps ...*sbdb.Encap) error { m := newModelClient(sbClient) - opModels := []operationModel{ - { + opModels := make([]operationModel, 0, len(encaps)+1) + for i := range encaps { + encap := encaps[i] + opModel := operationModel{ Model: encap, DoAfter: func() { - chassis.Encaps = []string{encap.UUID} + encapsList := append(chassis.Encaps, encap.UUID) + chassis.Encaps = sets.New(encapsList...).UnsortedList() }, - OnModelUpdates: onModelUpdatesAllNonDefault(), + OnModelUpdates: onModelUpdatesNone(), ErrNotFound: false, BulkOp: false, - }, - { - Model: chassis, - OnModelMutations: []interface{}{&chassis.OtherConfig}, - OnModelUpdates: []interface{}{&chassis.Encaps}, - ErrNotFound: false, - BulkOp: false, - }, + } + opModels = append(opModels, opModel) + } + + opModel := operationModel{ + Model: chassis, + OnModelMutations: []interface{}{&chassis.OtherConfig}, + OnModelUpdates: []interface{}{&chassis.Encaps}, + ErrNotFound: false, + BulkOp: false, } + opModels = append(opModels, opModel) if _, err := m.CreateOrUpdate(opModels...); err != nil { return err } diff --git a/go-controller/pkg/libovsdb/ops/chassis_test.go b/go-controller/pkg/libovsdb/ops/chassis_test.go index 1d8f338081..7c60cc4217 100644 --- a/go-controller/pkg/libovsdb/ops/chassis_test.go +++ b/go-controller/pkg/libovsdb/ops/chassis_test.go @@ -181,3 +181,97 @@ func TestDeleteChassis(t *testing.T) { }) } } + +func TestCreateOrUpdateChassis(t *testing.T) { + uuid1 := "b9998337-2498-4d1e-86e6-fc0417abb2f0" + uuid2 := "b9998337-2498-4d1e-86e6-fc0417abb2f1" + uuid3 := "b9998337-2498-4d1e-86e6-fc0417abb2f2" + tests := []struct { + desc string + chassis *sbdb.Chassis + encaps []*sbdb.Encap + initialDB []libovsdbtest.TestData + expectedDB []libovsdbtest.TestData + }{ + { + desc: "create new chassis with encap records", + chassis: &sbdb.Chassis{Name: "test1"}, + encaps: []*sbdb.Encap{{ChassisName: "test1", IP: "10.0.0.10", Type: "geneve"}, + {ChassisName: "test1", IP: "10.0.0.11", Type: "geneve"}}, + initialDB: []libovsdbtest.TestData{}, + expectedDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test1", Encaps: []string{uuid2, uuid3}}, + &sbdb.Encap{UUID: uuid2, ChassisName: "test1", IP: "10.0.0.10", Type: "geneve"}, + &sbdb.Encap{UUID: uuid3, ChassisName: "test1", IP: "10.0.0.11", Type: "geneve"}, + }, + }, + { + desc: "update chassis by inserting new encap record", + chassis: &sbdb.Chassis{Name: "test2"}, + encaps: []*sbdb.Encap{{ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"}, + {ChassisName: "test2", IP: "10.0.0.11", Type: "geneve"}}, + initialDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test2", Encaps: []string{uuid2}}, + &sbdb.Encap{UUID: uuid2, ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"}, + }, + expectedDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test2", Encaps: []string{uuid2, uuid3}}, + &sbdb.Encap{UUID: uuid2, ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"}, + &sbdb.Encap{UUID: uuid3, ChassisName: "test2", IP: "10.0.0.11", Type: "geneve"}, + }, + }, + { + desc: "update chassis by removing obsolete encap record", + chassis: &sbdb.Chassis{Name: "test3"}, + encaps: []*sbdb.Encap{{ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"}}, + initialDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test3", Encaps: []string{uuid2, uuid3}}, + &sbdb.Encap{UUID: uuid2, ChassisName: "test3", IP: "10.0.0.10", Type: "geneve"}, + &sbdb.Encap{UUID: uuid3, ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"}, + }, + expectedDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test3", Encaps: []string{uuid3}}, + &sbdb.Encap{UUID: uuid3, ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"}, + }, + }, + { + desc: "update chassis by adding new encap record and deleting the old one", + chassis: &sbdb.Chassis{Name: "test4"}, + encaps: []*sbdb.Encap{{ChassisName: "test4", IP: "10.0.0.11", Type: "geneve"}}, + initialDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test4", Encaps: []string{uuid2}}, + &sbdb.Encap{UUID: uuid2, ChassisName: "test4", IP: "10.0.0.10", Type: "geneve"}, + }, + expectedDB: []libovsdbtest.TestData{ + &sbdb.Chassis{UUID: uuid1, Name: "test4", Encaps: []string{uuid3}}, + &sbdb.Encap{UUID: uuid3, ChassisName: "test4", IP: "10.0.0.11", Type: "geneve"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + dbSetup := libovsdbtest.TestSetup{ + SBData: tt.initialDB, + } + sbClient, cleanup, err := libovsdbtest.NewSBTestHarness(dbSetup, nil) + if err != nil { + t.Fatalf("%s: failed to set up test harness: %v", tt.desc, err) + } + t.Cleanup(cleanup.Cleanup) + + err = CreateOrUpdateChassis(sbClient, tt.chassis, tt.encaps...) + if err != nil { + t.Fatal(fmt.Errorf("%s: got unexpected error: %v", tt.desc, err)) + } + + matcher := libovsdbtest.HaveDataIgnoringUUIDs(tt.expectedDB) + match, err := matcher.Match(sbClient) + if err != nil { + t.Fatalf("%s: matcher error: %v", tt.desc, err) + } + if !match { + t.Fatalf("%s: DB state did not match: %s", tt.desc, matcher.FailureMessage(sbClient)) + } + }) + } +} diff --git a/go-controller/pkg/node/default_node_network_controller.go b/go-controller/pkg/node/default_node_network_controller.go index 8afed02171..dfede1c2a8 100644 --- a/go-controller/pkg/node/default_node_network_controller.go +++ b/go-controller/pkg/node/default_node_network_controller.go @@ -20,6 +20,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" @@ -925,6 +926,13 @@ func (nc *DefaultNodeNetworkController) Init(ctx context.Context) error { if err := util.SetNodeZone(nodeAnnotator, sbZone); err != nil { return fmt.Errorf("failed to set node zone annotation for node %s: %w", nc.name, err) } + + encapIPList := sets.New[string]() + encapIPList.Insert(strings.Split(config.Default.EffectiveEncapIP, ",")...) + if err := util.SetNodeEncapIPs(nodeAnnotator, encapIPList); err != nil { + return fmt.Errorf("failed to set node-encap-ips annotation for node %s: %w", nc.name, err) + } + if err := nodeAnnotator.Run(); err != nil { return fmt.Errorf("failed to set node %s annotations: %w", nc.name, err) } diff --git a/go-controller/pkg/node/gateway_shared_intf.go b/go-controller/pkg/node/gateway_shared_intf.go index acdc814430..4f8d4bfea7 100644 --- a/go-controller/pkg/node/gateway_shared_intf.go +++ b/go-controller/pkg/node/gateway_shared_intf.go @@ -1937,10 +1937,9 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone, netConfig.masqCTMark, ofPortPhys)) - // Allow (a) OVN->host traffic on the same node - // (b) host->host traffic on the same node + // Allow OVN->Host traffic on the same node if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal { - dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...) + dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...) } } else { // for UDN we additionally SNAT the packet from masquerade IP -> node IP @@ -2034,10 +2033,9 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin "actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:%s", defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone, netConfig.masqCTMark, ofPortPhys)) - // Allow (a) OVN->host traffic on the same node - // (b) host->host traffic on the same node + // Allow OVN->Host traffic on the same node if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal { - dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...) + dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...) } } else { // for UDN we additionally SNAT the packet from masquerade IP -> node IP @@ -2217,15 +2215,23 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin return dftFlows, nil } -// hostNetworkNormalActionFlows returns the flows that allow IP{v4,v6} traffic: -// a. from pods in the OVN network to pods in a localnet network, on the same node -// b. from pods on the host to pods in a localnet network, on the same node -// when the localnet is mapped to breth0. -// The expected srcMAC is the MAC address of breth0 and the expected hostSubnets is the host subnets found on the node -// primary interface. -func hostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC string, hostSubnets []*net.IPNet, isV6 bool) []string { +// ovnToHostNetworkNormalActionFlows returns the flows that allow IP{v4,v6} traffic from the OVN network to the host network +// when the destination is on the same node as the sender. This is necessary for pods in the default network to reach +// localnet pods on the same node, when the localnet is mapped to breth0. The expected srcMAC is the MAC address of breth0 +// and the expected hostSubnets is the host subnets found on the node primary interface. +func ovnToHostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC string, hostSubnets []*net.IPNet, isV6 bool) []string { + var inPort, ctMark, ipFamily, ipFamilyDest string var flows []string - var ipFamily, ipFamilyDest string + + if config.Gateway.Mode == config.GatewayModeShared { + inPort = netConfig.ofPortPatch + ctMark = netConfig.masqCTMark + } else if config.Gateway.Mode == config.GatewayModeLocal { + inPort = "LOCAL" + ctMark = ctMarkHost + } else { + return nil + } if isV6 { ipFamily = "ipv6" @@ -2235,69 +2241,38 @@ func hostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC stri ipFamilyDest = "nw_dst" } - formatFlow := func(inPort, destIP, ctMark string) string { - // Matching IP traffic will be handled by the bridge instead of being output directly - // to the NIC by the existing flow at prio=100. - flowTemplate := "cookie=%s, priority=102, in_port=%s, dl_src=%s, %s, %s=%s, " + - "actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL" - return fmt.Sprintf(flowTemplate, - defaultOpenFlowCookie, - inPort, - srcMAC, - ipFamily, - ipFamilyDest, - destIP, - config.Default.ConntrackZone, - ctMark) - } - - // Traffic path (a): OVN->localnet for shared gw mode - if config.Gateway.Mode == config.GatewayModeShared { - for _, hostSubnet := range hostSubnets { - if utilnet.IsIPv6(hostSubnet.IP) != isV6 { - continue - } - flows = append(flows, formatFlow(netConfig.ofPortPatch, hostSubnet.String(), netConfig.masqCTMark)) - } - } - - // Traffic path (a): OVN->localnet for local gw mode - // Traffic path (b): host->localnet for both gw modes for _, hostSubnet := range hostSubnets { - if utilnet.IsIPv6(hostSubnet.IP) != isV6 { + if (hostSubnet.IP.To4() == nil) != isV6 { continue } - flows = append(flows, formatFlow("LOCAL", hostSubnet.String(), ctMarkHost)) - } - - if isV6 { - // IPv6 neighbor discovery uses ICMPv6 messages sent to a special destination (ff02::1:ff00:0/104) - // that is unrelated to the host subnets matched in the prio=102 flow above. - // Allow neighbor discovery by matching against ICMP type and ingress port. - formatICMPFlow := func(inPort, ctMark string, icmpType int) string { - icmpFlowTemplate := "cookie=%s, priority=102, in_port=%s, dl_src=%s, icmp6, icmpv6_type=%d, " + - "actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL" - return fmt.Sprintf(icmpFlowTemplate, + // IP traffic from the OVN network to the host network should be handled normally by the bridge instead of + // being output directly to the NIC by the existing flow at prio=100. + flows = append(flows, + fmt.Sprintf("cookie=%s, priority=102, in_port=%s, dl_src=%s, %s, %s=%s, "+ + "actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL", defaultOpenFlowCookie, inPort, srcMAC, - icmpType, + ipFamily, + ipFamilyDest, + hostSubnet.String(), config.Default.ConntrackZone, - ctMark) - } + ctMark)) + } + if isV6 { + // Neighbor discovery in IPv6 happens through ICMPv6 messages to a special destination (ff02::1:ff00:0/104), + // which has nothing to do with the host subnets we're matching against in the flow above at prio=102. + // Let's allow neighbor discovery by matching against icmp type and in_port. for _, icmpType := range []int{types.NeighborSolicitationICMPType, types.NeighborAdvertisementICMPType} { - // Traffic path (a) for ICMP: OVN-> localnet for shared gw mode - if config.Gateway.Mode == config.GatewayModeShared { - flows = append(flows, - formatICMPFlow(netConfig.ofPortPatch, netConfig.masqCTMark, icmpType)) - } - - // Traffic path (a) for ICMP: OVN->localnet for local gw mode - // Traffic path (b) for ICMP: host->localnet for both gw modes - flows = append(flows, formatICMPFlow("LOCAL", ctMarkHost, icmpType)) + flows = append(flows, + fmt.Sprintf("cookie=%s, priority=102, in_port=%s, dl_src=%s, icmp6, icmpv6_type=%d, "+ + "actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL", + defaultOpenFlowCookie, inPort, srcMAC, icmpType, + config.Default.ConntrackZone, ctMark)) } } + return flows } diff --git a/go-controller/pkg/ovn/default_network_controller.go b/go-controller/pkg/ovn/default_network_controller.go index 705a4da468..cd693e6c18 100644 --- a/go-controller/pkg/ovn/default_network_controller.go +++ b/go-controller/pkg/ovn/default_network_controller.go @@ -943,6 +943,8 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int newNodeIsLocalZoneNode := h.oc.isLocalZoneNode(newNode) zoneClusterChanged := h.oc.nodeZoneClusterChanged(oldNode, newNode, newNodeIsLocalZoneNode, types.DefaultNetworkName) nodeSubnetChange := nodeSubnetChanged(oldNode, newNode, types.DefaultNetworkName) + nodeEncapIPsChanged := util.NodeEncapIPsChanged(oldNode, newNode) + var aggregatedErrors []error if newNodeIsLocalZoneNode { var nodeSyncsParam *nodeSyncs @@ -989,7 +991,8 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int // Check if the node moved from local zone to remote zone and if so syncZoneIC should be set to true. // Also check if node subnet changed, so static routes are properly set // Also check if the node is used to be a hybrid overlay node - syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChange || zoneClusterChanged || primaryAddrChanged(oldNode, newNode) || switchToOvnNode + syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChange || zoneClusterChanged || + switchToOvnNode || nodeEncapIPsChanged if syncZoneIC { klog.Infof("Node %s in remote zone %s needs interconnect zone sync up. Zone cluster changed: %v", newNode.Name, util.GetNodeZone(newNode), zoneClusterChanged) diff --git a/go-controller/pkg/ovn/zone_interconnect/chassis_handler.go b/go-controller/pkg/ovn/zone_interconnect/chassis_handler.go index 800aa8bcf1..172cac5e33 100644 --- a/go-controller/pkg/ovn/zone_interconnect/chassis_handler.go +++ b/go-controller/pkg/ovn/zone_interconnect/chassis_handler.go @@ -134,9 +134,29 @@ func (zch *ZoneChassisHandler) createOrUpdateNodeChassis(node *corev1.Node, isRe node.Name, parsedErr) } - nodePrimaryIp, err := util.GetNodePrimaryIP(node) + // Get the encap IPs. + encapIPs, err := util.ParseNodeEncapIPsAnnotation(node) if err != nil { - return fmt.Errorf("failed to parse node %s primary IP %w", node.Name, err) + return fmt.Errorf("failed to parse node-encap-ips for node - %s, error: %w", + node.Name, err) + } + + encaps := make([]*sbdb.Encap, 0, len(encapIPs)) + encapOptions := map[string]string{} + encapOptions["csum"] = "true" + // set the geneve port if using something else than default + if config.Default.EncapPort != config.DefaultEncapPort { + encapOptions["dst_port"] = strconv.FormatUint(uint64(config.Default.EncapPort), 10) + } + + for _, ovnEncapIP := range encapIPs { + encap := sbdb.Encap{ + ChassisName: chassisID, + IP: strings.TrimSpace(ovnEncapIP), + Type: "geneve", + Options: encapOptions, + } + encaps = append(encaps, &encap) } chassis := sbdb.Chassis{ @@ -147,17 +167,5 @@ func (zch *ZoneChassisHandler) createOrUpdateNodeChassis(node *corev1.Node, isRe }, } - encap := sbdb.Encap{ - ChassisName: chassisID, - IP: nodePrimaryIp, - Type: "geneve", - Options: map[string]string{"csum": "true"}, - } - - // set the geneve port if using something else than default - if config.Default.EncapPort != config.DefaultEncapPort { - encap.Options["dst_port"] = strconv.FormatUint(uint64(config.Default.EncapPort), 10) - } - - return libovsdbops.CreateOrUpdateChassis(zch.sbClient, &chassis, &encap) + return libovsdbops.CreateOrUpdateChassis(zch.sbClient, &chassis, encaps...) } diff --git a/go-controller/pkg/ovn/zone_interconnect/chassis_handler_test.go b/go-controller/pkg/ovn/zone_interconnect/chassis_handler_test.go index a9d3dcd92c..05b9fb6b9c 100644 --- a/go-controller/pkg/ovn/zone_interconnect/chassis_handler_test.go +++ b/go-controller/pkg/ovn/zone_interconnect/chassis_handler_test.go @@ -16,6 +16,7 @@ import ( libovsdbops "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdb/ops" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/sbdb" libovsdbtest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/libovsdb" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" ) var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { @@ -25,9 +26,14 @@ var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { testNode1 corev1.Node testNode2 corev1.Node testNode3 corev1.Node + testNode4 corev1.Node + testNode5 corev1.Node node1Chassis sbdb.Chassis node2Chassis sbdb.Chassis node3Chassis sbdb.Chassis + node4Chassis sbdb.Chassis + node5Chassis sbdb.Chassis + node5Encap sbdb.Encap initialSBDB []libovsdbtest.TestData ) @@ -50,11 +56,17 @@ var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { node1Chassis = sbdb.Chassis{Name: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac6", Hostname: "node1", UUID: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac6"} node2Chassis = sbdb.Chassis{Name: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac7", Hostname: "node2", UUID: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac7"} node3Chassis = sbdb.Chassis{Name: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac8", Hostname: "node3", UUID: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac8"} + node4Chassis = sbdb.Chassis{Name: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac9", Hostname: "node4", UUID: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac9"} + node5Chassis = sbdb.Chassis{Name: "cb9ec8fa-b409-4ef3-9f42-d9283c47aaca", Hostname: "node5", UUID: "cb9ec8fa-b409-4ef3-9f42-d9283c47aac9a", + Encaps: []string{"cb9ec8fa-b409-4ef3-9f42-d9283c47aacb"}} + node5Encap = sbdb.Encap{ChassisName: "cb9ec8fa-b409-4ef3-9f42-d9283c47aaca", IP: "10.0.0.16", Type: "geneve", + UUID: "cb9ec8fa-b409-4ef3-9f42-d9283c47aacb"} testNode1 = corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac6"}, + Name: "node1", + Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac6", + "k8s.ovn.org/node-encap-ips": "[\"10.0.0.10\"]"}, }, Status: corev1.NodeStatus{ Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.0.0.10"}}, @@ -62,8 +74,9 @@ var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { } testNode2 = corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "node2", - Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac7"}, + Name: "node2", + Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac7", + "k8s.ovn.org/node-encap-ips": "[\"10.0.0.11\"]"}, }, Status: corev1.NodeStatus{ Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.0.0.11"}}, @@ -71,16 +84,31 @@ var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { } testNode3 = corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: "node3", - Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac8"}, + Name: "node3", + Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac8", + "k8s.ovn.org/node-encap-ips": "[\"10.0.0.12\"]"}, }, Status: corev1.NodeStatus{ Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.0.0.12"}}, }, } + testNode4 = corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node4", + Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aac9", + "k8s.ovn.org/node-encap-ips": "[\"10.0.0.14\", \"10.0.0.15\"]"}, + }, + } + testNode5 = corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node5", + Annotations: map[string]string{"k8s.ovn.org/node-chassis-id": "cb9ec8fa-b409-4ef3-9f42-d9283c47aaca", + "k8s.ovn.org/node-encap-ips": "[\"10.0.0.11\"]"}, + }, + } initialSBDB = []libovsdbtest.TestData{ - &node1Chassis, &node2Chassis} + &node1Chassis, &node2Chassis, &node5Chassis, &node5Encap} }) ginkgo.AfterEach(func() { @@ -155,9 +183,12 @@ var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { err = zoneChassisHandler.AddRemoteZoneNode(&testNode3) gomega.Expect(err).NotTo(gomega.HaveOccurred()) + encapIP, err := util.ParseNodeEncapIPsAnnotation(&testNode3) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + encap := &sbdb.Encap{ Type: "geneve", - IP: testNode3.Status.Addresses[0].Address, + IP: encapIP[0], } err = libovsdbOvnSBClient.Get(context.Background(), encap) gomega.Expect(err).NotTo(gomega.HaveOccurred()) @@ -175,6 +206,108 @@ var _ = ginkgo.Describe("Zone Interconnect Chassis Operations", func() { gomega.Expect(err).NotTo(gomega.HaveOccurred()) }) + ginkgo.It("Add multiple encap records", func() { + app.Action = func(ctx *cli.Context) error { + dbSetup := libovsdbtest.TestSetup{ + SBData: initialSBDB, + } + + _, err := config.InitConfig(ctx, nil, nil) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + config.Kubernetes.HostNetworkNamespace = "" + + var libovsdbOvnSBClient libovsdbclient.Client + _, libovsdbOvnSBClient, libovsdbCleanup, err = libovsdbtest.NewNBSBTestHarness(dbSetup) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + zoneChassisHandler := NewZoneChassisHandler(libovsdbOvnSBClient) + err = zoneChassisHandler.AddRemoteZoneNode(&testNode4) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + encapIP, err := util.ParseNodeEncapIPsAnnotation(&testNode4) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + encap1 := &sbdb.Encap{ + Type: "geneve", + IP: encapIP[0], + } + + encap2 := &sbdb.Encap{ + Type: "geneve", + IP: encapIP[1], + } + + err = libovsdbOvnSBClient.Get(context.Background(), encap1) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + err = libovsdbOvnSBClient.Get(context.Background(), encap2) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + nodeCh, err := libovsdbops.GetChassis(libovsdbOvnSBClient, &node4Chassis) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(nodeCh.Encaps).To(gomega.HaveLen(2)) + gomega.Expect(nodeCh.Encaps).To(gomega.ContainElements(string(encap1.UUID)), string(encap2.UUID)) + + return nil + } + err := app.Run([]string{ + app.Name, + "-cluster-subnets=" + clusterCIDR, + "-init-cluster-manager", + "-zone-join-switch-subnets=" + joinSubnetCIDR, + "-enable-interconnect", + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) + + ginkgo.It("Update encap record when chassis exists", func() { + app.Action = func(ctx *cli.Context) error { + dbSetup := libovsdbtest.TestSetup{ + SBData: initialSBDB, + } + + _, err := config.InitConfig(ctx, nil, nil) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + config.Kubernetes.HostNetworkNamespace = "" + + var libovsdbOvnSBClient libovsdbclient.Client + _, libovsdbOvnSBClient, libovsdbCleanup, err = libovsdbtest.NewNBSBTestHarness(dbSetup) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + zoneChassisHandler := NewZoneChassisHandler(libovsdbOvnSBClient) + err = zoneChassisHandler.AddRemoteZoneNode(&testNode5) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + encapIP, err := util.ParseNodeEncapIPsAnnotation(&testNode5) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + encap := &sbdb.Encap{ + Type: "geneve", + IP: encapIP[0], + } + + err = libovsdbOvnSBClient.Get(context.Background(), encap) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + nodeCh, err := libovsdbops.GetChassis(libovsdbOvnSBClient, &node5Chassis) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(nodeCh.Encaps).To(gomega.HaveLen(1)) + gomega.Expect(nodeCh.Encaps).To(gomega.ContainElements(string(encap.UUID))) + + err = libovsdbOvnSBClient.Get(context.Background(), &node5Encap) + gomega.Expect(err).To(gomega.SatisfyAny(gomega.BeNil(), gomega.MatchError(libovsdbclient.ErrNotFound))) + + return nil + } + err := app.Run([]string{ + app.Name, + "-cluster-subnets=" + clusterCIDR, + "-init-cluster-manager", + "-zone-join-switch-subnets=" + joinSubnetCIDR, + "-enable-interconnect", + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) + ginkgo.It("Move chassis zone", func() { app.Action = func(ctx *cli.Context) error { dbSetup := libovsdbtest.TestSetup{ diff --git a/go-controller/pkg/ovnwebhook/nodeadmission.go b/go-controller/pkg/ovnwebhook/nodeadmission.go index dbdc9b767e..b21a51bc87 100644 --- a/go-controller/pkg/ovnwebhook/nodeadmission.go +++ b/go-controller/pkg/ovnwebhook/nodeadmission.go @@ -52,6 +52,7 @@ var commonNodeAnnotationChecks = map[string]checkNodeAnnot{ return fmt.Errorf("%s can only be set to %s or %s, it cannot be removed", util.OvnNodeZoneName, types.OvnDefaultZone, nodeName) }, + util.OVNNodeEncapIPs: nil, } // interconnectNodeAnnotationChecks holds annotations allowed for ovnkube-node: users in IC environments diff --git a/go-controller/pkg/util/node_annotations.go b/go-controller/pkg/util/node_annotations.go index 7d6e524048..d3be36f2db 100644 --- a/go-controller/pkg/util/node_annotations.go +++ b/go-controller/pkg/util/node_annotations.go @@ -151,6 +151,9 @@ const ( // "l2-network-b":"10"} // }", ovnUDNLayer2NodeGRLRPTunnelIDs = "k8s.ovn.org/udn-layer2-node-gateway-router-lrp-tunnel-ids" + + // ovnNodeEncapIPs is used to indicate encap IPs set on the node + OVNNodeEncapIPs = "k8s.ovn.org/node-encap-ips" ) type L3GatewayConfig struct { @@ -1474,3 +1477,27 @@ func filterIPVersion(cidrs []netip.Prefix, v6 bool) []netip.Prefix { } return validCIDRs } + +func SetNodeEncapIPs(nodeAnnotator kube.Annotator, encapips sets.Set[string]) error { + return nodeAnnotator.Set(OVNNodeEncapIPs, sets.List(encapips)) +} + +// ParseNodeEncapIPsAnnotation returns the encap IPs set on a node +func ParseNodeEncapIPsAnnotation(node *corev1.Node) ([]string, error) { + encapIPsAnnotation, ok := node.Annotations[OVNNodeEncapIPs] + if !ok { + return nil, newAnnotationNotSetError("%s annotation not found for node %q", OVNNodeEncapIPs, node.Name) + } + + var encapIPs []string + if err := json.Unmarshal([]byte(encapIPsAnnotation), &encapIPs); err != nil { + return nil, fmt.Errorf("failed to unmarshal %s annotation for node %q: %v", + encapIPsAnnotation, node.Name, err) + } + + return encapIPs, nil +} + +func NodeEncapIPsChanged(oldNode, newNode *corev1.Node) bool { + return oldNode.Annotations[OVNNodeEncapIPs] != newNode.Annotations[OVNNodeEncapIPs] +} diff --git a/test/e2e/multihoming.go b/test/e2e/multihoming.go index 7715469c3d..88b15568d8 100644 --- a/test/e2e/multihoming.go +++ b/test/e2e/multihoming.go @@ -331,31 +331,17 @@ var _ = Describe("Multi Homing", func() { kickstartPod(cs, clientPodConfig) // Check that the client pod can reach the server pod on the server localnet interface - var serverIPs []string - if serverPodConfig.hostNetwork { - serverIPs, err = podIPsFromStatus(cs, serverPodConfig.namespace, serverPodConfig.name) - } else { - serverIPs, err = podIPsForAttachment(cs, serverPod.Namespace, serverPod.Name, netConfig.name) - - } + serverIPs, err := podIPsForAttachment(cs, f.Namespace.Name, serverPod.GetName(), netConfig.name) Expect(err).NotTo(HaveOccurred()) - for _, serverIP := range serverIPs { By(fmt.Sprintf("asserting the *client* can contact the server pod exposed endpoint: %q on port %q", serverIP, port)) - curlArgs := []string{} - pingArgs := []string{} - if clientPodConfig.attachments != nil { - // When the client is attached to a localnet, send probes from the localnet interface - curlArgs = []string{"--interface", "net1"} - pingArgs = []string{"-I", "net1"} - } Eventually(func() error { - return reachServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, port, curlArgs...) + return reachServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, port) }, 2*time.Minute, 6*time.Second).Should(Succeed()) By(fmt.Sprintf("asserting the *client* can ping the server pod exposed endpoint: %q", serverIP)) Eventually(func() error { - return pingServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, pingArgs...) + return pingServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP) }, 2*time.Minute, 6*time.Second).Should(Succeed()) } }, @@ -403,52 +389,6 @@ var _ = Describe("Multi Homing", func() { }, Label("BUG", "OCPBUGS-43004"), ), - ginkgo.Entry( - "can reach a host-networked pod on a different node", - networkAttachmentConfigParams{ - name: secondaryNetworkName, - topology: "localnet", - }, - podConfiguration{ // client on localnet - attachments: []nadapi.NetworkSelectionElement{{ - Name: secondaryNetworkName, - }}, - name: clientPodName, - nodeSelector: map[string]string{nodeHostnameKey: workerOneNodeName}, - isPrivileged: true, - needsIPRequestFromHostSubnet: true, - }, - podConfiguration{ // server on default network, pod is host-networked - name: podName, - containerCmd: httpServerContainerCmd(port), - nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName}, - hostNetwork: true, - }, - Label("STORY", "SDN-5345"), - ), - ginkgo.Entry( - "can reach a host-networked pod on the same node", - networkAttachmentConfigParams{ - name: secondaryNetworkName, - topology: "localnet", - }, - podConfiguration{ // client on localnet - attachments: []nadapi.NetworkSelectionElement{{ - Name: secondaryNetworkName, - }}, - name: clientPodName, - nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName}, - isPrivileged: true, - needsIPRequestFromHostSubnet: true, - }, - podConfiguration{ // server on default network, pod is host-networked - name: podName, - containerCmd: httpServerContainerCmd(port), - nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName}, - hostNetwork: true, - }, - Label("STORY", "SDN-5345"), - ), ) }) @@ -908,6 +848,7 @@ var _ = Describe("Multi Homing", func() { Context("localnet OVN-K secondary network", func() { const ( clientPodName = "client-pod" + nodeHostnameKey = "kubernetes.io/hostname" servicePort = 9000 dockerNetworkName = "underlay" underlayServiceIP = "60.128.0.1" diff --git a/test/e2e/multihoming_utils.go b/test/e2e/multihoming_utils.go index 7ddd109eae..1c0d1a7435 100644 --- a/test/e2e/multihoming_utils.go +++ b/test/e2e/multihoming_utils.go @@ -161,7 +161,6 @@ type podConfiguration struct { isPrivileged bool labels map[string]string requiresExtraNamespace bool - hostNetwork bool needsIPRequestFromHostSubnet bool } @@ -172,7 +171,6 @@ func generatePodSpec(config podConfiguration) *v1.Pod { } podSpec.Spec.NodeSelector = config.nodeSelector podSpec.Labels = config.labels - podSpec.Spec.HostNetwork = config.hostNetwork if config.isPrivileged { podSpec.Spec.Containers[0].SecurityContext.Privileged = ptr.To(true) } else { @@ -255,19 +253,17 @@ func inRange(cidr string, ip string) error { return fmt.Errorf("ip [%s] is NOT in range %s", ip, cidr) } -func connectToServer(clientPodConfig podConfiguration, serverIP string, port int, args ...string) error { - target := net.JoinHostPort(serverIP, fmt.Sprintf("%d", port)) - baseArgs := []string{ +func connectToServer(clientPodConfig podConfiguration, serverIP string, port int) error { + _, err := e2ekubectl.RunKubectl( + clientPodConfig.namespace, "exec", clientPodConfig.name, "--", "curl", "--connect-timeout", "2", - } - baseArgs = append(baseArgs, args...) - - _, err := e2ekubectl.RunKubectl(clientPodConfig.namespace, append(baseArgs, target)...) + net.JoinHostPort(serverIP, fmt.Sprintf("%d", port)), + ) return err } @@ -312,19 +308,16 @@ func getSecondaryInterfaceMTU(clientPodConfig podConfiguration) (int, error) { return mtu, nil } -func pingServer(clientPodConfig podConfiguration, serverIP string, args ...string) error { - baseArgs := []string{ +func pingServer(clientPodConfig podConfiguration, serverIP string) error { + _, err := e2ekubectl.RunKubectl( + clientPodConfig.namespace, "exec", clientPodConfig.name, "--", "ping", "-c", "1", // send one ICMP echo request "-W", "2", // timeout after 2 seconds if no response - } - baseArgs = append(baseArgs, args...) - - _, err := e2ekubectl.RunKubectl(clientPodConfig.namespace, append(baseArgs, serverIP)...) - + serverIP) return err } @@ -388,18 +381,6 @@ func podIPForAttachment(k8sClient clientset.Interface, podNamespace string, podN return ips[ipIndex], nil } -func podIPsFromStatus(k8sClient clientset.Interface, podNamespace string, podName string) ([]string, error) { - pod, err := k8sClient.CoreV1().Pods(podNamespace).Get(context.Background(), podName, metav1.GetOptions{}) - if err != nil { - return nil, err - } - podIPs := make([]string, 0, len(pod.Status.PodIPs)) - for _, podIP := range pod.Status.PodIPs { - podIPs = append(podIPs, podIP.IP) - } - return podIPs, nil -} - func allowedClient(podName string) string { return "allowed-" + podName } @@ -629,27 +610,27 @@ func allowedTCPPortsForPolicy(allowPorts ...int) []mnpapi.MultiNetworkPolicyPort return portAllowlist } -func reachServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, serverPort int, args ...string) error { +func reachServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, serverPort int) error { updatedPod, err := cs.CoreV1().Pods(serverConfig.namespace).Get(context.Background(), serverConfig.name, metav1.GetOptions{}) if err != nil { return err } if updatedPod.Status.Phase == v1.PodRunning { - return connectToServer(clientConfig, serverIP, serverPort, args...) + return connectToServer(clientConfig, serverIP, serverPort) } return fmt.Errorf("pod not running. /me is sad") } -func pingServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, args ...string) error { +func pingServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string) error { updatedPod, err := cs.CoreV1().Pods(serverConfig.namespace).Get(context.Background(), serverConfig.name, metav1.GetOptions{}) if err != nil { return err } if updatedPod.Status.Phase == v1.PodRunning { - return pingServer(clientConfig, serverIP, args...) + return pingServer(clientConfig, serverIP) } return fmt.Errorf("pod not running. /me is sad")