Add more robust plugin installation check (#12858)

src/backend/InvenTree/plugin/installer.py

@@ -3,6 +3,7 @@
 import re
 import subprocess
 import sys
+from pathlib import Path
 from typing import Optional
 
 from django.conf import settings
@@ -128,6 +129,67 @@ def plugins_file_hash():
         return None
 
 
+def plugin_env_marker_path() -> Path:
+    """Return the path to the plugin-install marker file for the *current* python environment.
+
+    This lives inside the running interpreter's environment (``sys.prefix``)
+    rather than in the database, so it disappears along with the rest of the
+    environment whenever a fresh virtual environment is created - e.g. a
+    container replaced without a persistent venv volume. A database-only hash
+    cannot tell such a fresh environment apart from one that already has the
+    packages installed (inventree/InvenTree#12848).
+    """
+    return Path(sys.prefix) / '.inventree_plugins_hash'
+
+
+# Process-local fallback for get_env_plugin_hash(), used when the marker file
+# itself cannot be written (e.g. a read-only sys.prefix).
+_env_plugin_hash_cache: Optional[str] = None
+
+
+def get_env_plugin_hash() -> Optional[str]:
+    """Return the plugin file hash last installed into the *current* python environment.
+
+    Returns None if no install has been recorded here (e.g. a fresh environment).
+    """
+    if _env_plugin_hash_cache is not None:
+        return _env_plugin_hash_cache
+
+    path = plugin_env_marker_path()
+
+    if not path.exists():
+        return None
+
+    try:
+        return path.read_text().strip()
+    except Exception:
+        log_error('get_env_plugin_hash', scope='plugins')
+        return None
+
+
+def set_env_plugin_hash(file_hash: str) -> None:
+    """Record that the current python environment has installed the given plugin file hash."""
+    global _env_plugin_hash_cache
+
+    _env_plugin_hash_cache = file_hash
+
+    try:
+        plugin_env_marker_path().write_text(file_hash)
+    except Exception:
+        # Not logged via log_error/the database error log: on a deployment
+        # where sys.prefix is not writable (by design, e.g. a read-only
+        # root filesystem) this would otherwise happen on every single
+        # process start forever, and it is an environment property rather
+        # than an application bug.
+        logger.warning(
+            "Could not persist plugin install marker to '%s' - this "
+            'environment will be re-verified on every process restart '
+            'instead of only when %s changes',
+            plugin_env_marker_path(),
+            settings.PLUGIN_FILE,
+        )
+
+
 def install_plugins_file():
     """Install plugins from the plugins file."""
     logger.info('Installing plugins from plugins file')

src/backend/InvenTree/plugin/registry.py

@@ -682,32 +682,31 @@ class PluginsRegistry:
     def install_plugin_file(self):
         """Make sure all plugins are installed in the current environment.
 
-        The hash is only persisted *after* a successful install, never before -
+        - The hash is only persisted *after* a successful install
-        the lease alone is what stops two processes from installing at once (a
+        - Store the hash into the database
-        process that cannot acquire it simply skips, since another one is
+        - Also store the hash as a local marker file in the current environment
-        already handling it). Writing the hash as soon as the lease is acquired
+
-        would be a narrower window with a worse failure mode: a process killed
-        outright (OOM, a container stopped mid-install) between that write and
-        actually finishing would leave the hash pointing at content that was
-        never installed, and - unlike a normal exception - a hard kill does not
-        run `finally`, so nothing would ever revert it. The next check would
-        then see the hash already matches and skip forever, until the plugins
-        file changes again. Persisting only on success means a kill at any
-        point simply leaves the previous hash in place, so the next check
-        retries normally.
         """
-        from plugin.installer import install_plugins_file, plugins_file_hash
+        from plugin.installer import (
+            get_env_plugin_hash,
+            install_plugins_file,
+            plugins_file_hash,
+            set_env_plugin_hash,
+        )
 
         file_hash = plugins_file_hash()
 
         if file_hash is None:
             return
 
-        current_hash = get_global_setting(
+        def already_satisfied() -> bool:
-            '_PLUGIN_FILE_HASH', '', create=False, cache=False
+            """True only if the database *and* this environment agree it's installed."""
-        )
+            current_hash = get_global_setting(
+                '_PLUGIN_FILE_HASH', '', create=False, cache=False
+            )
+            return current_hash == file_hash and get_env_plugin_hash() == file_hash
 
-        if current_hash == file_hash:
+        if already_satisfied():
             return
 
         if not try_acquire_lease('_PLUGIN_FILE_HASH'):
@@ -716,15 +715,12 @@ class PluginsRegistry:
         try:
             # Re-check under the lease: another process may have already
             # installed this exact change while we were waiting to acquire it
-            current_hash = get_global_setting(
+            if already_satisfied():
-                '_PLUGIN_FILE_HASH', '', create=False, cache=False
-            )
-
-            if current_hash == file_hash:
                 return
 
             if install_plugins_file() is not False:
                 set_global_setting('_PLUGIN_FILE_HASH', file_hash)
+                set_env_plugin_hash(file_hash)
         finally:
             release_lease('_PLUGIN_FILE_HASH')
 

src/backend/InvenTree/plugin/test_plugin.py

@@ -753,14 +753,22 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase):
             )
             return True
 
-        with mock.patch(
+        with (
-            'plugin.installer.plugins_file_hash', return_value='test-hash-1'
+            tempfile.TemporaryDirectory() as tmpdir,
-        ):
+            mock.patch('plugin.installer._env_plugin_hash_cache', None),
-            with mock.patch(
+            mock.patch(
+                'plugin.installer.plugin_env_marker_path',
+                return_value=Path(tmpdir) / '.inventree_plugins_hash',
+            ),
+            mock.patch(
+                'plugin.installer.plugins_file_hash', return_value='test-hash-1'
+            ),
+            mock.patch(
                 'plugin.installer.install_plugins_file',
                 side_effect=fake_install_plugins_file,
-            ):
+            ),
-                registry.install_plugin_file()
+        ):
+            registry.install_plugin_file()
 
         # While the install was in progress, the hash must not yet reflect the
         # new (not-yet-installed) value
@@ -778,13 +786,19 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase):
 
         set_global_setting('_PLUGIN_FILE_HASH', '')
 
-        with mock.patch(
+        with (
-            'plugin.installer.plugins_file_hash', return_value='test-hash-2'
+            tempfile.TemporaryDirectory() as tmpdir,
+            mock.patch('plugin.installer._env_plugin_hash_cache', None),
+            mock.patch(
+                'plugin.installer.plugin_env_marker_path',
+                return_value=Path(tmpdir) / '.inventree_plugins_hash',
+            ),
+            mock.patch(
+                'plugin.installer.plugins_file_hash', return_value='test-hash-2'
+            ),
+            mock.patch('plugin.installer.install_plugins_file', return_value=False),
         ):
-            with mock.patch(
+            registry.install_plugin_file()
-                'plugin.installer.install_plugins_file', return_value=False
-            ):
-                registry.install_plugin_file()
 
         self.assertEqual(get_global_setting('_PLUGIN_FILE_HASH', '', create=False), '')
 
@@ -792,19 +806,104 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase):
         """Test that install_plugin_file() is a no-op once the hash already matches."""
         from plugin.registry import registry
 
-        with mock.patch(
+        with (
-            'plugin.installer.plugins_file_hash', return_value='test-hash-3'
+            tempfile.TemporaryDirectory() as tmpdir,
-        ):
+            mock.patch('plugin.installer._env_plugin_hash_cache', None),
-            with mock.patch(
+            mock.patch(
+                'plugin.installer.plugin_env_marker_path',
+                return_value=Path(tmpdir) / '.inventree_plugins_hash',
+            ),
+            mock.patch(
+                'plugin.installer.plugins_file_hash', return_value='test-hash-3'
+            ),
+            mock.patch(
                 'plugin.installer.install_plugins_file', return_value=True
-            ) as mock_install:
+            ) as mock_install,
-                registry.install_plugin_file()
+        ):
-                mock_install.assert_called_once()
+            registry.install_plugin_file()
+            mock_install.assert_called_once()
 
-                # A second call with the same (already-installed) hash must not
+            # A second call with the same (already-installed) hash must not
-                # install again
+            # install again
-                registry.install_plugin_file()
+            registry.install_plugin_file()
-                mock_install.assert_called_once()
+            mock_install.assert_called_once()
+
+    def test_install_plugin_file_reinstalls_in_fresh_environment(self):
+        """Test that a matching database hash alone does not skip installation.
+
+        Regression test for inventree/InvenTree#12848: a fresh python
+        environment (e.g. a container replaced without a persistent venv
+        volume) must reinstall even though the database still remembers a
+        previous environment's successful install of the exact same file.
+        """
+        from common.settings import set_global_setting
+        from plugin.registry import registry
+
+        # Simulate a database that already believes this hash is installed -
+        # as if a *previous* (now-replaced) environment installed it
+        set_global_setting('_PLUGIN_FILE_HASH', 'test-hash-4')
+
+        with (
+            tempfile.TemporaryDirectory() as tmpdir,
+            mock.patch('plugin.installer._env_plugin_hash_cache', None),
+            mock.patch(
+                'plugin.installer.plugin_env_marker_path',
+                # A fresh environment has no marker file at all
+                return_value=Path(tmpdir) / '.inventree_plugins_hash',
+            ),
+            mock.patch(
+                'plugin.installer.plugins_file_hash', return_value='test-hash-4'
+            ),
+            mock.patch(
+                'plugin.installer.install_plugins_file', return_value=True
+            ) as mock_install,
+        ):
+            registry.install_plugin_file()
+            mock_install.assert_called_once()
+
+            # Now that this environment has recorded the install, a second
+            # call is correctly skipped
+            registry.install_plugin_file()
+            mock_install.assert_called_once()
+
+    def test_install_plugin_file_falls_back_to_process_cache_when_marker_unwritable(
+        self,
+    ):
+        """Test that a non-writable marker location still settles within one process.
+
+        If sys.prefix is not writable (e.g. a read-only root filesystem),
+        set_env_plugin_hash() cannot persist the marker file to disk. Without
+        an in-process fallback, get_env_plugin_hash() would then return None
+        forever, and install_plugin_file() would re-attempt `pip install` on
+        every single call within the same process - not just once per
+        process start.
+        """
+        from common.settings import set_global_setting
+        from plugin.registry import registry
+
+        set_global_setting('_PLUGIN_FILE_HASH', '')
+
+        marker = mock.MagicMock()
+        marker.exists.return_value = False
+        marker.write_text.side_effect = OSError('Read-only file system')
+
+        with (
+            mock.patch('plugin.installer._env_plugin_hash_cache', None),
+            mock.patch('plugin.installer.plugin_env_marker_path', return_value=marker),
+            mock.patch(
+                'plugin.installer.plugins_file_hash', return_value='test-hash-5'
+            ),
+            mock.patch(
+                'plugin.installer.install_plugins_file', return_value=True
+            ) as mock_install,
+        ):
+            registry.install_plugin_file()
+            mock_install.assert_called_once()
+
+            # The marker file could not be written, but the in-memory cache
+            # still settles this process - a second call must not reinstall
+            registry.install_plugin_file()
+            mock_install.assert_called_once()
 
     def test_builtin_mandatory_plugins(self):
         """Test that mandatory builtin plugins are always loaded."""