From 6522a351153358072298677bc4ecf7ebe49b7892 Mon Sep 17 00:00:00 2001 From: root Date: Mon, 20 Jul 2026 09:49:31 +0000 Subject: [PATCH 1/2] Don't refuse recreating the canonical already-deployed target itself The duplicate-host check in create-proxmox-resource.sh compared by hostName only, so it fired even when the target being created was exactly the one variables.nix's deployedTargets already names (e.g. rebuilding lxc-nix-cache after destroying its old container to pick up new sops secrets) -- there's no other machine at risk of an identity collision in that case, just the normal redeploy workflow. Skip the check when dt == flake_target; the later VMID-existence check still guards against clobbering a resource that's actually live on the node. Co-Authored-By: Claude Sonnet 5 --- scripts/create-proxmox-resource.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/create-proxmox-resource.sh b/scripts/create-proxmox-resource.sh index 5d6ce11..f1e952d 100755 --- a/scripts/create-proxmox-resource.sh +++ b/scripts/create-proxmox-resource.sh @@ -289,11 +289,17 @@ fi # --- refuse to duplicate a host that's already really deployed ---------- # Checked by hostName, not exact flake target: proxmox-server being # deployed also blocks --type lxc --host server, since both would carry -# the same hosts/server/host.nix identity (hostName, hostId). +# the same hosts/server/host.nix identity (hostName, hostId). But skip the +# check when dt == flake_target -- that's not a duplicate, it's recreating +# the canonical deployed target itself (e.g. redeploying nix-cache after +# destroying its old VM/container to pick up new sops secrets), and the +# later VMID-existence check already guards against clobbering a live +# resource. if [[ "$allow_duplicate_host" -eq 0 ]]; then deployed_targets_json="$(nix eval --json --no-use-registries --no-accept-flake-config \ --file "${repo_root}/variables.nix" deployedTargets)" for dt in $(echo "$deployed_targets_json" | jq -r '.[]'); do + [[ "$dt" == "$flake_target" ]] && continue dt_hostname="$(nix eval --raw --no-use-registries --no-accept-flake-config \ "${repo_root}#nixosConfigurations.${dt}.config.networking.hostName" 2>/dev/null || true)" if [[ "$dt_hostname" == "$host" ]]; then -- 2.54.0 From 86a1660adc3302a1267dfbba7294b8a871f5609f Mon Sep 17 00:00:00 2001 From: root Date: Mon, 20 Jul 2026 10:03:00 +0000 Subject: [PATCH 2/2] Replace the duplicate-host check with a live Proxmox query, drop deployedTargets variables.nix's deployedTargets was a manually-maintained list with no enforcement keeping it in sync with reality -- it caused two separate false refusals in a row (naming a VM as deployed well after it had been destroyed, then matching a target against itself once the list was "corrected"). Static files can't track whether a resource still actually exists. create-proxmox-resource.sh's duplicate-host guard now queries the Proxmox node directly (qm/pct's own name/hostname config, matched against --host) instead. Also fixes a gap in that live check: it originally swallowed ssh failures and would have silently treated "can't reach the node" the same as "checked, nothing there" -- it now refuses instead of guessing when the node can't be reached. deployedTargets is removed entirely from variables.nix since nothing else in the repo consumed it once this script no longer does; README.md's Hosts table remains the sole source of truth for "(real, deployed)" status. CLAUDE.md and the script's own --help/comments updated to match. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 7 ++- README.md | 10 ++-- scripts/create-proxmox-resource.sh | 89 ++++++++++++++++++++---------- variables.nix | 16 ------ 4 files changed, 69 insertions(+), 53 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ff28eab..da28e41 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,9 +93,10 @@ Beyond `codex-setup.sh`/`codex-maintenance.sh` above, `scripts/` also has: (`--force-rebuild` to skip that and always rebuild), and probes nix-cache's substituter/remote-builder reachability once up front rather than letting every `nix build` call retry against it individually. - Refuses to create a target whose host identity already has a real - deployment elsewhere (`variables.nix`'s `deployedTargets`) unless - `--allow-duplicate-host` is passed. `--dry-run` throughout both modes. + Refuses to create a target whose host identity already exists live on + the node (checked directly via `qm`/`pct`, not any file in this repo) + unless `--allow-duplicate-host` is passed. `--dry-run` throughout both + modes. - `scripts/env.sh` — shared config (`PROXMOX_HOST`, storage pool, bridge, default cores/memory) sourced by `create-proxmox-resource.sh`. Add new cross-script config here instead of duplicating it per-script. diff --git a/README.md b/README.md index 77e62b3..bcb85c2 100644 --- a/README.md +++ b/README.md @@ -28,10 +28,12 @@ list: | `proxmox-pxe-boot` / `lxc-pxe-boot` | HTTP/iPXE boot asset host (`proxmox-pxe-boot` is the real, deployed one — previously the flat `pxe-boot` target) | | `linode-tailscale-exit-node` / `proxmox-tailscale-exit-node` / `lxc-tailscale-exit-node` | Tailscale exit node (no deployed target yet; `lxc-tailscale-exit-node` is the one planned for actual use) | -The "(real, deployed)" targets above are also tracked machine-readably in -`variables.nix`'s `deployedTargets` — keep both in sync when a deployment -changes. `scripts/create-proxmox-resource.sh` reads that list to refuse -creating a same-identity duplicate of an already-deployed host by accident. +This table is the only place "(real, deployed)" status is tracked — there's +no separate machine-readable copy to keep in sync. `scripts/create-proxmox-resource.sh` +guards against creating a same-identity duplicate of an already-deployed host +by checking the Proxmox node itself (live `qm`/`pct` state) rather than any +file in this repo, since a static list can't track whether a resource still +actually exists. Each buildtype's `hosts//host.nix` carries the per-machine identity (hostname, hostId, per-machine secrets, `system.stateVersion`) that must stay diff --git a/scripts/create-proxmox-resource.sh b/scripts/create-proxmox-resource.sh index f1e952d..e10473e 100755 --- a/scripts/create-proxmox-resource.sh +++ b/scripts/create-proxmox-resource.sh @@ -10,7 +10,9 @@ # # SAFETY: # - The default (create) mode only ever creates a NEW resource -- it -# refuses to run if the target VMID already exists on the node. +# refuses to run if the target VMID already exists on the node, or if +# a VM/CT identified as --host already exists under any other VMID +# (checked live against the node; --allow-duplicate-host overrides). # - --modify only ever touches a resource you name explicitly via # --vmid, shows exactly what will change first, and (outside # --dry-run) always requires typing that VMID back to confirm before @@ -56,10 +58,11 @@ Create mode (default): --force-rebuild Skip the "does the node already have this image" check -- always build fresh and overwrite what's there. - --allow-duplicate-host Required if --host already has a real - deployment elsewhere (variables.nix's - deployedTargets) -- otherwise refused, since - it'd share that host's hostName/hostId. + --allow-duplicate-host Required if a VM/CT identified as --host + already exists on the node (checked live via + qm/pct, not any file in this repo) -- + otherwise refused, since it'd share that + host's hostName/hostId. Modify mode (reconfigure an EXISTING resource -- requires --modify): --modify Switch to modify mode. @@ -286,31 +289,57 @@ fi # feeds straight into the guest's real hostname) disagree with host.nix. [[ -z "$name" ]] && name="$host" -# --- refuse to duplicate a host that's already really deployed ---------- -# Checked by hostName, not exact flake target: proxmox-server being -# deployed also blocks --type lxc --host server, since both would carry -# the same hosts/server/host.nix identity (hostName, hostId). But skip the -# check when dt == flake_target -- that's not a duplicate, it's recreating -# the canonical deployed target itself (e.g. redeploying nix-cache after -# destroying its old VM/container to pick up new sops secrets), and the -# later VMID-existence check already guards against clobbering a live -# resource. -if [[ "$allow_duplicate_host" -eq 0 ]]; then - deployed_targets_json="$(nix eval --json --no-use-registries --no-accept-flake-config \ - --file "${repo_root}/variables.nix" deployedTargets)" - for dt in $(echo "$deployed_targets_json" | jq -r '.[]'); do - [[ "$dt" == "$flake_target" ]] && continue - dt_hostname="$(nix eval --raw --no-use-registries --no-accept-flake-config \ - "${repo_root}#nixosConfigurations.${dt}.config.networking.hostName" 2>/dev/null || true)" - if [[ "$dt_hostname" == "$host" ]]; then - echo "ERROR: '${host}' already has a real deployment (${dt}, per variables.nix's" >&2 - echo "deployedTargets). Creating ${flake_target} would share its hostName/hostId --" >&2 - echo "refusing by default. Pass --allow-duplicate-host if you really mean to spin" >&2 - echo "up a separate test instance of this host (it'll still get its own distinct" >&2 - echo "sops key and VMID, never touching ${dt})." >&2 - exit 1 - fi - done +# --- refuse to duplicate a host that's already live on the node --------- +# Queries the node itself (qm/pct's own name/hostname config), not any +# static list in this repo -- a file can't track whether a resource still +# actually exists, and this used to be checked against variables.nix's +# deployedTargets, which drifted stale (it kept naming a VM as "the real +# deployment" well after that VM had been destroyed, blocking its own +# redeploy) until that list was dropped in favour of this live check. This +# only catches guests identified with the default --name (== --host, what +# this script itself always uses unless --name is overridden) -- a guest +# manually renamed on the node afterwards wouldn't match, but nothing here +# creates guests that way. +if [[ "$allow_duplicate_host" -eq 1 ]]; then + echo + echo "--allow-duplicate-host: skipping the check for an existing '${host}' on ${node}." +elif [[ "$dry_run" -eq 1 ]]; then + echo + echo "[dry-run] would check ${node} for an existing VM/CT identified as '${host}'" +else + echo + echo "==> Checking ${node} for an existing VM/CT identified as '${host}'..." + ssh_check_status=0 + existing="$(ssh "$ssh_target" bash -s -- "$host" <<'REMOTE_SCRIPT' +target="$1" +for id in $(qm list 2>/dev/null | awk 'NR>1{print $1}'); do + n="$(qm config "$id" 2>/dev/null | grep -oP '^name:\s*\K\S+' || true)" + [[ "$n" == "$target" ]] && echo "vm ${id} ${n}" +done +for id in $(pct list 2>/dev/null | awk 'NR>1{print $1}'); do + n="$(pct config "$id" 2>/dev/null | grep -oP '^hostname:\s*\K\S+' || true)" + [[ "$n" == "$target" ]] && echo "lxc ${id} ${n}" +done +REMOTE_SCRIPT + )" || ssh_check_status=$? + if [[ "$ssh_check_status" -ne 0 ]]; then + echo "ERROR: couldn't reach ${node} (ssh exited ${ssh_check_status}) to check for an" >&2 + echo "existing '${host}' resource -- refusing to guess. Fix connectivity and retry," >&2 + echo "or pass --allow-duplicate-host if you're sure none exists (this skips the" >&2 + echo "check entirely)." >&2 + exit 1 + fi + if [[ -n "$existing" ]]; then + echo "ERROR: '${host}' already exists on ${node}:" >&2 + echo "$existing" | while read -r kind id n; do + echo " - ${kind} VMID ${id} (${n})" >&2 + done + echo "Refusing to create a second resource sharing this identity. Pass" >&2 + echo "--allow-duplicate-host to create one anyway (it gets its own distinct" >&2 + echo "sops key and VMID -- the existing resource(s) above are left untouched)," >&2 + echo "or use --modify to reconfigure the existing one instead." >&2 + exit 1 + fi fi echo "Target: ${flake_target} (host=${host}, type=${type}) -> Proxmox resource '${name}'" diff --git a/variables.nix b/variables.nix index 77fc155..f86b81c 100644 --- a/variables.nix +++ b/variables.nix @@ -156,20 +156,4 @@ keep = 20; # number of rotated logs to retain before deleting the oldest }; - # Flake targets with a real, currently-running deployment somewhere — - # matches README.md's Hosts table "(real, deployed)" annotations; update - # both together. Not consumed by any NixOS module (nothing in the actual - # system config should behave differently because of this) — it's read - # by scripts/create-proxmox-resource.sh to refuse creating a same-identity - # duplicate of an already-deployed host (shared hostName/hostId) unless - # you explicitly pass --allow-duplicate-host. - deployedTargets = [ - "linode-minimal" - "proxmox-minimal" - "lxc-nix-cache" - "proxmox-server" - "proxmox-docker" - "proxmox-gui" - "proxmox-pxe-boot" - ]; } -- 2.54.0