Skip to content

Commit d5a3021

Browse files
authored
feat(cli): enable progress tracker for "task run" (#1891)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
1 parent 605a343 commit d5a3021

File tree

11 files changed

+103
-124
lines changed

11 files changed

+103
-124
lines changed

internal/pkg/cli/interfaces.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ type appResourcesGetter interface {
346346
}
347347

348348
type taskDeployer interface {
349-
DeployTask(input *deploy.CreateTaskResourcesInput, opts ...cloudformation.StackOption) error
349+
DeployTask(out termprogress.FileWriter, input *deploy.CreateTaskResourcesInput, opts ...cloudformation.StackOption) error
350350
}
351351

352352
type taskStackManager interface {

internal/pkg/cli/job_deploy.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,6 @@ func (o *deployJobOpts) deployJob(addonsURL string) error {
273273
return err
274274
}
275275
if err := o.jobCFN.DeployService(os.Stderr, conf, awscloudformation.WithRoleARN(o.targetEnvironment.ExecutionRoleARN)); err != nil {
276-
o.spinner.Stop(log.Serrorf("Failed to deploy job.\n\n"))
277276
return fmt.Errorf("deploy job: %w", err)
278277
}
279278
log.Successf("Deployed %s.\n", color.HighlightUserInput(o.name))

internal/pkg/cli/mocks/mock_interfaces.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/pkg/cli/task_delete.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,10 @@ func BuildTaskDeleteCmd() *cobra.Command {
476476
return err
477477
}
478478

479+
if len(opts.RecommendedActions()) == 0 {
480+
return nil
481+
}
482+
479483
log.Infoln("Recommended follow-up actions:")
480484
for _, followup := range opts.RecommendedActions() {
481485
log.Infof("- %s\n", followup)

internal/pkg/cli/task_run.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -463,22 +463,16 @@ func (o *runTaskOpts) buildAndPushImage() error {
463463
}
464464

465465
func (o *runTaskOpts) deployTaskResources() error {
466-
o.spinner.Start(fmt.Sprintf("Provisioning resources and permissions for task %s.", color.HighlightUserInput(o.groupName)))
467466
if err := o.deploy(); err != nil {
468-
o.spinner.Stop(log.Serror("Failed to provision task resources.\n\n"))
469467
return fmt.Errorf("provision resources for task %s: %w", o.groupName, err)
470468
}
471-
o.spinner.Stop(log.Ssuccess("Successfully provisioned task resources.\n\n"))
472469
return nil
473470
}
474471

475472
func (o *runTaskOpts) updateTaskResources() error {
476-
o.spinner.Start(fmt.Sprintf("Updating image to task %s.", color.HighlightUserInput(o.groupName)))
477473
if err := o.deploy(); err != nil {
478-
o.spinner.Stop(log.Serror("Failed to update task resources.\n\n"))
479474
return fmt.Errorf("update resources for task %s: %w", o.groupName, err)
480475
}
481-
o.spinner.Stop(log.Ssuccess("Successfully updated image to task.\n\n"))
482476
return nil
483477
}
484478

@@ -504,7 +498,7 @@ func (o *runTaskOpts) deploy() error {
504498
Env: o.env,
505499
AdditionalTags: o.resourceTags,
506500
}
507-
return o.deployer.DeployTask(input, deployOpts...)
501+
return o.deployer.DeployTask(os.Stderr, input, deployOpts...)
508502
}
509503

510504
func (o *runTaskOpts) validateAppName() error {

internal/pkg/cli/task_run_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ func TestTaskRunOpts_Execute(t *testing.T) {
591591
setupMocks: func(m runTaskMocks) {
592592
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).AnyTimes()
593593
m.defaultClusterGetter.EXPECT().HasDefaultCluster().Return(true, nil)
594-
m.deployer.EXPECT().DeployTask(gomock.Any()).Return(nil).AnyTimes()
594+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
595595
mockRepositoryAnytime(m)
596596
m.runner.EXPECT().Run().AnyTimes()
597597
},
@@ -605,15 +605,15 @@ func TestTaskRunOpts_Execute(t *testing.T) {
605605
Return(&config.Environment{
606606
ExecutionRoleARN: "env execution role",
607607
}, nil)
608-
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
608+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
609609
mockRepositoryAnytime(m)
610610
m.runner.EXPECT().Run().AnyTimes()
611611
},
612612
},
613613
"error deploying resources": {
614614
setupMocks: func(m runTaskMocks) {
615615
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).AnyTimes()
616-
m.deployer.EXPECT().DeployTask(&deploy.CreateTaskResourcesInput{
616+
m.deployer.EXPECT().DeployTask(gomock.Any(), &deploy.CreateTaskResourcesInput{
617617
Name: inGroupName,
618618
Image: "",
619619
Command: []string{},
@@ -625,14 +625,14 @@ func TestTaskRunOpts_Execute(t *testing.T) {
625625
"error updating resources": {
626626
setupMocks: func(m runTaskMocks) {
627627
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).AnyTimes()
628-
m.deployer.EXPECT().DeployTask(&deploy.CreateTaskResourcesInput{
628+
m.deployer.EXPECT().DeployTask(gomock.Any(), &deploy.CreateTaskResourcesInput{
629629
Name: inGroupName,
630630
Image: "",
631631
Command: []string{},
632632
}).Return(nil)
633633
m.repository.EXPECT().BuildAndPush(gomock.Any(), gomock.Eq(&defaultBuildArguments))
634634
m.repository.EXPECT().URI().Return(mockRepoURI)
635-
m.deployer.EXPECT().DeployTask(&deploy.CreateTaskResourcesInput{
635+
m.deployer.EXPECT().DeployTask(gomock.Any(), &deploy.CreateTaskResourcesInput{
636636
Name: inGroupName,
637637
Image: "uri/repo:latest",
638638
Command: []string{},
@@ -644,7 +644,7 @@ func TestTaskRunOpts_Execute(t *testing.T) {
644644
"error running tasks": {
645645
setupMocks: func(m runTaskMocks) {
646646
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).AnyTimes()
647-
m.deployer.EXPECT().DeployTask(gomock.Any()).Return(nil).Times(2)
647+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any()).Return(nil).Times(2)
648648
mockRepositoryAnytime(m)
649649
m.runner.EXPECT().Run().Return(nil, errors.New("error running"))
650650
mockHasDefaultCluster(m)
@@ -658,7 +658,7 @@ func TestTaskRunOpts_Execute(t *testing.T) {
658658
Return(&config.Environment{
659659
ExecutionRoleARN: "env execution role",
660660
}, nil)
661-
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Len(1)).AnyTimes() // NOTE: matching length because gomock is unable to match function arguments.
661+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any(), gomock.Len(1)).AnyTimes() // NOTE: matching length because gomock is unable to match function arguments.
662662
mockRepositoryAnytime(m)
663663
m.runner.EXPECT().Run().AnyTimes()
664664
m.defaultClusterGetter.EXPECT().HasDefaultCluster().Times(0)
@@ -667,7 +667,7 @@ func TestTaskRunOpts_Execute(t *testing.T) {
667667
"deploy without execution role option if env is empty": {
668668
setupMocks: func(m runTaskMocks) {
669669
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).Times(0)
670-
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Len(0)).AnyTimes() // NOTE: matching length because gomock is unable to match function arguments.
670+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any(), gomock.Len(0)).AnyTimes() // NOTE: matching length because gomock is unable to match function arguments.
671671
mockRepositoryAnytime(m)
672672
m.runner.EXPECT().Run().AnyTimes()
673673
mockHasDefaultCluster(m)
@@ -677,7 +677,7 @@ func TestTaskRunOpts_Execute(t *testing.T) {
677677
inTag: tag,
678678
setupMocks: func(m runTaskMocks) {
679679
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).AnyTimes()
680-
m.deployer.EXPECT().DeployTask(gomock.Any()).AnyTimes()
680+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any()).AnyTimes()
681681
m.repository.EXPECT().BuildAndPush(gomock.Any(), gomock.Eq(
682682
&exec.BuildArguments{
683683
Context: filepath.Dir(defaultDockerfilePath),
@@ -694,14 +694,14 @@ func TestTaskRunOpts_Execute(t *testing.T) {
694694
inCommand: `/bin/sh -c "curl $ECS_CONTAINER_METADATA_URI_V4"`,
695695
setupMocks: func(m runTaskMocks) {
696696
m.store.EXPECT().GetEnvironment(gomock.Any(), gomock.Any()).AnyTimes()
697-
m.deployer.EXPECT().DeployTask(&deploy.CreateTaskResourcesInput{
697+
m.deployer.EXPECT().DeployTask(gomock.Any(), &deploy.CreateTaskResourcesInput{
698698
Name: inGroupName,
699699
Image: "",
700700
Command: []string{"/bin/sh", "-c", "curl $ECS_CONTAINER_METADATA_URI_V4"},
701701
}).Times(1).Return(nil)
702702
m.repository.EXPECT().BuildAndPush(gomock.Any(), gomock.Eq(&defaultBuildArguments))
703703
m.repository.EXPECT().URI().Return(mockRepoURI)
704-
m.deployer.EXPECT().DeployTask(&deploy.CreateTaskResourcesInput{
704+
m.deployer.EXPECT().DeployTask(gomock.Any(), &deploy.CreateTaskResourcesInput{
705705
Name: inGroupName,
706706
Image: "uri/repo:latest",
707707
Command: []string{"/bin/sh", "-c", "curl $ECS_CONTAINER_METADATA_URI_V4"},
@@ -714,7 +714,7 @@ func TestTaskRunOpts_Execute(t *testing.T) {
714714
inFollow: true,
715715
inImage: "image",
716716
setupMocks: func(m runTaskMocks) {
717-
m.deployer.EXPECT().DeployTask(gomock.Any()).AnyTimes()
717+
m.deployer.EXPECT().DeployTask(gomock.Any(), gomock.Any()).AnyTimes()
718718
m.runner.EXPECT().Run().Return([]*task.Task{
719719
{
720720
TaskARN: "task-1",

internal/pkg/deploy/cloudformation/cloudformation.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,12 @@ func (cf CloudFormation) newRenderWorkloadInput(w progress.FileWriter, stack *cl
151151
in.stackDescription = fmt.Sprintf("Updating the infrastructure for stack %s", stack.Name)
152152
changeSetID, err = cf.cfnClient.Update(stack)
153153
if err != nil {
154-
spinner.Stop(log.Serrorf("%s\n", label))
154+
msg := log.Serrorf("%s\n", label)
155+
var errChangeSetEmpty *cloudformation.ErrChangeSetEmpty
156+
if errors.As(err, &errChangeSetEmpty) {
157+
msg = fmt.Sprintf("- No new infrastructure changes for stack %s\n", stack.Name)
158+
}
159+
spinner.Stop(msg)
155160
return "", cf.handleStackError(stack.Name, err)
156161
}
157162
spinner.Stop(log.Ssuccessf("%s\n", label))

internal/pkg/deploy/cloudformation/cloudformation_test.go

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package cloudformation
55

66
import (
77
"errors"
8+
"fmt"
89
"io"
910
"strings"
1011
"testing"
@@ -45,6 +46,25 @@ func testDeployWorkload_OnCreateChangeSetFailure(t *testing.T, when func(w progr
4546
require.True(t, errors.Is(err, wantedErr), `expected returned error to be wrapped with "some error"`)
4647
}
4748

49+
func testDeployWorkload_ReturnNilOnEmptyChangeSetWhileUpdatingStack(t *testing.T, when func(w progress.FileWriter, cf CloudFormation) error) {
50+
// GIVEN
51+
ctrl := gomock.NewController(t)
52+
defer ctrl.Finish()
53+
wantedErr := &cloudformation.ErrChangeSetEmpty{}
54+
m := mocks.NewMockcfnClient(ctrl)
55+
m.EXPECT().Create(gomock.Any()).Return("", &cloudformation.ErrStackAlreadyExists{})
56+
m.EXPECT().Update(gomock.Any()).Return("", wantedErr)
57+
m.EXPECT().ErrorEvents(gomock.Any()).Return(nil, nil)
58+
client := CloudFormation{cfnClient: m}
59+
buf := new(strings.Builder)
60+
61+
// WHEN
62+
err := when(mockFileWriter{Writer: buf}, client)
63+
64+
// THEN
65+
require.Nil(t, err, "should not fail if the changeset is empty")
66+
}
67+
4868
func testDeployWorkload_OnUpdateChangeSetFailure(t *testing.T, when func(w progress.FileWriter, cf CloudFormation) error) {
4969
// GIVEN
5070
ctrl := gomock.NewController(t)
@@ -119,7 +139,7 @@ func testDeployWorkload_StackStreamerFailureShouldCancelRenderer(t *testing.T, w
119139
require.True(t, errors.Is(err, wantedErr), "expected streamer error to be wrapped and returned")
120140
}
121141

122-
func testDeployWorkload_StreamUntilStackCreationFails(t *testing.T, when func(w progress.FileWriter, cf CloudFormation) error) {
142+
func testDeployWorkload_StreamUntilStackCreationFails(t *testing.T, stackName string, when func(w progress.FileWriter, cf CloudFormation) error) {
123143
// GIVEN
124144
ctrl := gomock.NewController(t)
125145
defer ctrl.Finish()
@@ -131,14 +151,14 @@ func testDeployWorkload_StreamUntilStackCreationFails(t *testing.T, when func(w
131151
StackEvents: []*sdkcloudformation.StackEvent{
132152
{
133153
EventId: aws.String("2"),
134-
LogicalResourceId: aws.String("hello"),
154+
LogicalResourceId: aws.String(stackName),
135155
PhysicalResourceId: aws.String("AWS::CloudFormation::Stack"),
136156
ResourceStatus: aws.String("CREATE_FAILED"), // Send failure event for stack.
137157
Timestamp: aws.Time(time.Now()),
138158
},
139159
},
140160
}, nil).AnyTimes()
141-
m.EXPECT().Describe("hello").Return(&cloudformation.StackDescription{
161+
m.EXPECT().Describe(stackName).Return(&cloudformation.StackDescription{
142162
StackStatus: aws.String("CREATE_FAILED"),
143163
}, nil)
144164
client := CloudFormation{cfnClient: m}
@@ -148,10 +168,10 @@ func testDeployWorkload_StreamUntilStackCreationFails(t *testing.T, when func(w
148168
err := when(mockFileWriter{Writer: buf}, client)
149169

150170
// THEN
151-
require.EqualError(t, err, "stack hello did not complete successfully and exited with status CREATE_FAILED")
171+
require.EqualError(t, err, fmt.Sprintf("stack %s did not complete successfully and exited with status CREATE_FAILED", stackName))
152172
}
153173

154-
func testDeployWorkload_RenderNewlyCreatedStackWithECSService(t *testing.T, when func(w progress.FileWriter, cf CloudFormation) error) {
174+
func testDeployWorkload_RenderNewlyCreatedStackWithECSService(t *testing.T, stackName string, when func(w progress.FileWriter, cf CloudFormation) error) {
155175
// GIVEN
156176
ctrl := gomock.NewController(t)
157177
defer ctrl.Finish()
@@ -160,7 +180,7 @@ func testDeployWorkload_RenderNewlyCreatedStackWithECSService(t *testing.T, when
160180
deploymentTime := time.Date(2020, time.November, 23, 18, 0, 0, 0, time.UTC)
161181

162182
mockCFN.EXPECT().Create(gomock.Any()).Return("1234", nil)
163-
mockCFN.EXPECT().DescribeChangeSet("1234", "hello").Return(&cloudformation.ChangeSetDescription{
183+
mockCFN.EXPECT().DescribeChangeSet("1234", stackName).Return(&cloudformation.ChangeSetDescription{
164184
Changes: []*sdkcloudformation.Change{
165185
{
166186
ResourceChange: &sdkcloudformation.ResourceChange{
@@ -170,15 +190,15 @@ func testDeployWorkload_RenderNewlyCreatedStackWithECSService(t *testing.T, when
170190
},
171191
},
172192
}, nil)
173-
mockCFN.EXPECT().TemplateBodyFromChangeSet("1234", "hello").Return(`
193+
mockCFN.EXPECT().TemplateBodyFromChangeSet("1234", stackName).Return(`
174194
Resources:
175195
Service:
176196
Metadata:
177197
'aws:copilot:description': 'My ECS Service'
178198
Type: AWS::ECS::Service
179199
`, nil)
180200
mockCFN.EXPECT().DescribeStackEvents(&sdkcloudformation.DescribeStackEventsInput{
181-
StackName: aws.String("hello"),
201+
StackName: aws.String(stackName),
182202
}).Return(&sdkcloudformation.DescribeStackEventsOutput{
183203
StackEvents: []*sdkcloudformation.StackEvent{
184204
{
@@ -199,7 +219,7 @@ Resources:
199219
},
200220
{
201221
EventId: aws.String("3"),
202-
LogicalResourceId: aws.String("hello"),
222+
LogicalResourceId: aws.String(stackName),
203223
ResourceType: aws.String("AWS::CloudFormation::Stack"),
204224
ResourceStatus: aws.String("CREATE_COMPLETE"),
205225
Timestamp: aws.Time(deploymentTime),
@@ -216,7 +236,7 @@ Resources:
216236
},
217237
},
218238
}, nil)
219-
mockCFN.EXPECT().Describe("hello").Return(&cloudformation.StackDescription{
239+
mockCFN.EXPECT().Describe(stackName).Return(&cloudformation.StackDescription{
220240
StackStatus: aws.String("CREATE_COMPLETE"),
221241
}, nil)
222242
client := CloudFormation{cfnClient: mockCFN, ecsClient: mockECS}
@@ -232,14 +252,14 @@ Resources:
232252
require.Contains(t, buf.String(), "[completed]", "Rollout state of service should be rendered")
233253
}
234254

235-
func testDeployWorkload_RenderNewlyCreatedStackWithAddons(t *testing.T, when func(w progress.FileWriter, cf CloudFormation) error) {
255+
func testDeployWorkload_RenderNewlyCreatedStackWithAddons(t *testing.T, stackName string, when func(w progress.FileWriter, cf CloudFormation) error) {
236256
ctrl := gomock.NewController(t)
237257
defer ctrl.Finish()
238258
m := mocks.NewMockcfnClient(ctrl)
239259

240260
// Mocks for the parent stack.
241261
m.EXPECT().Create(gomock.Any()).Return("1234", nil)
242-
m.EXPECT().DescribeChangeSet("1234", "hello").Return(&cloudformation.ChangeSetDescription{
262+
m.EXPECT().DescribeChangeSet("1234", stackName).Return(&cloudformation.ChangeSetDescription{
243263
Changes: []*sdkcloudformation.Change{
244264
{
245265
ResourceChange: &sdkcloudformation.ResourceChange{
@@ -257,7 +277,7 @@ func testDeployWorkload_RenderNewlyCreatedStackWithAddons(t *testing.T, when fun
257277
},
258278
}, nil)
259279

260-
m.EXPECT().TemplateBodyFromChangeSet("1234", "hello").Return(`
280+
m.EXPECT().TemplateBodyFromChangeSet("1234", stackName).Return(`
261281
Resources:
262282
Cluster:
263283
Metadata:
@@ -270,7 +290,7 @@ Resources:
270290
`, nil)
271291

272292
m.EXPECT().DescribeStackEvents(&sdkcloudformation.DescribeStackEventsInput{
273-
StackName: aws.String("hello"),
293+
StackName: aws.String(stackName),
274294
}).Return(&sdkcloudformation.DescribeStackEventsOutput{
275295
StackEvents: []*sdkcloudformation.StackEvent{
276296
{
@@ -289,15 +309,15 @@ Resources:
289309
},
290310
{
291311
EventId: aws.String("3"),
292-
LogicalResourceId: aws.String("hello"),
312+
LogicalResourceId: aws.String(stackName),
293313
PhysicalResourceId: aws.String("AWS::CloudFormation::Stack"),
294314
ResourceStatus: aws.String("CREATE_COMPLETE"),
295315
Timestamp: aws.Time(time.Now()),
296316
},
297317
},
298318
}, nil).AnyTimes()
299319

300-
m.EXPECT().Describe("hello").Return(&cloudformation.StackDescription{
320+
m.EXPECT().Describe(stackName).Return(&cloudformation.StackDescription{
301321
StackStatus: aws.String("CREATE_COMPLETE"),
302322
}, nil)
303323

0 commit comments

Comments
 (0)