ec2_asg: Add purge_tags to AutoScalingGroups. #960
Conversation
ansibullbot
added
community_review
feature
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
mandar242
changed the title
mandar242
changed the title
alinabuzachis
added
the
backport-3
This comment was marked as outdated.
This comment was marked as outdated.
mandar242
added
mergeit
mandar242
added
mergeit
Backport to stable-3: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 1ca997a on top of patchback/backports/stable-3/1ca997af9c7bb31197d7951d3d395c9cc48ab842/pr-960 Backporting merged PR #960 into main
🤖 @patchback |
ec2_asg: Add purge_tags to AutoScalingGroups. SUMMARY Add purge_tags to ec2_asg module. Fixes ansible-collections#481. ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_asg ADDITIONAL INFORMATION There was another PR (currently closed) ansible-collections#482 - with similar functionality but I'm not sure if having modules to handle tags for individual services/modules is a good way to have this functionality. It will certainly cause increase in number of modules. Hence tried modifying existing ec2_asg module to be able to do this. This utilizes underlying API calls to: https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_DescribeTags.html https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_CreateOrUpdateTags.html https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_DeleteTags.html Reviewed-by: Alina Buzachis <None> Reviewed-by: Jill R <None> Reviewed-by: Mandar Kulkarni <[email protected]>
ec2_asg: backport PRs 960 and 933 SUMMARY ec2_asg: backports PRs #960 and #933 ISSUE TYPE Bugfix Pull Request Docs Pull Request Feature Pull Request New Module Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <[email protected]>
| description: | ||
| - If C(true), existing tags will be purged from the resource to match exactly what is defined by I(tags) parameter. | ||
| - If the I(tags) parameter is not set then tags will not be modified. | ||
| default: true |
There was a problem hiding this comment.
Forgive me for commenting on an already merged PR, but I just found out about this from a complete breakage of all ansible AWS services that use tags.
Why was default=True chosen, for a new field that no one was using previously, deemed a good idea?
Now, everything that touches any asset that uses tags MUST now add a purge_tags: False just to leave things as they are??
Otherwise all tags are deleted???
Existing use case to scale in/out an ASG, and touch nothing else:
- community.aws.ec2_asg:
name: "{{ item.auto_scaling_group_name }}"
min_size: "{{ count }}"
max_size: "{{ count }}"
desired_capacity: "{{ count }}"
region: eu-central-1
Now, the above and strips all tags as well.
Given that most ASG tags are used to propagate context information to the ec2 instances, this now destroys all context information for the ASG and the instances it creates have no information about the runtime context.
How is that the "principle of least surprising behavior"?
This now requires adding the purge_tags: False everywhere where tags could be impacted.
- community.aws.ec2_asg:
name: "{{ item.auto_scaling_group_name }}"
min_size: "{{ count }}"
max_size: "{{ count }}"
desired_capacity: "{{ count }}"
purge_tags: False <----- This is now needed everywhere tags are used to say "leave alone" ?
region: eu-central-1
This is the ONLY case where one has to specify a non-default option just to say: "leave alone".
Note also that
purge_tags: False
is NOT backwards compatible, because that field didn't exist in previous versions, so you need 2 different cases depending on which side of this version you fall on.
Given that the purge_tags things seems to have propagated to many AWS assets, all in slightly different releases, one needs to account for this on a case by case basis depending on when the purge_tags was introduced into each module.
Now, if the default had been purge_tags=False, none of this would be an issue.
There would have been no changes required as the new behavior would match the old.
Am I the only one complaining about this? I can't believe this didn't hit more people.
Does no use use ansible to alter "only what is specified" and expect the rest to remain as-is?
I can't decide now whether to walk away from ansible as an AWS "minor edit" tool or, try deal with this "maybe we'll change something that destroys something else later, that you can't know about until it happens.
|
Hi @bedge. I'm sorry to hear that this change caused unexpected behaviour in your environment, that's definitely not the experience we want users to have. The developers who work on this collection are human and while we try our best sometimes humans make mistakes, or we make decisions without the same context that someone else has. I can assure you that no one is setting out to "change something that destroys something else later, that you can't know about until it happens". We strive to communicate all changes in a release in the changelog, if there's something we could have done differently with that changelog entry we would also welcome that feedback in the bug report. |
ec2_asg: Change purge_tags default value to False SUMMARY Changed default value of purge_tags to False. With the addition of purge_tags to ec2_asg module #960, the default value was kept to True similar to many other modules in this collection and also as specified in ansible dev guide. With this addition, the issue was discovered that there is a possibility of this change breaking existing playbooks for users if they don't update their playbooks and specify purge_tags: false where required. This PR's change will prevent accidental breakage. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_asg Reviewed-by: Mark Chappell <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Woolley <[email protected]> Reviewed-by: Alina Buzachis <None>
ec2_asg: Change purge_tags default value to False SUMMARY Changed default value of purge_tags to False. With the addition of purge_tags to ec2_asg module #960, the default value was kept to True similar to many other modules in this collection and also as specified in ansible dev guide. With this addition, the issue was discovered that there is a possibility of this change breaking existing playbooks for users if they don't update their playbooks and specify purge_tags: false where required. This PR's change will prevent accidental breakage. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_asg Reviewed-by: Mark Chappell <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Mark Woolley <[email protected]> Reviewed-by: Alina Buzachis <None> (cherry picked from commit d0c9f1a)
[PR #1064/d0c9f1a6 backport][stable-3] ec2_asg: Change purge_tags default value to False This is a backport of PR #1064 as merged into main (d0c9f1a). SUMMARY Changed default value of purge_tags to False. With the addition of purge_tags to ec2_asg module #960, the default value was kept to True similar to many other modules in this collection and also as specified in ansible dev guide. With this addition, the issue was discovered that there is a possibility of this change breaking existing playbooks for users if they don't update their playbooks and specify purge_tags: false where required. This PR's change will prevent accidental breakage. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_asg Reviewed-by: Mark Chappell <None>
SUMMARY
Add
purge_tagsto ec2_asg module.Fixes #481.
ISSUE TYPE
COMPONENT NAME
ec2_asg
ADDITIONAL INFORMATION
There was another PR (currently closed) #482 - with similar functionality but I'm not sure if having modules to handle tags for individual services/modules is a good way to have this functionality. It will certainly cause increase in number of modules.
Hence tried modifying existing
ec2_asgmodule to be able to do this.This utilizes underlying API calls to:
https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_DescribeTags.html
https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_CreateOrUpdateTags.html
https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_DeleteTags.html