UPSTREAM: 125337: add more testing for node.status.addresses

openshift · Jul 1, 2024 · d45cff7 · d45cff7
1 parent 80243a1
commit d45cff7
Show file tree

Hide file tree

Showing 3 changed files with 133 additions and 7 deletions.
diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go
@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apimachinery/pkg/util/sets"
 	cloudprovider "k8s.io/cloud-provider"
 	cloudproviderapi "k8s.io/cloud-provider/api"
@@ -778,7 +779,7 @@ func (kl *Kubelet) defaultNodeStatusFuncs() []func(context.Context, *v1.Node) er
 	}
 	var setters []func(ctx context.Context, n *v1.Node) error
 	setters = append(setters,
-		nodestatus.NodeAddress(kl.nodeIPs, kl.nodeIPValidator, kl.hostname, kl.hostnameOverridden, kl.externalCloudProvider, kl.cloud, nodeAddressesFunc),
+		nodestatus.NodeAddress(kl.nodeIPs, kl.nodeIPValidator, kl.hostname, kl.hostnameOverridden, kl.externalCloudProvider, kl.cloud, nodeAddressesFunc, utilnet.ResolveBindAddress),
 		nodestatus.MachineInfo(string(kl.nodeName), kl.maxPods, kl.podsPerCore, kl.GetCachedMachineInfo, kl.containerManager.GetCapacity,
 			kl.containerManager.GetDevicePluginResourceCapacity, kl.containerManager.GetNodeAllocatableReservation, kl.recordEvent, kl.supportLocalStorageCapacityIsolation()),
 		nodestatus.VersionInfo(kl.cadvisor.VersionInfo, kl.containerRuntime.Type, kl.containerRuntime.Version),

diff --git a/pkg/kubelet/nodestatus/setters.go b/pkg/kubelet/nodestatus/setters.go
@@ -31,7 +31,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/errors"
-	utilnet "k8s.io/apimachinery/pkg/util/net"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	cloudprovider "k8s.io/cloud-provider"
 	cloudproviderapi "k8s.io/cloud-provider/api"
@@ -66,6 +65,7 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs
 	externalCloudProvider bool, // typically Kubelet.externalCloudProvider
 	cloud cloudprovider.Interface, // typically Kubelet.cloud
 	nodeAddressesFunc func() ([]v1.NodeAddress, error), // typically Kubelet.cloudResourceSyncManager.NodeAddresses
+	resolveAddressFunc func(net.IP) (net.IP, error), // typically k8s.io/apimachinery/pkg/util/net.ResolveBindAddress
 ) Setter {
 	var nodeIP, secondaryNodeIP net.IP
 	if len(nodeIPs) > 0 {
@@ -225,7 +225,7 @@ func NodeAddress(nodeIPs []net.IP, // typically Kubelet.nodeIPs
 				}
 
 				if ipAddr == nil {
-					ipAddr, err = utilnet.ResolveBindAddress(nodeIP)
+					ipAddr, err = resolveAddressFunc(nodeIP)
 				}
 			}
 

diff --git a/pkg/kubelet/nodestatus/setters_test.go b/pkg/kubelet/nodestatus/setters_test.go
@@ -63,12 +63,14 @@ func TestNodeAddress(t *testing.T) {
 		cloudProviderExternal
 		cloudProviderNone
 	)
+
 	existingNodeAddress := v1.NodeAddress{Address: "10.1.1.2"}
 	cases := []struct {
 		name                           string
 		hostnameOverride               bool
 		nodeIP                         net.IP
 		secondaryNodeIP                net.IP
+		resolvedIP                     net.IP
 		cloudProviderType              cloudProviderType
 		nodeAddresses                  []v1.NodeAddress
 		expectedAddresses              []v1.NodeAddress
@@ -234,17 +236,120 @@ func TestNodeAddress(t *testing.T) {
 			shouldError: false,
 		},
 		{
-			name:              "cloud provider is external and nodeIP unspecified",
+			name:              "no cloud provider and nodeIP IPv4 unspecified",
+			nodeIP:            netutils.ParseIPSloppy("0.0.0.0"),
+			resolvedIP:        netutils.ParseIPSloppy("10.0.0.2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderNone,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.0.0.2"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:              "no cloud provider and nodeIP IPv6 unspecified",
+			nodeIP:            netutils.ParseIPSloppy("::"),
+			resolvedIP:        netutils.ParseIPSloppy("2001:db2::2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderNone,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "2001:db2::2"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:              "legacy cloud provider and nodeIP IPv4 unspecified",
+			nodeIP:            netutils.ParseIPSloppy("0.0.0.0"),
+			resolvedIP:        netutils.ParseIPSloppy("10.0.0.2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderLegacy,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: true,
+		},
+		{
+			name:              "legacy cloud provider and nodeIP IPv6 unspecified",
+			nodeIP:            netutils.ParseIPSloppy("::"),
+			resolvedIP:        netutils.ParseIPSloppy("2001:db2::2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderLegacy,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: true,
+		},
+		{
+			name:              "cloud provider is external and nodeIP IPv4 unspecified",
+			nodeIP:            netutils.ParseIPSloppy("0.0.0.0"),
+			resolvedIP:        netutils.ParseIPSloppy("10.0.0.2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderExternal,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.0.0.2"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:              "cloud provider is external and nodeIP IPv6 unspecified",
 			nodeIP:            netutils.ParseIPSloppy("::"),
+			resolvedIP:        netutils.ParseIPSloppy("2001:db2::2"),
 			nodeAddresses:     []v1.NodeAddress{},
 			cloudProviderType: cloudProviderExternal,
 			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "2001:db2::2"},
 				{Type: v1.NodeHostName, Address: testKubeletHostname},
 			},
 			shouldError: false,
 		},
 		{
-			name:              "cloud provider is external and no nodeIP",
+			name:              "no cloud provider and no nodeIP resolve IPv4",
+			resolvedIP:        netutils.ParseIPSloppy("10.0.0.2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderNone,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.0.0.2"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:              "no cloud provider and no nodeIP resolve IPv6",
+			resolvedIP:        netutils.ParseIPSloppy("2001:db2::2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderNone,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "2001:db2::2"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:              "legacy cloud provider and no nodeIP",
+			resolvedIP:        netutils.ParseIPSloppy("10.0.0.2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderLegacy,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: true,
+		},
+		{
+			name:              "cloud provider is external and no nodeIP resolve IPv4",
+			resolvedIP:        netutils.ParseIPSloppy("10.0.0.2"),
+			nodeAddresses:     []v1.NodeAddress{},
+			cloudProviderType: cloudProviderExternal,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
+		{
+			name:              "cloud provider is external and no nodeIP resolve IPv6",
+			resolvedIP:        netutils.ParseIPSloppy("2001:db2::2"),
 			nodeAddresses:     []v1.NodeAddress{},
 			cloudProviderType: cloudProviderExternal,
 			expectedAddresses: []v1.NodeAddress{
@@ -638,6 +743,20 @@ func TestNodeAddress(t *testing.T) {
 				return testCase.nodeAddresses, nil
 			}
 
+			net.DefaultResolver = &net.Resolver{
+				PreferGo: true,
+				Dial: func(ctx context.Context, network string, address string) (net.Conn, error) {
+					return nil, fmt.Errorf("error")
+				},
+			}
+			defer func() {
+				net.DefaultResolver = &net.Resolver{}
+			}()
+
+			resolveAddressFunc := func(net.IP) (net.IP, error) {
+				return testCase.resolvedIP, nil
+			}
+
 			// cloud provider is expected to be nil if external provider is set or there is no cloud provider
 			var cloud cloudprovider.Interface
 			if testCase.cloudProviderType == cloudProviderLegacy {
@@ -659,7 +778,9 @@ func TestNodeAddress(t *testing.T) {
 				testCase.hostnameOverride,
 				testCase.cloudProviderType == cloudProviderExternal,
 				cloud,
-				nodeAddressesFunc)
+				nodeAddressesFunc,
+				resolveAddressFunc,
+			)
 
 			// call setter on existing node
 			err := setter(ctx, existingNode)
@@ -739,6 +860,9 @@ func TestNodeAddress_NoCloudProvider(t *testing.T) {
 			nodeAddressesFunc := func() ([]v1.NodeAddress, error) {
 				return nil, fmt.Errorf("not reached")
 			}
+			resolvedAddressesFunc := func(net.IP) (net.IP, error) {
+				return nil, fmt.Errorf("not reached")
+			}
 
 			// construct setter
 			setter := NodeAddress(testCase.nodeIPs,
@@ -747,7 +871,8 @@ func TestNodeAddress_NoCloudProvider(t *testing.T) {
 				false, // hostnameOverridden
 				false, // externalCloudProvider
 				nil,   // cloud
-				nodeAddressesFunc)
+				nodeAddressesFunc,
+				resolvedAddressesFunc)
 
 			// call setter on existing node
 			err := setter(ctx, existingNode)