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

fix(vm_provision_mixin): query node-scoped storage, not cluster-wide

Browse Source 
The cluster-wide /storage endpoint lists every storage regardless of
its "nodes" restriction, so _find_default_image_storage (and the
snippet-storage lookup) could pick a storage not actually available on
the node the VM is being created on. On a real server this stranded a
freshly-created VM shell with no disk attached: "qm importdisk" failed
with "storage 'local-lvm' is not available on node 'pve-02'" after the
VM (VMID 103) already existed. Querying /nodes/{node}/storage instead
fixes this, since Proxmox itself only lists what's available there.

Also adds get_image_storages() and an optional storage= override on
create_vm_from_cloud_init, so callers aren't stuck with auto-detection.
This commit is contained in:
Christian Manivong
2026-07-07 22:36:14 +02:00
parent 12135735cb
commit 4d568bc6dc
2 changed files with 208 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_vm_provision_mixin.py
+165 -21
View File
@@ -74,12 +74,12 @@ def test_create_vm_from_cloud_init_single_nic():
# Mock API hierarchy
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 101
mock_api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1},
{"storage": "snippets", "type": "dir", "content": "snippets", "enabled": 1},
]
mock_node = MagicMock()
mixin._api = mock_api
mixin._node_api = MagicMock(return_value=mock_node)
mixin._download_cloud_image = MagicMock(return_value="/var/lib/vz/template/netork-images/debian-12.qcow2")
@@ -139,12 +139,12 @@ def test_create_vm_from_cloud_init_dual_nic_trunk():
# Mock API hierarchy
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 102
mock_api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1},
{"storage": "snippets", "type": "dir", "content": "snippets", "enabled": 1},
]
mock_node = MagicMock()
mixin._api = mock_api
mixin._node_api = MagicMock(return_value=mock_node)
mixin._download_cloud_image = MagicMock(return_value="/var/lib/vz/template/netork-images/debian-12.qcow2")
@@ -213,13 +213,13 @@ def test_create_vm_missing_snippet_storage():
# Mock API with images storage but no snippet storage
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 101
mock_api.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1}
]
mixin._api = mock_api
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1}
]
mixin._node_api = MagicMock(return_value=mock_node)
mixin._download_cloud_image = MagicMock(return_value="/var/lib/vz/template/netork-images/debian-12.qcow2")
mixin._run_node_command = MagicMock(return_value="")
@@ -254,12 +254,12 @@ def test_create_vm_with_disk_resize():
# Mock API hierarchy
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 103
mock_api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1},
{"storage": "snippets", "type": "dir", "content": "snippets", "enabled": 1},
]
mock_node = MagicMock()
mixin._api = mock_api
mixin._node_api = MagicMock(return_value=mock_node)
mixin._download_cloud_image = MagicMock(return_value="/var/lib/vz/template/netork-images/debian-12.qcow2")
@@ -652,10 +652,11 @@ def test_find_default_image_storage_treats_absent_enabled_as_enabled():
treated as falsy, causing every real Proxmox server to report no usable
image storage even when several existed."""
mixin = ProxmoxVMProvisionMixin()
mixin._api = MagicMock()
mixin._api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir"},
]
mixin._node_api = MagicMock(return_value=mock_node)
storage = mixin._find_default_image_storage()
@@ -664,11 +665,12 @@ def test_find_default_image_storage_treats_absent_enabled_as_enabled():
def test_find_default_image_storage_excludes_explicitly_disabled():
mixin = ProxmoxVMProvisionMixin()
mixin._api = MagicMock()
mixin._api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "old-storage", "type": "dir", "content": "images", "enabled": 0},
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1},
]
mixin._node_api = MagicMock(return_value=mock_node)
storage = mixin._find_default_image_storage()
@@ -677,15 +679,38 @@ def test_find_default_image_storage_excludes_explicitly_disabled():
def test_find_default_image_storage_raises_when_none_found():
mixin = ProxmoxVMProvisionMixin()
mixin._api = MagicMock()
mixin._api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local", "type": "dir", "content": "backup,iso,vztmpl"},
]
mixin._node_api = MagicMock(return_value=mock_node)
with pytest.raises(ValueError, match="images"):
mixin._find_default_image_storage()
def test_find_default_image_storage_excludes_storage_restricted_to_other_nodes():
"""Regression: a storage configured cluster-wide but restricted via 'nodes'
to other cluster members must not be picked — /nodes/{node}/storage (unlike
the cluster-wide /storage endpoint) only lists what's actually available on
this node, so querying it is what makes this exclusion happen naturally.
This was the real-world bug: local-lvm (nodes=pve-garden) was picked for a
VM on pve-02, and 'qm importdisk' failed with 'storage not available on
node', leaving a VM shell with no disk attached."""
mixin = ProxmoxVMProvisionMixin()
mock_node = MagicMock()
# /nodes/pve-02/storage never even lists local-lvm — Proxmox itself filters
# node-restricted storages out of this endpoint.
mock_node.storage.get.return_value = [
{"storage": "local-zfs", "type": "zfspool", "content": "images,rootdir"},
]
mixin._node_api = MagicMock(return_value=mock_node)
storage = mixin._find_default_image_storage()
assert storage == "local-zfs"
def test_create_vm_from_cloud_init_with_real_world_storage_shape():
"""Regression: real Proxmox servers omit "enabled" for never-toggled storages
(observed live: local-lvm/local-zfs/fast-zfs all had content=images but no
@@ -697,14 +722,14 @@ def test_create_vm_from_cloud_init_with_real_world_storage_shape():
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 104
mock_api.storage.get.return_value = [
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir"},
{"storage": "local-zfs", "type": "zfspool", "content": "rootdir,images"},
{"storage": "local", "type": "dir", "content": "backup,iso,vztmpl"},
{"storage": "snippets", "type": "dir", "content": "snippets"},
]
mock_node = MagicMock()
mixin._api = mock_api
mixin._node_api = MagicMock(return_value=mock_node)
mixin._download_cloud_image = MagicMock(return_value="/var/lib/vz/template/netork-images/debian-12.qcow2")
@@ -739,3 +764,122 @@ def test_create_vm_from_cloud_init_with_real_world_storage_shape():
)
assert result["vmid"] == "104"
def test_create_vm_from_cloud_init_explicit_storage_skips_auto_detect():
"""When storage= is given explicitly, it's used directly and
_find_default_image_storage is never consulted (so an explicit choice
always wins, even if auto-detect would have picked something else)."""
mixin = ProxmoxVMProvisionMixin()
mixin._node_name = "pve1"
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 105
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-lvm", "type": "lvmthin", "content": "images,rootdir", "enabled": 1},
{"storage": "fast-zfs", "type": "zfspool", "content": "images,rootdir", "enabled": 1},
{"storage": "snippets", "type": "dir", "content": "snippets", "enabled": 1},
]
mixin._api = mock_api
mixin._node_api = MagicMock(return_value=mock_node)
mixin._download_cloud_image = MagicMock(return_value="/var/lib/vz/template/netork-images/debian-12.qcow2")
mixin._find_default_image_storage = MagicMock(side_effect=AssertionError("should not be called"))
mixin._run_node_command = MagicMock(return_value="")
mock_vm = MagicMock()
mock_node.qemu.return_value = mock_vm
mock_node.qemu.post.return_value = None
mock_vm.config.post.return_value = None
mock_vm.config.get.return_value = {
"unused0": "fast-zfs:vm-105-disk-0",
"scsi0": "fast-zfs:vm-105-disk-0",
}
mock_vm.status.start.post.return_value = "UPID:pve1:131:start"
mock_task = MagicMock()
mock_task.status.get.return_value = {"status": "stopped", "exitstatus": "OK"}
mock_node.tasks.return_value = mock_task
mock_storage = MagicMock()
mock_storage.upload.post.return_value = {"filename": "snippets:snippets/105-user-data.yaml"}
mock_node.storage.return_value = mock_storage
with patch("time.sleep"):
result = mixin.create_vm_from_cloud_init(
name="explicit-storage-vm",
image_url="https://cloud.debian.org/images/cloud/bookworm/latest/debian-12-genericcloud-amd64.qcow2",
cpu=2,
memory=2048,
nics=[{"bridge": "vmbr0"}],
cloud_init_config={"hostname": "explicit-storage-vm"},
storage="fast-zfs",
)
assert result["vmid"] == "105"
mixin._find_default_image_storage.assert_not_called()
import_cmd = next(
c[0][0] for c in mixin._run_node_command.call_args_list if "qm importdisk" in c[0][0]
)
assert "fast-zfs" in import_cmd
assert "local-lvm" not in import_cmd
# ---------------------------------------------------------------------------
# get_image_storages
# ---------------------------------------------------------------------------
def test_get_image_storages_filters_to_images_content():
mixin = ProxmoxVMProvisionMixin()
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{
"storage": "local-zfs",
"type": "zfspool",
"content": "images,rootdir",
"total": 100 * 1024**3,
"avail": 40 * 1024**3,
},
{"storage": "local", "type": "dir", "content": "backup,iso,vztmpl,snippets"},
]
mixin._node_api = MagicMock(return_value=mock_node)
targets = mixin.get_image_storages()
assert len(targets) == 1
assert targets[0]["name"] == "local-zfs"
assert targets[0]["type"] == "zfspool"
assert targets[0]["total_gb"] == 100.0
assert targets[0]["available_gb"] == 40.0
def test_get_image_storages_excludes_disabled_and_inactive():
mixin = ProxmoxVMProvisionMixin()
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "disabled-store", "type": "dir", "content": "images", "enabled": 0},
{"storage": "inactive-store", "type": "nfs", "content": "images", "active": 0},
{"storage": "ok-store", "type": "dir", "content": "images"},
]
mixin._node_api = MagicMock(return_value=mock_node)
targets = mixin.get_image_storages()
assert [t["name"] for t in targets] == ["ok-store"]
def test_get_image_storages_excludes_storage_restricted_to_other_nodes():
"""Node-scoped query naturally excludes storages Proxmox itself doesn't
list for this node (e.g. restricted via 'nodes' to other cluster members)."""
mixin = ProxmoxVMProvisionMixin()
mock_node = MagicMock()
mock_node.storage.get.return_value = [
{"storage": "local-zfs", "type": "zfspool", "content": "images,rootdir"},
]
mixin._node_api = MagicMock(return_value=mock_node)
targets = mixin.get_image_storages()
assert [t["name"] for t in targets] == ["local-zfs"]