reloader: rearm symlink after target change

nginx_config_reloader/__init__.py

@@ -42,6 +42,8 @@
 logger = logging.getLogger(__name__)
 dbus_loop: EventLoop | None = None
 
+SymlinkTargets = dict[str, tuple[str, int | None, int | None, int | None]]
+
 
 class NginxConfigReloader(FileSystemEventHandler):
     def __init__(
@@ -78,6 +80,7 @@ def __init__(
         self.use_systemd = use_systemd
         self.dirty = False
         self.applying = False
+        self.watched_symlink_targets: SymlinkTargets = {}
         self._on_config_reload = Signal()
         self.error_file = error_file
 
@@ -198,7 +201,7 @@ def remove_error_file(self):
     def apply_new_config(self):
         # Wrapper function to prevent multiple config applications
         if self.applying:
-            logger.debug(f"A config is already being applied. Skipping this one.")
+            logger.debug("A config is already being applied. Skipping this one.")
             return False
 
         self.applying = True
@@ -236,7 +239,7 @@ def _apply(self):
                 error_output = str(e)
                 if hasattr(e, "output"):
                     extra_output = e.output
-                    if isinstance(e.output, bytes):
+                    if isinstance(extra_output, bytes):
                         extra_output = extra_output.decode()
                     error_output += f"\n\n{extra_output}"
                 self.logger.error("Installation of custom config failed")
@@ -340,27 +343,65 @@ def start_observer(self):
             self, self.dir_to_watch, recursive=True, follow_symlink=True
         )
         self.observer.start()
+        self.watched_symlink_targets = self.get_symlink_targets()
 
     def stop_observer(self):
         self.observer.stop()
         self.observer.join()
 
+    def restart_observer(self):
+        self.stop_observer()
+        self.start_observer()
+
+    def get_symlink_targets(self):
+        targets: SymlinkTargets = {}
+        for root, dirnames, _ in os.walk(self.dir_to_watch, followlinks=True):
+            for dirname in dirnames:
+                path = os.path.join(root, dirname)
+                if not os.path.islink(path):
+                    continue
+                try:
+                    st = os.stat(path)
+                    targets[path] = (
+                        os.path.realpath(path),
+                        st.st_dev,
+                        st.st_ino,
+                        st.st_ctime_ns,
+                    )
+                except OSError:
+                    targets[path] = (os.path.realpath(path), None, None, None)
+        return targets
+
+    def symlink_targets_changed(self):
+        return self.get_symlink_targets() != self.watched_symlink_targets
+
 
 class ListenTargetTerminated(BaseException):
     pass
 
 
 def after_loop(nginx_config_reloader: NginxConfigReloader) -> None:
+    if nginx_config_reloader.symlink_targets_changed():
+        nginx_config_reloader.logger.info(
+            "Symlink target changed under watched dir, restarting observer"
+        )
+        try:
+            nginx_config_reloader.restart_observer()
+        except Exception as e:
+            logger.exception(e)
+        nginx_config_reloader.dirty = True
+
     if nginx_config_reloader.dirty:
         try:
             nginx_config_reloader.reload()
-        except:
+        except Exception:
             pass
         nginx_config_reloader.dirty = False
         nginx_config_reloader.applying = False
 
 
 def dbus_event_loop():
+    global dbus_loop
     dbus_loop = EventLoop()
     dbus_loop.run()
 

tests/helpers.py

@@ -0,0 +1,5 @@
+import sys
+
+import pytest
+
+requires_linux = pytest.mark.skipif(sys.platform != "linux", reason="Linux only")

tests/test_after_loop.py

@@ -39,6 +39,40 @@ def test_it_does_not_apply_config_if_tree_not_dirty(self):
         tm.apply_new_config.assert_not_called()
         self.assertFalse(tm.dirty)
 
+    def test_it_restarts_observer_and_reloads_when_symlink_targets_change(self):
+        tm = self._get_nginx_config_reloader_instance()
+        tm.symlink_targets_changed = Mock(return_value=True)
+        tm.restart_observer = Mock()
+        tm.reload = Mock()
+
+        nginx_config_reloader.after_loop(tm)
+
+        tm.restart_observer.assert_called_once_with()
+        tm.reload.assert_called_once_with()
+        self.assertFalse(tm.dirty)
+
+    def test_it_does_not_restart_observer_when_symlink_targets_are_stable(self):
+        tm = self._get_nginx_config_reloader_instance()
+        tm.symlink_targets_changed = Mock(return_value=False)
+        tm.restart_observer = Mock()
+        tm.reload = Mock()
+
+        nginx_config_reloader.after_loop(tm)
+
+        tm.restart_observer.assert_not_called()
+        tm.reload.assert_not_called()
+
+    def test_it_swallows_errors_while_restarting_observer(self):
+        tm = self._get_nginx_config_reloader_instance()
+        tm.symlink_targets_changed = Mock(return_value=True)
+        tm.restart_observer = Mock(side_effect=OSError("inotify watch limit reached"))
+        tm.reload = Mock()
+
+        nginx_config_reloader.after_loop(tm)
+
+        tm.restart_observer.assert_called_once_with()
+        tm.reload.assert_called_once_with()
+
     def _get_nginx_config_reloader_instance(
         self,
         no_magento_config=False,

tests/test_symlink_rearm_integration.py

@@ -0,0 +1,93 @@
+import os
+import shutil
+import time
+from tempfile import mkdtemp
+from unittest.mock import Mock
+
+import nginx_config_reloader
+from tests.helpers import requires_linux
+from tests.testcase import TestCase
+
+
+class TestSymlinkRearmIntegration(TestCase):
+    def setUp(self):
+        self.handler = None
+        self.watch_dir = mkdtemp()
+        self.target_a = mkdtemp()
+        self.target_b = mkdtemp()
+
+    def tearDown(self):
+        if self.handler is not None:
+            try:
+                self.handler.stop_observer()
+            except Exception:
+                pass
+        for d in (self.watch_dir, self.target_a, self.target_b):
+            shutil.rmtree(d, ignore_errors=True)
+
+    def _write(self, path, contents):
+        with open(path, "w") as f:
+            f.write(contents)
+            f.flush()
+            os.fsync(f.fileno())
+
+    def _repoint(self, link, new_target):
+        tmp = link + ".tmp"
+        os.symlink(new_target, tmp)
+        os.replace(tmp, link)
+
+    def _wait_for_dirty(self, handler, timeout=5.0):
+        deadline = time.monotonic() + timeout
+        while time.monotonic() < deadline:
+            if handler.dirty:
+                return True
+            time.sleep(0.02)
+        return handler.dirty
+
+    def _clear_pending_events(self, handler, quiet=0.3, timeout=3.0):
+        deadline = time.monotonic() + timeout
+        handler.dirty = False
+        quiet_until = time.monotonic() + quiet
+        while time.monotonic() < deadline:
+            if handler.dirty:
+                handler.dirty = False
+                quiet_until = time.monotonic() + quiet
+            elif time.monotonic() >= quiet_until:
+                return
+            time.sleep(0.02)
+
+    @requires_linux
+    def test_repointed_symlink_goes_stale_until_after_loop_restarts_observer(self):
+        site = os.path.join(self.watch_dir, "site")
+        os.symlink(self.target_a, site)
+
+        handler = self.handler = nginx_config_reloader.NginxConfigReloader(
+            dir_to_watch=self.watch_dir
+        )
+        handler.reload = Mock()
+        handler.start_observer()
+
+        self._write(os.path.join(site, "a.conf"), "one")
+        self.assertTrue(
+            self._wait_for_dirty(handler),
+            "precondition failed: change under the original target was not detected",
+        )
+
+        self._repoint(site, self.target_b)
+        self._clear_pending_events(handler)
+
+        self._write(os.path.join(site, "b.conf"), "two")
+        self.assertFalse(
+            self._wait_for_dirty(handler, timeout=1.5),
+            "expected the stale watch to miss changes under the repointed target",
+        )
+
+        self.assertTrue(handler.symlink_targets_changed())
+        nginx_config_reloader.after_loop(handler)
+        self._clear_pending_events(handler)
+
+        self._write(os.path.join(site, "c.conf"), "three")
+        self.assertTrue(
+            self._wait_for_dirty(handler),
+            "change under the repointed target was still missed after observer restart",
+        )

tests/test_symlink_targets.py

@@ -0,0 +1,129 @@
+import os
+import shutil
+from tempfile import mkdtemp
+from unittest.mock import Mock
+
+import nginx_config_reloader
+from tests.testcase import TestCase
+
+
+class TestSymlinkTargets(TestCase):
+    def setUp(self):
+        self.watch_dir = mkdtemp()
+        self.target_a = mkdtemp()
+        self.target_b = mkdtemp()
+
+    def tearDown(self):
+        for d in (self.watch_dir, self.target_a, self.target_b):
+            shutil.rmtree(d, ignore_errors=True)
+
+    def _handler(self):
+        return nginx_config_reloader.NginxConfigReloader(dir_to_watch=self.watch_dir)
+
+    def _symlink(self, name, target):
+        path = os.path.join(self.watch_dir, name)
+        os.symlink(target, path)
+        return path
+
+    def test_plain_directories_are_not_recorded(self):
+        os.mkdir(os.path.join(self.watch_dir, "plain"))
+
+        targets = self._handler().get_symlink_targets()
+
+        self.assertEqual(targets, {})
+
+    def test_symlinked_directory_is_recorded_with_target_identity(self):
+        link = self._symlink("example.com", self.target_a)
+        st = os.stat(self.target_a)
+
+        targets = self._handler().get_symlink_targets()
+
+        self.assertEqual(
+            targets,
+            {
+                link: (
+                    os.path.realpath(self.target_a),
+                    st.st_dev,
+                    st.st_ino,
+                    st.st_ctime_ns,
+                )
+            },
+        )
+
+    def test_breaking_a_symlink_is_detected_as_a_change(self):
+        target = os.path.join(self.target_a, "release")
+        os.mkdir(target)
+        self._symlink("example.com", target)
+        handler = self._handler()
+        handler.watched_symlink_targets = handler.get_symlink_targets()
+
+        shutil.rmtree(target)
+
+        self.assertEqual(handler.get_symlink_targets(), {})
+        self.assertTrue(handler.symlink_targets_changed())
+
+    def test_nested_symlink_below_followed_symlink_is_recorded(self):
+        self._symlink("example.com", self.target_a)
+        os.symlink(self.target_b, os.path.join(self.target_a, "inner"))
+
+        targets = self._handler().get_symlink_targets()
+
+        self.assertIn(os.path.join(self.watch_dir, "example.com"), targets)
+        self.assertIn(os.path.join(self.watch_dir, "example.com", "inner"), targets)
+
+    def test_changed_is_false_when_targets_are_stable(self):
+        self._symlink("example.com", self.target_a)
+        handler = self._handler()
+        handler.watched_symlink_targets = handler.get_symlink_targets()
+
+        self.assertFalse(handler.symlink_targets_changed())
+
+    def test_changed_is_true_when_symlink_is_repointed(self):
+        link = self._symlink("example.com", self.target_a)
+        handler = self._handler()
+        handler.watched_symlink_targets = handler.get_symlink_targets()
+
+        os.unlink(link)
+        os.symlink(self.target_b, link)
+
+        self.assertTrue(handler.symlink_targets_changed())
+
+    def test_changed_is_true_when_target_inode_is_replaced(self):
+        target = os.path.join(self.target_a, "release")
+        os.mkdir(target)
+        self._symlink("example.com", target)
+        handler = self._handler()
+        handler.watched_symlink_targets = handler.get_symlink_targets()
+
+        shutil.rmtree(target)
+        os.mkdir(target)
+
+        self.assertTrue(handler.symlink_targets_changed())
+
+    def test_changed_is_true_when_a_new_symlink_appears(self):
+        handler = self._handler()
+        handler.watched_symlink_targets = handler.get_symlink_targets()
+
+        self._symlink("example.com", self.target_a)
+
+        self.assertTrue(handler.symlink_targets_changed())
+
+    def test_after_loop_restarts_and_reloads_when_symlink_is_repointed(self):
+        link = self._symlink("example.com", self.target_a)
+        handler = self._handler()
+        handler.watched_symlink_targets = handler.get_symlink_targets()
+        self.assertFalse(handler.symlink_targets_changed())
+
+        os.unlink(link)
+        os.symlink(self.target_b, link)
+        self.assertFalse(handler.dirty)
+
+        self.assertTrue(handler.symlink_targets_changed())
+
+        handler.restart_observer = Mock()
+        handler.reload = Mock()
+        nginx_config_reloader.after_loop(handler)
+
+        handler.restart_observer.assert_called_once_with()
+        handler.reload.assert_called_once_with()
+        self.assertFalse(handler.dirty)