-
Notifications
You must be signed in to change notification settings - Fork 405
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
Force delete support for ASGs #315
base: master
Are you sure you want to change the base?
Conversation
@@ -450,13 +452,15 @@ class AutoScalingController { | |||
String name = params.name | |||
AutoScalingGroup group = awsAutoScalingService.getAutoScalingGroup(userContext, name) | |||
Boolean showGroupNext = false | |||
Boolean showTask = false | |||
if (!group) { | |||
flash.message = "Auto Scaling Group '${name}' not found." | |||
} else { | |||
if (group?.instances?.size() <= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping this check defeats the purpose of this change.
It's helpful to include the Jira ID at the start of the pull request name, and at the start of one of the commits within the pull request. |
asgard-pull-requests #70 SUCCESS |
Almost all of the code of the method deserves to be deleted and replaced with something more similar to what Cluster.delete does. The booleans and instance count protection and the try-catch blocks were only present because we were doing a synchronous Amazon call with a lot of edge cases to handle. |
This actually wouldn't be an improvement for the user, since the purpose of using Force Delete is that it's possible to delete an ASG that still contains some instances, whether the instances are real or phantoms. There was an internal email this afternoon about this very problem. |
asgard-pull-requests #71 SUCCESS |
asgard-pull-requests #72 SUCCESS |
@joesondow Is there value in the extra work that GroupDeleteOperation is doing here? I'm asking not really for this issue, but to see if it should be in the new auto deploy workflow. |
asgard-pull-requests #73 SUCCESS |
This patch should allow users to force-delete ASGs. It follows a similar approach to what was already implemented for clusters, using the GroupDeleteOperation class.