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 -- with force_new_deployment user can specify taskdef or not #1680

Conversation

karcadia
Copy link
Contributor

SUMMARY

Fixes #1106
Support force_new_deployment without having to specify a task definition.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ecs_service

ADDITIONAL INFORMATION

Previously task_definition was required when state was present; regardless of whether force_new_deployment was set or not.
Previous error was along the lines of "state is present but all of the following are missing: task_definition".
New behavior enforces either task_definition or force_new_deployment is set.
If both are provided, the user's task_definition will be sent through to boto.
If only task_definition is defined, original behavior resumes.
If only force_new_deployment is set, pull the taskDefinition from existing and pass it through to boto.

@karcadia karcadia changed the title If force_new_deployment, user can specify taskdef or not and we will pull existing taskdef. ecs_service -- with force_new_deployment user can specify taskdef or not Jan 24, 2023
@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage plugins plugin (any type) labels Jan 24, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 59s
✔️ build-ansible-collection SUCCESS in 6m 10s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 21s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 12m 41s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 58s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 46s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 9m 50s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 23s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 46s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 37s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 5m 50s
✔️ ansible-test-changelog SUCCESS in 2m 30s
✔️ ansible-test-splitter SUCCESS in 2m 45s
✔️ integration-community.aws-1 SUCCESS in 7m 46s
⚠️ 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
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
⚠️ integration-community.aws-19 SKIPPED
⚠️ integration-community.aws-20 SKIPPED
⚠️ integration-community.aws-21 SKIPPED
⚠️ integration-community.aws-22 SKIPPED

Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

@karcadia Thank you for taking time to open this pull request. Would you be willing to add some integration tests for this functionality? Thanks.

@github-actions
Copy link

github-actions bot commented Jan 30, 2023

Docs Build 📝

Thank you for contribution!✨

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

@ansibullbot ansibullbot added integration tests/integration tests tests labels Jan 30, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 36s
✔️ build-ansible-collection SUCCESS in 5m 37s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 13s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 07s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 36s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 05s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 8m 43s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 03s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 26s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 45s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 5m 44s
✔️ ansible-test-changelog SUCCESS in 2m 14s
✔️ ansible-test-splitter SUCCESS in 2m 31s
✔️ integration-community.aws-1 SUCCESS in 5m 20s
⚠️ 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
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
⚠️ integration-community.aws-19 SKIPPED
⚠️ integration-community.aws-20 SKIPPED
⚠️ integration-community.aws-21 SKIPPED
⚠️ integration-community.aws-22 SKIPPED

@markuman
Copy link
Member

fyi @karcadia
this PR is on-hold until #1716 is merged.

@markuman
Copy link
Member

markuman commented Mar 1, 2023

fyi @karcadia this PR is on-hold until #1716 is merged.

it's merged now. @karcadia can you please rebase your PR?

@tremble tremble force-pushed the force_redeploy_taskdef_optional branch from f8f0e5f to 0d43b28 Compare March 1, 2023 12:30
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/2909c666ea3443358f450d0c0900d37e

ansible-galaxy-importer FAILURE in 4m 56s
✔️ build-ansible-collection SUCCESS in 14m 33s
✔️ ansible-test-sanity-docker-devel SUCCESS in 8m 41s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 10s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 13m 26s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 8m 44s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 11m 24s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 7m 06s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 34s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 12s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 7m 41s
✔️ ansible-test-changelog SUCCESS in 4m 34s
✔️ ansible-test-splitter SUCCESS in 5m 12s
integration-community.aws-1 FAILURE in 16m 20s
Skipped 21 jobs

@markuman markuman added the backport-5 PR should be backported to the stable-5 branch label Mar 1, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/5531eda7f791457a8ac8396f99b69c8e

✔️ ansible-galaxy-importer SUCCESS in 4m 27s
✔️ build-ansible-collection SUCCESS in 12m 32s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 55s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 31s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 33m 29s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 11m 54s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 11m 39s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 7m 55s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 7m 32s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 7m 32s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 8m 40s
✔️ ansible-test-changelog SUCCESS in 4m 29s
✔️ ansible-test-splitter SUCCESS in 4m 36s
✔️ integration-community.aws-1 SUCCESS in 21m 55s
Skipped 21 jobs

@markuman markuman added mergeit Merge the PR (SoftwareFactory) and removed mergeit Merge the PR (SoftwareFactory) labels Mar 2, 2023
@markuman
Copy link
Member

markuman commented Mar 2, 2023

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/25da10cf974f4b208058220c3047adfc

✔️ ansible-galaxy-importer SUCCESS in 5m 48s
✔️ build-ansible-collection SUCCESS in 12m 33s
✔️ ansible-test-sanity-docker-devel SUCCESS in 14m 31s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 14m 11s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 14m 17s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 14m 23s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 14m 29s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 9m 36s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 9m 20s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 53s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 8m 49s
✔️ ansible-test-changelog SUCCESS in 4m 15s
✔️ ansible-test-splitter SUCCESS in 4m 52s
✔️ integration-community.aws-1 SUCCESS in 18m 11s
Skipped 21 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 6f84424 into ansible-collections:main Mar 2, 2023
@patchback
Copy link

patchback bot commented Mar 2, 2023

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/6f844249a91b2d86dfd2a9be1f843bcb90366b2d/pr-1680

Backported as #1732

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Mar 2, 2023
…not (#1680)

ecs_service -- with force_new_deployment user can specify taskdef or not

SUMMARY
Fixes #1106
Support force_new_deployment without having to specify a task definition.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION
Previously task_definition was required when state was present; regardless of whether force_new_deployment was set or not.
Previous error was along the lines of "state is present but all of the following are missing: task_definition".
New behavior enforces either task_definition or force_new_deployment is set.
If both are provided, the user's task_definition will be sent through to boto.
If only task_definition is defined, original behavior resumes.
If only force_new_deployment is set, pull the taskDefinition from existing and pass it through to boto.

Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit 6f84424)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Mar 2, 2023
…not (#1680) (#1732)

[PR #1680/6f844249 backport][stable-5] ecs_service -- with force_new_deployment user can specify taskdef or not

This is a backport of PR #1680 as merged into main (6f84424).
SUMMARY
Fixes #1106
Support force_new_deployment without having to specify a task definition.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION
Previously task_definition was required when state was present; regardless of whether force_new_deployment was set or not.
Previous error was along the lines of "state is present but all of the following are missing: task_definition".
New behavior enforces either task_definition or force_new_deployment is set.
If both are provided, the user's task_definition will be sent through to boto.
If only task_definition is defined, original behavior resumes.
If only force_new_deployment is set, pull the taskDefinition from existing and pass it through to boto.

Reviewed-by: Mark Chappell
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-5 PR should be backported to the stable-5 branch community_review feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ecs_service - Support force_new_deployment without having to specify a task definition
5 participants