fix using inventory and cache plugins in a collection (#56469)

* Allow custom inventory plugins and cache plugins

If _load_name is not set correctly the cache plugin can't load the documentation (which is also the arg spec)

Fix the existing inventory plugin in the collections tests

Add integration tests for using a cache plugin in a collection

* Set the attribute on the instance instead of the class

Deprecate importing custom CacheModules directly - they should use the cache_loader

lib/ansible/plugins/cache/__init__.py

@@ -59,7 +59,10 @@ class BaseCacheModule(AnsiblePlugin):
     _display = display
 
     def __init__(self, *args, **kwargs):
-        self._load_name = self.__module__.split('.')[-1]
+        # Third party code is not using cache_loader to load plugin - fall back to previous behavior
+        if not hasattr(self, '_load_name'):
+            display.deprecated('Rather than importing custom CacheModules directly, use ansible.plugins.loader.cache_loader', version='2.14')
+            self._load_name = self.__module__.split('.')[-1]
         super(BaseCacheModule, self).__init__()
         self.set_options(var_options=args, direct=kwargs)
 

lib/ansible/plugins/loader.py

@@ -570,7 +570,12 @@ class PluginLoader:
 
         if not class_only:
             try:
-                obj = obj(*args, **kwargs)
+                # A plugin may need to use its _load_name in __init__ (for example, to set
+                # or get options from config), so update the object before using the constructor
+                instance = object.__new__(obj)
+                self._update_object(instance, name, path)
+                obj.__init__(instance, *args, **kwargs)
+                obj = instance
             except TypeError as e:
                 if "abstract" in e.args[0]:
                     # Abstract Base Class.  The found plugin file does not

test/integration/targets/collections/cache.statichost.yml

@@ -0,0 +1,6 @@
+# use inventory and cache plugins defined in a content-adjacent collection
+plugin: testns.content_adj.statichost
+hostname: cache_host_a
+cache_plugin: testns.content_adj.custom_jsonfile
+cache: yes
+cache_connection: inventory_cache

test/integration/targets/collections/check_populated_inventory.yml

@@ -0,0 +1,11 @@
+---
+- hosts: localhost
+  connection: local
+  gather_facts: no
+  tasks:
+  - assert:
+      that:
+        - "groups.all | length == 2"
+        - "groups.ungrouped == groups.all"
+        - "'cache_host_a' in groups.all"
+        - "'dynamic_host_a' in groups.all"

test/integration/targets/collections/collections/ansible_collections/testns/content_adj/plugins/cache/custom_jsonfile.py

@@ -0,0 +1,63 @@
+# (c) 2014, Brian Coca, Josh Drake, et al
+# (c) 2017 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+DOCUMENTATION = '''
+    cache: jsonfile
+    short_description: JSON formatted files.
+    description:
+        - This cache uses JSON formatted, per host, files saved to the filesystem.
+    version_added: "1.9"
+    author: Ansible Core (@ansible-core)
+    options:
+      _uri:
+        required: True
+        description:
+          - Path in which the cache plugin will save the JSON files
+        env:
+          - name: ANSIBLE_CACHE_PLUGIN_CONNECTION
+        ini:
+          - key: fact_caching_connection
+            section: defaults
+      _prefix:
+        description: User defined prefix to use when creating the JSON files
+        env:
+          - name: ANSIBLE_CACHE_PLUGIN_PREFIX
+        ini:
+          - key: fact_caching_prefix
+            section: defaults
+      _timeout:
+        default: 86400
+        description: Expiration timeout for the cache plugin data
+        env:
+          - name: ANSIBLE_CACHE_PLUGIN_TIMEOUT
+        ini:
+          - key: fact_caching_timeout
+            section: defaults
+        type: integer
+'''
+
+import codecs
+import json
+
+from ansible.parsing.ajson import AnsibleJSONEncoder, AnsibleJSONDecoder
+from ansible.plugins.cache import BaseFileCacheModule
+
+
+class CacheModule(BaseFileCacheModule):
+    """
+    A caching module backed by json files.
+    """
+
+    def _load(self, filepath):
+        # Valid JSON is always UTF-8 encoded.
+        with codecs.open(filepath, 'r', encoding='utf-8') as f:
+            return json.load(f, cls=AnsibleJSONDecoder)
+
+    def _dump(self, value, filepath):
+        with codecs.open(filepath, 'w', encoding='utf-8') as f:
+            f.write(json.dumps(value, cls=AnsibleJSONEncoder, sort_keys=True, indent=4))

test/integration/targets/collections/collections/ansible_collections/testns/content_adj/plugins/inventory/statichost.py

@@ -7,22 +7,25 @@ __metaclass__ = type
 DOCUMENTATION = '''
     inventory: statichost
     short_description: Add a single host
+    description: Add a single host
+    extends_documentation_fragment:
+      - inventory_cache
     options:
       plugin:
         description: plugin name (must be statichost)
         required: true
       hostname:
         description: Toggle display of stderr even when script was successful
-        type: list
+        required: True
 '''
 
 from ansible.errors import AnsibleParserError
 from ansible.plugins.inventory import BaseInventoryPlugin, Cacheable
 
 
-class InventoryModule(BaseInventoryPlugin):
+class InventoryModule(BaseInventoryPlugin, Cacheable):
 
-    NAME = 'statichost'
+    NAME = 'testns.content_adj.statichost'
 
     def __init__(self):
 
@@ -41,14 +44,24 @@ class InventoryModule(BaseInventoryPlugin):
 
         super(InventoryModule, self).parse(inventory, loader, path)
 
-        config_data = loader.load_from_file(path, cache=False)
+        # Initialize and validate options
-        host_to_add = config_data.get('hostname')
+        self._read_config_data(path)
-
+
-        if not host_to_add:
+        # Exercise cache
-            raise AnsibleParserError("hostname was not specified")
+        cache_key = self.get_cache_key(path)
+        attempt_to_read_cache = self.get_option('cache') and cache
+        cache_needs_update = self.get_option('cache') and not cache
+        if attempt_to_read_cache:
+            try:
+                host_to_add = self._cache[cache_key]
+            except KeyError:
+                cache_needs_update = True
+        if not attempt_to_read_cache or cache_needs_update:
+            host_to_add = self.get_option('hostname')
 
         # this is where the magic happens
         self.inventory.add_host(host_to_add, 'all')
+        self._cache[cache_key] = host_to_add
 
         # self.inventory.add_group()...
         # self.inventory.add_child()...

test/integration/targets/collections/runme.sh

@@ -47,4 +47,27 @@ fi
 export ANSIBLE_COLLECTIONS_PATHS=''
 ANSIBLE_INVENTORY_ANY_UNPARSED_IS_FAILED=1 ansible-inventory -i a.statichost.yml --list --export --playbook-dir=. -v "$@"
 
+# use an inventory source with caching enabled
+ansible-playbook -i a.statichost.yml -i ./cache.statichost.yml -v check_populated_inventory.yml
+
+# Check that the inventory source with caching enabled was stored
+if [[ "$(find ./inventory_cache -type f ! -path "./inventory_cache/.keep" | wc -l)" -ne "1" ]]; then
+    echo "Failed to find the expected single cache"
+    exit 1
+fi
+
+CACHEFILE="$(find ./inventory_cache -type f ! -path './inventory_cache/.keep')"
+
+# Check the cache for the expected hosts
+
+if [[ "$(grep -wc "cache_host_a" "$CACHEFILE")" -ne "1" ]]; then
+    echo "Failed to cache host as expected"
+    exit 1
+fi
+
+if [[ "$(grep -wc "dynamic_host_a" "$CACHEFILE")" -ne "0" ]]; then
+    echo "Cached an incorrect source"
+    exit 1
+fi
+
 ./vars_plugin_tests.sh