address feedback

internal/pkg/aws/ec2/ec2.go

@@ -43,7 +43,7 @@ func New(s *session.Session) *EC2 {
 	}
 }
 
-// GetDefaultSubnetIDs finds the subnet IDs associated with the environment of the application; if env is a None env,
+// GetSubnetIDs finds the subnet IDs associated with the environment of the application; if env is a None env,
 // it finds the default subnet IDs.
 func (c *EC2) GetSubnetIDs(app string, env string) ([]string, error) {
 	if env == config.EnvNameNone {
@@ -63,7 +63,7 @@ func (c *EC2) GetSubnetIDs(app string, env string) ([]string, error) {
 	return c.getSubnetIDs(filters)
 }
 
-// GetDefaultSubnetIDs finds the security group IDs associated with the environment of the application;
+// GetSecurityGroups finds the security group IDs associated with the environment of the application;
 // if env is a None env, it returns nil
 func (c *EC2) GetSecurityGroups(app string, env string) ([]string, error) {
 	if env == config.EnvNameNone {
@@ -94,11 +94,11 @@ func (c *EC2) getSubnetIDs(filters []*ec2.Filter) ([]string, error) {
 	})
 
 	if err != nil {
-		return nil, fmt.Errorf("find subnet IDs: %w", err)
+		return nil, fmt.Errorf("find subnets: %w", err)
 	}
 
 	if len(response.Subnets) == 0 {
-		return nil, errors.New("no subnet ID found")
+		return nil, errors.New("no subnets found")
 	}
 
 	subnetIDs := make([]string, len(response.Subnets))

internal/pkg/aws/ecs/ecs.go

@@ -182,22 +182,22 @@ func (e *ECS) ServiceTasks(clusterName, serviceName string) ([]*Task, error) {
 }
 
 // DefaultClusters returns the default clusters in the account and region
-func (e *ECS) DefaultClusters() ([]string, error) {
+func (e *ECS) DefaultCluster() (string, error) {
 	resp, err := e.client.DescribeClusters(&ecs.DescribeClustersInput{})
 	if err != nil {
-		return nil, fmt.Errorf("get default clusters: %w", err)
+		return "", fmt.Errorf("get default cluster: %w", err)
 	}
 
 	if len(resp.Clusters) == 0 {
-		return nil, errors.New("no default cluster is found")
+		return "", errors.New("no default cluster is found")
 	}
 
 	clusters := make([]string, len(resp.Clusters))
 	for idx, cluster := range resp.Clusters {
 		clusters[idx] = aws.StringValue(cluster.ClusterArn)
 	}
 
-	return clusters, nil
+	return clusters[0], 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

@@ -333,7 +333,7 @@ func TestECS_DefaultCluster(t *testing.T) {
 		mockECSClient func(m *mocks.Mockapi)
 
 		wantedError    error
-		wantedClusters []string
+		wantedClusters string
 	}{
 		"get default clusters success": {
 			mockECSClient: func(m *mocks.Mockapi) {
@@ -353,15 +353,15 @@ func TestECS_DefaultCluster(t *testing.T) {
 					}, nil)
 			},
 
-			wantedClusters: []string{"arn:aws:ecs:us-east-1:0123456:cluster/cluster1", "arn:aws:ecs:us-east-1:0123456:cluster/cluster2"},
+			wantedClusters: "arn:aws:ecs:us-east-1:0123456:cluster/cluster1",
 		},
 		"failed to get default clusters": {
 			mockECSClient: func(m *mocks.Mockapi) {
 				m.EXPECT().
 					DescribeClusters(&ecs.DescribeClustersInput{}).
 					Return(nil, errors.New("error"))
 			},
-			wantedError: fmt.Errorf("get default clusters: %s", "error"),
+			wantedError: fmt.Errorf("get default cluster: %s", "error"),
 		},
 	}
 
@@ -376,7 +376,7 @@ func TestECS_DefaultCluster(t *testing.T) {
 			ecs := ECS{
 				client: mockECSClient,
 			}
-			clusters, err := ecs.DefaultClusters()
+			clusters, err := ecs.DefaultCluster()
 			if tc.wantedError != nil {
 				require.EqualError(t, tc.wantedError, err.Error())
 			} else {

internal/pkg/cli/flag.go

@@ -59,10 +59,10 @@ const (
 	memoryFlag         = "memory"
 	imageFlag          = "image"
 	taskRoleFlag       = "task-role"
-	subnetFlag         = "subnet"
+	subnetsFlag        = "subnets"
 	securityGroupsFlag = "security-groups"
 	envVarsFlag        = "env-vars"
-	commandsFlag       = "commands"
+	commandFlag        = "command"
 )
 
 // Short flag names.
@@ -137,14 +137,15 @@ Must be of the format '<keyName>:<dataType>'.`
 	storageLSIConfigFlagDescription = `Optional. Attribute to use as an alternate sort key. May be specified up to 5 times.
 Must be of the format '<keyName>:<dataType>'.`
 
-	countFlagDescription          = "Optional. The number of tasks to set up. Default 1."
-	cpuFlagDescription            = "Optional. The number of CPU units to reserve for each task. Default 256 (1/4 vCPU)."
-	memoryFlagDescription         = "Optional. The amount of memory to reserve in MiB for each task. Default 512."
+	countFlagDescription          = "Optional. The number of tasks to set up."
+	cpuFlagDescription            = "Optional. The number of CPU units to reserve for each task."
+	memoryFlagDescription         = "Optional. The amount of memory to reserve in MiB for each task."
 	imageFlagDescription          = "Optional. The image to run instead of building a Dockerfile."
 	taskRoleFlagDescription       = "Optional. The role for the task to use."
-	subnetFlagDescription         = "Optional. The subnet id for the task to use."
-	securityGroupsFlagDescription = "Optional. The security group id(s) for the task to use. Can be specified multiple times."
+	subnetsFlagDescription        = "Optional. The subnet IDs for the task to use."
+	securityGroupsFlagDescription = "Optional. The security group ID(s) for the task to use. Can be specified multiple times."
 	envVarsFlagDescription        = "Optional. Environment variables specified by key=value separated with commas."
-	commandsFlagDescription       = "Optional. List of commands that are passed to docker run. Can be specified multiple times."
+	commandFlagDescription        = "Optional. The command that is passed to docker run to override the default command."
 	taskGroupFlagDescription      = "The group name of the task. Tasks with the same group name share the same set of resources."
+	taskImageTagDescription       = "Optional. The task's image tag."
 )

internal/pkg/cli/interfaces.go

@@ -144,7 +144,7 @@ type cwlogService interface {
 }
 
 type ecsService interface {
-	DefaultClusters() ([]string, error)
+	DefaultCluster() (string, error)
 }
 
 type templater interface {

internal/pkg/cli/task_run.go

@@ -6,7 +6,6 @@ package cli
 import (
 	"errors"
 	"fmt"
-
 	awscfn "github.com/aws/copilot-cli/internal/pkg/aws/cloudformation"
 	"github.com/aws/copilot-cli/internal/pkg/aws/ec2"
 	"github.com/aws/copilot-cli/internal/pkg/aws/ecr"
@@ -195,7 +194,7 @@ func (o *runTaskOpts) Execute() error {
 		return err
 	}
 
-	if err := o.createAndUpdateTaskResources(); err != nil {
+	if err := o.deployTaskResources(); err != nil {
 		return err
 	}
 
@@ -208,7 +207,7 @@ func (o *runTaskOpts) Execute() error {
 		o.image = fmt.Sprintf(fmtImageURL, uri, o.imageTag)
 
 		// update image to stack
-		if err := o.createAndUpdateTaskResources(); err != nil {
+		if err := o.updateTaskResources(); err != nil {
 			return err
 		}
 	}
@@ -218,11 +217,11 @@ func (o *runTaskOpts) Execute() error {
 
 func (o *runTaskOpts) getCluster() (string, error) {
 	if o.env == config.EnvNameNone {
-		clusters, err := o.ecsGetter.DefaultClusters()
+		cluster, err := o.ecsGetter.DefaultCluster()
 		if err != nil {
 			return "", fmt.Errorf("get default cluster: %w", err)
 		}
-		return clusters[0], nil
+		return cluster, nil
 	}
 
 	clusters, err := o.resourceGetter.GetResourcesByTags(clusterResourceType, map[string]string{
@@ -231,9 +230,12 @@ func (o *runTaskOpts) getCluster() (string, error) {
 	})
 
 	if err != nil {
-		return "", fmt.Errorf("get cluster: %w", err)
+		return "", fmt.Errorf("get cluster by env %s: %w", o.env, err)
 	}
 
+	if len(clusters) == 0 {
+		return "", fmt.Errorf("no cluster found under env %s in app %s", o.env, o.AppName())
+	}
 	return clusters[0], nil
 }
 
@@ -277,27 +279,41 @@ func (o *runTaskOpts) getNetworkConfig() error {
 	return nil
 }
 
-func (o *runTaskOpts) createAndUpdateTaskResources() error {
+func (o *runTaskOpts) deployTaskResources() error {
 	o.spinner.Start(fmt.Sprintf("Deploying resources for task %s", color.HighlightUserInput(o.groupName)))
-	if err := o.resourceDeployer.DeployTask(&deploy.CreateTaskResourcesInput{
-		Name:     o.groupName,
-		Cpu:      o.cpu,
-		Memory:   o.memory,
-		Image:    o.image,
-		TaskRole: o.taskRole,
-		Command:  o.command,
-		EnvVars:  o.envVars,
-	}); err != nil {
+	if err := o.deploy(); err != nil {
 		var errChangeSetEmpty *awscfn.ErrChangeSetEmpty
 		if !errors.As(err, &errChangeSetEmpty) {
 			o.spinner.Stop(log.Serrorln("Failed to deploy task resources."))
 			return fmt.Errorf("failed to deploy resources for task %s: %w", o.groupName, err)
 		}
 	}
-	o.spinner.Stop("\n")
+	o.spinner.Stop(log.Ssuccessln("Successfully deployed task resources."))
 	return nil
 }
 
+func (o *runTaskOpts) updateTaskResources() error {
+	o.spinner.Start(fmt.Sprintf("Updating image to task %s", color.HighlightUserInput(o.groupName)))
+	if err := o.deploy(); err != nil {
+		o.spinner.Stop(log.Serrorln("Failed to update task resources."))
+		return fmt.Errorf("failed to update resources for task %s: %w", o.groupName, err)
+	}
+	o.spinner.Stop(log.Ssuccessln("Successfully updated image to task."))
+	return nil
+}
+
+func (o *runTaskOpts) deploy() error {
+	return o.resourceDeployer.DeployTask(&deploy.CreateTaskResourcesInput{
+		Name:     o.groupName,
+		Cpu:      o.cpu,
+		Memory:   o.memory,
+		Image:    o.image,
+		TaskRole: o.taskRole,
+		Command:  o.command,
+		EnvVars:  o.envVars,
+	})
+}
+
 func (o *runTaskOpts) pushToECRRepo() (string, error) {
 	repoName := fmt.Sprintf(fmtRepoName, o.groupName)
 
@@ -320,7 +336,7 @@ func (o *runTaskOpts) pushToECRRepo() (string, error) {
 	}
 
 	if err := o.docker.Push(uri, o.imageTag); err != nil {
-		return "", fmt.Errorf("push to repo: %w", err)
+		return "", fmt.Errorf("push to repo %s: %w", repoName, err)
 	}
 	return uri, nil
 }
@@ -428,17 +444,17 @@ Run a task with environment variables.
 
 	cmd.Flags().StringVar(&vars.image, imageFlag, "", imageFlagDescription)
 	cmd.Flags().StringVar(&vars.dockerfilePath, dockerFileFlag, "", dockerFileFlagDescription)
-	cmd.Flags().StringVar(&vars.imageTag, imageTagFlag, "", imageFlagDescription)
+	cmd.Flags().StringVar(&vars.imageTag, imageTagFlag, "", taskImageTagDescription)
 
 	cmd.Flags().StringVar(&vars.taskRole, taskRoleFlag, "", taskRoleFlagDescription)
 
 	cmd.Flags().StringVar(&vars.appName, appFlag, "", appFlagDescription)
 	cmd.Flags().StringVar(&vars.env, envFlag, "", envFlagDescription)
-	cmd.Flags().StringSliceVar(&vars.subnets, subnetFlag, nil, subnetFlagDescription)
+	cmd.Flags().StringSliceVar(&vars.subnets, subnetsFlag, nil, subnetsFlagDescription)
 	cmd.Flags().StringSliceVar(&vars.securityGroups, securityGroupsFlag, nil, securityGroupsFlagDescription)
 
 	cmd.Flags().StringToStringVar(&vars.envVars, envVarsFlag, nil, envVarsFlagDescription)
-	cmd.Flags().StringVar(&vars.command, commandsFlag, "", commandsFlagDescription)
+	cmd.Flags().StringVar(&vars.command, commandFlag, "", commandFlagDescription)
 
 	return cmd
 }

internal/pkg/deploy/cloudformation/stack/svc.go

@@ -16,6 +16,10 @@ import (
 	"github.com/aws/copilot-cli/internal/pkg/template"
 )
 
+const (
+	logRetention = "30"
+)
+
 // Template rendering configuration common across services.
 const (
 	svcParamsTemplatePath = "services/params.json.tmpl"
@@ -96,7 +100,7 @@ func (s *svc) Parameters() []*cloudformation.Parameter {
 		},
 		{
 			ParameterKey:   aws.String(ServiceLogRetentionParamKey),
-			ParameterValue: aws.String("30"),
+			ParameterValue: aws.String(logRetention),
 		},
 		{
 			ParameterKey:   aws.String(ServiceAddonsTemplateURLParamKey),

internal/pkg/deploy/cloudformation/stack/task.go

@@ -13,7 +13,6 @@ import (
 
 const (
 	taskTemplatePath = "task/cf.yml"
-	taskLogRetention = "30"
 
 	TaskNameParamKey         = "TaskName"
 	TaskCPUParamKey          = "TaskCPU"
@@ -86,7 +85,7 @@ func (t *taskStackConfig) Parameters() ([]*cloudformation.Parameter, error) {
 		},
 		{
 			ParameterKey:   aws.String(TaskLogRetentionParamKey),
-			ParameterValue: aws.String(taskLogRetention),
+			ParameterValue: aws.String(logRetention),
 		},
 		{
 			ParameterKey:   aws.String(TaskContainerImageParamKey),

internal/pkg/deploy/cloudformation/stack/task_test.go

@@ -86,7 +86,7 @@ func TestTask_Parameters(t *testing.T) {
 		},
 		{
 			ParameterKey:   aws.String(TaskLogRetentionParamKey),
-			ParameterValue: aws.String(taskLogRetention),
+			ParameterValue: aws.String(logRetention),
 		},
 		{
 			ParameterKey:   aws.String(TaskTaskRoleParamKey),

templates/task/cf.yml

@@ -11,7 +11,6 @@ Parameters:
     Type: String
   LogRetention:
     Type: Number
-    Default: 30
   ContainerImage:
     Type: String
   TaskRole:
@@ -41,10 +40,10 @@ Resources:
               awslogs-region: !Ref AWS::Region
               awslogs-group: !Ref LogGroup
               awslogs-stream-prefix: copilot-task
-          Name: !Ref TaskName
-          {{if .EnvVars}}Environment:{{range $name, $value := .EnvVars}}
-          - Name: {{$name}}
-          - Value: {{$value}}{{end}}{{end}}
+          Name: !Ref TaskName{{if .EnvVars}}
+          Environment:{{range $name, $value := .EnvVars}}
+            - Name: {{$name}}
+              Value: {{$value}}{{end}}{{end}}
       Family: !Join ['-', ["copilot", !Ref TaskName]]
       RequiresCompatibilities:
         - "FARGATE"