From 2b7662a3c6a7cb1b77908e9d169b3f44c651e4d3 Mon Sep 17 00:00:00 2001 From: Mike Lang Date: Tue, 17 Jan 2017 03:02:47 -0800 Subject: [PATCH] Make reporter tests a seperate package to appease linter This requires making All The Things public. Yuck. --- probe/awsecs/client.go | 105 +++++++++++++++--------------- probe/awsecs/reporter.go | 52 +++++++-------- probe/awsecs/reporter_test.go | 116 +++++++++++++++++----------------- 3 files changed, 138 insertions(+), 135 deletions(-) diff --git a/probe/awsecs/client.go b/probe/awsecs/client.go index b21444013..e27f5b748 100644 --- a/probe/awsecs/client.go +++ b/probe/awsecs/client.go @@ -14,10 +14,10 @@ import ( ) // A wrapper around an AWS client that makes all the needed calls and just exposes the final results. -// We create an interface so we can mock for testing -type ecsClient interface { - // Returns a ecsInfo struct containing data needed for a report. - getInfo([]string) ecsInfo +// We create an interface so we can mock for testing. +type EcsClient interface { + // Returns a EcsInfo struct containing data needed for a report. + GetInfo([]string) EcsInfo } // actual implementation @@ -30,37 +30,40 @@ type ecsClientImpl struct { // Since we're caching tasks heavily, we ensure no mistakes by casting into a structure // that only contains immutable attributes of the resource. -type ecsTask struct { - taskARN string - createdAt time.Time - taskDefinitionARN string +// Exported for test. +type EcsTask struct { + TaskARN string + CreatedAt time.Time + TaskDefinitionARN string // These started fields are immutable once set, and guaranteed to be set once the task is running, // which we know it is because otherwise we wouldn't be looking at it. - startedAt time.Time - startedBy string // tag or deployment id + StartedAt time.Time + StartedBy string // tag or deployment id } // Services are highly mutable and so we can only cache them on a best-effort basis. // We have to refresh referenced (ie. has an associated task) services each report // but we avoid re-listing services unless we can't find a service for a task. -type ecsService struct { - serviceName string +// Exported for test. +type EcsService struct { + ServiceName string // The following values may be stale in a cached copy - deploymentIDs []string - desiredCount int64 - pendingCount int64 - runningCount int64 - taskDefinitionARN string + DeploymentIDs []string + DesiredCount int64 + PendingCount int64 + RunningCount int64 + TaskDefinitionARN string } -type ecsInfo struct { - tasks map[string]ecsTask - services map[string]ecsService - taskServiceMap map[string]string +// Exported for test +type EcsInfo struct { + Tasks map[string]EcsTask + Services map[string]EcsService + TaskServiceMap map[string]string } -func newClient(cluster string, cacheSize int, cacheExpiry time.Duration) (ecsClient, error) { +func newClient(cluster string, cacheSize int, cacheExpiry time.Duration) (EcsClient, error) { sess := session.New() region, err := ec2metadata.New(sess).Region() @@ -76,28 +79,28 @@ func newClient(cluster string, cacheSize int, cacheExpiry time.Duration) (ecsCli }, nil } -func newECSTask(task *ecs.Task) ecsTask { - return ecsTask{ - taskARN: *task.TaskArn, - createdAt: *task.CreatedAt, - taskDefinitionARN: *task.TaskDefinitionArn, - startedAt: *task.StartedAt, - startedBy: *task.StartedBy, +func newECSTask(task *ecs.Task) EcsTask { + return EcsTask{ + TaskARN: *task.TaskArn, + CreatedAt: *task.CreatedAt, + TaskDefinitionARN: *task.TaskDefinitionArn, + StartedAt: *task.StartedAt, + StartedBy: *task.StartedBy, } } -func newECSService(service *ecs.Service) ecsService { +func newECSService(service *ecs.Service) EcsService { deploymentIDs := make([]string, len(service.Deployments)) for i, deployment := range service.Deployments { deploymentIDs[i] = *deployment.Id } - return ecsService{ - serviceName: *service.ServiceName, - deploymentIDs: deploymentIDs, - desiredCount: *service.DesiredCount, - pendingCount: *service.PendingCount, - runningCount: *service.RunningCount, - taskDefinitionARN: *service.TaskDefinition, + return EcsService{ + ServiceName: *service.ServiceName, + DeploymentIDs: deploymentIDs, + DesiredCount: *service.DesiredCount, + PendingCount: *service.PendingCount, + RunningCount: *service.RunningCount, + TaskDefinitionARN: *service.TaskDefinition, } } @@ -128,7 +131,7 @@ func (c ecsClientImpl) listServices() <-chan string { } // Returns (input, done) channels. Service ARNs given to input are batched and details are fetched, -// with full ecsService objects being put into the cache. Closes done when finished. +// with full EcsService objects being put into the cache. Closes done when finished. func (c ecsClientImpl) describeServices() (chan<- string, <-chan struct{}) { input := make(chan string) done := make(chan struct{}) @@ -238,8 +241,8 @@ func (c ecsClientImpl) matchTasksServices(taskARNs []string) (map[string]string, continue } serviceName := serviceNameRaw.(string) - service := serviceRaw.(ecsService) - for _, deployment := range service.deploymentIDs { + service := serviceRaw.(EcsService) + for _, deployment := range service.DeploymentIDs { deploymentMap[deployment] = serviceName } } @@ -253,12 +256,12 @@ func (c ecsClientImpl) matchTasksServices(taskARNs []string) (map[string]string, // this can happen if we have a failure while describing tasks, just pretend the task doesn't exist continue } - task := taskRaw.(ecsTask) - if !strings.HasPrefix(task.startedBy, servicePrefix) { + task := taskRaw.(EcsTask) + if !strings.HasPrefix(task.StartedBy, servicePrefix) { // task was not started by a service continue } - if serviceName, ok := deploymentMap[task.startedBy]; ok { + if serviceName, ok := deploymentMap[task.StartedBy]; ok { results[taskARN] = serviceName } else { unmatched = append(unmatched, taskARN) @@ -312,26 +315,26 @@ func (c ecsClientImpl) describeAllServices(servicesRefreshed map[string]bool) { <-done } -func (c ecsClientImpl) makeECSInfo(taskARNs []string, taskServiceMap map[string]string) ecsInfo { +func (c ecsClientImpl) makeECSInfo(taskARNs []string, taskServiceMap map[string]string) EcsInfo { // The maps to return are the referenced subsets of the full caches - tasks := map[string]ecsTask{} + tasks := map[string]EcsTask{} for _, taskARN := range taskARNs { // It's possible that tasks could still be missing from the cache if describe tasks failed. // We'll just pretend they don't exist. if taskRaw, err := c.taskCache.Get(taskARN); err == nil { - task := taskRaw.(ecsTask) + task := taskRaw.(EcsTask) tasks[taskARN] = task } } - services := map[string]ecsService{} + services := map[string]EcsService{} for taskARN, serviceName := range taskServiceMap { if _, ok := taskServiceMap[serviceName]; ok { // Already present. This is expected since multiple tasks can map to the same service. continue } if serviceRaw, err := c.serviceCache.Get(serviceName); err == nil { - service := serviceRaw.(ecsService) + service := serviceRaw.(EcsService) services[serviceName] = service } else { log.Errorf("Service %s referenced by task %s in service map but not found in cache, this shouldn't be able to happen. Removing task and continuing.", serviceName, taskARN) @@ -339,11 +342,11 @@ func (c ecsClientImpl) makeECSInfo(taskARNs []string, taskServiceMap map[string] } } - return ecsInfo{services: services, tasks: tasks, taskServiceMap: taskServiceMap} + return EcsInfo{Services: services, Tasks: tasks, TaskServiceMap: taskServiceMap} } -// Implements ecsClient.getInfo -func (c ecsClientImpl) getInfo(taskARNs []string) ecsInfo { +// Implements EcsClient.GetInfo +func (c ecsClientImpl) GetInfo(taskARNs []string) EcsInfo { log.Debugf("Getting ECS info on %d tasks", len(taskARNs)) // We do a weird order of operations here to minimize unneeded cache refreshes. diff --git a/probe/awsecs/reporter.go b/probe/awsecs/reporter.go index 67d539989..344c0ba5a 100644 --- a/probe/awsecs/reporter.go +++ b/probe/awsecs/reporter.go @@ -32,14 +32,16 @@ var ( } ) -type taskLabelInfo struct { - containerIDs []string - family string +// Used in return value of GetLabelInfo. Exported for test. +type TaskLabelInfo struct { + ContainerIDs []string + Family string } -// return map from cluster to map of task arns to task infos -func getLabelInfo(rpt report.Report) map[string]map[string]*taskLabelInfo { - results := map[string]map[string]*taskLabelInfo{} +// Return map from cluster to map of task arns to task infos. +// Exported for test. +func GetLabelInfo(rpt report.Report) map[string]map[string]*TaskLabelInfo { + results := map[string]map[string]*TaskLabelInfo{} log.Debug("scanning for ECS containers") for nodeID, node := range rpt.Container.Nodes { @@ -60,17 +62,17 @@ func getLabelInfo(rpt report.Report) map[string]map[string]*taskLabelInfo { taskMap, ok := results[cluster] if !ok { - taskMap = map[string]*taskLabelInfo{} + taskMap = map[string]*TaskLabelInfo{} results[cluster] = taskMap } task, ok := taskMap[taskArn] if !ok { - task = &taskLabelInfo{containerIDs: []string{}, family: family} + task = &TaskLabelInfo{ContainerIDs: []string{}, Family: family} taskMap[taskArn] = task } - task.containerIDs = append(task.containerIDs, nodeID) + task.ContainerIDs = append(task.ContainerIDs, nodeID) } log.Debug("Got ECS container info: %v", results) return results @@ -78,7 +80,7 @@ func getLabelInfo(rpt report.Report) map[string]map[string]*taskLabelInfo { // Reporter implements Tagger, Reporter type Reporter struct { - clientsByCluster map[string]ecsClient + ClientsByCluster map[string]EcsClient // Exported for test cacheSize int cacheExpiry time.Duration } @@ -86,7 +88,7 @@ type Reporter struct { // Make creates a new Reporter func Make(cacheSize int, cacheExpiry time.Duration) Reporter { return Reporter{ - clientsByCluster: map[string]ecsClient{}, + ClientsByCluster: map[string]EcsClient{}, cacheSize: cacheSize, cacheExpiry: cacheExpiry, } @@ -96,12 +98,12 @@ func Make(cacheSize int, cacheExpiry time.Duration) Reporter { func (r Reporter) Tag(rpt report.Report) (report.Report, error) { rpt = rpt.Copy() - clusterMap := getLabelInfo(rpt) + clusterMap := GetLabelInfo(rpt) for cluster, taskMap := range clusterMap { log.Debugf("Fetching ECS info for cluster %v with %v tasks", cluster, len(taskMap)) - client, ok := r.clientsByCluster[cluster] + client, ok := r.ClientsByCluster[cluster] if !ok { log.Debugf("Creating new ECS client") var err error @@ -109,7 +111,7 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) { if err != nil { return rpt, err } - r.clientsByCluster[cluster] = client + r.ClientsByCluster[cluster] = client } taskArns := make([]string, 0, len(taskMap)) @@ -117,22 +119,22 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) { taskArns = append(taskArns, taskArn) } - ecsInfo := client.getInfo(taskArns) - log.Debugf("Got info from ECS: %d tasks, %d services", len(ecsInfo.tasks), len(ecsInfo.services)) + ecsInfo := client.GetInfo(taskArns) + log.Debugf("Got info from ECS: %d tasks, %d services", len(ecsInfo.Tasks), len(ecsInfo.Services)) // Create all the services first - for serviceName, service := range ecsInfo.services { + for serviceName, service := range ecsInfo.Services { serviceID := report.MakeECSServiceNodeID(serviceName) rpt.ECSService = rpt.ECSService.AddNode(report.MakeNodeWith(serviceID, map[string]string{ Cluster: cluster, - ServiceDesiredCount: fmt.Sprintf("%d", service.desiredCount), - ServiceRunningCount: fmt.Sprintf("%d", service.runningCount), + ServiceDesiredCount: fmt.Sprintf("%d", service.DesiredCount), + ServiceRunningCount: fmt.Sprintf("%d", service.RunningCount), })) } - log.Debugf("Created %v ECS service nodes", len(ecsInfo.services)) + log.Debugf("Created %v ECS service nodes", len(ecsInfo.Services)) for taskArn, info := range taskMap { - task, ok := ecsInfo.tasks[taskArn] + task, ok := ecsInfo.Tasks[taskArn] if !ok { // can happen due to partial failures, just skip it continue @@ -141,20 +143,20 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) { // new task node taskID := report.MakeECSTaskNodeID(taskArn) node := report.MakeNodeWith(taskID, map[string]string{ - TaskFamily: info.family, + TaskFamily: info.Family, Cluster: cluster, - CreatedAt: task.createdAt.Format(time.RFC3339Nano), + CreatedAt: task.CreatedAt.Format(time.RFC3339Nano), }) rpt.ECSTask = rpt.ECSTask.AddNode(node) // parents sets to merge into all matching container nodes parentsSets := report.MakeSets() parentsSets = parentsSets.Add(report.ECSTask, report.MakeStringSet(taskID)) - if serviceName, ok := ecsInfo.taskServiceMap[taskArn]; ok { + if serviceName, ok := ecsInfo.TaskServiceMap[taskArn]; ok { serviceID := report.MakeECSServiceNodeID(serviceName) parentsSets = parentsSets.Add(report.ECSService, report.MakeStringSet(serviceID)) } - for _, containerID := range info.containerIDs { + for _, containerID := range info.ContainerIDs { if containerNode, ok := rpt.Container.Nodes[containerID]; ok { rpt.Container.Nodes[containerID] = containerNode.WithParents(parentsSets) } else { diff --git a/probe/awsecs/reporter_test.go b/probe/awsecs/reporter_test.go index 06331ded9..117b5364d 100644 --- a/probe/awsecs/reporter_test.go +++ b/probe/awsecs/reporter_test.go @@ -1,32 +1,30 @@ -package awsecs +package awsecs_test import ( "reflect" "testing" "time" + "github.com/weaveworks/scope/probe/awsecs" "github.com/weaveworks/scope/probe/docker" "github.com/weaveworks/scope/report" ) var ( - testCluster = "test-cluster" - testFamily = "test-family" - testTaskARN = "arn:aws:ecs:us-east-1:123456789012:task/12345678-9abc-def0-1234-56789abcdef0" - testTaskCreatedAt = time.Unix(1483228800, 0) + testCluster = "test-cluster" + testFamily = "test-family" + testTaskARN = "arn:aws:ecs:us-east-1:123456789012:task/12345678-9abc-def0-1234-56789abcdef0" + testTaskCreatedAt = time.Unix(1483228800, 0) testTaskDefinitionARN = "arn:aws:ecs:us-east-1:123456789012:task-definition/deadbeef-dead-beef-dead-beefdeadbeef" - testTaskStartedAt = time.Unix(1483228805, 0) - testDeploymentID = "ecs-svc/1121123211234321" - testServiceName = "test-service" - testServiceCount = 1 - testContainer = "test-container" - testContainerData = map[string]string{ - docker.LabelPrefix + "com.amazonaws.ecs.task-arn": - testTaskARN, - docker.LabelPrefix + "com.amazonaws.ecs.cluster": - testCluster, - docker.LabelPrefix + "com.amazonaws.ecs.task-definition-family": - testFamily, + testTaskStartedAt = time.Unix(1483228805, 0) + testDeploymentID = "ecs-svc/1121123211234321" + testServiceName = "test-service" + testServiceCount = 1 + testContainer = "test-container" + testContainerData = map[string]string{ + docker.LabelPrefix + "com.amazonaws.ecs.task-arn": testTaskARN, + docker.LabelPrefix + "com.amazonaws.ecs.cluster": testCluster, + docker.LabelPrefix + "com.amazonaws.ecs.task-definition-family": testFamily, } ) @@ -38,24 +36,24 @@ func getTestContainerNode() report.Node { } func TestGetLabelInfo(t *testing.T) { - r := Make(1e6, time.Hour) + r := awsecs.Make(1e6, time.Hour) rpt, err := r.Report() if err != nil { - t.Fatalf("Error making report", err) + t.Fatalf("Error making report: %v", err) } - labelInfo := getLabelInfo(rpt) - expected := map[string]map[string]*taskLabelInfo{} + labelInfo := awsecs.GetLabelInfo(rpt) + expected := map[string]map[string]*awsecs.TaskLabelInfo{} if !reflect.DeepEqual(labelInfo, expected) { t.Errorf("Empty report did not produce empty label info: %v != %v", labelInfo, expected) } rpt.Container = rpt.Container.AddNode(getTestContainerNode()) - labelInfo = getLabelInfo(rpt) - expected = map[string]map[string]*taskLabelInfo{ - testCluster: map[string]*taskLabelInfo{ - testTaskARN: &taskLabelInfo{ - containerIDs: []string{report.MakeContainerNodeID(testContainer)}, - family: testFamily, + labelInfo = awsecs.GetLabelInfo(rpt) + expected = map[string]map[string]*awsecs.TaskLabelInfo{ + testCluster: { + testTaskARN: { + ContainerIDs: []string{report.MakeContainerNodeID(testContainer)}, + Family: testFamily, }, }, } @@ -64,14 +62,14 @@ func TestGetLabelInfo(t *testing.T) { } } -// Implements ecsClient +// Implements EcsClient type mockEcsClient struct { - t *testing.T + t *testing.T expectedARNs []string - info ecsInfo + info awsecs.EcsInfo } -func newMockEcsClient(t *testing.T, expectedARNs []string, info ecsInfo) ecsClient { +func newMockEcsClient(t *testing.T, expectedARNs []string, info awsecs.EcsInfo) awsecs.EcsClient { return &mockEcsClient{ t, expectedARNs, @@ -79,40 +77,40 @@ func newMockEcsClient(t *testing.T, expectedARNs []string, info ecsInfo) ecsClie } } -func (c mockEcsClient) getInfo(taskARNs []string) ecsInfo { +func (c mockEcsClient) GetInfo(taskARNs []string) awsecs.EcsInfo { if !reflect.DeepEqual(taskARNs, c.expectedARNs) { - c.t.Fatalf("getInfo called with wrong ARNs: %v != %v", taskARNs, c.expectedARNs) + c.t.Fatalf("GetInfo called with wrong ARNs: %v != %v", taskARNs, c.expectedARNs) } return c.info } func TestTagReport(t *testing.T) { - r := Make(1e6, time.Hour) + r := awsecs.Make(1e6, time.Hour) - r.clientsByCluster[testCluster] = newMockEcsClient( + r.ClientsByCluster[testCluster] = newMockEcsClient( t, []string{testTaskARN}, - ecsInfo{ - tasks: map[string]ecsTask{ - testTaskARN: ecsTask{ - taskARN: testTaskARN, - createdAt: testTaskCreatedAt, - taskDefinitionARN: testTaskDefinitionARN, - startedAt: testTaskStartedAt, - startedBy: testDeploymentID, + awsecs.EcsInfo{ + Tasks: map[string]awsecs.EcsTask{ + testTaskARN: { + TaskARN: testTaskARN, + CreatedAt: testTaskCreatedAt, + TaskDefinitionARN: testTaskDefinitionARN, + StartedAt: testTaskStartedAt, + StartedBy: testDeploymentID, }, }, - services: map[string]ecsService{ - testServiceName: ecsService{ - serviceName: testServiceName, - deploymentIDs: []string{testDeploymentID}, - desiredCount: 1, - pendingCount: 0, - runningCount: 1, - taskDefinitionARN: testTaskDefinitionARN, + Services: map[string]awsecs.EcsService{ + testServiceName: { + ServiceName: testServiceName, + DeploymentIDs: []string{testDeploymentID}, + DesiredCount: 1, + PendingCount: 0, + RunningCount: 1, + TaskDefinitionARN: testTaskDefinitionARN, }, }, - taskServiceMap: map[string]string{ + TaskServiceMap: map[string]string{ testTaskARN: testServiceName, }, }, @@ -134,9 +132,9 @@ func TestTagReport(t *testing.T) { t.Fatalf("Result report did not contain task %v: %v", testTaskARN, rpt.ECSTask.Nodes) } taskExpected := map[string]string{ - TaskFamily: testFamily, - Cluster: testCluster, - CreatedAt: testTaskCreatedAt.Format(time.RFC3339Nano), + awsecs.TaskFamily: testFamily, + awsecs.Cluster: testCluster, + awsecs.CreatedAt: testTaskCreatedAt.Format(time.RFC3339Nano), } for key, expectedValue := range taskExpected { value, ok := task.Latest.Lookup(key) @@ -155,9 +153,9 @@ func TestTagReport(t *testing.T) { t.Fatalf("Result report did not contain service %v: %v", testServiceName, rpt.ECSService.Nodes) } serviceExpected := map[string]string{ - Cluster: testCluster, - ServiceDesiredCount: "1", - ServiceRunningCount: "1", + awsecs.Cluster: testCluster, + awsecs.ServiceDesiredCount: "1", + awsecs.ServiceRunningCount: "1", } for key, expectedValue := range serviceExpected { value, ok := service.Latest.Lookup(key) @@ -176,7 +174,7 @@ func TestTagReport(t *testing.T) { t.Fatalf("Result report did not contain container %v: %v", testContainer, rpt.Container.Nodes) } containerParentsExpected := map[string]string{ - report.ECSTask: report.MakeECSTaskNodeID(testTaskARN), + report.ECSTask: report.MakeECSTaskNodeID(testTaskARN), report.ECSService: report.MakeECSServiceNodeID(testServiceName), } for key, expectedValue := range containerParentsExpected {