Don't cache the file client at the class level

Signed-off-by: Pedro Algarvio <[email protected]>

salt/utils/jinja.py

@@ -58,19 +58,6 @@ class SaltCacheLoader(BaseLoader):
     and only loaded once per loader instance.
     """
 
-    _cached_pillar_client = None
-    _cached_client = None
-
-    @classmethod
-    def shutdown(cls):
-        for attr in ("_cached_client", "_cached_pillar_client"):
-            client = getattr(cls, attr, None)
-            if client is not None:
-                # PillarClient and LocalClient objects do not have a destroy method
-                if hasattr(client, "destroy"):
-                    client.destroy()
-                setattr(cls, attr, None)
-
     def __init__(
         self,
         opts,
@@ -93,8 +80,7 @@ def __init__(
         log.debug("Jinja search path: %s", self.searchpath)
         self.cached = []
         self._file_client = _file_client
-        # Instantiate the fileclient
-        self.file_client()
+        self._close_file_client = _file_client is None
 
     def file_client(self):
         """
@@ -108,18 +94,9 @@ def file_client(self):
             or not hasattr(self._file_client, "opts")
             or self._file_client.opts["file_roots"] != self.opts["file_roots"]
         ):
-            attr = "_cached_pillar_client" if self.pillar_rend else "_cached_client"
-            cached_client = getattr(self, attr, None)
-            if (
-                cached_client is None
-                or not hasattr(cached_client, "opts")
-                or cached_client.opts["file_roots"] != self.opts["file_roots"]
-            ):
-                cached_client = salt.fileclient.get_file_client(
-                    self.opts, self.pillar_rend
-                )
-                setattr(SaltCacheLoader, attr, cached_client)
-            self._file_client = cached_client
+            self._file_client = salt.fileclient.get_file_client(
+                self.opts, self.pillar_rend
+            )
         return self._file_client
 
     def cache_file(self, template):
@@ -222,15 +199,18 @@ def uptodate():
         raise TemplateNotFound(template)
 
     def destroy(self):
-        for attr in ("_cached_client", "_cached_pillar_client"):
-            client = getattr(self, attr, None)
-            if client is not None:
-                try:
-                    client.destroy()
-                except AttributeError:
-                    # PillarClient and LocalClient objects do not have a destroy method
-                    pass
-                setattr(self, attr, None)
+        if self._close_file_client is False:
+            return
+        if self._file_client is None:
+            return
+        file_client = self._file_client
+        self._file_client = None
+
+        try:
+            file_client.destroy()
+        except AttributeError:
+            # PillarClient and LocalClient objects do not have a destroy method
+            pass
 
     def __enter__(self):
         self.file_client()

tests/pytests/integration/states/test_include.py

@@ -10,6 +10,8 @@
 
 @pytest.mark.slow_test
 def test_issue_64111(salt_master, salt_minion, salt_call_cli):
+    # This needs to be an integration test. A functional test does not trigger
+    # the issue fixed.
 
     macros_jinja = """
     {% macro a_jinja_macro(arg) -%}

tests/pytests/unit/utils/jinja/test_salt_cache_loader.py

@@ -222,16 +222,3 @@ def test_file_client_kwarg(minion_opts, mock_file_client):
     mock_file_client.opts = minion_opts
     loader = SaltCacheLoader(minion_opts, _file_client=mock_file_client)
     assert loader._file_client is mock_file_client
-
-
-def test_cache_loader_shutdown(minion_opts, mock_file_client):
-    """
-    The shudown method can be called without raising an exception when the
-    file_client does not have a destroy method
-    """
-    assert not hasattr(mock_file_client, "destroy")
-    mock_file_client.opts = minion_opts
-    loader = SaltCacheLoader(minion_opts, _file_client=mock_file_client)
-    assert loader._file_client is mock_file_client
-    # Shutdown method should not raise any exceptions
-    loader.shutdown()