Yet another rebase attempt for standardizing AWS parameters - #3894

Sorry you have to keep doing that, but you are touching a large number of files so any other commits on those files that get merged in ahead of this will trample it and require a rebase (and the ec2 modules seem to be high-traffic areas).

@lwade any chance you could take a look at this? Overall it looks ok to me.

It does look like my change to the route53 module broke the clean pull request again, but I'll go ahead and just cherry pick these myself and resolve the conflict if need-be (the second commit appears to be a .orig file that should probably be left out anyway).

I've found a couple of problems with the ec2_elb module already:

diff --git a/library/cloud/ec2_elb b/library/cloud/ec2_elb
index a8131c2..a946f41 100644
--- a/library/cloud/ec2_elb
+++ b/library/cloud/ec2_elb
@@ -196,7 +196,7 @@ def main():
             state={'required': True,
                     'choices': ['present', 'absent']},
             instance_id={'required': True},
-            ec2_elbs={'default': None, 'required': False},
+            ec2_elbs={'default': None, 'required': False, 'type':'list'},
             aws_secret_key={'default': None, 'aliases': ['ec2_secret_key', 'secret_key'], 'no_log': True},
             aws_access_key={'default': None, 'aliases': ['ec2_access_key', 'access_key']},
             region={'default': None, 'required': False, 'aliases':['aws_region', 'ec2_region'], 'choices':AWS_REGIONS}
@@ -230,16 +230,16 @@ def main():
             region = os.environ['EC2_REGION']
 
     if not region:
-        module.fail_json(msg = str("Either region or EC2_REGION environment variable must be set."))
+        module.fail_json(msg=str("Either region or EC2_REGION environment variable must be set."))
 
     instance_id = module.params['instance_id']
     elb_man = ElbManager(module, instance_id, ec2_elbs, aws_access_key,
                          aws_secret_key, region=region)
 
-    for elb in [ ec2_elbs ]:
+    for elb in ec2_elbs:
         if not elb_man.exists(elb):
-            str="ELB %s does not exist" % elb
-            module.fail_json(msg=str)
+            msg="ELB %s does not exist" % elb
+            module.fail_json(msg=msg)
 
     if module.params['state'] == 'present':
         elb_man.register()

Even with the above, I'm getting a hang after registering an instance into the ELB:

PLAY [all] ******************************************************************** 
GATHERING FACTS *************************************************************** 
ok: [ec2-******.compute-1.amazonaws.com]
TASK: [Gathering ec2 facts] *************************************************** 
ok: [ec2-******.compute-1.amazonaws.com]
TASK: [Register instance with ELB] ******************************************** 

The instance is registered into the ELB according to the EC2 console.

Did some testing on this today with at least the ec2 and ec2_elb module, which seem solid. @lwade, have you been able to do any testing on this yet?

Making a note regarding #3849 so I remember to close that once this gets merged in.

Hey James. I've tested the ec2_vol and s3 modules with these changes, both
are fine.

On that note, not sure why but this PR seems to be referencing: A
library/cloud/s3.orighttps://github.com//pull/3894/files#diff-8(473)?

On 23 August 2013 19:48, James Cammarata [email protected] wrote:

Did some testing on this today with at least the ec2 and ec2_elb module,
which seem solid. @lwade https://github.com/lwade, have you been able
to do any testing on this yet?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3894#issuecomment-23183394
.

Note that the AWS API can be very slow here. Did it work out in the end?

On 23 August 2013 18:28, James Cammarata [email protected] wrote:

I've found a couple of problems with the ec2_elb module already:

diff --git a/library/cloud/ec2_elb b/library/cloud/ec2_elb
index a8131c2..a946f41 100644
--- a/library/cloud/ec2_elb
+++ b/library/cloud/ec2_elb
@@ -196,7 +196,7 @@ def main():
state={'required': True,
'choices': ['present', 'absent']},
instance_id={'required': True},

•        ec2_elbs={'default': None, 'required': False},
  

•        ec2_elbs={'default': None, 'required': False, 'type':'list'},
       aws_secret_key={'default': None, 'aliases': ['ec2_secret_key', 'secret_key'], 'no_log': True},
       aws_access_key={'default': None, 'aliases': ['ec2_access_key', 'access_key']},
       region={'default': None, 'required': False, 'aliases':['aws_region', 'ec2_region'], 'choices':AWS_REGIONS}
  

  @@ -230,16 +230,16 @@ def main():
  region = os.environ['EC2_REGION']

  if not region:

•    module.fail_json(msg = str("Either region or EC2_REGION environment variable must be set."))
  

•    module.fail_json(msg=str("Either region or EC2_REGION environment variable must be set."))
  

  instance_id = module.params['instance_id']
  elb_man = ElbManager(module, instance_id, ec2_elbs, aws_access_key,
  aws_secret_key, region=region)

• for elb in [ ec2_elbs ]:

• for elb in ec2_elbs:
  if not elb_man.exists(elb):

•        str="ELB %s does not exist" % elb
  

•        module.fail_json(msg=str)
  

•        msg="ELB %s does not exist" % elb)
  

•        module.fail_json(msg=msg)
  

  if module.params['state'] == 'present':
  elb_man.register()

Even with the above, I'm getting a hang after registering an instance into
the ELB:

PLAY [all] ********************************************************************

GATHERING FACTS ***************************************************************
ok: [ec2-******.compute-1.amazonaws.com]

TASK: [Gathering ec2 facts] ***************************************************
ok: [ec2-******.compute-1.amazonaws.com]

TASK: [Register instance with ELB] ********************************************

The instance is registered into the ELB according to the EC2 console.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3894#issuecomment-23178365
.

It was waiting for the service to come online, but since I hadn't setup an http server on the image that never happened. I had Bruce add a flag to tell it not to wait. Ideally there should also be a timeout so it doesn't wait forever but that's a future feature.

And merged! Thanks!