diff --git a/decnet/fleet/__init__.py b/decnet/fleet/__init__.py index f41dbeee..f0252e2e 100644 --- a/decnet/fleet/__init__.py +++ b/decnet/fleet/__init__.py @@ -104,8 +104,14 @@ def build_deckies_from_ini( host_ip: str, randomize: bool, cli_mutate_interval: int | None = None, + reserved_ips: set[str] | None = None, ) -> list[DeckyConfig]: - """Build DeckyConfig list from an IniConfig, auto-allocating missing IPs.""" + """Build DeckyConfig list from an IniConfig, auto-allocating missing IPs. + + *reserved_ips* lets the additive deploy path pass the IPs of the + already-deployed fleet so auto-allocation skips them instead of + handing out a colliding address. + """ from ipaddress import IPv4Address, IPv4Network import time now = time.time() @@ -121,6 +127,8 @@ def build_deckies_from_ini( IPv4Address(gateway), IPv4Address(host_ip), } | explicit_ips + if reserved_ips: + reserved |= {IPv4Address(ip) for ip in reserved_ips} auto_pool = (str(addr) for addr in net.hosts() if addr not in reserved) diff --git a/decnet/web/db/models/deploy.py b/decnet/web/db/models/deploy.py index 423baec9..a194c148 100644 --- a/decnet/web/db/models/deploy.py +++ b/decnet/web/db/models/deploy.py @@ -17,6 +17,15 @@ class DeployIniRequest(BaseModel): # This field now enforces strict INI structure during Pydantic initialization. # The OpenAPI schema correctly shows it as a required string. ini_content: IniContent = PydanticField(..., description="A valid INI formatted string") + replace_fleet: bool = PydanticField( + default=False, + description=( + "If true, the INI is treated as the complete desired fleet — any " + "existing decky absent from the INI is torn down. Default false: " + "the INI is appended to the existing fleet; name or IP collisions " + "with already-deployed deckies yield 409." + ), + ) class DeployResponse(BaseModel): diff --git a/decnet/web/router/fleet/api_deploy_deckies.py b/decnet/web/router/fleet/api_deploy_deckies.py index 24a14ce3..05c80619 100644 --- a/decnet/web/router/fleet/api_deploy_deckies.py +++ b/decnet/web/router/fleet/api_deploy_deckies.py @@ -76,9 +76,21 @@ async def api_deploy_deckies(req: DeployIniRequest, admin: dict = Depends(requir "Add a [general] section with interface=, net=, and gw= to the INI." ) + # Snapshot the existing fleet (from prior state, NOT the freshly-built + # config below) so additive collision checks compare new against prior + # rather than against themselves. Existing IPs are passed into + # build_deckies_from_ini as reserved so auto-allocation skips them. + existing_deckies = list(config.deckies) if config is not None else [] + reserved_ips: set[str] | None = ( + {d.ip for d in existing_deckies if d.ip} + if not req.replace_fleet and existing_deckies + else None + ) + try: new_decky_configs = build_deckies_from_ini( - ini, subnet_cidr, gateway, host_ip, False, cli_mutate_interval=None + ini, subnet_cidr, gateway, host_ip, False, + cli_mutate_interval=None, reserved_ips=reserved_ips, ) except ValueError as e: log.debug("deploy: build_deckies_from_ini rejected input: %s", e) @@ -96,12 +108,40 @@ async def api_deploy_deckies(req: DeployIniRequest, admin: dict = Depends(requir mutate_interval=ini.mutate_interval or DEFAULT_MUTATE_INTERVAL, ) - # The INI is the source of truth for *which* deckies exist this deploy. - # The old "merge with prior state" behaviour meant submitting `[decky1]` - # after a 3-decky run silently redeployed decky2/decky3 too — and then - # collided on their stale IPs ("Address already in use"). Full replace - # matches what the operator sees in the submitted config. - config.deckies = list(new_decky_configs) + # Two intents collapse onto one endpoint: + # + # * replace_fleet=True (explicit): INI is the complete desired fleet. + # Anything absent is torn down by the reconciler. This is the path + # for set-desired-state callers (CLI, declarative tooling). + # * replace_fleet=False (default, wizard path): INI is appended to the + # existing fleet. The wizard only POSTs the new decky and would + # otherwise see prior deckies silently deleted by the reconciler. + # + # The historical "always full replace" behaviour was added to dodge + # stale-IP collisions on redeploy ("Address already in use"); that's + # now scoped to replace_fleet=True. + if req.replace_fleet: + config.deckies = list(new_decky_configs) + else: + existing_names = {d.name for d in existing_deckies} + existing_ips = {d.ip for d in existing_deckies if d.ip} + for d in new_decky_configs: + if d.name in existing_names: + raise HTTPException( + status_code=409, + detail=( + f"decky '{d.name}' already exists; " + "submit with replace_fleet=true to overwrite the fleet" + ), + ) + if d.ip and d.ip in existing_ips: + raise HTTPException( + status_code=409, + detail=( + f"IP {d.ip} is already in use by an existing decky" + ), + ) + config.deckies = existing_deckies + list(new_decky_configs) limits_state = await repo.get_state("config_limits") deployment_limit = limits_state.get("deployment_limit", 10) if limits_state else 10 @@ -152,8 +192,16 @@ async def api_deploy_deckies(req: DeployIniRequest, admin: dict = Depends(requir } await repo.set_state("deployment", new_state_payload) + # Lifecycle rows track THIS call's deployments only. In additive mode + # the existing deckies are already running and don't get a new + # lifecycle row — the caller polls /deckies/lifecycle for just the + # deckies they submitted. In replace mode every decky in the new + # config is being (re)deployed and gets a row. + new_names = {d.name for d in new_decky_configs} lifecycle_ids: dict[str, str] = {} for d in config.deckies: + if not req.replace_fleet and d.name not in new_names: + continue lid = await repo.create_lifecycle({ "decky_name": d.name, "host_uuid": d.host_uuid, @@ -174,7 +222,7 @@ async def api_deploy_deckies(req: DeployIniRequest, admin: dict = Depends(requir return { "message": ( - f"Deploy accepted ({len(config.deckies)} decky/ies, mode={mode}). " + f"Deploy accepted ({len(lifecycle_ids)} decky/ies, mode={mode}). " f"Poll /deckies/lifecycle?ids=... for completion." ), "mode": mode, diff --git a/decnet_web/src/components/DeckyFleet/DeployWizard.tsx b/decnet_web/src/components/DeckyFleet/DeployWizard.tsx index 6ec8ded6..03ccb9e2 100644 --- a/decnet_web/src/components/DeckyFleet/DeployWizard.tsx +++ b/decnet_web/src/components/DeckyFleet/DeployWizard.tsx @@ -236,7 +236,7 @@ export const DeployWizard: React.FC = ({ try { const res = await api.post<{ lifecycle_ids?: string[]; message?: string; mode?: string }>( '/deckies/deploy', - { ini_content: ini }, + { ini_content: ini, replace_fleet: false }, ); const ids = res.data?.lifecycle_ids ?? []; setLifecycleIds(ids); diff --git a/tests/api/fleet/test_deploy_additive.py b/tests/api/fleet/test_deploy_additive.py new file mode 100644 index 00000000..17e7292d --- /dev/null +++ b/tests/api/fleet/test_deploy_additive.py @@ -0,0 +1,162 @@ +"""POST /deckies/deploy additive vs replace_fleet semantics. + +Default behaviour (replace_fleet=False) appends the INI to the existing +fleet so the wizard's "deploy one more decky" submit no longer wipes +prior deckies. replace_fleet=True preserves the historical +set-desired-state semantics for CLI / declarative callers. +""" +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from decnet.web.dependencies import repo + + +@pytest.fixture(autouse=True) +def contract_test_mode(monkeypatch): + monkeypatch.setenv("DECNET_CONTRACT_TEST", "true") + + +@pytest.fixture(autouse=True) +def mock_network(): + with patch("decnet.web.router.fleet.api_deploy_deckies.get_host_ip", return_value="192.168.1.100"): + with patch("decnet.web.router.fleet.api_deploy_deckies.detect_interface", return_value="eth0"): + with patch("decnet.web.router.fleet.api_deploy_deckies.detect_subnet", return_value=("192.168.1.0/24", "192.168.1.1")): + yield + + +@pytest.fixture(autouse=True) +async def _isolate_state(): + for row in await repo.list_swarm_hosts(): + await repo.delete_swarm_host(row["uuid"]) + await repo.set_state("deployment", None) + yield + await repo.set_state("deployment", None) + + +@pytest.mark.anyio +async def test_additive_default_appends_to_existing_fleet(client, auth_token, monkeypatch): + """Two sequential deploys with replace_fleet unset → both deckies in state.""" + monkeypatch.setenv("DECNET_MODE", "master") + + r1 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-01]\nservices = ssh\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r1.status_code == 202, r1.text + + r2 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-02]\nservices = http\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r2.status_code == 202, r2.text + + committed = await repo.get_state("deployment") + assert committed is not None + names = {d["name"] for d in committed["config"]["deckies"]} + assert names == {"decky-01", "decky-02"} + + +@pytest.mark.anyio +async def test_additive_name_collision_returns_409(client, auth_token, monkeypatch): + """Re-submitting an existing decky name without replace_fleet → 409.""" + monkeypatch.setenv("DECNET_MODE", "master") + + r1 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-01]\nservices = ssh\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r1.status_code == 202, r1.text + + r2 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-01]\nservices = http\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r2.status_code == 409, r2.text + assert "decky-01" in r2.json()["detail"] + assert "replace_fleet" in r2.json()["detail"] + + +@pytest.mark.anyio +async def test_additive_ip_collision_returns_409(client, auth_token, monkeypatch): + """A new decky pinned to an IP already in use → 409 with the IP.""" + monkeypatch.setenv("DECNET_MODE", "master") + + r1 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-01]\nservices = ssh\nip = 192.168.1.50\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r1.status_code == 202, r1.text + + r2 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-02]\nservices = http\nip = 192.168.1.50\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r2.status_code == 409, r2.text + assert "192.168.1.50" in r2.json()["detail"] + + +@pytest.mark.anyio +async def test_replace_fleet_true_overwrites_existing(client, auth_token, monkeypatch): + """replace_fleet=True preserves the historical full-replace semantics.""" + monkeypatch.setenv("DECNET_MODE", "master") + + r1 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-01]\nservices = ssh\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r1.status_code == 202, r1.text + + r2 = await client.post( + "/api/v1/deckies/deploy", + json={ + "ini_content": "[decky-02]\nservices = http\n", + "replace_fleet": True, + }, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r2.status_code == 202, r2.text + + committed = await repo.get_state("deployment") + assert committed is not None + names = {d["name"] for d in committed["config"]["deckies"]} + assert names == {"decky-02"} + + +@pytest.mark.anyio +async def test_additive_lifecycle_ids_scoped_to_new_deckies(client, auth_token, monkeypatch): + """In additive mode the response's lifecycle_ids cover only the deckies + the caller submitted, not carryover. Operators polling + /deckies/lifecycle?ids=... see exactly what this call deployed.""" + monkeypatch.setenv("DECNET_MODE", "master") + + r1 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-01]\nservices = ssh\n[decky-02]\nservices = http\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r1.status_code == 202, r1.text + assert len(r1.json()["lifecycle_ids"]) == 2 + + r2 = await client.post( + "/api/v1/deckies/deploy", + json={"ini_content": "[decky-03]\nservices = ssh\n"}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert r2.status_code == 202, r2.text + body2 = r2.json() + assert len(body2["lifecycle_ids"]) == 1 + + committed = await repo.get_state("deployment") + assert committed is not None + names = {d["name"] for d in committed["config"]["deckies"]} + assert names == {"decky-01", "decky-02", "decky-03"}