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 <akasurde@redhat.com>

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

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)

lib/ansible/modules/cloud/vmware/vmware_tag.py

@@ -230,6 +230,12 @@ class VmwareTag(VmwareRestClient):
         Returns: 'present' if tag found, 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
 
@@ -240,7 +246,8 @@ 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,
+            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

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)
 

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

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 <akasurde@redhat.com>
+# 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