From 8fe884ac3ffd6c4babda5bcdd1164692198c8d69 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Thu, 29 Sep 2016 01:29:05 -0400 Subject: [PATCH] Use nodeutil.GetHostIP consistently when talking to nodes Most of our communications from apiserver -> nodes used nodutil.GetNodeHostIP, but a few places didn't - and this meant that the node name needed to be resolvable _and_ we needed to populate valid IP addresses. Fix the last few places that used the NodeName. Issue #18525 Issue #9451 Issue #9728 Issue #17643 Issue #11543 Issue #22063 Issue #2462 Issue #22109 Issue #22770 Issue #32286 --- pkg/kubelet/client/kubelet_client.go | 11 ++++---- pkg/registry/core/node/etcd/etcd.go | 35 +++++++++++++----------- pkg/registry/core/node/etcd/etcd_test.go | 3 +- pkg/registry/core/node/strategy.go | 5 ++-- pkg/registry/core/pod/strategy.go | 19 +++++++------ 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/pkg/kubelet/client/kubelet_client.go b/pkg/kubelet/client/kubelet_client.go index 4e987bf5cbd..8cb5937f679 100644 --- a/pkg/kubelet/client/kubelet_client.go +++ b/pkg/kubelet/client/kubelet_client.go @@ -28,6 +28,7 @@ import ( "k8s.io/kubernetes/pkg/api/validation" "k8s.io/kubernetes/pkg/client/restclient" "k8s.io/kubernetes/pkg/client/transport" + "k8s.io/kubernetes/pkg/types" utilnet "k8s.io/kubernetes/pkg/util/net" ) @@ -51,11 +52,11 @@ type KubeletClientConfig struct { // KubeletClient is an interface for all kubelet functionality type KubeletClient interface { - GetRawConnectionInfo(ctx api.Context, nodeName string) (scheme string, port uint, transport http.RoundTripper, err error) + GetRawConnectionInfo(ctx api.Context, nodeName types.NodeName) (scheme string, port uint, transport http.RoundTripper, err error) } type ConnectionInfoGetter interface { - GetConnectionInfo(ctx api.Context, nodeName string) (scheme string, port uint, transport http.RoundTripper, err error) + GetConnectionInfo(ctx api.Context, nodeName types.NodeName) (scheme string, host string, port uint, transport http.RoundTripper, err error) } // HTTPKubeletClient is the default implementation of KubeletHealthchecker, accesses the kubelet over HTTP. @@ -98,8 +99,8 @@ func NewStaticKubeletClient(config *KubeletClientConfig) (KubeletClient, error) } // In default HTTPKubeletClient ctx is unused. -func (c *HTTPKubeletClient) GetRawConnectionInfo(ctx api.Context, nodeName string) (string, uint, http.RoundTripper, error) { - if errs := validation.ValidateNodeName(nodeName, false); len(errs) != 0 { +func (c *HTTPKubeletClient) GetRawConnectionInfo(ctx api.Context, nodeName types.NodeName) (string, uint, http.RoundTripper, error) { + if errs := validation.ValidateNodeName(string(nodeName), false); len(errs) != 0 { return "", 0, nil, fmt.Errorf("invalid node name: %s", strings.Join(errs, ";")) } scheme := "http" @@ -114,7 +115,7 @@ func (c *HTTPKubeletClient) GetRawConnectionInfo(ctx api.Context, nodeName strin // no kubelets. type FakeKubeletClient struct{} -func (c FakeKubeletClient) GetRawConnectionInfo(ctx api.Context, nodeName string) (string, uint, http.RoundTripper, error) { +func (c FakeKubeletClient) GetRawConnectionInfo(ctx api.Context, nodeName types.NodeName) (string, uint, http.RoundTripper, error) { return "", 0, nil, errors.New("Not Implemented") } diff --git a/pkg/registry/core/node/etcd/etcd.go b/pkg/registry/core/node/etcd/etcd.go index a7f56fe5b36..64893e34982 100644 --- a/pkg/registry/core/node/etcd/etcd.go +++ b/pkg/registry/core/node/etcd/etcd.go @@ -30,6 +30,8 @@ import ( "k8s.io/kubernetes/pkg/registry/generic" "k8s.io/kubernetes/pkg/registry/generic/registry" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/types" + nodeutil "k8s.io/kubernetes/pkg/util/node" ) // NodeStorage includes storage for nodes and all sub resources @@ -126,30 +128,31 @@ func (r *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Roun var _ = client.ConnectionInfoGetter(&REST{}) -func (r *REST) getKubeletPort(ctx api.Context, nodeName string) (int, error) { - // We probably shouldn't care about context when looking for Node object. - obj, err := r.Get(ctx, nodeName) +func (r *REST) GetConnectionInfo(ctx api.Context, nodeName types.NodeName) (string, string, uint, http.RoundTripper, error) { + scheme, port, transport, err := r.connection.GetRawConnectionInfo(ctx, nodeName) if err != nil { - return 0, err + return "", "", 0, nil, err + } + + // We probably shouldn't care about context when looking for Node object. + obj, err := r.Get(ctx, string(nodeName)) + if err != nil { + return "", "", 0, nil, err } node, ok := obj.(*api.Node) if !ok { - return 0, fmt.Errorf("Unexpected object type: %#v", node) + return "", "", 0, nil, fmt.Errorf("Unexpected object type: %#v", node) } - return int(node.Status.DaemonEndpoints.KubeletEndpoint.Port), nil -} -func (c *REST) GetConnectionInfo(ctx api.Context, nodeName string) (string, uint, http.RoundTripper, error) { - scheme, port, transport, err := c.connection.GetRawConnectionInfo(ctx, nodeName) + hostIP, err := nodeutil.GetNodeHostIP(node) if err != nil { - return "", 0, nil, err - } - daemonPort, err := c.getKubeletPort(ctx, nodeName) - if err != nil { - return "", 0, nil, err + return "", "", 0, nil, err } + host := hostIP.String() + + daemonPort := int(node.Status.DaemonEndpoints.KubeletEndpoint.Port) if daemonPort > 0 { - return scheme, uint(daemonPort), transport, nil + return scheme, host, uint(daemonPort), transport, nil } - return scheme, port, transport, nil + return scheme, host, port, transport, nil } diff --git a/pkg/registry/core/node/etcd/etcd_test.go b/pkg/registry/core/node/etcd/etcd_test.go index 396b847555d..35200e3a59a 100644 --- a/pkg/registry/core/node/etcd/etcd_test.go +++ b/pkg/registry/core/node/etcd/etcd_test.go @@ -28,12 +28,13 @@ import ( "k8s.io/kubernetes/pkg/registry/registrytest" "k8s.io/kubernetes/pkg/runtime" etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing" + "k8s.io/kubernetes/pkg/types" ) type fakeConnectionInfoGetter struct { } -func (fakeConnectionInfoGetter) GetRawConnectionInfo(ctx api.Context, nodeName string) (string, uint, http.RoundTripper, error) { +func (fakeConnectionInfoGetter) GetRawConnectionInfo(ctx api.Context, nodeName types.NodeName) (string, uint, http.RoundTripper, error) { return "http", 12345, nil, nil } diff --git a/pkg/registry/core/node/strategy.go b/pkg/registry/core/node/strategy.go index 077f7b5bf95..c838617aac4 100644 --- a/pkg/registry/core/node/strategy.go +++ b/pkg/registry/core/node/strategy.go @@ -33,6 +33,7 @@ import ( "k8s.io/kubernetes/pkg/registry/generic" "k8s.io/kubernetes/pkg/runtime" pkgstorage "k8s.io/kubernetes/pkg/storage" + "k8s.io/kubernetes/pkg/types" utilnet "k8s.io/kubernetes/pkg/util/net" nodeutil "k8s.io/kubernetes/pkg/util/node" "k8s.io/kubernetes/pkg/util/validation/field" @@ -189,13 +190,13 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet // We check if we want to get a default Kubelet's transport. It happens if either: // - no port is specified in request (Kubelet's port is default), // - we're using Port stored as a DaemonEndpoint and requested port is a Kubelet's port stored in the DaemonEndpoint, - // - there's no information in the API about DaemonEnpoint (legacy cluster) and requested port is equal to ports.KubeletPort (cluster-wide config) + // - there's no information in the API about DaemonEndpoint (legacy cluster) and requested port is equal to ports.KubeletPort (cluster-wide config) kubeletPort := node.Status.DaemonEndpoints.KubeletEndpoint.Port if kubeletPort == 0 { kubeletPort = ports.KubeletPort } if portReq == "" || strconv.Itoa(int(kubeletPort)) == portReq { - scheme, port, kubeletTransport, err := connection.GetConnectionInfo(ctx, node.Name) + scheme, host, port, kubeletTransport, err := connection.GetConnectionInfo(ctx, types.NodeName(node.Name)) if err != nil { return nil, nil, err } diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index aa16a924a9b..39e7ac84bcf 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/registry/generic" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/storage" + "k8s.io/kubernetes/pkg/types" utilnet "k8s.io/kubernetes/pkg/util/net" "k8s.io/kubernetes/pkg/util/validation/field" ) @@ -299,12 +300,12 @@ func LogLocation( return nil, nil, errors.NewBadRequest(fmt.Sprintf("container %s is not valid for pod %s", container, name)) } } - nodeHost := pod.Spec.NodeName - if len(nodeHost) == 0 { + nodeName := types.NodeName(pod.Spec.NodeName) + if len(nodeName) == 0 { // If pod has not been assigned a host, return an empty location return nil, nil, nil } - nodeScheme, nodePort, nodeTransport, err := connInfo.GetConnectionInfo(ctx, nodeHost) + nodeScheme, nodeHost, nodePort, nodeTransport, err := connInfo.GetConnectionInfo(ctx, nodeName) if err != nil { return nil, nil, err } @@ -450,12 +451,12 @@ func streamLocation( return nil, nil, errors.NewBadRequest(fmt.Sprintf("container %s is not valid for pod %s", container, name)) } } - nodeHost := pod.Spec.NodeName - if len(nodeHost) == 0 { + nodeName := types.NodeName(pod.Spec.NodeName) + if len(nodeName) == 0 { // If pod has not been assigned a host, return an empty location return nil, nil, errors.NewBadRequest(fmt.Sprintf("pod %s does not have a host assigned", name)) } - nodeScheme, nodePort, nodeTransport, err := connInfo.GetConnectionInfo(ctx, nodeHost) + nodeScheme, nodeHost, nodePort, nodeTransport, err := connInfo.GetConnectionInfo(ctx, nodeName) if err != nil { return nil, nil, err } @@ -484,12 +485,12 @@ func PortForwardLocation( return nil, nil, err } - nodeHost := pod.Spec.NodeName - if len(nodeHost) == 0 { + nodeName := types.NodeName(pod.Spec.NodeName) + if len(nodeName) == 0 { // If pod has not been assigned a host, return an empty location return nil, nil, errors.NewBadRequest(fmt.Sprintf("pod %s does not have a host assigned", name)) } - nodeScheme, nodePort, nodeTransport, err := connInfo.GetConnectionInfo(ctx, nodeHost) + nodeScheme, nodeHost, nodePort, nodeTransport, err := connInfo.GetConnectionInfo(ctx, nodeName) if err != nil { return nil, nil, err }