From 2857de73ce387473463ac90f3205e7fdf9236bb8 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 22 Aug 2018 21:12:24 -0400 Subject: [PATCH] Honor --hostname-override, report compatible hostname addresses with cloud provider --- pkg/kubelet/kubelet.go | 7 ++- pkg/kubelet/kubelet_node_status.go | 2 +- pkg/kubelet/nodestatus/setters.go | 54 +++++++++++++++-- pkg/kubelet/nodestatus/setters_test.go | 81 ++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 6 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 3bf170bb3b4..af1e371fdfb 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -476,6 +476,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, klet := &Kubelet{ hostname: hostname, + hostnameOverridden: len(hostnameOverride) > 0, nodeName: nodeName, kubeClient: kubeDeps.KubeClient, heartbeatClient: kubeDeps.HeartbeatClient, @@ -855,7 +856,11 @@ type serviceLister interface { type Kubelet struct { kubeletConfiguration kubeletconfiginternal.KubeletConfiguration - hostname string + // hostname is the hostname the kubelet detected or was given via flag/config + hostname string + // hostnameOverridden indicates the hostname was overridden via flag/config + hostnameOverridden bool + nodeName types.NodeName runtimeCache kubecontainer.RuntimeCache kubeClient clientset.Interface diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index bb2117fe579..1d51230bc28 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/pkg/kubelet/kubelet_node_status.go @@ -485,7 +485,7 @@ func (kl *Kubelet) defaultNodeStatusFuncs() []func(*v1.Node) error { } var setters []func(n *v1.Node) error setters = append(setters, - nodestatus.NodeAddress(kl.nodeIP, kl.nodeIPValidator, kl.hostname, kl.externalCloudProvider, kl.cloud, nodeAddressesFunc), + nodestatus.NodeAddress(kl.nodeIP, kl.nodeIPValidator, kl.hostname, kl.hostnameOverridden, kl.externalCloudProvider, kl.cloud, nodeAddressesFunc), nodestatus.MachineInfo(string(kl.nodeName), kl.maxPods, kl.podsPerCore, kl.GetCachedMachineInfo, kl.containerManager.GetCapacity, kl.containerManager.GetDevicePluginResourceCapacity, kl.containerManager.GetNodeAllocatableReservation, kl.recordEvent), 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 index abe6970ba24..8718dac1620 100644 --- a/pkg/kubelet/nodestatus/setters.go +++ b/pkg/kubelet/nodestatus/setters.go @@ -59,6 +59,7 @@ type Setter func(node *v1.Node) error func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP validateNodeIPFunc func(net.IP) error, // typically Kubelet.nodeIPValidator hostname string, // typically Kubelet.hostname + hostnameOverridden bool, // was the hostname force set? externalCloudProvider bool, // typically Kubelet.externalCloudProvider cloud cloudprovider.Interface, // typically Kubelet.cloud nodeAddressesFunc func() ([]v1.NodeAddress, error), // typically Kubelet.cloudResourceSyncManager.NodeAddresses @@ -111,10 +112,38 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP return fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", nodeIP) } - // Only add a NodeHostName address if the cloudprovider did not specify any addresses. - // (we assume the cloudprovider is authoritative if it specifies any addresses) - if len(nodeAddresses) == 0 { - nodeAddresses = []v1.NodeAddress{{Type: v1.NodeHostName, Address: hostname}} + switch { + case len(nodeAddresses) == 0: + // the cloud provider didn't specify any addresses + nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeHostName, Address: hostname}) + + case !hasAddressType(nodeAddresses, v1.NodeHostName) && hasAddressValue(nodeAddresses, hostname): + // the cloud provider didn't specify an address of type Hostname, + // but the auto-detected hostname matched an address reported by the cloud provider, + // so we can add it and count on the value being verifiable via cloud provider metadata + nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeHostName, Address: hostname}) + + case hostnameOverridden: + // the hostname was force-set via flag/config. + // this means the hostname might not be able to be validated via cloud provider metadata, + // but was a choice by the kubelet deployer we should honor + var existingHostnameAddress *v1.NodeAddress + for i := range nodeAddresses { + if nodeAddresses[i].Type == v1.NodeHostName { + existingHostnameAddress = &nodeAddresses[i] + break + } + } + + if existingHostnameAddress == nil { + // no existing Hostname address found, add it + glog.Warningf("adding overridden hostname of %v to cloudprovider-reported addresses", hostname) + nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeHostName, Address: hostname}) + } else { + // override the Hostname address reported by the cloud provider + glog.Warningf("replacing cloudprovider-reported hostname of %v with overridden hostname of %v", existingHostnameAddress.Address, hostname) + existingHostnameAddress.Address = hostname + } } node.Status.Addresses = nodeAddresses } else { @@ -163,6 +192,23 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP } } +func hasAddressType(addresses []v1.NodeAddress, addressType v1.NodeAddressType) bool { + for _, address := range addresses { + if address.Type == addressType { + return true + } + } + return false +} +func hasAddressValue(addresses []v1.NodeAddress, addressValue string) bool { + for _, address := range addresses { + if address.Address == addressValue { + return true + } + } + return false +} + // MachineInfo returns a Setter that updates machine-related information on the node. func MachineInfo(nodeName string, maxPods int, diff --git a/pkg/kubelet/nodestatus/setters_test.go b/pkg/kubelet/nodestatus/setters_test.go index 2952d2db098..35a9ab9b288 100644 --- a/pkg/kubelet/nodestatus/setters_test.go +++ b/pkg/kubelet/nodestatus/setters_test.go @@ -55,6 +55,7 @@ const ( func TestNodeAddress(t *testing.T) { cases := []struct { name string + hostnameOverride bool nodeIP net.IP nodeAddresses []v1.NodeAddress expectedAddresses []v1.NodeAddress @@ -151,6 +152,85 @@ func TestNodeAddress(t *testing.T) { expectedAddresses: nil, shouldError: true, }, + { + name: "no cloud reported hostnames", + nodeAddresses: []v1.NodeAddress{}, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeHostName, Address: testKubeletHostname}, // detected hostname is auto-added in the absence of cloud-reported hostnames + }, + shouldError: false, + }, + { + name: "cloud reports hostname, no override", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: "cloud-host"}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: "cloud-host"}, // cloud-reported hostname wins over detected hostname + }, + shouldError: false, + }, + { + name: "cloud reports hostname, overridden", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: "cloud-host"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, // hostname-override wins over cloud-reported hostname + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + }, + hostnameOverride: true, + shouldError: false, + }, + { + name: "cloud doesn't report hostname, no override, detected hostname mismatch", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + // detected hostname is not auto-added if it doesn't match any cloud-reported addresses + }, + shouldError: false, + }, + { + name: "cloud doesn't report hostname, no override, detected hostname match", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeExternalDNS, Address: testKubeletHostname}, // cloud-reported address value matches detected hostname + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeExternalDNS, Address: testKubeletHostname}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, // detected hostname gets auto-added + }, + shouldError: false, + }, + { + name: "cloud doesn't report hostname, hostname override, hostname mismatch", + nodeAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + }, + expectedAddresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.1.1.1"}, + {Type: v1.NodeExternalIP, Address: "55.55.55.55"}, + {Type: v1.NodeHostName, Address: testKubeletHostname}, // overridden hostname gets auto-added + }, + hostnameOverride: true, + shouldError: false, + }, } for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { @@ -178,6 +258,7 @@ func TestNodeAddress(t *testing.T) { setter := NodeAddress(nodeIP, nodeIPValidator, hostname, + testCase.hostnameOverride, externalCloudProvider, cloud, nodeAddressesFunc)