From 4a4e0c722128fe2128afb0daaf77a63e183f6bd5 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Wed, 21 Aug 2024 11:19:07 +1000 Subject: [PATCH] [Resolve #723] Fix update command with change sets for multiple stacks (#1480) * [Resolves #723] Fix update command with change sets for multiple stacks Previously the update command would exit if any change sets status was not equal to READY. However, when a stack does not contain any updates, it will not be READY since there is nothing to execute. This should not prevent other change sets to be executed. To get around this, we introduce another change set status, NO_CHANGES, and handle that gracefully. To clean up the output a bit, we also pass on describing the changes for empty change sets. Continuation of #917. Thanks @henrist for the original PR and @jfalkenstein for the review: Fix original PR feedback Simplify code for skipping output for empty change set Roll back optional delete, we should always clean up Add test covering "change set" version of update command --------- Co-authored-by: Henrik Steen --- sceptre/cli/update.py | 30 ++++++-- sceptre/plan/actions.py | 7 ++ sceptre/stack_status.py | 1 + tests/test_actions.py | 9 +++ tests/test_cli/test_cli_commands.py | 112 +++++++++++++++++++++++++++- 5 files changed, 152 insertions(+), 7 deletions(-) diff --git a/sceptre/cli/update.py b/sceptre/cli/update.py index 35d4239ae..f685ecc68 100644 --- a/sceptre/cli/update.py +++ b/sceptre/cli/update.py @@ -61,14 +61,33 @@ def update_command( try: # Wait for change set to be created statuses = plan.wait_for_cs_completion(change_set_name) - # Exit if change set fails to create - for status in list(statuses.values()): - if status != StackChangeSetStatus.READY: + + at_least_one_ready = False + + for status in statuses.values(): + # Exit if change set fails to create + if status not in ( + StackChangeSetStatus.READY, + StackChangeSetStatus.NO_CHANGES, + ): + write("Failed to create change set", context.output_format) exit(1) + if status == StackChangeSetStatus.READY: + at_least_one_ready = True + + # If none are ready, and we haven't exited, there are no changes + if not at_least_one_ready: + write("No changes detected", context.output_format) + exit(0) + # Describe changes descriptions = plan.describe_change_set(change_set_name) - for description in list(descriptions.values()): + for stack, description in descriptions.items(): + # No need to print if there are no changes + if statuses[stack] == StackChangeSetStatus.NO_CHANGES: + continue + if not verbose: description = simplify_change_set_description(description) write(description, context.output_format) @@ -76,8 +95,7 @@ def update_command( # Execute change set if happy with changes if yes or click.confirm("Proceed with stack update?"): plan.execute_change_set(change_set_name) - except Exception as e: - raise e + finally: # Clean up by deleting change set plan.delete_change_set(change_set_name) diff --git a/sceptre/plan/actions.py b/sceptre/plan/actions.py index 7b7fd5718..8cc67ef5f 100644 --- a/sceptre/plan/actions.py +++ b/sceptre/plan/actions.py @@ -949,6 +949,7 @@ def _get_cs_status(self, change_set_name): cs_description = self.describe_change_set(change_set_name) cs_status = cs_description["Status"] + cs_reason = cs_description.get("StatusReason") cs_exec_status = cs_description["ExecutionStatus"] possible_statuses = [ "CREATE_PENDING", @@ -983,6 +984,12 @@ def _get_cs_status(self, change_set_name): "CREATE_COMPLETE", ] and cs_exec_status in ["UNAVAILABLE", "AVAILABLE"]: return StackChangeSetStatus.PENDING + elif ( + cs_status == "FAILED" + and cs_reason is not None + and self._change_set_creation_failed_due_to_no_changes(cs_reason) + ): + return StackChangeSetStatus.NO_CHANGES elif cs_status in ["DELETE_COMPLETE", "FAILED"] or cs_exec_status in [ "EXECUTE_IN_PROGRESS", "EXECUTE_COMPLETE", diff --git a/sceptre/stack_status.py b/sceptre/stack_status.py index 34cc516ac..1cbf7ccb8 100644 --- a/sceptre/stack_status.py +++ b/sceptre/stack_status.py @@ -27,3 +27,4 @@ class StackChangeSetStatus(object): PENDING = "pending" READY = "ready" DEFUNCT = "defunct" + NO_CHANGES = "no changes" diff --git a/tests/test_actions.py b/tests/test_actions.py index 9e66b7601..113ccf6c8 100644 --- a/tests/test_actions.py +++ b/tests/test_actions.py @@ -1207,6 +1207,15 @@ def test_get_cs_status_handles_all_statuses(self, mock_describe_change_set): response = self.actions._get_cs_status(sentinel.change_set_name) assert response == returns[i] + mock_describe_change_set.return_value = { + "Status": "FAILED", + "StatusReason": "The submitted information didn't contain changes. " + "Submit different information to create a change set.", + "ExecutionStatus": "UNAVAILABLE", + } + response = self.actions._get_cs_status(sentinel.change_set_name) + assert response == scss.NO_CHANGES + for status in return_values["Status"]: mock_describe_change_set.return_value = { "Status": status, diff --git a/tests/test_cli/test_cli_commands.py b/tests/test_cli/test_cli_commands.py index b5b8c88a6..4bcee0d18 100644 --- a/tests/test_cli/test_cli_commands.py +++ b/tests/test_cli/test_cli_commands.py @@ -35,7 +35,7 @@ from sceptre.exceptions import SceptreException from sceptre.plan.actions import StackActions from sceptre.stack import Stack -from sceptre.stack_status import StackStatus +from sceptre.stack_status import StackChangeSetStatus, StackStatus class TestCli: @@ -514,6 +514,116 @@ def test_stack_commands(self, command, success, yes_flag, exit_code): run_command.assert_called_with() assert result.exit_code == exit_code + @pytest.mark.parametrize("verbose_flag", [True, False]) + def test_update_with_change_set_ready(self, verbose_flag): + create_command = self.mock_stack_actions.create_change_set + wait_command = self.mock_stack_actions.wait_for_cs_completion + execute_command = self.mock_stack_actions.execute_change_set + delete_command = self.mock_stack_actions.delete_change_set + describe_command = self.mock_stack_actions.describe_change_set + + change_set_status = StackChangeSetStatus.READY + wait_command.return_value = change_set_status + + response = { + "VerboseProperty": "VerboseProperty", + "ChangeSetName": "ChangeSetName", + "CreationTime": "CreationTime", + "ExecutionStatus": "ExecutionStatus", + "StackName": "StackName", + "Status": "Status", + "StatusReason": "StatusReason", + "Changes": [ + { + "ResourceChange": { + "Action": "Action", + "LogicalResourceId": "LogicalResourceId", + "PhysicalResourceId": "PhysicalResourceId", + "Replacement": "Replacement", + "ResourceType": "ResourceType", + "Scope": "Scope", + "VerboseProperty": "VerboseProperty", + } + } + ], + } + + if not verbose_flag: + del response["VerboseProperty"] + del response["Changes"][0]["ResourceChange"]["VerboseProperty"] + + describe_command.return_value = response + + kwargs = {"args": ["update", "--change-set", "dev/vpc.yaml", "-y"]} + if verbose_flag: + kwargs["args"].append("-v") + + result = self.runner.invoke(cli, **kwargs) + + change_set_name = create_command.call_args[0][0] + assert "change-set" in change_set_name + + wait_command.assert_called_once_with(change_set_name) + delete_command.assert_called_once_with(change_set_name) + execute_command.assert_called_once_with(change_set_name) + describe_command.assert_called_once_with(change_set_name) + + output = result.output.splitlines()[0] + assert yaml.safe_load(output) == response + assert result.exit_code == 0 + + @pytest.mark.parametrize("yes_flag", [True, False]) + def test_update_with_change_set_defunct(self, yes_flag): + create_command = self.mock_stack_actions.create_change_set + wait_command = self.mock_stack_actions.wait_for_cs_completion + delete_command = self.mock_stack_actions.delete_change_set + + change_set_status = StackChangeSetStatus.DEFUNCT + wait_command.return_value = change_set_status + + kwargs = {"args": ["update", "--change-set", "dev/vpc.yaml"]} + if yes_flag: + kwargs["args"].append("-y") + else: + kwargs["input"] = "y\n" + + result = self.runner.invoke(cli, **kwargs) + + change_set_name = create_command.call_args[0][0] + assert "change-set" in change_set_name + + wait_command.assert_called_once_with(change_set_name) + delete_command.assert_called_once_with(change_set_name) + + assert "Failed to create change set" in result.output + assert result.exit_code == 1 + + @pytest.mark.parametrize("yes_flag", [True, False]) + def test_update_with_change_set_no_changes(self, yes_flag): + create_command = self.mock_stack_actions.create_change_set + wait_command = self.mock_stack_actions.wait_for_cs_completion + delete_command = self.mock_stack_actions.delete_change_set + + change_set_status = StackChangeSetStatus.NO_CHANGES + wait_command.return_value = change_set_status + + kwargs = {"args": ["update", "--change-set", "dev/vpc.yaml"]} + if yes_flag: + kwargs["args"].append("-y") + else: + kwargs["input"] = "y\n" + + result = self.runner.invoke(cli, **kwargs) + + change_set_name = create_command.call_args[0][0] + assert "change-set" in change_set_name + + wait_command.assert_called_once_with(change_set_name) + delete_command.assert_called_once_with(change_set_name) + + assert "No changes detected" in result.output + assert result.exit_code == 0 + @pytest.mark.parametrize( "command, ignore_dependencies", [