diff --git a/src/dstack/_internal/proxy/gateway/app.py b/src/dstack/_internal/proxy/gateway/app.py index 6627e9e3f..7bd690eab 100644 --- a/src/dstack/_internal/proxy/gateway/app.py +++ b/src/dstack/_internal/proxy/gateway/app.py @@ -1,6 +1,8 @@ """FastAPI app running on a gateway.""" -from contextlib import asynccontextmanager +import asyncio +import random +from contextlib import asynccontextmanager, suppress from pathlib import Path from typing import Optional @@ -30,11 +32,16 @@ from dstack._internal.proxy.gateway.services.stats import StatsCollector from dstack._internal.proxy.lib.routers.model_proxy import router as model_proxy_router from dstack._internal.utils.common import run_async +from dstack._internal.utils.logging import get_logger from dstack.version import __version__ STATE_FILE = DSTACK_DIR_ON_GATEWAY / "state-v2.json" LEGACY_STATE_FILE = DSTACK_DIR_ON_GATEWAY / "state.json" LEGACY_KEYS_DIR = Path("~/.ssh/projects").expanduser().resolve() +# `certbot renew` only renews certificates close to expiry. Twice a day matches the +# schedule of the timer that certbot's own packages install. +CERT_RENEWAL_INTERVAL = 12 * 3600 +logger = get_logger(__name__) @asynccontextmanager @@ -45,12 +52,29 @@ async def lifespan(app: FastAPI): service_conn_pool = await injector.get_service_connection_pool() await run_async(nginx.write_global_conf) await apply_all(repo, nginx, service_conn_pool) + renewal_task = asyncio.create_task(renew_certificates_periodically(nginx)) yield + renewal_task.cancel() + with suppress(asyncio.CancelledError): + await renewal_task await service_conn_pool.remove_all() +async def renew_certificates_periodically(nginx: Nginx) -> None: + # Gateways on some backends, e.g. Kubernetes, have no system timer that runs + # `certbot renew`, so the gateway renews its certificates itself. The random + # first delay spreads renewals of many gateways over the interval. + await asyncio.sleep(random.uniform(0, CERT_RENEWAL_INTERVAL)) + while True: + try: + await nginx.renew_certificates() + except Exception as e: + logger.warning("Failed to renew TLS certificates: %r", e) + await asyncio.sleep(CERT_RENEWAL_INTERVAL) + + def make_app(repo: Optional[GatewayProxyRepo] = None, nginx: Optional[Nginx] = None) -> FastAPI: if repo is None: migrate_from_state_v1( diff --git a/src/dstack/_internal/proxy/gateway/services/nginx.py b/src/dstack/_internal/proxy/gateway/services/nginx.py index df71594f5..9a76a5502 100644 --- a/src/dstack/_internal/proxy/gateway/services/nginx.py +++ b/src/dstack/_internal/proxy/gateway/services/nginx.py @@ -18,6 +18,7 @@ CERTBOT_TIMEOUT = 40 CERTBOT_2ND_TIMEOUT = 5 +CERTBOT_RENEW_TIMEOUT = 600 NGINX_DIR = Path("/etc/nginx") logger = get_logger(__name__) @@ -109,6 +110,29 @@ async def unregister(self, service: models.Service) -> None: await run_async(self.reload) logger.info("Unregistered domain %s", domain) + async def renew_certificates(self) -> None: + """Renew the certificates that are due.""" + # Holding the lock keeps this certbot run from colliding with `run_certbot` + async with self._lock: + await run_async(self.run_certbot_renew) + + @staticmethod + def run_certbot_renew() -> None: + # Certbot renews each certificate with the ACME server that issued it. + # The deploy hook reloads nginx only if some certificate was renewed. + cmd = ["sudo", "timeout", "--kill-after", str(CERTBOT_2ND_TIMEOUT)] + cmd += [str(CERTBOT_RENEW_TIMEOUT), "certbot", "renew"] + cmd += ["--non-interactive", "--quiet", "--no-random-sleep-on-renew"] + cmd += ["--deploy-hook", "systemctl reload nginx.service"] + r = subprocess.run( + cmd, + capture_output=True, + timeout=CERTBOT_RENEW_TIMEOUT + CERTBOT_2ND_TIMEOUT + 1, # shouldn't happen + ) + if r.returncode != 0: + msg = r.stderr.decode().strip().replace("\n", " ") + raise UnexpectedProxyError(f"Error renewing TLS certificates: {msg}") + @staticmethod def reload() -> None: cmd = ["sudo", "systemctl", "reload", "nginx.service"] diff --git a/src/tests/_internal/proxy/gateway/services/test_nginx.py b/src/tests/_internal/proxy/gateway/services/test_nginx.py new file mode 100644 index 000000000..85abe3a85 --- /dev/null +++ b/src/tests/_internal/proxy/gateway/services/test_nginx.py @@ -0,0 +1,40 @@ +import asyncio +from pathlib import Path +from unittest.mock import Mock, patch + +import pytest + +from dstack._internal.proxy.gateway.services.nginx import Nginx +from dstack._internal.proxy.lib.errors import UnexpectedProxyError + +SUBPROCESS_RUN = "dstack._internal.proxy.gateway.services.nginx.subprocess.run" + + +class TestRunCertbotRenew: + def test_renews_and_reloads_nginx_on_deploy(self) -> None: + with patch(SUBPROCESS_RUN) as run: + run.return_value = Mock(returncode=0, stderr=b"") + Nginx.run_certbot_renew() + cmd = run.call_args.args[0] + assert cmd[0] == "sudo" + assert cmd[cmd.index("certbot") + 1] == "renew" + assert "--non-interactive" in cmd + assert cmd[cmd.index("--deploy-hook") + 1] == "systemctl reload nginx.service" + + def test_raises_on_failure(self) -> None: + with patch(SUBPROCESS_RUN) as run: + run.return_value = Mock(returncode=1, stderr=b"Failed to renew\nexample.com\n") + with pytest.raises(UnexpectedProxyError, match="Failed to renew example.com"): + Nginx.run_certbot_renew() + + +@pytest.mark.asyncio +async def test_renew_certificates_waits_for_registration(tmp_path: Path) -> None: + nginx = Nginx(nginx_dir=tmp_path) + with patch.object(Nginx, "run_certbot_renew") as run_certbot_renew: + async with nginx._lock: # e.g. a service registration is running certbot + task = asyncio.create_task(nginx.renew_certificates()) + await asyncio.sleep(0.1) + run_certbot_renew.assert_not_called() + await task + run_certbot_renew.assert_called_once() diff --git a/src/tests/_internal/proxy/gateway/test_app.py b/src/tests/_internal/proxy/gateway/test_app.py index 335be84fa..685586731 100644 --- a/src/tests/_internal/proxy/gateway/test_app.py +++ b/src/tests/_internal/proxy/gateway/test_app.py @@ -1,8 +1,14 @@ +import asyncio from pathlib import Path +from unittest.mock import AsyncMock, Mock, patch import pytest -from dstack._internal.proxy.gateway.app import lifespan, make_app +from dstack._internal.proxy.gateway.app import ( + lifespan, + make_app, + renew_certificates_periodically, +) from dstack._internal.proxy.gateway.models import ModelEntrypoint from dstack._internal.proxy.gateway.repo.repo import GatewayProxyRepo from dstack._internal.proxy.gateway.services.nginx import Nginx @@ -32,3 +38,14 @@ async def test_lifespan(tmp_path: Path, system_mocks: Mocks) -> None: assert system_mocks.open_conn.call_count == 1 assert system_mocks.close_conn.call_count == 0 assert system_mocks.close_conn.call_count == 1 + + +@pytest.mark.asyncio +async def test_renews_certificates_periodically_despite_errors() -> None: + nginx = Mock(renew_certificates=AsyncMock(side_effect=[RuntimeError("certbot failed"), None])) + # The first delay, then one after each of the two renewals; stop at the third + sleep = AsyncMock(side_effect=[None, None, asyncio.CancelledError()]) + with patch("dstack._internal.proxy.gateway.app.asyncio.sleep", sleep): + with pytest.raises(asyncio.CancelledError): + await renew_certificates_periodically(nginx) + assert nginx.renew_certificates.await_count == 2