Merge pull request #18281 from pravisankar/fix-master-nodeevent

Automatic merge from submit-queue.

Bug 1538389 - Allow node IP change to update Host IP in HostSubnet resource

- Node status addresses may have both old and new IP address and by
validating against these addrs we may not be able to update HostSubnet
with new node IP. Openshift node service waits for its HostSubnet resource
with new Host IP and eventually fails.

- This change fixes the above issue by reverting the change for node flip/flop.
Recommended/correct way to handle node flip/flop case is to specify nodeIP config option in node-config.yaml

Author: openshift-merge-robot

pkg/network/master/subnets.go

@@ -60,10 +60,10 @@ func (master *OsdnMaster) SubnetStartMaster(clusterNetworks []common.ClusterNetw
 	return nil
 }
 
-// addNode takes the nodeName, a preferred nodeIP, the node's annotations and other valid ip addresses
+// addNode takes the nodeName, a preferred nodeIP and the node's annotations
 // Creates or updates a HostSubnet if needed
 // Returns the IP address used for hostsubnet (either the preferred or one from the otherValidAddresses) and any error
-func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations map[string]string, otherValidAddresses []kapi.NodeAddress) (string, error) {
+func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations map[string]string) (string, error) {
 	// Validate node IP before proceeding
 	if err := master.networkInfo.ValidateNodeIP(nodeIP); err != nil {
 		return "", err
@@ -74,8 +74,6 @@ func (master *OsdnMaster) addNode(nodeName string, nodeIP string, hsAnnotations
 	if err == nil {
 		if sub.HostIP == nodeIP {
 			return nodeIP, nil
-		} else if isValidNodeIP(otherValidAddresses, sub.HostIP) {
-			return sub.HostIP, nil
 		} else {
 			// Node IP changed, update old subnet
 			sub.HostIP = nodeIP
@@ -131,23 +129,6 @@ func (master *OsdnMaster) deleteNode(nodeName string) error {
 	return nil
 }
 
-func isValidNodeIP(validAddresses []kapi.NodeAddress, nodeIP string) bool {
-	for _, addr := range validAddresses {
-		if addr.Address == nodeIP {
-			return true
-		}
-	}
-	return false
-}
-
-func getNodeIP(node *kapi.Node) (string, error) {
-	if len(node.Status.Addresses) > 0 && node.Status.Addresses[0].Address != "" {
-		return node.Status.Addresses[0].Address, nil
-	} else {
-		return netutils.GetNodeIP(node.Name)
-	}
-}
-
 // Because openshift-sdn uses an overlay and doesn't need GCE Routes, we need to
 // clear the NetworkUnavailable condition that kubelet adds to initial node
 // status when using GCE.
@@ -210,20 +191,27 @@ func (master *OsdnMaster) watchNodes() {
 
 func (master *OsdnMaster) handleAddOrUpdateNode(obj, _ interface{}, eventType watch.EventType) {
 	node := obj.(*kapi.Node)
-	nodeIP, err := getNodeIP(node)
-	if err != nil {
-		glog.Errorf("Failed to get node IP for node %s, skipping %s event, node: %v", node.Name, eventType, node)
+
+	var nodeIP string
+	for _, addr := range node.Status.Addresses {
+		if addr.Type == kapi.NodeInternalIP {
+			nodeIP = addr.Address
+			break
+		}
+	}
+	if len(nodeIP) == 0 {
+		glog.Errorf("Node IP is not set for node %s, skipping %s event, node: %v", node.Name, eventType, node)
 		return
 	}
 	master.clearInitialNodeNetworkUnavailableCondition(node)
 
-	if oldNodeIP, ok := master.hostSubnetNodeIPs[node.UID]; ok && ((nodeIP == oldNodeIP) || isValidNodeIP(node.Status.Addresses, oldNodeIP)) {
+	if oldNodeIP, ok := master.hostSubnetNodeIPs[node.UID]; ok && (nodeIP == oldNodeIP) {
 		return
 	}
 	// Node status is frequently updated by kubelet, so log only if the above condition is not met
 	glog.V(5).Infof("Watch %s event for Node %q", eventType, node.Name)
 
-	usedNodeIP, err := master.addNode(node.Name, nodeIP, nil, node.Status.Addresses)
+	usedNodeIP, err := master.addNode(node.Name, nodeIP, nil)
 	if err != nil {
 		glog.Errorf("Error creating subnet for node %s, ip %s: %v", node.Name, nodeIP, err)
 		return
@@ -280,7 +268,7 @@ func (master *OsdnMaster) handleAddOrUpdateSubnet(obj, _ interface{}, eventType
 			glog.Errorf("VNID %s is an invalid value for annotation %s. Annotation will be ignored.", vnid, networkapi.FixedVNIDHostAnnotation)
 		}
 	}
-	_, err = master.addNode(hs.Name, hs.HostIP, hsAnnotations, nil)
+	_, err = master.addNode(hs.Name, hs.HostIP, hsAnnotations)
 	if err != nil {
 		glog.Errorf("Error creating subnet for node %s, ip %s: %v", hs.Name, hs.HostIP, err)
 		return