Skip to content

Commit

Permalink
[Resolve Sceptre#1483] Handle errors in change sets (Sceptre#1486)
Browse files Browse the repository at this point in the history
* [Resolve Sceptre#1483] Handle errors in change sets

This adds code to handle the case of failures in stack groups that have missing or errored stacks when creating, describing and executing change-sets.

Before this, any stack group that contained a stack that had been deleted, or had errored out, resulted in all change set operations on that stack group failing.

The reason for this is no error or exception handling had ever been added for change sets, but assumed all stacks in the stack group were clean and could respond to the change set APIs.

For example, deleting change sets might error out with:

% sceptre delete -y network.yaml sceptre-network
The Change Set will be delete on the following stacks, if applicable:
network

"An error occurred (ValidationError) when calling the DeleteChangeSet
operation: Stack [test-e2e-direct-connect] does not exist"
This is particularly an issue for sites that employ as best practice analysis of change sets for safety prior to launching updates.

This patch adds the missing error handling for delete_change_set, create_change_set, describe_change_set and
execute_change_set.
  • Loading branch information
alex-harvey-z3q authored Aug 3, 2024
1 parent 82ac3f5 commit deef08e
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 23 deletions.
3 changes: 1 addition & 2 deletions integration-tests/features/describe-change-set.feature
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ Feature: Describe change sets
Given stack "1/A" exists in "CREATE_COMPLETE" state
and stack "1/A" has no change sets
When the user describes change set "A" for stack "1/A"
Then a "ClientError" is raised
and the user is told "change set does not exist"
Then the user is told "Failed describing Change Set"

Scenario: describe a change set that exists with ignore dependencies
Given stack "1/A" exists in "CREATE_COMPLETE" state
Expand Down
3 changes: 1 addition & 2 deletions integration-tests/features/execute-change-set.feature
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ Feature: Execute change set
Given stack "1/A" exists in "CREATE_COMPLETE" state
And stack "1/A" does not have change set "A"
When the user executes change set "A" for stack "1/A"
Then a "ClientError" is raised
And the user is told "change set does not exist"
Then the user is told "change set does not exist"

Scenario: execute a change set that exists with ignore dependencies
Given stack "1/A" exists in "CREATE_COMPLETE" state
Expand Down
5 changes: 3 additions & 2 deletions integration-tests/steps/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ def step_impl(context, message):
msg = context.error.response["Error"]["Message"]
assert msg.endswith("does not exist")
elif message == "change set does not exist":
msg = context.error.response["Error"]["Message"]
assert msg.endswith("does not exist")
assert context.log_capture.find_event("does not exist")
elif message == "the template is valid":
for stack, status in context.response.items():
assert status["ResponseMetadata"]["HTTPStatusCode"] == 200
elif message == "the template is malformed":
msg = context.error.response["Error"]["Message"]
assert msg.startswith("Template format error")
elif message == "Failed describing Change Set":
assert context.log_capture.find_event(message)
else:
raise Exception("Step has incorrect message")

Expand Down
3 changes: 3 additions & 0 deletions sceptre/cli/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,9 @@ def simplify_change_set_description(response):
:returns: A more concise description of the change set.
:rtype: dict
"""
if not response:
return {"ChangeSetName": "ChangeSetNotFound"}

desired_response_items = [
"ChangeSetName",
"CreationTime",
Expand Down
84 changes: 67 additions & 17 deletions sceptre/plan/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,21 @@ def create_change_set(self, change_set_name):
{"Key": str(k), "Value": str(v)} for k, v in self.stack.tags.items()
],
}

create_change_set_kwargs.update(self.stack.template.get_boto_call_parameter())
create_change_set_kwargs.update(self._get_role_arn())

try:
self._create_change_set(change_set_name, create_change_set_kwargs)
except Exception as err:
self.logger.info(
"%s - Failed creating Change Set '%s'\n%s",
self.stack.name,
change_set_name,
err,
)

def _create_change_set(self, change_set_name, create_change_set_kwargs):
self.logger.debug(
"%s - Creating Change Set '%s'", self.stack.name, change_set_name
)
Expand All @@ -490,6 +503,24 @@ def delete_change_set(self, change_set_name):
:param change_set_name: The name of the Change Set.
:type change_set_name: str
"""
# If the call successfully completes, AWS CloudFormation
# successfully deleted the Change Set.
try:
self._delete_change_set(change_set_name)
self.logger.info(
"%s - Successfully deleted Change Set '%s'",
self.stack.name,
change_set_name,
)
except Exception as err:
self.logger.info(
"%s - Failed deleting Change Set '%s'\n%s",
self.stack.name,
change_set_name,
err,
)

def _delete_change_set(self, change_set_name):
self.logger.debug(
"%s - Deleting Change Set '%s'", self.stack.name, change_set_name
)
Expand All @@ -501,13 +532,6 @@ def delete_change_set(self, change_set_name):
"StackName": self.stack.external_name,
},
)
# If the call successfully completes, AWS CloudFormation
# successfully deleted the Change Set.
self.logger.info(
"%s - Successfully deleted Change Set '%s'",
self.stack.name,
change_set_name,
)

def describe_change_set(self, change_set_name):
"""
Expand All @@ -518,6 +542,21 @@ def describe_change_set(self, change_set_name):
:returns: The description of the Change Set.
:rtype: dict
"""
return_val = {}

try:
return_val = self._describe_change_set(change_set_name)
except Exception as err:
self.logger.info(
"%s - Failed describing Change Set '%s'\n%s",
self.stack.name,
change_set_name,
err,
)

return return_val

def _describe_change_set(self, change_set_name):
self.logger.debug(
"%s - Describing Change Set '%s'", self.stack.name, change_set_name
)
Expand All @@ -543,16 +582,32 @@ def execute_change_set(self, change_set_name):
change_set = self.describe_change_set(change_set_name)
status = change_set.get("Status")
reason = change_set.get("StatusReason")
if status == "FAILED" and self.change_set_creation_failed_due_to_no_changes(

return_val = 0

if status == "FAILED" and self._change_set_creation_failed_due_to_no_changes(
reason
):
self.logger.info(
"Skipping ChangeSet on Stack: {} - there are no changes".format(
change_set.get("StackName")
)
)
return 0
return return_val

try:
return_val = self._execute_change_set(change_set_name)
except Exception as err:
self.logger.info(
"%s - Failed describing Change Set '%s'\n%s",
self.stack.name,
change_set_name,
err,
)

return return_val

def _execute_change_set(self, change_set_name):
self.logger.debug(
"%s - Executing Change Set '%s'", self.stack.name, change_set_name
)
Expand All @@ -567,8 +622,9 @@ def execute_change_set(self, change_set_name):
status = self._wait_for_completion(boto_response=response)
return status

def change_set_creation_failed_due_to_no_changes(self, reason: str) -> bool:
"""Indicates the change set failed when it was created because there were actually
def _change_set_creation_failed_due_to_no_changes(self, reason: str) -> bool:
"""
Indicates the change set failed when it was created because there were actually
no changes introduced from the change set.
:param reason: The reason reported by CloudFormation for the Change Set failure
Expand Down Expand Up @@ -1103,9 +1159,6 @@ def _log_drift_status(self, response: dict) -> None:
self.logger.debug(f"{self.stack.name} - {key} - {response[key]}")

def _detect_stack_drift(self) -> dict:
"""
Run detect_stack_drift.
"""
self.logger.info(f"{self.stack.name} - Detecting Stack Drift")

return self.connection_manager.call(
Expand All @@ -1115,9 +1168,6 @@ def _detect_stack_drift(self) -> dict:
)

def _describe_stack_drift_detection_status(self, detection_id: str) -> dict:
"""
Run describe_stack_drift_detection_status.
"""
self.logger.info(f"{self.stack.name} - Describing Stack Drift Detection Status")

return self.connection_manager.call(
Expand Down

0 comments on commit deef08e

Please sign in to comment.