From da3eb16f0fcb625dae96a09923f2dc6959806dd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cyril=20Bont=C3=A9?= Date: Thu, 13 Jul 2017 12:16:49 +0200 Subject: [PATCH] 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 #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 #48760 --- pkg/kubelet/BUILD | 1 + pkg/kubelet/kubelet_node_status.go | 12 ++-- pkg/kubelet/kubelet_node_status_test.go | 82 +++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index 848f44146c7b2..b6585dd7fb291 100644 --- 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 index a0da169f6b0fc..6046ff8b2b6ed 100644 --- 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 index 1f57051a932e9..099b67f7729e6 100644 --- 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)