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

Azure Common Module (azure_rm_common.py) #28373

Closed
wants to merge 3 commits into from
Closed

Azure Common Module (azure_rm_common.py) #28373

wants to merge 3 commits into from

Conversation

haroldwongms
Copy link
Contributor

Remove tags

SUMMARY

Provide update to handle removing of tags from Azure resources.

If Azure resource has tags and you want to remove all tags (empty list), the current update_tags method is returning false which is incorrect.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_common.py

ANSIBLE VERSION
ansible 2.4.0 (common 393909d0cc) last updated 2017/08/15 15:36:14 (GMT -700)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/haroldwong/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /mnt/c/Users/haroldw/Documents/develop/ansible24/lib/ansible
  executable location = /mnt/c/Users/haroldw/Documents/develop/ansible24/bin/ansible
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bugfix_pull_request c:module_utils/ needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Aug 17, 2017
@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Aug 17, 2017
@nitzmahone
Copy link
Member

I don't think this is actually the behavior you want in nearly any case- this would cause things to be completely untagged if a later task came to modify some property of any resource and forgot to specify tags.

The behavior you want should already work if tags was actually specified as an empty dictionary...

@haroldwongms
Copy link
Contributor Author

@nitzmahone That is a good point. I will update to look for empty dictionary.

specify empty set - {} to remove all tags
@haroldwongms
Copy link
Contributor Author

@nitzmahone made the changes to look for empty dictionary - {} to handle deleting of tags.

@nitzmahone
Copy link
Member

I don't think that works (or if it does, it's kinda by accident)- what I'm saying is that at least by inspection, the existing code should already do the right thing if you pass an empty dict- in particular this section:

if isinstance(tags, dict):
  for key, value in tags.items():
    if not self.module.params['tags'].get(key):
      new_tags.pop(key)

@haroldwongms
Copy link
Contributor Author

@nitzmahone You are so correct! My mistake. Closing PR.

@haroldwongms haroldwongms deleted the azure_rm_common-fix branch August 18, 2017 23:17
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. c:module_utils/ support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants