Archived
fix(create-proxmox-resource): fix VM disk never attaching after import
Check NixOS configurations / eval-hosts (pull_request) Successful in 10m31s
Check NixOS configurations / eval-hosts (pull_request) Successful in 10m31s
Three bugs combined to leave every VM build with a shell but no boot disk: 1. The remote build script moved the raw image to /var/lib/vz/import/ before qm importdisk could use it. If the mv failed (cross-filesystem copy, sudo path, or any other reason) the remote script exited non-zero -- but the local script's set -e handling of the SSH heredoc was inconsistent, so qm create sometimes ran anyway, leaving a diskless VM shell. Fix: skip the mv entirely. The diskoImagesScript writes <hostname>.raw into its CWD (the remote repo dir, $out = $PWD at invocation). Import directly from that path; clean it up after a successful import. 2. The qm importdisk output regex expected "Successfully imported disk as '...'" but current Proxmox emits "unusedN: successfully imported disk '...'" (lowercase, no "as"). The grep returned no match and exited 1. 3. The disk_id assignment used $(... | grep ...) without || true inside the substitution. With set -euo pipefail, a non-zero grep exit aborts the script before the fallback could run -- so the VM was always left with an unattached unused0 disk. Fix: update the primary regex to match the actual PVE format; add || true inside the substitution so set -e never fires on a grep miss; add a qm config fallback (scan for unusedN: lines) that works regardless of PVE output format changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uRcikkTp3D5VbXj2DwNpQ
This commit is contained in:
@@ -354,6 +354,14 @@ fi
|
|||||||
# feeds straight into the guest's real hostname) disagree with host.nix.
|
# feeds straight into the guest's real hostname) disagree with host.nix.
|
||||||
[[ -z "$name" ]] && name="$host"
|
[[ -z "$name" ]] && name="$host"
|
||||||
|
|
||||||
|
# For VM builds: the diskoImagesScript (run via QEMU on the node) writes the
|
||||||
|
# raw disk image as <hostname>.raw into the CWD it was called from (the remote
|
||||||
|
# repo dir), not to /var/lib/vz/import/ or anywhere else. Import directly from
|
||||||
|
# there -- no intermediate mv that can fail crossing filesystem boundaries or
|
||||||
|
# leave a stale file on error.
|
||||||
|
vm_built_raw=""
|
||||||
|
[[ "$type" == "vm" ]] && vm_built_raw="${remote_repo_dir}/${host}.raw"
|
||||||
|
|
||||||
# --- refuse to duplicate a host that's already live on the node ---------
|
# --- 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
|
# 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
|
# static list in this repo -- a file can't track whether a resource still
|
||||||
@@ -688,10 +696,14 @@ if [[ -n "$image" ]]; then
|
|||||||
elif [[ "$force_rebuild" -eq 1 ]]; then
|
elif [[ "$force_rebuild" -eq 1 ]]; then
|
||||||
echo "--force-rebuild: skipping the existing-image check on ${node}."
|
echo "--force-rebuild: skipping the existing-image check on ${node}."
|
||||||
else
|
else
|
||||||
echo "==> Checking whether ${node} already has ${remote_path}..."
|
# VMs: check for the raw image in the remote repo dir (where disko writes it).
|
||||||
|
# LXC: check for the tarball in iso_storage (where the LXC build stages it).
|
||||||
|
_check_path="$remote_path"
|
||||||
|
[[ "$type" == "vm" ]] && _check_path="$vm_built_raw"
|
||||||
|
echo "==> Checking whether ${node} already has ${_check_path}..."
|
||||||
if [[ "$dry_run" -eq 1 ]]; then
|
if [[ "$dry_run" -eq 1 ]]; then
|
||||||
echo "[dry-run] would check: ssh ${ssh_target} -- test -f ${remote_path}"
|
echo "[dry-run] would check: ssh ${ssh_target} -- test -f ${_check_path}"
|
||||||
elif ssh "$ssh_target" "test -f '${remote_path}'" 2>/dev/null; then
|
elif ssh "$ssh_target" "test -f '${_check_path}'" 2>/dev/null; then
|
||||||
echo "Found it -- reusing, skipping build (use --force-rebuild to override)."
|
echo "Found it -- reusing, skipping build (use --force-rebuild to override)."
|
||||||
image_already_remote=1
|
image_already_remote=1
|
||||||
else
|
else
|
||||||
@@ -772,17 +784,18 @@ REMOTE_SCRIPT
|
|||||||
echo "[dry-run] --pre-format-files host-keys/${flake_target}_ssh_host_ed25519_key /etc/ssh/ssh_host_ed25519_key \\"
|
echo "[dry-run] --pre-format-files host-keys/${flake_target}_ssh_host_ed25519_key /etc/ssh/ssh_host_ed25519_key \\"
|
||||||
echo "[dry-run] --pre-format-files host-keys/${flake_target}_ssh_host_ed25519_key.pub /etc/ssh/ssh_host_ed25519_key.pub \\"
|
echo "[dry-run] --pre-format-files host-keys/${flake_target}_ssh_host_ed25519_key.pub /etc/ssh/ssh_host_ed25519_key.pub \\"
|
||||||
echo "[dry-run] --build-memory 2048"
|
echo "[dry-run] --build-memory 2048"
|
||||||
echo "[dry-run] would stage the result at ${remote_path}"
|
echo "[dry-run] image will be at ${vm_built_raw} (imported from there; no mv to /var/lib/vz/import/)"
|
||||||
local_image="<built-image>.raw"
|
local_image="<built-image>.raw"
|
||||||
else
|
else
|
||||||
echo "==> Building Disko image for ${flake_target} on ${node}..."
|
echo "==> Building Disko image for ${flake_target} on ${node}..."
|
||||||
# See the LXC branch above for why this is one %q-quoted command
|
# See the LXC branch above for why this is one %q-quoted command
|
||||||
# string rather than separate ssh argv elements.
|
# string rather than separate ssh argv elements.
|
||||||
printf -v remote_cmd 'bash -s -- %q %q %q %q %q %q' \
|
# $7 = image_name (hostname, the diskoImagesScript's own output filename).
|
||||||
"$remote_repo_dir" "$flake_target" "$remote_dir" "$remote_filename" "$NIX_EXTRA_OPTS" "$sudo_prefix"
|
printf -v remote_cmd 'bash -s -- %q %q %q %q %q %q %q' \
|
||||||
|
"$remote_repo_dir" "$flake_target" "$remote_dir" "$remote_filename" "$NIX_EXTRA_OPTS" "$sudo_prefix" "$host"
|
||||||
ssh "$ssh_target" "$remote_cmd" <<'REMOTE_SCRIPT'
|
ssh "$ssh_target" "$remote_cmd" <<'REMOTE_SCRIPT'
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
repo_dir="$1"; target="$2"; dest_dir="$3"; dest_name="$4"; nix_extra_opts_str="$5"; sudo_pfx="$6"
|
repo_dir="$1"; target="$2"; dest_dir="$3"; dest_name="$4"; nix_extra_opts_str="$5"; sudo_pfx="$6"; image_name="$7"
|
||||||
declare -a NIX_OPTS=()
|
declare -a NIX_OPTS=()
|
||||||
[[ -n "$nix_extra_opts_str" ]] && eval "NIX_OPTS=(${nix_extra_opts_str})"
|
[[ -n "$nix_extra_opts_str" ]] && eval "NIX_OPTS=(${nix_extra_opts_str})"
|
||||||
cd "$repo_dir"
|
cd "$repo_dir"
|
||||||
@@ -797,21 +810,21 @@ fi
|
|||||||
nix build --no-use-registries --no-accept-flake-config "${NIX_OPTS[@]}" \
|
nix build --no-use-registries --no-accept-flake-config "${NIX_OPTS[@]}" \
|
||||||
".#nixosConfigurations.${target}.config.system.build.diskoImagesScript" \
|
".#nixosConfigurations.${target}.config.system.build.diskoImagesScript" \
|
||||||
--out-link "result-${target}"
|
--out-link "result-${target}"
|
||||||
|
# Remove any stale .raw from a previous failed build so the post-build check
|
||||||
|
# below is unambiguous (diskoImagesScript writes to CWD as ${image_name}.raw).
|
||||||
|
$sudo_pfx rm -f "${image_name}.raw" 2>/dev/null || true
|
||||||
$sudo_pfx "./result-${target}" \
|
$sudo_pfx "./result-${target}" \
|
||||||
--pre-format-files "$(pwd)/host-keys/${target}_ssh_host_ed25519_key" /etc/ssh/ssh_host_ed25519_key \
|
--pre-format-files "$(pwd)/host-keys/${target}_ssh_host_ed25519_key" /etc/ssh/ssh_host_ed25519_key \
|
||||||
--pre-format-files "$(pwd)/host-keys/${target}_ssh_host_ed25519_key.pub" /etc/ssh/ssh_host_ed25519_key.pub \
|
--pre-format-files "$(pwd)/host-keys/${target}_ssh_host_ed25519_key.pub" /etc/ssh/ssh_host_ed25519_key.pub \
|
||||||
--build-memory 2048
|
--build-memory 2048
|
||||||
built="$(find . -maxdepth 1 -name '*.raw' -newer "result-${target}" | head -1)"
|
if [[ ! -f "${image_name}.raw" ]]; then
|
||||||
if [[ -z "$built" ]]; then
|
echo "ERROR: ${image_name}.raw not found in ${repo_dir} after build -- disko/QEMU may have failed." >&2
|
||||||
echo "ERROR: no .raw image found in ${repo_dir} after build." >&2
|
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
$sudo_pfx mkdir -p "$dest_dir"
|
echo "Built image: ${repo_dir}/${image_name}.raw"
|
||||||
$sudo_pfx mv "$built" "${dest_dir}/${dest_name}"
|
|
||||||
echo "Built and staged: ${dest_dir}/${dest_name}"
|
|
||||||
REMOTE_SCRIPT
|
REMOTE_SCRIPT
|
||||||
local_image="$remote_path"
|
local_image="$vm_built_raw"
|
||||||
echo "Built on ${node}: ${remote_path}"
|
echo "Built on ${node}: ${vm_built_raw}"
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
@@ -889,14 +902,35 @@ else
|
|||||||
--net0 virtio,bridge=${bridge} --bios ovmf --machine q35 --scsihw virtio-scsi-pci \
|
--net0 virtio,bridge=${bridge} --bios ovmf --machine q35 --scsihw virtio-scsi-pci \
|
||||||
--efidisk0 ${storage}:1,efitype=4m,pre-enrolled-keys=0 --agent enabled=1"
|
--efidisk0 ${storage}:1,efitype=4m,pre-enrolled-keys=0 --agent enabled=1"
|
||||||
|
|
||||||
|
# VMs built on the node: import from the repo dir (where disko/QEMU wrote it).
|
||||||
|
# VMs from --image: import from remote_path (where scp uploaded it).
|
||||||
|
_import_path="${remote_path}"
|
||||||
|
[[ -z "$image" ]] && _import_path="${vm_built_raw}"
|
||||||
if [[ "$dry_run" -eq 1 ]]; then
|
if [[ "$dry_run" -eq 1 ]]; then
|
||||||
echo "[dry-run] ssh ${ssh_target} -- ${sudo_display}qm importdisk ${vmid} ${remote_path} ${storage}"
|
echo "[dry-run] ssh ${ssh_target} -- ${sudo_display}qm importdisk ${vmid} ${_import_path} ${storage}"
|
||||||
echo "[dry-run] (would parse the resulting disk identifier from that output)"
|
echo "[dry-run] (would parse the resulting disk identifier from that output)"
|
||||||
echo "[dry-run] ssh ${ssh_target} -- ${sudo_display}qm set ${vmid} --scsi0 ${storage}:<parsed-disk-id>"
|
echo "[dry-run] ssh ${ssh_target} -- ${sudo_display}qm set ${vmid} --scsi0 ${storage}:<parsed-disk-id>"
|
||||||
else
|
else
|
||||||
importdisk_output="$(ssh "$ssh_target" "${sudo_prefix} qm importdisk ${vmid} ${remote_path} ${storage}")"
|
if ! importdisk_output="$(ssh "$ssh_target" "${sudo_prefix} qm importdisk ${vmid} ${_import_path} ${storage}" 2>&1)"; then
|
||||||
|
echo "ERROR: qm importdisk failed:" >&2
|
||||||
|
echo "${importdisk_output}" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
echo "$importdisk_output"
|
echo "$importdisk_output"
|
||||||
disk_id="$(echo "$importdisk_output" | grep -oP "(?<=Successfully imported disk as ')[^']+" | sed 's/^unused[0-9]*://')"
|
# PVE output format: "unusedN: successfully imported disk '<storage>:<vol>'"
|
||||||
|
# (lowercase "successfully", no "as"; the primary regex targets this form; the
|
||||||
|
# || true inside the substitution prevents set -e from aborting when grep finds
|
||||||
|
# no match -- without it the script would silently exit before reaching the
|
||||||
|
# fallback whenever the PVE format doesn't match).
|
||||||
|
disk_id="$(echo "$importdisk_output" | grep -oP "successfully imported disk '\\K[^']+" || true)"
|
||||||
|
if [[ -z "$disk_id" ]]; then
|
||||||
|
# Fallback for other PVE output variants: read qm config directly.
|
||||||
|
unused_line="$(ssh "$ssh_target" "${sudo_prefix} qm config ${vmid}" | grep '^unused[0-9]*:' | head -1 || true)"
|
||||||
|
if [[ -n "$unused_line" ]]; then
|
||||||
|
disk_id="${unused_line#*: }"
|
||||||
|
echo "Note: disk ID resolved from qm config: ${disk_id}"
|
||||||
|
fi
|
||||||
|
fi
|
||||||
if [[ -z "$disk_id" ]]; then
|
if [[ -z "$disk_id" ]]; then
|
||||||
echo "ERROR: couldn't parse the imported disk identifier from qm importdisk's output above." >&2
|
echo "ERROR: couldn't parse the imported disk identifier from qm importdisk's output above." >&2
|
||||||
echo "The VM shell (${vmid}) and imported disk both exist -- finish attaching it by hand:" >&2
|
echo "The VM shell (${vmid}) and imported disk both exist -- finish attaching it by hand:" >&2
|
||||||
@@ -905,6 +939,12 @@ else
|
|||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
remote "${sudo_prefix} qm set ${vmid} --scsi0 ${disk_id}"
|
remote "${sudo_prefix} qm set ${vmid} --scsi0 ${disk_id}"
|
||||||
|
# The disk data is now in ZFS; remove the source raw file (only for images
|
||||||
|
# we built on the node -- --image uploads are the operator's to manage).
|
||||||
|
if [[ -z "$image" ]]; then
|
||||||
|
ssh "$ssh_target" "${sudo_prefix} rm -f '${_import_path}'" 2>/dev/null || \
|
||||||
|
echo "Warning: couldn't remove ${_import_path} from ${node} -- you can delete it manually" >&2
|
||||||
|
fi
|
||||||
fi
|
fi
|
||||||
remote "${sudo_prefix} qm set ${vmid} --boot order=scsi0"
|
remote "${sudo_prefix} qm set ${vmid} --boot order=scsi0"
|
||||||
remote "${sudo_prefix} qm start ${vmid}"
|
remote "${sudo_prefix} qm start ${vmid}"
|
||||||
|
|||||||
Reference in New Issue
Block a user