diff --git a/pkg/controller/cloud/node_controller.go b/pkg/controller/cloud/node_controller.go index c10f69bb5d..a7db815559 100644 --- a/pkg/controller/cloud/node_controller.go +++ b/pkg/controller/cloud/node_controller.go @@ -191,7 +191,6 @@ func (cnc *CloudNodeController) updateNodeAddress(node *v1.Node, instances cloud glog.Errorf("Specified Node IP not found in cloudprovider") return } - nodeAddresses = []v1.NodeAddress{*nodeIP} } newNode := node.DeepCopy() newNode.Status.Addresses = nodeAddresses diff --git a/pkg/controller/cloud/node_controller_test.go b/pkg/controller/cloud/node_controller_test.go index 019b68d6a1..4c5101209e 100644 --- a/pkg/controller/cloud/node_controller_test.go +++ b/pkg/controller/cloud/node_controller_test.go @@ -854,7 +854,7 @@ func TestNodeProvidedIPAddresses(t *testing.T) { assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated") assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated") - assert.Equal(t, 1, len(fnh.UpdatedNodes[0].Status.Addresses), "Node status unexpectedly updated") + assert.Equal(t, 3, len(fnh.UpdatedNodes[0].Status.Addresses), "Node status unexpectedly updated") cloudNodeController.Run() @@ -862,7 +862,7 @@ func TestNodeProvidedIPAddresses(t *testing.T) { updatedNodes := fnh.GetUpdatedNodesCopy() - assert.Equal(t, 1, len(updatedNodes[0].Status.Addresses), 1, "Node Addresses not correctly updated") + assert.Equal(t, 3, len(updatedNodes[0].Status.Addresses), "Node Addresses not correctly updated") assert.Equal(t, "10.0.0.1", updatedNodes[0].Status.Addresses[0].Address, "Node Addresses not correctly updated") } diff --git a/pkg/kubelet/apis/well_known_annotations.go b/pkg/kubelet/apis/well_known_annotations.go index bc7e899efd..f1bf0620e5 100644 --- a/pkg/kubelet/apis/well_known_annotations.go +++ b/pkg/kubelet/apis/well_known_annotations.go @@ -19,7 +19,7 @@ package apis const ( // When kubelet is started with the "external" cloud provider, then // it sets this annotation on the node to denote an ip address set from the - // cmd line flag. This ip is verified with the cloudprovider as valid by + // cmd line flag (--node-ip). This ip is verified with the cloudprovider as valid by // the cloud-controller-manager AnnotationProvidedIPAddr = "alpha.kubernetes.io/provided-node-ip" ) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e1efabbb53..f5dca1f58b 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -521,6 +521,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, containerManager: kubeDeps.ContainerManager, containerRuntimeName: containerRuntime, nodeIP: parsedNodeIP, + nodeIPValidator: validateNodeIP, clock: clock.RealClock{}, enableControllerAttachDetach: kubeCfg.EnableControllerAttachDetach, iptClient: utilipt.New(utilexec.New(), utildbus.New(), utilipt.ProtocolIpv4), @@ -1070,6 +1071,9 @@ type Kubelet struct { // If non-nil, use this IP address for the node nodeIP net.IP + // use this function to validate the kubelet nodeIP + nodeIPValidator func(net.IP) error + // If non-nil, this is a unique identifier for the node in an external database, eg. cloudprovider providerID string diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index 3d842b2f72..dfeaa1a72e 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/pkg/kubelet/kubelet_node_status.go @@ -407,7 +407,7 @@ func (kl *Kubelet) recordNodeStatusEvent(eventType, event string) { // Set IP and hostname addresses for the node. func (kl *Kubelet) setNodeAddress(node *v1.Node) error { if kl.nodeIP != nil { - if err := validateNodeIP(kl.nodeIP); err != nil { + if err := kl.nodeIPValidator(kl.nodeIP); err != nil { return fmt.Errorf("failed to validate nodeIP: %v", err) } glog.V(2).Infof("Using node IP: %q", kl.nodeIP.String()) @@ -467,12 +467,22 @@ func (kl *Kubelet) setNodeAddress(node *v1.Node) error { } if kl.nodeIP != nil { enforcedNodeAddresses := []v1.NodeAddress{} + + var nodeIPType v1.NodeAddressType for _, nodeAddress := range nodeAddresses { if nodeAddress.Address == kl.nodeIP.String() { enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address}) + nodeIPType = nodeAddress.Type + break } } if len(enforcedNodeAddresses) > 0 { + for _, nodeAddress := range nodeAddresses { + if nodeAddress.Type != nodeIPType && nodeAddress.Type != v1.NodeHostName { + enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: nodeAddress.Type, Address: nodeAddress.Address}) + } + } + enforcedNodeAddresses = append(enforcedNodeAddresses, v1.NodeAddress{Type: v1.NodeHostName, Address: kl.GetHostname()}) node.Status.Addresses = enforcedNodeAddresses return nil @@ -513,7 +523,7 @@ func (kl *Kubelet) setNodeAddress(node *v1.Node) error { var addrs []net.IP addrs, _ = net.LookupIP(node.Name) for _, addr := range addrs { - if err = validateNodeIP(addr); err == nil { + if err = kl.nodeIPValidator(addr); err == nil { if addr.To4() != nil { ipAddr = addr break diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index 20ab8c8c74..048508b22f 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -139,80 +139,163 @@ func TestNodeStatusWithCloudProviderNodeIP(t *testing.T) { kubelet.kubeClient = nil // ensure only the heartbeat client is used kubelet.hostname = testKubeletHostname - existingNode := v1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: testKubeletHostname, Annotations: make(map[string]string)}, - Spec: v1.NodeSpec{}, + cases := []struct { + name string + nodeIP net.IP + nodeAddresses []v1.NodeAddress + expectedAddresses []v1.NodeAddress + shouldError bool + }{ + { + name: "A single InternalIP", + nodeIP: net.ParseIP("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: net.ParseIP("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.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, + }, + shouldError: false, + }, + { + // Accommodating #45201 and #49202 + name: "InternalIP and ExternalIP are the same", + nodeIP: net.ParseIP("55.55.55.55"), + nodeAddresses: []v1.NodeAddress{ + {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: net.ParseIP("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: net.ParseIP("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: net.ParseIP("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, + }, } - - // TODO : is it possible to mock 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 + for _, testCase := range cases { + // testCase setup + existingNode := v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: testKubeletHostname, Annotations: make(map[string]string)}, + Spec: v1.NodeSpec{}, } - if ip != nil && !ip.IsLoopback() && ip.To4() != nil { - kubelet.nodeIP = ip - break + + kubelet.nodeIP = testCase.nodeIP + + fakeCloud := &fakecloud.FakeCloud{ + Addresses: testCase.nodeAddresses, + Err: nil, + } + kubelet.cloud = fakeCloud + kubelet.cloudproviderRequestParallelism = make(chan int, 1) + kubelet.cloudproviderRequestSync = make(chan int) + kubelet.cloudproviderRequestTimeout = 10 * time.Second + kubelet.nodeIPValidator = func(nodeIP net.IP) error { + return nil } - } - 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.cloudproviderRequestParallelism = make(chan int, 1) - kubelet.cloudproviderRequestSync = make(chan int) - kubelet.cloudproviderRequestTimeout = 10 * time.Second + // execute method + err := kubelet.setNodeAddress(&existingNode) + if err != nil && !testCase.shouldError { + t.Errorf("Unexpected error for test %s: %q", testCase.name, err) + continue + } else if err != nil && testCase.shouldError { + // expected an error + continue + } - kubelet.setNodeAddress(&existingNode) + // Sort both sets for consistent equality + sortNodeAddresses(testCase.expectedAddresses) + sortNodeAddresses(existingNode.Status.Addresses) - 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( + testCase.expectedAddresses, + existingNode.Status.Addresses, + ), + fmt.Sprintf("Test %s failed %%s", testCase.name), + diff.ObjectDiff(testCase.expectedAddresses, existingNode.Status.Addresses), + ) } - assert.True(t, apiequality.Semantic.DeepEqual(expectedAddresses, existingNode.Status.Addresses), "%s", diff.ObjectDiff(expectedAddresses, existingNode.Status.Addresses)) +} + +// sortableNodeAddress is a type for sorting []v1.NodeAddress +type sortableNodeAddress []v1.NodeAddress + +func (s sortableNodeAddress) Len() int { return len(s) } +func (s sortableNodeAddress) Less(i, j int) bool { + return (string(s[i].Type) + s[i].Address) < (string(s[j].Type) + s[j].Address) +} +func (s sortableNodeAddress) Swap(i, j int) { s[j], s[i] = s[i], s[j] } + +func sortNodeAddresses(addrs sortableNodeAddress) { + sort.Sort(addrs) } func TestUpdateNewNodeStatus(t *testing.T) {