Merge pull request #36095 from caesarxuchao/deprecate-LegacyHostIP

Automatic merge from submit-queue

[RFC] Prepare for deprecating NodeLegacyHostIP

Ref https://github.com/kubernetes/kubernetes/issues/9267#issuecomment-257994766

*What this PR does*
- Add comments saying "LegacyHostIP" will be deprecated in 1.7;
- Add v1.NodeLegacyHostIP to be consistent with the internal API (useful for client-go migration #35159)
- Let cloudproviders who used to only set LegacyHostIP set the IP as both InternalIP and ExternalIP
- Master used to ssh tunnel to node's ExternalIP or LegacyHostIP to do [healthz check](https://github.com/kubernetes/kubernetes/blame/master/pkg/master/master.go#L328-L332). OTOH, if on-prem, kubelet only [sets](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/kubelet_node_status.go#L430-L431) LegacyHostIP or InternalIP. In order to deprecate LegacyHostIP in 1.7, I let healthz check to use InternalIP if ExternalIP is not available. (The healthz check is the only consumer of LegacyHostIP in k8s.)

@liggitt @justinsb @bgrant0607 

```release-note
LegacyHostIP will be deprecated in 1.7.
```

pkg/api/types.go

@@ -2598,6 +2598,7 @@ type NodeAddressType string
 // These are valid address types of node. NodeLegacyHostIP is used to transit
 // from out-dated HostIP field to NodeAddress.
 const (
+	// Deprecated: NodeLegacyHostIP will be removed in 1.7.
 	NodeLegacyHostIP NodeAddressType = "LegacyHostIP"
 	NodeHostName     NodeAddressType = "Hostname"
 	NodeExternalIP   NodeAddressType = "ExternalIP"

pkg/api/v1/types.go

@@ -3004,9 +3004,11 @@ type NodeAddressType string
 
 // These are valid address type of node.
 const (
-	NodeHostName   NodeAddressType = "Hostname"
-	NodeExternalIP NodeAddressType = "ExternalIP"
-	NodeInternalIP NodeAddressType = "InternalIP"
+	// Deprecated: NodeLegacyHostIP will be removed in 1.7.
+	NodeLegacyHostIP NodeAddressType = "LegacyHostIP"
+	NodeHostName     NodeAddressType = "Hostname"
+	NodeExternalIP   NodeAddressType = "ExternalIP"
+	NodeInternalIP   NodeAddressType = "InternalIP"
 )
 
 // NodeAddress contains information for the node's address.

pkg/cloudprovider/providers/mesos/mesos.go

@@ -290,5 +290,9 @@ func (c *MesosCloud) NodeAddresses(nodeName types.NodeName) ([]api.NodeAddress,
 	if err != nil {
 		return nil, err
 	}
-	return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: ip.String()}}, nil
+	return []api.NodeAddress{
+		{Type: api.NodeLegacyHostIP, Address: ip.String()},
+		{Type: api.NodeInternalIP, Address: ip.String()},
+		{Type: api.NodeExternalIP, Address: ip.String()},
+	}, nil
 }

pkg/cloudprovider/providers/ovirt/ovirt.go

@@ -173,7 +173,11 @@ func (v *OVirtCloud) NodeAddresses(nodeName types.NodeName) ([]api.NodeAddress,
 		address = resolved[0]
 	}
 
-	return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: address.String()}}, nil
+	return []api.NodeAddress{
+		{Type: api.NodeLegacyHostIP, Address: address.String()},
+		{Type: api.NodeInternalIP, Address: address.String()},
+		{Type: api.NodeExternalIP, Address: address.String()},
+	}, nil
 }
 
 // mapNodeNameToInstanceName maps from a k8s NodeName to an ovirt instance name (the hostname)

pkg/cloudprovider/providers/rackspace/rackspace.go

@@ -408,7 +408,12 @@ func (i *Instances) NodeAddresses(nodeName types.NodeName) ([]api.NodeAddress, e
 	glog.V(2).Infof("NodeAddresses(%v) => %v", serverName, ip)
 
 	// net.ParseIP().String() is to maintain compatibility with the old code
-	return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: net.ParseIP(ip).String()}}, nil
+	parsedIP := net.ParseIP(ip).String()
+	return []api.NodeAddress{
+		{Type: api.NodeLegacyHostIP, Address: parsedIP},
+		{Type: api.NodeInternalIP, Address: parsedIP},
+		{Type: api.NodeExternalIP, Address: parsedIP},
+	}, nil
 }
 
 // mapNodeNameToServerName maps from a k8s NodeName to a rackspace Server Name

pkg/master/BUILD

@@ -78,6 +78,7 @@ go_library(
         "//pkg/util/async:go_default_library",
         "//pkg/util/intstr:go_default_library",
         "//pkg/util/net:go_default_library",
+        "//pkg/util/node:go_default_library",
         "//pkg/util/runtime:go_default_library",
         "//pkg/util/wait:go_default_library",
         "//vendor:github.com/golang/glog",

pkg/master/master.go

@@ -44,6 +44,7 @@ import (
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
 	"k8s.io/kubernetes/pkg/master/thirdparty"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
+	nodeutil "k8s.io/kubernetes/pkg/util/node"
 
 	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/registry/generic/registry"
@@ -354,6 +355,10 @@ type nodeAddressProvider struct {
 }
 
 func (n nodeAddressProvider) externalAddresses() (addresses []string, err error) {
+	preferredAddressTypes := []api.NodeAddressType{
+		api.NodeExternalIP,
+		api.NodeLegacyHostIP,
+	}
 	nodes, err := n.nodeClient.List(api.ListOptions{})
 	if err != nil {
 		return nil, err
@@ -361,7 +366,7 @@ func (n nodeAddressProvider) externalAddresses() (addresses []string, err error)
 	addrs := []string{}
 	for ix := range nodes.Items {
 		node := &nodes.Items[ix]
-		addr, err := findExternalAddress(node)
+		addr, err := nodeutil.GetPreferredNodeAddress(node, preferredAddressTypes)
 		if err != nil {
 			return nil, err
 		}
@@ -370,24 +375,6 @@ func (n nodeAddressProvider) externalAddresses() (addresses []string, err error)
 	return addrs, nil
 }
 
-// findExternalAddress returns ExternalIP of provided node with fallback to LegacyHostIP.
-func findExternalAddress(node *api.Node) (string, error) {
-	var fallback string
-	for ix := range node.Status.Addresses {
-		addr := &node.Status.Addresses[ix]
-		if addr.Type == api.NodeExternalIP {
-			return addr.Address, nil
-		}
-		if fallback == "" && addr.Type == api.NodeLegacyHostIP {
-			fallback = addr.Address
-		}
-	}
-	if fallback != "" {
-		return fallback, nil
-	}
-	return "", fmt.Errorf("Couldn't find external address: %v", node)
-}
-
 func DefaultAPIResourceConfigSource() *genericapiserver.ResourceConfig {
 	ret := genericapiserver.NewResourceConfig()
 	ret.EnableVersions(

pkg/master/master_test.go

@@ -167,29 +167,6 @@ func TestVersion(t *testing.T) {
 	}
 }
 
-// TestFindExternalAddress verifies both pass and fail cases for the unexported
-// findExternalAddress function
-func TestFindExternalAddress(t *testing.T) {
-	assert := assert.New(t)
-	expectedIP := "172.0.0.1"
-
-	nodes := []*api.Node{new(api.Node), new(api.Node), new(api.Node)}
-	nodes[0].Status.Addresses = []api.NodeAddress{{Type: "ExternalIP", Address: expectedIP}}
-	nodes[1].Status.Addresses = []api.NodeAddress{{Type: "LegacyHostIP", Address: expectedIP}}
-	nodes[2].Status.Addresses = []api.NodeAddress{{Type: "ExternalIP", Address: expectedIP}, {Type: "LegacyHostIP", Address: "172.0.0.2"}}
-
-	// Pass Case
-	for _, node := range nodes {
-		ip, err := findExternalAddress(node)
-		assert.NoError(err, "error getting node external address")
-		assert.Equal(expectedIP, ip, "expected ip to be %s, but was %s", expectedIP, ip)
-	}
-
-	// Fail case
-	_, err := findExternalAddress(new(api.Node))
-	assert.Error(err, "expected findExternalAddress to fail on a node with missing ip information")
-}
-
 type fakeEndpointReconciler struct{}
 
 func (*fakeEndpointReconciler) ReconcileEndpoints(serviceName string, ip net.IP, endpointPorts []api.EndpointPort, reconcilePorts bool) error {