Log certificate status and stop renewal reload storm

Print the SSL Refresh Thread status table (domain, time left, self-signed
list, next check) when the watch set changes and when the renewal thread
starts. Queue only one forced reload per renewal trigger while the renewal
pass is in flight, and force a reload when a certificate file already in
use is replaced during an ordinary reload. Require certapi 1.1.16, which
stops re-signalling the callback every second.

Author: mesudip

nginx_proxy/WebServer.py

@@ -251,6 +251,10 @@ def _do_reload(self, forced=False, validate=True) -> bool:
         """
         # print("web_server._do_reload(forced="+str(forced)+")")
         output = self._render_config(self.config_data, update_ssl_watch_domains=True)
+        # Backstop for renewals that happen inside an ordinary (non-forced) reload, e.g. a docker event
+        # whose pass found a due certificate: the file contents changed but the rendered text did not.
+        if self.ssl_processor.pop_certificate_changes():
+            forced = True
         response = self.nginx.update_config(output, force=forced, validate=validate)
         return response
 

nginx_proxy/post_processors/ssl_certificate_processor.py

@@ -1,3 +1,4 @@
+import threading
 from datetime import datetime, timedelta, timezone
 from typing import List, Optional, Tuple
 
@@ -39,6 +40,14 @@ def __init__(
         )
         self._status_rows: List[Tuple[str, str, Optional[datetime]]] = []
         self._last_logged_status: Optional[Tuple] = None
+        # The renewal worker may invoke the callback repeatedly (once a second) while a renewal pass is
+        # still in flight on the reload thread. Only one forced reload is queued until that reload has run.
+        self._renewal_reload_lock = threading.Lock()
+        self._renewal_reload_pending = False
+        # Expiry of every certificate file seen in the last pass, used to detect files replaced by a
+        # renewal so that nginx is reloaded even when the rendered configuration text is unchanged.
+        self._known_file_expiry: dict = {}
+        self._certificates_changed = False
 
         if start_ssl_thread:
             self.start()
@@ -54,10 +63,21 @@ def start(self):
         self._log_next_check()
 
     def ssl_renewal_callback(self):
-        print("[SSL] Renewal callback triggered")
         if self.server is None:
             return
-        self.server.enqueue_reload(force=True)
+        with self._renewal_reload_lock:
+            if self._renewal_reload_pending:
+                return
+            self._renewal_reload_pending = True
+        print("[SSL Refresh Thread] Renewal due, requesting forced nginx reload")
+        try:
+            # Forced on purpose: a renewal replaces certificate file contents, not paths, so the rendered
+            # config is byte-identical and a plain reload would be skipped by the config diff.
+            self.server.enqueue_reload(force=True)
+        except Exception:
+            with self._renewal_reload_lock:
+                self._renewal_reload_pending = False
+            raise
 
     def _find_certificate_for_domain(self, domain: str) -> None | Tuple[str, Key, List[Certificate]]:
         if hasattr(self.key_store, "find_key_and_cert_covering_domain"):
@@ -127,13 +147,38 @@ def process_ssl_certificates(self, hosts: List[Host], update_watch_domains: bool
 
         secured_domains = sorted({host.hostname for host in secured_hosts})
         if update_watch_domains:
-            self.renewal_manager.update_watch_domains(secured_domains)
+            try:
+                self.renewal_manager.update_watch_domains(secured_domains)
+            finally:
+                with self._renewal_reload_lock:
+                    self._renewal_reload_pending = False
 
         for host in secured_hosts:
             host.ssl_file = self._select_ssl_file(host)
 
         if update_watch_domains:
             self.log_certificate_status(secured_hosts)
+            self._detect_certificate_changes()
+
+    def _detect_certificate_changes(self):
+        """Flag a forced reload when a certificate file already in use now carries a different expiry."""
+        current = {}
+        for _domain, ssl_file, expiry in self._status_rows:
+            if ssl_file and expiry is not None:
+                current[ssl_file] = expiry
+        changed = [f for f, e in current.items() if f in self._known_file_expiry and self._known_file_expiry[f] != e]
+        if changed:
+            print(
+                f"[SSL Refresh Thread] Certificates renewed on disk, nginx reload required: {', '.join(sorted(changed))}"
+            )
+            self._certificates_changed = True
+        self._known_file_expiry = current
+
+    def pop_certificate_changes(self) -> bool:
+        """Return True once if certificate files changed since the last call, then reset."""
+        changed = self._certificates_changed
+        self._certificates_changed = False
+        return changed
 
     # ------------------------------------------------------------------
     # Status logging

tests/unit/test_ssl_status_logging.py

@@ -173,3 +173,74 @@ def test_start_without_certificates_reports_nothing_to_watch(capsys):
     assert "[SSL Refresh Thread] Looks like there are no ssl certificates, sleeping until there's one" in (
         capsys.readouterr().out
     )
+
+
+def test_renewal_callback_queues_only_one_reload_until_it_has_run(capsys):
+    now = datetime.now(timezone.utc)
+    processor, _ = _build_processor({"api.example.com": now + timedelta(days=40)})
+    hosts = [Host("api.example.com", 443, {"https"})]
+
+    # The renewal worker ticks once a second while a renewal pass is in flight.
+    for _ in range(5):
+        processor.ssl_renewal_callback()
+
+    processor.server.enqueue_reload.assert_called_once_with(force=True)
+    assert capsys.readouterr().out.count("Renewal due, requesting forced nginx reload") == 1
+
+    # The queued reload runs and refreshes the watch set, after which a new renewal may queue again.
+    processor.process_ssl_certificates(hosts)
+    processor.ssl_renewal_callback()
+    assert processor.server.enqueue_reload.call_count == 2
+
+
+def test_renewal_callback_pending_flag_is_released_when_enqueue_fails():
+    now = datetime.now(timezone.utc)
+    processor, _ = _build_processor({"api.example.com": now + timedelta(days=40)})
+    processor.server.enqueue_reload.side_effect = [RuntimeError("queue closed"), True]
+
+    try:
+        processor.ssl_renewal_callback()
+    except RuntimeError:
+        pass
+    processor.ssl_renewal_callback()
+
+    assert processor.server.enqueue_reload.call_count == 2
+
+
+def test_certificate_change_is_flagged_once_when_a_used_file_gets_a_new_expiry(capsys):
+    now = datetime.now(timezone.utc)
+    expiries = {"api.example.com": now + timedelta(days=20), "*.example.org": now + timedelta(days=50)}
+    processor, _ = _build_processor(expiries)
+    hosts = [Host("api.example.com", 443, {"https"}), Host("www.example.org", 443, {"https"})]
+
+    processor.process_ssl_certificates(hosts)
+    assert processor.pop_certificate_changes() is False  # first sight of the files is not a change
+
+    processor.process_ssl_certificates(hosts)
+    assert processor.pop_certificate_changes() is False  # nothing renewed
+
+    expiries["api.example.com"] = now + timedelta(days=89)  # renewal replaced the file on disk
+    processor.process_ssl_certificates(hosts)
+    out = capsys.readouterr().out
+    assert "Certificates renewed on disk, nginx reload required: api.example.com" in out
+    assert processor.pop_certificate_changes() is True
+    assert processor.pop_certificate_changes() is False  # consumed
+
+
+def test_new_or_dry_run_files_do_not_flag_a_certificate_change():
+    now = datetime.now(timezone.utc)
+    expiries = {"api.example.com": now + timedelta(days=60)}
+    processor, _ = _build_processor(expiries)
+    processor.process_ssl_certificates([Host("api.example.com", 443, {"https"})])
+    processor.pop_certificate_changes()
+
+    # A domain appearing for the first time changes the rendered config anyway, so no force needed.
+    expiries["new.example.com"] = now + timedelta(days=60)
+    hosts = [Host("api.example.com", 443, {"https"}), Host("new.example.com", 443, {"https"})]
+    processor.process_ssl_certificates(hosts)
+    assert processor.pop_certificate_changes() is False
+
+    # Dry runs never touch the tracking state.
+    expiries["api.example.com"] = now + timedelta(days=89)
+    processor.process_ssl_certificates(hosts, update_watch_domains=False)
+    assert processor.pop_certificate_changes() is False

tests/unit/test_web_server.py

@@ -632,3 +632,17 @@ def test_rescan_prefer_local_keeps_health_gate_for_swarm_task(web_server):
         web_server.rescan_all_container(bypass_start_grace=True)
 
     mock_register.assert_not_called()
+
+
+def test_do_reload_forces_nginx_reload_only_when_certificate_files_changed(web_server):
+    web_server.ssl_processor.pop_certificate_changes.return_value = False
+    web_server._do_reload(forced=False)
+    assert web_server.nginx.update_config.call_args.kwargs["force"] is False
+
+    web_server.ssl_processor.pop_certificate_changes.return_value = True
+    web_server._do_reload(forced=False)
+    assert web_server.nginx.update_config.call_args.kwargs["force"] is True
+
+    web_server.ssl_processor.pop_certificate_changes.return_value = False
+    web_server._do_reload(forced=True)
+    assert web_server.nginx.update_config.call_args.kwargs["force"] is True