Allow CPU and Memory to be templatable

[nikhilo]

Issue Type:

• Bugfix Pull Request

Plugin Name:

ecs_taskdefinition

Ansible Version:

ansible 2.0.0.2

Summary:

Currently, the module does not allow the CPU and Memory parameters to be templatable. They must be integer/long. This makes the module unusable with variables. In other words, we would need to call the module for every task definition.
This change will allow the CPU and Memory to be templatable like so,

    containers:
      - name: "nikhil-test"
        image: "nikhil-test-nginx:{{deploy_tag}}"
        memory: "{{mycontainers.nginx.memory}}"
        cpu: "{{mycontainers.nginx.cpu}}"

Example output:

Error before change:

botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter containerDefinitions[0].memory, value: 1920, type: <type 'str'>, valid types: <type 'int'>, <type 'long'>
Invalid type for parameter containerDefinitions[0].cpu, value: 2560, type: <type 'str'>, valid types: <type 'int'>, <type 'long'>

This error is not seen with suggested change

[bcoca]

this is why complex fields are a problem, we cannot validate in the spec

[gregdek]

Thanks @nikhilo for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

[nikhilo]

ready_for_review

[gregdek]

Thanks @nikhilo. To the current maintainers, @Java1Guy please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit', 'needs_revision' or 'close_me' as appropriate.

[This message brought to you by your friendly Ansibull-bot.]

[skorochkin]

any news on that?

[sgviking]

If you are going to make memory and cpu template-able it would be nice to be able to template portMappings.containerPort and portMappings.hostPort as well. This way the whole module could be handed an array of containers and their individual values.

Inside of the for loop you added add an inner for loop.

    for mapping in container['portMappings']:
        mapping['containerPort'] = int(mapping['containerPort'])
        mapping['hostPort'] = int(mapping['hostPort'])

[nikhilo]

@sgviking I don't think it's necessary to make the port mappings templatable. Since the port mappings is an array, you would be able to templatize them like so,

test: <-- This is the APP_ENV
  containers:
    nginx:
      memory: 1920
      cpu: 2048
      portMappings:
        - containerPort: 80
          hostPort: 80
        - containerPort: 443
          hostPort: 443

tasks:
  - name: "nikhil-nginx"
    containers:
      - name: "nikhil-nginx"
        image: "nginx:latest"
        memory: "{{vars.get(APP_ENV).containers.nginx.memory}}"
        cpu: "{{vars.get(APP_ENV).containers.nginx.cpu}}"
        portMappings: "{{vars.get(APP_ENV).containers.nginx.portMappings}}"

I verified that above pattern works

[simplesteph]

Any progress on merging this? Looks good to me, and this is a highly important bug fix

[gregdek]

Thanks @nikhilo for this PR. Unfortunately, it is not mergeable in its current state due to merge conflicts. Please rebase your PR. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

For help on how to do this cleanly please see http://docs.ansible.com/ansible/community.html#contributing-code-features-or-bugfixes

[This message brought to you by your friendly Ansibull-bot.]

[gregdek]

@nikhilo A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer.

[This message brought to you by your friendly Ansibull-bot.]

[ansibot]

This repository has been locked. All new issues and pull requests should be filed in https://github.com/ansible/ansible

Please read through the repomerge page in the dev guide. The guide contains links to tools which automatically move your issue or pull request to the ansible/ansible repo.

[nikhilo]

Since I have lost access to the repo I contributed from;
Moving this PR to ansible/ansible#19263