Move module_utils out of subdir

[flowerysong]

SUMMARY

The migration script preserved the separation of (most of) our util files into an aws subdirectory in module_utils; it would be nice to get rid of this naming redundancy/inconsistency before the collection has a stable release and starts being widely used.

The split repos make this complicated to do without breaking CI. This PR creates the new files and adjusts all of the imports on this side. ansible-collections/community.aws#23 will theoretically pass once this is merged, and then it will be possible to remove the old files in a third PR.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

[jillr]

This largely looks good, I want to pull this and the community PR down and do a bunch of extra testing to be safe though.

[bcoca]

just to note, that ansible-base "routing" will include module_utils for backwards compatibility with 2.9, I would test/ensure that it works with this relocation. Worst case scenario you might be able to just make the old locations symlinks (as long as the internals are also not rearranged).

[flowerysong]

@bcoca Redirection of module_utils is pretty broken right now, and I don't think this PR introduces any new concerns.

I was able to get ansible/ansible#67684 working for from ansible.module_utils.ec2 import HAS_BOTO3 by updating routing.yml, but from ansible.module_utils.aws.s3 import HAS_MD5 always resulted in an error regardless of where I tried to point it.

routing.yml diff:

       redirect: amazon.aws.iam
     rds:
       redirect: amazon.aws.rds
+    # Try pointing at a destination we know loads correctly
     s3:
-      redirect: amazon.aws.s3
+      redirect: ansible_collections.amazon.aws.plugins.module_utils.ec2
     waf:
       redirect: amazon.aws.waf
     waiters:
       redirect: amazon.aws.waiters
     ec2:
-      redirect: amazon.aws.ec2
+      redirect: ansible_collections.amazon.aws.plugins.module_utils.ec2
     ipaddress:
       redirect: f5networks.f5_modules.ipaddress
     network:

Error:

TASK [test_ping] ***************************************************************
task path: /home/ec2-user/testing/test.yml:3
The full traceback is:
Traceback (most recent call last):
  File "/home/ec2-user/testing/ansible/lib/ansible/executor/task_executor.py", line 146, in run
    res = self._execute()
  File "/home/ec2-user/testing/ansible/lib/ansible/executor/task_executor.py", line 645, in _execute
    result = self._handler.run(task_vars=variables)
  File "/home/ec2-user/testing/ansible/lib/ansible/plugins/action/normal.py", line 46, in run
    result = merge_hash(result, self._execute_module(task_vars=task_vars, wrap_async=wrap_async))
  File "/home/ec2-user/testing/ansible/lib/ansible/plugins/action/__init__.py", line 822, in _execute_module
    (module_style, shebang, module_data, module_path) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars)
  File "/home/ec2-user/testing/ansible/lib/ansible/plugins/action/__init__.py", line 216, in _configure_module
    **become_kwargs)
  File "/home/ec2-user/testing/ansible/lib/ansible/executor/module_common.py", line 1287, in modify_module
    environment=environment)
  File "/home/ec2-user/testing/ansible/lib/ansible/executor/module_common.py", line 1124, in _find_module_utils
    py_module_cache, zf)
  File "/home/ec2-user/testing/ansible/lib/ansible/executor/module_common.py", line 871, in recursive_finder
    [os.path.join(p, *relative_module_utils[:-1]) for p in module_utils_paths])
  File "/home/ec2-user/testing/ansible/lib/ansible/executor/module_common.py", line 612, in __init__
    raise ImportError("No module named '%s'" % name)
ImportError: No module named '__init__'
fatal: [localhost]: FAILED! => {
    "msg": "Unexpected failure during module execution.",
    "stdout": ""
}

Adding an empty lib/ansible/module_utils/aws/__init__.py fixes this and then redirection works more or less as I would expect, though the use of bare filenames as the routing key means that ansible.module_utils.ec2, ansible.module_utils.aws.ec2, ansible.module_utils.common.ec2 etc. can all be successfully imported.

[bcoca]

^ @nitzmahone as the person that can 'unbreak' the redirection issues

[tremble]

@flowerysong thanks for this, I think this is definitely the right move now that we have our own namespace to use.

[jillr]

@flowerysong I haven't forgotten about this PR, just waiting on 67684

[tremble]

@flowerysong I've split off the unit test fix so that we can get that in without the rest of this change.

Given we'll need to coordinate this change across the two repos I don't know how long it'll take to get the module_util migrations into place, we should at least get these unit tests fixed up.

[tremble]

@jillr @s-hertel Some follow up questions:

Since @flowerysong has taken the time to clean up the imports and the ordering how would you feel about sanity tests for:

1. import order
2. unused imports
3. multiple imports on a single line

[jillr]

@tremble I'm +1 unused imports, less so the others. I'd prefer not to get too strict on formatting via sanity tests without a much larger conversation and documented guidelines as I think that could increase contributor friction without a ton of benefit.

The network team is using black for their collections so there's a model there we could work from but we'll need to do more than just turning on sanity tests for it.

[jillr]

67684 has merged, additional local testing on devel and 2.9 LGTM. What do you think @tremble @s-hertel ?

[tremble]

+1 from my side.

[s-hertel]

Also +1

[jillr]

Thanks very much for this work @flowerysong!