fix(ecs): filter out non-active ECS services

internal/pkg/aws/ecs/ecs.go

@@ -19,7 +19,7 @@ import (
 )
 
 const (
-	clusterStatusActive              = "ACTIVE"
+	statusActive                     = "ACTIVE"
 	waitServiceStablePollingInterval = 15 * time.Second
 	waitServiceStableMaxTry          = 80
 	stableServiceDeploymentNum       = 1
@@ -301,7 +301,7 @@ func (e *ECS) DefaultCluster() (string, error) {
 
 	// NOTE: right now at most 1 default cluster is possible, so cluster[0] must be the default cluster
 	cluster := resp.Clusters[0]
-	if aws.StringValue(cluster.Status) != clusterStatusActive {
+	if aws.StringValue(cluster.Status) != statusActive {
 		return "", ErrNoDefaultCluster
 	}
 
@@ -319,7 +319,7 @@ func (e *ECS) HasDefaultCluster() (bool, error) {
 	return true, nil
 }
 
-// ActiveClusters returns the subset of arns that have an ACTIVE status.
+// ActiveClusters returns the subset of cluster arns that have an ACTIVE status.
 func (e *ECS) ActiveClusters(arns ...string) ([]string, error) {
 	resp, err := e.client.DescribeClusters(&ecs.DescribeClustersInput{
 		Clusters: aws.StringSlice(arns),
@@ -333,13 +333,47 @@ func (e *ECS) ActiveClusters(arns ...string) ([]string, error) {
 
 	var active []string
 	for _, cluster := range resp.Clusters {
-		if aws.StringValue(cluster.Status) == clusterStatusActive {
+		if aws.StringValue(cluster.Status) == statusActive {
 			active = append(active, aws.StringValue(cluster.ClusterArn))
 		}
 	}
 
 	return active, nil
+}
 
+// ActiveServices returns the subset of service arns that have an ACTIVE status.
+// Note that all services should be in the same cluster.
+func (e *ECS) ActiveServices(serviceARNs ...string) ([]string, error) {
+	var prevSvcArn *ServiceArn
+	for _, arn := range serviceARNs {
+		svcArn, err := ParseServiceArn(arn)
+		if err != nil {
+			return nil, err
+		}
+		if prevSvcArn != nil && prevSvcArn.clusterName != svcArn.clusterName {
+			return nil, fmt.Errorf("service %q and service %q should be in the same cluster", prevSvcArn.String(), svcArn.String())
+		}
+		prevSvcArn = svcArn
+	}
+	resp, err := e.client.DescribeServices(&ecs.DescribeServicesInput{
+		Cluster:  aws.String(prevSvcArn.ClusterArn()),
+		Services: aws.StringSlice(serviceARNs),
+	})
+	switch {
+	case err != nil:
+		return nil, fmt.Errorf("describe services: %w", err)
+	case len(resp.Failures) > 0:
+		return nil, fmt.Errorf("describe services: %s", resp.Failures[0].GoString())
+	}
+
+	var active []string
+	for _, svc := range resp.Services {
+		if aws.StringValue(svc.Status) == statusActive {
+			active = append(active, aws.StringValue(svc.ServiceArn))
+		}
+	}
+
+	return active, nil
 }
 
 // RunTask runs a number of tasks with the task definition and network configurations in a cluster, and returns after

internal/pkg/aws/ecs/ecs_test.go

@@ -732,12 +732,12 @@ func TestECS_DefaultCluster(t *testing.T) {
 							{
 								ClusterArn:  aws.String("arn:aws:ecs:us-east-1:0123456:cluster/cluster1"),
 								ClusterName: aws.String("cluster1"),
-								Status:      aws.String(clusterStatusActive),
+								Status:      aws.String(statusActive),
 							},
 							{
 								ClusterArn:  aws.String("arn:aws:ecs:us-east-1:0123456:cluster/cluster2"),
 								ClusterName: aws.String("cluster2"),
-								Status:      aws.String(clusterStatusActive),
+								Status:      aws.String(statusActive),
 							},
 						},
 					}, nil)
@@ -822,7 +822,7 @@ func TestECS_HasDefaultCluster(t *testing.T) {
 						Clusters: []*ecs.Cluster{
 							{
 								ClusterArn: aws.String("cluster"),
-								Status:     aws.String(clusterStatusActive),
+								Status:     aws.String(statusActive),
 							},
 						},
 					}, nil)
@@ -857,12 +857,14 @@ func TestECS_HasDefaultCluster(t *testing.T) {
 
 func TestECS_ActiveClusters(t *testing.T) {
 	testCases := map[string]struct {
+		inArns        []string
 		mockECSClient func(m *mocks.Mockapi)
 
 		wantedError    error
 		wantedClusters []string
 	}{
 		"describe clusters returns error": {
+			inArns: []string{"arn1"},
 			mockECSClient: func(m *mocks.Mockapi) {
 				m.EXPECT().
 					DescribeClusters(gomock.Any()).
@@ -871,22 +873,25 @@ func TestECS_ActiveClusters(t *testing.T) {
 			wantedError: fmt.Errorf("describe clusters: some error"),
 		},
 		"ignore inactive cluster": {
+			inArns: []string{"arn1", "arn2"},
 			mockECSClient: func(m *mocks.Mockapi) {
 				m.EXPECT().
-					DescribeClusters(gomock.Any()).
+					DescribeClusters(&ecs.DescribeClustersInput{
+						Clusters: aws.StringSlice([]string{"arn1", "arn2"}),
+					}).
 					Return(&ecs.DescribeClustersOutput{
 						Clusters: []*ecs.Cluster{
 							{
 								ClusterArn: aws.String("cluster1"),
-								Status:     aws.String(clusterStatusActive),
+								Status:     aws.String(statusActive),
 							},
 							{
 								ClusterArn: aws.String("cluster2"),
 								Status:     aws.String("INACTIVE"),
 							},
 							{
 								ClusterArn: aws.String("cluster3"),
-								Status:     aws.String(clusterStatusActive),
+								Status:     aws.String(statusActive),
 							},
 							{
 								ClusterArn: aws.String("cluster4"),
@@ -913,7 +918,7 @@ func TestECS_ActiveClusters(t *testing.T) {
 			ecs := ECS{
 				client: mockECSClient,
 			}
-			clusters, err := ecs.ActiveClusters("arn1", "arn2")
+			clusters, err := ecs.ActiveClusters(tc.inArns...)
 			if tc.wantedError != nil {
 				require.EqualError(t, tc.wantedError, err.Error())
 			} else {
@@ -923,6 +928,76 @@ func TestECS_ActiveClusters(t *testing.T) {
 	}
 }
 
+func TestECS_ActiveServices(t *testing.T) {
+	testCases := map[string]struct {
+		inArns        []string
+		mockECSClient func(m *mocks.Mockapi)
+
+		wantedError    error
+		wantedServices []string
+	}{
+		"error if services are not in the same cluster": {
+			inArns: []string{"arn:aws:ecs:us-west-2:1234567890:service/cluster1/svc1", "arn:aws:ecs:us-west-2:1234567890:service/cluster2/svc2"},
+			mockECSClient: func(m *mocks.Mockapi) {
+				m.EXPECT().
+					DescribeServices(gomock.Any()).Times(0)
+			},
+			wantedError: fmt.Errorf(`service "arn:aws:ecs:us-west-2:1234567890:service/cluster1/svc1" and service "arn:aws:ecs:us-west-2:1234567890:service/cluster2/svc2" should be in the same cluster`),
+		},
+		"describe services returns error": {
+			inArns: []string{"arn:aws:ecs:us-west-2:1234567890:service/cluster1/svc1"},
+			mockECSClient: func(m *mocks.Mockapi) {
+				m.EXPECT().
+					DescribeServices(gomock.Any()).
+					Return(nil, fmt.Errorf("some error"))
+			},
+			wantedError: fmt.Errorf("describe services: some error"),
+		},
+		"ignore inactive service": {
+			inArns: []string{"arn:aws:ecs:us-west-2:1234567890:service/cluster1/svc1", "arn:aws:ecs:us-west-2:1234567890:service/cluster1/svc2"},
+			mockECSClient: func(m *mocks.Mockapi) {
+				m.EXPECT().
+					DescribeServices(gomock.Any()).
+					Return(&ecs.DescribeServicesOutput{
+						Services: []*ecs.Service{
+							{
+								ServiceArn: aws.String("service1"),
+								Status:     aws.String(statusActive),
+							},
+							{
+								ServiceArn: aws.String("service2"),
+								Status:     aws.String("random"),
+							},
+						},
+					}, nil)
+			},
+			wantedServices: []string{
+				"service1",
+			},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockECSClient := mocks.NewMockapi(ctrl)
+			tc.mockECSClient(mockECSClient)
+
+			ecs := ECS{
+				client: mockECSClient,
+			}
+			services, err := ecs.ActiveServices(tc.inArns...)
+			if tc.wantedError != nil {
+				require.EqualError(t, tc.wantedError, err.Error())
+			} else {
+				require.Equal(t, tc.wantedServices, services)
+			}
+		})
+	}
+}
+
 func TestECS_RunTask(t *testing.T) {
 	type input struct {
 		cluster         string

internal/pkg/aws/ecs/service.go

@@ -117,38 +117,63 @@ func (s *Service) TargetGroups() []string {
 }
 
 // ServiceArn is the arn of an ECS service.
-type ServiceArn string
+type ServiceArn struct {
+	accountID   string
+	partition   string
+	region      string
+	name        string
+	clusterName string
+}
 
-// ClusterName returns the cluster name.
+// ParseServiceArn parses an arn into ServiceArn.
 // For example: arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB
-// will return my-project-test-Cluster-9F7Y0RLP60R7
-func (s *ServiceArn) ClusterName() (string, error) {
-	serviceArn := string(*s)
-	parsedArn, err := arn.Parse(serviceArn)
+func ParseServiceArn(s string) (*ServiceArn, error) {
+	parsedArn, err := arn.Parse(s)
 	if err != nil {
-		return "", err
+		return nil, err
+	}
+	if parsedArn.Service != EndpointsID {
+		return nil, fmt.Errorf("expected an ECS arn, but got %q", s)
 	}
 	resources := strings.Split(parsedArn.Resource, "/")
 	if len(resources) != 3 {
-		return "", fmt.Errorf("cannot parse resource for ARN %s", serviceArn)
+		return nil, fmt.Errorf("cannot parse resource for ARN %q", s)
+	}
+	if resources[0] != "service" {
+		return nil, fmt.Errorf("expect an ECS service: got %q", s)
 	}
-	return resources[1], nil
+	return &ServiceArn{
+		accountID:   parsedArn.AccountID,
+		partition:   parsedArn.Partition,
+		region:      parsedArn.Region,
+		name:        resources[2],
+		clusterName: resources[1],
+	}, nil
+}
+
+func (s *ServiceArn) String() string {
+	return fmt.Sprintf("arn:%s:%s:%s:%s:service/%s/%s", s.partition, EndpointsID, s.region, s.accountID, s.clusterName, s.name)
+}
+
+// ClusterArn returns the cluster arn.
+// For example: arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB
+// will return arn:aws:ecs:us-west-2:1234567890:cluster/my-project-test-Cluster-9F7Y0RLP60R7
+func (s *ServiceArn) ClusterArn() string {
+	return fmt.Sprintf("arn:%s:%s:%s:%s:cluster/%s", s.partition, EndpointsID, s.region, s.accountID, s.clusterName)
 }
 
 // ServiceName returns the service name.
 // For example: arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB
 // will return my-project-test-myService-JSOH5GYBFAIB
-func (s *ServiceArn) ServiceName() (string, error) {
-	serviceArn := string(*s)
-	parsedArn, err := arn.Parse(serviceArn)
-	if err != nil {
-		return "", err
-	}
-	resources := strings.Split(parsedArn.Resource, "/")
-	if len(resources) != 3 {
-		return "", fmt.Errorf("cannot parse resource for ARN %s", serviceArn)
-	}
-	return resources[2], nil
+func (s *ServiceArn) ServiceName() string {
+	return s.name
+}
+
+// ClusterName returns the cluster name.
+// For example: arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB
+// will return my-project-test-Cluster-9F7Y0RLP60R7
+func (s *ServiceArn) ClusterName() string {
+	return s.clusterName
 }
 
 // NetworkConfiguration holds service's NetworkConfiguration.

internal/pkg/aws/ecs/service_test.go

@@ -4,6 +4,7 @@
 package ecs
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -178,3 +179,51 @@ func TestService_ServiceConnectAliases(t *testing.T) {
 		})
 	}
 }
+
+func TestParseServiceArn(t *testing.T) {
+	tests := map[string]struct {
+		inArnStr string
+
+		wantedError error
+		wantedArn   ServiceArn
+	}{
+		"error if invalid arn": {
+			inArnStr:    "random string",
+			wantedError: fmt.Errorf("arn: invalid prefix"),
+		},
+		"error if non ecs arn": {
+			inArnStr:    "arn:aws:acm:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB",
+			wantedError: fmt.Errorf(`expected an ECS arn, but got "arn:aws:acm:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB"`),
+		},
+		"error if invalid resource": {
+			inArnStr:    "arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7",
+			wantedError: fmt.Errorf(`cannot parse resource for ARN "arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7"`),
+		},
+		"error if invalid resource type": {
+			inArnStr:    "arn:aws:ecs:us-west-2:1234567890:task/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB",
+			wantedError: fmt.Errorf(`expect an ECS service: got "arn:aws:ecs:us-west-2:1234567890:task/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB"`),
+		},
+		"success": {
+			inArnStr: "arn:aws:ecs:us-west-2:1234567890:service/my-project-test-Cluster-9F7Y0RLP60R7/my-project-test-myService-JSOH5GYBFAIB",
+			wantedArn: ServiceArn{
+				accountID:   "1234567890",
+				partition:   "aws",
+				region:      "us-west-2",
+				name:        "my-project-test-myService-JSOH5GYBFAIB",
+				clusterName: "my-project-test-Cluster-9F7Y0RLP60R7",
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			// WHEN
+			arn, err := ParseServiceArn(tc.inArnStr)
+			if tc.wantedError != nil {
+				require.EqualError(t, tc.wantedError, err.Error())
+			} else {
+				require.Equal(t, tc.wantedArn, *arn)
+			}
+		})
+	}
+}

internal/pkg/cli/task_run.go

@@ -829,16 +829,11 @@ func (o *runTaskOpts) runTaskCommand() (cliStringer, error) {
 }
 
 func (o *runTaskOpts) parseARN() (string, string, error) {
-	svcARN := awsecs.ServiceArn(o.generateCommandTarget)
-	clusterName, err := svcARN.ClusterName()
+	svcARN, err := awsecs.ParseServiceArn(o.generateCommandTarget)
 	if err != nil {
-		return "", "", fmt.Errorf("extract cluster name from arn %s: %w", svcARN, err)
+		return "", "", fmt.Errorf("parse service arn %s: %w", o.generateCommandTarget, err)
 	}
-	serviceName, err := svcARN.ServiceName()
-	if err != nil {
-		return "", "", fmt.Errorf("extract service name from arn %s: %w", svcARN, err)
-	}
-	return clusterName, serviceName, nil
+	return svcARN.ClusterName(), svcARN.ServiceName(), nil
 }
 
 func (o *runTaskOpts) runTaskCommandFromECSService(sess *session.Session, clusterName, serviceName string) (cliStringer, error) {