From 35cc26f8c0447ab1ad4427eafcc7283c4356370d Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Wed, 20 Nov 2019 14:43:04 +0530 Subject: [PATCH] VMware: Find unique tag using category id (#61082) If two tags with same name and different category exists, vmware_tag_manager used to take first found tag. This commit use combination of tag and category to identify the category. Fixes: #59379 Signed-off-by: Abhijeet Kasurde --- .../59379-vmware_tag_manager-use_category.yml | 2 + .../module_utils/vmware_rest_client.py | 72 +++++++- .../modules/cloud/vmware/vmware_tag.py | 19 +- .../cloud/vmware/vmware_tag_manager.py | 26 ++- .../targets/vmware_tag/tasks/main.yml | 1 + .../tasks/tag_manager_duplicate_tag_cat.yml | 163 ++++++++++++++++++ 6 files changed, 252 insertions(+), 31 deletions(-) create mode 100644 changelogs/fragments/59379-vmware_tag_manager-use_category.yml create mode 100644 test/integration/targets/vmware_tag/tasks/tag_manager_duplicate_tag_cat.yml diff --git a/changelogs/fragments/59379-vmware_tag_manager-use_category.yml b/changelogs/fragments/59379-vmware_tag_manager-use_category.yml new file mode 100644 index 0000000000..e9ba6762b2 --- /dev/null +++ b/changelogs/fragments/59379-vmware_tag_manager-use_category.yml @@ -0,0 +1,2 @@ +bugfixes: + - Use Category id to uniquely identify a tag when multiple tags with same name is available (https://github.com/ansible/ansible/issues/59379). diff --git a/lib/ansible/module_utils/vmware_rest_client.py b/lib/ansible/module_utils/vmware_rest_client.py index b0baf62675..dbe0a621c0 100644 --- a/lib/ansible/module_utils/vmware_rest_client.py +++ b/lib/ansible/module_utils/vmware_rest_client.py @@ -162,12 +162,11 @@ class VmwareRestClient(object): return tags - def get_tags_for_dynamic_obj(self, mid=None, type=None): + def get_tags_for_dynamic_obj(self, mid=None): """ Return list of tag object details associated with object Args: mid: Dynamic object for specified object - type: Type of DynamicID to lookup Returns: List of tag object details associated with the given object @@ -175,9 +174,8 @@ class VmwareRestClient(object): tags = [] if mid is None: return tags - dynamic_managed_object = DynamicID(type=type, id=mid) - temp_tags_model = self.get_tags_for_object(dynamic_managed_object) + temp_tags_model = self.get_tags_for_object(dobj=mid) category_service = self.api_client.tagging.Category @@ -201,7 +199,7 @@ class VmwareRestClient(object): Returns: List of tag object associated with the given cluster """ - return self.get_tags_for_dynamic_obj(mid=cluster_mid, type='ClusterComputeResource') + return self.get_tags_for_dynamic_obj(mid=cluster_mid) def get_tags_for_hostsystem(self, hostsystem_mid=None): """ @@ -212,7 +210,7 @@ class VmwareRestClient(object): Returns: List of tag object associated with the given host system """ - return self.get_tags_for_dynamic_obj(mid=hostsystem_mid, type='HostSystem') + return self.get_tags_for_dynamic_obj(mid=hostsystem_mid) def get_tags_for_vm(self, vm_mid=None): """ @@ -223,7 +221,7 @@ class VmwareRestClient(object): Returns: List of tag object associated with the given virtual machine """ - return self.get_tags_for_dynamic_obj(mid=vm_mid, type='VirtualMachine') + return self.get_tags_for_dynamic_obj(mid=vm_mid) def get_vm_tags(self, tag_service=None, tag_association_svc=None, vm_mid=None): """ @@ -241,9 +239,12 @@ class VmwareRestClient(object): tags = [] if vm_mid is None: return tags - dynamic_managed_object = DynamicID(type='VirtualMachine', id=vm_mid) - temp_tags_model = self.get_tags_for_object(tag_service, tag_association_svc, dynamic_managed_object) + temp_tags_model = self.get_tags_for_object( + tag_service=tag_service, + tag_assoc_svc=tag_association_svc, + dobj=vm_mid + ) for tag_obj in temp_tags_model: tags.append(tag_obj.name) @@ -369,3 +370,56 @@ class VmwareRestClient(object): if svc_obj.name == svc_obj_name: return svc_obj return None + + def get_tag_by_name(self, tag_name=None): + """ + Return tag object by name + Args: + tag_name: Name of tag + + Returns: Tag object if found else None + """ + if not tag_name: + return None + + return self.search_svc_object_by_name(service=self.api_client.tagging.Tag, svc_obj_name=tag_name) + + def get_category_by_name(self, category_name=None): + """ + Return category object by name + Args: + category_name: Name of category + + Returns: Category object if found else None + """ + if not category_name: + return None + + return self.search_svc_object_by_name(service=self.api_client.tagging.Category, svc_obj_name=category_name) + + def get_tag_by_category(self, tag_name=None, category_name=None): + """ + Return tag object by name and category name specified + Args: + tag_name: Name of tag + category_name: Name of category + + Returns: Tag object if found else None + """ + + if not tag_name: + return None + + if category_name: + category_obj = self.get_category_by_name(category_name=category_name) + + if not category_obj: + return None + + for tag_object in self.api_client.tagging.Tag.list(): + tag_obj = self.api_client.tagging.Tag.get(tag_object) + + if tag_obj.name == tag_name and tag_obj.category_id == category_obj.id: + return tag_obj + else: + return self.search_svc_object_by_name(service=self.api_client.tagging.Tag, svc_obj_name=tag_name) diff --git a/lib/ansible/modules/cloud/vmware/vmware_tag.py b/lib/ansible/modules/cloud/vmware/vmware_tag.py index 0cc74c340e..b2c5e53adb 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_tag.py +++ b/lib/ansible/modules/cloud/vmware/vmware_tag.py @@ -230,7 +230,13 @@ class VmwareTag(VmwareRestClient): Returns: 'present' if tag found, else 'absent' """ - ret = 'present' if self.tag_name in self.global_tags else 'absent' + if 'category_id' in self.params: + if self.tag_name in self.global_tags and self.params['category_id'] == self.global_tags[self.tag_name]['tag_category_id']: + ret = 'present' + else: + ret = 'absent' + else: + ret = 'present' if self.tag_name in self.global_tags else 'absent' return ret def get_all_tags(self): @@ -240,11 +246,12 @@ class VmwareTag(VmwareRestClient): """ for tag in self.tag_service.list(): tag_obj = self.tag_service.get(tag) - self.global_tags[tag_obj.name] = dict(tag_description=tag_obj.description, - tag_used_by=tag_obj.used_by, - tag_category_id=tag_obj.category_id, - tag_id=tag_obj.id - ) + self.global_tags[tag_obj.name] = dict( + tag_description=tag_obj.description, + tag_used_by=tag_obj.used_by, + tag_category_id=tag_obj.category_id, + tag_id=tag_obj.id + ) def main(): diff --git a/lib/ansible/modules/cloud/vmware/vmware_tag_manager.py b/lib/ansible/modules/cloud/vmware/vmware_tag_manager.py index 1cdc698d1f..48b5c2e87f 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_tag_manager.py +++ b/lib/ansible/modules/cloud/vmware/vmware_tag_manager.py @@ -195,12 +195,6 @@ class VmwareTagManager(VmwareRestClient): self.tag_names = self.params.get('tag_names') - def is_tag_category(self, cat_obj, tag_obj): - for tag in self.tag_service.list_tags_for_category(cat_obj.id): - if tag_obj.name == self.tag_service.get(tag).name: - return True - return False - def ensure_state(self): """ Manage the internal state of tags @@ -215,9 +209,9 @@ class VmwareTagManager(VmwareRestClient): available_tag_obj = self.get_tags_for_object(tag_service=self.tag_service, tag_assoc_svc=self.tag_association_svc, dobj=self.dynamic_managed_object) - # Already existing tags from the given object - avail_tag_obj_name_list = [tag.name for tag in available_tag_obj] - results['tag_status']['previous_tags'] = avail_tag_obj_name_list + + _temp_prev_tags = ["%s:%s" % (tag['category_name'], tag['name']) for tag in self.get_tags_for_dynamic_obj(mid=self.dynamic_managed_object)] + results['tag_status']['previous_tags'] = _temp_prev_tags results['tag_status']['desired_tags'] = self.tag_names # Check if category and tag combination exists as per user request @@ -234,13 +228,14 @@ class VmwareTagManager(VmwareRestClient): # User specified only tag tag_name = tag - tag_obj = self.search_svc_object_by_name(self.tag_service, tag_name) + if category_name: + tag_obj = self.get_tag_by_category(tag_name=tag_name, category_name=category_name) + else: + tag_obj = self.get_tag_by_name(tag_name=tag_name) + if not tag_obj: self.module.fail_json(msg="Unable to find the tag %s" % tag_name) - if category_name and category_obj and not self.is_tag_category(category_obj, tag_obj): - self.module.fail_json(msg="Category %s does not contain tag %s" % (category_name, tag_name)) - if action in ('add', 'present'): if tag_obj not in available_tag_obj: # Tag is not already applied @@ -270,9 +265,8 @@ class VmwareTagManager(VmwareRestClient): except Error as error: self.module.fail_json(msg="%s" % self.get_error_message(error)) - results['tag_status']['current_tags'] = [tag.name for tag in self.get_tags_for_object(self.tag_service, - self.tag_association_svc, - self.dynamic_managed_object)] + _temp_curr_tags = ["%s:%s" % (tag['category_name'], tag['name']) for tag in self.get_tags_for_dynamic_obj(mid=self.dynamic_managed_object)] + results['tag_status']['current_tags'] = _temp_curr_tags results['changed'] = changed self.module.exit_json(**results) diff --git a/test/integration/targets/vmware_tag/tasks/main.yml b/test/integration/targets/vmware_tag/tasks/main.yml index 95c9753e36..8961c6916a 100644 --- a/test/integration/targets/vmware_tag/tasks/main.yml +++ b/test/integration/targets/vmware_tag/tasks/main.yml @@ -4,3 +4,4 @@ - include: tag_crud_ops.yml - include: tag_manager_ops.yml +- include: tag_manager_duplicate_tag_cat.yml diff --git a/test/integration/targets/vmware_tag/tasks/tag_manager_duplicate_tag_cat.yml b/test/integration/targets/vmware_tag/tasks/tag_manager_duplicate_tag_cat.yml new file mode 100644 index 0000000000..e6ccb044ab --- /dev/null +++ b/test/integration/targets/vmware_tag/tasks/tag_manager_duplicate_tag_cat.yml @@ -0,0 +1,163 @@ +# Test code for the vmware_tag_manager +# Copyright: (c) 2019, Abhijeet Kasurde +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +- when: vcsim is not defined + block: + - name: Create first category + vmware_category: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + category_name: "{{ cat_one }}" + category_cardinality: 'multiple' + state: present + register: category_one_create + + - name: Create second category + vmware_category: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + category_name: "{{ cat_two }}" + category_cardinality: 'multiple' + state: present + register: category_two_create + + - name: Check categories are created + assert: + that: + - category_two_create.changed + - category_one_create.changed + + - name: Set category one id + set_fact: cat_one_id={{ category_one_create['category_results']['category_id'] }} + + - name: Set category two id + set_fact: cat_two_id={{ category_two_create['category_results']['category_id'] }} + + - name: Create duplicate tags in two different categories + vmware_tag: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + tag_name: "{{ tag_one }}" + category_id: "{{ cat_one_id }}" + state: present + register: tag_one_create + + - name: Check tag is created + assert: + that: + - tag_one_create.changed + + - name: Create duplicate tags in two different categories + vmware_tag: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + tag_name: "{{ tag_one }}" + category_id: "{{ cat_two_id }}" + state: present + register: tag_two_create + + - name: Check tag is created + assert: + that: + - tag_two_create.changed + + - name: Create duplicate tags in two different categories + vmware_tag: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + tag_name: "{{ tag_two }}" + category_id: "{{ cat_one_id }}" + state: present + register: tag_one_create + + - name: Check tag is created + assert: + that: + - tag_one_create.changed + + - name: Create duplicate tags in two different categories + vmware_tag: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + tag_name: "{{ tag_two }}" + category_id: "{{ cat_two_id }}" + state: present + register: tag_two_create + + - name: Check tag is created + assert: + that: + - tag_two_create.changed + + - name: Get VM Facts + vmware_vm_facts: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + register: vm_facts + + - set_fact: vm_name="{{ vm_facts['virtual_machines'][0]['guest_name'] }}" + + - name: Assign tags to given virtual machine + vmware_tag_manager: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + tag_names: + - "{{ cat_one }}:{{ tag_one }}" + object_name: "{{ vm_name }}" + object_type: VirtualMachine + state: add + delegate_to: localhost + register: vm_tag_info + + - name: Check if we assigned correct tags + assert: + that: + - vm_tag_info.changed + + - name: Delete Tags + vmware_tag: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + tag_name: "{{ item }}" + state: absent + register: delete_tag + with_items: + - "{{ tag_one }}" + - "{{ tag_two }}" + + - name: Delete Categories + vmware_category: + hostname: '{{ vcenter_hostname }}' + username: '{{ vcenter_username }}' + password: '{{ vcenter_password }}' + validate_certs: no + category_name: "{{ item }}" + state: absent + register: delete_categories + with_items: + - "{{ cat_one }}" + - "{{ cat_two }}" + vars: + cat_one: category_1001 + cat_two: category_1002 + tag_one: tag_1001 + tag_two: tag_1002