diff --git a/docs/configuration.rst b/docs/configuration.rst index 33c0ed9..3d219b7 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -311,7 +311,11 @@ dispatched. The flag file (``ops.alert_flag``) is **always** written regardless of the transport setting — it serves as a simple signal for external monitoring tools. -Optionally, an email alert can be sent via SMTP or Mailgun. +Optionally, an email alert can be sent via SMTP or Mailgun. Every failing +cycle sends an ``ALERT``, including consecutive failures on the same +condition — a halt that repeats is a sync that is still not happening. +``RESOLVED`` is sent once, on the cycle that clears an alert that was actually +active, so a healthy daemon does not mail after every sync. Flag-file only (default) """""""""""""""""""""""" diff --git a/src/door_sync/alert.py b/src/door_sync/alert.py index 93ea84f..2ba5d2a 100644 --- a/src/door_sync/alert.py +++ b/src/door_sync/alert.py @@ -5,6 +5,11 @@ regardless of transport — external monitoring (Nagios, Prometheus textfile collector, etc.) can detect halts without parsing logs. +Every failing cycle sends an ALERT, including consecutive failures on the same +condition. RESOLVED is edge-triggered on the flag: it goes out once, on the +cycle that clears an alert that was actually active, so a healthy daemon does +not mail after every sync. + Email failures are logged at ERROR but never crash a reconcile cycle. """ @@ -36,6 +41,11 @@ def raise_( ) -> None: """Write flag file and, if configured, send an alert email. + Every failing cycle mails, including consecutive failures on the same + condition. A halt that repeats is a sync that is still not happening, and + each one is worth surfacing -- only RESOLVED is edge-triggered, so that a + healthy daemon stays silent (see `clear()`). + Args: reason: Human-readable description of the alert condition. path: Path to the alert flag file. @@ -52,18 +62,39 @@ def clear( path: Path, alert_config: AlertConfig | None = None, ) -> None: - """Remove flag file and, if configured, send a resolved email. + """Remove flag file and, if an alert was active, send a resolved email. + + RESOLVED is edge-triggered on the flag file: it goes out only on the cycle + that actually removes a flag, so a healthy daemon does not mail the + operator once per reconcile. A cycle that finds no flag has nothing to + resolve and says nothing. + + If the flag could not be written when the alert was raised, no RESOLVED + follows that alert -- the unwritable path is already on the logger. + + A RESOLVED that fails to send is not retried. The flag is already gone by + then, and that -- not the email -- is what external monitoring reads, so + the recovery is not actually lost. Re-raising the flag to force a retry + would tell monitoring the system is still halted, which is worse than a + missed courtesy email. The send failure is logged at ERROR with the + subject, so a dropped notification is traceable. Args: path: Path to the alert flag file. alert_config: Email transport settings, or None for flag-file only. """ try: - path.unlink(missing_ok=True) + path.unlink() + except FileNotFoundError: + # No alert was active. Nothing to clear, nobody to notify. + return except OSError as exc: # A flag that cannot be cleared errs toward alarming, which is the safe - # direction, but the operator needs to know why it is stuck. + # direction, but the operator needs to know why it is stuck. No RESOLVED + # either: the flag is still there, so this was not a transition, and a + # stuck flag would otherwise mail on every subsequent cycle. _logger.error("could not clear alert flag %s: %s", path, exc) + return if alert_config is not None: _dispatch(alert_config, subject="RESOLVED", body="Previous alert cleared.") @@ -117,7 +148,7 @@ def _send_smtp(cfg: SmtpConfig, *, subject: str, body: str) -> None: server.send_message(msg) _logger.info("alert email sent via SMTP: %s", subject) except Exception as exc: - _logger.error("failed to send alert email via SMTP", exc_info=exc) + _logger.error("failed to send alert email via SMTP: %s", subject, exc_info=exc) def _send_mailgun(cfg: MailgunConfig, *, subject: str, body: str) -> None: @@ -138,4 +169,4 @@ def _send_mailgun(cfg: MailgunConfig, *, subject: str, body: str) -> None: resp.raise_for_status() _logger.info("alert email sent via Mailgun: %s", subject) except Exception as exc: - _logger.error("failed to send alert email via Mailgun", exc_info=exc) + _logger.error("failed to send alert email via Mailgun: %s", subject, exc_info=exc) diff --git a/tests/test_alert.py b/tests/test_alert.py index efd71e6..b9b95d3 100644 --- a/tests/test_alert.py +++ b/tests/test_alert.py @@ -317,3 +317,120 @@ def test_clear_survives_unremovable_flag(tmp_path: Path, caplog: pytest.LogCaptu holder.chmod(0o700) assert "could not clear alert flag" in caplog.text + + +# --- RESOLVED is edge-triggered on the flag, not sent every cycle --- + + +def test_clear_without_active_flag_sends_no_email(tmp_path: Path) -> None: + """A healthy cycle must stay silent. + + clear() runs after every successful reconcile, so dispatching RESOLVED + unconditionally mails the operator once per cycle forever. + """ + path = tmp_path / "alert.flag" + cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) + + with patch("door_sync.alert.httpx.post") as mock_post: + alert.clear(path=path, alert_config=cfg) + + mock_post.assert_not_called() + assert not path.exists() + + +def test_clear_sends_resolved_once_then_stays_silent(tmp_path: Path) -> None: + path = tmp_path / "alert.flag" + cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) + + with patch("door_sync.alert.httpx.post") as mock_post: + alert.raise_("safety halt", path=path, alert_config=cfg) + alert.clear(path=path, alert_config=cfg) + alert.clear(path=path, alert_config=cfg) + alert.clear(path=path, alert_config=cfg) + + subjects = [c.kwargs["data"]["subject"] for c in mock_post.call_args_list] + assert subjects == ["[door-sync] ALERT", "[door-sync] RESOLVED"] + + +@pytest.mark.skipif(os.geteuid() == 0, reason="root ignores file permissions") +def test_clear_sends_no_resolved_when_flag_cannot_be_removed(tmp_path: Path) -> None: + """A stuck flag is not a transition, so it must not mail on every cycle.""" + holder = tmp_path / "ro" + holder.mkdir() + flag = holder / "alert.flag" + flag.write_text("stale\n") + cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) + holder.chmod(0o500) + try: + with patch("door_sync.alert.httpx.post") as mock_post: + alert.clear(path=flag, alert_config=cfg) + finally: + holder.chmod(0o700) + + mock_post.assert_not_called() + + +# --- ALERT is per-failure, not per-episode --- + + +def test_raise_mails_every_failing_cycle(tmp_path: Path) -> None: + """Consecutive failures each mail: a repeat halt is a sync still not happening.""" + path = tmp_path / "alert.flag" + cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) + + with patch("door_sync.alert.httpx.post") as mock_post: + alert.raise_("mass deactivation: 12 of 30 active users", path=path, alert_config=cfg) + alert.raise_("mass deactivation: 12 of 30 active users", path=path, alert_config=cfg) + alert.raise_( + "crashed: ConnectionError: controller unreachable", path=path, alert_config=cfg + ) + + assert mock_post.call_count == 3 + bodies = [c.kwargs["data"]["text"] for c in mock_post.call_args_list] + assert bodies[1] == "mass deactivation: 12 of 30 active users" + assert bodies[2] == "crashed: ConnectionError: controller unreachable" + # The flag tracks the most recent reason. + assert path.read_text(encoding="utf-8") == "crashed: ConnectionError: controller unreachable\n" + + +def test_repeated_failures_then_recovery_sends_one_resolved(tmp_path: Path) -> None: + path = tmp_path / "alert.flag" + cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) + + with patch("door_sync.alert.httpx.post") as mock_post: + alert.raise_("safety halt", path=path, alert_config=cfg) + alert.raise_("safety halt", path=path, alert_config=cfg) + alert.clear(path=path, alert_config=cfg) + alert.clear(path=path, alert_config=cfg) + alert.clear(path=path, alert_config=cfg) + + subjects = [c.kwargs["data"]["subject"] for c in mock_post.call_args_list] + assert subjects == [ + "[door-sync] ALERT", + "[door-sync] ALERT", + "[door-sync] RESOLVED", + ] + + +def test_failed_resolved_dispatch_names_the_lost_notification( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """A RESOLVED that cannot be delivered is not retried, so the log must name it. + + The flag is already gone, so external monitoring sees the recovery either + way -- but the operator waiting on the all-clear email needs to be able to + tell from the log which notification was dropped. + """ + path = tmp_path / "alert.flag" + path.write_text("reason\n", encoding="utf-8") + cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) + + with ( + patch("door_sync.alert.httpx.post", side_effect=ConnectionError("refused")), + caplog.at_level(logging.ERROR, logger="door_sync.alert"), + ): + alert.clear(path=path, alert_config=cfg) + + assert not path.exists() + assert any("RESOLVED" in r.getMessage() for r in caplog.records)