Support changing of launch type #840
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
@mjmayer Thank you for taking time to work on this. Could you please add a changelog fragment for this fix?
|
Build succeeded.
|
|
I had troubles running the test suite locally. I was seeing failures unrelated to the code changes I made. In the IRC channel it was suggested to open a PR and see if the tests run. Looking at the zuul build I'm not sure it is running the tests properly. |
alinabuzachis
requested changes
Jan 20, 2022
CHANGELOG.rst
Outdated
| @@ -85,6 +85,7 @@ Bugfixes | |||
| - route53 - fix diff mode when deleting records (https://github.com/ansible-collections/community.aws/pull/802). | |||
| - route53 - return empty result for nonexistent records (https://github.com/ansible-collections/community.aws/pull/799). | |||
| - sns_topic - define suboptions for delivery_policy option (https://github.com/ansible-collections/community.aws/issues/713). | |||
| - ecs_taskdefinition - include launch_type comparison when comparing task definitions (https://github.com/ansible-collections/community.aws/pull/840) | |||
There was a problem hiding this comment.
There is no need to modify this file (this file is autogenerated). You have to place your changelog here https://github.com/ansible-collections/community.aws/tree/main/changelogs/fragments. For example, you can name the file 840-ecs_taskdefinition-fix-task-definition-comparison.yml (or if you find a better name) and place inside the file
bugfixes:
- ecs_taskdefinition - include launch_type comparison when comparing task definitions (https://github.com/ansible-collections/community.aws/pull/840)
There was a problem hiding this comment.
Thanks for being very explicit about the change required. I've made the requested changes.
Concerning the integrations tests, since they are marked with |
mjmayer
force-pushed
the
support_changing_of_launch_type
branch
from
5cda82c
to
207d190
Compare
|
Build succeeded.
|
markuman
requested changes
Jan 21, 2022
There was a problem hiding this comment.
The integration test of ecs_cluster is a bad adventure ...
Because of so many issues, it's marked as unstable I think
TASK [ecs_cluster : create ECS service definition] *****************************
An error occurred (InvalidParameterException) when calling the CreateService operation:
Unable to Start a service that is still Draining.
The service will be deleted: https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ecs_cluster/tasks/force_service_deletion.yml#L244
and afterwards instantly recreated: https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/ecs_cluster/tasks/force_service_deletion.yml#L257
Furthermore the ressources are not cleaned up after failure.
When I reduce the integration test to - include_tasks: full_test.yml, that is touched in this PR, we run in more technical debt issues
TASK [ecs_cluster : provision ec2 instance to create an image] *****************
An error occurred (InvalidParameterCombination) when calling the RunInstances operation:
Enhanced networking with the Elastic Network Adapter (ENA) is required for the 't3.micro' instance type.
Ensure that you are using an AMI that is enabled for ENA.
A hotfix for this error is
diff --git a/tests/integration/targets/ecs_cluster/tasks/full_test.yml b/tests/integration/targets/ecs_cluster/tasks/full_test.yml
index 88e29ce..a48bf98 100644
--- a/tests/integration/targets/ecs_cluster/tasks/full_test.yml
+++ b/tests/integration/targets/ecs_cluster/tasks/full_test.yml
@@ -111,7 +111,7 @@
- name: set image id fact
set_fact:
- ecs_image_id: "{{ (ec2_ami_info.images|first).image_id }}"
+ ecs_image_id: "{{ (ec2_ami_info.images|last).image_id }}"
- name: provision ec2 instance to create an image
ec2_instance:There are so many "igonoring errors" due undefined variables errors, fixme notes etc. Cleanup the ecs_cluster integration test is a mammoth task
The list of errors is endless
TASK [ecs_cluster : create fargate ECS task with run task] *********************
botocore.errorfactory.ClientException: An error occurred (ClientException) when calling the RunTask operation:
ECS was unable to assume the role 'arn:aws:iam::611139332414:role/ecsTaskExecutionRole' that was provided for this task.
Please verify that the role being passed has the proper trust relationship and permissions and that your IAM user has permissions to pass this role.
But back to this PR.
This test is failing which is successfull in main branch.
TASK [ecs_cluster : recreate task definition] **********************************
changed: [testhost] => {"changed": true, "resource_actions": ["ecs:RegisterTaskDefinition", "ecs:ListTaskDefinitions", "ecs:DescribeTaskDefinition"], "taskdefinition": {"compatibilities": ["EXTERNAL", "EC2"], "containerDefinitions": [{"cpu": 0, "environment": [], "essential": true, "image": "nginx", "linuxParameters": {"maxSwap": 0, "swappiness": 80}, "memory": 128, "mountPoints": [], "name": "ansible-test-64854856-rocket-task", "portMappings": [{"containerPort": 8080, "hostPort": 0, "protocol": "tcp"}], "volumesFrom": []}], "family": "ansible-test-64854856-rocket-task", "networkMode": "bridge", "placementConstraints": [], "revision": 2, "status": "ACTIVE", "taskDefinitionArn": "arn:aws:ecs:eu-central-1:611139332414:task-definition/ansible-test-64854856-rocket-task:2", "volumes": []}}
TASK [ecs_cluster : check that task definition does not change] ****************
fatal: [testhost]: FAILED! => {
"assertion": "not ecs_task_definition_again.changed",
"changed": false,
"evaluated_to": false,
"msg": "Assertion failed"
}
So it must be fixed before merging
|
@markuman Thank you for the detailed description of the state of The test is still failing, but the failures appears unrelated to my changes. TASK [ecs_cluster : disable taskLongArnFormat] *************************************************************************************************************************************************************
task path: /root/ansible_collections/community/aws/tests/output/.tmp/integration/ecs_cluster-bmqn6pdd-ÅÑŚÌβŁÈ/tests/integration/targets/ecs_cluster/tasks/full_test.yml:759
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<testhost> EXEC /bin/sh -c 'echo ~root && sleep 0'
<testhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /root/.ansible/tmp `"&& mkdir "` echo /root/.ansible/tmp/ansible-tmp-1644012273.1265073-8916-231925734205881 `" && echo ansible-tmp-1644012273.1265073-8916-231925734205881="` echo /root/.ansible/tmp/ansible-tmp-1644012273.1265073-8916-231925734205881 `" ) && sleep 0'
Using module file /root/ansible/lib/ansible/modules/command.py
<testhost> PUT /root/.ansible/tmp/ansible-local-7475irwy76p7/tmp2avad1wm TO /root/.ansible/tmp/ansible-tmp-1644012273.1265073-8916-231925734205881/AnsiballZ_command.py
<testhost> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1644012273.1265073-8916-231925734205881/ /root/.ansible/tmp/ansible-tmp-1644012273.1265073-8916-231925734205881/AnsiballZ_command.py && sleep 0'
<testhost> EXEC /bin/sh -c 'ANSIBLE_DEBUG_BOTOCORE_LOGS=True AWS_ACCESS_KEY_ID=****************8 AWS_SECRET_ACCESS_KEY=**********************AWS_SESSION_TOKEN=************************* AWS_DEFAULT_REGION=us-west-2 /usr/bin/python /root/.ansible/tmp/ansible-tmp-1644012273.1265073-8916-231925734205881/AnsiballZ_command.py && sleep 0'
fatal: [testhost]: FAILED! => {
"changed": true,
"cmd": [
"aws",
"ecs",
"put-account-setting",
"--name",
"taskLongArnFormat",
"--value",
"disabled"
],
"delta": "0:00:00.881105",
... |
ansibullbot
added
bug
|
Build succeeded (third-party-check pipeline).
|
|
Build failed.
|
|
@alinabuzachis Could you kick off another build of this PR? The latest build failed because of some issue with the CI/CD trying to install a Python package, which was totally unrelated to the changes in this PR. |
|
recheck |
|
Build failed.
|
|
Build succeeded.
|
alinabuzachis
approved these changes
Feb 17, 2022
There was a problem hiding this comment.
@mjmayer Thank you very much for your contribution. This LGTM!
markuman
added
backport-2
markuman
approved these changes
Feb 18, 2022
|
Build failed (gate pipeline). For information on how to proceed, see
|
alinabuzachis
added
mergeit
|
Build failed (gate pipeline). For information on how to proceed, see
|
alinabuzachis
added
mergeit
|
regate |
Backport to stable-2: 💚 backport PR created✅ Backport PR branch: Backported as #1007 🤖 @patchback |
patchback bot
pushed a commit
that referenced
this pull request
Mar 18, 2022
Support changing of launch type SUMMARY When changing the launch_type parameter for an ecs_taskdefition there was no change reported by the module. This adds a check to see if launch_type in the ecs_taskdefinition has changed. If there is a change detected the module reports back there is not a matching task definition and creates a new one. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_taskdefinition Reviewed-by: Alina Buzachis <None> Reviewed-by: Michael Mayer <mjmayer@gmail.com> Reviewed-by: Markus Bergholz <git@osuv.de> (cherry picked from commit c67c890)
Backport to stable-3: 💚 backport PR created✅ Backport PR branch: Backported as #1008 🤖 @patchback |
patchback bot
pushed a commit
that referenced
this pull request
Mar 18, 2022
Support changing of launch type SUMMARY When changing the launch_type parameter for an ecs_taskdefition there was no change reported by the module. This adds a check to see if launch_type in the ecs_taskdefinition has changed. If there is a change detected the module reports back there is not a matching task definition and creates a new one. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_taskdefinition Reviewed-by: Alina Buzachis <None> Reviewed-by: Michael Mayer <mjmayer@gmail.com> Reviewed-by: Markus Bergholz <git@osuv.de> (cherry picked from commit c67c890)
softwarefactory-project-zuul bot
pushed a commit
that referenced
this pull request
Mar 19, 2022
[PR #840/c67c8907 backport][stable-3] Support changing of launch type This is a backport of PR #840 as merged into main (c67c890). SUMMARY When changing the launch_type parameter for an ecs_taskdefition there was no change reported by the module. This adds a check to see if launch_type in the ecs_taskdefinition has changed. If there is a change detected the module reports back there is not a matching task definition and creates a new one. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_taskdefinition
softwarefactory-project-zuul bot
pushed a commit
that referenced
this pull request
Mar 19, 2022
[PR #840/c67c8907 backport][stable-2] Support changing of launch type This is a backport of PR #840 as merged into main (c67c890). SUMMARY When changing the launch_type parameter for an ecs_taskdefition there was no change reported by the module. This adds a check to see if launch_type in the ecs_taskdefinition has changed. If there is a change detected the module reports back there is not a matching task definition and creates a new one. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_taskdefinition
1 task
abikouo
pushed a commit
to abikouo/community.aws
that referenced
this pull request
Oct 24, 2023
…sible-collections#840) Add support for copy_db_cluster_snapshot for rds_cluster_snapshot SUMMARY Add support for copy_db_cluster_snapshot for rds_cluster_snapshot Necessary for ansible-collections#788 Just to verify: Depends-On: ansible/ansible-zuul-jobs#1520 ISSUE TYPE Feature Pull Request COMPONENT NAME module_utils/rds.py ADDITIONAL INFORMATION Reviewed-by: Joseph Torcasso <None> Reviewed-by: Jill R <None>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
When changing the
launch_typeparameter for anecs_taskdefitionthere was no change reported by the module. This adds a check to see iflaunch_typein theecs_taskdefinitionhas changed. If there is a change detected the module reports back there is not a matching task definition and creates a new one.ISSUE TYPE
COMPONENT NAME
ecs_taskdefinition