ecs_service cannot handle distinctInstance Placement Constraint

superwesman commented 2 years ago

Summary

Most Placement Constraint options have both a type and an expression. You can see this is ecs_service.py main() ...

        placement_constraints=dict(
            required=False,
            default=[],
            type='list',
            elements='dict',
            options=dict(
                type=dict(type='str'),
                expression=dict(type='str')
            )
        ),

However, the distinctInstance Placement Constraint does not require an expression. The bug here is that the above code effectively mandates that expression is required. If it is omitted from the playbook, we end up with the value being None which I was able to capture by modifying the code ....

[{'type': 'distinctInstance', 'expression': None}]

The presence of expression here, even with the value of None, yields the following error:

botocore.exceptions.ParamValidationError: Parameter validation failed:\nInvalid type for parameter placementConstraints[0].expression, value: None,

I've confirmed that boto supports this just fine ...

params = dict(
    cluster="default",
    serviceName="some-service-name",
    taskDefinition="some-task-definition,
    desiredCount=0,
    placementConstraints=[dict(type="distinctInstance")]
)

print("params:" + str(params))
ecs.create_service(**params)

The bug appears to be in ecs_service.py during the create service path of execution.

Issue Type

Bug Report

Component Name

ecs_service

Ansible Version

$ ansible --version
ansible [core 2.12.4]
  config file = None
  configured module search path = ['/Users/wetorres/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/wetorres/Library/Python/3.8/lib/python/site-packages/ansible
  ansible collection location = /Users/wetorres/.ansible/collections:/usr/share/ansible/collections
  executable location = /Users/wetorres/Library/Python/3.8/bin/ansible
  python version = 3.8.9 (default, Feb 18 2022, 07:45:34) [Clang 13.1.6 (clang-1316.0.21.2)]
  jinja version = 3.1.1
  libyaml = True

Collection Versions

$ ansible-galaxy collection list

# /Users/wetorres/Library/Python/3.8/lib/python/site-packages/ansible_collections
Collection                    Version
----------------------------- -------
amazon.aws                    2.1.0  
ansible.netcommon             2.5.1  
ansible.posix                 1.3.0  
ansible.utils                 2.5.2  
ansible.windows               1.9.0  
arista.eos                    3.1.0  
awx.awx                       19.4.0 
azure.azcollection            1.11.0 
check_point.mgmt              2.3.0  
chocolatey.chocolatey         1.2.0  
cisco.aci                     2.1.0  
cisco.asa                     2.1.0  
cisco.intersight              1.0.18 
cisco.ios                     2.8.0  
cisco.iosxr                   2.8.1  
cisco.ise                     1.2.1  
cisco.meraki                  2.6.0  
cisco.mso                     1.3.0  
cisco.nso                     1.0.3  
cisco.nxos                    2.9.0  
cisco.ucs                     1.7.0  
cloud.common                  2.1.0  
cloudscale_ch.cloud           2.2.0  
community.aws                 2.3.0  
community.azure               1.1.0  
community.ciscosmb            1.0.4  
community.crypto              2.2.3  
community.digitalocean        1.15.1 
community.dns                 2.0.8  
community.docker              2.2.1  
community.fortios             1.0.0  
community.general             4.6.0  
community.google              1.0.0  
community.grafana             1.3.3  
community.hashi_vault         2.3.0  
community.hrobot              1.2.2  
community.kubernetes          2.0.1  
community.kubevirt            1.0.0  
community.libvirt             1.0.2  
community.mongodb             1.3.2  
community.mysql               2.3.5  
community.network             3.1.0  
community.okd                 2.1.0  
community.postgresql          1.7.1  
community.proxysql            1.3.1  
community.rabbitmq            1.1.0  
community.routeros            2.0.0  
community.skydive             1.0.0  
community.sops                1.2.0  
community.vmware              1.17.1 
community.windows             1.9.0  
community.zabbix              1.5.1  
containers.podman             1.9.1  
cyberark.conjur               1.1.0  
cyberark.pas                  1.0.13 
dellemc.enterprise_sonic      1.1.0  
dellemc.openmanage            4.4.0  
dellemc.os10                  1.1.1  
dellemc.os6                   1.0.7  
dellemc.os9                   1.0.4  
f5networks.f5_modules         1.15.0 
fortinet.fortimanager         2.1.4  
fortinet.fortios              2.1.4  
frr.frr                       1.0.3  
gluster.gluster               1.0.2  
google.cloud                  1.0.2  
hetzner.hcloud                1.6.0  
hpe.nimble                    1.1.4  
ibm.qradar                    1.0.3  
infinidat.infinibox           1.3.3  
infoblox.nios_modules         1.2.1  
inspur.sm                     1.3.0  
junipernetworks.junos         2.9.0  
kubernetes.core               2.2.3  
mellanox.onyx                 1.0.0  
netapp.aws                    21.7.0 
netapp.azure                  21.10.0
netapp.cloudmanager           21.15.0
netapp.elementsw              21.7.0 
netapp.ontap                  21.17.3
netapp.storagegrid            21.9.0 
netapp.um_info                21.8.0 
netapp_eseries.santricity     1.2.13 
netbox.netbox                 3.6.0  
ngine_io.cloudstack           2.2.3  
ngine_io.exoscale             1.0.0  
ngine_io.vultr                1.1.0  
openstack.cloud               1.7.2  
openvswitch.openvswitch       2.1.0  
ovirt.ovirt                   1.6.6  
purestorage.flasharray        1.12.1 
purestorage.flashblade        1.9.0  
sensu.sensu_go                1.13.0 
servicenow.servicenow         1.0.6  
splunk.es                     1.0.2  
t_systems_mms.icinga_director 1.27.1 
theforeman.foreman            2.2.0  
vyos.vyos                     2.8.0  
wti.remote                    1.0.3  

# /Users/wetorres/.ansible/collections/ansible_collections
Collection Version
---------- -------
amazon.aws 3.1.1

AWS SDK versions

$ pip show boto boto3 botocore
WARNING: Package(s) not found: boto
Name: boto3
Version: 1.21.29
Summary: The AWS SDK for Python
Home-page: https://github.com/boto/boto3
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /Users/wetorres/Library/Python/3.8/lib/python/site-packages
Requires: botocore, jmespath, s3transfer
Required-by: 
---
Name: botocore
Version: 1.24.29
Summary: Low-level, data-driven core of boto 3.
Home-page: https://github.com/boto/botocore
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /Users/wetorres/Library/Python/3.8/lib/python/site-packages
Requires: jmespath, python-dateutil, urllib3
Required-by: boto3, s3transfer

We do a lot of our ansible stuff from with virtual envs and docker containers. I don't happen to have boto installed, but don't let that be a distraction. The bug here is in ecs_service.py.

Configuration

$ ansible-config dump --only-changed

I ran this and it produced no output

OS / Environment

uname -a
Darwin REDACTED 21.4.0 Darwin Kernel Version 21.4.0: Fri Mar 18 00:45:05 PDT 2022; root:xnu-8020.101.4~15/RELEASE_X86_64 x86_64

Steps to Reproduce

---
- name: "ecs_service_bug"
  hosts: localhost

  tasks:
  - name: "ecs_service_bug"
    ecs_service:
      state: present
      name: "ecs_service_bug"
      cluster: "default"
      desired_count: 0
      region: "us-east-1"
      task_definition: "any-valid-task-definition"
      force_new_deployment: "yes"
      placement_constraints:
        - type: distinctInstance

Run ansible-playbook ./this.yml to reproduce the error.

Notes:

If the ECS Service does not exist, running this playbook will generate the error above.
If the ECS Service already exists, no error will be produced
- If the existing ECS Service has the distinctInstance Placement Constraint, it will be retained.
- If the existing ECS Service does not have the distinctInstance Placement Constraint, it will not be added.

Expected Results

I expect to be able to define distinctInstance Placement Strategy in my Ansible playbook, regardless of the state of the ECS Service.
I expect the distinctInstance Placement Strategy defined in the playbok should be applied to the ECS Service in question.

Actual Results

If the ECS Service doesn't exist yet, ansible-playbook generates an error and fails. No ECS Service is created.
If the ECS Service exists with distinctInstance Placement Strategy, this Placement Strategy is retained.
If the ECS Service exists without distinctInstance Placement Strategy, this Placement Strategy (which is supplied in the playbook) is not applied to the ECS Service.

Code of Conduct

[X] I agree to follow the Ansible Code of Conduct

ansibullbot commented 2 years ago

Files identified in the description:

[plugins/modules/ecs_service.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/ecs_service.py)

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 2 years ago

cc @Java1Guy @jillr @kaczynskid @markuman @s-hertel @tremble @zacblazic click here for bot help

markuman commented 2 years ago

@superwesman
Thx for the detail reporting. I can confirm the bug.

The bug here is that the above code effectively mandates that expression is required.

But that's not valid. The parameter is set to required=False,.
The parameter placement_constraints is just path through, from 789 to Line 599.

A possible fix might be easy. All what's to do is to pop None values from params['placement_constraints'] somewhere here ...

plus some integration test to cover that case
plus a changelog fragment

@superwesman are you willing to provide a PR to fix this issue?

superwesman commented 2 years ago

hello @markuman - Unfortunately, I don't think I'm in a position to provide a PR. I've discovered this bug at work and the exact corporate policies related to open source development are many, and not clear to me. I hope that my input is at least helpful for anyone who does have capacity to implement. I'm sorry I can't do more at the moment.

novak-as commented 2 years ago

@markuman I think I can provide a PR. I have been investigating issues with our automation and ended up having a solution which looks pretty similar to your proposal. I can confirm that this worked for me, so it looks like not much work is left

markuman commented 2 years ago

@markuman I think I can provide a PR. I have been investigating issues with our automation and ended up having a solution which looks pretty similar to your proposal. I can confirm that this worked for me, so it looks like not much work is left

That would be awesome @novak-as
I've test your branch and the current integration test passes. From my perspective, there are 3 steps left.

rebase your branch with upstream main branch
open PR and add a bugfixes changelog fragment
expand the integration test that covers the case

just a few days left until we're going to release 3.3.0. But I think we can made it.

ansible-collections / community.aws