Repository: saltstack/salt
Pull request: Fixes to pillar cache when backend is memory
### What does this PR do?
Log a different obeject when debugging if we're using disk cache vs memory cache.  The disk cache pillar class has the _dict_ object but the cache pillar object which is used with the memory cache does not include a _dict obeject because it is a dict already.  Adding a test when pillar_cache_backend is memory.

### What issues does this PR fix or reference?
Fixes: #58838 

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltstack.com/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [ ] Docs
- [x] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes/No

Please review [Salt's Contributing Guide](https://docs.saltstack.com/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
[BUG] AttributeError: 'CacheDict' object has no attribute '_dict' when memory used as pillar_cache_backend
**Description**
`pillar_cache_backed: memory` is not experimental it is just broken. pillar makes log.debug calls to `self.cache._dict` however the CacheDict object does not have a _dict Attribute. while the CacheDisk object does. this completely breaks pillar if you try setting the pillar_cache backend to memory. 

**Setup**
set up pillar cache with `pillar_cache_backend: memory` refresh the minions cache and very the masters log and also see there are no pillars on the minions. 

**Steps to Reproduce the behavior**

See setup steps above and look in the logs for the following error and see that no minions have pillar. 

```
2020-10-29 08:40:17,845 [salt.master      :2019][ERROR   ][7443] Error in function _pillar:
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/master.py", line 2013, in run_func
    ret = getattr(self, func)(load)
  File "/usr/lib/python3.6/site-packages/salt/master.py", line 1710, in _pillar
    data = pillar.compile_pillar()
  File "/usr/lib/python3.6/site-packages/salt/pillar/__init__.py", line 445, in compile_pillar
    log.debug("Scanning cache: %s", self.cache._dict)
AttributeError: 'CacheDict' object has no attribute '_dict'
```


**Expected behavior**
Even if experimental the basic functionality should at least keep a catastrophic failing of pillar rendering from happening. 


**Versions Report**
<details><summary>salt --versions-report</summary>
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) 

```
Salt Version:
           Salt: 3002

Dependency Versions:
           cffi: 1.14.0
       cherrypy: unknown
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: 0.26.6
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 1.0.0
   mysql-python: Not Installed
      pycparser: 2.20
       pycrypto: Not Installed
   pycryptodome: 3.9.8
         pygit2: 0.26.4
         Python: 3.6.8 (default, Apr  2 2020, 13:34:55)
   python-gnupg: 2.3.1
         PyYAML: 3.13
          PyZMQ: 17.0.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4

System Versions:
           dist: centos 7 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1127.13.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7 Core

```
</details>

Although this issue goes back to at least 2018.3.5 maybe as far back as when the option to include `pillar_cache_backend` was allowed.

Hints from issue discussion:
zd-5931
It looks like the `CacheDisk` class has a `_dict` attribute, but the CacheDict one does not, since it is itself a Dict.  It's likely that the log messages were added to handle when the cache option was to use disk instead of memory.

Commit message: Adding changelog.

Patch to review:
diff --git a/changelog/58861.fixed b/changelog/58861.fixed
new file mode 100644
index 000000000000..4655432225c8
--- /dev/null
+++ b/changelog/58861.fixed
@@ -0,0 +1 @@
+Log a different obeject when debugging if we're using disk cache vs memory cache. The disk cache pillar class has the dict object but the cache pillar object which is used with the memory cache does not include a _dict obeject because it is a dict already.
diff --git a/salt/pillar/__init__.py b/salt/pillar/__init__.py
index 3dba4ddc1797..57f4b4d36757 100644
--- a/salt/pillar/__init__.py
+++ b/salt/pillar/__init__.py
@@ -2,7 +2,6 @@
 Render the pillar data
 """
 
-# Import python libs
 
 import collections
 import copy
@@ -15,8 +14,6 @@
 
 import salt.ext.tornado.gen
 import salt.fileclient
-
-# Import salt libs
 import salt.loader
 import salt.minion
 import salt.transport.client
@@ -27,8 +24,6 @@
 import salt.utils.dictupdate
 import salt.utils.url
 from salt.exceptions import SaltClientError
-
-# Import 3rd-party libs
 from salt.ext import six
 from salt.template import compile_template
 
@@ -442,7 +437,12 @@ def compile_pillar(self, *args, **kwargs):  # Will likely just be pillar_dirs
             self.minion_id,
             self.pillarenv,
         )
-        log.debug("Scanning cache: %s", self.cache._dict)
+        if self.opts["pillar_cache_backend"] == "memory":
+            cache_dict = self.cache
+        else:
+            cache_dict = self.cache._dict
+
+        log.debug("Scanning cache: %s", cache_dict)
         # Check the cache!
         if self.minion_id in self.cache:  # Keyed by minion_id
             # TODO Compare grains, etc?
@@ -473,7 +473,7 @@ def compile_pillar(self, *args, **kwargs):  # Will likely just be pillar_dirs
             fresh_pillar = self.fetch_pillar()
             self.cache[self.minion_id] = {self.pillarenv: fresh_pillar}
             log.debug("Pillar cache miss for minion %s", self.minion_id)
-            log.debug("Current pillar cache: %s", self.cache._dict)  # FIXME hack!
+            log.debug("Current pillar cache: %s", cache_dict)  # FIXME hack!
             return fresh_pillar
 
 
diff --git a/tests/unit/test_pillar.py b/tests/unit/test_pillar.py
index a203afc9ea04..59202fb73282 100644
--- a/tests/unit/test_pillar.py
+++ b/tests/unit/test_pillar.py
@@ -6,7 +6,6 @@
     ~~~~~~~~~~~~~~~~~~~~~~
 """
 
-# Import python libs
 
 import logging
 import os
@@ -14,7 +13,6 @@
 import tempfile
 import textwrap
 
-# Import salt libs
 import salt.config
 import salt.exceptions
 import salt.fileclient
@@ -22,8 +20,6 @@
 from salt.utils.files import fopen
 from tests.support.helpers import with_tempdir
 from tests.support.mock import MagicMock, patch
-
-# Import Salt Testing libs
 from tests.support.runtests import RUNTIME_VARS
 from tests.support.unit import TestCase
 
@@ -1332,7 +1328,7 @@ def tearDown(self):
             except AttributeError:
                 continue
 
-    def test_compile_pillar(self):
+    def test_compile_pillar_disk_cache(self):
         self.mock_master_default_opts.update(
             {"pillar_cache_backend": "disk", "pillar_cache_ttl": 3600}
         )
@@ -1376,3 +1372,47 @@ def test_compile_pillar(self):
                     "mocked_minion": {"base": {"foo": "bar"}, "dev": {"foo": "baz"}}
                 }
                 self.assertEqual(pillar.cache._dict, expected_cache)
+
+    def test_compile_pillar_memory_cache(self):
+        self.mock_master_default_opts.update(
+            {"pillar_cache_backend": "memory", "pillar_cache_ttl": 3600}
+        )
+
+        pillar = salt.pillar.PillarCache(
+            self.mock_master_default_opts,
+            self.grains,
+            "mocked_minion",
+            "fake_env",
+            pillarenv="base",
+        )
+
+        with patch(
+            "salt.pillar.PillarCache.fetch_pillar",
+            side_effect=[{"foo": "bar"}, {"foo": "baz"}],
+        ):
+            # Run once for pillarenv base
+            ret = pillar.compile_pillar()
+            expected_cache = {"base": {"foo": "bar"}}
+            self.assertIn("mocked_minion", pillar.cache)
+            self.assertEqual(pillar.cache["mocked_minion"], expected_cache)
+
+            # Run a second time for pillarenv base
+            ret = pillar.compile_pillar()
+            expected_cache = {"base": {"foo": "bar"}}
+            self.assertIn("mocked_minion", pillar.cache)
+            self.assertEqual(pillar.cache["mocked_minion"], expected_cache)
+
+            # Change the pillarenv
+            pillar.pillarenv = "dev"
+
+            # Run once for pillarenv dev
+            ret = pillar.compile_pillar()
+            expected_cache = {"base": {"foo": "bar"}, "dev": {"foo": "baz"}}
+            self.assertIn("mocked_minion", pillar.cache)
+            self.assertEqual(pillar.cache["mocked_minion"], expected_cache)
+
+            # Run a second time for pillarenv dev
+            ret = pillar.compile_pillar()
+            expected_cache = {"base": {"foo": "bar"}, "dev": {"foo": "baz"}}
+            self.assertIn("mocked_minion", pillar.cache)
+            self.assertEqual(pillar.cache["mocked_minion"], expected_cache)


Code review comments:
diff_hunk: @@ -0,0 +1 @@
+Log a different obeject when debugging if we're using disk cache vs memory cache. The disk cache pillar class has the dict object but the cache pillar object which is used with the memory cache does not include a _dict obeject because it is a dict already.
path: changelog/58861.fixed
text: @user1:
```suggestion
Log a different object when debugging if we're using disk cache vs memory cache. The disk cache pillar class has the dict object but the cache pillar object which is used with the memory cache does not include a _dict obeject because it is a dict already.
```