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

ec2_vol_facts: moved to boto3 #43348

Merged
merged 13 commits into from
Aug 31, 2018
92 changes: 52 additions & 40 deletions lib/ansible/modules/cloud/amazon/ec2_vol_facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
description:
- Gather facts about ec2 volumes in AWS
version_added: "2.1"
requirements: [ boto3 ]
author: "Rob White (@wimnat)"
options:
filters:
Expand Down Expand Up @@ -59,57 +60,67 @@
import traceback

try:
import boto.ec2
from boto.exception import BotoServerError
HAS_BOTO = True
from botocore.exceptions import ClientError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module should handle BotoCoreError and ClientError. ProfileNotFound should be handled by boto3_conn and doesn't need to be imported by the module.

except ImportError:
HAS_BOTO = False
pass # caught by imported HAS_BOTO3

from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.ec2 import connect_to_aws, ec2_argument_spec, get_aws_connection_info
from ansible.module_utils.ec2 import connect_to_aws, ec2_argument_spec, get_aws_connection_info, boto3_conn, HAS_BOTO3, boto3_tag_list_to_ansible_dict
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could update this module to AnsibleAWSModule if you wanted. That would simplify correcting the exception handling and eliminate the need to use/import all of:
connect_to_aws, ec2_argument_spec, get_aws_connection_info, boto3_conn, and HAS_BOTO3

Doing this would end up looking something like:

from ansible.module_utils.aws.core import AnsibleAWSModule
...
module = AnsibleAWSModule(argument_spec=argument_spec)
connection = module.client('ec2')

And your exception handling would look like:

try:
    connection.describe_volumes(Filters=ansible_dict_to_boto3_filter_list(sanitized_filters))
except (BotoCoreError, ClientError) as e:
    module.fail_json_aws(e, msg="Unable to describe volumes with filters {0}".format(sanitized_filters))

Otherwise you end up having a mess handling the different cases of what attributes exist for what exception because they don't inherit the same properties (such as, you should include e.message and e.response if they exist, but e.message does not exist on Python 3 and BotoCoreErrors never have a response attribute).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't manage to get value for region in this case and it has to be presented in module output.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, no problem either way. You could still get region via get_aws_connection_info, which is how AnsibleAWSModule gets it too.

from ansible.module_utils.ec2 import ansible_dict_to_boto3_filter_list, camel_dict_to_snake_dict
from ansible.module_utils._text import to_native


def get_volume_info(volume):
def get_volume_info(volume, region):

attachment = volume.attach_data
attachment = volume["attachments"]

volume_info = {
'create_time': volume.create_time,
'id': volume.id,
'encrypted': volume.encrypted,
'iops': volume.iops,
'size': volume.size,
'snapshot_id': volume.snapshot_id,
'status': volume.status,
'type': volume.type,
'zone': volume.zone,
'region': volume.region.name,
'create_time': volume["create_time"],
'id': volume["volume_id"],
'encrypted': volume["encrypted"],
'iops': volume["iops"] if "iops" in volume else None,
'size': volume["size"],
'snapshot_id': volume["snapshot_id"],
'status': volume["state"],
'type': volume["volume_type"],
'zone': volume["availability_zone"],
'region': region,
'attachment_set': {
'attach_time': attachment.attach_time,
'device': attachment.device,
'instance_id': attachment.instance_id,
'status': attachment.status
'attach_time': attachment[0]["attach_time"] if len(attachment) > 0 else None,
'device': attachment[0]["device"] if len(attachment) > 0 else None,
'instance_id': attachment[0]["instance_id"] if len(attachment) > 0 else None,
'status': attachment[0]["state"] if len(attachment) > 0 else None,
'delete_on_termination': attachment[0]["delete_on_termination"] if len(attachment) > 0 else None
},
'tags': volume.tags
'tags': boto3_tag_list_to_ansible_dict(volume['tags'])
}

return volume_info


def list_ec2_volumes(connection, module):
def describe_volumes_with_backoff(connection, filters):
paginator = connection.get_paginator('describe_volumes')
return paginator.paginate(Filters=filters).build_full_result()

filters = module.params.get("filters")

def list_ec2_volumes(connection, module, region):

# Replace filter key underscores with dashes, for compatibility, except if we're dealing with tags
sanitized_filters = module.params.get("filters")
for key in sanitized_filters:
if not key.startswith("tag:"):
sanitized_filters[key.replace("_", "-")] = sanitized_filters.pop(key)
volume_dict_array = []

try:
all_volumes = connection.get_all_volumes(filters=filters)
except BotoServerError as e:
module.fail_json(msg=e.message)
all_volumes = describe_volumes_with_backoff(connection, ansible_dict_to_boto3_filter_list(sanitized_filters))

for volume in all_volumes:
volume_dict_array.append(get_volume_info(volume))
except ClientError as e:
module.fail_json(msg=e.response, exception=traceback.format_exc())

for volume in all_volumes["Volumes"]:
volume = camel_dict_to_snake_dict(volume, ignore_list=['Tags'])
volume_dict_array.append(get_volume_info(volume, region))
module.exit_json(volumes=volume_dict_array)


Expand All @@ -123,20 +134,21 @@ def main():

module = AnsibleModule(argument_spec=argument_spec)

if not HAS_BOTO:
module.fail_json(msg='boto required for this module')
if not HAS_BOTO3:
module.fail_json(msg='boto3 required for this module')

region, ec2_url, aws_connect_params = get_aws_connection_info(module)
region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True)

if region:
try:
connection = connect_to_aws(boto.ec2, region, **aws_connect_params)
except (boto.exception.NoAuthHandlerFound, Exception) as e:
module.fail_json(msg=to_native(e), exception=traceback.format_exc())
else:
module.fail_json(msg="region must be specified")
connection = boto3_conn(
module,
conn_type='client',
resource='ec2',
region=region,
endpoint=ec2_url,
**aws_connect_params
)

list_ec2_volumes(connection, module)
list_ec2_volumes(connection, module, region)


if __name__ == '__main__':
Expand Down
2 changes: 2 additions & 0 deletions test/integration/targets/ec2_vol_facts/aliases
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
cloud/aws
unsupported
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like testing this PR is blocked on CI updates that @s-hertel is working on.

We should wait for those updates rather than disabling the tests.

3 changes: 3 additions & 0 deletions test/integration/targets/ec2_vol_facts/meta/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
dependencies:
- prepare_tests
- setup_ec2
111 changes: 111 additions & 0 deletions test/integration/targets/ec2_vol_facts/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
---
# tasks file for test_ec2_vol_facts
- name: Set up AWS connection info
set_fact:
aws_connection_info: &aws_connection_info
aws_access_key: "{{ aws_access_key }}"
aws_secret_key: "{{ aws_secret_key }}"
security_token: "{{ security_token }}"
region: "{{ aws_region }}"
no_log: true

- block:
- ec2_ami_facts:
<<: *aws_connection_info
filters:
architecture: x86_64
virtualization-type: hvm
root-device-type: ebs
name: "amzn-ami-hvm*"
register: amis

- name: Create test instance
ec2_instance:
name: "{{ resource_prefix }}_ansible_ec2_vol_facts_test"
instance_type: t2.nano
image_id: "{{ (amis.images | sort(attribute='creation_date') | last).image_id }}"
wait: yes
tags:
Environment: test
<<: *aws_connection_info
register: instance

- name: Ensure there's only one matching instance
assert:
that:
- "instance.instance_ids|length == 1"
- "instance.instances|length == 1"

- name: Create test volume
ec2_vol:
instance: "{{ instance.instance_ids[0] }}"
volume_size: 4
name: "{{ resource_prefix }}_ansible_ec2_vol_facts_test.db"
device_name: /dev/xvdf
iops: 100
tags:
Tag Name with Space-and-dash: Tag Value with Space-and-dash
<<: *aws_connection_info
delete_on_termination: yes
register: volume

- name: Gather volume info
ec2_vol_facts:
<<: *aws_connection_info
filters:
"tag:Name": "{{ resource_prefix }}_ansible_ec2_vol_facts_test.db"
register: volume_facts
check_mode: no

- name: Format check
assert:
that:
- "volume_facts.volumes|length == 1"
- "v.attachment_set.attach_time is defined"
- "v.attachment_set.device is defined and v.attachment_set.device == volume.device"
- "v.attachment_set.instance_id is defined and v.attachment_set.instance_id == instance.instance_ids[0]"
- "v.attachment_set.status is defined and v.attachment_set.status == 'attached'"
- "v.create_time is defined"
- "v.encrypted is defined and v.encrypted == false"
- "v.id is defined and v.id == volume.volume_id"
- "v.iops is defined and v.iops == 100"
- "v.region is defined and v.region == aws_region"
- "v.size is defined and v.size == 4"
- "v.snapshot_id is defined and v.snapshot_id == ''"
- "v.status is defined and v.status == 'in-use'"
- "v.tags.Name is defined and v.tags.Name == resource_prefix + '_ansible_ec2_vol_facts_test.db'"
- "v.tags['Tag Name with Space-and-dash'] == 'Tag Value with Space-and-dash'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails - this is the only reference to this tag.

- "v.type is defined and v.type == 'io1'"
- "v.zone is defined and v.zone == instance.instances[0].placement.availability_zone"
vars:
v: "{{ volume_facts.volumes[0] }}"

- name: New format check
assert:
that:
- "v.attachment_set.delete_on_termination is defined"
vars:
v: "{{ volume_facts.volumes[0] }}"
when: ansible_version.full is version('2.7', '>=')

always:
- name: Remove the instance
ec2_instance:
state: absent
filters:
"tag:Name": "{{ resource_prefix }}_ansible_ec2_vol_facts_test"
<<: *aws_connection_info
register: result
until: result is not failed
ignore_errors: yes
retries: 10

- name: Remove the volume
ec2_vol:
id: "{{ volume.volume_id }}"
state: absent
<<: *aws_connection_info
register: result
until: result is not failed
ignore_errors: yes
retries: 10