Skip to content

Commit

Permalink
Rename ECS Service node ids to be cluster;serviceName
Browse files Browse the repository at this point in the history
This is important for two reasons:
* It prevents nasty false-equality bugs when two different services from different ECS clusters
  are present in the same report
* It allows us to retrieve the cluster and service name - all the info we need to look up the service -
  using only the node ID. This matters, for example, when trying to handle a control request.
  • Loading branch information
ekimekim committed Feb 3, 2017
1 parent 43cb754 commit fad3e88
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 12 deletions.
4 changes: 2 additions & 2 deletions probe/awsecs/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions probe/awsecs/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion render/detailed/parents.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion render/detailed/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
25 changes: 19 additions & 6 deletions report/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit fad3e88

Please sign in to comment.