Allow a node switching between OVN node and hybrid overlay node

With this patch, when hybrid overlay is enabled, a node can
dynamically be switched from a OVN node to a hybrid overlay node or
versa vice.

When the node is switched to a ovn node, OVN-K will treat it as adding
a new OVN node, and try to remove the HO static routes and policies like
a HO node is removed.

When the node is switched to a HO node, OVN-K will treat it as deleting
a existing OVN node, and adding a HO node.

Signed-off-by: Peng Liu <pliu@redhat.com>

go-controller/hybrid-overlay/pkg/controller/node.go

@@ -60,7 +60,8 @@ func nodeChanged(old, new interface{}) bool {
 	newCidr, newNodeIP, newDrMAC, _ := getNodeDetails(newNode)
 
 	return !reflect.DeepEqual(oldCidr, newCidr) || !reflect.DeepEqual(oldNodeIP, newNodeIP) || !reflect.DeepEqual(oldDrMAC, newDrMAC) ||
-		!reflect.DeepEqual(newNode.Annotations[hotypes.HybridOverlayDRIP], oldNode.Annotations[hotypes.HybridOverlayDRIP])
+		!reflect.DeepEqual(newNode.Annotations[hotypes.HybridOverlayDRIP], oldNode.Annotations[hotypes.HybridOverlayDRIP]) ||
+		util.NoHostSubnet(oldNode) != util.NoHostSubnet(newNode)
 }
 
 // podChanged returns true if any relevant pod attributes changed

go-controller/hybrid-overlay/pkg/controller/node_linux.go

@@ -16,6 +16,7 @@ import (
 // This controller is running in ovnkube-node binary. It's responsible for local
 // node configuration and annotation.
 type NodeController struct {
+	kube kube.Interface
 	sync.RWMutex
 	nodeName  string
 	initState hotypes.HybridInitState

go-controller/hybrid-overlay/pkg/controller/ovn_node_linux.go

@@ -60,6 +60,7 @@ func newOVNNodeController(
 	localPodLister listers.PodLister,
 ) (nodeController, error) {
 	node := &NodeController{
+		kube:                kube,
 		nodeName:            nodeName,
 		initState:           new(uint32),
 		vxlanPort:           uint16(config.HybridOverlay.VXLANPort),
@@ -151,7 +152,8 @@ func nameToCookie(nodeName string) string {
 // nodes in the cluster
 func (n *NodeController) hybridOverlayNodeUpdate(node *kapi.Node) error {
 	if !util.NoHostSubnet(node) {
-		return nil
+		// remove possible hybrid overlay remaining
+		return n.DeleteNode(node)
 	}
 
 	cidr, nodeIP, drMAC, err := getNodeDetails(node)
@@ -230,7 +232,6 @@ func (n *NodeController) hybridOverlayNodeUpdate(node *kapi.Node) error {
 func (n *NodeController) AddNode(node *kapi.Node) error {
 	klog.Info("Add Node ", node.Name)
 	var err error
-
 	if node.Name == n.nodeName {
 		// Add local node and setup DR
 		// Retry hybrid overlay initialization if the master was
@@ -239,21 +240,12 @@ func (n *NodeController) AddNode(node *kapi.Node) error {
 		if err != nil {
 			return err
 		}
-	}
 
-	if atomic.LoadUint32(n.initState) >= hotypes.DistributedRouterInitialized {
-		if node.Name != n.nodeName {
-			// add remote node
-			klog.Infof("Add hybridOverlay remote Node %s", node.Name)
-			err = n.hybridOverlayNodeUpdate(node)
-			if err != nil {
-				return err
-			}
-		} else if atomic.LoadUint32(n.initState) < hotypes.PodsInitialized {
+		if atomic.LoadUint32(n.initState) < hotypes.PodsInitialized {
 			// add pods local to our node
 			pods, err := n.localPodLister.List(labels.Everything())
 			if err != nil {
-				return fmt.Errorf("cannot fully initialize node %s for hybrid overlay, cannot list pods: %v", n.nodeName, err)
+				return fmt.Errorf("cannot fully initialize node %s for hybrid overlay, cannot list pods: %w", n.nodeName, err)
 			}
 			for _, pod := range pods {
 				if pod.Spec.NodeName != n.nodeName {
@@ -266,6 +258,25 @@ func (n *NodeController) AddNode(node *kapi.Node) error {
 			}
 			atomic.StoreUint32(n.initState, hotypes.PodsInitialized)
 		}
+	} else {
+		// Make sure the local node has been initialized before adding a hybridOverlay remote node
+		if atomic.LoadUint32(n.initState) < hotypes.DistributedRouterInitialized {
+			localNode, err := n.kube.GetNode(n.nodeName)
+			if err != nil {
+				return fmt.Errorf("cannot get local node: %s: %w", n.nodeName, err)
+			}
+			klog.Info("Initialize local node before adding a hybridOverlay remote node")
+			err = n.EnsureHybridOverlayBridge(localNode)
+			if err != nil {
+				return err
+			}
+		}
+		// add remote node
+		klog.Infof("Add hybridOverlay remote Node %s", node.Name)
+		err = n.hybridOverlayNodeUpdate(node)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -278,7 +289,7 @@ func (n *NodeController) deleteFlowsByCookie(cookie string) {
 
 // DeleteNode handles node deletions
 func (n *NodeController) DeleteNode(node *kapi.Node) error {
-	if node.Name == n.nodeName || !util.NoHostSubnet(node) {
+	if node.Name == n.nodeName {
 		return nil
 	}
 
@@ -301,7 +312,7 @@ func (n *NodeController) DeleteNode(node *kapi.Node) error {
 
 		route := makeRoute(cidr, n.drIP, mgmtPortLink)
 		err = util.GetNetLinkOps().RouteDel(route)
-		if err != nil && !os.IsExist(err) {
+		if err != nil && !strings.Contains(err.Error(), "no such process") {
 			return fmt.Errorf("failed to delete route for subnet %s via gateway %s: %v",
 				route.Dst, route.Gw, err)
 		}

go-controller/hybrid-overlay/pkg/controller/ovn_node_linux_test.go

@@ -1002,4 +1002,116 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() {
 		}
 		appRun(app)
 	})
+	ovntest.OnSupportedPlatformsIt("node updates itself vxlan tunnel when a node is switched to hybrid overlay node", func() {
+		app.Action = func(ctx *cli.Context) error {
+			const (
+				node1Name   string = "node1"
+				node1Subnet string = "10.11.12.0/24"
+				node1DRMAC  string = "00:00:00:7f:af:03"
+				node1IP     string = "10.11.12.1"
+
+				pod1IP   string = "1.2.3.5"
+				pod1CIDR string = pod1IP + "/24"
+				pod1MAC  string = "aa:bb:cc:dd:ee:ff"
+
+				updatedDRMAC string = "77:66:55:44:33:22"
+			)
+
+			annotations := createNodeAnnotationsForSubnet(thisNodeSubnet)
+			annotations[hotypes.HybridOverlayDRMAC] = thisNodeDRMAC
+			annotations["k8s.ovn.org/node-gateway-router-lrp-ifaddr"] = "{\"ipv4\":\"100.64.0.3/16\"}"
+			annotations[hotypes.HybridOverlayDRIP] = thisNodeDRIP
+			node := createNode(thisNode, "linux", thisNodeIP, annotations)
+			fakeClient := fake.NewSimpleClientset(&v1.NodeList{
+				Items: []v1.Node{
+					*node,
+				},
+			})
+
+			// Node setup from initial node sync
+			addNodeSetupCmds(fexec, thisNode)
+			_, err := config.InitConfig(ctx, fexec, nil)
+			Expect(err).NotTo(HaveOccurred())
+
+			f := informers.NewSharedInformerFactory(fakeClient, informer.DefaultResyncInterval)
+
+			n, err := NewNode(
+				&kube.Kube{KClient: fakeClient},
+				thisNode,
+				f.Core().V1().Nodes().Informer(),
+				f.Core().V1().Pods().Informer(),
+				informer.NewTestEventHandler,
+				false,
+			)
+			Expect(err).NotTo(HaveOccurred())
+			linuxNode, okay := n.controller.(*NodeController)
+			Expect(okay).To(BeTrue())
+			// setting the flowCacheSyncPeriod to 1 hour effectively disabling for testing
+			linuxNode.flowCacheSyncPeriod = 1 * time.Hour
+
+			addEnsureHybridOverlayBridgeMocks(nlMock, thisNodeDRIP, "")
+			// initial flowSync
+			addSyncFlows(fexec)
+			// flowsync after EnsureHybridOverlayBridge()
+			addSyncFlows(fexec)
+
+			f.Start(stopChan)
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				n.Run(stopChan)
+			}()
+
+			Eventually(func() bool {
+				return atomic.LoadUint32(linuxNode.initState) == hotypes.PodsInitialized
+			}).Should(BeTrue())
+
+			Eventually(fexec.CalledMatchesExpected, 2).Should(BeTrue(), fexec.ErrorDesc)
+			initialFlowCache := map[string]*flowCacheEntry{
+				"0x0": generateInitialFlowCacheEntry(mgmtIfAddr.IP.String(), thisNodeDRIP, thisNodeDRMAC),
+			}
+			Eventually(func() error {
+				linuxNode.flowMutex.Lock()
+				defer linuxNode.flowMutex.Unlock()
+				return compareFlowCache(linuxNode.flowCache, initialFlowCache)
+			}, 2).Should(BeNil())
+
+			// setup hybrid overlay node
+			windowsAnnotation := createNodeAnnotationsForSubnet(node1Subnet)
+			windowsAnnotation[hotypes.HybridOverlayDRMAC] = node1DRMAC
+			_, err = fakeClient.CoreV1().Nodes().Create(context.TODO(), createNode(node1Name, "windows", node1IP, windowsAnnotation), metav1.CreateOptions{})
+			Expect(err).NotTo(HaveOccurred())
+			// flowsync after AddNode
+			addSyncFlows(fexec)
+			Eventually(fexec.CalledMatchesExpected, 2).Should(BeTrue(), fexec.ErrorDesc)
+
+			node1Cookie := nameToCookie(node1Name)
+			initialFlowCache[node1Cookie] = &flowCacheEntry{
+				flows: []string{
+					"cookie=0x" + node1Cookie + ",table=0,priority=100,arp,in_port=ext,arp_tpa=" + node1Subnet + ",actions=move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],mod_dl_src:" + node1DRMAC + ",load:0x2->NXM_OF_ARP_OP[],move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],load:0x" + strings.ReplaceAll(node1DRMAC, ":", "") + "->NXM_NX_ARP_SHA[],move:NXM_OF_ARP_TPA[]->NXM_NX_REG0[],move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],move:NXM_NX_REG0[]->NXM_OF_ARP_SPA[],IN_PORT",
+					"cookie=0x" + node1Cookie + ",table=0,priority=100,ip,nw_dst=" + node1Subnet + ",actions=load:4097->NXM_NX_TUN_ID[0..31],set_field:" + node1IP + "->tun_dst,set_field:" + node1DRMAC + "->eth_dst,output:ext-vxlan",
+					"cookie=0x" + node1Cookie + ",table=0,priority=101,ip,nw_dst=" + node1Subnet + ",nw_src=100.64.0.3,actions=load:4097->NXM_NX_TUN_ID[0..31],set_field:" + thisNodeDRIP + "->nw_src,set_field:" + node1IP + "->tun_dst,set_field:" + node1DRMAC + "->eth_dst,output:ext-vxlan",
+				},
+			}
+
+			Eventually(func() error {
+				linuxNode.flowMutex.Lock()
+				defer linuxNode.flowMutex.Unlock()
+				return compareFlowCache(linuxNode.flowCache, initialFlowCache)
+			}, 2).Should(BeNil())
+
+			// node is swiched to ovn node
+			_, err = fakeClient.CoreV1().Nodes().Update(context.TODO(), createNode(node1Name, "linux", node1IP, windowsAnnotation), metav1.UpdateOptions{})
+			Expect(err).NotTo(HaveOccurred())
+
+			Eventually(func() bool {
+				linuxNode.flowMutex.Lock()
+				defer linuxNode.flowMutex.Unlock()
+				_, ok := linuxNode.flowCache[node1Cookie]
+				return ok
+			}, 2).Should(BeFalse())
+			return nil
+		}
+		appRun(app)
+	})
 })

go-controller/pkg/clustermanager/node/node_allocator.go

@@ -91,7 +91,8 @@ func (na *NodeAllocator) Init() error {
 }
 
 func (na *NodeAllocator) hasHybridOverlayAllocation() bool {
-	return config.HybridOverlay.Enabled && !na.netInfo.IsSecondary()
+	// When config.HybridOverlay.ClusterSubnets is empty, assume the subnet allocation will be managed by an external component.
+	return config.HybridOverlay.Enabled && !na.netInfo.IsSecondary() && len(config.HybridOverlay.ClusterSubnets) > 0
 }
 
 func (na *NodeAllocator) recordSubnetCount() {

go-controller/pkg/clustermanager/zone_cluster_controller.go

@@ -158,6 +158,10 @@ func (zcc *zoneClusterController) Stop() {
 
 // handleAddUpdateNodeEvent handles the add or update node event
 func (zcc *zoneClusterController) handleAddUpdateNodeEvent(node *corev1.Node) error {
+	if config.HybridOverlay.Enabled && util.NoHostSubnet(node) {
+		// skip hybrid overlay nodes
+		return nil
+	}
 	allocatedNodeID, err := zcc.nodeIDAllocator.AllocateID(node.Name)
 	if err != nil {
 		return fmt.Errorf("failed to allocate an id to the node %s : err - %w", node.Name, err)
@@ -370,6 +374,10 @@ func (h *zoneClusterControllerEventHandler) AreResourcesEqual(obj1, obj2 interfa
 		if util.NodeTransitSwitchPortAddrAnnotationChanged(node1, node2) {
 			return false, nil
 		}
+		// Check if a node is switched between ho node to ovn node
+		if util.NoHostSubnet(node1) != util.NoHostSubnet(node2) {
+			return false, nil
+		}
 		return true, nil
 	}
 

go-controller/pkg/ovn/controller/services/node_tracker.go

@@ -133,13 +133,15 @@ func (nt *nodeTracker) Start(nodeInformer coreinformers.NodeInformer) (cache.Res
 			// - the name of the node (very rare) has changed
 			// - the `host-cidrs` annotation changed
 			// - node changes its zone
+			// - node becomes a hybrid overlay node from a ovn node or vice verse
 			// . No need to trigger update for any other field change.
 			if util.NodeSubnetAnnotationChanged(oldObj, newObj) ||
 				util.NodeL3GatewayAnnotationChanged(oldObj, newObj) ||
 				oldObj.Name != newObj.Name ||
 				util.NodeHostCIDRsAnnotationChanged(oldObj, newObj) ||
 				util.NodeZoneAnnotationChanged(oldObj, newObj) ||
-				util.NodeMigratedZoneAnnotationChanged(oldObj, newObj) {
+				util.NodeMigratedZoneAnnotationChanged(oldObj, newObj) ||
+				util.NoHostSubnet(oldObj) != util.NoHostSubnet(newObj) {
 				nt.updateNode(newObj)
 			}
 		},
@@ -223,9 +225,9 @@ func (nt *nodeTracker) removeNode(nodeName string) {
 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, types.DefaultNetworkName)
-	if err != nil || hsn == nil {
+	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): %v", node.Name, err)
+		klog.Infof("Node %s has invalid / no HostSubnet annotations (probably waiting on initialization), or it's a hybrid overlay node: %v", node.Name, err)
 		nt.removeNode(node.Name)
 		return
 	}

go-controller/pkg/ovn/default_network_controller.go

@@ -735,6 +735,16 @@ func (h *defaultNetworkControllerEventHandler) AddResource(obj interface{}, from
 		if !ok {
 			return fmt.Errorf("could not cast %T object to *kapi.Node", obj)
 		}
+		if config.HybridOverlay.Enabled {
+			if util.NoHostSubnet(node) {
+				return h.oc.addUpdateHoNodeEvent(node)
+			} else {
+				// clean possible remainings for a node that is used to be a HO node
+				if err := h.oc.deleteHoNodeEvent(node); err != nil {
+					return err
+				}
+			}
+		}
 		if h.oc.isLocalZoneNode(node) {
 			var nodeParams *nodeSyncs
 			if fromRetryLoop {
@@ -857,6 +867,19 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int
 		if !ok {
 			return fmt.Errorf("could not cast oldObj of type %T to *kapi.Node", oldObj)
 		}
+		var switchToOvnNode bool
+		if config.HybridOverlay.Enabled {
+			if util.NoHostSubnet(newNode) && !util.NoHostSubnet(oldNode) {
+				klog.Infof("Node %s has been updated to be a remote/unmanaged hybrid overlay node", newNode.Name)
+				return h.oc.addUpdateHoNodeEvent(newNode)
+			} else if !util.NoHostSubnet(newNode) && util.NoHostSubnet(oldNode) {
+				klog.Infof("Node %s has been updated to be an ovn-kubernetes managed node", newNode.Name)
+				if err := h.oc.deleteHoNodeEvent(newNode); err != nil {
+					return err
+				}
+				switchToOvnNode = true
+			}
+		}
 
 		// +--------------------+-------------------+-------------------------------------------------+
 		// |    oldNode         |      newNode      |       Action                                    |
@@ -916,7 +939,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
-			syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChanged || zoneClusterChanged || primaryAddrChanged(oldNode, newNode)
+			// Also check if the node is used to be a hybrid overlay node
+			syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChanged || zoneClusterChanged || primaryAddrChanged(oldNode, newNode) || switchToOvnNode
 			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)