Fix(lib.cache): apply fallback cache fix, fix tests (ansible#656)

Apply fix for Fallback cache premature file creation.

AAP-36305

Author: BrennanPaciorek

ansible_base/lib/cache/fallback_cache.py

@@ -17,14 +17,6 @@
 PRIMARY_CACHE = 'primary'
 FALLBACK_CACHE = 'fallback'
 
-_temp_path = get_setting('ANSIBLE_BASE_FALLBACK_CACHE_FILE_PATH', tempfile.gettempdir())
-_temp_file = Path().joinpath(_temp_path, 'gw_primary_cache_failed')
-
-
-def create_temp_file():
-    _temp_file.touch()
-    _temp_file.chmod(mode=0o660)
-
 
 class DABCacheWithFallback(BaseCache):
     _instance = None
@@ -44,15 +36,17 @@ def __init__(self, location, params):
 
         self._primary_cache = django_cache.caches.create_connection(PRIMARY_CACHE)
         self._fallback_cache = django_cache.caches.create_connection(FALLBACK_CACHE)
+        self._temp_path = get_setting('ANSIBLE_BASE_FALLBACK_CACHE_FILE_PATH', tempfile.gettempdir())
+        self._temp_file = Path().joinpath(self._temp_path, 'gw_primary_cache_failed')
         self.thread_pool = ThreadPoolExecutor()
 
-        if _temp_file.exists():
-            _temp_file.unlink()
+        if self._temp_file.exists():
+            self._temp_file.unlink()
 
         self.__initialized = True
 
     def get_active_cache(self):
-        return FALLBACK_CACHE if _temp_file.exists() else PRIMARY_CACHE
+        return FALLBACK_CACHE if self._temp_file.exists() else PRIMARY_CACHE
 
     # Main cache interface
     def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
@@ -71,7 +65,7 @@ def clear(self):
         return self._op_with_fallback("clear")
 
     def _op_with_fallback(self, operation, *args, **kwargs):
-        if _temp_file.exists():
+        if self._temp_file.exists():
             response = getattr(self._fallback_cache, operation)(*args, **kwargs)
             self.start_recovery()
         else:
@@ -83,9 +77,10 @@ def _op_with_fallback(self, operation, *args, **kwargs):
                     # Attempt to ensure one thread/process goes first
                     # dynamic settings especially are read in a batch very quickly
                     time.sleep(random.uniform(10, 100) / 100.0)
-                    if not _temp_file.exists():
+                    if not self._temp_file.exists():
                         logger.error("Primary cache unavailable, switching to fallback cache.")
-                    create_temp_file()
+                    self.create_temp_file()
+                    # _temp_file.touch()
                 response = getattr(self._fallback_cache, operation)(*args, **kwargs)
 
         return response
@@ -104,14 +99,18 @@ def check_primary_cache(self):
         try:
             self._primary_cache.get('up_test')
             with multiprocessing.Lock():
-                if _temp_file.exists():
+                if self._temp_file.exists():
                     logger.warning("Primary cache recovered, clearing and resuming use.")
                     # Clear the primary cache
                     self._primary_cache.clear()
                     # Clear the backup cache just incase we need to fall back again (don't want it out of sync)
                     self._fallback_cache.clear()
-                    _temp_file.unlink()
+                    self._temp_file.unlink()
         except Exception:
             pass
         finally:
             self._fallback_cache.delete('RECOVERY_IN_PROGRESS')
+
+    def create_temp_file(self):
+        self._temp_file.touch()
+        self._temp_file.chmod(mode=0o660)

test_app/tests/lib/cache/test_fallback_cache.py

@@ -142,8 +142,9 @@ def test_dead_primary():
 
 @override_settings(CACHES=cache_settings)
 def test_ensure_temp_file_is_removed_on_init():
-    temp_file = Path(tempfile.NamedTemporaryFile().name)
-    with mock.patch('ansible_base.lib.cache.fallback_cache._temp_file', temp_file):
+    tmp_path = tempfile.gettempdir()
+    temp_file = Path().joinpath(tmp_path, 'gw_primary_cache_failed')
+    with override_settings(ANSIBLE_BASE_FALLBACK_CACHE_FILE_PATH=tmp_path):
         temp_file.touch()
         # Remove singleton instance
         DABCacheWithFallback._instance = None
@@ -196,72 +197,64 @@ def test_all_methods_are_overwritten(method):
 @override_settings(CACHES=cache_settings)
 def test_check_primary_cache(file_exists):
     temp_file = Path(tempfile.NamedTemporaryFile().name)
-    with mock.patch('ansible_base.lib.cache.fallback_cache._temp_file', temp_file):
-        # Remove singleton instance
-        DABCacheWithFallback._instance = None
-        # Initialization of the cache will clear the temp file so do this first
-        cache = DABCacheWithFallback(None, {})
-        # Ensure cache is working
-        cache._primary_cache.fixit()
-
-        # Create the temp file if needed
-        if file_exists:
-            temp_file.touch()
-        else:
-            try:
-                temp_file.unlink()
-            except Exception:
-                pass
-        mocked_function = mock.MagicMock(return_value=None)
-        cache._primary_cache.clear = mocked_function
-        cache.check_primary_cache()
-        if file_exists:
-            mocked_function.assert_called_once()
-        else:
-            mocked_function.assert_not_called()
-        assert temp_file.exists() is False
+    # Remove singleton instance
+    DABCacheWithFallback._instance = None
+    # Initialization of the cache will clear the temp file so do this first
+    cache = DABCacheWithFallback(None, {})
+    cache._temp_file = temp_file
+    # Ensure cache is working
+    cache._primary_cache.fixit()
+
+    # Create the temp file if needed
+    if file_exists:
+        temp_file.touch()
+    else:
+        try:
+            temp_file.unlink()
+        except Exception:
+            pass
+    mocked_function = mock.MagicMock(return_value=None)
+    cache._primary_cache.clear = mocked_function
+    cache.check_primary_cache()
+    if file_exists:
+        mocked_function.assert_called_once()
+    else:
+        mocked_function.assert_not_called()
+    assert temp_file.exists() is False
 
 
 @override_settings(CACHES=cache_settings)
 def test_file_unlink_exception_does_not_cause_failure():
     temp_file = Path(tempfile.NamedTemporaryFile().name)
-    with mock.patch('ansible_base.lib.cache.fallback_cache._temp_file', temp_file):
-        cache = DABCacheWithFallback(None, {})
-        # We can't do: temp_file.unlink = mock.MagicMock(side_effect=Exception('failed to unlink exception'))
-        # Because unlink is marked as read only so we will just mock the cache.clear to raise in its place
-        mocked_function = mock.MagicMock(side_effect=Exception('failed to delete a file exception'))
-        cache._primary_cache.clear = mocked_function
+    cache = DABCacheWithFallback(None, {})
+    cache._temp_file = temp_file
+    # We can't do: temp_file.unlink = mock.MagicMock(side_effect=Exception('failed to unlink exception'))
+    # Because unlink is marked as read only so we will just mock the cache.clear to raise in its place
+    mocked_function = mock.MagicMock(side_effect=Exception('failed to delete a file exception'))
+    cache._primary_cache.clear = mocked_function
 
-        temp_file.touch()
-        cache.check_primary_cache()
-        # No assertion needed because we just want to make sure check_primary_cache does not raise
+    temp_file.touch()
+    cache.check_primary_cache()
+    # No assertion needed because we just want to make sure check_primary_cache does not raise
 
 
+@override_settings(CACHES=cache_settings)
 def test_temp_file_setting(tmp_path):
-    with override_settings(ANSIBLE_BASE_FALLBACK_CACHE_FILE_PATH=tmp_path):
-        import importlib
-
-        from ansible_base.lib.cache import fallback_cache
-
-        importlib.reload(fallback_cache)
+    import os
 
+    with override_settings(ANSIBLE_BASE_FALLBACK_CACHE_FILE_PATH=tmp_path):
+        # Remove singleton instance
+        DABCacheWithFallback._instance = None
+        fallback_cache = DABCacheWithFallback(None, {})
         # Assert that the temp file does not exist yet
         assert not fallback_cache._temp_file.exists()
 
         fallback_cache.create_temp_file()
 
         # The temp file should now exist at the specified cache path
         assert fallback_cache._temp_file.exists()
+
         expected_path = Path().joinpath(tmp_path, 'junk.txt')
         assert fallback_cache._temp_file.parent in expected_path.parents
-
-
-def test_temp_file_permissions():
-    import os
-
-    from ansible_base.lib.cache.fallback_cache import _temp_file, create_temp_file
-
-    create_temp_file()
-    assert _temp_file.exists
-    status = os.stat(_temp_file)
-    assert oct(status.st_mode)[-3:] == "660"
+        status = os.stat(fallback_cache._temp_file)
+        assert oct(status.st_mode)[-3:] == "660"