ansible-collections / google.cloud

GCP Ansible Collection https://galaxy.ansible.com/google/cloud
https://cloud.google.com
GNU General Public License v3.0
99 stars 126 forks source link

gcp_compute_instance module doesn't update instance tags #307

Open averzicco opened 3 years ago

averzicco commented 3 years ago

SUMMARY

gcp_compute_instance module doesn't update instance tags for an existing instance. For example the same module can update labels, but apparently not tags. code to update labels: https://github.com/ansible-collections/google.cloud/blob/7fa00db18cdc287629b851b80f05cb321f15e8d9/plugins/modules/gcp_compute_instance.py#L1209-L1214

but no code to update tags

fyi this issue was also reported in the ansible repository few years ago: https://github.com/ansible/ansible/issues/47883

ISSUE TYPE
COMPONENT NAME

gcp_compute_instance

ANSIBLE VERSION
ansible 2.9.11
  config file = /home/alex/ansible-project/ansible.cfg
  configured module search path = ['/home/alex/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/alex/.local/lib/python3.8/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.8.5 (default, Aug 12 2020, 00:00:00) [GCC 10.2.1 20200723 (Red Hat 10.2.1-1)]
CONFIGURATION
DEFAULT_VAULT_PASSWORD_FILE(env: ANSIBLE_VAULT_PASSWORD_FILE) = /home/alex/.vault_pass.txt
HOST_KEY_CHECKING(/home/alex/ansible-project/ansible.cfg) = False
INVENTORY_ENABLED(/home/alex/ansible-project/ansible.cfg) = ['google.cloud.gcp_compute', 'auto', 'yaml', 'ini', 'toml']
OS / ENVIRONMENT

Fedora 32

STEPS TO REPRODUCE
EXPECTED RESULTS

machine has the tags defined

ACTUAL RESULTS

machine doesn't have tags defined

smemsh commented 3 years ago

also, the module use docs are a bit misleading here:

You must always provide an up-to-date fingerprint hash in order to update or change metadata.

because even if setting tags on existing instances worked, one would not need to provide the fingerprint in module args, it would be done internally by the code (as it's done for labels, in the code snippet shown in description).

Note, setting tags does work at instance create-time, also without setting fingerprint. So the fingerprint should probably just be removed from the docs (and for that matter, the strange need to set "items" key with the tag list as its value).

NeverUsedID commented 3 years ago

Same problem here, metadata is not deployed. Lables are working. For metadata you will find the reason in the sorucecode:

"# TODO(alexstephen): Implement updating metadata on existing resources."

Rylon commented 1 year ago

Same issue still, this breaks the idempotency of the module, it's supported by Compute Engine to modify these settings, but the Ansible module does not attempt to make changes, nor does it report any changes as being required, even though the desired state and actual state do not match. Can anything be done?

valkiriaaquatica commented 6 months ago

Hey @Rylon @NeverUsedID @smemsh @averzicco you still have the same problem? This module is focused on "creating" and instance, not in updating the instance. Do you wanna add some labels/tags to the instance with idempotency? (the instance has label env: dev and you want to add office: thailand and you want the two labels to exists, right?)

Thanks in advanced!

Rylon commented 6 months ago

Hi @valkiriaaquatica yeah, it's not very idiomatic otherwise. Ansible is declarative, so you should think in terms of declaring a desired state, and the module then compares this to the actual state, and makes any changes that may be necessary. If certain attributes are "write once" this breaks that expectation, and instead you have some tasks for creating, some tasks for updating, which feels more imperative than declarative.

For example maybe the name of a department label changes, and you want to apply that, you should be able to change it once in the Ansible role/tasks, and then re-run the necessary playbooks, and the necessary changes be applied declaratively, without needing to think of it like a series of changes to be executed.

valkiriaaquatica commented 6 months ago

Oh I forgot @Rylon to put here the pr i did implmenting this changes https://github.com/ansible-collections/google.cloud/pull/622 , is it that what you meant? I added an option in case the user wants the previous tags to keep them or to remoe them

Thanks in advance!

Rylon commented 6 months ago

Yeah that new module helps for sure, but it would mean you can't manage the tags in the first module, as they'll end up clashing with one another, or being incorrect, for example if you changed the tags in the first module, and re-applied, nothing would happen, which would break expectations. It still feels more idiomatic in Ansible that the module handles all these scenarios for you, so the user only needs to declare the desired state, not worry about how to get there.