NAPALM/napalm-proxmox
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(vm_provision_mixin): stale same-named cloud image cache causes checksum mismatch

Browse Source 
_download_cloud_image() cached downloaded images under just the URL's
basename (e.g. ubuntu-26.04-server-cloudimg-amd64.img). Ubuntu's per-build
download URLs change daily under that same stable basename
(.../release-20260713/... vs .../release-20260714/...), so a previous
day's cached file satisfied the "already cached" check and got checksum-
verified against the *new* day's expected hash from NetOrk's daily catalog
sync — failing outright and aborting the whole provisioning job, even
though a plain retry would have re-downloaded and succeeded (the bad file
was already being deleted on mismatch, just never re-fetched).

Found live during a NetOrk deploy: "Checksum mismatch for
https://cloud-images.ubuntu.com/.../release-20260713/
ubuntu-26.04-server-cloudimg-amd64.img: expected 0826c500..., got
3ee4f67f...".

Fix: key the cache path on a hash of the full URL (not just the
basename), and retry the download once after a checksum-mismatch cleanup
before raising.
This commit is contained in:
Christian Manivong
2026-07-14 16:23:13 +02:00
parent 3181ade728
commit 38f0c0a656
2 changed files with 112 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
napalm_proxmox/vm_provision_mixin.py
+46 -20
View File
@@ -3,6 +3,7 @@
from __future__ import annotations
import base64
import hashlib
import logging
import time
import yaml
@@ -74,38 +75,63 @@ class ProxmoxVMProvisionMixin:
"""
Download image_url to the node's image cache dir if not already present.
Returns the local path on the hypervisor node. Verifies image_checksum
(format "<algo>:<hex>", e.g. "sha256:abc123...") if given, re-downloading
is left to the caller's next attempt if verification fails.
Returns the local path on the hypervisor node. The cache filename is
prefixed with a hash of the *full* URL, not just its basename —
Ubuntu (and others) publish per-build URLs that change daily under a
stable basename (e.g. .../release-20260713/ubuntu-26.04-server-
cloudimg-amd64.img), so keying the cache on the basename alone let a
stale previous-day build satisfy the "already cached" check and fail
checksum verification against today's expected hash.
Verifies image_checksum (format "<algo>:<hex>", e.g. "sha256:abc123...")
if given. On mismatch, removes the bad file and retries the download
once (covers a corrupted/partial transfer or a stale same-keyed file)
before raising.
"""
filename = image_url.rstrip("/").rsplit("/", 1)[-1]
local_path = f"{_IMAGE_CACHE_DIR}/{filename}"
url_hash = hashlib.sha256(image_url.encode()).hexdigest()[:12]
local_path = f"{_IMAGE_CACHE_DIR}/{url_hash}-{filename}"
exists = self._run_node_command(
f"mkdir -p {_IMAGE_CACHE_DIR} && test -f {local_path} && echo EXISTS || echo MISSING",
timeout=30,
)
if "EXISTS" not in exists:
_logger.info(f"Downloading cloud image {image_url} -> {local_path}")
self._run_node_command(
f"wget -q -O {local_path}.tmp '{image_url}' && mv {local_path}.tmp {local_path}",
timeout=timeout,
algo, _, expected = (image_checksum or "").partition(":")
algo = (algo or "sha256").lower()
max_attempts = 2
for attempt in range(1, max_attempts + 1):
exists = self._run_node_command(
f"mkdir -p {_IMAGE_CACHE_DIR} && test -f {local_path} "
f"&& echo EXISTS || echo MISSING",
timeout=30,
)
if "EXISTS" not in exists:
_logger.info(f"Downloading cloud image {image_url} -> {local_path}")
self._run_node_command(
f"wget -q -O {local_path}.tmp '{image_url}' "
f"&& mv {local_path}.tmp {local_path}",
timeout=timeout,
)
if not image_checksum:
return local_path
if image_checksum:
algo, _, expected = image_checksum.partition(":")
algo = (algo or "sha256").lower()
actual = self._run_node_command(
f"{algo}sum {local_path} | awk '{{print $1}}'", timeout=60
)
if actual.lower() != expected.lower():
# Remove the bad file so a retry re-downloads instead of reusing it.
self._run_node_command(f"rm -f {local_path}", timeout=30)
if actual.lower() == expected.lower():
return local_path
# Remove the bad file so the next attempt re-downloads instead of
# reusing it.
self._run_node_command(f"rm -f {local_path}", timeout=30)
if attempt == max_attempts:
raise RuntimeError(
f"Checksum mismatch for {image_url}: expected {expected}, got {actual}"
)
_logger.warning(
f"Checksum mismatch for {image_url} on attempt {attempt}/{max_attempts} "
"— retrying download"
)
return local_path
raise AssertionError("unreachable") # loop always returns or raises above
def _find_default_image_storage(self) -> str:
"""Find a storage suitable for VM root disks (content includes 'images').
tests/test_vm_provision_mixin.py
+66 -3
View File
@@ -678,10 +678,73 @@ def test_download_cloud_image_verifies_matching_checksum():
assert path.endswith("debian-12-genericcloud-amd64.qcow2")
def test_download_cloud_image_checksum_mismatch_raises_and_removes_file():
def test_download_cloud_image_cache_key_differs_for_same_basename_different_url():
"""Regression: Ubuntu's per-build URLs change daily under a stable
basename (ubuntu-26.04-server-cloudimg-amd64.img) — a cache key derived
from just the basename let a stale build from a previous day collide
with today's cache lookup, skip re-downloading, and fail checksum
verification against today's expected hash. Gitea issue filed for this."""
mixin = ProxmoxVMProvisionMixin()
mixin._run_node_command = MagicMock(return_value="MISSING")
url_a = (
"https://cloud-images.ubuntu.com/releases/server/releases/resolute/"
"release-20260713/ubuntu-26.04-server-cloudimg-amd64.img"
)
url_b = (
"https://cloud-images.ubuntu.com/releases/server/releases/resolute/"
"release-20260714/ubuntu-26.04-server-cloudimg-amd64.img"
)
path_a = mixin._download_cloud_image(url_a, None, timeout=300)
path_b = mixin._download_cloud_image(url_b, None, timeout=300)
assert path_a != path_b
assert path_a.endswith("ubuntu-26.04-server-cloudimg-amd64.img")
assert path_b.endswith("ubuntu-26.04-server-cloudimg-amd64.img")
def test_download_cloud_image_checksum_mismatch_retries_and_succeeds():
"""The reported bug's actual scenario: a stale same-named cache entry
fails checksum, gets removed, and the retry re-downloads + verifies
successfully instead of failing the whole provisioning job outright."""
mixin = ProxmoxVMProvisionMixin()
mixin._run_node_command = MagicMock(
side_effect=["EXISTS", "wrong-checksum", ""] # existence, checksum, rm
side_effect=[
"EXISTS", # attempt 1: stale file already present
"wrong-checksum", # attempt 1: checksum against stale content
"", # rm -f
"MISSING", # attempt 2: file gone, re-download
"", # wget
"abc123", # attempt 2: checksum against fresh content
]
)
path = mixin._download_cloud_image(
"https://cloud-images.ubuntu.com/releases/server/releases/resolute/"
"release-20260713/ubuntu-26.04-server-cloudimg-amd64.img",
"sha256:abc123",
timeout=300,
)
assert path.endswith("ubuntu-26.04-server-cloudimg-amd64.img")
commands = [c[0][0] for c in mixin._run_node_command.call_args_list]
assert sum(1 for cmd in commands if "wget" in cmd) == 1
assert sum(1 for cmd in commands if cmd.startswith("rm -f")) == 1
def test_download_cloud_image_checksum_mismatch_persists_raises_after_retry():
mixin = ProxmoxVMProvisionMixin()
mixin._run_node_command = MagicMock(
side_effect=[
"EXISTS",
"wrong-checksum",
"", # rm -f (attempt 1)
"MISSING",
"", # wget
"still-wrong",
"", # rm -f (attempt 2)
]
)
with pytest.raises(RuntimeError, match="Checksum mismatch"):
@@ -692,7 +755,7 @@ def test_download_cloud_image_checksum_mismatch_raises_and_removes_file():
)
commands = [c[0][0] for c in mixin._run_node_command.call_args_list]
assert any(cmd.startswith("rm -f") for cmd in commands)
assert sum(1 for cmd in commands if cmd.startswith("rm -f")) == 2
# ---------------------------------------------------------------------------