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

ecs_service cannot handle distinctInstance Placement Constraint #1058

Closed
1 task done
superwesman opened this issue Apr 12, 2022 · 6 comments · Fixed by #1170
Closed
1 task done

ecs_service cannot handle distinctInstance Placement Constraint #1058

superwesman opened this issue Apr 12, 2022 · 6 comments · Fixed by #1170
Labels
bug This issue/PR relates to a bug module module plugins plugin (any type)

Comments

@superwesman
Copy link

superwesman commented Apr 12, 2022

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

  1. If the ECS Service doesn't exist yet, ansible-playbook generates an error and fails. No ECS Service is created.
  2. If the ECS Service exists with distinctInstance Placement Strategy, this Placement Strategy is retained.
  3. 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

  • I agree to follow the Ansible Code of Conduct
@ansibullbot
Copy link

Files identified in the description:

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
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module needs_triage plugins plugin (any type) labels Apr 12, 2022
@markuman
Copy link
Member

@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
Copy link
Author

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.

@markuman markuman mentioned this issue May 10, 2022
10 tasks
@novak-as
Copy link
Contributor

@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
Copy link
Member

@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.

  1. rebase your branch with upstream main branch
  2. open PR and add a bugfixes changelog fragment
  3. 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.

softwarefactory-project-zuul bot pushed a commit that referenced this issue May 30, 2022
ecs_service - fix validation for `placementConstraints`

SUMMARY
Fixes #1058
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ecs_service

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Oleksandr Novak <[email protected]>
Reviewed-by: Alina Buzachis <None>
patchback bot pushed a commit that referenced this issue May 30, 2022
ecs_service - fix validation for `placementConstraints`

SUMMARY
Fixes #1058
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ecs_service

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Oleksandr Novak <[email protected]>
Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 688c7e8)
patchback bot pushed a commit that referenced this issue May 30, 2022
ecs_service - fix validation for `placementConstraints`

SUMMARY
Fixes #1058
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ecs_service

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Oleksandr Novak <[email protected]>
Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 688c7e8)
softwarefactory-project-zuul bot pushed a commit that referenced this issue May 30, 2022
[PR #1170/688c7e89 backport][stable-3] ecs_service - fix validation for `placementConstraints`

This is a backport of PR #1170 as merged into main (688c7e8).
SUMMARY
Fixes #1058
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ecs_service

Reviewed-by: Markus Bergholz <[email protected]>
softwarefactory-project-zuul bot pushed a commit that referenced this issue May 30, 2022
[PR #1170/688c7e89 backport][stable-2] ecs_service - fix validation for `placementConstraints`

This is a backport of PR #1170 as merged into main (688c7e8).
SUMMARY
Fixes #1058
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ecs_service

Reviewed-by: Markus Bergholz <[email protected]>
abikouo pushed a commit to abikouo/community.aws that referenced this issue Oct 24, 2023
…e_rds_subnet_group

Migrate rds_subnet_group* modules and tests

Depends-On: ansible/zuul-config#445
Migrate rds_subnet_group* modules and tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants