-
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
ec2_elb: times out on de-register #3849
Comments
Which region does your elb exist in? Does it exist in the same region as On 14 August 2013 18:41, Shubhang Mani notifications@github.com wrote:
|
Yes, Looking at the code, I think it might be assuming that the module is being run within EC2 since it tries to query the metadata service if region is not specified. That might be why it's timing out (the first error). Not sure yet why the ELB isn't being found. |
Sorry, I missed that. Yes, that's correct. BruceP has removed that in his On 14 August 2013 19:10, Shubhang Mani notifications@github.com wrote:
|
I should have expanded my last comment a little more. As you have probably On 14 August 2013 19:22, Lester Wade lestertron@gmail.com wrote:
|
So, I guess I'll temporarily use the older version until the PR makes it in. Should I close this issue ? |
Sounds like it needs to stay open but I'll reference it |
Or better still, try out the changes which will soon be merged by grabbing On 14 August 2013 21:01, Shubhang Mani notifications@github.com wrote:
|
Can we confirm if this is still an issue? The PR above was merged. |
I pulled the latest from github but I don't see any modifications for this module. |
Yeah my bad, I thought the above had been merged but it had just been closed. I'll take a look into this today. Can you share an example of your playbook to help replicate this? |
Once Bruce's changes get merged, specifying the region will resolve this I On 23 August 2013 14:10, James Cammarata notifications@github.com wrote:
|
@jimi1283 - here's an example of what I'm trying to do: https://gist.github.com/bhang/6320004 @lwade - looking forward to the merge |
@bhang I've been testing Bruce's patch, and it does appear that this resolves the error, so as soon as that's merged in I'll close this as being fixed. The one gotcha is you'll have to add "region: " to all of your ec2_elb tasks (in fact region is going to be require for most if not all of the ec2* modules going forward). |
@jimi1283 - sounds good, thanks! |
Merged, give it a test and let me know if you're still having problems. |
So, after pulling the latest, the de-register works fine when I add the
Of course, if I don't supply the
|
That's odd, it appears the name is being interpreted as a list. Can you look in the library/cloud/ec2_elb code and tell me if you see something like this towards the end of the file? for elb in ec2_elbs: if not elb_man.exists(elb): msg="ELB %s does not exist" % elb module.fail_json(msg=msg) |
Yep, I see it. Here's the end of the file on my machine: for elb in ec2_elbs:
if not elb_man.exists(elb):
msg="ELB %s does not exist" % elb
module.fail_json(msg=msg)
if module.params['state'] == 'present':
elb_man.register(wait)
elif module.params['state'] == 'absent':
elb_man.deregister(wait)
ansible_facts = {'ec2_elbs': [lb.name for lb in elb_man.lbs]}
ec2_facts_result = dict(changed=elb_man.changed, ansible_facts=ansible_facts)
module.exit_json(**ec2_facts_result)
# this is magic, see lib/ansible/module_common.py
#<<INCLUDE_ANSIBLE_MODULE_COMMON>>
main() |
And how are you specifying the ELB's? Is it like this: ec2_elbs: - testing Or like this: ec2_elbs: testing If it's the later, could you try the top syntax? |
It's the former i.e.
|
That is very odd, that's how I'm testing it and not running into that problem. |
Can we see your playbook for registration and deregistration?
|
@lwade - I've updated the gist that I shared earlier to include the region arg. This playbook fails at the "Register instance with ELB" task. |
@bhang actually I forgot that you were using a variable for your register step. Could you make sure that variable is an array? I think that may be where the code is getting confused. |
I'm confused. The de-register is supposed to return a fact ec2_elbs which is the list of ELB names (also provided as a list to the de-register). Shouldn't the fact be interpolated as a list when I register? This was working just fine earlier. |
So it looks like the issue here is that the fact being passed in as an argument results in it being interpreted as a string, despite the fact that it is an array. If you use with_items as follows, it works perfectly, albeit with multiple iterations: - name: Deregister instance from ELB local_action: module: ec2_elb ec2_elbs: "{{ item }}" instance_id: '{{ ansible_ec2_instance_id }}' state: absent region: us-east-1 sudo: false with_items: ec2_elbs This is despite the fact that the module arg is specifically listed as an array, so I'm not quite sure why that's the case here. As a temporary workaround, you can use the syntax above to get this working, and I'll continue debugging. |
It looks like this is a known issue with the way array facts are interpreted (this is being handled in another issue). Right now, I'd recommend continuing to use the with_items method I posted above, or don't use the fact since you know the names of the LB's you're using in the registration step. |
@jimi1283 - Thanks, I'll try the options you suggested. |
Ansible version: git checkout of devel - 10a0f03
Module: ec2_elb
Version: 8171b7b
I have a playbook that deals with a common scenario - software deploys to instances behind an ELB. The playbook runs serially and performs the following steps:
This works well with the ec2_elb module until the commit prior to the one I listed above. With the latest commit, I get an error: operation timed out.
invalid output was:
Thinking that perhaps I had to specify the region (even though it's not required), I added the
ec2_region
argument but then I got a new error:The previous version of this module d644971 works just fine.
The text was updated successfully, but these errors were encountered: