ec2_vol: boto3 migration (migrating ansible/ansible#45500 to amazon.aws)

[falcon78921]

SUMMARY

In order to keep the boto3 migration work in ansible/ansible#45500 alive, I attempted to add
the changes that were reviewed previously into a new PR.

ansible/ansible#45500 and some of the changes made in this PR were courtesy of @zeenlym.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

plugins/modules/ec2_vol.py

[tremble]

Thanks for porting this over and getting this boto3-update moving again. A couple of minor niggles while you're still working on this

[falcon78921]

Just wanted to say I'm still working on this PR.

[falcon78921]

@tremble I've been working on integrating the paginator for get_volume() and get_volumes(), per the previous PR's review.

I'm able to gather results using something like this:

def getVolume():
    vols = []
    paginator = ec2_conn.get_paginator('describe_volumes')
    vols_response = paginator.paginate(Filters=[{'Name':'volume-id', 'Values':["{}".format(vol_id)]}])
    for p in vols_response:
        vols.append(p['Volumes'])
    return vols[0]

def getVolumes():
    vols = []
    paginator = ec2_conn.get_paginator('describe_volumes')
    vols_response = paginator.paginate(Filters=[{'Name':'attachment.instance-id', 'Values':["{}".format(instance)]}])
    for p in vols_response:
        vols.append(p['Volumes'])
    return vols

I'm just a little lost on how to integrate the preceding back into ec2_vol.py. The Paginator object returns an iterator, but I'm not sure if this may change behaviors in other functions.

[falcon78921]

@tremble Ok, I think I have the paginator working now. I tested the changes against my EC2 instance and the playbook appeared to work.

Other than implementing the paginator and some minor formatting fixes, there were three things left:

ansible/ansible#45500 (comment)
ansible/ansible#45500 (comment)
ansible/ansible#45500 (comment)

I'll keep moving forward with the requested changes.

[falcon78921]

@s-hertel Regarding ansible/ansible#45500 (comment), do you mean removing changed = True and just letting ec2_conn.delete_volume() run? If it fails, botocore can return why.

[ansibullbot]

cc @jillr @lwade @wimnat
click here for bot help

[falcon78921]

In my AWS environment, ec2_vol seems to work when running through tests in main/tests/integration/targets/ec2_vol/tasks/main.yml.

[tremble]

In my AWS environment, ec2_vol seems to work when running through tests in main/tests/integration/targets/ec2_vol/tasks/main.yml.

Interesting, they're working for me. I wonder if there's something going on due to the way CI works...

[falcon78921]

@tremble 🤔

[tremble]

I've done some digging and it looks like the ec2 instance isn't reporting the new volume in its description. This is after the waiter's done its thing.

By the time I run ec2_instance_info after the failed test it's showing up. I suspect that this is a very subtle race condition in the AWS APIs.

[tremble]

So... I've got something that seems to work: de6a699

Looks like this is a race condition of some variety: Basically the waiter which looks at the volume returns with "I'm attached to something" before the instance description includes the new volume.

[falcon78921]

@tremble Thanks for investigating the testing oddity. I applied your diff.

[tremble]

Thanks for working on getting this boto3 migration in :)

[falcon78921]

@tremble Looks like there's still one test failing, but the rest are passing! 🎉

[tremble]

2 last minor changes and then I think we're ready to get this merged.

In theory we could also make more use of AWSRetry, but I'd rather see this merged and I'll CC you on the relevant change so you could see how it would work.

[falcon78921]

@tremble 👍

[tremble]

Thanks for seeing this through. Scratch one more module off the boto v2 list.