fix(zone_sync): thaw and warn when a sync is stopped or killed

mdb/management/commands/zone_sync.py

@@ -1,11 +1,30 @@
 import getpass
 import os
+import signal
 
 from django.core.management.base import BaseCommand
 
 from mdb.services import discord
 from mdb.services.zone_sync import ZoneSyncService
 
+# What stops a sync run by hand: a dropped ssh session, a kill, Ctrl-C.
+STOPPING_SIGNALS = (signal.SIGHUP, signal.SIGTERM, signal.SIGINT)
+
+
+class Interrupted(Exception):
+    def __init__(self, signum):
+        self.signum = signum
+        super().__init__(signal.Signals(signum).name)
+
+
+def raise_interrupted(signum, frame):
+    raise Interrupted(signum)
+
+
+def set_handlers(handler):
+    for signum in STOPPING_SIGNALS:
+        signal.signal(signum, handler)
+
 
 class Command(BaseCommand):
     args = "[--force] [--debug]"
@@ -46,6 +65,9 @@ class Command(BaseCommand):
         os.path.isdir(service.checkzone_dir) or os.mkdir(service.checkzone_dir)
         debug and (os.path.isdir(service.debug_dir) or os.mkdir(service.debug_dir))
 
+        if not debug:
+            self.thaw_after_killed_sync(service)
+
         if zone_name:
             zone = service.find_zone(zone_name)
             if zone is None:
@@ -63,14 +85,75 @@ class Command(BaseCommand):
                 service.sync_zone(zone)
             return
 
+        previous = {signum: signal.getsignal(signum) for signum in STOPPING_SIGNALS}
+        try:
+            # From the freeze to the thaw, a signal must not stop the sync in between:
+            # ignored while rndc runs, it stops the writes for a reload and thaw.
+            set_handlers(signal.SIG_IGN)
             self.freeze(service, zones)
+            set_handlers(raise_interrupted)
             try:
                 for zone in zones:
                     service.sync_zone(zone)
-        except BaseException:
-            service.reload()
+            except BaseException as e:
+                set_handlers(signal.SIG_IGN)
+                self.reload_after_interruption(service, zones, e)
+                if isinstance(e, Interrupted):
+                    raise SystemExit(128 + e.signum) from e
                 raise
+            set_handlers(signal.SIG_IGN)
             self.reload_and_notify(service, service.changes)
+        finally:
+            for signum, handler in previous.items():
+                signal.signal(signum, handler)
+
+    def thaw_after_killed_sync(self, service):
+        since = service.frozen_since()
+        if since is None:
+            return
+
+        self.stdout.write("a previous sync left the zones frozen, thawing... ")
+        summary = (
+            f"A zone sync killed after {since:%Y-%m-%d %H:%M} left every zone frozen"
+        )
+        success, failed_command, output = service.reload()
+        if not success:
+            self.stdout.write("fail\n")
+            self.notify_failure(
+                f"{summary}, and thawing them failed", failed_command, output, []
+            )
+        self.stdout.write("ok\n")
+        discord.notify(
+            subject="⚠️ Zones thawed after a killed zone sync",
+            message=f"{summary}, refusing dynamic updates until now. "
+            "They are reloaded and thawed.",
+        )
+
+    def reload_after_interruption(self, service, zones, error):
+        reason = str(error) if isinstance(error, Interrupted) else repr(error)
+        written = [z for z, c in service.changes.items() if c["success"]]
+        not_written = [z.domain_name for z in zones if z.domain_name not in written]
+
+        self.stdout.write(f"stopped by {reason}, reloading zones... ")
+        success, failed_command, output = service.reload()
+        if not success:
+            self.stdout.write("fail\n")
+            self.notify_failure(
+                f"Zone sync stopped by {reason}, and reloading the zones failed; "
+                "they may still be frozen",
+                failed_command,
+                output,
+                not_written,
+            )
+        self.stdout.write("ok\n")
+        message = f"Zone sync stopped by {reason}. The zones are reloaded and thawed."
+        if written:
+            message += "\n\nWritten:\n" + "\n".join("- " + z for z in written)
+        if not_written:
+            message += "\n\nNot written, run mdb-sync again:\n" + "\n".join(
+                "- " + z for z in not_written
+            )
+        discord.notify(subject="⚠️ Zone sync interrupted, zones thawed", message=message)
 
     def freeze(self, service, zones):
         self.stdout.write("freezing zones... ")
@@ -78,7 +161,7 @@ class Command(BaseCommand):
         if not success:
             self.stdout.write("fail\n")
             # A failed freeze may have frozen some zones before stopping.
-            service.run_commands([service.thaw_command])
+            service.thaw()
             self.notify_failure(
                 "Failed to freeze zones, nothing was written",
                 failed_command,

mdb/services/zone_sync.py

@@ -4,6 +4,7 @@ import shutil
 import stat
 import sys
 import tempfile
+from datetime import datetime
 from difflib import context_diff
 from itertools import chain
 from subprocess import getstatusoutput
@@ -44,17 +45,50 @@ class ZoneSyncService:
                 return False, command, output
         return True, "", ""
 
+    @property
+    def frozen_marker(self):
+        """Exists from just before rndc freeze until the zones are thawed.
+
+        A sync killed in between (SIGKILL, the OOM killer) leaves it behind, and the
+        next sync thaws the zones it finds frozen.
+        """
+        return os.path.join(self.checkzone_dir, "zones-frozen")
+
+    def frozen_since(self):
+        """When a sync that never thawed froze the zones, or None."""
+        try:
+            return datetime.fromtimestamp(os.path.getmtime(self.frozen_marker))
+        except FileNotFoundError:
+            return None
+
     def freeze(self):
         """Stop dynamic updates and flush each dynamic zone's journal to its file.
 
         Must run before zone files are written: freeze dumps the zone BIND
         holds in memory, which would overwrite a file written earlier.
         """
+        with open(self.frozen_marker, "w") as f:
+            f.write(f"{os.getpid()}\n")
         return self.run_commands([self.freeze_command])
 
+    def thaw(self):
+        """Resume dynamic updates, which a frozen zone refuses (ACME dns-01 among them)."""
+        result = self.run_commands([self.thaw_command])
+        if result[0]:
+            try:
+                os.unlink(self.frozen_marker)
+            except FileNotFoundError:
+                pass
+        return result
+
     def reload(self):
-        """Load the written zone files and resume dynamic updates."""
-        return self.run_commands([self.reload_command, self.thaw_command])
+        """Load the written zone files and resume dynamic updates.
+
+        Thaws even when the reload fails, and reports the first failure.
+        """
+        reloaded = self.run_commands([self.reload_command])
+        thawed = self.thaw()
+        return reloaded if not reloaded[0] else thawed
 
     def find_zone(self, name):
         for model in (Domain, Ip4Subnet, Ip6Subnet):

tests/test_zone_sync_command.py

@@ -1,4 +1,5 @@
 import os
+import signal
 from io import StringIO
 from unittest.mock import MagicMock, patch
 
@@ -8,6 +9,13 @@ from mdb.management.commands.zone_sync import Command as CommandClass
 from mdb.services.zone_sync import ZoneSyncService
 
 
+@pytest.fixture(autouse=True)
+def checkzone_dir(tmp_path, monkeypatch):
+    """Where the frozen marker lives; not the real /tmp/mdb-checkzone."""
+    monkeypatch.setattr(ZoneSyncService, "checkzone_dir", str(tmp_path))
+    return tmp_path
+
+
 @pytest.fixture
 def cmd():
     c = CommandClass()
@@ -185,7 +193,7 @@ def test_handle_writes_nothing_when_freeze_fails(cmd, db_domain):
         patch.object(
             ZoneSyncService, "freeze", return_value=(False, "rndc freeze", "no")
         ),
-        patch.object(ZoneSyncService, "run_commands") as mock_run,
+        patch.object(ZoneSyncService, "thaw", return_value=(True, "", "")) as thaw,
         patch("mdb.management.commands.zone_sync.discord") as mock_discord,
         patch("getpass.getuser", return_value="testuser"),
         pytest.raises(SystemExit),
@@ -193,7 +201,7 @@ def test_handle_writes_nothing_when_freeze_fails(cmd, db_domain):
         cmd.handle(debug=False, force=False, zone=None)
 
     assert not os.path.exists(db_domain.domain_filename)
-    mock_run.assert_called_once_with([ZoneSyncService.thaw_command])
+    thaw.assert_called_once()
     assert "- example.org" in mock_discord.notify.call_args.kwargs["message"]
 
 
@@ -201,10 +209,161 @@ def test_handle_writes_nothing_when_freeze_fails(cmd, db_domain):
 def test_handle_thaws_when_sync_raises(cmd, db_domain):
     with (
         patch.object(ZoneSyncService, "freeze", return_value=(True, "", "")),
-        patch.object(ZoneSyncService, "reload") as mock_reload,
+        patch.object(
+            ZoneSyncService, "reload", return_value=(True, "", "")
+        ) as mock_reload,
         patch.object(ZoneSyncService, "sync_zone", side_effect=RuntimeError),
+        patch("mdb.management.commands.zone_sync.discord"),
         pytest.raises(RuntimeError),
     ):
         cmd.handle(debug=False, force=False, zone=None)
 
     mock_reload.assert_called_once()
+
+
+# --- stopped and killed syncs ---
+
+
+def rndc(results=None):
+    """Patch rndc: every command succeeds unless results maps it to (status, output)."""
+    results = results or {}
+    calls = []
+
+    def run(command):
+        calls.append(command.rsplit(" ", 1)[-1])
+        return results.get(command, (0, ""))
+
+    return calls, patch("mdb.services.zone_sync.getstatusoutput", side_effect=run)
+
+
+def test_reload_thaws_even_when_reload_fails(service):
+    calls, rndc_patch = rndc({ZoneSyncService.reload_command: (1, "timed out")})
+    with rndc_patch:
+        success, failed_command, output = service.reload()
+    assert calls == ["reload", "thaw"]
+    assert (success, failed_command, output) == (
+        False,
+        ZoneSyncService.reload_command,
+        "timed out",
+    )
+
+
+def test_marker_exists_only_while_frozen(service):
+    calls, rndc_patch = rndc()
+    with rndc_patch:
+        service.freeze()
+        assert service.frozen_since() is not None
+        service.reload()
+    assert service.frozen_since() is None
+
+
+def test_marker_stays_when_thaw_fails(service):
+    calls, rndc_patch = rndc({ZoneSyncService.thaw_command: (1, "no")})
+    with rndc_patch:
+        service.freeze()
+        service.reload()
+    assert service.frozen_since() is not None
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize("signum", [signal.SIGHUP, signal.SIGTERM, signal.SIGINT])
+def test_handle_thaws_and_warns_when_stopped_by_signal(cmd, db_domain, signum):
+    def killed(zone):
+        os.kill(os.getpid(), signum)
+
+    calls, rndc_patch = rndc()
+    previous = signal.getsignal(signum)
+    with (
+        rndc_patch,
+        patch.object(ZoneSyncService, "sync_zone", side_effect=killed),
+        patch("mdb.management.commands.zone_sync.discord") as mock_discord,
+        pytest.raises(SystemExit) as exc,
+    ):
+        cmd.handle(debug=False, force=False, zone=None)
+
+    assert exc.value.code == 128 + signum
+    assert calls == ["freeze", "reload", "thaw"]
+    assert ZoneSyncService().frozen_since() is None
+    kwargs = mock_discord.notify.call_args.kwargs
+    assert kwargs["subject"] == "⚠️ Zone sync interrupted, zones thawed"
+    assert signal.Signals(signum).name in kwargs["message"]
+    assert "Not written, run mdb-sync again:\n- example.org" in kwargs["message"]
+    assert signal.getsignal(signum) == previous
+
+
+@pytest.mark.django_db
+def test_handle_ignores_signals_while_rndc_runs(cmd, db_domain):
+    seen = []
+
+    def run(commands):
+        seen.append((commands, signal.getsignal(signal.SIGHUP)))
+        return True, "", ""
+
+    with (
+        patch.object(ZoneSyncService, "run_commands", side_effect=run),
+        patch("mdb.management.commands.zone_sync.discord"),
+    ):
+        cmd.handle(debug=False, force=False, zone=None)
+
+    # Signals still stop named-checkzone and the writes, between freeze and reload.
+    assert seen == [
+        ([ZoneSyncService.freeze_command], signal.SIG_IGN),
+        ([ZoneSyncService.reload_command], signal.SIG_IGN),
+        ([ZoneSyncService.thaw_command], signal.SIG_IGN),
+    ]
+
+
+@pytest.mark.django_db
+def test_handle_warns_when_reload_after_signal_fails(cmd, db_domain):
+    calls, rndc_patch = rndc({ZoneSyncService.thaw_command: (1, "no")})
+    with (
+        rndc_patch,
+        patch.object(
+            ZoneSyncService,
+            "sync_zone",
+            side_effect=lambda zone: os.kill(os.getpid(), signal.SIGHUP),
+        ),
+        patch("mdb.management.commands.zone_sync.discord") as mock_discord,
+        patch("getpass.getuser", return_value="testuser"),
+        pytest.raises(SystemExit),
+    ):
+        cmd.handle(debug=False, force=False, zone=None)
+
+    kwargs = mock_discord.notify.call_args.kwargs
+    assert kwargs["subject"] == "⚠️ FAIL: bind"
+    assert "may still be frozen" in kwargs["message"]
+    assert ZoneSyncService().frozen_since() is not None
+
+
+@pytest.mark.django_db
+def test_handle_thaws_zones_a_killed_sync_left_frozen(cmd, checkzone_dir):
+    (checkzone_dir / "zones-frozen").write_text("1234\n")
+    calls, rndc_patch = rndc()
+    with (
+        rndc_patch,
+        patch("mdb.management.commands.zone_sync.discord") as mock_discord,
+    ):
+        cmd.handle(debug=False, force=False, zone=None)
+
+    assert calls == ["reload", "thaw"]
+    assert ZoneSyncService().frozen_since() is None
+    kwargs = mock_discord.notify.call_args.kwargs
+    assert kwargs["subject"] == "⚠️ Zones thawed after a killed zone sync"
+    assert "left every zone frozen" in kwargs["message"]
+
+
+@pytest.mark.django_db
+def test_handle_stops_when_thawing_a_killed_sync_fails(cmd, db_domain, checkzone_dir):
+    (checkzone_dir / "zones-frozen").write_text("1234\n")
+    calls, rndc_patch = rndc({ZoneSyncService.reload_command: (1, "no")})
+    with (
+        rndc_patch,
+        patch("mdb.management.commands.zone_sync.discord") as mock_discord,
+        patch("getpass.getuser", return_value="testuser"),
+        pytest.raises(SystemExit),
+    ):
+        cmd.handle(debug=False, force=False, zone=None)
+
+    assert calls == ["reload", "thaw"]  # nothing frozen again, nothing written
+    assert not os.path.exists(db_domain.domain_filename)
+    assert "thawing them failed" in mock_discord.notify.call_args.kwargs["message"]