diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index ee5c1e0..a4ee824 100644 --- a/nginx_config_reloader/__init__.py +++ b/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() diff --git a/tests/helpers.py b/tests/helpers.py new file mode 100644 index 0000000..d958187 --- /dev/null +++ b/tests/helpers.py @@ -0,0 +1,5 @@ +import sys + +import pytest + +requires_linux = pytest.mark.skipif(sys.platform != "linux", reason="Linux only") diff --git a/tests/test_after_loop.py b/tests/test_after_loop.py index b603e97..8ce298e 100644 --- a/tests/test_after_loop.py +++ b/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, diff --git a/tests/test_symlink_rearm_integration.py b/tests/test_symlink_rearm_integration.py new file mode 100644 index 0000000..d630f42 --- /dev/null +++ b/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", + ) diff --git a/tests/test_symlink_targets.py b/tests/test_symlink_targets.py new file mode 100644 index 0000000..58847c0 --- /dev/null +++ b/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)