Download a corrupt local template again instead of keeping it

.github/workflows/verify-split-files.yml

@@ -31,6 +31,7 @@ on:
       - "tests/docker-lxcfs.sh"
       - "tests/java-memory.sh"
       - "tests/sudo-env.sh"
+      - "tests/template-broken.sh"
       - "tests/deb822-local-keyring.sh"
       - "tests/gpu-pci-class.sh"
       - "tests/nvidia-version-pin.sh"
@@ -92,6 +93,7 @@ on:
       - "tests/docker-lxcfs.sh"
       - "tests/java-memory.sh"
       - "tests/sudo-env.sh"
+      - "tests/template-broken.sh"
       - "tests/deb822-local-keyring.sh"
       - "tests/gpu-pci-class.sh"
       - "tests/nvidia-version-pin.sh"
@@ -179,6 +181,9 @@ jobs:
       - name: sudo environment tests
         run: bash tests/sudo-env.sh
 
+      - name: Broken template tests
+        run: bash tests/template-broken.sh
+
       - name: deb822 local keyring tests
         run: bash tests/deb822-local-keyring.sh
 

pve/backend.func

@@ -276,6 +276,13 @@ _template_path() {
   echo "${p:-/var/lib/vz/template/cache/$1}"
 }
 
+# Missing, cut short or not unpackable: pct create fails on it every time.
+_template_broken() {
+  [[ -s "$1" ]] || return 0
+  (($(stat -c%s "$1" 2>/dev/null || echo 0) < 1000000)) && return 0
+  ! tar -tf "$1" &>/dev/null
+}
+
 # pveam needs template/cache, which a storage with create-subdirs 0 lacks
 _pveam_download() {
   local path
@@ -3365,7 +3372,7 @@ create_lxc_container() {
     # Download if missing, too small, or corrupt
     if [[ ! -f "$TEMPLATE_PATH" ]]; then
       download_catalog_template "$TEMPLATE_PATH"
-    elif [[ "$(stat -c%s "$TEMPLATE_PATH")" -lt 1000000 ]] || ! tar -tf "$TEMPLATE_PATH" &>/dev/null; then
+    elif _template_broken "$TEMPLATE_PATH"; then
       msg_warn "Local template invalid - re-downloading."
       rm -f "$TEMPLATE_PATH"
       download_catalog_template "$TEMPLATE_PATH"
@@ -3538,20 +3545,10 @@ create_lxc_container() {
     elif [[ ! -r "$TEMPLATE_PATH" ]]; then
       msg_error "Template file exists but is not readable – check permissions."
       exit 221
-    elif [[ "$(stat -c%s "$TEMPLATE_PATH")" -lt 1000000 ]]; then
-      if [[ -n "$ONLINE_TEMPLATE" ]]; then
-        msg_warn "Template file too small (<1MB) – re-downloading."
-        NEED_DOWNLOAD=1
-      else
-        msg_warn "Template looks too small, but no online version exists. Keeping local file."
-      fi
-    elif ! tar -tf "$TEMPLATE_PATH" &>/dev/null; then
-      if [[ -n "$ONLINE_TEMPLATE" ]]; then
-        msg_warn "Template appears corrupted – re-downloading."
-        NEED_DOWNLOAD=1
-      else
-        msg_warn "Template appears corrupted, but no online version exists. Keeping local file."
-      fi
+    elif _template_broken "$TEMPLATE_PATH"; then
+      # Even without ONLINE_TEMPLATE (a local find skips the catalog): kept, it failed pct create every time.
+      msg_warn "Template $TEMPLATE is incomplete or corrupt – downloading it again."
+      NEED_DOWNLOAD=1
     else
       $STD msg_ok "Template $TEMPLATE is present and valid."
     fi
@@ -3683,20 +3680,11 @@ create_lxc_container() {
   }
 
   # Validate template before pct create (while holding lock)
-  if [[ ! -s "$TEMPLATE_PATH" || "$(stat -c%s "$TEMPLATE_PATH" 2>/dev/null || echo 0)" -lt 1000000 ]]; then
-    msg_info "Template file missing or too small – downloading"
+  if _template_broken "$TEMPLATE_PATH"; then
+    msg_info "Template missing or corrupt – downloading"
     rm -f "$TEMPLATE_PATH"
     download_template
     msg_ok "Template downloaded"
-  elif ! tar -tf "$TEMPLATE_PATH" &>/dev/null; then
-    if [[ -n "$CUSTOM_TEMPLATE_PATH" || -n "$ONLINE_TEMPLATE" ]]; then
-      msg_info "Template appears corrupted – re-downloading"
-      rm -f "$TEMPLATE_PATH"
-      download_template
-      msg_ok "Template re-downloaded"
-    else
-      msg_warn "Template appears corrupted, but no online version exists. Skipping re-download."
-    fi
   fi
 
   # Both run as ostype centos, whose PVE plugin reads a /etc/redhat-release neither ships.
@@ -3774,7 +3762,7 @@ create_lxc_container() {
         exit 209
       fi
     else
-      if grep -qiE 'unable to open|corrupt|invalid' "$LOGFILE"; then
+      if grep -qiE "unable to open|corrupt|invalid|unexpected EOF|doesn't match checksum|premature end" "$LOGFILE"; then
         msg_info "Template may be corrupted – re-downloading"
         rm -f "$TEMPLATE_PATH"
         download_template

tests/template-broken.sh

@@ -0,0 +1,66 @@
+#!/usr/bin/env bash
+# A template found locally skipped the catalog, so a corrupt one was kept
+# ("no online version exists") and pct create failed on it at every retry.
+set -uo pipefail
+
+CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)"
+BACKEND="$CORE_DIR/pve/backend.func"
+eval "$(awk '/^_template_broken[(][)] [{]/,/^}/' "$BACKEND")"
+
+TEST_DIR="$(mktemp -d)"
+trap 'rm -rf -- "$TEST_DIR"' EXIT
+passed=0 failed=0
+
+expect() {
+  if [[ "$2" == "$3" ]]; then
+    echo "PASS $1"
+    passed=$((passed + 1))
+  else
+    echo "FAIL $1: expected <$2>, got <$3>" >&2
+    failed=$((failed + 1))
+  fi
+}
+broken() { _template_broken "$1" && echo broken || echo ok; }
+
+mkdir -p "$TEST_DIR/root"
+head -c 3000000 /dev/urandom >"$TEST_DIR/root/blob"
+tar -C "$TEST_DIR/root" -czf "$TEST_DIR/good.tar.gz" blob
+size=$(stat -c%s "$TEST_DIR/good.tar.gz")
+head -c $((size / 2)) "$TEST_DIR/good.tar.gz" >"$TEST_DIR/cut.tar.gz"
+cp "$TEST_DIR/good.tar.gz" "$TEST_DIR/flipped.tar.gz"
+head -c 4096 /dev/zero | dd of="$TEST_DIR/flipped.tar.gz" bs=1 seek=$((size / 2)) conv=notrunc status=none
+: >"$TEST_DIR/empty.tar.gz"
+head -c 500000 "$TEST_DIR/good.tar.gz" >"$TEST_DIR/small.tar.gz"
+
+expect "an intact template is kept" ok "$(broken "$TEST_DIR/good.tar.gz")"
+expect "a missing template is fetched" broken "$(broken "$TEST_DIR/none.tar.gz")"
+expect "an empty template is fetched" broken "$(broken "$TEST_DIR/empty.tar.gz")"
+expect "a template under 1 MB is fetched" broken "$(broken "$TEST_DIR/small.tar.gz")"
+expect "a cut-off download is fetched again" broken "$(broken "$TEST_DIR/cut.tar.gz")"
+expect "damaged bytes inside are fetched again" broken "$(broken "$TEST_DIR/flipped.tar.gz")"
+
+if command -v zstd >/dev/null; then
+  tar -C "$TEST_DIR/root" --zstd -cf "$TEST_DIR/good.tar.zst" blob
+  size=$(stat -c%s "$TEST_DIR/good.tar.zst")
+  head -c $((size * 3 / 4)) "$TEST_DIR/good.tar.zst" >"$TEST_DIR/cut.tar.zst"
+  expect "an intact zstd template is kept" ok "$(broken "$TEST_DIR/good.tar.zst")"
+  expect "a cut-off zstd template is fetched again" broken "$(broken "$TEST_DIR/cut.tar.zst")"
+fi
+
+expect "every template check uses the helper, none keeps a broken file" "3 0" \
+  "$(grep -c '_template_broken "$TEMPLATE_PATH"' "$BACKEND") $(grep -c 'no online version exists' "$BACKEND")"
+
+# What pct create printed for a broken template, from the telemetry.
+pattern="$(grep -oE "grep -qiE \"unable to open[^\"]*\"" "$BACKEND" | sed 's/^grep -qiE "//; s/"$//')"
+expect "the retry after a failed create has its pattern" yes "$([[ -n "$pattern" ]] && echo yes || echo no)"
+pattern="${pattern:-^-no pattern-$}"
+for line in \
+  "/*stdin*\\ : Decoding error (36) : Restored data doesn't match checksum" \
+  "/*stdin*\\ : Read error (39) : premature end" \
+  "tar: Unexpected EOF in archive"; do
+  expect "a failed create retries with a fresh template: ${line##*: }" yes \
+    "$(grep -qiE "$pattern" <<<"$line" && echo yes || echo no)"
+done
+
+printf '%s passed, %s failed\n' "$passed" "$failed"
+((failed == 0))