Merge pull request #107750 from shiftstack/issues/cloud-provider/56

Prefer user-provided node IP

pkg/kubelet/nodestatus/setters.go

@@ -34,6 +34,7 @@ import (
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	cloudprovider "k8s.io/cloud-provider"
 	cloudproviderapi "k8s.io/cloud-provider/api"
+	cloudprovidernodeutil "k8s.io/cloud-provider/node/helpers"
 	"k8s.io/component-base/version"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 	"k8s.io/kubernetes/pkg/features"
@@ -115,56 +116,9 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs
 				return err
 			}
 
-			var nodeAddresses []v1.NodeAddress
+			nodeAddresses, err := cloudprovidernodeutil.PreferNodeIP(nodeIP, cloudNodeAddresses)
-
+			if err != nil {
-			// For every address supplied by the cloud provider that matches nodeIP, nodeIP is the enforced node address for
+				return err
-			// that address Type (like InternalIP and ExternalIP), meaning other addresses of the same Type are discarded.
-			// See #61921 for more information: some cloud providers may supply secondary IPs, so nodeIP serves as a way to
-			// ensure that the correct IPs show up on a Node object.
-			if nodeIPSpecified {
-				enforcedNodeAddresses := []v1.NodeAddress{}
-
-				nodeIPTypes := make(map[v1.NodeAddressType]bool)
-				for _, nodeAddress := range cloudNodeAddresses {
-					if netutils.ParseIPSloppy(nodeAddress.Address).Equal(nodeIP) {
-						enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
-						nodeIPTypes[nodeAddress.Type] = true
-					}
-				}
-
-				// nodeIP must be among the addresses supplied by the cloud provider
-				if len(enforcedNodeAddresses) == 0 {
-					return fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", nodeIP)
-				}
-
-				// nodeIP was found, now use all other addresses supplied by the cloud provider NOT of the same Type as nodeIP.
-				for _, nodeAddress := range cloudNodeAddresses {
-					if !nodeIPTypes[nodeAddress.Type] {
-						enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
-					}
-				}
-
-				nodeAddresses = enforcedNodeAddresses
-			} else if nodeIP != nil {
-				// nodeIP is "0.0.0.0" or "::"; sort cloudNodeAddresses to
-				// prefer addresses of the matching family
-				sortedAddresses := make([]v1.NodeAddress, 0, len(cloudNodeAddresses))
-				for _, nodeAddress := range cloudNodeAddresses {
-					ip := netutils.ParseIPSloppy(nodeAddress.Address)
-					if ip == nil || isPreferredIPFamily(ip) {
-						sortedAddresses = append(sortedAddresses, nodeAddress)
-					}
-				}
-				for _, nodeAddress := range cloudNodeAddresses {
-					ip := netutils.ParseIPSloppy(nodeAddress.Address)
-					if ip != nil && !isPreferredIPFamily(ip) {
-						sortedAddresses = append(sortedAddresses, nodeAddress)
-					}
-				}
-				nodeAddresses = sortedAddresses
-			} else {
-				// If nodeIP is unset, just use the addresses provided by the cloud provider as-is
-				nodeAddresses = cloudNodeAddresses
 			}
 
 			switch {

staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"time"
 
 	v1 "k8s.io/api/core/v1"
@@ -42,6 +43,7 @@ import (
 	cloudnodeutil "k8s.io/cloud-provider/node/helpers"
 	nodeutil "k8s.io/component-helpers/node/util"
 	"k8s.io/klog/v2"
+	netutils "k8s.io/utils/net"
 )
 
 // labelReconcileInfo lists Node labels to reconcile, and how to reconcile them.
@@ -349,12 +351,20 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.
 			}
 		}
 	}
-	// If nodeIP was suggested by user, ensure that
+	// If kubelet provided a node IP, prefer it in the node address list
-	// it can be found in the cloud as well (consistent with the behaviour in kubelet)
+	nodeIP, err := getNodeProvidedIP(node)
-	if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok && nodeIP == nil {
+	if err != nil {
-		klog.Errorf("Specified Node IP not found in cloudprovider for node %q", node.Name)
+		klog.Errorf("Failed to get preferred node IP for node %q: %v", node.Name, err)
 		return
 	}
+
+	if nodeIP != nil {
+		nodeAddresses, err = cloudnodeutil.PreferNodeIP(nodeIP, nodeAddresses)
+		if err != nil {
+			klog.Errorf("Failed to update node addresses for node %q: %v", node.Name, err)
+			return
+		}
+	}
 	if !nodeAddressesChangeDetected(node.Status.Addresses, nodeAddresses) {
 		return
 	}
@@ -483,10 +493,19 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(
 		}
 	}
 
-	// If user provided an IP address, ensure that IP address is found
+	// If kubelet annotated the node with a node IP, ensure that it is valid
-	// in the cloud provider before removing the taint on the node
+	// and can be applied to the discovered node addresses before removing
-	if nodeIP, ok := ensureNodeProvidedIPExists(node, instanceMeta.NodeAddresses); ok && nodeIP == nil {
+	// the taint on the node.
-		return nil, errors.New("failed to find kubelet node IP from cloud provider")
+	nodeIP, err := getNodeProvidedIP(node)
+	if err != nil {
+		return nil, err
+	}
+
+	if nodeIP != nil {
+		_, err := cloudnodeutil.PreferNodeIP(nodeIP, instanceMeta.NodeAddresses)
+		if err != nil {
+			return nil, fmt.Errorf("provided node ip for node %q is not valid: %w", node.Name, err)
+		}
 	}
 
 	if instanceMeta.InstanceType != "" {
@@ -693,19 +712,18 @@ func nodeAddressesChangeDetected(addressSet1, addressSet2 []v1.NodeAddress) bool
 	return false
 }
 
-func ensureNodeProvidedIPExists(node *v1.Node, nodeAddresses []v1.NodeAddress) (*v1.NodeAddress, bool) {
+func getNodeProvidedIP(node *v1.Node) (net.IP, error) {
-	var nodeIP *v1.NodeAddress
+	providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]
-	nodeIPExists := false
+	if !ok {
-	if providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]; ok {
+		return nil, nil
-		nodeIPExists = true
-		for i := range nodeAddresses {
-			if nodeAddresses[i].Address == providedIP {
-				nodeIP = &nodeAddresses[i]
-				break
-			}
-		}
 	}
-	return nodeIP, nodeIPExists
+
+	nodeIP := netutils.ParseIPSloppy(providedIP)
+	if nodeIP == nil {
+		return nil, fmt.Errorf("failed to parse node IP %q for node %q", providedIP, node.Name)
+	}
+
+	return nodeIP, nil
 }
 
 // getInstanceTypeByProviderIDOrName will attempt to get the instance type of node using its providerID

staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go

@@ -572,6 +572,202 @@ func Test_syncNode(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "provided node IP address is not valid",
+			fakeCloud: &fakecloud.Cloud{
+				EnableInstancesV2: false,
+				Addresses: []v1.NodeAddress{
+					{
+						Type:    v1.NodeInternalIP,
+						Address: "10.0.0.1",
+					},
+					{
+						Type:    v1.NodeExternalIP,
+						Address: "132.143.154.163",
+					},
+				},
+				ExistsByProviderID: true,
+				Err:                nil,
+			},
+			existingNode: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "invalid-ip",
+					},
+				},
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						{
+							Key:    "ImproveCoverageTaint",
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+						{
+							Key:    cloudproviderapi.TaintExternalCloudProvider,
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+					},
+					ProviderID: "node0.aws.12345",
+				},
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{
+						{
+							Type:               v1.NodeReady,
+							Status:             v1.ConditionUnknown,
+							LastHeartbeatTime:  metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+							LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+						},
+					},
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeHostName,
+							Address: "node0.cloud.internal",
+						},
+					},
+				},
+			},
+			updatedNode: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "invalid-ip",
+					},
+				},
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						{
+							Key:    "ImproveCoverageTaint",
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+						{
+							Key:    cloudproviderapi.TaintExternalCloudProvider,
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+					},
+					ProviderID: "node0.aws.12345",
+				},
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{
+						{
+							Type:               v1.NodeReady,
+							Status:             v1.ConditionUnknown,
+							LastHeartbeatTime:  metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+							LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+						},
+					},
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeHostName,
+							Address: "node0.cloud.internal",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "provided node IP address is not present",
+			fakeCloud: &fakecloud.Cloud{
+				EnableInstancesV2: false,
+				Addresses: []v1.NodeAddress{
+					{
+						Type:    v1.NodeInternalIP,
+						Address: "10.0.0.1",
+					},
+					{
+						Type:    v1.NodeExternalIP,
+						Address: "132.143.154.163",
+					},
+				},
+				ExistsByProviderID: true,
+				Err:                nil,
+			},
+			existingNode: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.2",
+					},
+				},
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						{
+							Key:    "ImproveCoverageTaint",
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+						{
+							Key:    cloudproviderapi.TaintExternalCloudProvider,
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+					},
+					ProviderID: "node0.aws.12345",
+				},
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{
+						{
+							Type:               v1.NodeReady,
+							Status:             v1.ConditionUnknown,
+							LastHeartbeatTime:  metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+							LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+						},
+					},
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeHostName,
+							Address: "node0.cloud.internal",
+						},
+					},
+				},
+			},
+			updatedNode: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "node0",
+					CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
+					Annotations: map[string]string{
+						cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.2",
+					},
+				},
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						{
+							Key:    "ImproveCoverageTaint",
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+						{
+							Key:    cloudproviderapi.TaintExternalCloudProvider,
+							Value:  "true",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+					},
+					ProviderID: "node0.aws.12345",
+				},
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{
+						{
+							Type:               v1.NodeReady,
+							Status:             v1.ConditionUnknown,
+							LastHeartbeatTime:  metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+							LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
+						},
+					},
+					Addresses: []v1.NodeAddress{
+						{
+							Type:    v1.NodeHostName,
+							Address: "node0.cloud.internal",
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "provider ID already set",
 			fakeCloud: &fakecloud.Cloud{

staging/src/k8s.io/cloud-provider/node/helpers/address.go

@@ -17,7 +17,11 @@ limitations under the License.
 package helpers
 
 import (
+	"fmt"
+	"net"
+
 	"k8s.io/api/core/v1"
+	netutils "k8s.io/utils/net"
 )
 
 // AddToNodeAddresses appends the NodeAddresses to the passed-by-pointer slice,
@@ -36,3 +40,74 @@ func AddToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddr
 		}
 	}
 }
+
+// PreferNodeIP filters node addresses to prefer a specific node IP or address
+// family.
+//
+// If nodeIP is either '0.0.0.0' or '::' it is taken to represent any address of
+// that address family: IPv4 or IPv6. i.e. if nodeIP is '0.0.0.0' we will return
+// node addresses sorted such that all IPv4 addresses are listed before IPv6
+// addresses.
+//
+// If nodeIP is a specific IP, either IPv4 or IPv6, we will return node
+// addresses filtered such that:
+// * Any address matching nodeIP will be listed first.
+// * If nodeIP matches an address of a particular type (internal or external),
+//   that will be the *only* address of that type returned.
+// * All remaining addresses are listed after.
+func PreferNodeIP(nodeIP net.IP, cloudNodeAddresses []v1.NodeAddress) ([]v1.NodeAddress, error) {
+	// If nodeIP is unset, just use the addresses provided by the cloud provider as-is
+	if nodeIP == nil {
+		return cloudNodeAddresses, nil
+	}
+
+	// nodeIP is "0.0.0.0" or "::"; sort cloudNodeAddresses to
+	// prefer addresses of the matching family
+	if nodeIP.IsUnspecified() {
+		preferIPv4 := nodeIP.To4() != nil
+		isPreferredIPFamily := func(ip net.IP) bool { return (ip.To4() != nil) == preferIPv4 }
+
+		sortedAddresses := make([]v1.NodeAddress, 0, len(cloudNodeAddresses))
+		for _, nodeAddress := range cloudNodeAddresses {
+			ip := netutils.ParseIPSloppy(nodeAddress.Address)
+			if ip == nil || isPreferredIPFamily(ip) {
+				sortedAddresses = append(sortedAddresses, nodeAddress)
+			}
+		}
+		for _, nodeAddress := range cloudNodeAddresses {
+			ip := netutils.ParseIPSloppy(nodeAddress.Address)
+			if ip != nil && !isPreferredIPFamily(ip) {
+				sortedAddresses = append(sortedAddresses, nodeAddress)
+			}
+		}
+		return sortedAddresses, nil
+	}
+
+	// For every address supplied by the cloud provider that matches nodeIP, nodeIP is the enforced node address for
+	// that address Type (like InternalIP and ExternalIP), meaning other addresses of the same Type are discarded.
+	// See #61921 for more information: some cloud providers may supply secondary IPs, so nodeIP serves as a way to
+	// ensure that the correct IPs show up on a Node object.
+	enforcedNodeAddresses := []v1.NodeAddress{}
+
+	nodeIPTypes := make(map[v1.NodeAddressType]bool)
+	for _, nodeAddress := range cloudNodeAddresses {
+		if netutils.ParseIPSloppy(nodeAddress.Address).Equal(nodeIP) {
+			enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
+			nodeIPTypes[nodeAddress.Type] = true
+		}
+	}
+
+	// nodeIP must be among the addresses supplied by the cloud provider
+	if len(enforcedNodeAddresses) == 0 {
+		return nil, fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", nodeIP)
+	}
+
+	// nodeIP was found, now use all other addresses supplied by the cloud provider NOT of the same Type as nodeIP.
+	for _, nodeAddress := range cloudNodeAddresses {
+		if !nodeIPTypes[nodeAddress.Type] {
+			enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
+		}
+	}
+
+	return enforcedNodeAddresses, nil
+}

staging/src/k8s.io/cloud-provider/node/helpers/address_test.go

@@ -17,10 +17,17 @@ limitations under the License.
 package helpers
 
 import (
+	"net"
+	"reflect"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	netutils "k8s.io/utils/net"
+)
+
+const (
+	testKubeletHostname = "hostname"
 )
 
 func TestAddToNodeAddresses(t *testing.T) {
@@ -86,3 +93,222 @@ func TestAddToNodeAddresses(t *testing.T) {
 		})
 	}
 }
+
+func TestPreferNodeIP(t *testing.T) {
+	cases := []struct {
+		name              string
+		nodeIP            net.IP
+		nodeAddresses     []v1.NodeAddress
+		expectedAddresses []v1.NodeAddress
+		shouldError       bool
+	}{
+		{
+			name:   "A single InternalIP",
+			nodeIP: netutils.ParseIPSloppy("10.1.1.1"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "NodeIP is external",
+			nodeIP: netutils.ParseIPSloppy("55.55.55.55"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			// Accommodating #45201 and #49202
+			name:   "InternalIP and ExternalIP are the same",
+			nodeIP: netutils.ParseIPSloppy("55.55.55.55"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "44.44.44.44"},
+				{Type: v1.NodeExternalIP, Address: "44.44.44.44"},
+				{Type: v1.NodeInternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "An Internal/ExternalIP, an Internal/ExternalDNS",
+			nodeIP: netutils.ParseIPSloppy("10.1.1.1"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeInternalDNS, Address: "ip-10-1-1-1.us-west-2.compute.internal"},
+				{Type: v1.NodeExternalDNS, Address: "ec2-55-55-55-55.us-west-2.compute.amazonaws.com"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeInternalDNS, Address: "ip-10-1-1-1.us-west-2.compute.internal"},
+				{Type: v1.NodeExternalDNS, Address: "ec2-55-55-55-55.us-west-2.compute.amazonaws.com"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "An Internal with multiple internal IPs",
+			nodeIP: netutils.ParseIPSloppy("10.1.1.1"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeInternalIP, Address: "10.2.2.2"},
+				{Type: v1.NodeInternalIP, Address: "10.3.3.3"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "An InternalIP that isn't valid: should error",
+			nodeIP: netutils.ParseIPSloppy("10.2.2.2"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeExternalIP, Address: "55.55.55.55"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: nil,
+			shouldError:       true,
+		},
+		{
+			name:   "Dual-stack cloud, with nodeIP, different IPv6 formats",
+			nodeIP: netutils.ParseIPSloppy("2600:1f14:1d4:d101::ba3d"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeInternalIP, Address: "2600:1f14:1d4:d101:0:0:0:ba3d"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "2600:1f14:1d4:d101:0:0:0:ba3d"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name: "Dual-stack cloud, IPv4 first, no nodeIP",
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name: "Dual-stack cloud, IPv6 first, no nodeIP",
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "Dual-stack cloud, IPv4 first, request IPv4",
+			nodeIP: netutils.ParseIPSloppy("0.0.0.0"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "Dual-stack cloud, IPv6 first, request IPv4",
+			nodeIP: netutils.ParseIPSloppy("0.0.0.0"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "Dual-stack cloud, IPv4 first, request IPv6",
+			nodeIP: netutils.ParseIPSloppy("::"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+			},
+			shouldError: false,
+		},
+		{
+			name:   "Dual-stack cloud, IPv6 first, request IPv6",
+			nodeIP: netutils.ParseIPSloppy("::"),
+			nodeAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+				{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
+			},
+			shouldError: false,
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := PreferNodeIP(tt.nodeIP, tt.nodeAddresses)
+			if (err != nil) != tt.shouldError {
+				t.Errorf("PreferNodeIP() error = %v, wantErr %v", err, tt.shouldError)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.expectedAddresses) {
+				t.Errorf("PreferNodeIP() = %v, want %v", got, tt.expectedAddresses)
+			}
+		})
+	}
+}