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_eni: Move to boto3, add support for tags and name #54950

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@healem
Copy link
Contributor

healem commented Apr 7, 2019

SUMMARY

This PR contains 2 major changes to ec2_eni:

  1. Move the module to boto3 from boto (boto is no longer used by this module)
  2. Add support for tags and add a name field (that uses tag:Name in AWS)

This also adds a new way to find an interface - by name and subnet_id. This will make it
easier to remain idempotent when using dynamic ip addresses.

I've also squashed a minor bug where secondary ip addresses were not included in the
output. #54630 addresses this bug, but in the old boto-dependent code.

This closes #54630

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_eni

ADDITIONAL INFORMATION

Sanity checks run locally against python 2.7, 3.5, and 3.7 using the following command and varying the python version:
ansible-test sanity -v --docker --python 2.7 ec2_eni

Manually ran the integration test:
ansible-test integration ec2_eni --docker --python 3.7

Manual testing against AWS performed with ansible version:

ansible 2.8.0a1.post0
  config file = None
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /ansible/lib/ansible
  executable location = /ansible/bin/ansible
  python version = 3.7.3rc1 (default, Mar 13 2019, 11:01:15) [GCC 8.3.0]

Before:

 "interface": {
        "description": "myComplexNic descriptn",
        "groups": {
            "sg-0609923bf1372031d": "onlySshAccess"
        },
        "id": "eni-0539ff06598413abc",
        "mac_address": "0a:72:3d:f1:1c:b0",
        "owner_id": "686104308526",
        "private_ip_address": "10.0.84.2",
        "private_ip_addresses": [
            {
                "primary_address": true,
                "private_ip_address": "10.0.84.2"
            }
        ],
        "source_dest_check": true,
        "status": "pending",
        "subnet_id": "subnet-02dc6a04a5460ea5f",
        "vpc_id": "vpc-00fd8af7d3a908c13"
    },

After:

"interface": {
        "description": "myComplexNic descriptn",
        "groups": {
            "sg-0609923bf1372031d": "onlySshAccess"
        },
        "id": "eni-0e6eb8bf710e26822",
        "mac_address": "0a:24:b8:de:13:e8",
        "name": "myENI-1",
        "owner_id": "686104308526",
        "private_ip_address": "10.0.84.2",
        "private_ip_addresses": [
            {
                "primary_address": true,
                "private_ip_address": "10.0.84.2"
            },
            {
                "primary_address": false,
                "private_ip_address": "10.0.18.102"
            }
        ],
        "source_dest_check": true,
        "status": "available",
        "subnet_id": "subnet-02dc6a04a5460ea5f",
        "tags": {
            "Name": "myENI-1",
            "another": "tag",
            "group": "Finance"
        },
        "vpc_id": "vpc-00fd8af7d3a908c13"
    }
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 7, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Apr 7, 2019

@healem, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@jeking3
Copy link
Contributor

jeking3 left a comment

A few questions... also this needs an integration test (I know it didn't have one before though).

Show resolved Hide resolved lib/ansible/modules/cloud/amazon/ec2_eni.py Outdated
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/ec2_eni.py
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/ec2_eni.py
['secondary_private_ip_addresses', 'secondary_private_ip_address_count']
],
required_if=([
('attached', True, ['instance_id']),

This comment has been minimized.

Copy link
@jeking3

jeking3 Apr 7, 2019

Contributor

The state: absent qualifier was dropped. Is that ok?

This comment has been minimized.

Copy link
@healem

healem Apr 7, 2019

Author Contributor

Yes. Otherwise you can only delete an eni by eni_id. I dropped it to allows delete of the eni by (name and subnet_id), (subnet_id and private_ip_address), or (instance_id and device_index).

This comment has been minimized.

Copy link
@healem

healem Apr 7, 2019

Author Contributor

Leaving this conversation open - its worth getting feedback from others on this point.

@jeking3

This comment has been minimized.

Copy link
Contributor

jeking3 commented Apr 7, 2019

You could take the integration test from #54630 as a start for integration testing.

@healem healem force-pushed the healem:eni_boto3_tags branch from 9ff6a10 to fe90538 Apr 7, 2019

@healem

This comment has been minimized.

Copy link
Contributor Author

healem commented Apr 7, 2019

I added the integration test from #54630 and extended it. I also addressed the feedback provided by @jeking3

@healem

This comment has been minimized.

Copy link
Contributor Author

healem commented Apr 7, 2019

The failure is the same failure seen in #54630 regarding permissions in the CI system.

"msg": "Failed to create eni shippable-118040-102-eni for subnet-08ef34b3891cda01d in vpc-03fbae6810df95fc6 with None: An error occurred (UnauthorizedOperation) when calling the CreateNetworkInterface operation: You are not authorized to perform this operation."
@jeking3

This comment has been minimized.

Copy link
Contributor

jeking3 commented Apr 13, 2019

I can confirm these changes work and provide idempotency if you specify the name and subnet_id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.