VMware: Refactor guest inventory plugin (#52642)

* VMware: Refactor guest inventory plugin
* Review comments

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

changelogs/fragments/52381-vmware_vm_inventory-fix_yaml_flag.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- Fixed issue related to --yaml flag in vmware_vm_inventory. Also fixed caching issue in vmware_vm_inventory (https://github.com/ansible/ansible/issues/52381).

lib/ansible/plugins/inventory/vmware_vm_inventory.py

@@ -11,7 +11,7 @@ DOCUMENTATION = '''
     name: vmware_vm_inventory
     plugin_type: inventory
     short_description: VMware Guest inventory source
-    version_added: "2.6"
+    version_added: "2.7"
     author:
       - Abhijeet Kasurde (@Akasurde)
     description:
@@ -112,35 +112,25 @@ except ImportError:
 from ansible.plugins.inventory import BaseInventoryPlugin, Cacheable
 
 
-class InventoryModule(BaseInventoryPlugin, Cacheable):
+class BaseVMwareInventory:
+    def __init__(self, hostname, username, password, port, validate_certs, with_tags):
+        self.hostname = hostname
+        self.username = username
+        self.password = password
+        self.port = port
+        self.with_tags = with_tags
+        self.validate_certs = validate_certs
+        self.content = None
+        self.rest_content = None
 
-    NAME = 'vmware_vm_inventory'
+    def do_login(self):
-
-    def _set_credentials(self):
         """
-        Set credentials
+        Check requirements and do login
         """
-        self.hostname = self.get_option('hostname')
+        self.check_requirements()
-        self.username = self.get_option('username')
+        self.content = self._login()
-        self.password = self.get_option('password')
+        if self.with_tags:
-        self.port = self.get_option('port')
+            self.rest_content = self._login_vapi()
-        self.with_tags = self.get_option('with_tags')
-
-        self.validate_certs = self.get_option('validate_certs')
-
-        if not HAS_VSPHERE and self.with_tags:
-            raise AnsibleError("Unable to find 'vSphere Automation SDK' Python library which is required."
-                               " Please refer this URL for installation steps"
-                               " - https://code.vmware.com/web/sdk/65/vsphere-automation-python")
-
-        if not HAS_VCLOUD and self.with_tags:
-            raise AnsibleError("Unable to find 'vCloud Suite SDK' Python library which is required."
-                               " Please refer this URL for installation steps"
-                               " - https://code.vmware.com/web/sdk/60/vcloudsuite-python")
-
-        if not all([self.hostname, self.username, self.password]):
-            raise AnsibleError("Missing one of the following : hostname, username, password. Please read "
-                               "the documentation for more information.")
 
     def _login_vapi(self):
         """
@@ -217,27 +207,8 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
         atexit.register(connect.Disconnect, service_instance)
         return service_instance.RetrieveContent()
 
-    def verify_file(self, path):
+    def check_requirements(self):
-        """
+        """ Check all requirements for this inventory are satisified"""
-        Verify plugin configuration file and mark this plugin active
-        Args:
-            path: Path of configuration YAML file
-
-        Returns: True if everything is correct, else False
-
-        """
-        valid = False
-        if super(InventoryModule, self).verify_file(path):
-            if path.endswith(('vmware.yaml', 'vmware.yml')):
-                valid = True
-
-        return valid
-
-    def parse(self, inventory, loader, path, cache=True):
-        """
-        Parses the inventory file
-        """
-
         if not HAS_REQUESTS:
             raise AnsibleParserError('Please install "requests" Python module as this is required'
                                      ' for VMware Guest dynamic inventory plugin.')
@@ -259,150 +230,19 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
                                          " be >= %s, found: %s." % (".".join([str(w) for w in required_version]),
                                                                     requests.__version__))
 
-        super(InventoryModule, self).parse(inventory, loader, path, cache=cache)
+        if not HAS_VSPHERE and self.with_tags:
+            raise AnsibleError("Unable to find 'vSphere Automation SDK' Python library which is required."
+                               " Please refer this URL for installation steps"
+                               " - https://code.vmware.com/web/sdk/65/vsphere-automation-python")
 
-        cache_key = self.get_cache_key(path)
+        if not HAS_VCLOUD and self.with_tags:
+            raise AnsibleError("Unable to find 'vCloud Suite SDK' Python library which is required."
+                               " Please refer this URL for installation steps"
+                               " - https://code.vmware.com/web/sdk/60/vcloudsuite-python")
 
-        config_data = self._read_config_data(path)
+        if not all([self.hostname, self.username, self.password]):
-
+            raise AnsibleError("Missing one of the following : hostname, username, password. Please read "
-        source_data = None
+                               "the documentation for more information.")
-        if cache:
-            cache = self.get_option('cache')
-
-        update_cache = False
-        if cache:
-            try:
-                source_data = self.cache.get(cache_key)
-            except KeyError:
-                update_cache = True
-
-        # set _options from config data
-        self._consume_options(config_data)
-
-        self._set_credentials()
-        self.content = self._login()
-        if self.with_tags:
-            self.rest_content = self._login_vapi()
-
-        using_current_cache = cache and not update_cache
-        cacheable_results = self._populate_from_source(source_data, using_current_cache)
-
-        if update_cache:
-            self.cache.set(cache_key, cacheable_results)
-
-    def _populate_from_cache(self, source_data):
-        """
-        Populate inventory from cache
-        """
-        hostvars = source_data.pop('_meta', {}).get('hostvars', {})
-        for group in source_data:
-            if group == 'all':
-                continue
-            else:
-                self.inventory.add_group(group)
-                self.inventory.add_child('all', group)
-        if not source_data:
-            for host in hostvars:
-                self.inventory.add_host(host)
-
-    @staticmethod
-    def _get_vm_prop(vm, attributes):
-        """Safely get a property or return None"""
-        result = vm
-        for attribute in attributes:
-            try:
-                result = getattr(result, attribute)
-            except (AttributeError, IndexError):
-                return None
-        return result
-
-    def _populate_from_source(self, source_data, using_current_cache):
-        """
-        Populate inventory data from direct source
-
-        """
-        if using_current_cache:
-            self._populate_from_cache(source_data)
-            return source_data
-
-        cacheable_results = {}
-        hostvars = {}
-        objects = self._get_managed_objects_properties(vim_type=vim.VirtualMachine, properties=['name'])
-
-        if self.with_tags:
-            tag_svc = Tag(self.rest_content)
-            tag_association = TagAssociation(self.rest_content)
-
-            tags_info = dict()
-            tags = tag_svc.list()
-            for tag in tags:
-                tag_obj = tag_svc.get(tag)
-                tags_info[tag_obj.id] = tag_obj.name
-                if tag_obj.name not in cacheable_results:
-                    cacheable_results[tag_obj.name] = {'hosts': []}
-                    self.inventory.add_group(tag_obj.name)
-
-        for temp_vm_object in objects:
-            for temp_vm_object_property in temp_vm_object.propSet:
-                # VMware does not provide a way to uniquely identify VM by its name
-                # i.e. there can be two virtual machines with same name
-                # Appending "_" and VMware UUID to make it unique
-                current_host = temp_vm_object_property.val + "_" + temp_vm_object.obj.config.uuid
-
-                if current_host not in hostvars:
-                    hostvars[current_host] = {}
-                    self.inventory.add_host(current_host)
-                    host_ip = temp_vm_object.obj.guest.ipAddress
-                    if host_ip:
-                        self.inventory.set_variable(current_host, 'ansible_host', host_ip)
-
-                    # Load VM properties in host_vars
-                    vm_properties = [
-                        'name',
-                        'config.cpuHotAddEnabled',
-                        'config.cpuHotRemoveEnabled',
-                        'config.instanceUuid',
-                        'config.hardware.numCPU',
-                        'config.template',
-                        'config.name',
-                        'guest.hostName',
-                        'guest.ipAddress',
-                        'guest.guestId',
-                        'guest.guestState',
-                        'runtime.maxMemoryUsage',
-                        'customValue',
-                    ]
-                    for vm_prop in vm_properties:
-                        vm_value = self._get_vm_prop(temp_vm_object.obj, vm_prop.split("."))
-                        self.inventory.set_variable(current_host, vm_prop, vm_value)
-                    # Only gather facts related to tag if vCloud and vSphere is installed.
-                    if HAS_VCLOUD and HAS_VSPHERE and self.with_tags:
-                        # Add virtual machine to appropriate tag group
-                        vm_mo_id = temp_vm_object.obj._GetMoId()
-                        vm_dynamic_id = DynamicID(type='VirtualMachine', id=vm_mo_id)
-                        attached_tags = tag_association.list_attached_tags(vm_dynamic_id)
-
-                        for tag_id in attached_tags:
-                            self.inventory.add_child(tags_info[tag_id], current_host)
-                            cacheable_results[tags_info[tag_id]]['hosts'].append(current_host)
-
-                    # Based on power state of virtual machine
-                    vm_power = temp_vm_object.obj.summary.runtime.powerState
-                    if vm_power not in cacheable_results:
-                        cacheable_results[vm_power] = []
-                        self.inventory.add_group(vm_power)
-                    cacheable_results[vm_power].append(current_host)
-                    self.inventory.add_child(vm_power, current_host)
-
-                    # Based on guest id
-                    vm_guest_id = temp_vm_object.obj.config.guestId
-                    if vm_guest_id and vm_guest_id not in cacheable_results:
-                        cacheable_results[vm_guest_id] = []
-                        self.inventory.add_group(vm_guest_id)
-                    cacheable_results[vm_guest_id].append(current_host)
-                    self.inventory.add_child(vm_guest_id, current_host)
-
-        return cacheable_results
 
     def _get_managed_objects_properties(self, vim_type, properties=None):
         """
@@ -450,3 +290,197 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
         )
 
         return self.content.propertyCollector.RetrieveContents([filter_spec])
+
+    @staticmethod
+    def _get_object_prop(vm, attributes):
+        """Safely get a property or return None"""
+        result = vm
+        for attribute in attributes:
+            try:
+                result = getattr(result, attribute)
+            except (AttributeError, IndexError):
+                return None
+        return result
+
+
+class InventoryModule(BaseInventoryPlugin, Cacheable):
+
+    NAME = 'vmware_vm_inventory'
+
+    def verify_file(self, path):
+        """
+        Verify plugin configuration file and mark this plugin active
+        Args:
+            path: Path of configuration YAML file
+
+        Returns: True if everything is correct, else False
+
+        """
+        valid = False
+        if super(InventoryModule, self).verify_file(path):
+            if path.endswith(('vmware.yaml', 'vmware.yml')):
+                valid = True
+
+        return valid
+
+    def parse(self, inventory, loader, path, cache=True):
+        """
+        Parses the inventory file
+        """
+        super(InventoryModule, self).parse(inventory, loader, path, cache=cache)
+
+        cache_key = self.get_cache_key(path)
+
+        config_data = self._read_config_data(path)
+
+        # set _options from config data
+        self._consume_options(config_data)
+
+        self.pyv = BaseVMwareInventory(
+            hostname=self.get_option('hostname'),
+            username=self.get_option('username'),
+            password=self.get_option('password'),
+            port=self.get_option('port'),
+            with_tags=self.get_option('with_tags'),
+            validate_certs=self.get_option('validate_certs')
+        )
+
+        self.pyv.do_login()
+
+        self.pyv.check_requirements()
+
+        source_data = None
+        if cache:
+            cache = self.get_option('cache')
+
+        update_cache = False
+        if cache:
+            try:
+                source_data = self.cache.get(cache_key)
+            except KeyError:
+                update_cache = True
+
+        using_current_cache = cache and not update_cache
+        cacheable_results = self._populate_from_source(source_data, using_current_cache)
+
+        if update_cache:
+            self.cache.set(cache_key, cacheable_results)
+
+    def _populate_from_cache(self, source_data):
+        """ Populate cache using source data """
+        hostvars = source_data.pop('_meta', {}).get('hostvars', {})
+        for group in source_data:
+            if group == 'all':
+                continue
+            else:
+                self.inventory.add_group(group)
+                hosts = source_data[group].get('hosts', [])
+                for host in hosts:
+                    self._populate_host_vars([host], hostvars.get(host, {}), group)
+                self.inventory.add_child('all', group)
+
+    def _populate_from_source(self, source_data, using_current_cache):
+        """
+        Populate inventory data from direct source
+
+        """
+        if using_current_cache:
+            self._populate_from_cache(source_data)
+            return source_data
+
+        cacheable_results = {'_meta': {'hostvars': {}}}
+        hostvars = {}
+        objects = self.pyv._get_managed_objects_properties(vim_type=vim.VirtualMachine,
+                                                           properties=['name'])
+
+        if self.pyv.with_tags:
+            tag_svc = Tag(self.pyv.rest_content)
+            tag_association = TagAssociation(self.pyv.rest_content)
+
+            tags_info = dict()
+            tags = tag_svc.list()
+            for tag in tags:
+                tag_obj = tag_svc.get(tag)
+                tags_info[tag_obj.id] = tag_obj.name
+                if tag_obj.name not in cacheable_results:
+                    cacheable_results[tag_obj.name] = {'hosts': []}
+                    self.inventory.add_group(tag_obj.name)
+
+        for vm_obj in objects:
+            for vm_obj_property in vm_obj.propSet:
+                # VMware does not provide a way to uniquely identify VM by its name
+                # i.e. there can be two virtual machines with same name
+                # Appending "_" and VMware UUID to make it unique
+                current_host = vm_obj_property.val + "_" + vm_obj.obj.config.uuid
+
+                if current_host not in hostvars:
+                    hostvars[current_host] = {}
+                    self.inventory.add_host(current_host)
+
+                    host_ip = vm_obj.obj.guest.ipAddress
+                    if host_ip:
+                        self.inventory.set_variable(current_host, 'ansible_host', host_ip)
+
+                    self._populate_host_properties(vm_obj, current_host)
+
+                    # Only gather facts related to tag if vCloud and vSphere is installed.
+                    if HAS_VCLOUD and HAS_VSPHERE and self.pyv.with_tags:
+                        # Add virtual machine to appropriate tag group
+                        vm_mo_id = vm_obj.obj._GetMoId()
+                        vm_dynamic_id = DynamicID(type='VirtualMachine', id=vm_mo_id)
+                        attached_tags = tag_association.list_attached_tags(vm_dynamic_id)
+
+                        for tag_id in attached_tags:
+                            self.inventory.add_child(tags_info[tag_id], current_host)
+                            cacheable_results[tags_info[tag_id]]['hosts'].append(current_host)
+
+                    # Based on power state of virtual machine
+                    vm_power = str(vm_obj.obj.summary.runtime.powerState)
+                    if vm_power not in cacheable_results:
+                        cacheable_results[vm_power] = {'hosts': []}
+                        self.inventory.add_group(vm_power)
+                    cacheable_results[vm_power]['hosts'].append(current_host)
+                    self.inventory.add_child(vm_power, current_host)
+
+                    # Based on guest id
+                    vm_guest_id = vm_obj.obj.config.guestId
+                    if vm_guest_id and vm_guest_id not in cacheable_results:
+                        cacheable_results[vm_guest_id] = {'hosts': []}
+                        self.inventory.add_group(vm_guest_id)
+                    cacheable_results[vm_guest_id]['hosts'].append(current_host)
+                    self.inventory.add_child(vm_guest_id, current_host)
+
+        for host in hostvars:
+            h = self.inventory.get_host(host)
+            cacheable_results['_meta']['hostvars'][h.name] = h.vars
+
+        return cacheable_results
+
+    def _populate_host_properties(self, vm_obj, current_host):
+        # Load VM properties in host_vars
+        vm_properties = [
+            'name',
+            'config.cpuHotAddEnabled',
+            'config.cpuHotRemoveEnabled',
+            'config.instanceUuid',
+            'config.hardware.numCPU',
+            'config.template',
+            'config.name',
+            'guest.hostName',
+            'guest.ipAddress',
+            'guest.guestId',
+            'guest.guestState',
+            'runtime.maxMemoryUsage',
+            'customValue',
+        ]
+        field_mgr = self.pyv.content.customFieldsManager.field
+
+        for vm_prop in vm_properties:
+            if vm_prop == 'customValue':
+                for cust_value in vm_obj.obj.customValue:
+                    self.inventory.set_variable(current_host,
+                                                [y.name for y in field_mgr if y.key == cust_value.key][0],
+                                                cust_value.value)
+            else:
+                vm_value = self.pyv._get_object_prop(vm_obj.obj, vm_prop.split("."))
+                self.inventory.set_variable(current_host, vm_prop, vm_value)

test/integration/targets/inventory_vmware_vm_inventory/ansible.cfg

@@ -3,3 +3,6 @@ inventory = test-config.vmware.yaml
 
 [inventory]
 enable_plugins = vmware_vm_inventory
+cache = True
+cache_plugin = jsonfile
+cache_connection = inventory_cache

test/integration/targets/inventory_vmware_vm_inventory/runme.sh

@@ -4,6 +4,9 @@
 
 set -euo pipefail
 
+# Required to differentiate between Python 2 and 3 environ
+PYTHON=${ANSIBLE_TEST_PYTHON_INTERPRETER:-python}
+
 export ANSIBLE_CONFIG=ansible.cfg
 export vcenter_host="${vcenter_host:-0.0.0.0}"
 export VMWARE_SERVER="${vcenter_host}"
@@ -18,7 +21,20 @@ validate_certs: False
 with_tags: False
 VMWARE_YAML
 
-trap 'rm -f "${VMWARE_CONFIG}"' INT TERM EXIT
+cleanup() {
+    echo "Cleanup"
+    if [ -f "${VMWARE_CONFIG}" ]; then
+        rm -f "${VMWARE_CONFIG}"
+    fi
+    if [ -d "$(pwd)/inventory_cache" ]; then
+        echo "Removing $(pwd)/inventory_cache"
+        rm -rf "$(pwd)/inventory_cache"
+    fi
+    echo "Done"
+    exit 0
+}
+
+trap cleanup INT TERM EXIT
 
 echo "DEBUG: Using ${vcenter_host} with username ${VMWARE_USERNAME} and password ${VMWARE_PASSWORD}"
 
@@ -33,5 +49,23 @@ curl "http://${vcenter_host}:5000/govc_find"
 
 # Get inventory
 ansible-inventory -i ${VMWARE_CONFIG} --list
+
+# Get inventory using YAML
+ansible-inventory -i ${VMWARE_CONFIG} --list --yaml
+
+# Install TOML for --toml
+${PYTHON} -m pip install toml
+
+# Get inventory using TOML
+ansible-inventory -i ${VMWARE_CONFIG} --list --toml
+
+echo "Check if cache is working for inventory plugin"
+ls "$(pwd)/inventory_cache/vmware_vm_*" > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+    echo "Cache directory not found. Please debug"
+    exit 1
+fi
+echo "Cache is working"
+
 # Test playbook with given inventory
 ansible-playbook -i ${VMWARE_CONFIG} test_vmware_vm_inventory.yml --connection=local "$@"