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 - document circuit breaker feature #1215

Conversation

karcadia
Copy link
Contributor

@karcadia karcadia commented Jun 4, 2022

SUMMARY

Fixes #921
This feature works with the existing code, so this was mainly adding documentation, examples, and an integration test.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ecs_service

ADDITIONAL INFORMATION

The deployment circuit breaker is part of the deployment configuration dictionary, which is already snake<=>camel cased. Thus the existing code was handling 99% of the feature, we just added some type validation, documentation, examples, and an integration test.

- community.aws.ecs_service:
    state: present
    name: test-service
    cluster: test-cluster
    task_definition: test-task-definition
    desired_count: 3
    deployment_configuration:
      deployment_circuit_breaker:
        enable: True
        rollback: True

@karcadia karcadia changed the title #921 - document circuit breaker feature ecs_service - document circuit breaker feature Jun 4, 2022
@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Jun 4, 2022
@github-actions
Copy link

github-actions bot commented Jun 4, 2022

Docs Build 📝

Thank you for contribution!✨

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

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

ansible-galaxy-importer FAILURE in 4m 10s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 01s
ansible-test-sanity-docker-devel FAILURE in 10m 46s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 10m 02s
ansible-test-sanity-docker-stable-2.9 FAILURE in 10m 54s
ansible-test-sanity-docker-stable-2.11 FAILURE in 10m 14s
ansible-test-sanity-docker-stable-2.12 FAILURE in 10m 28s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 36s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 29s
✔️ ansible-test-splitter SUCCESS in 2m 30s
✔️ integration-community.aws-1 SUCCESS in 5m 43s
⚠️ 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 added 2 commits June 4, 2022 10:28
There was already a deploymentConfiguration block in the delete_service return object doc.
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 38s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 21s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 02s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 08s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 51s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 34s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 26s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 20s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 39s
✔️ ansible-test-splitter SUCCESS in 2m 25s
✔️ integration-community.aws-1 SUCCESS in 5m 36s
⚠️ 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

plugins/modules/ecs_service.py Outdated Show resolved Hide resolved
@tremble
Copy link
Contributor

tremble commented Jun 4, 2022

Thanks for this PR. LGTM.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 29s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 46s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 17s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 13m 05s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 15m 21s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 40s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 47s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 59s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 47s
✔️ ansible-test-splitter SUCCESS in 2m 45s
✔️ integration-community.aws-1 SUCCESS in 5m 15s
⚠️ 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

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

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 3m 59s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 48s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 42s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 29s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 36s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 12m 57s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 15s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 47s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 12s
✔️ ansible-test-splitter SUCCESS in 2m 26s
✔️ integration-community.aws-1 SUCCESS in 27m 00s
⚠️ 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 a510cbb into ansible-collections:main Jun 4, 2022
@karcadia karcadia deleted the circuit_breaker branch June 4, 2022 21:59
@markuman
Copy link
Member

markuman commented Jun 5, 2022

The integration test is failing

TASK [ecs_cluster : check that ECS service was created with deployment_circuit_breaker] *************************************************************************************************************************************
fatal: [testhost]: FAILED! => {"msg": "The conditional check 'ecs_service.service.deploymentCircuitBreaker' failed. The error was: error while evaluating conditional (ecs_service.service.deploymentCircuitBreaker): 'dict object' has no attribute 'deploymentCircuitBreaker'"}

softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 5, 2022
fix assert of deploymentCircuitBreaker

SUMMARY
fix broken ecs_cluster integration test of #1215
cc @tremble  @karcadia
ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell <None>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 7, 2022
backport-3: ecs_service deployment_circuit_breaker

SUMMARY
manual backport of

#1215
#1217

there is no version_added, because the module already supports this feature. it was just undocumented.
ISSUE TYPE


Bugfix Pull Request
Docs Pull Request
Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 7, 2022
backport-2: ecs_service deployment_circuit_breaker

SUMMARY
manual backport of

#1215
#1217

there is no version_added, because the module already supports this feature. it was just undocumented.
ISSUE TYPE


Bugfix Pull Request
Docs Pull Request
Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
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 integration tests/integration mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

circuit breaker settings missing in ecs_service module
4 participants