Skip updating node status address if already set (#195)

If node status addresses field has already been set, we want to skip
updating it again. This is to work around sporadic API issues that don't
guarantee in the ordering between learned IPs based on source i.e DHCP
assigned IP priority before loadbalancer ARP-ed IP.

---------

Co-authored-by: Daniel Lipovetsky <daniel.lipovetsky@nutanix.com>

pkg/provider/manager.go

@@ -132,9 +132,12 @@ func (n *nutanixManager) getInstanceMetadata(ctx context.Context, node *v1.Node)
 	}
 
 	klog.V(1).Infof("fetching nodeAddresses for node %s", nodeName)
-	nodeAddresses, err := n.getNodeAddresses(ctx, vm)
-	if err != nil {
-		return nil, err
+	nodeAddresses := node.Status.Addresses
+	if !n.isNodeAddressesSet(node) {
+		nodeAddresses, err = n.getNodeAddresses(ctx, vm)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	topologyInfo, err := n.getTopologyInfo(ctx, nClient, vm)
@@ -297,6 +300,27 @@ func (n *nutanixManager) generateProviderID(ctx context.Context, vmUUID string)
 	return fmt.Sprintf("%s://%s", constants.ProviderName, strings.ToLower(vmUUID)), nil
 }
 
+func (n *nutanixManager) isNodeAddressesSet(node *v1.Node) bool {
+	if node == nil {
+		return false
+	}
+
+	var hasHostname, hasInternalIP bool
+	for _, address := range node.Status.Addresses {
+		if address.Type == v1.NodeHostName {
+			hasHostname = true
+		}
+
+		if address.Type == v1.NodeInternalIP {
+			hasInternalIP = true
+		}
+	}
+
+	// We always set at least two address types: one internal IP and one hostname.
+	// If either type is not found, then we have not set the node addresses.
+	return hasHostname && hasInternalIP
+}
+
 func (n *nutanixManager) getNodeAddresses(_ context.Context, vm *prismclientv3.VMIntentResponse) ([]v1.NodeAddress, error) {
 	if vm == nil {
 		return nil, fmt.Errorf("vm cannot be nil when getting node addresses")

pkg/provider/manager_unit_test.go

@@ -0,0 +1,83 @@
+package provider
+
+import (
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+)
+
+func TestIsNodeAddressesSet(t *testing.T) {
+	tests := []struct {
+		name string
+		node *v1.Node
+		want bool
+	}{
+		{
+			name: "found an internalIP and a hostname",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeHostName,
+							Address: "example.com",
+						},
+						{
+							Type:    v1.NodeInternalIP,
+							Address: "1.2.3.4",
+						},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "found only internalIPs",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeInternalIP,
+							Address: "1.2.3.4",
+						},
+						{
+							Type:    v1.NodeInternalIP,
+							Address: "5.6.7.8",
+						},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "found only a hostname",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeHostName,
+							Address: "example.com",
+						},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "found no addresses",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Addresses: []v1.NodeAddress{},
+				},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			n := &nutanixManager{}
+			if got := n.isNodeAddressesSet(tt.node); got != tt.want {
+				t.Errorf("nutanixManager.isNodeAddressesSet() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}