ecs_taskdefinition can absent without containers argument

[calvin620707]

SUMMARY

Issue:
Following task will raise module error

ecs_taskdefinition:
  profile: personal
  state: absent
  arn: "{{ task_output.taskdefinition.taskDefinitionArn }}"

Exception:

Traceback (most recent call last):
  File "/Users/CalvinWu/.pyenv/versions/3.6.1/lib/python3.6/pdb.py", line 1667, in main
    pdb._runscript(mainpyfile)
  File "/Users/CalvinWu/.pyenv/versions/3.6.1/lib/python3.6/pdb.py", line 1548, in _runscript
    self.run(statement)
  File "/Users/CalvinWu/.pyenv/versions/3.6.1/lib/python3.6/bdb.py", line 431, in run
    exec(cmd, globals, locals)
  File "<string>", line 1, in <module>
  File "/Users/CalvinWu/Documents/ansible/lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py", line 17, in <module>
    ANSIBLE_METADATA = {'metadata_version': '1.1',
  File "/Users/CalvinWu/Documents/ansible/lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py", line 323, in main
    for container in module.params.get('containers', []):
TypeError: 'NoneType' object is not iterable

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

ecs_taskdefinition

ANSIBLE VERSION

ansible 2.7.0.dev0 (bugfix-ecs-taskdefiniation-absent a731ffbc56) last updated 2018/06/11 21:58:06 (GMT +800)
  config file = None
  configured module search path = ['/Users/CalvinWu/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/CalvinWu/Documents/ansible/lib/ansible
  executable location = /Users/CalvinWu/Documents/ansible/bin/ansible
  python version = 3.6.1 (default, Apr  4 2017, 14:20:20) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)]

ADDITIONAL INFORMATION

Following playbook should be able to reproduce the issue

---
- name: Debug ecs_taskdefinition module absent
  hosts: localhost
  connection: local
  environment:
    AWS_REGION: us-east-1
  gather_facts: no
  tasks:
    - name: present
      ecs_taskdefinition:
        containers:
        - name: simple-app
          cpu: 10
          essential: true
          image: "httpd:2.4"
          memory: 300
          portMappings:
          - containerPort: 80
            hostPort: 80
        family: test-cluster-taskdef
        state: present
      register: task_output

    - name: absent
      ecs_taskdefinition:
        state: absent
        arn: "{{ task_output.taskdefinition.taskDefinitionArn }}"

[ansibot]

cc @Java1Guy @willthames
click here for bot help

[calvin620707]

bot_status

[ansibot]

Components

lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py
support: community
maintainers: Java1Guy willthames

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

[willthames]

This change looks great, but it would be better if we could have a regression test for this too.

Adding a suitable version of your state: absent task to test/integration/targets/ecs_cluster/playbooks/roles/ecs_cluster/tasks/main.yml just before the always: block would make sense.

Let me know if you need any help (if need be I can always add the test to this change)

[willthames]

Candidate for backport to 2.6 and 2.5

[calvin620707]

@willthames Thanks for the feedback, I will add the test for this.

[ansibot]

@calvin620707 this PR contains the following merge commits:

• fd37fcd

Please rebase your branch to remove these commits.

click here for bot help

[calvin620707]

ready_for_review

[ryansb]

Looks great! Thank you for adding a test along with your change!

[willthames]

Looks good to me, but I haven't run the test suite yet - I'll be happy to merge once I've done so (or someone else says they have)

[calvin620707]

@ryansb You're welcome.
@willthames I don't know how to run the integration test by myself, so I haven't run the test yet. Please help to run the test and feel free to let me know if any questions. Thanks~

[willthames]

@calvin620707 no problem, in general I'd want someone other than the PR author to sign off the tests anyway.

If you're interested, there are some docs - if they're not sufficient to get you up and running, then that's a bug in the guidelines - let us know and we can improve them.

[calvin620707]

@willthames Thank you. I will check out the document later.

[willthames]

I needed to update hacking/aws_config/testing_policies/compute-policy.json, but suspect that was missing from previous changes!

My local tests all pass, so once CI has gone green again I'll merge

[ryansb]

@willthames local tests pass for me as well, FWIW

[willthames]

Merged, thanks @calvin620707.

And thanks @ryansb for double checking the tests.

[gundalow]

Hi,
I'm randomly selecting some PRs to see how it was for you, with the aim of making it easier for future contributors. As a new contributor I'm particularly interested as you don't have any prior knowledge of the Ansible process.

1. Generally speaking, how was the process for you?
2. Did it make sense what to do to progress the PR?
3. Did you read any of the developer (dev_guide) or contributor documentation?
4. What in the documentation was useful or confusing, missing or could be improved
5. Did you know you could run ansible-test locally?
6. What areas would you be interested in contributing to?

ansible/community#353

[calvin620707]

1. It's okay for me. I just need to read some document from Ansible doc and trace parts of code in the module I want to contribute to. However, how to operate ansible-bot and the progress/status of PR isn't quite clear. I'm not sure if I need to send some commands for ansible-bot to notify maintainer to review my PR.
   There may also a bug of ansible-bot? ansible-bot didn't ping maintainer after 2 weeks.

If the pull request is labeled community_review and the reviewer has not responded lately:
The reviewer is first politely pinged after two weeks, pinged again after two more weeks and labeled pending_action, and then may be reassigned to $team_ansible or labeled core_review, or often the submitter of the pull request is asked to step up as a maintainer.

2. It make sense.
3. I read The Ansible Development Process
4. A document to introduce the process to send a PR to a module step by step may help.
5. I know I can run ansible-test locally after maintainer mentioned, but I didn't run it because this module need to run integration test. I'm not sure if I will be charged by AWS if I run integration test with me personal account.
6. I'm considering contributing to ecs_service module because I need this module to support force-new-deployment option.