fix(proxmox): make host registration idempotent on name

alembic/versions/0002_proxmox_host_unique_name.py

@@ -0,0 +1,101 @@
+"""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()
+
+    groups: dict[str, list[str]] = {}
+    for host_id, name in rows:
+        groups.setdefault(name, []).append(host_id)
+
+    for name, ids in groups.items():
+        if len(ids) == 1:
+            continue
+        keeper_id, *loser_ids = ids
+
+        # The id is the oldest row's, because deployments reference it. The
+        # connection details are the NEWEST row's: duplicates accumulated one
+        # per scenario re-run, so the last registration holds the credentials
+        # in force. Keeping the first row wholesale would resurrect a token
+        # that may since have been rotated away, and every deployment
+        # repointed at it would start failing auth the moment this ran.
+        conn.execute(
+            sa.text(
+                "UPDATE proxmox_hosts SET"
+                "   api_url = latest.api_url,"
+                "   node_name = latest.node_name,"
+                "   token_ref = latest.token_ref,"
+                "   token_scope = latest.token_scope,"
+                "   default_bridge = latest.default_bridge,"
+                "   protected_vmids_override_json ="
+                "       latest.protected_vmids_override_json,"
+                "   last_health_check_json = latest.last_health_check_json"
+                " FROM (SELECT * FROM proxmox_hosts WHERE id = :newest) AS latest"
+                " WHERE proxmox_hosts.id = :keeper"
+            ),
+            {"newest": loser_ids[-1], "keeper": keeper_id},
+        )
+
+        for loser_id in loser_ids:
+            moved = conn.execute(
+                sa.text(
+                    "UPDATE deployments SET target_host_id = :keeper"
+                    " WHERE target_host_id = :loser"
+                ),
+                {"keeper": keeper_id, "loser": loser_id},
+            ).rowcount
+            conn.execute(
+                sa.text("DELETE FROM proxmox_hosts WHERE id = :loser"),
+                {"loser": loser_id},
+            )
+            log.info(
+                "proxmox_hosts: collapsed duplicate %r (%s) into %s, "
+                "repointed %d deployment(s)",
+                name, loser_id, keeper_id, moved,
+            )
+        log.info(
+            "proxmox_hosts: %r kept id %s with the connection details from %s",
+            name, keeper_id, loser_ids[-1],
+        )
+
+    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,8 +7,9 @@
 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.exc import IntegrityError
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.core.errors import AuthFailedError, Range42Error
@@ -47,6 +48,26 @@ def _row_to_out(row: ProxmoxHost) -> HostOut:
     )
 
 
+async def _find_host_by_name(
+    session: AsyncSession, name: str
+) -> ProxmoxHost | None:
+    return (
+        await session.execute(
+            select(ProxmoxHost).where(ProxmoxHost.name == name)
+        )
+    ).scalar_one_or_none()
+
+
+def _refresh_host(row: ProxmoxHost, payload: HostIn, overrides_json: str | None) -> None:
+    """Carry a re-registration onto an existing row, id and added_at intact."""
+    row.api_url = str(payload.api_url)
+    row.node_name = payload.node_name
+    row.token_ref = payload.token_ref
+    row.token_scope = payload.token_scope
+    row.default_bridge = payload.default_bridge
+    row.protected_vmids_override_json = overrides_json
+
+
 @router.get("/hosts", response_model=Page[HostOut])
 async def list_hosts(
     session: AsyncSession = Depends(_session),
@@ -68,11 +89,49 @@ 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
+    )
+
+    async def _update_in_place(row: ProxmoxHost) -> HostOut:
+        # added_at deliberately untouched: it records when this host was first
+        # registered, and the dedupe migration keys on it.
+        _refresh_host(row, payload, overrides_json)
+        await session.commit()
+        await session.refresh(row)
+        log.info("proxmox_host_reregistered", host_id=row.id, name=row.name)
+        response.status_code = status.HTTP_200_OK
+        return _row_to_out(row)
+
+    existing = await _find_host_by_name(session, payload.name)
+    if existing is not None:
+        return await _update_in_place(existing)
+
     row = ProxmoxHost(
         id=uuid.uuid4().hex[:16],
         name=payload.name,
@@ -81,14 +140,22 @@ 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()
+    try:
+        await session.commit()
+    except IntegrityError:
+        # Lost the race: another request registered this name between the
+        # lookup above and this commit. Recover into the update path instead
+        # of surfacing a 500 — the caller asked for a registration and one
+        # now exists, which is the outcome they wanted.
+        await session.rollback()
+        winner = await _find_host_by_name(session, payload.name)
+        if winner is None:
+            raise
+        log.info("proxmox_host_register_race", name=payload.name, host_id=winner.id)
+        return await _update_in_place(winner)
     await session.refresh(row)
     return _row_to_out(row)
 

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"
+                }
+              }
+            }
           }
         }
       }