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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aws ecs taskdefinition #53926

Open
wants to merge 3 commits into
base: devel
from

Conversation

Projects
None yet
2 participants
@thisdougb
Copy link
Contributor

thisdougb commented Mar 17, 2019

SUMMARY

Memory, cpu, and memoryReservation are integer types, but non-int values were not handled correctly. Put a try/except block around the cast of parameter values to int. I'm not sure the error message is good, I dislike the single quotes around the parameter name. Any better way to do this?

Also, the type for the memory param in argument_spec was str, so corrected to int.

Fixes #53915 which was a (still needed) documentation fix.

I didn't raise a bug, just felt shamed into fixing this after the above was commented on @willthames 馃榿

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ecs_taskdefinition

ADDITIONAL INFORMATION

This was the failure output before the change (memory: 1G):

TASK [aws_ecs_task_definition : Create task definition] *****************************************************************************************************************************************************
failed: [frontend.ansible] (item={'name': 'tls-stream-server', 'containers': [{'name': 'nginx', 'essential': True, 'image': 'nginx', 'portMappings': [{'containerPort': 8080, 'hostPort': 8080}], 'cpu': 512, 'memory': '1G', 'environment': [{'name': 'testkey', 'value': 'testvar'}, {'name': 'test2key', 'value': 'testvar'}]}]}) => {"changed": false, "item": {"containers": [{"cpu": 512, "environment": [{"name": "testkey", "value": "testvar"}, {"name": "test2key", "value": "testvar"}], "essential": true, "image": "nginx", "memory": "1G", "name": "nginx", "portMappings": [{"containerPort": 8080, "hostPort": 8080}]}], "name": "tls-stream-server"}, "module_stderr": "/Users/dougb/.ansible/tmp/ansible-tmp-1552849163.397866-69804585829101/AnsiballZ_ecs_taskdefinition.py:17: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses\n  import imp\nTraceback (most recent call last):\n  File \"/Users/dougb/.ansible/tmp/ansible-tmp-1552849163.397866-69804585829101/AnsiballZ_ecs_taskdefinition.py\", line 113, in <module>\n    _ansiballz_main()\n  File \"/Users/dougb/.ansible/tmp/ansible-tmp-1552849163.397866-69804585829101/AnsiballZ_ecs_taskdefinition.py\", line 105, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/Users/dougb/.ansible/tmp/ansible-tmp-1552849163.397866-69804585829101/AnsiballZ_ecs_taskdefinition.py\", line 48, in invoke_module\n    imp.load_module('__main__', mod, module, MOD_DESC)\n  File \"/usr/local/Cellar/python/3.7.2_2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/imp.py\", line 234, in load_module\n    return load_source(name, filename, file)\n  File \"/usr/local/Cellar/python/3.7.2_2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/imp.py\", line 169, in load_source\n    module = _exec(spec, sys.modules[name])\n  File \"<frozen importlib._bootstrap>\", line 630, in _exec\n  File \"<frozen importlib._bootstrap_external>\", line 728, in exec_module\n  File \"<frozen importlib._bootstrap>\", line 219, in _call_with_frames_removed\n  File \"/var/folders/7w/lnj27mtd4238ll_r_7wx4gb80000gn/T/ansible_ecs_taskdefinition_payload_tuz8i5hl/__main__.py\", line 497, in <module>\n  File \"/var/folders/7w/lnj27mtd4238ll_r_7wx4gb80000gn/T/ansible_ecs_taskdefinition_payload_tuz8i5hl/__main__.py\", line 465, in main\n  File \"/var/folders/7w/lnj27mtd4238ll_r_7wx4gb80000gn/T/ansible_ecs_taskdefinition_payload_tuz8i5hl/__main__.py\", line 216, in register_task\nValueError: invalid literal for int() with base 10: '1G'\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

PLAY RECAP **************************************************************************************************************************************************************************************************
frontend.ansible           : ok=8    changed=0    unreachable=0    failed=1  

This is the output with the change (memory: 1G):

TASK [aws_ecs_task_definition : Create task definition] *****************************************************************************************************************************************************
failed: [frontend.ansible] (item={'name': 'tls-stream-server', 'containers': [{'name': 'nginx', 'essential': True, 'image': 'nginx', 'portMappings': [{'containerPort': 8080, 'hostPort': 8080}], 'cpu': 512, 'memory': '1G', 'environment': [{'name': 'testkey', 'value': 'testvar'}, {'name': 'test2key', 'value': 'testvar'}]}]}) => {"changed": false, "item": {"containers": [{"cpu": 512, "environment": [{"name": "testkey", "value": "testvar"}, {"name": "test2key", "value": "testvar"}], "essential": true, "image": "nginx", "memory": "1G", "name": "nginx", "portMappings": [{"containerPort": 8080, "hostPort": 8080}]}], "name": "tls-stream-server"}, "msg": "Container parameter 'memory' requires an integer value, '1G' given"}

PLAY RECAP **************************************************************************************************************************************************************************************************
frontend.ansible           : ok=8    changed=0    unreachable=0    failed=1 

And this is the playbook/task working with the correct value (memory: 1024):

TASK [aws_ecs_task_definition : Create task definition] *****************************************************************************************************************************************************
changed: [frontend.ansible] => (item={'name': 'tls-stream-server', 'containers': [{'name': 'nginx', 'essential': True, 'image': 'nginx', 'portMappings': [{'containerPort': 8080, 'hostPort': 8080}], 'cpu': 512, 'memory': 1024, 'environment': [{'name': 'testkey', 'value': 'testvar'}, {'name': 'test2key', 'value': 'testvar'}]}]})

Doug Bridgens added some commits Mar 17, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 17, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 17, 2019

@thisdougb, 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

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 17, 2019

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

lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py:219:20: bare-except No exception type(s) specified

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py:219:21: E722 do not use bare 'except'
lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py:220:26: E111 indentation is not a multiple of four
lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py:220:26: E117 over-indented

click here for bot help

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