Fix gateway-route drift when a service is edited
Editing a service's routing (port, path_prefix, host) and then hitting "apply" or "gateway reload" — rather than a full deploy — left the running Caddyfile reflecting the old routing. The Caddyfile was generated from the derived registry.yaml snapshot, which only `deploy()` rebuilds from castle.yaml; apply/reload regenerated from that stale snapshot. Make castle.yaml the single source of truth for local routes: - caddyfile.py: new shared `service_proxy_targets()` derives a service's (proxy_path, proxy_host, port, base_url) from its spec. `compute_routes` now builds local routes from `config.services` when castle.yaml is loadable, falling back to `registry.deployed` only when it isn't. Since deploy/apply/reload all funnel through `compute_routes`, every regeneration path now tracks the spec. Mesh/remote and static-frontend routes unchanged. - deploy.py: `_build_deployed_service` uses the same shared deriver, so the written registry and the computed routes can't diverge. - config_editor.py: `/config/apply` now runs a full `deploy()` (rebuilding units + Caddyfile and reloading the gateway) before restarting services, so changed unit ExecStarts actually take effect. Scheduled jobs are no longer restarted on apply. Drops the now-unused direct caddy-reload path. - Regression tests: a stale registry's old port/path loses to castle.yaml; registry fallback still works when config is unavailable.
This commit is contained in:
@@ -3,7 +3,6 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import shutil
|
|
||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
from fastapi import APIRouter, HTTPException, status
|
from fastapi import APIRouter, HTTPException, status
|
||||||
@@ -323,14 +322,32 @@ def delete_job(name: str) -> dict:
|
|||||||
|
|
||||||
@router.post("/apply", response_model=ApplyResponse)
|
@router.post("/apply", response_model=ApplyResponse)
|
||||||
async def apply_config() -> ApplyResponse:
|
async def apply_config() -> ApplyResponse:
|
||||||
"""Apply config: restart managed services + regenerate and reload gateway."""
|
"""Apply config: rebuild runtime from castle.yaml, then restart services.
|
||||||
registry = get_registry()
|
|
||||||
|
Runs a full ``deploy`` so the registry, systemd units, and Caddyfile are all
|
||||||
|
regenerated from the current castle.yaml (and the gateway reloaded) — this is
|
||||||
|
what keeps the running config from drifting behind an edit. Then restarts the
|
||||||
|
managed services so the freshly written units take effect (``deploy`` only
|
||||||
|
daemon-reloads; a running unit keeps its old ExecStart until restarted).
|
||||||
|
Scheduled jobs are left alone — applying config shouldn't fire every job.
|
||||||
|
"""
|
||||||
|
from castle_core.deploy import deploy
|
||||||
|
|
||||||
actions: list[str] = []
|
actions: list[str] = []
|
||||||
errors: list[str] = []
|
errors: list[str] = []
|
||||||
|
|
||||||
# Restart managed services
|
# Rebuild registry + units + Caddyfile from castle.yaml off the event loop
|
||||||
|
# (deploy is blocking: it shells out to systemctl and the gateway).
|
||||||
|
try:
|
||||||
|
result = await asyncio.to_thread(deploy)
|
||||||
|
except Exception as e:
|
||||||
|
return ApplyResponse(ok=False, actions=actions, errors=[f"Deploy failed: {e}"])
|
||||||
|
actions.extend(result.messages)
|
||||||
|
|
||||||
|
# Restart managed services so the new units take effect (skip scheduled jobs).
|
||||||
|
registry = result.registry or get_registry()
|
||||||
for name, deployed in registry.deployed.items():
|
for name, deployed in registry.deployed.items():
|
||||||
if not deployed.managed:
|
if not deployed.managed or deployed.schedule:
|
||||||
continue
|
continue
|
||||||
unit = f"castle-{name}.service"
|
unit = f"castle-{name}.service"
|
||||||
ok, output = await _systemctl("restart", unit)
|
ok, output = await _systemctl("restart", unit)
|
||||||
@@ -339,24 +356,6 @@ async def apply_config() -> ApplyResponse:
|
|||||||
else:
|
else:
|
||||||
errors.append(f"Failed to restart {name}: {output}")
|
errors.append(f"Failed to restart {name}: {output}")
|
||||||
|
|
||||||
# Reload gateway
|
|
||||||
from castle_core.config import SPECS_DIR, ensure_dirs
|
|
||||||
from castle_core.generators.caddyfile import generate_caddyfile_from_registry
|
|
||||||
|
|
||||||
ensure_dirs()
|
|
||||||
caddyfile_path = SPECS_DIR / "Caddyfile"
|
|
||||||
caddyfile_path.write_text(generate_caddyfile_from_registry(registry))
|
|
||||||
actions.append("Generated Caddyfile")
|
|
||||||
|
|
||||||
if shutil.which("caddy"):
|
|
||||||
ok, output = await _run(
|
|
||||||
"caddy", "reload", "--config", str(caddyfile_path), "--adapter", "caddyfile"
|
|
||||||
)
|
|
||||||
if ok:
|
|
||||||
actions.append("Reloaded gateway")
|
|
||||||
else:
|
|
||||||
errors.append(f"Gateway reload failed: {output}")
|
|
||||||
|
|
||||||
await broadcast("config-changed", {"actions": actions})
|
await broadcast("config-changed", {"actions": actions})
|
||||||
return ApplyResponse(ok=len(errors) == 0, actions=actions, errors=errors)
|
return ApplyResponse(ok=len(errors) == 0, actions=actions, errors=errors)
|
||||||
|
|
||||||
@@ -372,13 +371,3 @@ async def _systemctl(action: str, unit: str) -> tuple[bool, str]:
|
|||||||
)
|
)
|
||||||
stdout, stderr = await proc.communicate()
|
stdout, stderr = await proc.communicate()
|
||||||
return proc.returncode == 0, (stdout or stderr or b"").decode().strip()
|
return proc.returncode == 0, (stdout or stderr or b"").decode().strip()
|
||||||
|
|
||||||
|
|
||||||
async def _run(*args: str) -> tuple[bool, str]:
|
|
||||||
proc = await asyncio.create_subprocess_exec(
|
|
||||||
*args,
|
|
||||||
stdout=asyncio.subprocess.PIPE,
|
|
||||||
stderr=asyncio.subprocess.PIPE,
|
|
||||||
)
|
|
||||||
stdout, stderr = await proc.communicate()
|
|
||||||
return proc.returncode == 0, (stdout or stderr or b"").decode().strip()
|
|
||||||
|
|||||||
@@ -22,7 +22,10 @@ from castle_core.config import (
|
|||||||
load_config,
|
load_config,
|
||||||
resolve_env_split,
|
resolve_env_split,
|
||||||
)
|
)
|
||||||
from castle_core.generators.caddyfile import generate_caddyfile_from_registry
|
from castle_core.generators.caddyfile import (
|
||||||
|
generate_caddyfile_from_registry,
|
||||||
|
service_proxy_targets,
|
||||||
|
)
|
||||||
from castle_core.generators.systemd import (
|
from castle_core.generators.systemd import (
|
||||||
SECRET_ENV_DIR,
|
SECRET_ENV_DIR,
|
||||||
generate_timer,
|
generate_timer,
|
||||||
@@ -221,10 +224,12 @@ def _build_deployed_service(
|
|||||||
if svc.manage and svc.manage.systemd and not svc.manage.systemd.enable:
|
if svc.manage and svc.manage.systemd and not svc.manage.systemd.enable:
|
||||||
managed = False
|
managed = False
|
||||||
|
|
||||||
port = None
|
# Routing fields (port/proxy_path/proxy_host/base_url) come from the shared
|
||||||
|
# deriver, so the registry written here and the Caddyfile computed from
|
||||||
|
# castle.yaml stay in lockstep.
|
||||||
|
proxy_path, proxy_host, port, base_url = service_proxy_targets(name, svc)
|
||||||
health_path = None
|
health_path = None
|
||||||
if svc.expose and svc.expose.http:
|
if svc.expose and svc.expose.http:
|
||||||
port = svc.expose.http.internal.port
|
|
||||||
health_path = svc.expose.http.health_path
|
health_path = svc.expose.http.health_path
|
||||||
|
|
||||||
# Env is exactly what's declared in defaults.env — no hidden convention
|
# Env is exactly what's declared in defaults.env — no hidden convention
|
||||||
|
|||||||
@@ -17,7 +17,8 @@ from __future__ import annotations
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from castle_core.config import SPECS_DIR
|
from castle_core.config import SPECS_DIR, CastleConfig
|
||||||
|
from castle_core.manifest import ServiceSpec
|
||||||
from castle_core.registry import NodeRegistry
|
from castle_core.registry import NodeRegistry
|
||||||
|
|
||||||
|
|
||||||
@@ -36,9 +37,60 @@ class GatewayRoute:
|
|||||||
return not self.address.startswith("/")
|
return not self.address.startswith("/")
|
||||||
|
|
||||||
|
|
||||||
|
# (proxy_path, proxy_host, port, base_url) for a service's gateway route(s).
|
||||||
|
ProxyTargets = tuple[str | None, str | None, int | None, str | None]
|
||||||
|
|
||||||
|
|
||||||
|
def service_proxy_targets(name: str, svc: ServiceSpec) -> ProxyTargets:
|
||||||
|
"""Derive a service's gateway routing fields from its spec.
|
||||||
|
|
||||||
|
The single source of truth shared by the registry build (``deploy``) and
|
||||||
|
route computation (``compute_routes``), so the deployed registry and a
|
||||||
|
freshly regenerated Caddyfile can never disagree about ports/paths/hosts.
|
||||||
|
"""
|
||||||
|
port = None
|
||||||
|
if svc.expose and svc.expose.http:
|
||||||
|
port = svc.expose.http.internal.port
|
||||||
|
|
||||||
|
proxy_path = None
|
||||||
|
proxy_host = None
|
||||||
|
if svc.proxy and svc.proxy.caddy and svc.proxy.caddy.enable:
|
||||||
|
caddy = svc.proxy.caddy
|
||||||
|
proxy_host = caddy.host
|
||||||
|
if caddy.path_prefix:
|
||||||
|
proxy_path = caddy.path_prefix
|
||||||
|
elif not caddy.host:
|
||||||
|
# No explicit path and no host → default to /<name>.
|
||||||
|
proxy_path = f"/{name}"
|
||||||
|
|
||||||
|
base_url = getattr(svc.run, "base_url", None)
|
||||||
|
return proxy_path, proxy_host, port, base_url
|
||||||
|
|
||||||
|
|
||||||
|
def _local_proxy_targets(
|
||||||
|
config: CastleConfig | None, registry: NodeRegistry
|
||||||
|
) -> list[tuple[str, ProxyTargets]]:
|
||||||
|
"""Local services' routing fields, name-sorted for deterministic output.
|
||||||
|
|
||||||
|
Prefers ``castle.yaml`` (``config.services``) as the source of truth so a
|
||||||
|
regenerated Caddyfile always reflects the current spec — this is what closes
|
||||||
|
the registry-staleness drift. Falls back to the deployed registry snapshot
|
||||||
|
only when config isn't available (load failed, or a pure-registry context).
|
||||||
|
"""
|
||||||
|
services = getattr(config, "services", None)
|
||||||
|
if services is not None:
|
||||||
|
return sorted(
|
||||||
|
(name, service_proxy_targets(name, svc)) for name, svc in services.items()
|
||||||
|
)
|
||||||
|
return sorted(
|
||||||
|
(name, (d.proxy_path, d.proxy_host, d.port, d.base_url))
|
||||||
|
for name, d in registry.deployed.items()
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def compute_routes(
|
def compute_routes(
|
||||||
registry: NodeRegistry,
|
registry: NodeRegistry,
|
||||||
config: object | None = None,
|
config: CastleConfig | None = None,
|
||||||
remote_registries: dict[str, NodeRegistry] | None = None,
|
remote_registries: dict[str, NodeRegistry] | None = None,
|
||||||
) -> list[GatewayRoute]:
|
) -> list[GatewayRoute]:
|
||||||
"""Build the full ordered list of gateway routes (host, proxy, remote, static).
|
"""Build the full ordered list of gateway routes (host, proxy, remote, static).
|
||||||
@@ -57,18 +109,22 @@ def compute_routes(
|
|||||||
routes: list[GatewayRoute] = []
|
routes: list[GatewayRoute] = []
|
||||||
local_paths: set[str] = set()
|
local_paths: set[str] = set()
|
||||||
|
|
||||||
|
# Local proxy routes, derived from castle.yaml when available (else the
|
||||||
|
# deployed registry) so a regenerated Caddyfile tracks the current spec.
|
||||||
|
local = _local_proxy_targets(config, registry)
|
||||||
|
|
||||||
# Host-based proxy routes (whole host → backend root).
|
# Host-based proxy routes (whole host → backend root).
|
||||||
for name, d in registry.deployed.items():
|
for name, (proxy_path, proxy_host, port, base_url) in local:
|
||||||
if d.proxy_host and (d.port or d.base_url):
|
if proxy_host and (port or base_url):
|
||||||
target = d.base_url or f"localhost:{d.port}"
|
target = base_url or f"localhost:{port}"
|
||||||
routes.append(GatewayRoute(d.proxy_host, "proxy", target, name, node))
|
routes.append(GatewayRoute(proxy_host, "proxy", target, name, node))
|
||||||
|
|
||||||
# Path-prefix proxy routes.
|
# Path-prefix proxy routes.
|
||||||
for name, d in registry.deployed.items():
|
for name, (proxy_path, proxy_host, port, base_url) in local:
|
||||||
if d.proxy_path and (d.port or d.base_url):
|
if proxy_path and (port or base_url):
|
||||||
local_paths.add(d.proxy_path)
|
local_paths.add(proxy_path)
|
||||||
target = d.base_url or f"localhost:{d.port}"
|
target = base_url or f"localhost:{port}"
|
||||||
routes.append(GatewayRoute(d.proxy_path, "proxy", target, name, node))
|
routes.append(GatewayRoute(proxy_path, "proxy", target, name, node))
|
||||||
|
|
||||||
# Cross-node routes — local paths take precedence.
|
# Cross-node routes — local paths take precedence.
|
||||||
if remote_registries:
|
if remote_registries:
|
||||||
|
|||||||
@@ -5,7 +5,20 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from castle_core.generators.caddyfile import generate_caddyfile_from_registry
|
from castle_core.config import CastleConfig, GatewayConfig
|
||||||
|
from castle_core.generators.caddyfile import (
|
||||||
|
compute_routes,
|
||||||
|
generate_caddyfile_from_registry,
|
||||||
|
)
|
||||||
|
from castle_core.manifest import (
|
||||||
|
CaddySpec,
|
||||||
|
ExposeSpec,
|
||||||
|
HttpExposeSpec,
|
||||||
|
HttpInternal,
|
||||||
|
ProxySpec,
|
||||||
|
RunPython,
|
||||||
|
ServiceSpec,
|
||||||
|
)
|
||||||
from castle_core.registry import Deployment, NodeConfig, NodeRegistry
|
from castle_core.registry import Deployment, NodeConfig, NodeRegistry
|
||||||
|
|
||||||
|
|
||||||
@@ -145,6 +158,71 @@ class TestCaddyfileFromRegistry:
|
|||||||
assert "reverse_proxy localhost:9002" in caddyfile
|
assert "reverse_proxy localhost:9002" in caddyfile
|
||||||
|
|
||||||
|
|
||||||
|
def _service(port: int, path: str | None = None, host: str | None = None) -> ServiceSpec:
|
||||||
|
"""A minimal python ServiceSpec with an HTTP port and a caddy proxy route."""
|
||||||
|
caddy = CaddySpec(path_prefix=path, host=host)
|
||||||
|
return ServiceSpec(
|
||||||
|
run=RunPython(runner="python", program="svc"),
|
||||||
|
expose=ExposeSpec(http=HttpExposeSpec(internal=HttpInternal(port=port))),
|
||||||
|
proxy=ProxySpec(caddy=caddy),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _config(services: dict[str, ServiceSpec]) -> CastleConfig:
|
||||||
|
return CastleConfig(
|
||||||
|
root=None, # type: ignore[arg-type]
|
||||||
|
gateway=GatewayConfig(port=9000),
|
||||||
|
repo=None,
|
||||||
|
programs={},
|
||||||
|
services=services,
|
||||||
|
jobs={},
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestLocalRoutesFromConfig:
|
||||||
|
"""castle.yaml is the source of truth for local routes — a regenerated
|
||||||
|
Caddyfile must track the spec even when the deployed registry is stale.
|
||||||
|
This is the regression guard for the service-edit drift."""
|
||||||
|
|
||||||
|
def test_config_port_overrides_stale_registry(self) -> None:
|
||||||
|
# Registry was deployed with the OLD port; castle.yaml now says 8002.
|
||||||
|
registry = _make_registry(
|
||||||
|
deployed={
|
||||||
|
"app": Deployment(
|
||||||
|
runner="python", run_cmd=["app"], port=8001, proxy_path="/app"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
config = _config({"app": _service(port=8002, path="/app")})
|
||||||
|
routes = compute_routes(registry, config)
|
||||||
|
targets = {r.target for r in routes if r.address == "/app"}
|
||||||
|
assert targets == {"localhost:8002"} # config wins, not the stale 8001
|
||||||
|
|
||||||
|
def test_config_path_overrides_stale_registry(self) -> None:
|
||||||
|
registry = _make_registry(
|
||||||
|
deployed={
|
||||||
|
"app": Deployment(
|
||||||
|
runner="python", run_cmd=["app"], port=8001, proxy_path="/old"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
config = _config({"app": _service(port=8001, path="/new")})
|
||||||
|
addrs = {r.address for r in compute_routes(registry, config) if r.kind == "proxy"}
|
||||||
|
assert addrs == {"/new"}
|
||||||
|
|
||||||
|
def test_falls_back_to_registry_without_config(self) -> None:
|
||||||
|
# No config available (load_config is isolated to raise) → use registry.
|
||||||
|
registry = _make_registry(
|
||||||
|
deployed={
|
||||||
|
"app": Deployment(
|
||||||
|
runner="python", run_cmd=["app"], port=8001, proxy_path="/app"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
caddyfile = generate_caddyfile_from_registry(registry)
|
||||||
|
assert "reverse_proxy localhost:8001" in caddyfile
|
||||||
|
|
||||||
|
|
||||||
class TestCaddyfileRemoteRegistries:
|
class TestCaddyfileRemoteRegistries:
|
||||||
"""Tests for cross-node routing in Caddyfile."""
|
"""Tests for cross-node routing in Caddyfile."""
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user