Fix setNodeAddress when a node IP and a cloud provider are set

When a node IP is set and a cloud provider returns the same address with several types, on the first address was accepted. With the changes made in PR kubernetes#45201, the vSphere cloud provider returned the ExternalIP first, which led to a node without any InternalIP. The behaviour is modified to return all the address types for the specified node IP. Issue kubernetes#48760
cbonte · Sep 9, 2017 · da3eb16 · da3eb16
1 parent 39ae9e5
commit da3eb16
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 5 deletions.
diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD
@@ -165,6 +165,7 @@ go_test(
         "//pkg/apis/componentconfig:go_default_library",
         "//pkg/capabilities:go_default_library",
         "//pkg/client/clientset_generated/clientset/fake:go_default_library",
+        "//pkg/cloudprovider/providers/fake:go_default_library",
         "//pkg/kubelet/cadvisor/testing:go_default_library",
         "//pkg/kubelet/cm:go_default_library",
         "//pkg/kubelet/config:go_default_library",

diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go
@@ -405,15 +405,17 @@ func (kl *Kubelet) setNodeAddress(node *v1.Node) error {
 			return fmt.Errorf("failed to get node address from cloud provider: %v", err)
 		}
 		if kl.nodeIP != nil {
+			enforcedNodeAddresses := []v1.NodeAddress{}
 			for _, nodeAddress := range nodeAddresses {
 				if nodeAddress.Address == kl.nodeIP.String() {
-					node.Status.Addresses = []v1.NodeAddress{
-						{Type: nodeAddress.Type, Address: nodeAddress.Address},
-						{Type: v1.NodeHostName, Address: kl.GetHostname()},
-					}
-					return nil
+					enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address})
 				}
 			}
+			if len(enforcedNodeAddresses) > 0 {
+				enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: v1.NodeHostName, Address: kl.GetHostname()})
+				node.Status.Addresses = enforcedNodeAddresses
+				return nil
+			}
 			return fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", kl.nodeIP)
 		}
 

diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go
@@ -19,6 +19,7 @@ package kubelet
 import (
 	"encoding/json"
 	"fmt"
+	"net"
 	"reflect"
 	goruntime "runtime"
 	"sort"
@@ -28,6 +29,7 @@ import (
 
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
+	"github.com/stretchr/testify/assert"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -41,6 +43,7 @@ import (
 	core "k8s.io/client-go/testing"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
+	fakecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/fake"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
@@ -119,6 +122,85 @@ func (lcm *localCM) GetNodeAllocatableReservation() v1.ResourceList {
 	return lcm.allocatable
 }
 
+func TestNodeStatusWithCloudProviderNodeIP(t *testing.T) {
+	testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
+	defer testKubelet.Cleanup()
+	kubelet := testKubelet.kubelet
+	kubelet.hostname = testKubeletHostname
+
+	existingNode := v1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: testKubeletHostname, Annotations: make(map[string]string)},
+		Spec:       v1.NodeSpec{},
+	}
+
+	// TODO : is it possible to mock kubelet.validateNodeIP() to avoid relying on the host interface addresses ?
+	addrs, err := net.InterfaceAddrs()
+	assert.NoError(t, err)
+	for _, addr := range addrs {
+		var ip net.IP
+		switch v := addr.(type) {
+		case *net.IPNet:
+			ip = v.IP
+		case *net.IPAddr:
+			ip = v.IP
+		}
+		if ip != nil && !ip.IsLoopback() && ip.To4() != nil {
+			kubelet.nodeIP = ip
+			break
+		}
+	}
+	assert.NotNil(t, kubelet.nodeIP)
+
+	fakeCloud := &fakecloud.FakeCloud{
+		Addresses: []v1.NodeAddress{
+			{
+				Type:    v1.NodeExternalIP,
+				Address: "132.143.154.163",
+			},
+			{
+				Type:    v1.NodeExternalIP,
+				Address: kubelet.nodeIP.String(),
+			},
+			{
+				Type:    v1.NodeInternalIP,
+				Address: "132.143.154.164",
+			},
+			{
+				Type:    v1.NodeInternalIP,
+				Address: kubelet.nodeIP.String(),
+			},
+			{
+				Type:    v1.NodeInternalIP,
+				Address: "132.143.154.165",
+			},
+			{
+				Type:    v1.NodeHostName,
+				Address: testKubeletHostname,
+			},
+		},
+		Err: nil,
+	}
+	kubelet.cloud = fakeCloud
+
+	kubelet.setNodeAddress(&existingNode)
+
+	expectedAddresses := []v1.NodeAddress{
+		{
+			Type:    v1.NodeExternalIP,
+			Address: kubelet.nodeIP.String(),
+		},
+		{
+			Type:    v1.NodeInternalIP,
+			Address: kubelet.nodeIP.String(),
+		},
+		{
+			Type:    v1.NodeHostName,
+			Address: testKubeletHostname,
+		},
+	}
+	assert.True(t, apiequality.Semantic.DeepEqual(expectedAddresses, existingNode.Status.Addresses), "%s", diff.ObjectDiff(expectedAddresses, existingNode.Status.Addresses))
+}
+
 func TestUpdateNewNodeStatus(t *testing.T) {
 	// generate one more than maxImagesInNodeStatus in inputImageList
 	inputImageList, expectedImageList := generateTestingImageList(maxImagesInNodeStatus + 1)