-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Boto - Attaching to Load balancer #5076
Comments
Here's the play:
|
Are you sure that i-880518f3 exists in us-east-1 ? |
here's debugging getting the instance info before:
|
As suggested by @jctanner, tried HEAD and got no difference:
|
This commit (3ea0b2b) caused it to stop working |
adding |
This line specifically broke it: 3ea0b2b#diff-65143e5debe6a07b9e81d5d14fc8abbbR154 |
We should definitely not revert the above as it has a lot of useful things in it. Do you have some cycles to attempt to fix this in a less invasive way or are you just asking for us to do it? :) |
Looking now. I might need some feedback though since my python fu is not up
|
On HEAD, i get the following error if I don't specify the
|
Disregard the last comment, old version of boto. So it appears that the newest version of boto (2.18) does work with HEAD. Closing. |
Opening this back up...I hit it again on devel branch with boto version 2.19. Maybe someone can point me in the right direction. LB is there, this worked ok for me in 1.3.x.
|
Confirmed that checking out 1.3.4 restores this functionality. |
Replicated with RHEL available boto (2.13.3) and ansible 1.4 Updating ansible to 1.5 reproduces the error. Updating boto to 2.19.0 reproduces the error. So in simpler terms the ansible ec2_elb module is checking the health of the instance as a "pre" state to confirm against the registration? How does this make sense when adding something to the pool requires its state in the pool to be checked as a member? I feel like I'm taking crazy pills! |
I'd like to help fix this since it's a blocker for me, but I need some suggestions. Here is the offending line: https://github.com/ansible/ansible/blob/devel/library/cloud/ec2_elb#L154 initial_state = lb.get_instance_health([self.instance_id])[0] The instance is being checked in the context of the load balancer, which it is not attached to, so it fails with:
In Ruby, I might consider rescuing that exception when it doesn't find it and set some default. Any suggestions? |
@jsdalton I'm hoping you could add some insight to this (https://github.com/ansible/ansible/pull/4112/files#diff-65143e5debe6a07b9e81d5d14fc8abbbR194)...This line seems to have broken ELB for me and others with the errors above. I don't know python and I was hoping to get your insight and maybe some assistance with a fix. |
Thanks for alerting me to this @brandonhilkert. Your diagnosis appears correct as I'm reviewing the traceback. My assumption here is that an instance that has never been registered with a given load balancer will throw a Boto exception if it's instance health is queried, whereas once it has already been registered at some point, if it's not in service it will return Out of Service. The EC2 API is filled with corner cases like these that are tricky to develop for. :) I think you're right that catching this particular error is the right approach. The state checking is really only necessary if "wait" mode is on -- i.e. if you want this module to wait around for the actual registration or deregistration to occur, and report the new state, do idempotence checks etc. So really that check for initial state should be scoped under if wait. I'll try to find some time to put together a fix today if I can. I hate that feeling of knowing exactly what some code needs to do but not being quite familiar enough with the language to be able to express it. |
Thanks for the detailed feedback. ...And I hate knowing sort of what's going on, but not being able to help much. If I can, in any way, please let me know. I'm happy to do whatever necessary to get over this road block. Thanks! |
Okay @brandonhilkert I just submitted a pull request that hopefully fixes the issue. There was actually another issue hiding behind the first one you discovered, where an instance enters a "pending" state when first registered and we weren't accounting for it. All in all this was kind of an annoying issue to deal with. It's a bit odd to me that AWS chooses to treat this as an error condition, though I guess they have their reasons. They also don't do a good job of distinguishing between an instance that's out of service because the operation is pending vs. one that's out of service for failed health check (the first will eventually go in service, the second presumably never will). Anyhow, I'd appreciate it if you were able to try this code out yourself and let me know if it works for you. it's definitely a pain to test corner cases....this one in particular was annoying since i needed to use a fresh instance or EBL every time I wanted to see if it worked. :) Let me know and hopefully an admin will accept my pull request shortly. |
Thanks @jsdalton (and @brandonhilkert)! I'd been meaning to write a patch too but got sidetracked on some other work. Fingers crossed for an acceptance! |
@nickdevereaux No problem at all -- sorry for the initial difficulties you hit. If you have a chance at all to try out my patch, let me know if it works or if you run into any problems... |
Thanks @jadalton for the attention. I'll check it out tomorrow first thing. On Monday, December 9, 2013, jsdalton wrote:
|
Got it working! Thanks @jsdalton |
Confirmed that it works for me too. Thanks @jsdalton! This is a huge life-saver... |
Closing in favor of the PR. |
Account for instances that have not yet been registered. Fixes #5076
This seems to still exist on master? From the docs, it's not clear what I'm supposed to do. I see mentions of availability zones here, but I don't follow how that comes from this error message.
|
(Well, I mean, obviously from the stack trace it's about zones, but I'm surprised that an attribute error is thrown if the wrong config option is in place. This may just be my ignorance of how boto works.) |
@jmhodges what version boto? |
Oh, oh dear. I have 2.9.6-1. This is terribly old, isn't it? I'll upgrade. Apologies. |
Yup, the method |
) * Restart EC2 instances with multiple network interfaces A previous bug, #3234, caused instances with multiple ENI's to fail when being started or stopped because sourceDestCheck is a per-interface attribute, but we use the boto global access to it (which only works when there's a single ENI). This patch handles a variant of that bug that only surfaced when restarting an instance, and catches the same type of exception. * Default termination_protection to None instead of False AWS defaults the value of termination_protection to False, so we don't need to explicitly send `False` when the user hasn't specified a termination protection level. Before this patch, the below pair of tasks would: 1. Create an instance (enabling termination_protection) 2. Restart that instance (disabling termination_protection) Now, the default None value would prevent the restart task from disabling termination_protection. ``` - name: make an EC2 instance ec2: vpc_subnet_id: {{ subnet }} instance_type: t2.micro termination_protection: yes exact_count: 1 count_tag: Name: TestInstance instance_tags: Name: TestInstance group_id: {{ group }} image: ami-7172b611 wait: yes - name: restart a protected EC2 instance ec2: vpc_subnet_id: {{ subnet }} state: restarted instance_tags: Name: TestInstance group_id: {{ group }} image: ami-7172b611 wait: yes ```
boto==2.17.0 (also tried with 2.18.0)
I updated to Ansible 1.4, and ever since, I get the following error when attaching a machine to an EC2 load balancer. I realize this is a boto response, but I was hoping you have might have insight to version issues or something else that might have caused this.
The DNS name is right and the instance name is says that it can't find it there as well.
The text was updated successfully, but these errors were encountered: