ec2_eni: Fix idempotency when security_groups is specified

[ichekaldin]

SUMMARY

If security_groups attribute is specified - module always returns Changed = True.

Fixes #255.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

ec2_eni

ADDITIONAL INFORMATION

Example:

- amazon.aws.ec2_eni:
    name: my-eni
    security_groups:
      - my-sg
    subnet_id: subnet-123456

Will always return Changed = True.

[ansibullbot]

cc @healem @jillr @s-hertel @tremble @wimnat
click here for bot help

[ichekaldin]

Strange. Integration tests fail with the following error:

ERROR! couldn't resolve module/action 'ec2_instance'. This often indicates a misspelling, missing collection, or incorrect module path.

The error appears to be in '/home/zuul/.ansible/collections/ansible_collections/amazon/aws/tests/integration/targets/ec2_eni/tasks/main.yaml': line 71, column 5, but may be elsewhere in the file depending on the exact syntax problem.

The line in question is:

  - name: Create 2 instances to test attaching and detaching network interfaces
    ec2_instance:

It has nothing to do with the changes I made. Has this integration test worked previously?

[goneri]

recheck

[goneri]

recheck

[ichekaldin]

The test fails with the following error:

TASK [ec2_eni : Assert that ec2_eni_info returns all the values we expect] *****
task path: /home/zuul/.ansible/collections/ansible_collections/amazon/aws/tests/integration/targets/ec2_eni/tasks/test_eni_basic_creation.yaml:24
[WARNING]: an unexpected error occurred during Jinja2 environment setup: unable
to locate collection ansible.netcommon
exception during Jinja2 environment setup: Traceback (most recent call last):
  File "/tmp/ansible-test-bntq2_0j/ansible/utils/collection_loader/_collection_finder.py", line 1058, in _get_collection_metadata
    collection_pkg = import_module('ansible_collections.' + collection_name)
  File "/usr/lib64/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 953, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'ansible_collections.ansible.netcommon'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/ansible-test-bntq2_0j/ansible/template/__init__.py", line 485, in __getitem__
    ts = _get_collection_metadata(acr.collection)
  File "/tmp/ansible-test-bntq2_0j/ansible/utils/collection_loader/_collection_finder.py", line 1060, in _get_collection_metadata
    raise ValueError('unable to locate collection {0}'.format(collection_name))
ValueError: unable to locate collection ansible.netcommon
fatal: [testhost]: FAILED! => {
    "msg": "The conditional check '_interface_0.private_ip_address | ansible.netcommon.ipaddr' failed. The error was: template error while templating string: unable to locate collection ansible.netcommon. String: {% if _interface_0.private_ip_address | ansible.netcommon.ipaddr %} True {% else %} False {% endif %}"
}

I think I hit a wall in trying to fix it.

I'm inclined to remove lines in 60 and 155 in tests/integration/targets/ec2_eni/tasks/test_eni_basic_creation.yaml:

      - _interface_0.private_ip_address | ansible.netcommon.ipaddr

and

      - _interface_0.private_ip_address | ansible.netcommon.ipaddr

My thinking here is that the appropriate validation is done by the subsequent lines:

      - _interface_0.private_ip_address == ip_1

and

      - _interface_0.private_ip_address == ip_5

Would that be acceptable?

[tremble]

ansible.netcommon.ipaddr feels like something we should have available to us during testing, many of our modules return IP addresses, and being able to validate this seems like core testing functionality.

Originally we had

• ansible.netcommon
• community.general
• community.aws

While I think it's reasonable to minimise our test dependencies, supported collections (like ansible.netcommon) feel like something we should have available to us.

@goneri thoughts?

[ichekaldin]

I realize that everyone is probably busy after release 1.5.0, but still wanted to follow up on this. What's the best way to proceed?

[jillr]

@tremble @ichekaldin I have no objection to adding ansible.netcommon back to tests/requirements.yml. It looks like these tests was missed when we recently removed ansible.netcommon from the requirements after other, unrelated usages were removed. Collections in that file will be installed to the test environment when integration tests run.

[ichekaldin]

@jillr @tremble What can we do to move this forward? Thanks!

[jillr]

recheck

[goneri]

recheck

[ichekaldin]

@jillr @goneri Is something blocking this PR?

[jillr]

Hi @ichekaldin apologies, just needed to be reviewed once the CI was passing.

[jillr]

recheck

[jillr]

gate job isn't running because one of the zuul changes hasn't merged, we'll get that merged asap

[ichekaldin]

@jillr Looks like the zuul changes are merged. Could this be merged as well?

[tremble]

@ichekaldin - Apologies for any spam, trying to kick the CI

[tremble]

'check' was automatically cancelled because gate's now running - as long as gate passes it should merge.

[ichekaldin]

@tremble looks like the gate test fails with this:

TASK [ec2_eni : Assert that ec2_eni_info returns all the values we expect] *****
task path: /home/zuul/.ansible/collections/ansible_collections/amazon/aws/tests/integration/targets/ec2_eni/tasks/test_eni_basic_creation.yaml:24
fatal: [testhost]: FAILED! => {
    "msg": "The conditional check '_interface_0.private_ip_address | ansible.netcommon.ipaddr' failed. The error was: The ipaddr filter requires python's netaddr be installed on the ansible controller"
}

[ichekaldin]

@tremble Thank you!