diff --git a/pkg/kubelet/nodestatus/setters.go b/pkg/kubelet/nodestatus/setters.go index 687e8212403..e6460f14d9f 100644 --- a/pkg/kubelet/nodestatus/setters.go +++ b/pkg/kubelet/nodestatus/setters.go @@ -131,7 +131,7 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs return err } - nodeAddresses, err := cloudprovidernodeutil.PreferNodeIP(nodeIP, cloudNodeAddresses) + nodeAddresses, err := cloudprovidernodeutil.GetNodeAddressesFromNodeIPLegacy(nodeIP, cloudNodeAddresses) if err != nil { return err } diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go index e77b45e89d6..19b02b99c1b 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go @@ -391,7 +391,7 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1. } if nodeIP != nil { - nodeAddresses, err = cloudnodeutil.PreferNodeIP(nodeIP, nodeAddresses) + nodeAddresses, err = cloudnodeutil.GetNodeAddressesFromNodeIP(nodeIP, nodeAddresses) if err != nil { klog.Errorf("Failed to update node addresses for node %q: %v", node.Name, err) return @@ -539,7 +539,7 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider( } if nodeIP != nil { - _, err := cloudnodeutil.PreferNodeIP(nodeIP, instanceMeta.NodeAddresses) + _, err := cloudnodeutil.GetNodeAddressesFromNodeIP(nodeIP, instanceMeta.NodeAddresses) if err != nil { return nil, fmt.Errorf("provided node ip for node %q is not valid: %w", node.Name, err) } diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/address.go b/staging/src/k8s.io/cloud-provider/node/helpers/address.go index 6eb44a90feb..533de163ac8 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/address.go +++ b/staging/src/k8s.io/cloud-provider/node/helpers/address.go @@ -41,8 +41,8 @@ func AddToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddr } } -// PreferNodeIP filters node addresses to prefer a specific node IP or address -// family. +// GetNodeAddressesFromNodeIPLegacy filters node addresses to prefer a specific node IP or +// address family. This function is used only with legacy cloud providers. // // 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 @@ -55,7 +55,7 @@ func AddToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddr // - 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) { +func GetNodeAddressesFromNodeIPLegacy(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 @@ -83,6 +83,29 @@ func PreferNodeIP(nodeIP net.IP, cloudNodeAddresses []v1.NodeAddress) ([]v1.Node return sortedAddresses, nil } + // Otherwise the result is the same as for GetNodeAddressesFromNodeIP + return GetNodeAddressesFromNodeIP(nodeIP, cloudNodeAddresses) +} + +// GetNodeAddressesFromNodeIP filters the provided list of nodeAddresses to +// match the provided nodeIP. This is used for external cloud providers. +// +// It 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. +// +// (This does not have the same behavior with `0.0.0.0` and `::` as +// GetNodeAddressesFromNodeIPLegacy, because that case never occurs for external cloud +// providers, because kubelet does not set the `provided-node-ip` annotation in that +// case.) +func GetNodeAddressesFromNodeIP(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 + } + // 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 diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go b/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go index eccb4a75b81..f5d2c5cad02 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go +++ b/staging/src/k8s.io/cloud-provider/node/helpers/address_test.go @@ -94,7 +94,7 @@ func TestAddToNodeAddresses(t *testing.T) { } } -func TestPreferNodeIP(t *testing.T) { +func TestGetNodeAddressesFromNodeIPLegacy(t *testing.T) { cases := []struct { name string nodeIP net.IP @@ -301,13 +301,172 @@ func TestPreferNodeIP(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { - got, err := PreferNodeIP(tt.nodeIP, tt.nodeAddresses) + got, err := GetNodeAddressesFromNodeIPLegacy(tt.nodeIP, tt.nodeAddresses) if (err != nil) != tt.shouldError { - t.Errorf("PreferNodeIP() error = %v, wantErr %v", err, tt.shouldError) + t.Errorf("GetNodeAddressesFromNodeIPLegacy() error = %v, wantErr %v", err, tt.shouldError) return } if !reflect.DeepEqual(got, tt.expectedAddresses) { - t.Errorf("PreferNodeIP() = %v, want %v", got, tt.expectedAddresses) + t.Errorf("GetNodeAddressesFromNodeIPLegacy() = %v, want %v", got, tt.expectedAddresses) + } + }) + } +} + +func TestGetNodeAddressesFromNodeIP(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, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + got, err := GetNodeAddressesFromNodeIP(tt.nodeIP, tt.nodeAddresses) + if (err != nil) != tt.shouldError { + t.Errorf("GetNodeAddressesFromNodeIP() error = %v, wantErr %v", err, tt.shouldError) + return + } + if !reflect.DeepEqual(got, tt.expectedAddresses) { + t.Errorf("GetNodeAddressesFromNodeIP() = %v, want %v", got, tt.expectedAddresses) } }) }