Merge pull request #2186 from weaveworks/mike/ecs/clusterise-service-ids

Rename ECS Service node ids to be cluster;serviceName

probe/awsecs/reporter.go

@@ -124,7 +124,7 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
 
 		// Create all the services first
 		for serviceName, service := range ecsInfo.Services {
-			serviceID := report.MakeECSServiceNodeID(serviceName)
+			serviceID := report.MakeECSServiceNodeID(cluster, serviceName)
 			rpt.ECSService = rpt.ECSService.AddNode(report.MakeNodeWith(serviceID, map[string]string{
 				Cluster:             cluster,
 				ServiceDesiredCount: fmt.Sprintf("%d", service.DesiredCount),
@@ -153,7 +153,7 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
 			parentsSets := report.MakeSets()
 			parentsSets = parentsSets.Add(report.ECSTask, report.MakeStringSet(taskID))
 			if serviceName, ok := ecsInfo.TaskServiceMap[taskArn]; ok {
-				serviceID := report.MakeECSServiceNodeID(serviceName)
+				serviceID := report.MakeECSServiceNodeID(cluster, serviceName)
 				parentsSets = parentsSets.Add(report.ECSService, report.MakeStringSet(serviceID))
 			}
 			for _, containerID := range info.ContainerIDs {

probe/awsecs/reporter_test.go

@@ -148,7 +148,7 @@ func TestTagReport(t *testing.T) {
 	}
 
 	// Check service node is present and contains expected values
-	service, ok := rpt.ECSService.Nodes[report.MakeECSServiceNodeID(testServiceName)]
+	service, ok := rpt.ECSService.Nodes[report.MakeECSServiceNodeID(testCluster, testServiceName)]
 	if !ok {
 		t.Fatalf("Result report did not contain service %v: %v", testServiceName, rpt.ECSService.Nodes)
 	}
@@ -175,7 +175,7 @@ func TestTagReport(t *testing.T) {
 	}
 	containerParentsExpected := map[string]string{
 		report.ECSTask:    report.MakeECSTaskNodeID(testTaskARN),
-		report.ECSService: report.MakeECSServiceNodeID(testServiceName),
+		report.ECSService: report.MakeECSServiceNodeID(testCluster, testServiceName),
 	}
 	for key, expectedValue := range containerParentsExpected {
 		values, ok := container.Parents.Lookup(key)

render/detailed/parents.go

@@ -105,7 +105,7 @@ func ecsTaskParent(n report.Node) Parent {
 }
 
 func ecsServiceParent(n report.Node) Parent {
-	name, _ := report.ParseECSServiceNodeID(n.ID)
+	_, name, _ := report.ParseECSServiceNodeID(n.ID)
 	return Parent{
 		ID:         n.ID,
 		Label:      name,

render/detailed/summary.go

@@ -249,7 +249,7 @@ func ecsTaskNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 }
 
 func ecsServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
-	base.Label, _ = report.ParseECSServiceNodeID(n.ID)
+	_, base.Label, _ = report.ParseECSServiceNodeID(n.ID)
 	base.Stack = true
 	return base, true
 }

report/id.go

@@ -77,6 +77,11 @@ func MakeProcessNodeID(hostID, pid string) string {
 	return hostID + ScopeDelim + pid
 }
 
+// MakeECSServiceNodeID produces an ECS Service node ID from its composite parts.
+func MakeECSServiceNodeID(cluster, serviceName string) string {
+	return cluster + ScopeDelim + serviceName
+}
+
 var (
 	// MakeHostNodeID produces a host node ID from its composite parts.
 	MakeHostNodeID = makeSingleComponentID("host")
@@ -125,12 +130,6 @@ var (
 
 	// ParseECSTaskNodeID parses a replica set node ID
 	ParseECSTaskNodeID = parseSingleComponentID("ecs_task")
-
-	// MakeECSServiceNodeID produces a replica set node ID from its composite parts.
-	MakeECSServiceNodeID = makeSingleComponentID("ecs_service")
-
-	// ParseECSServiceNodeID parses a replica set node ID
-	ParseECSServiceNodeID = parseSingleComponentID("ecs_service")
 )
 
 // makeSingleComponentID makes a single-component node id encoder
@@ -204,6 +203,20 @@ func ParseAddressNodeID(addressNodeID string) (hostID, address string, ok bool)
 	return fields[0], fields[1], true
 }
 
+// ParseECSServiceNodeID produces the cluster, service name from an ECS Service node ID
+func ParseECSServiceNodeID(ecsServiceNodeID string) (cluster, serviceName string, ok bool) {
+	fields := strings.SplitN(ecsServiceNodeID, ScopeDelim, 2)
+	if len(fields) != 2 {
+		return "", "", false
+	}
+	// In previous versions, ECS Service node IDs were of form serviceName + "<ecs_service>".
+	// For backwards compatibility, we should still return a sensical serviceName for these cases.
+	if fields[1] == "<ecs_service>" {
+		return "unknown", fields[0], true
+	}
+	return fields[0], fields[1], true
+}
+
 // ExtractHostID extracts the host id from Node
 func ExtractHostID(m Node) string {
 	hostNodeID, _ := m.Latest.Lookup(HostNodeID)

report/id_test.go

@@ -66,3 +66,15 @@ func TestEndpointNodeID(t *testing.T) {
 		}
 	}
 }
+
+func TestECSServiceNodeIDCompat(t *testing.T) {
+	testID := "my-service;<ecs_service>"
+	testName := "my-service"
+	_, name, ok := report.ParseECSServiceNodeID(testID)
+	if !ok {
+		t.Errorf("Failed to parse backwards-compatible id %q", testID)
+	}
+	if name != testName {
+		t.Errorf("Backwards-compatible id %q parsed name to %q, expected %q", testID, name, testName)
+	}
+}