Repository: aws/amazon-ecs-cli-v2
Pull request #416 (2019-11-19 23:20:09)

Ignore empty changesets
This change makes it so we don't fail
if there are no changes to a stack.

It also cleans up empty changesets so they
don't clutter a customer's account.

I yoinked some of the code directly from @sonofachamp :D
<!-- Provide summary of changes -->

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.



Patch:
diff --git a/internal/pkg/deploy/cloudformation/changeset.go b/internal/pkg/deploy/cloudformation/changeset.go
index 5bc96126..e478a184 100644
--- a/internal/pkg/deploy/cloudformation/changeset.go
+++ b/internal/pkg/deploy/cloudformation/changeset.go
@@ -103,6 +103,16 @@ func (set *changeSet) execute() error {
 	return nil
 }
 
+func (set *changeSet) delete() error {
+	if _, err := set.c.DeleteChangeSet(&cloudformation.DeleteChangeSetInput{
+		ChangeSetName: aws.String(set.name),
+		StackName:     aws.String(set.stackID),
+	}); err != nil {
+		return fmt.Errorf("failed to delete changeSet %s: %w", set, err)
+	}
+	return nil
+}
+
 // createChangeSetOpt is a functional option to add additional settings to a CreateChangeSetInput.
 type createChangeSetOpt func(in *cloudformation.CreateChangeSetInput)
 
diff --git a/internal/pkg/deploy/cloudformation/cloudformation.go b/internal/pkg/deploy/cloudformation/cloudformation.go
index 9377cbfe..1e5ab752 100644
--- a/internal/pkg/deploy/cloudformation/cloudformation.go
+++ b/internal/pkg/deploy/cloudformation/cloudformation.go
@@ -285,6 +285,21 @@ func (cf CloudFormation) deployChangeSet(in *cloudformation.CreateChangeSetInput
 		return err
 	}
 	if err := set.waitForCreation(); err != nil {
+		// NOTE: If WaitUntilChangeSetCreateComplete returns an error it's possible that there
+		// are simply no changes between the previous and proposed Stack ChangeSets. We make a call to
+		// DescribeChangeSet to see if that is indeed the case and handle it gracefully.
+		if err := set.describe(); err != nil {
+			return fmt.Errorf("describing failed change set: %w", err)
+		}
+
+		// The change set was empty - so we clean it up and don't return an error.
+		// We have to clean up the changeSet because there's a limit on the number
+		// of failed changesets a customer can have on a particular stack.
+		if len(set.changes) == 0 {
+			set.delete()
+			return nil
+		}
+
 		return err
 	}
 	if err := set.execute(); err != nil {


Test patch:
diff --git a/internal/pkg/deploy/cloudformation/cloudformation_test.go b/internal/pkg/deploy/cloudformation/cloudformation_test.go
index b052befd..1db8d388 100644
--- a/internal/pkg/deploy/cloudformation/cloudformation_test.go
+++ b/internal/pkg/deploy/cloudformation/cloudformation_test.go
@@ -37,6 +37,7 @@ type mockCloudFormation struct {
 	mockDescribeChangeSet                           func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error)
 	mockDescribeStacks                              func(t *testing.T, in *cloudformation.DescribeStacksInput) (*cloudformation.DescribeStacksOutput, error)
 	mockDeleteStack                                 func(t *testing.T, in *cloudformation.DeleteStackInput) (*cloudformation.DeleteStackOutput, error)
+	mockDeleteChangeSet                             func(t *testing.T, in *cloudformation.DeleteChangeSetInput) (*cloudformation.DeleteChangeSetOutput, error)
 	mockCreateStackSet                              func(t *testing.T, in *cloudformation.CreateStackSetInput) (*cloudformation.CreateStackSetOutput, error)
 	mockDescribeStackSet                            func(t *testing.T, in *cloudformation.DescribeStackSetInput) (*cloudformation.DescribeStackSetOutput, error)
 	mockUpdateStackSet                              func(t *testing.T, in *cloudformation.UpdateStackSetInput) (*cloudformation.UpdateStackSetOutput, error)
@@ -63,6 +64,10 @@ func (cf mockCloudFormation) DeleteStack(in *cloudformation.DeleteStackInput) (*
 	return cf.mockDeleteStack(cf.t, in)
 }
 
+func (cf mockCloudFormation) DeleteChangeSet(in *cloudformation.DeleteChangeSetInput) (*cloudformation.DeleteChangeSetOutput, error) {
+	return cf.mockDeleteChangeSet(cf.t, in)
+}
+
 func (cf mockCloudFormation) DescribeChangeSet(in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error) {
 	return cf.mockDescribeChangeSet(cf.t, in)
 }
@@ -468,6 +473,15 @@ func TestDeploy(t *testing.T) {
 					mockWaitUntilChangeSetCreateCompleteWithContext: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) error {
 						return errors.New("some AWS error")
 					},
+					mockDescribeChangeSet: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error) {
+						return &cloudformation.DescribeChangeSetOutput{
+							ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
+							StatusReason:    aws.String(noUpdatesReason),
+							Changes: []*cloudformation.Change{
+								&cloudformation.Change{},
+							},
+						}, nil
+					},
 				},
 				box: boxWithTemplateFile(),
 			},
@@ -513,6 +527,9 @@ func TestDeploy(t *testing.T) {
 						return &cloudformation.DescribeChangeSetOutput{
 							ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
 							StatusReason:    aws.String(noChangesReason),
+							Changes: []*cloudformation.Change{
+								&cloudformation.Change{},
+							},
 						}, nil
 					},
 				},
@@ -538,6 +555,9 @@ func TestDeploy(t *testing.T) {
 						return &cloudformation.DescribeChangeSetOutput{
 							ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
 							StatusReason:    aws.String(noUpdatesReason),
+							Changes: []*cloudformation.Change{
+								&cloudformation.Change{},
+							},
 						}, nil
 					},
 				},
@@ -563,6 +583,9 @@ func TestDeploy(t *testing.T) {
 						return &cloudformation.DescribeChangeSetOutput{
 							ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
 							StatusReason:    aws.String("some other reason"),
+							Changes: []*cloudformation.Change{
+								&cloudformation.Change{},
+							},
 						}, nil
 					},
 				},
@@ -594,6 +617,9 @@ func TestDeploy(t *testing.T) {
 					mockDescribeChangeSet: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error) {
 						return &cloudformation.DescribeChangeSetOutput{
 							ExecutionStatus: aws.String(cloudformation.ExecutionStatusAvailable),
+							Changes: []*cloudformation.Change{
+								&cloudformation.Change{},
+							},
 						}, nil
 					},
 					mockExecuteChangeSet: func(t *testing.T, in *cloudformation.ExecuteChangeSetInput) (output *cloudformation.ExecuteChangeSetOutput, e error) {
@@ -604,6 +630,42 @@ func TestDeploy(t *testing.T) {
 			},
 			want: fmt.Errorf("failed to execute changeSet %s: %s", fmt.Sprintf("name=%s, stackID=%s", mockChangeSetID, mockStackID), "some AWS error"),
 		},
+		"should gracefully skip deploys when there are no changes and clean up failed changeset": {
+			cf: CloudFormation{
+				client: &mockCloudFormation{
+					t: t,
+					mockCreateChangeSet: func(t *testing.T, in *cloudformation.CreateChangeSetInput) (*cloudformation.CreateChangeSetOutput, error) {
+						require.Equal(t, mockStackConfig.StackName(), *in.StackName)
+						require.True(t, isValidChangeSetName(*in.ChangeSetName))
+						require.Equal(t, mockTemplate, *in.TemplateBody)
+
+						return &cloudformation.CreateChangeSetOutput{
+							Id:      aws.String(mockChangeSetID),
+							StackId: aws.String(mockStackID),
+						}, nil
+					},
+					mockWaitUntilChangeSetCreateCompleteWithContext: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) error {
+						require.Equal(t, mockStackID, *in.StackName)
+						require.Equal(t, mockChangeSetID, *in.ChangeSetName)
+						return fmt.Errorf("No changes")
+					},
+					mockDescribeChangeSet: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error) {
+						return &cloudformation.DescribeChangeSetOutput{
+							ExecutionStatus: aws.String(cloudformation.ExecutionStatusUnavailable),
+							Changes:         []*cloudformation.Change{},
+						}, nil
+					},
+					mockDeleteChangeSet: func(t *testing.T, in *cloudformation.DeleteChangeSetInput) (*cloudformation.DeleteChangeSetOutput, error) {
+						require.Equal(t, mockStackID, *in.StackName)
+						require.Equal(t, mockChangeSetID, *in.ChangeSetName)
+						return &cloudformation.DeleteChangeSetOutput{}, nil
+					},
+				},
+				box: boxWithTemplateFile(),
+			},
+			input: mockStackConfig,
+			want:  nil,
+		},
 		"should deploy": {
 			cf: CloudFormation{
 				client: &mockCloudFormation{
@@ -626,6 +688,9 @@ func TestDeploy(t *testing.T) {
 					mockDescribeChangeSet: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error) {
 						return &cloudformation.DescribeChangeSetOutput{
 							ExecutionStatus: aws.String(cloudformation.ExecutionStatusAvailable),
+							Changes: []*cloudformation.Change{
+								&cloudformation.Change{},
+							},
 						}, nil
 					},
 					mockExecuteChangeSet: func(t *testing.T, in *cloudformation.ExecuteChangeSetInput) (output *cloudformation.ExecuteChangeSetOutput, e error) {