From 5738da3f6f0abec4275d2b58f97298110f15e72f Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Thu, 15 Aug 2024 11:55:20 +0100 Subject: [PATCH] Fix IPv6 network mgnt port IP address assignment For IPv6, if you enslave a link to a VRF device, then any non-Link Local addresses maybe removed. This commit adds the link addresses only after the link is enslaved. Signed-off-by: Martin Kennelly --- go-controller/pkg/node/gateway_udn.go | 76 ++++++++++--------- go-controller/pkg/node/gateway_udn_test.go | 6 +- .../secondary_node_network_controller_test.go | 2 +- 3 files changed, 46 insertions(+), 38 deletions(-) diff --git a/go-controller/pkg/node/gateway_udn.go b/go-controller/pkg/node/gateway_udn.go index 3dc65d2f927..ccdceaf98f8 100644 --- a/go-controller/pkg/node/gateway_udn.go +++ b/go-controller/pkg/node/gateway_udn.go @@ -198,7 +198,9 @@ func NewUserDefinedNetworkGateway(netInfo util.NetInfo, networkID int, node *v1. // AddNetwork will be responsible to create all plumbings // required by this UDN on the gateway side func (udng *UserDefinedNetworkGateway) AddNetwork() error { - mplink, err := udng.addUDNManagementPort() + // port is created first and its MAC address configured. The IP(s) on that link are added after enslaving to a VRF device (addUDNManagementPortIPs) + // because IPv6 addresses are removed by the kernel (if not link local) when enslaved to a VRF device. + mplink, macAddress, err := udng.addUDNManagementPort() if err != nil { return fmt.Errorf("could not create management port netdevice for network %s: %w", udng.GetNetworkName(), err) } @@ -212,6 +214,13 @@ func (udng *UserDefinedNetworkGateway) AddNetwork() error { if err != nil { return fmt.Errorf("could not add VRF %d for network %s, err: %v", vrfTableId, udng.GetNetworkName(), err) } + err = udng.addUDNManagementPortIPs(mplink) + if err != nil { + return fmt.Errorf("unable to add management port IP(s) for link %s, for network %s: %w", mplink.Attrs().Name, udng.GetNetworkName(), err) + } + if err := util.UpdateNodeManagementPortMACAddressesWithRetry(udng.node, udng.nodeLister, udng.kubeInterface, macAddress, udng.GetNetworkName()); err != nil { + return fmt.Errorf("unable to update mac address annotation for node %s, for network %s, err: %w", udng.node.Name, udng.GetNetworkName(), err) + } if udng.openflowManager != nil { udng.openflowManager.addNetwork(udng.NetInfo, udng.masqCTMark, udng.v4MasqIP, udng.v6MasqIP) @@ -266,26 +275,10 @@ func (udng *UserDefinedNetworkGateway) DelNetwork() error { // STEP2: It saves the MAC address generated on the 1st go as an option on the OVS interface // so that it persists on reboots // STEP3: sets up the management port link on the host -// STEP4: adds the management port IP .2 to the mplink -// STEP5: adds the mac address to the node management port annotation -func (udng *UserDefinedNetworkGateway) addUDNManagementPort() (netlink.Link, error) { +// Returns a netlink Link which is the UDN management port interface along with its MAC address +func (udng *UserDefinedNetworkGateway) addUDNManagementPort() (netlink.Link, net.HardwareAddr, error) { var err error interfaceName := util.GetNetworkScopedK8sMgmtHostIntfName(uint(udng.networkID)) - var networkLocalSubnets []*net.IPNet - if udng.TopologyType() == types.Layer3Topology { - networkLocalSubnets, err = util.ParseNodeHostSubnetAnnotation(udng.node, udng.GetNetworkName()) - if err != nil { - return nil, fmt.Errorf("waiting for node %s to start, no annotation found on node for network %s: %w", - udng.node.Name, udng.GetNetworkName(), err) - } - } else if udng.TopologyType() == types.Layer2Topology { - // NOTE: We don't support L2 networks without subnets as primary UDNs - globalFlatL2Networks := udng.Subnets() - for _, globalFlatL2Network := range globalFlatL2Networks { - networkLocalSubnets = append(networkLocalSubnets, globalFlatL2Network.CIDR) - } - } - // STEP1 stdout, stderr, err := util.RunOVSVsctl( "--", "--may-exist", "add-port", "br-int", interfaceName, @@ -294,7 +287,7 @@ func (udng *UserDefinedNetworkGateway) addUDNManagementPort() (netlink.Link, err "external-ids:iface-id="+udng.GetNetworkScopedK8sMgmtIntfName(udng.node.Name), ) if err != nil { - return nil, fmt.Errorf("failed to add port to br-int for network %s, stdout: %q, stderr: %q, error: %w", + return nil, nil, fmt.Errorf("failed to add port to br-int for network %s, stdout: %q, stderr: %q, error: %w", udng.GetNetworkName(), stdout, stderr, err) } klog.V(3).Infof("Added OVS management port interface %s for network %s", interfaceName, udng.GetNetworkName()) @@ -302,46 +295,59 @@ func (udng *UserDefinedNetworkGateway) addUDNManagementPort() (netlink.Link, err // STEP2 macAddress, err := util.GetOVSPortMACAddress(interfaceName) if err != nil { - return nil, fmt.Errorf("failed to get management port MAC address for network %s: %v", udng.GetNetworkName(), err) + return nil, nil, fmt.Errorf("failed to get management port MAC address for network %s: %v", udng.GetNetworkName(), err) } // persist the MAC address so that upon node reboot we get back the same mac address. _, stderr, err = util.RunOVSVsctl("set", "interface", interfaceName, fmt.Sprintf("mac=%s", strings.ReplaceAll(macAddress.String(), ":", "\\:"))) if err != nil { - return nil, fmt.Errorf("failed to persist MAC address %q for %q while plumbing network %s: stderr:%s (%v)", + return nil, nil, fmt.Errorf("failed to persist MAC address %q for %q while plumbing network %s: stderr:%s (%v)", macAddress.String(), interfaceName, udng.GetNetworkName(), stderr, err) } // STEP3 mplink, err := util.LinkSetUp(interfaceName) if err != nil { - return nil, fmt.Errorf("failed to set the link up for interface %s while plumbing network %s, err: %v", + return nil, nil, fmt.Errorf("failed to set the link up for interface %s while plumbing network %s, err: %v", interfaceName, udng.GetNetworkName(), err) } klog.V(3).Infof("Setup management port link %s for network %s succeeded", interfaceName, udng.GetNetworkName()) + return mplink, macAddress, nil +} - // STEP4 +func (udng *UserDefinedNetworkGateway) addUDNManagementPortIPs(mpLink netlink.Link) error { + var err error + var networkLocalSubnets []*net.IPNet + // fetch subnets which we will use to get management port IP(s) + if udng.TopologyType() == types.Layer3Topology { + networkLocalSubnets, err = util.ParseNodeHostSubnetAnnotation(udng.node, udng.GetNetworkName()) + if err != nil { + return fmt.Errorf("waiting for node %s to start, no annotation found on node for network %s: %w", + udng.node.Name, udng.GetNetworkName(), err) + } + } else if udng.TopologyType() == types.Layer2Topology { + // NOTE: We don't support L2 networks without subnets as primary UDNs + globalFlatL2Networks := udng.Subnets() + for _, globalFlatL2Network := range globalFlatL2Networks { + networkLocalSubnets = append(networkLocalSubnets, globalFlatL2Network.CIDR) + } + } + // extract management port IP from subnets and add it to link for _, subnet := range networkLocalSubnets { if config.IPv6Mode && utilnet.IsIPv6CIDR(subnet) || config.IPv4Mode && utilnet.IsIPv4CIDR(subnet) { ip := util.GetNodeManagementIfAddr(subnet) var err error var exists bool - if exists, err = util.LinkAddrExist(mplink, ip); err == nil && !exists { - err = util.LinkAddrAdd(mplink, ip, 0, 0, 0) + if exists, err = util.LinkAddrExist(mpLink, ip); err == nil && !exists { + err = util.LinkAddrAdd(mpLink, ip, 0, 0, 0) } if err != nil { - return nil, fmt.Errorf("failed to add management port IP from subnet %s to netdevice %s for network %s, err: %v", - subnet, interfaceName, udng.GetNetworkName(), err) + return fmt.Errorf("failed to add management port IP from subnet %s to netdevice %s for network %s, err: %v", + subnet, mpLink.Attrs().Name, udng.GetNetworkName(), err) } } } - - // STEP5 - if err := util.UpdateNodeManagementPortMACAddressesWithRetry(udng.node, udng.nodeLister, udng.kubeInterface, macAddress, udng.GetNetworkName()); err != nil { - return nil, fmt.Errorf("unable to update mac address annotation for node %s, for network %s, err: %v", udng.node.Name, udng.GetNetworkName(), err) - } - klog.V(3).Infof("Added management port mac address information of %s for network %s", interfaceName, udng.GetNetworkName()) - return mplink, nil + return nil } // deleteUDNManagementPort does the following: diff --git a/go-controller/pkg/node/gateway_udn_test.go b/go-controller/pkg/node/gateway_udn_test.go index 2bb4c69f72b..7b24edf0598 100644 --- a/go-controller/pkg/node/gateway_udn_test.go +++ b/go-controller/pkg/node/gateway_udn_test.go @@ -269,9 +269,10 @@ var _ = Describe("UserDefinedNetworkGateway", func() { kubeMock.On("UpdateNodeStatus", cnode).Return(nil) err = testNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - mpLink, err := udnGateway.addUDNManagementPort() + mpLink, _, err := udnGateway.addUDNManagementPort() Expect(err).NotTo(HaveOccurred()) Expect(mpLink).NotTo(BeNil()) + Expect(udnGateway.addUDNManagementPortIPs(mpLink)).Should(Succeed()) exists, err := util.LinkAddrExist(mpLink, ovntest.MustParseIPNet("100.128.0.2/24")) Expect(err).NotTo(HaveOccurred()) Expect(exists).To(BeTrue()) @@ -336,9 +337,10 @@ var _ = Describe("UserDefinedNetworkGateway", func() { kubeMock.On("UpdateNodeStatus", cnode).Return(nil) err = testNS.Do(func(ns.NetNS) error { defer GinkgoRecover() - mpLink, err := udnGateway.addUDNManagementPort() + mpLink, _, err := udnGateway.addUDNManagementPort() Expect(err).NotTo(HaveOccurred()) Expect(mpLink).NotTo(BeNil()) + Expect(udnGateway.addUDNManagementPortIPs(mpLink)).Should(Succeed()) exists, err := util.LinkAddrExist(mpLink, ovntest.MustParseIPNet("100.128.0.2/16")) Expect(err).NotTo(HaveOccurred()) Expect(exists).To(BeTrue()) diff --git a/go-controller/pkg/node/secondary_node_network_controller_test.go b/go-controller/pkg/node/secondary_node_network_controller_test.go index 774489dd4e3..e071f328b65 100644 --- a/go-controller/pkg/node/secondary_node_network_controller_test.go +++ b/go-controller/pkg/node/secondary_node_network_controller_test.go @@ -213,7 +213,7 @@ var _ = Describe("SecondaryNodeNetworkController", func() { Expect(err).NotTo(HaveOccurred()) err = controller.Start(context.Background()) Expect(err).To(HaveOccurred()) // we don't have the gateway pieces setup so its expected to fail here - Expect(err.Error()).To(ContainSubstring("no annotation found")) + Expect(err.Error()).To(ContainSubstring("could not create management port"), err.Error()) Expect(controller.gateway).To(Not(BeNil())) }) It("ensure UDNGateway is not invoked for Primary UDNs when feature gate is ON but network is not Primary", func() {