(WIP) Remove DNS tag in the event of rollback #253
Conversation
bootstrap_cfn/fab_tasks.py
Outdated
| @@ -725,7 +731,15 @@ def cfn_create(test=False): | |||
| # So delete the SSL cert that we uploaded | |||
| if 'ssl' in cfn_config.data: | |||
| iam.delete_ssl_certificate(cfn_config.ssl(), stack_name) | |||
| abort('Failed to create stack: {0}'.format(stack)) | |||
| # delete the DNS tag | |||
| for elb in get_public_elbs(): | |||
There was a problem hiding this comment.
I think a get_all_elbs() is more appropriate here.
bootstrap_cfn/fab_tasks.py
Outdated
| @@ -725,7 +731,15 @@ def cfn_create(test=False): | |||
| # So delete the SSL cert that we uploaded | |||
| if 'ssl' in cfn_config.data: | |||
| iam.delete_ssl_certificate(cfn_config.ssl(), stack_name) | |||
| abort('Failed to create stack: {0}'.format(stack)) | |||
| # delete the DNS tag | |||
| for elb in get_all_elbs(): | |||
There was a problem hiding this comment.
I created a PR to fix this chunk of code. basically if elb is empty, txt records won't be deleted. also txt record deletion is missing here. the PR has been merged. you could pull and have a look :p
| logger.info("Deleting DNS tag for '{stack} from '{zone}'...".format(stack=stack_id, zone=zone_name)) | ||
| try: | ||
| print green("Removing tag stack from DNS...") | ||
| r53_conn.delete_txt_record(zone_name, zone_id, txt_tag_record) |
There was a problem hiding this comment.
r53_conn.delete_txt_record only removes txt record. we probably need to delete/check Alias record too?
| r53_conn.delete_txt_record(zone_name, zone_id, txt_tag_record) | ||
| except boto.route53.exception.DNSServerError: | ||
| print red("Failed to remove DNS tag") | ||
| abort(red('Failed to create stack: {0}. You may want to delete it manually.'.format(stack))) |
There was a problem hiding this comment.
also, are we retaining the failed stack while removing the DNS records?
There was a problem hiding this comment.
Retaining the rolledback stack is current behaviour so I decided not to change it in this patch. I've a slight preference for not automatically deleting a rolledback stack but am open to discussion. But even if so, we should probably do it as a separate piece of work.
|
@mattpep & @yufangzhang any consensus on this or progress in merge? |
|
@yufangzhang @mattpep @ltsampros Not quite sure why we're looking to delete records on rollback. We need them for cfn_delete the stack(?) Anyways, happy have my ignorance crushed by catching up and finding out :) But going to put in WIP for the moment |
sevenmachines
changed the title
|
A bit late to the party, but this PR is still open/pending. Niall's comment sounds right, if we are not dealing with a half created stack because we want to cfn_delete then we shouldn't need this PR altogether. So we should close this PR and document this expectation, perhaps under a workflow headline? |
|
Invalid. |
Fixes #221