diff --git a/internal/pkg/aws/ec2/ec2.go b/internal/pkg/aws/ec2/ec2.go index c72bcbec4f1..e95e16b6d89 100644 --- a/internal/pkg/aws/ec2/ec2.go +++ b/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)) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index 97a196435f6..8643e5e5415 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -182,14 +182,14 @@ 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)) @@ -197,7 +197,7 @@ func (e *ECS) DefaultClusters() ([]string, error) { 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 diff --git a/internal/pkg/aws/ecs/ecs_test.go b/internal/pkg/aws/ecs/ecs_test.go index f47b5a06bbd..9b5efcb85e7 100644 --- a/internal/pkg/aws/ecs/ecs_test.go +++ b/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,7 +353,7 @@ 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) { @@ -361,7 +361,7 @@ func TestECS_DefaultCluster(t *testing.T) { 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 { diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index b53455a2850..a4cf630bf12 100644 --- a/internal/pkg/cli/flag.go +++ b/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 ':'.` storageLSIConfigFlagDescription = `Optional. Attribute to use as an alternate sort key. May be specified up to 5 times. Must be of the format ':'.` - 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." ) diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 178f2afd354..bbf40d41dc6 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -144,7 +144,7 @@ type cwlogService interface { } type ecsService interface { - DefaultClusters() ([]string, error) + DefaultCluster() (string, error) } type templater interface { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 2b82df49a66..1ae1060fa53 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -1306,19 +1306,19 @@ func (m *MockecsService) EXPECT() *MockecsServiceMockRecorder { return m.recorder } -// DefaultClusters mocks base method -func (m *MockecsService) DefaultClusters() ([]string, error) { +// DefaultCluster mocks base method +func (m *MockecsService) DefaultCluster() (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DefaultClusters") - ret0, _ := ret[0].([]string) + ret := m.ctrl.Call(m, "DefaultCluster") + ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) return ret0, ret1 } -// DefaultClusters indicates an expected call of DefaultClusters -func (mr *MockecsServiceMockRecorder) DefaultClusters() *gomock.Call { +// DefaultCluster indicates an expected call of DefaultCluster +func (mr *MockecsServiceMockRecorder) DefaultCluster() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DefaultClusters", reflect.TypeOf((*MockecsService)(nil).DefaultClusters)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DefaultCluster", reflect.TypeOf((*MockecsService)(nil).DefaultCluster)) } // Mocktemplater is a mock of templater interface diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 008f425abf5..07347b9a520 100644 --- a/internal/pkg/cli/task_run.go +++ b/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 } diff --git a/internal/pkg/deploy/cloudformation/stack/svc.go b/internal/pkg/deploy/cloudformation/stack/svc.go index 7a5a2607fc6..c29a945afb8 100644 --- a/internal/pkg/deploy/cloudformation/stack/svc.go +++ b/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), diff --git a/internal/pkg/deploy/cloudformation/stack/task.go b/internal/pkg/deploy/cloudformation/stack/task.go index 55622c46b8d..512987b0ef4 100644 --- a/internal/pkg/deploy/cloudformation/stack/task.go +++ b/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), diff --git a/internal/pkg/deploy/cloudformation/stack/task_test.go b/internal/pkg/deploy/cloudformation/stack/task_test.go index a7b72a82291..eaf64205a0b 100644 --- a/internal/pkg/deploy/cloudformation/stack/task_test.go +++ b/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), diff --git a/templates/task/cf.yml b/templates/task/cf.yml index 84972b5c283..6990c8bcba8 100644 --- a/templates/task/cf.yml +++ b/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"