From 6a2602a51bf3f52298f0644769d63832b9d95615 Mon Sep 17 00:00:00 2001
From: "Dr. Stefan Schimanski" <stefan.schimanski@gmail.com>
Date: Tue, 20 Oct 2015 14:39:19 +0100
Subject: [PATCH] Make cloud provider return disappearence of slave to the node
 controller

- implement ExternalID in Mesos cloud provider. This is used by the node
  controller to detect disappeared nodes.
- add test case for ExternalID
---
 pkg/cloudprovider/providers/mesos/client.go   | 10 ++++----
 pkg/cloudprovider/providers/mesos/mesos.go    | 16 +++++++++++-
 .../providers/mesos/mesos_test.go             | 25 +++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/pkg/cloudprovider/providers/mesos/client.go b/pkg/cloudprovider/providers/mesos/client.go
index c9f1ac222f3..da7d5e98551 100644
--- a/pkg/cloudprovider/providers/mesos/client.go
+++ b/pkg/cloudprovider/providers/mesos/client.go
@@ -55,7 +55,7 @@ type slaveNode struct {
 
 type mesosState struct {
 	clusterName string
-	nodes       []*slaveNode
+	nodes       map[string]*slaveNode
 }
 
 type stateCache struct {
@@ -94,7 +94,7 @@ func (c *stateCache) clusterName(ctx context.Context) (string, error) {
 }
 
 // nodes returns the cached list of slave nodes.
-func (c *stateCache) nodes(ctx context.Context) ([]*slaveNode, error) {
+func (c *stateCache) nodes(ctx context.Context) (map[string]*slaveNode, error) {
 	cached, err := c.cachedState(ctx)
 	return cached.nodes, err
 }
@@ -162,7 +162,7 @@ func unpackIPv4(ip uint32) string {
 
 // listSlaves returns a (possibly cached) list of slave nodes.
 // Callers must not mutate the contents of the returned slice.
-func (c *mesosClient) listSlaves(ctx context.Context) ([]*slaveNode, error) {
+func (c *mesosClient) listSlaves(ctx context.Context) (map[string]*slaveNode, error) {
 	return c.state.nodes(ctx)
 }
 
@@ -230,7 +230,7 @@ func parseMesosState(blob []byte) (*mesosState, error) {
 	if err := json.Unmarshal(blob, state); err != nil {
 		return nil, err
 	}
-	nodes := []*slaveNode{}
+	nodes := map[string]*slaveNode{}
 	for _, slave := range state.Slaves {
 		if slave.Hostname == "" {
 			continue
@@ -264,7 +264,7 @@ func parseMesosState(blob []byte) (*mesosState, error) {
 			}
 			log.V(4).Infof("node %q reporting capacity %v", node.hostname, cap)
 		}
-		nodes = append(nodes, node)
+		nodes[node.hostname] = node
 	}
 
 	result := &mesosState{
diff --git a/pkg/cloudprovider/providers/mesos/mesos.go b/pkg/cloudprovider/providers/mesos/mesos.go
index fb1404e310c..e376f1a0a1a 100644
--- a/pkg/cloudprovider/providers/mesos/mesos.go
+++ b/pkg/cloudprovider/providers/mesos/mesos.go
@@ -182,7 +182,21 @@ func ipAddress(name string) (net.IP, error) {
 
 // ExternalID returns the cloud provider ID of the specified instance (deprecated).
 func (c *MesosCloud) ExternalID(instance string) (string, error) {
-	ip, err := ipAddress(instance)
+	//TODO(jdef) use a timeout here? 15s?
+	ctx, cancel := context.WithCancel(context.TODO())
+	defer cancel()
+
+	nodes, err := c.client.listSlaves(ctx)
+	if err != nil {
+		return "", err
+	}
+
+	node := nodes[instance]
+	if node == nil {
+		return "", cloudprovider.InstanceNotFound
+	}
+
+	ip, err := ipAddress(node.hostname)
 	if err != nil {
 		return "", err
 	}
diff --git a/pkg/cloudprovider/providers/mesos/mesos_test.go b/pkg/cloudprovider/providers/mesos/mesos_test.go
index 748cbd25ad9..b7f909bf1e4 100644
--- a/pkg/cloudprovider/providers/mesos/mesos_test.go
+++ b/pkg/cloudprovider/providers/mesos/mesos_test.go
@@ -24,6 +24,7 @@ import (
 	"time"
 
 	log "github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/cloudprovider"
 )
 
 func TestIPAddress(t *testing.T) {
@@ -252,3 +253,27 @@ func Test_List(t *testing.T) {
 		t.Fatalf("List with a reject-all filter should return a list of size 0: (actual: %#v)", clusters)
 	}
 }
+
+func Test_ExternalID(t *testing.T) {
+	defer log.Flush()
+	md := FakeMasterDetector{}
+	httpServer, httpClient, httpTransport := makeHttpMocks()
+	defer httpServer.Close()
+	cacheTTL := 500 * time.Millisecond
+	mesosClient, err := createMesosClient(md, httpClient, httpTransport, cacheTTL)
+	mesosCloud := &MesosCloud{client: mesosClient, config: createDefaultConfig()}
+
+	_, err = mesosCloud.ExternalID("unknown")
+	if err != cloudprovider.InstanceNotFound {
+		t.Fatalf("ExternalID did not return InstanceNotFound on an unknown instance")
+	}
+
+	slaveName := "mesos3.internal.company.com"
+	id, err := mesosCloud.ExternalID(slaveName)
+	if id != "" {
+		t.Fatalf("ExternalID should not be able to resolve %q", slaveName)
+	}
+	if err == cloudprovider.InstanceNotFound {
+		t.Fatalf("ExternalID should find %q", slaveName)
+	}
+}