Repository: aws/amazon-ecs-cli-v2 (go)

Issue:
Pipeline update can't complete if trying to update an unchanged pipeline
Stuck at `Proposing infrastructure changes for the pipeline` when trying to deploy an unchanged pipeline.

However, it won't get stuck if the pipeline already got updated with a change and then do an update with no change.

Pull request description:
Stop waiting for pipeline update completion if there are no updates
Fixes #465 

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/cloudformation.go b/internal/pkg/deploy/cloudformation/cloudformation.go
index b90dc754..c87ef28f 100644
--- a/internal/pkg/deploy/cloudformation/cloudformation.go
+++ b/internal/pkg/deploy/cloudformation/cloudformation.go
@@ -220,9 +220,9 @@ func (cf CloudFormation) create(stackConfig stackConfiguration) error {
 	}
 }
 
-// update will update a given stack, so long as the stack already exists
-// and it isn't already deploying something. If there's already some action
-// happening on this stack, we'll return ErrStackUpdateInProgress.
+// update will update a given stack, so long as the stack already exists and it isn't already deploying something.
+// If there's already some action happening on this stack, it returns an ErrStackUpdateInProgress.
+// If there are no changes in the stack, it returns an errChangeSetEmpty.
 func (cf CloudFormation) update(stackConfig stackConfiguration) error {
 	describeStackInput := &cloudformation.DescribeStacksInput{StackName: aws.String(stackConfig.StackName())}
 	existingStack, err := cf.describeStack(describeStackInput)
@@ -238,7 +238,6 @@ func (cf CloudFormation) update(stackConfig stackConfiguration) error {
 			stackStatus: aws.StringValue(existingStack.StackStatus),
 		}
 	}
-
 	return cf.deploy(stackConfig, cloudformation.ChangeSetTypeUpdate)
 }
 
@@ -295,7 +294,7 @@ func (cf CloudFormation) deployChangeSet(in *cloudformation.CreateChangeSetInput
 		// of failed changesets a customer can have on a particular stack.
 		if len(set.changes) == 0 {
 			set.delete()
-			return nil
+			return errChangeSetEmpty
 		}
 
 		return err
diff --git a/internal/pkg/deploy/cloudformation/errors.go b/internal/pkg/deploy/cloudformation/errors.go
index 7de823b4..996453ab 100644
--- a/internal/pkg/deploy/cloudformation/errors.go
+++ b/internal/pkg/deploy/cloudformation/errors.go
@@ -96,3 +96,6 @@ func (err *ErrStackSetOutOfDate) Is(target error) bool {
 	}
 	return err.projectName == t.projectName
 }
+
+// errChangeSetEmpty occurs when the change set  does not contain any new or updated resources.
+var errChangeSetEmpty = errors.New("change set has no changes")
diff --git a/internal/pkg/deploy/cloudformation/pipeline.go b/internal/pkg/deploy/cloudformation/pipeline.go
index dc2ed3af..3096b360 100644
--- a/internal/pkg/deploy/cloudformation/pipeline.go
+++ b/internal/pkg/deploy/cloudformation/pipeline.go
@@ -30,6 +30,7 @@ func (cf CloudFormation) DeployPipeline(in *deploy.CreatePipelineInput) error {
 		if err != nil {
 			return err
 		}
+		return nil
 	}
 
 	// If the stack already exists - we update it
@@ -39,6 +40,10 @@ func (cf CloudFormation) DeployPipeline(in *deploy.CreatePipelineInput) error {
 	}
 
 	if err := cf.update(pipelineConfig); err != nil {
+		if err == errChangeSetEmpty {
+			// If there are no updates, then exit successfully.
+			return nil
+		}
 		return fmt.Errorf("updating pipeline: %w", err)
 	}
 


Test patch:
diff --git a/internal/pkg/deploy/cloudformation/cloudformation_test.go b/internal/pkg/deploy/cloudformation/cloudformation_test.go
index 1db8d388..daedfb4a 100644
--- a/internal/pkg/deploy/cloudformation/cloudformation_test.go
+++ b/internal/pkg/deploy/cloudformation/cloudformation_test.go
@@ -630,7 +630,7 @@ 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": {
+		"should clean up failed changeset and return errChangeSetEmpty when there are no changes": {
 			cf: CloudFormation{
 				client: &mockCloudFormation{
 					t: t,
@@ -664,7 +664,7 @@ func TestDeploy(t *testing.T) {
 				box: boxWithTemplateFile(),
 			},
 			input: mockStackConfig,
-			want:  nil,
+			want:  errChangeSetEmpty,
 		},
 		"should deploy": {
 			cf: CloudFormation{
diff --git a/internal/pkg/deploy/cloudformation/pipeline_test.go b/internal/pkg/deploy/cloudformation/pipeline_test.go
new file mode 100644
index 00000000..0ce57db4
--- /dev/null
+++ b/internal/pkg/deploy/cloudformation/pipeline_test.go
@@ -0,0 +1,104 @@
+// Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package cloudformation
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/deploy"
+	"github.com/aws/amazon-ecs-cli-v2/internal/pkg/manifest"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/cloudformation"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCloudFormation_DeployPipeline(t *testing.T) {
+	testCases := map[string]struct {
+		in *deploy.CreatePipelineInput
+
+		mockDescribeStacks                              func(t *testing.T, in *cloudformation.DescribeStacksInput) (*cloudformation.DescribeStacksOutput, error)
+		mockCreateChangeSet                             func(t *testing.T, in *cloudformation.CreateChangeSetInput) (*cloudformation.CreateChangeSetOutput, error)
+		mockWaitUntilChangeSetCreateCompleteWithContext func(t *testing.T, in *cloudformation.DescribeChangeSetInput) error
+		mockDescribeChangeSet                           func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error)
+		mockDeleteChangeSet                             func(t *testing.T, in *cloudformation.DeleteChangeSetInput) (*cloudformation.DeleteChangeSetOutput, error)
+
+		wantedError error
+	}{
+		"deletes changeset and successfully exists if a stack update has no changes": {
+			in: &deploy.CreatePipelineInput{
+				ProjectName: "phonetool",
+				Name:        "phonetool-pipeline",
+				Source: &deploy.Source{
+					ProviderName: manifest.GithubProviderName,
+					Properties: map[string]interface{}{
+						manifest.GithubSecretIdKeyName: "my secret",
+						"repository":                   "github.com/hello/phonetool",
+					},
+				},
+				Stages:          nil,
+				ArtifactBuckets: nil,
+			},
+
+			mockDescribeStacks: func(t *testing.T, in *cloudformation.DescribeStacksInput) (*cloudformation.DescribeStacksOutput, error) {
+				// Stack already exists.
+				return &cloudformation.DescribeStacksOutput{
+					Stacks: []*cloudformation.Stack{
+						{
+							StackStatus: aws.String(cloudformation.StackStatusCreateComplete),
+						},
+					},
+				}, nil
+			},
+			mockCreateChangeSet: func(t *testing.T, in *cloudformation.CreateChangeSetInput) (*cloudformation.CreateChangeSetOutput, error) {
+				return &cloudformation.CreateChangeSetOutput{
+					Id:      aws.String("1234"),
+					StackId: aws.String("phonetool-1234"),
+				}, nil
+			},
+			mockWaitUntilChangeSetCreateCompleteWithContext: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) error {
+				return errors.New("some changeset error")
+			},
+			mockDescribeChangeSet: func(t *testing.T, in *cloudformation.DescribeChangeSetInput) (*cloudformation.DescribeChangeSetOutput, error) {
+				return &cloudformation.DescribeChangeSetOutput{
+					Changes:         []*cloudformation.Change{},
+					ExecutionStatus: aws.String(cloudformation.ExecutionStatusAvailable),
+					StatusReason:    aws.String(""),
+				}, nil
+			},
+			mockDeleteChangeSet: func(t *testing.T, in *cloudformation.DeleteChangeSetInput) (*cloudformation.DeleteChangeSetOutput, error) {
+				require.Equal(t, "1234", *in.ChangeSetName)
+				require.Equal(t, "phonetool-1234", *in.StackName)
+				return &cloudformation.DeleteChangeSetOutput{}, nil
+			},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			// GIVEN
+			cf := CloudFormation{
+				client: &mockCloudFormation{
+					t:                   t,
+					mockDescribeStacks:  tc.mockDescribeStacks,
+					mockCreateChangeSet: tc.mockCreateChangeSet,
+					mockWaitUntilChangeSetCreateCompleteWithContext: tc.mockWaitUntilChangeSetCreateCompleteWithContext,
+					mockDescribeChangeSet:                           tc.mockDescribeChangeSet,
+					mockDeleteChangeSet:                             tc.mockDeleteChangeSet,
+				},
+				box: boxWithTemplateFile(),
+			}
+
+			// WHEN
+			err := cf.DeployPipeline(tc.in)
+
+			// THEN
+			if tc.wantedError != nil {
+				require.EqualError(t, err, tc.wantedError.Error())
+			} else {
+				require.Nil(t, err)
+			}
+		})
+	}
+}