ansible-collections / community.aws

Ansible Collection for Community AWS
GNU General Public License v3.0
189 stars 398 forks source link

ecs_service planning #1142

Open markuman opened 2 years ago

markuman commented 2 years ago

Summary

ecs_service needs some care

the three most important ToDos


easy, good first issues


features


bugs

markuman commented 2 years ago

bot_skip

markuman commented 2 years ago

everyone who is bored can join

jatorcasso commented 2 years ago

@markuman I can poke at a few of these after #1145 gets merged

jatorcasso commented 2 years ago

@markuman where are waiters needed here? are there specific test cases I can look at to validate the waiters do their job? I see for scaling down services

markuman commented 2 years ago

@markuman where are waiters needed here? are there specific test cases I can look at to validate the waiters do their job? I see for scaling down services

yes. one case is when the service is state: absent, it must wait until the service is inactive.
it would make this retry https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ecs_cluster/tasks/main.yml#L983-L991 superfluous, because the cluster can only be removed when no service is available anymore.
wait: yes in this tasks inside the always block: https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ecs_cluster/tasks/main.yml#L877-L916

another case is when the desired_count changes. https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ecs_cluster/tasks/main.yml#L301-L319
or when the service is created. it must wait until the service is table: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ecs.html#ECS.Waiter.ServicesStable
this would also solves this PR: https://github.com/ansible-collections/community.aws/pull/91

markuman commented 2 years ago

something else that might be need to be discussed is the format of the task_definition.

The input format is documented like this: https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/ecs_service.py#L214

    task_definition: 'new_cluster-task:1'

But the output is returning just the arn of the task_definition:

    taskDefinition: arn:aws:ecs:eu-central-1:123456789:task-definition/new_cluster-task:1

Shall we support both as valid input formats?

tremble commented 2 years ago

Shall we support both as valid input formats?

Having dealt with all sorts of cross-account fun in the past few years, I'm a fan of supporting full ARNs in addition to just the names.

jatorcasso commented 2 years ago

@ingmarfjolla this could be a good first contribution for ya

alinabuzachis commented 1 year ago

@markuman Can you please check what's missing in here?

markuman commented 1 year ago

@markuman Can you please check what's missing in here?

@alinabuzachis done. only two items are left. but they are more cosmetic in nature.