Merge pull request ovn-kubernetes#3833 from tssurya/fix-encap-port-de…

…adlock Fix encap port deadlock
kyrtapz · Aug 15, 2023 · bc0ec8a · bc0ec8a
2 parents 4bfacf9 + 90009c2
commit bc0ec8a
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 56 deletions.
diff --git a/go-controller/pkg/node/default_node_network_controller.go b/go-controller/pkg/node/default_node_network_controller.go
@@ -324,27 +324,6 @@ func setupOVNNode(node *kapi.Node) error {
 	if err != nil {
 		return fmt.Errorf("error setting OVS external IDs: %v\n  %q", err, stderr)
 	}
-	// If EncapPort is not the default tell sbdb to use specified port.
-	if config.Default.EncapPort != config.DefaultEncapPort {
-		systemID, err := util.GetNodeChassisID()
-		if err != nil {
-			return err
-		}
-		uuid, _, err := util.RunOVNSbctl("--data=bare", "--no-heading", "--columns=_uuid", "find", "Encap",
-			fmt.Sprintf("chassis_name=%s", systemID))
-		if err != nil {
-			return err
-		}
-		if len(uuid) == 0 {
-			return fmt.Errorf("unable to find encap uuid to set geneve port for chassis %s", systemID)
-		}
-		_, stderr, errSet := util.RunOVNSbctl("set", "encap", uuid,
-			fmt.Sprintf("options:dst_port=%d", config.Default.EncapPort),
-		)
-		if errSet != nil {
-			return fmt.Errorf("error setting OVS encap-port: %v\n  %q", errSet, stderr)
-		}
-	}
 
 	// clear stale ovs flow targets if needed
 	err = clearOVSFlowTargets()
@@ -360,6 +339,28 @@ func setupOVNNode(node *kapi.Node) error {
 	return nil
 }
 
+func setEncapPort() error {
+	systemID, err := util.GetNodeChassisID()
+	if err != nil {
+		return err
+	}
+	uuid, _, err := util.RunOVNSbctl("--data=bare", "--no-heading", "--columns=_uuid", "find", "Encap",
+		fmt.Sprintf("chassis_name=%s", systemID))
+	if err != nil {
+		return err
+	}
+	if len(uuid) == 0 {
+		return fmt.Errorf("unable to find encap uuid to set geneve port for chassis %s", systemID)
+	}
+	_, stderr, errSet := util.RunOVNSbctl("set", "encap", uuid,
+		fmt.Sprintf("options:dst_port=%d", config.Default.EncapPort),
+	)
+	if errSet != nil {
+		return fmt.Errorf("error setting OVS encap-port: %v\n  %q", errSet, stderr)
+	}
+	return nil
+}
+
 func isOVNControllerReady() (bool, error) {
 	// check node's connection status
 	runDir := util.GetOvnRunDir()
@@ -820,6 +821,16 @@ func (nc *DefaultNodeNetworkController) Start(ctx context.Context) error {
 		return fmt.Errorf("failed to set node %s annotations: %w", nc.name, err)
 	}
 
+	// If EncapPort is not the default tell sbdb to use specified port.
+	// We set the encap port after annotating the zone name so that ovnkube-controller has come up
+	// and configured the chassis in SBDB (ovnkube-controller waits for ovnkube-node to set annotation
+	// for at least one node in the given zone)
+	if config.Default.EncapPort != config.DefaultEncapPort {
+		if err := setEncapPort(); err != nil {
+			return err
+		}
+	}
+
 	/** HACK BEGIN **/
 	// TODO(tssurya): Remove this HACK a few months from now. This has been added only to
 	// minimize disruption for upgrades when moving to interconnect=true.

diff --git a/go-controller/pkg/node/default_node_network_controller_test.go b/go-controller/pkg/node/default_node_network_controller_test.go
@@ -292,35 +292,8 @@ var _ = Describe("Node", func() {
 					chassisUUID string = "1a3dfc82-2749-4931-9190-c30e7c0ecea3"
 					encapUUID   string = "e4437094-0094-4223-9f14-995d98d5fff8"
 				)
-				node := kapi.Node{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: nodeName,
-					},
-					Status: kapi.NodeStatus{
-						Addresses: []kapi.NodeAddress{
-							{
-								Type:    kapi.NodeExternalIP,
-								Address: nodeIP,
-							},
-						},
-					},
-				}
 
 				fexec := ovntest.NewFakeExec()
-				fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-					Cmd: fmt.Sprintf("ovs-vsctl --timeout=15 set Open_vSwitch . "+
-						"external_ids:ovn-encap-type=geneve "+
-						"external_ids:ovn-encap-ip=%s "+
-						"external_ids:ovn-remote-probe-interval=%d "+
-						"external_ids:ovn-openflow-probe-interval=%d "+
-						"other_config:bundle-idle-timeout=%d "+
-						"external_ids:hostname=\"%s\" "+
-						"external_ids:ovn-is-interconn=false "+
-						"external_ids:ovn-monitor-all=true "+
-						"external_ids:ovn-ofctrl-wait-before-clear=0 "+
-						"external_ids:ovn-enable-lflow-cache=true",
-						nodeIP, interval, ofintval, ofintval, nodeName),
-				})
 				fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 					Cmd: fmt.Sprintf("ovs-vsctl --timeout=15 " +
 						"--if-exists get Open_vSwitch . external_ids:system-id"),
@@ -335,13 +308,6 @@ var _ = Describe("Node", func() {
 					Cmd: fmt.Sprintf("ovn-sbctl --timeout=15 --no-leader-only set encap "+
 						"%s options:dst_port=%d", encapUUID, encapPort),
 				})
-				fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-					Cmd: "ovs-vsctl --timeout=15 -- clear bridge br-int netflow" +
-						" -- " +
-						"clear bridge br-int sflow" +
-						" -- " +
-						"clear bridge br-int ipfix",
-				})
 
 				err := util.SetExec(fexec)
 				Expect(err).NotTo(HaveOccurred())
@@ -350,7 +316,7 @@ var _ = Describe("Node", func() {
 				Expect(err).NotTo(HaveOccurred())
 				config.Default.EncapPort = encapPort
 
-				err = setupOVNNode(&node)
+				err = setEncapPort()
 				Expect(err).NotTo(HaveOccurred())
 
 				Expect(fexec.CalledMatchesExpected()).To(BeTrue(), fexec.ErrorDesc)