Merge pull request ovn-kubernetes#5052 from crnithya/encapip_fix

Do not update EncapIP if it is configured

Author: girishmg

go-controller/pkg/config/config.go

@@ -228,9 +228,11 @@ type DefaultConfig struct {
 	// EncapType value defines the encapsulation protocol to use to transmit packets between
 	// hypervisors. By default the value is 'geneve'
 	EncapType string `gcfg:"encap-type"`
-	// The IP address of the encapsulation endpoint. If not specified, the IP address the
-	// NodeName resolves to will be used
+	// Configured IP address of the encapsulation endpoint.
 	EncapIP string `gcfg:"encap-ip"`
+	// Effective encap IP. It may be different from EncapIP if EncapIP meant to be
+	// the node's primary IP which can be updated when node's primary IP changes.
+	EffectiveEncapIP string
 	// The UDP Port of the encapsulation endpoint. If not specified, the IP default port
 	// of 6081 will be used
 	EncapPort uint `gcfg:"encap-port"`

go-controller/pkg/node/default_node_network_controller.go

@@ -270,32 +270,76 @@ func setOVSFlowTargets(node *kapi.Node) error {
 	return nil
 }
 
+// validateEncapIP returns false if there is an error or if the given IP is not known local IP address.
+func validateEncapIP(encapIP string) (bool, error) {
+	links, err := netlink.LinkList()
+	if err != nil {
+		return false, fmt.Errorf("failed to get all the links on the node: %v", err)
+	}
+	for _, link := range links {
+		addrs, err := util.GetFilteredInterfaceAddrs(link, config.IPv4Mode, config.IPv6Mode)
+		if err != nil {
+			return false, err
+		}
+		for _, addr := range addrs {
+			if addr.IP.String() == encapIP {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 func setupOVNNode(node *kapi.Node) error {
 	var err error
 
+	nodePrimaryIP, err := util.GetNodePrimaryIP(node)
+	if err != nil {
+		return fmt.Errorf("failed to obtain local primary IP from node %q: %v", node.Name, err)
+	}
+
 	encapIP := config.Default.EncapIP
 	if encapIP == "" {
-		encapIP, err = util.GetNodePrimaryIP(node)
-		if err != nil {
-			return fmt.Errorf("failed to obtain local IP from node %q: %v", node.Name, err)
-		}
-		config.Default.EncapIP = encapIP
+		config.Default.EffectiveEncapIP = nodePrimaryIP
 	} else {
 		// OVN allows `external_ids:ovn-encap-ip` to be a list of IPs separated by comma.
+		config.Default.EffectiveEncapIP = encapIP
 		ovnEncapIps := strings.Split(encapIP, ",")
 		for _, ovnEncapIp := range ovnEncapIps {
 			if ip := net.ParseIP(strings.TrimSpace(ovnEncapIp)); ip == nil {
 				return fmt.Errorf("invalid IP address %q in provided encap-ip setting %q", ovnEncapIp, encapIP)
 			}
 		}
+		// if there are more than one encap IPs, it must be configured explicitly. otherwise:
+		if len(ovnEncapIps) == 1 {
+			encapIP = ovnEncapIps[0]
+			if encapIP == nodePrimaryIP {
+				// the current encap IP is node primary IP, unset config.Default.EncapIP to indicate it is
+				// implicitly configured through the old external_ids:ovn-encap-ip value and needs to be updated
+				// if node primary IP changes.
+				config.Default.EncapIP = ""
+			} else {
+				// the encap IP may be incorrectly set or;
+				// previous implicitly set with the old primary node IP through the old external_ids:ovn-encap-ip value,
+				// that has changed when ovnkube-node is down.
+				// validate it to see if it is still a valid local IP address.
+				valid, err := validateEncapIP(encapIP)
+				if err != nil {
+					return fmt.Errorf("invalid encap IP %s: %v", encapIP, err)
+				}
+				if !valid {
+					return fmt.Errorf("invalid encap IP %s: does not exist", encapIP)
+				}
+			}
+		}
 	}
 
 	setExternalIdsCmd := []string{
 		"set",
 		"Open_vSwitch",
 		".",
 		fmt.Sprintf("external_ids:ovn-encap-type=%s", config.Default.EncapType),
-		fmt.Sprintf("external_ids:ovn-encap-ip=%s", encapIP),
+		fmt.Sprintf("external_ids:ovn-encap-ip=%s", config.Default.EffectiveEncapIP),
 		fmt.Sprintf("external_ids:ovn-remote-probe-interval=%d",
 			config.Default.InactivityProbe),
 		fmt.Sprintf("external_ids:ovn-openflow-probe-interval=%d",
@@ -1285,11 +1329,11 @@ func (nc *DefaultNodeNetworkController) WatchNamespaces() error {
 // enough, it will return an error
 func (nc *DefaultNodeNetworkController) validateVTEPInterfaceMTU() error {
 	// OVN allows `external_ids:ovn-encap-ip` to be a list of IPs separated by comma
-	ovnEncapIps := strings.Split(config.Default.EncapIP, ",")
+	ovnEncapIps := strings.Split(config.Default.EffectiveEncapIP, ",")
 	for _, ip := range ovnEncapIps {
 		ovnEncapIP := net.ParseIP(strings.TrimSpace(ip))
 		if ovnEncapIP == nil {
-			return fmt.Errorf("invalid IP address %q in provided encap-ip setting %q", ovnEncapIP, config.Default.EncapIP)
+			return fmt.Errorf("invalid IP address %q in provided encap-ip setting %q", ovnEncapIP, config.Default.EffectiveEncapIP)
 		}
 		interfaceName, mtu, err := util.GetIFNameAndMTUForAddress(ovnEncapIP)
 		if err != nil {

go-controller/pkg/node/default_node_network_controller_test.go

@@ -74,7 +74,7 @@ var _ = Describe("Node", func() {
 			}
 
 			config.Default.MTU = configDefaultMTU
-			config.Default.EncapIP = "10.1.0.40"
+			config.Default.EffectiveEncapIP = "10.1.0.40"
 
 		})
 
@@ -219,7 +219,7 @@ var _ = Describe("Node", func() {
 			BeforeEach(func() {
 				config.IPv4Mode = true
 				config.IPv6Mode = false
-				config.Default.EncapIP = "10.1.0.40,10.2.0.50"
+				config.Default.EffectiveEncapIP = "10.1.0.40,10.2.0.50"
 				netlinkOpsMock.On("LinkByIndex", 5).Return(netlinkLinkMock, nil)
 			})
 

go-controller/pkg/node/node_ip_handler_linux.go

@@ -255,7 +255,7 @@ func (c *addressManager) handleNodePrimaryAddrChange() {
 		klog.Errorf("Address Manager failed to check node primary address change: %v", err)
 		return
 	}
-	if nodePrimaryAddrChanged {
+	if nodePrimaryAddrChanged && config.Default.EncapIP == "" {
 		klog.Infof("Node primary address changed to %v. Updating OVN encap IP.", c.nodePrimaryAddr)
 		updateOVNEncapIPAndReconnect(c.nodePrimaryAddr)
 	}
@@ -519,6 +519,7 @@ func updateOVNEncapIPAndReconnect(newIP net.IP) {
 		}
 	}
 
+	config.Default.EffectiveEncapIP = newIP.String()
 	confCmd := []string{
 		"set",
 		"Open_vSwitch",