Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cfn): delete CFN changeset if empty upon request #4101

Merged
merged 6 commits into from Oct 28, 2019

Conversation

xavileon
Copy link
Contributor

This patch introduces the DeleteCloudFormationChangeSet async operation,
so Orca can request to delete a CFN change set under some conditions
(e.g. when it's empty, or after the change set has been executed). This
will allow full control of the change set life cycle by Spinnaker.

This patch introduces the DeleteCloudFormationChangeSet async operation,
so Orca can request to delete a CFN change set under some conditions
(e.g. when it's empty, or after the change set has been executed). This
will allow full control of the change set life cycle by Spinnaker.
amazonCloudFormation.deleteChangeSet(deleteChangeSetRequest);
} catch (AmazonCloudFormationException e) {
log.error(
"Error removing change set {} on stack {}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about including something from the exception in the message to give an idea why it failed?

Copy link
Contributor Author

@xavileon xavileon Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea 🙂 I'll add the message.

@xavileon
Copy link
Contributor Author

@spinnaker/reviewers PTAL

task.updateStatus(BASE_PHASE, "Deleting CloudFormation ChangeSet");
amazonCloudFormation.deleteChangeSet(deleteChangeSetRequest);
} catch (AmazonCloudFormationException e) {
log.error(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it OK that this error is ignored?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean not returning any error back to orca? Fair comment, I'll return an error so orca can handle it appropiately, probably with a retryable task 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c7cd134

xavileon and others added 3 commits October 22, 2019 22:43
…ver/aws/deploy/ops/DeleteCloudFormationChangeSetAtomicOperation.java

Co-Authored-By: Mark Vulfson <markvu@live.com>
Copy link
Contributor

@marchello2000 marchello2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have much knowledge of CFN, but the code LGTM

@marchello2000 marchello2000 added the ready to merge Approved and ready for a merge label Oct 28, 2019
@mergify mergify bot added the auto merged Merged automatically by a bot label Oct 28, 2019
@mergify mergify bot merged commit 2aaab60 into spinnaker:master Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for a merge target-release/1.17
Projects
None yet
4 participants