purge_tags: True default causes version incompatibilities and strips all ASG tags

bedge commented 2 years ago

Summary

Using default=True for the new purge_tags field, causes any ansible action on an already existing ASG to strip all its tags.

https://github.com/ansible-collections/community.aws/pull/960/files#diff-629ed6761ca29636823559acc0c8f4aaa6c405c299e596ac5cddacc49447f569R227

This is a "bad thing" for ASGs especially because the ASG tags are used to convey context information to the instances created by the ASG.

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.

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.

I would expect ansible to alter "only what is specified" and expect the rest to remain as-is.

Issue Type

Bug Report

Component Name

ec2_asg

Ansible Version

 ansible [core 2.12.4]
   config file = None
   configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
   ansible python module location = /usr/lib/python3.9/site-packages/ansible
   ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
   executable location = /usr/bin/ansible
   python version = 3.9.7 (default, Nov 24 2021, 21:15:59) [GCC 10.3.1 20211027]
   jinja version = 3.1.1
   libyaml = False

Collection Versions

 # /usr/lib/python3.9/site-packages/ansible_collections
 Collection                    Version
 ----------------------------- -------
 amazon.aws                    1.5.1  
 ansible.netcommon             2.5.0  
 ansible.posix                 1.3.0  
 ansible.utils                 2.4.3  
 ansible.windows               1.8.0  
 arista.eos                    2.2.0  
 awx.awx                       19.4.0 
 azure.azcollection            1.10.0 
 check_point.mgmt              2.2.0  
 chocolatey.chocolatey         1.1.0  
 cisco.aci                     2.1.0  
 cisco.asa                     2.1.0  
 cisco.intersight              1.0.18 
 cisco.ios                     2.6.0  
 cisco.iosxr                   2.6.0  
 cisco.meraki                  2.5.0  
 cisco.mso                     1.2.0  
 cisco.nso                     1.0.3  
 cisco.nxos                    2.8.2  
 cisco.ucs                     1.6.0  
 cloudscale_ch.cloud           2.2.0  
 community.aws                 1.5.0  
 community.azure               1.1.0  
 community.crypto              1.9.8  
 community.digitalocean        1.13.0 
 community.docker              1.10.2 
 community.fortios             1.0.0  
 community.general             3.8.3  
 community.google              1.0.0  
 community.grafana             1.3.0  
 community.hashi_vault         1.5.0  
 community.hrobot              1.2.1  
 community.kubernetes          1.2.1  
 community.kubevirt            1.0.0  
 community.libvirt             1.0.2  
 community.mongodb             1.3.2  
 community.mysql               2.3.2  
 community.network             3.0.0  
 community.okd                 1.1.2  
 community.postgresql          1.6.0  
 community.proxysql            1.3.0  
 community.rabbitmq            1.1.0  
 community.routeros            1.2.0  
 community.skydive             1.0.0  
 community.sops                1.2.0  
 community.vmware              1.17.0 
 community.windows             1.8.0  
 community.zabbix              1.5.1  
 containers.podman             1.9.0  
 cyberark.conjur               1.1.0  
 cyberark.pas                  1.0.13 
 dellemc.enterprise_sonic      1.1.0  
 dellemc.openmanage            3.6.0  
 dellemc.os10                  1.1.1  
 dellemc.os6                   1.0.7  
 dellemc.os9                   1.0.4  
 f5networks.f5_modules         1.13.0 
 fortinet.fortimanager         2.1.4  
 fortinet.fortios              2.1.3  
 frr.frr                       1.0.3  
 gluster.gluster               1.0.2  
 google.cloud                  1.0.2  
 hetzner.hcloud                1.6.0  
 hpe.nimble                    1.1.4  
 ibm.qradar                    1.0.3  
 infinidat.infinibox           1.3.0  
 inspur.sm                     1.3.0  
 junipernetworks.junos         2.8.0  
 kubernetes.core               1.2.1  
 mellanox.onyx                 1.0.0  
 netapp.aws                    21.7.0 
 netapp.azure                  21.10.0
 netapp.cloudmanager           21.12.1
 netapp.elementsw              21.7.0 
 netapp.ontap                  21.14.1
 netapp.um_info                21.8.0 
 netapp_eseries.santricity     1.2.13 
 netbox.netbox                 3.4.0  
 ngine_io.cloudstack           2.2.2  
 ngine_io.exoscale             1.0.0  
 ngine_io.vultr                1.1.0  
 openstack.cloud               1.5.3  
 openvswitch.openvswitch       2.1.0  
 ovirt.ovirt                   1.6.6  
 purestorage.flasharray        1.11.0 
 purestorage.flashblade        1.8.1  
 sensu.sensu_go                1.12.0 
 servicenow.servicenow         1.0.6  
 splunk.es                     1.0.2  
 t_systems_mms.icinga_director 1.26.0 
 theforeman.foreman            2.2.0  
 vyos.vyos                     2.6.0  
 wti.remote                    1.0.3  

 # /root/.ansible/collections/ansible_collections
 Collection                    Version
 ----------------------------- -------
 amazon.aws                    3.2.0  
 ansible.netcommon             3.0.0  
 ansible.posix                 1.3.0  
 ansible.utils                 2.6.1  
 ansible.windows               1.9.0  
 arista.eos                    5.0.0  
 awx.awx                       21.0.0 
 azure.azcollection            1.12.0 
 check_point.mgmt              2.3.0  
 chocolatey.chocolatey         1.2.0  
 cisco.aci                     2.2.0  
 cisco.asa                     3.0.0  
 cisco.intersight              1.0.18 
 cisco.ios                     3.0.0  
 cisco.iosxr                   3.0.0  
 cisco.ise                     2.3.2  
 cisco.meraki                  2.6.1  
 cisco.mso                     2.0.0  
 cisco.nso                     1.0.3  
 cisco.nxos                    3.0.0  
 cisco.ucs                     1.8.0  
 cloud.common                  2.1.1  
 cloudscale_ch.cloud           2.2.1  
 community.aws                 3.2.1  
 community.azure               1.1.0  
 community.ciscosmb            1.0.5  
 community.crypto              2.2.4  
 community.digitalocean        1.18.0 
 community.dns                 2.1.0  
 community.docker              2.4.0  
 community.fortios             1.0.0  
 community.general             4.8.0  
 community.google              1.0.0  
 community.grafana             1.4.0  
 community.hashi_vault         2.4.0  
 community.hrobot              1.3.0  
 community.kubernetes          2.0.1  
 community.kubevirt            1.0.0  
 community.libvirt             1.0.2  
 community.mongodb             1.3.4  
 community.mysql               3.1.3  
 community.network             3.1.0  
 community.okd                 2.2.0  
 community.postgresql          2.1.4  
 community.proxysql            1.3.2  
 community.rabbitmq            1.1.0  
 community.routeros            2.0.0  
 community.sap                 1.0.0  
 community.skydive             1.0.0  
 community.sops                1.2.1  
 community.vmware              2.3.0  
 community.windows             1.9.0  
 community.zabbix              1.6.0  
 containers.podman             1.9.3  
 cyberark.conjur               1.1.0  
 cyberark.pas                  1.0.13 
 dellemc.enterprise_sonic      1.1.0  
 dellemc.openmanage            5.3.0  
 dellemc.os10                  1.1.1  
 dellemc.os6                   1.0.7  
 dellemc.os9                   1.0.4  
 f5networks.f5_modules         1.16.0 
 fortinet.fortimanager         2.1.5  
 fortinet.fortios              2.1.5  
 frr.frr                       2.0.0  
 gluster.gluster               1.0.2  
 google.cloud                  1.0.2  
 hetzner.hcloud                1.6.0  
 hpe.nimble                    1.1.4  
 ibm.qradar                    2.0.0  
 infinidat.infinibox           1.3.3  
 infoblox.nios_modules         1.2.1  
 inspur.sm                     2.0.0  
 junipernetworks.junos         3.0.0  
 kubernetes.core               2.3.1  
 mellanox.onyx                 1.0.0  
 netapp.aws                    21.7.0 
 netapp.azure                  21.10.0
 netapp.cloudmanager           21.17.0
 netapp.elementsw              21.7.0 
 netapp.ontap                  21.19.0
 netapp.storagegrid            21.10.0
 netapp.um_info                21.8.0 
 netapp_eseries.santricity     1.3.0  
 netbox.netbox                 3.7.1  
 ngine_io.cloudstack           2.2.3  
 ngine_io.exoscale             1.0.0  
 ngine_io.vultr                1.1.1  
 openstack.cloud               1.8.0  
 openvswitch.openvswitch       2.1.0  
 ovirt.ovirt                   2.0.3  
 purestorage.flasharray        1.13.0 
 purestorage.flashblade        1.9.0  
 sensu.sensu_go                1.13.1 
 servicenow.servicenow         1.0.6  
 splunk.es                     2.0.0  
 t_systems_mms.icinga_director 1.29.0 
 theforeman.foreman            3.3.0  
 vyos.vyos                     3.0.0  
 wti.remote                    1.0.3

AWS SDK versions

Name: botocore
 Version: 1.25.9

Configuration

$ ansible-config dump --only-changed
<empty>

OS / Environment

Ubuntu 18.04.6 LTS

Steps to Reproduce

Case 1)

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

With the current default of purge_tags = true, the above and strips all tags.

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.

Case 2)

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".

Also, note that this case 2 fails in earlier versions that didn't have the purge_tags element.

Expected Results

In case 1 above, the new default for purge_tags: True causes any ansible action on an already existing ASG to strip all its tags. This is a "bad thing" for ASGs especially because the ASG tags are used to convey context information to the instances created by the ASG.

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??

This is not "principle of least surprising behavior"?

The code in case 2 is now required to get ansible to NOT destroy all tags of existing ASGs.

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.

One has to know if the runtime is using a release before or after this PR and adapt accordingly. https://github.com/ansible-collections/community.aws/pull/960/files#diff-629ed6761ca29636823559acc0c8f4aaa6c405c299e596ac5cddacc49447f569R227 That is not backwards compatible

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.

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.

I would expect ansible to alter "only what is specified" and expect the rest to remain as-is.

My use case may differ from what others are doing and this may be the source of for problem - I am not provisioning with ansible, I am using ansible to alter state of existing assets provisioned by terraform. This is why I expect ansible change only what is specified.

Actual Results

Both cases are above, hard to separate and explain.

Code of Conduct

[X] I agree to follow the Ansible Code of Conduct

ansibullbot commented 2 years ago

Files identified in the description:

[plugins/modules/ec2_asg.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/ec2_asg.py)

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 2 years ago

cc @garethr @jillr @markuman @s-hertel @tremble click here for bot help

markuman commented 2 years ago

The purge_tag parameter with default true was released with 3.2.0 and revertet to defaults false in 3.2.1

This is why I expect ansible change only what is specified.

That is the behaviour when using purge_tags: false.
And it is backwards compatible, because the previous version wasn't able to purge tags that were not specified.

markuman commented 2 years ago

Closed via https://github.com/ansible-collections/community.aws/pull/1064
Released in 3.2.1

ansible-collections / community.aws