Merge pull request #5747 from mainred/comma-separated-provided-ips

fix: handle comma-separated provided IPs

Author: k8s-ci-robot

pkg/nodemanager/nodemanager.go

@@ -302,10 +302,8 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.
 	}
 	// If nodeIP was suggested by user, ensure that
 	// it can be found in the cloud as well (consistent with the behaviour in kubelet)
-	if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok {
-		if nodeIP == nil {
-			return fmt.Errorf("specified Node IP %s not found in cloudprovider for node %q", nodeAddresses, node.Name)
-		}
+	if existNodeIPs, ok := ensureNodeProvidedIPsExists(node, nodeAddresses); !ok {
+		return fmt.Errorf("not all specified Node IP %s found in cloudprovider for node %q, existing Node IPs are %s ", nodeAddresses, node.Name, existNodeIPs)
 	}
 	if !nodeAddressesChangeDetected(node.Status.Addresses, nodeAddresses) {
 		return nil
@@ -468,10 +466,8 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co
 
 	// If user provided an IP address, ensure that IP address is found
 	// in the cloud provider before removing the taint on the node
-	if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok {
-		if nodeIP == nil {
-			return nil, errors.New("failed to find kubelet node IP from cloud provider")
-		}
+	if _, ok := ensureNodeProvidedIPsExists(node, nodeAddresses); !ok {
+		return nil, errors.New("failed to find kubelet node IP from cloud provider")
 	}
 
 	if instanceType, err := cnc.getInstanceTypeByName(ctx, node); err != nil {
@@ -602,19 +598,30 @@ func nodeAddressesChangeDetected(addressSet1, addressSet2 []v1.NodeAddress) bool
 	return false
 }
 
-func ensureNodeProvidedIPExists(node *v1.Node, nodeAddresses []v1.NodeAddress) (*v1.NodeAddress, bool) {
-	var nodeIP *v1.NodeAddress
-	nodeIPExists := false
-	if providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]; ok {
-		nodeIPExists = true
-		for i := range nodeAddresses {
+// Ensure all provided node ip addresses are found in the cloud provider, otherwise return false
+// When there's no provided node ip addresses, it will return true.
+func ensureNodeProvidedIPsExists(node *v1.Node, nodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, bool) {
+	providedIPStr, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]
+	if !ok || len(providedIPStr) == 0 {
+		return []v1.NodeAddress{}, true
+	}
+	var existNodeIPs []v1.NodeAddress
+	nodeIPsExists := false
+	providedIPs := strings.Split(providedIPStr, ",")
+	for i := range nodeAddresses {
+		for _, providedIP := range providedIPs {
+			providedIP = strings.TrimSpace(providedIP)
 			if nodeAddresses[i].Address == providedIP {
-				nodeIP = &nodeAddresses[i]
+				existNodeIPs = append(existNodeIPs, nodeAddresses[i])
 				break
 			}
 		}
 	}
-	return nodeIP, nodeIPExists
+	if len(existNodeIPs) == len(providedIPs) {
+		nodeIPsExists = true
+	}
+
+	return existNodeIPs, nodeIPsExists
 }
 
 func (cnc *CloudNodeController) getInstanceTypeByName(ctx context.Context, node *v1.Node) (string, error) {

pkg/nodemanager/nodemanager_test.go

@@ -1338,3 +1338,173 @@ func TestNodeProviderIDNotSet(t *testing.T) {
 	// Node update should fail
 	assert.Equal(t, 0, len(fnh.UpdatedNodes), "Node was updated (unexpected)")
 }
+
+func Test_ensureNodeProvidedIPsExists(t *testing.T) {
+	testcases := []struct {
+		name                    string
+		node                    *v1.Node
+		nodeAddresses           []v1.NodeAddress
+		expectedExistingNodeIPs []v1.NodeAddress
+		nodeIPsExists           bool
+	}{
+		{
+			name: "return true when there's provide node ip address",
+			node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "node0",
+					Labels:      map[string]string{},
+					Annotations: map[string]string{},
+				},
+			},
+			nodeAddresses: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+			},
+			expectedExistingNodeIPs: []v1.NodeAddress{},
+			nodeIPsExists:           true,
+		},
+		{
+			name: "return true when all provided IPv4 IP address are found",
+			node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "node0",
+					Labels: map[string]string{},
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1",
+					},
+				},
+			},
+			nodeAddresses: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+			},
+			expectedExistingNodeIPs: []v1.NodeAddress{{Address: "10.0.0.1"}},
+			nodeIPsExists:           true,
+		},
+		{
+			name: "return true when all provided dual stack IP addresses are found",
+			node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "node0",
+					Labels: map[string]string{},
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1,fd47:c915:f8a8:e63d::5",
+					},
+				},
+			},
+			nodeAddresses: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+				{
+					Address: "fd47:c915:f8a8:e63d::5",
+				},
+			},
+			expectedExistingNodeIPs: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+				{
+					Address: "fd47:c915:f8a8:e63d::5",
+				},
+			},
+			nodeIPsExists: true,
+		},
+		{
+			name: "return true when all provided dual stack IP addresses are found but joined with extra space",
+			node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "node0",
+					Labels: map[string]string{},
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1, fd47:c915:f8a8:e63d::5",
+					},
+				},
+			},
+			nodeAddresses: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+				{
+					Address: "fd47:c915:f8a8:e63d::5",
+				},
+			},
+			expectedExistingNodeIPs: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+				{
+					Address: "fd47:c915:f8a8:e63d::5",
+				},
+			},
+			nodeIPsExists: true,
+		},
+		{
+			name: "return false when not all ip addresses are found for provided dual stack IP addresses",
+			node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "node0",
+					Labels: map[string]string{},
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1,fd47:c915:f8a8:e63d::5",
+					},
+				},
+			},
+			nodeAddresses: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+			},
+			expectedExistingNodeIPs: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+			},
+			nodeIPsExists: false,
+		},
+		{
+			name: "return false when wrong ip addresses are provided for provided dual stack IP addresses",
+			node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "node0",
+					Labels: map[string]string{},
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1,fd47:c915:f8a8:e63d::10",
+					},
+				},
+			},
+			nodeAddresses: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+				{
+					Address: "fd47:c915:f8a8:e63d::5",
+				},
+			},
+			expectedExistingNodeIPs: []v1.NodeAddress{
+				{
+					Address: "10.0.0.1",
+				},
+			},
+			nodeIPsExists: false,
+		},
+	}
+
+	for _, test := range testcases {
+		t.Run(test.name, func(t *testing.T) {
+
+			actualNodeIP, actualNodeIPsExists := ensureNodeProvidedIPsExists(test.node, test.nodeAddresses)
+
+			if !reflect.DeepEqual(actualNodeIP, test.expectedExistingNodeIPs) {
+				t.Logf("Actual existing node IPs: %v", actualNodeIP)
+				t.Logf("Expected existing  node IPs: %v", test.expectedExistingNodeIPs)
+				t.Errorf("Actual existing  node IP does not match expected existing  node IP")
+			}
+			if actualNodeIPsExists != test.nodeIPsExists {
+				t.Errorf("all node ip addresses exist result mismatch, got: %t, wanted: %t", actualNodeIPsExists, test.nodeIPsExists)
+			}
+		})
+	}
+}