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

Fix get_elb_list function when looking up from ELB #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wjordan
Copy link

@wjordan wjordan commented May 12, 2016

the get_elb_list function no longer works properly for retrieving a list of ELBs the instance is currently registered to directly (not in an auto scaling group). The underlying issue was that get_instance_health_elb always returned OutOfService rather than a non-zero exit code when the instance was not registered to the load balancer, because the API returns OutOfService with the description "Instance is not currently registered with the LoadBalancer." (Perhaps this used to return an error but was changed in the API more recently?)

This PR rewrites this function to return the list of load balancers which have the instance in question in their Instances array, rather than querying get_instance_health_elb for each load balancer individually. This also speeds up the function (only requires a single AWS API call versus a separate API call per load balancer).

I used grep and awk instead of the equivalent JMESPath filter expression because old versions of jmespath (my use case is v0.2.1 from the Ubuntu Trusty package) do not support these advanced expressions.

@suryanarayanan
Copy link
Contributor

Hi,
Thanks for the pull request. Could you please confirm this contribution is under the terms of the Apache 2.0 license. Thanks.

@wjordan
Copy link
Author

wjordan commented May 31, 2016

Confirmed, Apache 2.0 license applies to this contribution.

@CarsonF
Copy link

CarsonF commented Jan 19, 2017

This can be closed as it was applied with d30c8ff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants