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

Merge fix/snippet-write-via-ssh: write Cloud-Init snippet via SSH

Browse Source
This commit is contained in:
Christian Manivong
2026-07-07 23:24:10 +02:00
parent 86af2cb7a7 685d9b67ae
commit c8d45e4336
2 changed files with 110 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
napalm_proxmox/vm_provision_mixin.py
+32 -14
View File
@@ -2,7 +2,7 @@
from __future__ import annotations
import io
import base64
import logging
import time
import yaml
@@ -130,6 +130,25 @@ class ProxmoxVMProvisionMixin:
"No storage with content='images' found. Configure a storage for VM disks."
)
def _get_storage_path(self, storage: str) -> str:
"""Resolve a storage's filesystem path on the node.
Needed to write Cloud-Init snippets directly: Proxmox's
/storage/{s}/upload API only accepts content in {iso, vztmpl,
import} — "snippets" is rejected outright, so snippets must be
written straight to the filesystem instead. Only dir-backed storages
(dir, nfs, cifs, cephfs) expose "path"; those are also the only
storage types Proxmox itself allows content='snippets' on.
"""
config = self._api.storage(storage).get()
path = config.get("path")
if not path:
raise ValueError(
f"Storage '{storage}' has no filesystem path (content='snippets' "
"requires a dir/nfs/cifs/cephfs-backed storage)"
)
return path
def get_image_storages(self) -> List[StorageTargetDict]:
"""List node-available storage pools suitable for a new VM's root disk."""
targets: List[StorageTargetDict] = []
@@ -336,20 +355,19 @@ class ProxmoxVMProvisionMixin:
)
filename = f"{vmid}-user-data.yaml"
_logger.debug(f"Uploading Cloud-Init snippet {filename} to {snippet_storage}")
_logger.debug(f"Writing Cloud-Init snippet {filename} to {snippet_storage}")
# Upload to snippet storage. Proxmox's upload endpoint expects the
# "filename" parameter to BE the file (multipart), not a name
# string with separate content — proxmoxer only builds a
# multipart request when the value is an io.IOBase instance,
# otherwise it silently sends everything as a plain
# form-urlencoded POST, which real Proxmox rejects by dropping
# the connection (RemoteDisconnected, no HTTP response at all).
file_obj = io.BytesIO(user_data_yaml.encode("utf-8"))
file_obj.name = filename
self._node_api().storage(snippet_storage).upload.post(
content="snippets",
filename=file_obj,
# Proxmox's /storage/{s}/upload API only accepts content in
# {iso, vztmpl, import} — "snippets" is rejected outright
# ("does not have a value in the enumeration"). Snippets can only
# be written directly to the filesystem, so resolve the storage's
# backing path and write the file over SSH instead.
storage_path = self._get_storage_path(snippet_storage)
encoded = base64.b64encode(user_data_yaml.encode("utf-8")).decode("ascii")
self._run_node_command(
f"mkdir -p {storage_path}/snippets && "
f"echo {encoded} | base64 -d > {storage_path}/snippets/{filename}",
timeout=30,
)
# Step 7: Configure Cloud-Init references and SSH keys
tests/test_vm_provision_mixin.py
+78 -57
View File
@@ -2,7 +2,7 @@
from __future__ import annotations
import io
import base64
import pytest
from unittest.mock import MagicMock, patch
@@ -76,6 +76,7 @@ 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.return_value.get.return_value = {"path": "/var/lib/vz"}
mock_node = MagicMock()
mock_node.storage.get.return_value = [
@@ -103,11 +104,6 @@ def test_create_vm_from_cloud_init_single_nic():
mock_task.status.get.return_value = {"status": "stopped", "exitstatus": "OK"}
mock_node.tasks.return_value = mock_task
# Mock storage upload
mock_storage = MagicMock()
mock_storage.upload.post.return_value = {"filename": "snippets:snippets/101-user-data.yaml"}
mock_node.storage.return_value = mock_storage
with patch("time.sleep"):
result = mixin.create_vm_from_cloud_init(
name="test-vm",
@@ -132,14 +128,15 @@ def test_create_vm_from_cloud_init_single_nic():
assert "tag=10" in net_call_args[1]["net0"]
assert "vmbr0" in net_call_args[1]["net0"]
# Regression: the snippet must be uploaded as an actual file (io.IOBase),
# not a plain filename string with a separate "data" field — proxmoxer
# only builds a real multipart request for io.IOBase values, and real
# Proxmox drops the connection outright for anything else (see
# test_create_vm_uploads_snippet_as_file_object for the dedicated check).
upload_kwargs = mock_storage.upload.post.call_args[1]
assert "data" not in upload_kwargs
assert isinstance(upload_kwargs["filename"], io.IOBase)
# Regression: the snippet must be written directly to the filesystem via
# SSH, not uploaded via the /storage/upload API — real Proxmox rejects
# content='snippets' on that endpoint outright (see
# test_create_vm_writes_snippet_via_ssh_with_correct_content for the
# dedicated check).
write_cmd = next(
c[0][0] for c in mixin._run_node_command.call_args_list if "snippets/101-user-data.yaml" in c[0][0]
)
assert "/var/lib/vz/snippets" in write_cmd
def test_create_vm_from_cloud_init_dual_nic_trunk():
@@ -150,6 +147,7 @@ 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.return_value.get.return_value = {"path": "/var/lib/vz"}
mock_node = MagicMock()
mock_node.storage.get.return_value = [
@@ -177,11 +175,6 @@ def test_create_vm_from_cloud_init_dual_nic_trunk():
mock_task.status.get.return_value = {"status": "stopped", "exitstatus": "OK"}
mock_node.tasks.return_value = mock_task
# Mock storage upload
mock_storage = MagicMock()
mock_storage.upload.post.return_value = {"filename": "snippets:snippets/102-user-data.yaml"}
mock_node.storage.return_value = mock_storage
with patch("time.sleep"):
result = mixin.create_vm_from_cloud_init(
name="wireshark-sat-1",
@@ -265,6 +258,7 @@ def test_create_vm_with_disk_resize():
# Mock API hierarchy
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 103
mock_api.storage.return_value.get.return_value = {"path": "/var/lib/vz"}
mock_node = MagicMock()
mock_node.storage.get.return_value = [
@@ -293,11 +287,6 @@ def test_create_vm_with_disk_resize():
mock_task.status.get.return_value = {"status": "stopped", "exitstatus": "OK"}
mock_node.tasks.return_value = mock_task
# Mock storage upload
mock_storage = MagicMock()
mock_storage.upload.post.return_value = {"filename": "snippets:snippets/103-user-data.yaml"}
mock_node.storage.return_value = mock_storage
with patch("time.sleep"):
result = mixin.create_vm_from_cloud_init(
name="big-vm",
@@ -722,6 +711,42 @@ def test_find_default_image_storage_excludes_storage_restricted_to_other_nodes()
assert storage == "local-zfs"
# ---------------------------------------------------------------------------
# _get_storage_path
# ---------------------------------------------------------------------------
def test_get_storage_path_returns_path_from_cluster_config():
mixin = ProxmoxVMProvisionMixin()
mixin._api = MagicMock()
mixin._api.storage.return_value.get.return_value = {
"storage": "local",
"type": "dir",
"path": "/var/lib/vz",
}
path = mixin._get_storage_path("local")
assert path == "/var/lib/vz"
mixin._api.storage.assert_called_with("local")
def test_get_storage_path_raises_when_storage_has_no_path():
"""A storage type without a filesystem path (e.g. lvmthin, zfspool) can't
back content='snippets' at all — Proxmox itself only allows that content
type on dir/nfs/cifs/cephfs storages, so this should never actually be
reached for a real snippet storage, but must fail clearly if it is."""
mixin = ProxmoxVMProvisionMixin()
mixin._api = MagicMock()
mixin._api.storage.return_value.get.return_value = {
"storage": "local-lvm",
"type": "lvmthin",
}
with pytest.raises(ValueError, match="path"):
mixin._get_storage_path("local-lvm")
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
@@ -733,6 +758,7 @@ 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.return_value.get.return_value = {"path": "/var/lib/vz"}
mock_node = MagicMock()
mock_node.storage.get.return_value = [
@@ -760,10 +786,6 @@ def test_create_vm_from_cloud_init_with_real_world_storage_shape():
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/104-user-data.yaml"}
mock_node.storage.return_value = mock_storage
with patch("time.sleep"):
result = mixin.create_vm_from_cloud_init(
name="real-shape-vm",
@@ -786,6 +808,7 @@ def test_create_vm_from_cloud_init_explicit_storage_skips_auto_detect():
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 105
mock_api.storage.return_value.get.return_value = {"path": "/var/lib/vz"}
mock_node = MagicMock()
mock_node.storage.get.return_value = [
@@ -813,10 +836,6 @@ def test_create_vm_from_cloud_init_explicit_storage_skips_auto_detect():
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",
@@ -901,21 +920,22 @@ def test_get_image_storages_excludes_storage_restricted_to_other_nodes():
# ---------------------------------------------------------------------------
def test_create_vm_uploads_snippet_as_file_object_with_correct_content():
"""Regression: real Proxmox's /storage/{s}/upload endpoint expects the
"filename" parameter to be the file itself (multipart). proxmoxer only
builds a multipart request when the value is an io.IOBase instance —
passing a plain string (with a separate, nonexistent "data" field, as the
old code did) makes proxmoxer send a normal form-urlencoded POST instead,
which real Proxmox responds to by closing the connection outright
(observed live: requests.exceptions.ConnectionError /
RemoteDisconnected('Remote end closed connection without response'),
after the VM shell and disk import had already succeeded)."""
def test_create_vm_writes_snippet_via_ssh_with_correct_content():
"""Regression: real Proxmox's /storage/{s}/upload endpoint rejects
content='snippets' outright ("value 'snippets' does not have a value in
the enumeration 'iso, vztmpl, import'") — that endpoint only handles
ISOs, container templates, and imports. Snippets can only be written
directly to the storage's filesystem path, so this must go through SSH
(_run_node_command), not the upload API.
(observed live: 400 Bad Request from Proxmox, right after the earlier
multipart-upload fix had already gotten past a prior RemoteDisconnected
bug at the same step — two distinct real-world failures at this line)."""
mixin = ProxmoxVMProvisionMixin()
mixin._node_name = "pve1"
mock_api = MagicMock()
mock_api.cluster.nextid.get.return_value = 106
mock_api.storage.return_value.get.return_value = {"path": "/mnt/pve/snippet-nfs"}
mock_node = MagicMock()
mock_node.storage.get.return_value = [
@@ -941,28 +961,29 @@ def test_create_vm_uploads_snippet_as_file_object_with_correct_content():
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": "local:snippets/106-user-data.yaml"}
mock_node.storage.return_value = mock_storage
with patch("time.sleep"):
mixin.create_vm_from_cloud_init(
name="upload-shape-vm",
name="write-shape-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": "upload-shape-vm", "chpasswd": {"expire": False}},
cloud_init_config={"hostname": "write-shape-vm", "chpasswd": {"expire": False}},
)
upload_call = mock_storage.upload.post.call_args
assert upload_call[1]["content"] == "snippets"
assert "data" not in upload_call[1]
# No call to the upload API at all — snippets aren't a valid content
# type there.
mock_node.storage.assert_not_called()
file_obj = upload_call[1]["filename"]
assert isinstance(file_obj, io.IOBase)
assert file_obj.name == "106-user-data.yaml"
file_obj.seek(0)
content = file_obj.read().decode("utf-8")
write_cmd = next(
c[0][0]
for c in mixin._run_node_command.call_args_list
if "snippets/106-user-data.yaml" in c[0][0]
)
assert "mkdir -p /mnt/pve/snippet-nfs/snippets" in write_cmd
assert "/mnt/pve/snippet-nfs/snippets/106-user-data.yaml" in write_cmd
encoded = write_cmd.split("echo ", 1)[1].split(" | base64 -d")[0]
content = base64.b64decode(encoded).decode("utf-8")
assert content.startswith("#cloud-config\n")
assert "hostname: upload-shape-vm" in content
assert "hostname: write-shape-vm" in content