Fail with nice error message if elb target_type=ip not supported (#38313)

* Add helpful failure message if target_type=ip is not supported

Create test case for target_type=ip not supported

* Update elb_target_group module to latest standards

Use AnsibleAWSModule
Improve exception handling
Improve connection handling

hacking/aws_config/testing_policies/compute-policy.json

@@ -130,6 +130,7 @@
             "Sid": "AllowLoadBalancerOperations",
             "Effect": "Allow",
             "Action": [
+                "elasticloadbalancing:AddTags",
                 "elasticloadbalancing:ConfigureHealthCheck",
                 "elasticloadbalancing:CreateListener",
                 "elasticloadbalancing:CreateLoadBalancer",
@@ -144,12 +145,13 @@
                 "elasticloadbalancing:DescribeLoadBalancerAttributes",
                 "elasticloadbalancing:DescribeLoadBalancerPolicies",
                 "elasticloadbalancing:DescribeLoadBalancerPolicyTypes",
-                "elasticloadbalancing:DescribeLoadBalancerTags",
                 "elasticloadbalancing:DescribeLoadBalancers",
+                "elasticloadbalancing:DescribeTags",
                 "elasticloadbalancing:DisableAvailabilityZonesForLoadBalancer",
                 "elasticloadbalancing:EnableAvailabilityZonesForLoadBalancer",
                 "elasticloadbalancing:ModifyLoadBalancerAttributes",
-                "elasticloadbalancing:RegisterInstancesWithLoadBalancer"
+                "elasticloadbalancing:RegisterInstancesWithLoadBalancer",
+                "elasticloadbalancing:RemoveTags"
             ],
             "Resource": "*"
         },

lib/ansible/modules/cloud/amazon/elb_target_group.py

@@ -304,50 +304,44 @@ vpc_id:
 '''
 
 import time
-import traceback
 
 try:
-    import boto3
+    import botocore
-    from botocore.exceptions import ClientError, NoCredentialsError
-    HAS_BOTO3 = True
 except ImportError:
-    HAS_BOTO3 = False
+    pass  # handled by AnsibleAWSModule
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.aws.core import AnsibleAWSModule
-from ansible.module_utils.ec2 import (boto3_conn, get_aws_connection_info, camel_dict_to_snake_dict,
+from ansible.module_utils.ec2 import (camel_dict_to_snake_dict, boto3_tag_list_to_ansible_dict, ec2_argument_spec,
-                                      ec2_argument_spec, boto3_tag_list_to_ansible_dict,
                                       compare_aws_tags, ansible_dict_to_boto3_tag_list)
+from distutils.version import LooseVersion
 
 
 def get_tg_attributes(connection, module, tg_arn):
-
     try:
         tg_attributes = boto3_tag_list_to_ansible_dict(connection.describe_target_group_attributes(TargetGroupArn=tg_arn)['Attributes'])
-    except ClientError as e:
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-        module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+        module.fail_json_aws(e, msg="Couldn't get target group attributes")
 
     # Replace '.' with '_' in attribute key names to make it more Ansibley
     return dict((k.replace('.', '_'), v) for k, v in tg_attributes.items())
 
 
 def get_target_group_tags(connection, module, target_group_arn):
-
     try:
         return connection.describe_tags(ResourceArns=[target_group_arn])['TagDescriptions'][0]['Tags']
-    except ClientError as e:
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-        module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+        module.fail_json_aws(e, msg="Couldn't get target group tags")
 
 
 def get_target_group(connection, module):
-
     try:
         target_group_paginator = connection.get_paginator('describe_target_groups')
         return (target_group_paginator.paginate(Names=[module.params.get("name")]).build_full_result())['TargetGroups'][0]
-    except ClientError as e:
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
         if e.response['Error']['Code'] == 'TargetGroupNotFound':
             return None
         else:
-            module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+            module.fail_json_aws(e, msg="Couldn't get target group")
 
 
 def wait_for_status(connection, module, target_group_arn, targets, status):
@@ -363,13 +357,19 @@ def wait_for_status(connection, module, target_group_arn, targets, status):
                 break
             else:
                 time.sleep(polling_increment_secs)
-        except ClientError as e:
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-            module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+            module.fail_json_aws(e, msg="Couldn't describe target health")
 
     result = response
     return status_achieved, result
 
 
+def fail_if_ip_target_type_not_supported(module):
+    if LooseVersion(botocore.__version__) < LooseVersion('1.7.2'):
+        module.fail_json(msg="target_type ip requires botocore version 1.7.2 or later. Version %s is installed" %
+                         botocore.__version__)
+
+
 def create_or_update_target_group(connection, module):
 
     changed = False
@@ -415,6 +415,8 @@ def create_or_update_target_group(connection, module):
     # Get target type
     if module.params.get("target_type") is not None:
         params['TargetType'] = module.params.get("target_type")
+        if params['TargetType'] == 'ip':
+            fail_if_ip_target_type_not_supported(module)
 
     # Get target group
     tg = get_target_group(connection, module)
@@ -471,8 +473,8 @@ def create_or_update_target_group(connection, module):
                 if health_check_params:
                     connection.modify_target_group(TargetGroupArn=tg['TargetGroupArn'], **health_check_params)
                     changed = True
-            except ClientError as e:
+            except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                module.fail_json_aws(e, msg="Couldn't update target group")
 
         # Do we need to modify targets?
         if module.params.get("modify_targets"):
@@ -484,8 +486,8 @@ def create_or_update_target_group(connection, module):
 
                 try:
                     current_targets = connection.describe_target_health(TargetGroupArn=tg['TargetGroupArn'])
-                except ClientError as e:
+                except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                    module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                    module.fail_json_aws(e, msg="Couldn't get target group health")
 
                 current_instance_ids = []
 
@@ -507,8 +509,8 @@ def create_or_update_target_group(connection, module):
                     changed = True
                     try:
                         connection.register_targets(TargetGroupArn=tg['TargetGroupArn'], Targets=instances_to_add)
-                    except ClientError as e:
+                    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                        module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                        module.fail_json_aws(e, msg="Couldn't register targets")
 
                     if module.params.get("wait"):
                         status_achieved, registered_instances = wait_for_status(connection, module, tg['TargetGroupArn'], instances_to_add, 'healthy')
@@ -526,8 +528,8 @@ def create_or_update_target_group(connection, module):
                     changed = True
                     try:
                         connection.deregister_targets(TargetGroupArn=tg['TargetGroupArn'], Targets=instances_to_remove)
-                    except ClientError as e:
+                    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                        module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                        module.fail_json_aws(e, msg="Couldn't remove targets")
 
                     if module.params.get("wait"):
                         status_achieved, registered_instances = wait_for_status(connection, module, tg['TargetGroupArn'], instances_to_remove, 'unused')
@@ -536,8 +538,8 @@ def create_or_update_target_group(connection, module):
             else:
                 try:
                     current_targets = connection.describe_target_health(TargetGroupArn=tg['TargetGroupArn'])
-                except ClientError as e:
+                except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                    module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                    module.fail_json_aws(e, msg="Couldn't get target health")
 
                 current_instances = current_targets['TargetHealthDescriptions']
 
@@ -549,8 +551,8 @@ def create_or_update_target_group(connection, module):
                     changed = True
                     try:
                         connection.deregister_targets(TargetGroupArn=tg['TargetGroupArn'], Targets=instances_to_remove)
-                    except ClientError as e:
+                    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                        module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                        module.fail_json_aws(e, msg="Couldn't remove targets")
 
                     if module.params.get("wait"):
                         status_achieved, registered_instances = wait_for_status(connection, module, tg['TargetGroupArn'], instances_to_remove, 'unused')
@@ -561,8 +563,8 @@ def create_or_update_target_group(connection, module):
             connection.create_target_group(**params)
             changed = True
             new_target_group = True
-        except ClientError as e:
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-            module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+            module.fail_json_aws(e, msg="Couldn't create target group")
 
         tg = get_target_group(connection, module)
 
@@ -570,8 +572,8 @@ def create_or_update_target_group(connection, module):
             params['Targets'] = module.params.get("targets")
             try:
                 connection.register_targets(TargetGroupArn=tg['TargetGroupArn'], Targets=params['Targets'])
-            except ClientError as e:
+            except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                module.fail_json_aws(e, msg="Couldn't register targets")
 
             if module.params.get("wait"):
                 status_achieved, registered_instances = wait_for_status(connection, module, tg['TargetGroupArn'], params['Targets'], 'healthy')
@@ -601,11 +603,11 @@ def create_or_update_target_group(connection, module):
         try:
             connection.modify_target_group_attributes(TargetGroupArn=tg['TargetGroupArn'], Attributes=update_attributes)
             changed = True
-        except ClientError as e:
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
             # Something went wrong setting attributes. If this target group was created during this task, delete it to leave a consistent state
             if new_target_group:
                 connection.delete_target_group(TargetGroupArn=tg['TargetGroupArn'])
-            module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+            module.fail_json_aws(e, msg="Couldn't delete target group")
 
     # Tags - only need to play with tags if tags parameter has been set to something
     if tags:
@@ -617,16 +619,16 @@ def create_or_update_target_group(connection, module):
         if tags_to_delete:
             try:
                 connection.remove_tags(ResourceArns=[tg['TargetGroupArn']], TagKeys=tags_to_delete)
-            except ClientError as e:
+            except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                module.fail_json_aws(e, msg="Couldn't delete tags from target group")
             changed = True
 
         # Add/update tags
         if tags_need_modify:
             try:
                 connection.add_tags(ResourceArns=[tg['TargetGroupArn']], Tags=ansible_dict_to_boto3_tag_list(tags_need_modify))
-            except ClientError as e:
+            except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+                module.fail_json_aws(e, msg="Couldn't add tags to target group")
             changed = True
 
     # Get the target group again
@@ -644,7 +646,6 @@ def create_or_update_target_group(connection, module):
 
 
 def delete_target_group(connection, module):
-
     changed = False
     tg = get_target_group(connection, module)
 
@@ -652,66 +653,53 @@ def delete_target_group(connection, module):
         try:
             connection.delete_target_group(TargetGroupArn=tg['TargetGroupArn'])
             changed = True
-        except ClientError as e:
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-            module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
+            module.fail_json_aws(e, msg="Couldn't delete target group")
 
     module.exit_json(changed=changed)
 
 
 def main():
-
     argument_spec = ec2_argument_spec()
     argument_spec.update(
         dict(
             deregistration_delay_timeout=dict(type='int'),
-            health_check_protocol=dict(choices=['http', 'https', 'tcp', 'HTTP', 'HTTPS', 'TCP'], type='str'),
+            health_check_protocol=dict(choices=['http', 'https', 'tcp', 'HTTP', 'HTTPS', 'TCP']),
             health_check_port=dict(),
-            health_check_path=dict(default=None, type='str'),
+            health_check_path=dict(),
             health_check_interval=dict(type='int'),
             health_check_timeout=dict(type='int'),
             healthy_threshold_count=dict(type='int'),
             modify_targets=dict(default=True, type='bool'),
-            name=dict(required=True, type='str'),
+            name=dict(required=True),
             port=dict(type='int'),
-            protocol=dict(choices=['http', 'https', 'tcp', 'HTTP', 'HTTPS', 'TCP'], type='str'),
+            protocol=dict(choices=['http', 'https', 'tcp', 'HTTP', 'HTTPS', 'TCP']),
             purge_tags=dict(default=True, type='bool'),
             stickiness_enabled=dict(type='bool'),
-            stickiness_type=dict(default='lb_cookie', type='str'),
+            stickiness_type=dict(default='lb_cookie'),
             stickiness_lb_cookie_duration=dict(type='int'),
-            state=dict(required=True, choices=['present', 'absent'], type='str'),
+            state=dict(required=True, choices=['present', 'absent']),
-            successful_response_codes=dict(type='str'),
+            successful_response_codes=dict(),
             tags=dict(default={}, type='dict'),
-            target_type=dict(type='str', default='instance', choices=['instance', 'ip']),
+            target_type=dict(default='instance', choices=['instance', 'ip']),
             targets=dict(type='list'),
             unhealthy_threshold_count=dict(type='int'),
-            vpc_id=dict(type='str'),
+            vpc_id=dict(),
             wait_timeout=dict(type='int'),
             wait=dict(type='bool')
         )
     )
 
-    module = AnsibleModule(argument_spec=argument_spec,
+    module = AnsibleAWSModule(argument_spec=argument_spec,
-                           required_if=[
+                              required_if=[['state', 'present', ['protocol', 'port', 'vpc_id']]])
-                               ('state', 'present', ['protocol', 'port', 'vpc_id'])
-                           ]
-                           )
 
-    if not HAS_BOTO3:
+    connection = module.client('elbv2')
-        module.fail_json(msg='boto3 required for this module')
 
-    region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True)
+    if module.params.get('state') == 'present':
-
-    if region:
-        connection = boto3_conn(module, conn_type='client', resource='elbv2', region=region, endpoint=ec2_url, **aws_connect_params)
-    else:
-        module.fail_json(msg="region must be specified")
-
-    state = module.params.get("state")
-
-    if state == 'present':
         create_or_update_target_group(connection, module)
     else:
         delete_target_group(connection, module)
 
+
 if __name__ == '__main__':
     main()

test/integration/targets/elb_target/aliases

@@ -1,2 +1,3 @@
 cloud/aws
 unsupported
+elb_target_group

test/integration/targets/elb_target/defaults/main.yml

@@ -1,4 +0,0 @@
----
-ec2_ami_image:
-  us-east-1: ami-8c1be5f6
-  us-east-2: ami-c5062ba0

test/integration/targets/elb_target/playbooks/full_test.yml

@@ -0,0 +1,5 @@
+- hosts: localhost
+  connection: local
+
+  roles:
+    - elb_target

test/integration/targets/elb_target/playbooks/roles/elb_target/defaults/main.yml

@@ -0,0 +1,7 @@
+---
+ec2_ami_image:
+  us-east-1: ami-8c1be5f6
+  us-east-2: ami-c5062ba0
+
+tg_name: "ansible-test-{{ resource_prefix | regex_search('([0-9]+)$') }}-tg"
+lb_name: "ansible-test-{{ resource_prefix | regex_search('([0-9]+)$') }}-lb"

test/integration/targets/elb_target/playbooks/roles/elb_target/tasks/main.yml

@@ -5,7 +5,6 @@
 
     - name:
       debug: msg="********** Setting up elb_target test dependencies **********"
-
     # ============================================================
 
     - name: set up aws connection info
@@ -19,16 +18,6 @@
 
     # ============================================================
 
-    - name: create target group name
-      set_fact:
-        tg_name: "ansible-test-{{ resource_prefix | regex_search('([0-9]+)$') }}-tg"
-
-    - name: create application load balancer name
-      set_fact:
-        lb_name: "ansible-test-{{ resource_prefix | regex_search('([0-9]+)$') }}-lb"
-
-    # ============================================================
-
     - name: set up testing VPC
       ec2_vpc_net:
         name: "{{ resource_prefix }}-vpc"

test/integration/targets/elb_target/playbooks/version_fail.yml

@@ -0,0 +1,33 @@
+- hosts: localhost
+  connection: local
+
+  tasks:
+    - name: set up aws connection info
+      set_fact:
+        aws_connection_info: &aws_connection_info
+          aws_access_key: madeup
+          aws_secret_key: madeup
+          security_token: madeup
+          region: "{{ aws_region }}"
+      no_log: yes
+
+    - name: set up testing target group (type=ip)
+      elb_target_group:
+        name: "ansible-test-{{ resource_prefix | regex_search('([0-9]+)$') }}-tg"
+        health_check_port: 80
+        protocol: http
+        port: 80
+        vpc_id: 'vpc-abcd1234'
+        state: present
+        target_type: ip
+        tags:
+          Description: "Created by {{ resource_prefix }}"
+        <<: *aws_connection_info
+      register: elb_target_group_type_ip
+      ignore_errors: yes
+
+    - name: check that setting up target group with type=ip fails with friendly message
+      assert:
+        that:
+          - elb_target_group_type_ip is failed
+          - "'msg' in elb_target_group_type_ip"

test/integration/targets/elb_target/runme.sh

@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+
+# We don't set -u here, due to pypa/virtualenv#150
+set -ex
+
+MYTMPDIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
+
+trap 'rm -rf "${MYTMPDIR}"' EXIT
+
+# This is needed for the ubuntu1604py3 tests
+# Ubuntu patches virtualenv to make the default python2
+# but for the python3 tests we need virtualenv to use python3
+PYTHON=${ANSIBLE_TEST_PYTHON_INTERPRETER:-python}
+
+# Test graceful failure for older versions of botocore
+virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/botocore-1.7.1"
+source "${MYTMPDIR}/botocore-1.7.1/bin/activate"
+$PYTHON -m pip install 'botocore<=1.7.1' boto3
+ansible-playbook -i ../../inventory -e @../../integration_config.yml -e @../../cloud-config-aws.yml -v playbooks/version_fail.yml "$@"
+
+# Run full test suite
+virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/botocore-recent"
+source "${MYTMPDIR}/botocore-recent/bin/activate"
+$PYTHON -m pip install 'botocore>=1.8.0' boto3
+ansible-playbook -i ../../inventory -e @../../integration_config.yml -e @../../cloud-config-aws.yml -v playbooks/full_test.yml "$@"
+