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

Add execution_role_arn parameter #41849

Merged
merged 9 commits into from
Jul 4, 2018
Merged

Conversation

mjmayer
Copy link
Contributor

@mjmayer mjmayer commented Jun 22, 2018

SUMMARY

Fargate support requires containers to have the executionRoleArn defined. This PR adds that parameter.
Fixes #41848

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ecs_taskdefinition

ANSIBLE VERSION
ansible 2.7.0.dev0 (devel 46dbb3e1e8) last updated 2018/06/22 10:32:41 (GMT -700)
  config file = None
  configured module search path = [u'/home/mjmayer/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/mjmayer/ansible/lib/ansible
  executable location = /home/mjmayer/ansible/bin/ansible
  python version = 2.7.5 (default, Apr 11 2018, 17:41:36) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28.0.1)]

@ansibot
Copy link
Contributor

ansibot commented Jun 22, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py:306:0: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py:306:73: W291 trailing whitespace

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jun 22, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 aws ci_verified Changes made in this PR are causing tests to fail. cloud feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Jun 22, 2018
Botocore version checking is becomming more common. Changing the ecs_taskdefinition
to use AnsibleAWSmodule allows more easily for this.
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 22, 2018
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Jun 22, 2018
@@ -319,6 +326,10 @@ def main():
if not task_mgr.ecs_api_supports_requirescompatibilities():
module.fail_json(msg='botocore needs to be version 1.8.4 or higher to use launch_type')

if module.params['task_role_arn']:
if not module.botocore_at_least('1.10.44'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this! Do you mind doing the same for launch_type above by replacing if not task_mgr.ecs_api_supports_requirescompatibilities(): with if not module.botocore_at_least('1.8.4'):?

@@ -225,6 +230,7 @@ def register_task(self, family, task_role_arn, network_mode, container_definitio
params = dict(
family=family,
taskRoleArn=task_role_arn,
executionRoleArn=execution_role_arn,
Copy link
Contributor

Choose a reason for hiding this comment

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

executionRoleArn must not be set if it's not required, otherwise anyone relying on this module will need to upgrade to a very recent botocore.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jun 24, 2018
@willthames
Copy link
Contributor

It's worth noting that this fails the existing test suite.

<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /root/.ansible/tmp/ansible-tmp-1529926266.6214693-137085483742155/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_e53r2y2w/ansible_module_ecs_taskdefinition.py", line 488, in <module>
    main()
  File "/tmp/ansible_e53r2y2w/ansible_module_ecs_taskdefinition.py", line 456, in main
    module.params['memory'])
  File "/tmp/ansible_e53r2y2w/ansible_module_ecs_taskdefinition.py", line 246, in register_task
    response = self.ecs.register_task_definition(**params)
  File "/usr/lib/python3.6/site-packages/botocore/client.py", line 312, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/lib/python3.6/site-packages/botocore/client.py", line 579, in _make_api_call
    api_params, operation_model, context=request_context)
  File "/usr/lib/python3.6/site-packages/botocore/client.py", line 634, in _convert_to_request_dict
    api_params, operation_model)
  File "/usr/lib/python3.6/site-packages/botocore/validate.py", line 291, in serialize_to_request
    raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Unknown parameter in input: "executionRoleArn", must be one of: family, taskRoleArn, networkMode, containerDefinitions, volumes, placementConstraints

fatal: [localhost]: FAILED! => {
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_e53r2y2w/ansible_module_ecs_taskdefinition.py\", line 488, in <module>\n    main()\n  File \"/tmp/ansible_e53r2y2w/ansible_module_ecs_taskdefinition.py\", line 456, in main\n    module.params['memory'])\n  File \"/tmp/ansible_e53r2y2w/ansible_module_ecs_taskdefinition.py\", line 246, in register_task\n    response = self.ecs.register_task_definition(**params)\n  File \"/usr/lib/python3.6/site-packages/botocore/client.py\", line 312, in _api_call\n    return self._make_api_call(operation_name, kwargs)\n  File \"/usr/lib/python3.6/site-packages/botocore/client.py\", line 579, in _make_api_call\n    api_params, operation_model, context=request_context)\n  File \"/usr/lib/python3.6/site-packages/botocore/client.py\", line 634, in _convert_to_request_dict\n    api_params, operation_model)\n  File \"/usr/lib/python3.6/site-packages/botocore/validate.py\", line 291, in serialize_to_request\n    raise ParamValidationError(report=report.generate_report())\nbotocore.exceptions.ParamValidationError: Parameter validation failed:\nUnknown parameter in input: \"executionRoleArn\", must be one of: family, taskRoleArn, networkMode, containerDefinitions, volumes, placementConstraints\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE",
    "rc": 1
}

@willthames
Copy link
Contributor

Could we have a test for this change?

@ansibot ansibot added the test This PR relates to tests. label Jun 26, 2018
- AmazonEC2ContainerServiceRole
<<: *aws_connection_info

- name: gather facts for the ecsTaskExecutionRole
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be able to access this from registering the results of the previous task

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 registered the output from iam_role task and reran the tests. The tests passed.

Task was unecessary. The same information could be gathered by registering
the iam_role task.
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 27, 2018
@willthames willthames merged commit b60fc33 into ansible:devel Jul 4, 2018
@willthames
Copy link
Contributor

Merged, thanks @mjmayer

@ansible ansible locked and limited conversation to collaborators Jul 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 aws cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ecs_taskdefinition does not support execution_role_arn
4 participants