Proposed upgrades to ec2_elb module

There are a few upgrades I’d like to propose making to the ec2_elb module that extend its functionality a bit and cover some situations that aren’t being handled properly right now.

At present, the ec2_elb module:

  • Hangs when an instance is out of service for certain reasons. If you try to register an instance and it fails (e.g. if the instance doesn’t end up passing the health check), then the ec2_elb register action will hang if wait is true. This is because it’s expecting the instance to move from “OutOfService” to “InService”, but often times that is not going to happen.

  • Does not handle the situation where an ELB is not enabled for the availability zone of an instance you’re trying to add. If you add an instance in an AZ that hasn’t been enabled, it hangs for the reason described above.

  • Reports change too liberally. It basically reports a change if it finds a list of load balancer. It should only report a change if the state of the load balancer’s instance registration has been altered. This was mentioned here on the mailing list: https://groups.google.com/d/msg/ansible-project/sQFkX0upfR4/YmWCgF7u7KoJ (but never reported as an issue as far as I could tell).

I have a pull request ready that addresses these issues. My proposed changes are:

  • Add a parameter “enable_availability_zone” to the ec2_elb module defaulting to true. If this parameter is set, the module will enable the availability zone of the instance being registered if it is not yet enabled. If false, the task will fail.

  • Upgrade the state transition handling (mainly impacts instance registration). Now if you’re trying to register an instance, it’ll check the state as before. If the instance state is out of service and the reason code is “ELB”, this means that the ELB is waiting for instance to register and we proceed with waiting. If on the other hand the reason code is “Instance” it means there’s something on the instance preventing registration. This is a “permanent” error. Instead of hanging as now the task will simply fail.

  • Discover the initial state of the and compare it against the new state. Only if the instance state is different do we report a change.

The pull request includes documentation, comments etc. I left everything else alone so the patch should apply cleanly. Here’s a comparison look against the devel branch of my proposed changes:

https://github.com/jsdalton/ansible/compare/add_az_support_to_ec2_elb

Let me know if there are any thoughts or concerns about what I’m proposing. If not, I’ll go ahead and submit the pull request and you guys can decide if it merits inclusion.

I’m using it now on my own project in the manner described by the docs (deregister instance as a pre_task, register instance as a post_task) and it works great.

Can you submit these as a pull request on github and copy these comments into it?

This would make it easier to discuss and keep the code/details together.

I would also be interested in comments from the authors of the module.

During a recent bug fix we added an option (wait=true|false) that would prevent it from hanging when adding an instance that wasn’t “up” to the LB, so that one should be taken care of.

Great, submitted the pull request here: https://github.com/ansible/ansible/pull/4112