Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

wait_for to ensure ELB has been removed from AWS #2548

Merged
merged 2 commits into from
Jan 11, 2016

Conversation

Etherdaemon
Copy link
Contributor

@jsdalton, @RickMendes
Hi Rick and Jim,

Can I propose the following pull request which will allow users to specify if they would like to wait to ensure the ELB (and the attached interface as it does silly things) has been removed from AWS?

I've had consistent situations where I cannot remove the security groups as a result of the ELB not technically registering as removed yet from AWS (Eventual Consistency issue).

In this PR I haven't exposed the possiblity for providing the max increment or polling time but that can be updated as well.

Karen

@Etherdaemon Etherdaemon changed the title Proposing a wait_for function to ensure elb has been successfully rem… wait_for to ensure ELB has been removed from AWS Nov 25, 2015
@Etherdaemon Etherdaemon force-pushed the propose_wait_for_removed_ec2_elb_lb branch from 7e94362 to 2fe9376 Compare November 26, 2015 03:22
@Etherdaemon Etherdaemon force-pushed the propose_wait_for_removed_ec2_elb_lb branch from 2fe9376 to 185a371 Compare November 26, 2015 06:59
@gregdek
Copy link
Contributor

gregdek commented Nov 29, 2015

Thanks @Etherdaemon. @jsdalton @RickMendes please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit' or 'needs_revision' as appropriate.

@ghost
Copy link

ghost commented Dec 1, 2015

@Etherdaemon I like the idea of having this and I don't see any problems with the current changes.

To make it a full feature add, I would like to see it be more configurable. As such I am marking it as needs_revision.

As you suggest, you could offer users the chance to set max increment and polling time. I wonder if it would be better to simply offer a wait_time like many other modules do. Then you could just divide that time by your chosen number of increments to get your polling time. That also means every user does not need to know how your polling works.

This is just a code review. I don't have time this week to do any testing.

@Etherdaemon
Copy link
Contributor Author

@RickMendes Thanks for the feedback. I'll work on a revision and resubmit later this week. Thanks again.

@gregdek
Copy link
Contributor

gregdek commented Dec 3, 2015

Thanks @Etherdaemon for this PR. A maintainer of this module has asked for revisions to this PR. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

@Etherdaemon
Copy link
Contributor Author

Hi @RickMendes and @gregdek.

ready_for_review

Thanks heaps.

@ghost
Copy link

ghost commented Dec 7, 2015

@Etherdaemon Thanks for taking care of this. I believe it is time to shipit.

/cc @gregdek

@gregdek
Copy link
Contributor

gregdek commented Dec 8, 2015

Thanks again to @Etherdaemon for this PR, and thanks @jsdalton @RickMendes for reviewing. Marking for inclusion.

@bcoca bcoca added this to the next milestone Jan 11, 2016
bcoca added a commit that referenced this pull request Jan 11, 2016
…2_elb_lb

wait_for to ensure ELB has been removed from AWS
@bcoca bcoca merged commit 316749c into ansible:devel Jan 11, 2016
@Etherdaemon Etherdaemon deleted the propose_wait_for_removed_ec2_elb_lb branch January 21, 2016 04:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants