Merge pull request ovn-org#4626 from martinkennelly/fix-v6-mgnt-port-ip

Fix IPv6 network mgnt port IP address assignment
oshoval · Aug 22, 2024 · f77b1be · f77b1be
2 parents 8a3af1f + 5738da3
commit f77b1be
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 38 deletions.
diff --git 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,54 +287,67 @@ 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())
 
 	// 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
@@ -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
@@ -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() {