fix(proxmox): make host registration idempotent on name

alembic/versions/0002_proxmox_host_unique_name.py

@@ -0,0 +1,70 @@
+"""dedupe proxmox_hosts, then enforce a unique name
+
+Until now nothing stopped a second host row under an existing name, and the
+deploy bundle re-POSTs the same name on every scenario run — so field
+databases carry one duplicate per re-run. The duplicates are not inert:
+``deployments.target_host_id`` points at whichever one existed when the
+deployment was created.
+
+Collapsing keys on ``added_at``: the earliest row under a name is the one the
+first deploy registered, so it is the row most likely to be referenced and the
+one whose id callers may have recorded. Deployments on the later duplicates are
+repointed at it before those rows are deleted, so nothing is left dangling.
+
+Revision ID: 0002_proxmox_host_unique_name
+Revises: 0001_v1_initial
+Create Date: 2026-08-10
+"""
+import logging
+
+import sqlalchemy as sa
+from alembic import op
+
+revision = '0002_proxmox_host_unique_name'
+down_revision = '0001_v1_initial'
+branch_labels = None
+depends_on = None
+
+log = logging.getLogger("alembic.runtime.migration")
+
+
+def upgrade() -> None:
+    conn = op.get_bind()
+
+    # added_at first, id as a deterministic tie-break for rows registered
+    # within the same clock tick.
+    rows = conn.execute(
+        sa.text("SELECT id, name FROM proxmox_hosts ORDER BY name, added_at, id")
+    ).fetchall()
+
+    keepers: dict[str, str] = {}
+    for host_id, name in rows:
+        keeper_id = keepers.setdefault(name, host_id)
+        if keeper_id == host_id:
+            continue
+
+        moved = conn.execute(
+            sa.text(
+                "UPDATE deployments SET target_host_id = :keeper"
+                " WHERE target_host_id = :loser"
+            ),
+            {"keeper": keeper_id, "loser": host_id},
+        ).rowcount
+        conn.execute(
+            sa.text("DELETE FROM proxmox_hosts WHERE id = :loser"),
+            {"loser": host_id},
+        )
+        log.info(
+            "proxmox_hosts: collapsed duplicate %r (%s) into %s, "
+            "repointed %d deployment(s)",
+            name, host_id, keeper_id, moved,
+        )
+
+    with op.batch_alter_table("proxmox_hosts") as batch:
+        batch.create_unique_constraint("uq_proxmox_host_name", ["name"])
+
+
+def downgrade() -> None:
+    # Only the constraint is reversible; the collapsed rows are gone for good.
+    with op.batch_alter_table("proxmox_hosts") as batch:
+        batch.drop_constraint("uq_proxmox_host_name", type_="unique")

app/core/models.py

@@ -57,6 +57,11 @@ class ProxmoxHost(Base):
     protected_vmids_override_json: Mapped[str | None] = mapped_column(Text)
     added_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=_utcnow)
     last_health_check_json: Mapped[str | None] = mapped_column(Text)
+    # A host is identified by its name: the deploy bundle re-POSTs the same
+    # name on every scenario run, and deployments.target_host_id is a FK here,
+    # so a second row per re-run would strand earlier deployments on a host
+    # nobody updates. Uniqueness turns those re-runs into in-place updates.
+    __table_args__ = (UniqueConstraint("name", name="uq_proxmox_host_name"),)
 
 
 class Project(Base):

app/routes/v1/proxmox/hosts.py

@@ -7,7 +7,7 @@
 from datetime import datetime, timezone
 
 import httpx
-from fastapi import APIRouter, Depends, status
+from fastapi import APIRouter, Depends, Response, status
 from sqlalchemy import select
 from sqlalchemy.ext.asyncio import AsyncSession
 
@@ -68,11 +68,57 @@ async def list_hosts(
 
 
 @router.post(
-    "/hosts", response_model=HostOut, status_code=status.HTTP_201_CREATED
+    "/hosts",
+    response_model=HostOut,
+    status_code=status.HTTP_201_CREATED,
+    responses={
+        200: {
+            "model": HostOut,
+            "description": "Host already registered under this name; updated in place.",
+        }
+    },
 )
 async def create_host(
-    payload: HostIn, session: AsyncSession = Depends(_session)
+    payload: HostIn,
+    response: Response,
+    session: AsyncSession = Depends(_session),
 ):
+    """Register a Proxmox host, or refresh the one already under that name.
+
+    The deploy bundle POSTs this on every scenario run, so it has to be
+    idempotent. Re-registering keeps the existing row's id — ``deployments``
+    reference it by FK — and returns 200 instead of 201. Credentials are part
+    of what gets refreshed: a rotated PVE token reaches the backend on the next
+    deploy rather than leaving it authenticating with a stale one.
+    """
+    overrides_json = (
+        json.dumps(payload.protected_vmids_override)
+        if payload.protected_vmids_override
+        else None
+    )
+
+    existing = (
+        await session.execute(
+            select(ProxmoxHost).where(ProxmoxHost.name == payload.name)
+        )
+    ).scalar_one_or_none()
+    if existing is not None:
+        existing.api_url = str(payload.api_url)
+        existing.node_name = payload.node_name
+        existing.token_ref = payload.token_ref
+        existing.token_scope = payload.token_scope
+        existing.default_bridge = payload.default_bridge
+        existing.protected_vmids_override_json = overrides_json
+        # added_at deliberately untouched: it records when this host was first
+        # registered, and the dedupe migration keys on it.
+        await session.commit()
+        await session.refresh(existing)
+        log.info(
+            "proxmox_host_reregistered", host_id=existing.id, name=existing.name
+        )
+        response.status_code = status.HTTP_200_OK
+        return _row_to_out(existing)
+
     row = ProxmoxHost(
         id=uuid.uuid4().hex[:16],
         name=payload.name,
@@ -81,11 +127,7 @@ async def create_host(
         token_ref=payload.token_ref,
         token_scope=payload.token_scope,
         default_bridge=payload.default_bridge,
-        protected_vmids_override_json=(
-            json.dumps(payload.protected_vmids_override)
-            if payload.protected_vmids_override
-            else None
-        ),
+        protected_vmids_override_json=overrides_json,
     )
     session.add(row)
     await session.commit()

openapi.json

@@ -3507,6 +3507,7 @@
           "v1 proxmox"
         ],
         "summary": "Create Host",
+        "description": "Register a Proxmox host, or refresh the one already under that name.\n\nThe deploy bundle POSTs this on every scenario run, so it has to be\nidempotent. Re-registering keeps the existing row's id \u2014 ``deployments``\nreference it by FK \u2014 and returns 200 instead of 201. Credentials are part\nof what gets refreshed: a rotated PVE token reaches the backend on the next\ndeploy rather than leaving it authenticating with a stale one.",
         "operationId": "create_host_v1_proxmox_hosts_post",
         "requestBody": {
           "required": true,
@@ -3538,6 +3539,16 @@
                 }
               }
             }
+          },
+          "200": {
+            "description": "Host already registered under this name; updated in place.",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/HostOut"
+                }
+              }
+            }
           }
         }
       }

tests/routes/test_proxmox_hosts.py

@@ -120,3 +120,122 @@ async def test_health_unreachable_host_returns_unreachable_status(
             assert body["status"] == "unreachable"
     finally:
         await dbmod.dispose_engine()
+
+
+@pytest.mark.asyncio
+async def test_reseeding_the_same_host_updates_it_in_place(tmp_path, monkeypatch):
+    """A scenario re-run POSTs the same name; it must not pile up duplicates.
+
+    The id has to survive: deployments.target_host_id is a FK to it, so a new
+    row per re-run would strand every earlier deployment on a stale host.
+    """
+    app, dbmod = await _boot(tmp_path, monkeypatch)
+    try:
+        async with AsyncClient(
+            transport=ASGITransport(app=app), base_url="http://t"
+        ) as c:
+            first = await c.post(
+                "/v1/proxmox/hosts",
+                json={
+                    "name": "pve01",
+                    "api_url": "https://pve01:8006",
+                    "node_name": "pve01",
+                    "token_ref": "r42@pam!tok=abc",
+                },
+            )
+            assert first.status_code == 201, first.text
+
+            second = await c.post(
+                "/v1/proxmox/hosts",
+                json={
+                    "name": "pve01",
+                    "api_url": "https://pve01.lan:8006",
+                    "node_name": "pve-node-2",
+                    "token_ref": "r42@pam!tok=rotated",
+                },
+            )
+            assert second.status_code == 200, second.text
+            assert second.json()["id"] == first.json()["id"]
+            # pydantic HttpUrl normalises a bare-host URL with a trailing slash
+            assert second.json()["api_url"] == "https://pve01.lan:8006/"
+            assert second.json()["node_name"] == "pve-node-2"
+
+            listing = await c.get("/v1/proxmox/hosts")
+            assert listing.json()["total"] == 1
+    finally:
+        await dbmod.dispose_engine()
+
+
+@pytest.mark.asyncio
+async def test_reseeding_refreshes_a_rotated_token(tmp_path, monkeypatch):
+    """The stored PVE token must follow the re-seed, or deploys 401 forever."""
+    app, dbmod = await _boot(tmp_path, monkeypatch)
+    try:
+        async with AsyncClient(
+            transport=ASGITransport(app=app), base_url="http://t"
+        ) as c:
+            await c.post(
+                "/v1/proxmox/hosts",
+                json={
+                    "name": "pve01",
+                    "api_url": "https://pve01:8006",
+                    "node_name": "pve01",
+                    "token_ref": "r42@pam!tok=stale",
+                },
+            )
+            await c.post(
+                "/v1/proxmox/hosts",
+                json={
+                    "name": "pve01",
+                    "api_url": "https://pve01:8006",
+                    "node_name": "pve01",
+                    "token_ref": "r42@pam!tok=fresh",
+                },
+            )
+
+        from sqlalchemy import select
+        from app.core.models import ProxmoxHost
+        async with dbmod.get_session_factory()() as session:
+            stored = (
+                await session.execute(select(ProxmoxHost.token_ref))
+            ).scalars().all()
+        assert stored == ["r42@pam!tok=fresh"]
+    finally:
+        await dbmod.dispose_engine()
+
+
+@pytest.mark.asyncio
+async def test_a_second_host_under_a_different_name_is_still_created(
+    tmp_path, monkeypatch
+):
+    """Upsert keys on name only — a genuinely new host must still register."""
+    app, dbmod = await _boot(tmp_path, monkeypatch)
+    try:
+        async with AsyncClient(
+            transport=ASGITransport(app=app), base_url="http://t"
+        ) as c:
+            a = await c.post(
+                "/v1/proxmox/hosts",
+                json={
+                    "name": "pve01",
+                    "api_url": "https://pve01:8006",
+                    "node_name": "pve01",
+                    "token_ref": "r42@pam!tok=a",
+                },
+            )
+            b = await c.post(
+                "/v1/proxmox/hosts",
+                json={
+                    "name": "pve02",
+                    "api_url": "https://pve02:8006",
+                    "node_name": "pve02",
+                    "token_ref": "r42@pam!tok=b",
+                },
+            )
+            assert b.status_code == 201, b.text
+            assert b.json()["id"] != a.json()["id"]
+
+            listing = await c.get("/v1/proxmox/hosts")
+            assert listing.json()["total"] == 2
+    finally:
+        await dbmod.dispose_engine()