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 -- Capacity provider strategy #1181

Conversation

karcadia
Copy link
Contributor

@karcadia karcadia commented May 30, 2022

SUMMARY

Fixes #1137
Per request, allow for the user to provide a capacity_provider_strategy when creating or updating an ECS service. This capacity_provider_strategy is a list of 1-6 dictionaries. The new capacity_provider_strategy is mutually exclusive with launch_type and an existing service cannot be changed from one to the other.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ecs_service

ADDITIONAL INFORMATION

The new parameter is optional and non-default. If neither launch_type or capacity_provider_strategy are provided, the new service will default to EC2 launch_type. The module handles the mutually exclusivity and also catches and fails cleanly when trying to change an existing service from launch_type to capacity_provider_strategy or vice versa.
Tested pretty thoroughly against ansible 2.9.27. Updated parameters, examples, and return objects provided.

Before merge the module will just ignore the capacity_provider_strategy and default to EC2 launch_type.
After merge the module will handle either launch_type or capacity_provider_strategy and create/update the service as necessary.

- community.aws.ecs_service:
    state: present
    name: test-service
    cluster: test-cluster
    task_definition: test-task-definition
    desired_count: 1
    capacity_provider_strategy:
      - capacity_provider: test-capacity-provider-1
        weight: 1
        base: 0

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) labels May 30, 2022
@github-actions
Copy link

github-actions bot commented May 30, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@karcadia karcadia changed the title Capacity provider strategy ecs_service -- Capacity provider strategy May 30, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

ansible-galaxy-importer FAILURE in 3m 58s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 45s
ansible-test-sanity-docker-devel FAILURE in 10m 57s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 10m 02s
ansible-test-sanity-docker-stable-2.9 FAILURE in 14m 54s
ansible-test-sanity-docker-stable-2.11 FAILURE in 9m 59s
ansible-test-sanity-docker-stable-2.12 FAILURE in 9m 15s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 23s
✔️ ansible-test-splitter SUCCESS in 2m 34s
✔️ integration-community.aws-1 SUCCESS in 5m 11s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

ansible-galaxy-importer FAILURE in 4m 20s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 06s
ansible-test-sanity-docker-devel FAILURE in 9m 26s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 10m 34s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 06s
ansible-test-sanity-docker-stable-2.11 FAILURE in 11m 05s
ansible-test-sanity-docker-stable-2.12 FAILURE in 10m 52s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 48s
✔️ ansible-test-splitter SUCCESS in 2m 28s
✔️ integration-community.aws-1 SUCCESS in 5m 45s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 22s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 46s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 19s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 01s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 34s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 39s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 24s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 17s
✔️ ansible-test-splitter SUCCESS in 2m 40s
✔️ integration-community.aws-1 SUCCESS in 5m 12s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@markuman markuman mentioned this pull request May 30, 2022
10 tasks
@markuman markuman self-assigned this May 30, 2022
Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karcadia thanks for your contribution.
at first glance it looks good so far. can you also expand the integration test tests/integration/targets/ecs_cluster/ that covers the new parameter?

@markuman
Copy link
Member

@karcadia thanks for your contribution. at first glance it looks good so far. can you also expand the integration test tests/integration/targets/ecs_cluster/ that covers the new parameter?

...any better rebase your branch before touching the integration test. :)

karcadia and others added 9 commits May 30, 2022 13:26
Add requested feature for capacity provider strategy.

Capacity Provider Strategy is mutually exclusive with launch_type and a given service cannot be changed from one to the other.

Possibly still needs:
Camel <=> Snake casing for the subelements of capacity_provider_strategy.

Catch if provided list has more than 6 elements and fail cleanly.
…gy.yml

Co-authored-by: Markus Bergholz <git@osuv.de>
Co-authored-by: Markus Bergholz <git@osuv.de>
@karcadia karcadia force-pushed the capacity_provider_strategy branch from 63fac9b to e6a328e Compare May 30, 2022 18:26
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 23s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 23s
✔️ ansible-test-sanity-docker-devel SUCCESS in 12m 26s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 30s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 56s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 11m 03s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 34s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 53s
✔️ ansible-test-splitter SUCCESS in 2m 24s
✔️ integration-community.aws-1 SUCCESS in 6m 53s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@karcadia
Copy link
Contributor Author

karcadia commented Jun 1, 2022

So the integration test might not be possible until ecs_cluster.py supports capacityProviders. I believe it has to be defined in the ecs_cluster before it can be used in an ecs_service provider strategy.

I can probably add that as well, not sure if we are concerned about expanding the scope of this PR too much.

We should probably go ahead and add defaultCapacityProviderStrategy to ecs_cluster while we're at it.
@markuman

@markuman
Copy link
Member

markuman commented Jun 2, 2022

So the integration test might not be possible until ecs_cluster.py supports capacityProviders. I believe it has to be defined in the ecs_cluster before it can be used in an ecs_service provider strategy.

Good point. I guess it needs also a new module ecs_capacity_provider to create, delete and update capacity provider.
Because the ecs cluster itself is referring to it on its creation.

I can probably add that as well, not sure if we are concerned about expanding the scope of this PR too much.

Yes, +1, keep the scope small. +1

@markuman markuman added the mergeit Merge the PR (SoftwareFactory) label Jun 2, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 3m 51s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 25s
✔️ ansible-test-sanity-docker-devel SUCCESS in 12m 57s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 19s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 25s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 11m 32s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 05s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 09s
✔️ ansible-test-splitter SUCCESS in 2m 30s
✔️ integration-community.aws-1 SUCCESS in 5m 12s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 8a5ed47 into ansible-collections:main Jun 2, 2022
@karcadia karcadia deleted the capacity_provider_strategy branch June 2, 2022 14:43
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
Linting cleanup

SUMMARY
Minor linting cleanup

missing whitespace
unused variables
catching Exception
unused imports

in kms_key also replaces get_account_info with module_utils.iam.get_aws_account_info
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/inventory/aws_ec2.py
plugins/module_utils/cloud.py
plugins/modules/autoscaling_group.py
plugins/modules/cloudtrail_info.py
plugins/modules/cloudwatchlogs_log_group.py
plugins/modules/ec2_eip.py
plugins/modules/kms_key_info.py
plugins/modules/lambda.py
plugins/modules/lambda_execute.py
plugins/modules/rds_cluster_snapshot.py
plugins/modules/rds_instance.py
plugins/modules/rds_instance_snapshot.py
plugins/modules/route53_health_check.py
plugins/modules/s3_object_info.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request has_issue mergeit Merge the PR (SoftwareFactory) module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Capacity Provider Strategy for ECS Services
4 participants