Merge pull request #75229 from andrewsykim/kubelet-node-address-external

kubelet: lookup node address for external provider if none is set

pkg/kubelet/nodestatus/BUILD

@@ -61,6 +61,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/rand:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
+        "//staging/src/k8s.io/cloud-provider:go_default_library",
         "//staging/src/k8s.io/cloud-provider/fake:go_default_library",
         "//vendor/github.com/google/cadvisor/info/v1:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",

pkg/kubelet/nodestatus/setters.go

@@ -80,8 +80,14 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP
 				}
 				node.ObjectMeta.Annotations[kubeletapis.AnnotationProvidedIPAddr] = nodeIP.String()
 			}
-			// We rely on the external cloud provider to supply the addresses.
+
-			return nil
+			// If --cloud-provider=external and node address is already set,
+			// then we return early because provider set addresses should take precedence.
+			// Otherwise, we try to look up the node IP and let the cloud provider override it later
+			// This should alleviate a lot of the bootstrapping issues with out-of-tree providers
+			if len(node.Status.Addresses) > 0 {
+				return nil
+			}
 		}
 		if cloud != nil {
 			nodeAddresses, err := nodeAddressesFunc()

pkg/kubelet/nodestatus/setters_test.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	cloudprovider "k8s.io/cloud-provider"
 	fakecloud "k8s.io/cloud-provider/fake"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
@@ -55,12 +56,13 @@ const (
 // TODO(mtaufen): below is ported from the old kubelet_node_status_test.go code, potentially add more test coverage for NodeAddress setter in future
 func TestNodeAddress(t *testing.T) {
 	cases := []struct {
-		name              string
+		name                  string
-		hostnameOverride  bool
+		hostnameOverride      bool
-		nodeIP            net.IP
+		nodeIP                net.IP
-		nodeAddresses     []v1.NodeAddress
+		externalCloudProvider bool
-		expectedAddresses []v1.NodeAddress
+		nodeAddresses         []v1.NodeAddress
-		shouldError       bool
+		expectedAddresses     []v1.NodeAddress
+		shouldError           bool
 	}{
 		{
 			name:   "A single InternalIP",
@@ -192,6 +194,17 @@ func TestNodeAddress(t *testing.T) {
 			hostnameOverride: true,
 			shouldError:      false,
 		},
+		{
+			name:                  "cloud provider is external",
+			nodeIP:                net.ParseIP("10.0.0.1"),
+			nodeAddresses:         []v1.NodeAddress{},
+			externalCloudProvider: true,
+			expectedAddresses: []v1.NodeAddress{
+				{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
+				{Type: v1.NodeHostName, Address: testKubeletHostname},
+			},
+			shouldError: false,
+		},
 		{
 			name: "cloud doesn't report hostname, no override, detected hostname mismatch",
 			nodeAddresses: []v1.NodeAddress{
@@ -241,6 +254,9 @@ func TestNodeAddress(t *testing.T) {
 			existingNode := &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{Name: testKubeletHostname, Annotations: make(map[string]string)},
 				Spec:       v1.NodeSpec{},
+				Status: v1.NodeStatus{
+					Addresses: []v1.NodeAddress{},
+				},
 			}
 
 			nodeIP := testCase.nodeIP
@@ -248,21 +264,29 @@ func TestNodeAddress(t *testing.T) {
 				return nil
 			}
 			hostname := testKubeletHostname
-			externalCloudProvider := false
+
-			cloud := &fakecloud.Cloud{
-				Addresses: testCase.nodeAddresses,
-				Err:       nil,
-			}
 			nodeAddressesFunc := func() ([]v1.NodeAddress, error) {
 				return testCase.nodeAddresses, nil
 			}
 
+			// cloud provider is expected to be nil if external provider is set
+			var cloud cloudprovider.Interface
+			if testCase.externalCloudProvider {
+				cloud = nil
+			} else {
+				cloud = &fakecloud.Cloud{
+					Addresses: testCase.nodeAddresses,
+					Err:       nil,
+				}
+
+			}
+
 			// construct setter
 			setter := NodeAddress(nodeIP,
 				nodeIPValidator,
 				hostname,
 				testCase.hostnameOverride,
-				externalCloudProvider,
+				testCase.externalCloudProvider,
 				cloud,
 				nodeAddressesFunc)