community.aws icon indicating copy to clipboard operation
community.aws copied to clipboard

Support force_new_deployment without having to specify a task definition

Open giom-l opened this issue 3 years ago • 6 comments

Summary

Sometimes, it can be helpful to just restart all tasks from a service, without having to deal with all tasks definitions (since they do not need to change).

For that purpose, the aws cli offers the following possibility :

aws ecs update-service --cluster cluster_name --service service_name --force-new-deployment

This is particularly interesting when we want some external files to be taken (but the task definition itself does not need any change).

We can note that boto3 already support this option (at least since v1.14.2 i'm working with) :

import boto3
client = boto3.client('ecs')
client.update_service(cluster='cluster_name', service='service_name', forceNewDeployment=True)

However, the ecs_service module can not do that since when we specify state: present, we get the following error

fatal: [localhost]: FAILED! => {"changed": false, "msg": "state is present but all of the following are missing: task_definition"}

Issue Type

Feature Idea

Component Name

ecs_service

Additional Information

An rough idea would be to add an additional parameter (like preserve_tasks_definitions) which would, when true, avoid the check about the presence of task_definition.

Code of Conduct

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

giom-l avatar Apr 29 '22 09:04 giom-l

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 avatar Apr 29 '22 09:04 ansibullbot

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

ansibullbot avatar Apr 29 '22 09:04 ansibullbot

I forgot to mention : It is indeed workaroundable but requires some more tasks to retrieve informations about loadBalancers & taskdefinition.

- name: Retrieve service details
  community.aws.ecs_service_info:
    cluster: "{{ cluster_name }}"
    service: "{{ service_name }}"
    details: true
  register: service_details

- name: Reload ECS service
  community.aws.ecs_service:
    cluster: "{{ cluster_name }}"
    name: "{{ service_name }}"
    state: present
    force_new_deployment: yes
    load_balancers: "{{ service_details.services | map(attribute='loadBalancers') | first }}"
    task_definition: "{{ service_details.services | map(attribute='taskDefinition') | first }}"

The main idea of this feature request it to improve the user experience by not having to deal with data we do not care about at the moment.

giom-l avatar Apr 29 '22 13:04 giom-l

We can note that boto3 already support this option (at least since v1.14.2 i'm working with) :

import boto3
client = boto3.client('ecs')
client.update_service(cluster='cluster_name', service='service_name', forceNewDeployment=True)

However, the ecs_service module can not do that since when we specify state: present, we get the following error

fatal: [localhost]: FAILED! => {"changed": false, "msg": "state is present but all of the following are missing: task_definition"}

So a new boolean parameter force_new_deployment must be introduced.
When it got no default value, it can be mutually exclusive with the state parameter.

@giom-l do you have some time to implement this new feature?

markuman avatar May 01 '22 05:05 markuman

Hi,

Not this week but I can take a look at it the next one, sure. Just to be sure about what to do : you write about a new Boolean parameter, but there is already an existing one with this name : https://docs.ansible.com/ansible/latest/collections/community/aws/ecs_service_module.html#parameter-force_new_deployment

It is just the behaviour of this one that should be changed, right ? (NB : I haven't the whole code in front of me right now so I may have missed something)

giom-l avatar May 01 '22 09:05 giom-l

Ah sorry. I thought the entire parameter was missing.
So yes, imo the internally logic must just be fixed, so that force_new_deployment: yes works also on existing service without the need to specify the taskdefinition.

markuman avatar May 01 '22 11:05 markuman