From 1ce0a0b6f276544851e9c75a2ee8699ace292c67 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Thu, 24 Sep 2026 09:06:36 +0200 Subject: [PATCH 1/2] feat!: a VM's vmid is a string, and its config can describe its hardware VMDict.vmid and VMConfigDict.vmid were int. Proxmox numbers its guests, but VMware identifies a VM by UUID, which an int cannot hold. The provisioning dicts already carried vmid as a string; the read side now matches. Proxmox reports "100". VMConfigDict gains optional hardware details -- os_name, cpu_type, sockets, cores_per_socket, firmware, machine and passthrough (PCI/USB, as VMPassthroughDict) -- so netOrk's VM hardware view can be filled by any hypervisor instead of reading Proxmox's raw config through the driver's private API. Also fixes the README's hypervisor example, which still named the pre-contract snapshot_create. BREAKING CHANGE: VMDict.vmid and VMConfigDict.vmid are str. --- README.md | 2 +- napalm_device_types/hypervisor.py | 40 +++++++++++++++-------- napalm_device_types/models.py | 20 ++++++++++-- pyproject.toml | 2 +- tests/test_vm_identifiers.py | 53 +++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 18 deletions(-) create mode 100644 tests/test_vm_identifiers.py diff --git a/README.md b/README.md index 7e71a30..f15dff5 100644 --- a/README.md +++ b/README.md @@ -238,7 +238,7 @@ class ProxmoxDriver(HypervisorDriver): # return List[VMDict] ... - def snapshot_create(self, name, snapshot, description="", include_memory=False): + def create_vm_snapshot(self, name, snapshot, description="", include_memory=False): ... ``` diff --git a/napalm_device_types/hypervisor.py b/napalm_device_types/hypervisor.py index cf38562..268e8df 100644 --- a/napalm_device_types/hypervisor.py +++ b/napalm_device_types/hypervisor.py @@ -59,7 +59,8 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri Each entry contains: * name (string) - VM display name - * vmid (int) - hypervisor-internal numeric ID + * vmid (string) - hypervisor-internal ID (Proxmox: ``"100"``, + VMware: the instance UUID) * status (string) - ``"running"``, ``"stopped"``, ``"paused"``, ``"suspended"`` * vcpus (int) - number of virtual CPUs assigned * memory (int) - configured RAM in megabytes @@ -73,7 +74,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri [ { "name": "web01", - "vmid": 100, + "vmid": "100", "status": "running", "vcpus": 4, "memory": 8192, @@ -84,7 +85,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri }, { "name": "db-backup", - "vmid": 101, + "vmid": "101", "status": "stopped", "vcpus": 2, "memory": 4096, @@ -101,13 +102,14 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri """ Returns the full hardware configuration of a virtual machine. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :raises ValueError: If no VM with the given name/ID exists. The returned dictionary contains: * name (string) - VM display name - * vmid (int) - hypervisor-internal numeric ID + * vmid (string) - hypervisor-internal ID (Proxmox: ``"100"``, + VMware: the instance UUID) * vcpus (int) - number of virtual CPUs * memory (int) - RAM in megabytes * os_type (string) - guest OS type hint (e.g. ``"l26"``, ``"win11"``, ``"other"``) @@ -131,11 +133,21 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri * description (string) - free-text notes / description * tags (list of strings) - organisational tags + Optional, present only where the hypervisor exposes them: + + * os_name (string) - human-readable guest OS + * cpu_type (string) - emulated CPU model + * sockets (int), cores_per_socket (int) - vCPU topology + * firmware (string) - ``"bios"`` or ``"efi"`` + * machine (string) - machine type / virtual hardware version + * passthrough (list) - host devices handed to the VM, each with + ``slot``, ``kind`` (``"pci"``/``"usb"``) and ``config`` + Example:: { "name": "web01", - "vmid": 100, + "vmid": "100", "vcpus": 4, "memory": 8192, "os_type": "l26", @@ -174,7 +186,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri The method blocks until the hypervisor reports the VM as running. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :raises ValueError: If no VM with the given name/ID exists. :raises RuntimeError: If the VM cannot be started (e.g. resource limit). @@ -192,7 +204,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri and the method blocks until the VM is stopped. With ``force=True`` the VM is immediately powered off (equivalent to pulling the plug). - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :param force: ``True`` for immediate power-off, ``False`` for graceful shutdown. :raises ValueError: If no VM with the given name/ID exists. :raises RuntimeError: If the VM is already stopped. @@ -211,7 +223,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri With ``force=False`` (default) a graceful ACPI reboot is requested. With ``force=True`` the VM is reset immediately without OS shutdown. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :param force: ``True`` for an immediate reset, ``False`` for graceful reboot. :raises ValueError: If no VM with the given name/ID exists. :raises RuntimeError: If the VM is not currently running. @@ -228,7 +240,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri Suspends (pauses) a running virtual machine, preserving its in-memory state. The VM can be resumed with :meth:`start_vm`. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :raises ValueError: If no VM with the given name/ID exists. :raises RuntimeError: If the VM is not currently running. @@ -246,7 +258,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri """ Returns all snapshots of a virtual machine. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :raises ValueError: If no VM with the given name/ID exists. Each entry contains: @@ -286,7 +298,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri """ Creates a snapshot of a virtual machine. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :param snapshot: Name for the new snapshot. :param description: Optional human-readable description. :param include_memory: Whether to include the current RAM state @@ -306,7 +318,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri """ Deletes a snapshot of a virtual machine. - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :param snapshot: Name of the snapshot to delete. :raises ValueError: If the VM or snapshot does not exist. :raises RuntimeError: If other snapshots depend on this one (must delete children first). @@ -324,7 +336,7 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri The VM is stopped (if running), reverted, and then left in the state the snapshot recorded (running or stopped depending on ``has_memory``). - :param name: VM name or numeric VMID as a string. + :param name: VM name or its ``vmid``. :param snapshot: Name of the snapshot to roll back to. :raises ValueError: If the VM or snapshot does not exist. :raises RuntimeError: If the rollback fails. diff --git a/napalm_device_types/models.py b/napalm_device_types/models.py index 3e3a9fa..5319808 100644 --- a/napalm_device_types/models.py +++ b/napalm_device_types/models.py @@ -512,7 +512,7 @@ class VMNICDict(TypedDict): class VMDict(TypedDict): name: str - vmid: int + vmid: str status: str vcpus: int memory: int @@ -522,9 +522,17 @@ class VMDict(TypedDict): node: str +class VMPassthroughDict(TypedDict): + """A host device handed through to a VM (PCI, USB).""" + + slot: str # hypervisor's device key, e.g. "hostpci0" + kind: str # "pci" or "usb" + config: str # hypervisor's own description of the device + + class VMConfigDict(TypedDict): name: str - vmid: int + vmid: str vcpus: int memory: int os_type: str @@ -533,6 +541,14 @@ class VMConfigDict(TypedDict): nics: List[VMNICDict] description: str tags: List[str] + # Hardware details not every hypervisor exposes; absent when unknown. + os_name: NotRequired[str] # human-readable guest OS, e.g. "Ubuntu Linux (64-bit)" + cpu_type: NotRequired[str] # e.g. "host", "kvm64" + sockets: NotRequired[int] + cores_per_socket: NotRequired[int] + firmware: NotRequired[str] # "bios" or "efi" + machine: NotRequired[str] # machine type / virtual hardware version + passthrough: NotRequired[List[VMPassthroughDict]] class StorageVolumeDict(TypedDict): diff --git a/pyproject.toml b/pyproject.toml index a1042e6..32eab9d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "napalm-device-types" -version = "1.0.0" +version = "2.0.0" description = "Abstract device-type base classes for NAPALM drivers" readme = "README.md" requires-python = ">=3.10" diff --git a/tests/test_vm_identifiers.py b/tests/test_vm_identifiers.py new file mode 100644 index 0000000..d38f6e2 --- /dev/null +++ b/tests/test_vm_identifiers.py @@ -0,0 +1,53 @@ +"""A VM's ``vmid`` is a string on every hypervisor. + +Proxmox numbers its guests, but VMware identifies them by UUID or MoRef +(``"vm-42"``). An ``int`` in the contract forced netOrk to call ``int()`` on +whatever came back, which cannot represent the second kind at all. The +provisioning dicts already carried ``vmid`` as a string; these pin the +read-side dicts to the same type. +""" + +from __future__ import annotations + +from typing import get_type_hints + +import pytest +from napalm_device_types.models import ( + VMConfigDict, + VMDict, + VMProvisionResultDict, +) + + +@pytest.mark.parametrize("model", [VMDict, VMConfigDict, VMProvisionResultDict]) +def test_vmid_is_a_string(model): + assert get_type_hints(model)["vmid"] is str + + +class TestVMConfigCarriesWhatAHardwareViewShows: + """netOrk's VM hardware view used to read Proxmox's raw config through the + driver's private API. The contract has to carry those details so a second + hypervisor can fill the same view -- optionally, since not every platform + has every one of them.""" + + OPTIONAL = { + "os_name", + "cpu_type", + "sockets", + "cores_per_socket", + "firmware", + "machine", + "passthrough", + } + + def test_hardware_details_are_optional_fields(self): + assert self.OPTIONAL <= VMConfigDict.__optional_keys__ + + def test_contract_core_stays_required(self): + assert "vmid" in VMConfigDict.__required_keys__ + assert "disks" in VMConfigDict.__required_keys__ + + def test_passthrough_entry_shape(self): + from napalm_device_types.models import VMPassthroughDict + + assert get_type_hints(VMPassthroughDict) == {"slot": str, "kind": str, "config": str} -- 2.54.0 From 34b8f10ffa4676405e9eb261d28d0882386e5781 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Thu, 24 Sep 2026 10:00:04 +0200 Subject: [PATCH 2/2] feat: reboot_host contract, guest agent declaration, port group targets HostRebootMixin declares reboot_host(), mixed into DeviceTypeDriver so any device may be restartable. netOrk restarted hosts by sending /sbin/reboot through a driver's private _send_command; a driver talking to an API had no such method and the reboot was silently skipped. HypervisorDriver gains GUEST_AGENT_PACKAGES / GUEST_AGENT_RUNCMD, the agent cloud-init installs so the hypervisor can read a new VM's IP. The default stays qemu-guest-agent; VMware declares open-vm-tools. NetworkTargetDict.kind may be "portgroup": a VMware port group fixes its VLAN like an SDN vnet does, without being one. --- README.md | 2 ++ napalm_device_types/__init__.py | 3 +++ napalm_device_types/base.py | 3 ++- napalm_device_types/host_reboot.py | 30 ++++++++++++++++++++++++++++++ napalm_device_types/hypervisor.py | 5 +++++ napalm_device_types/models.py | 4 ++-- tests/test_host_reboot.py | 29 +++++++++++++++++++++++++++++ tests/test_vm_identifiers.py | 17 +++++++++++++++++ 8 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 napalm_device_types/host_reboot.py create mode 100644 tests/test_host_reboot.py diff --git a/README.md b/README.md index f15dff5..765e55c 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,8 @@ Behaviour shared across roles lives in a function class, exactly once, and a rol is a thin bundle over them — `PackageManagementMixin`, `HealthMetricsMixin`, `ServiceControlMixin`, `UpdateMixin`, `NatVpnMixin`, `MacAclMixin`, `FirewallRuleMixin`, `DhcpServerMixin`, `PingSweepMixin`, `ConfigLifecycleMixin`, `InterfaceFilterMixin`. +`HostRebootMixin` (`reboot_host`) is mixed into `DeviceTypeDriver` itself, since any +device may be restartable; like the others it only declares. A function class may use the **template form** — public method concrete, the device-specific part a `_hook` declared under `if TYPE_CHECKING` — *when the base diff --git a/napalm_device_types/__init__.py b/napalm_device_types/__init__.py index 37d2060..436e2d7 100644 --- a/napalm_device_types/__init__.py +++ b/napalm_device_types/__init__.py @@ -38,6 +38,7 @@ instead of being restated on every role that happens to need it: * :class:`~napalm_device_types.dhcp.DhcpServerMixin` * :class:`~napalm_device_types.firewall_rules.FirewallRuleMixin` * :class:`~napalm_device_types.health_metrics.HealthMetricsMixin` +* :class:`~napalm_device_types.host_reboot.HostRebootMixin` * :class:`~napalm_device_types.interface_filter.InterfaceFilterMixin` * :class:`~napalm_device_types.mac_acl.MacAclMixin` * :class:`~napalm_device_types.nat_vpn.NatVpnMixin` @@ -60,6 +61,7 @@ from napalm_device_types.hypervisor import HypervisorDriver from napalm_device_types.os import OSDriver from napalm_device_types.firewall_rules import FirewallRuleMixin from napalm_device_types.health_metrics import HealthMetricsMixin +from napalm_device_types.host_reboot import HostRebootMixin from napalm_device_types.interface_filter import InterfaceFilterMixin from napalm_device_types.mac_acl import MacAclMixin from napalm_device_types.media import MediaDriver @@ -83,6 +85,7 @@ __all__ = [ "FirewallDriver", "FirewallRuleMixin", "HealthMetricsMixin", + "HostRebootMixin", "HypervisorDriver", "InterfaceFilterMixin", "MacAclMixin", diff --git a/napalm_device_types/base.py b/napalm_device_types/base.py index 5f2d52d..110cefe 100644 --- a/napalm_device_types/base.py +++ b/napalm_device_types/base.py @@ -12,6 +12,7 @@ from typing import NamedTuple from napalm.base import NetworkDriver +from napalm_device_types.host_reboot import HostRebootMixin from napalm_device_types.ping_sweep import PingSweepMixin @@ -47,7 +48,7 @@ class PortSpec(NamedTuple): mandatory: bool = False -class DeviceTypeDriver(PingSweepMixin, NetworkDriver): +class DeviceTypeDriver(PingSweepMixin, HostRebootMixin, NetworkDriver): """Common base for all netOrk device-type drivers. Sits between napalm.base.NetworkDriver and the type-specific abstract diff --git a/napalm_device_types/host_reboot.py b/napalm_device_types/host_reboot.py new file mode 100644 index 0000000..2a21c1f --- /dev/null +++ b/napalm_device_types/host_reboot.py @@ -0,0 +1,30 @@ +"""Restarting the device itself. + +Declared under ``if TYPE_CHECKING``: a contract, not a placeholder. Only a +driver that can actually restart its device defines ``reboot_host``, so +``hasattr(driver, "reboot_host")`` tells a caller whether to offer it. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + + +class HostRebootMixin: + if TYPE_CHECKING: + + def reboot_host(self) -> None: + """ + Restarts the device this driver is connected to. + + Returns once the device has accepted the request; the session is + usually gone right after. Waiting for the device to come back is + the caller's business (see ``REBOOT_SETTLE_SECONDS``). + + A driver that manages other machines restarts its *own* host, never + one of them: a hypervisor restarts the hypervisor, not a VM. + + :raises RuntimeError: If the device refuses, e.g. an ESXi host that + is not in maintenance mode while VMs are running. + """ + ... diff --git a/napalm_device_types/hypervisor.py b/napalm_device_types/hypervisor.py index 268e8df..9e01036 100644 --- a/napalm_device_types/hypervisor.py +++ b/napalm_device_types/hypervisor.py @@ -43,6 +43,11 @@ class HypervisorDriver(PackageManagementMixin, HealthMetricsMixin, DeviceTypeDri ROLE: str = "hypervisor" TYPE_LABEL: str = "Hypervisor" + #: What cloud-init installs and starts on a VM provisioned through this + #: driver, so the hypervisor can read the guest's IP address back. + GUEST_AGENT_PACKAGES: tuple[str, ...] = ("qemu-guest-agent",) + GUEST_AGENT_RUNCMD: tuple[str, ...] = ("systemctl enable --now qemu-guest-agent",) + # ------------------------------------------------------------------ diff --git a/napalm_device_types/models.py b/napalm_device_types/models.py index 5319808..3f9d67b 100644 --- a/napalm_device_types/models.py +++ b/napalm_device_types/models.py @@ -889,8 +889,8 @@ class NetworkTargetDict(TypedDict): is surfaced via ``fixed_vlan_tag`` instead, for display purposes. """ - name: str # Bridge or vnet name, usable directly as NICConfigDict.bridge - kind: str # "bridge" or "vnet" + name: str # Bridge, vnet or port group name, usable directly as NICConfigDict.bridge + kind: str # "bridge", "vnet" or "portgroup" (VMware: VLAN fixed like a vnet's) vlan_aware: bool # True if a NICConfigDict.vlan_tag may be set on top of this target fixed_vlan_tag: NotRequired[int | None] # vnet only: the VLAN ID already baked into it diff --git a/tests/test_host_reboot.py b/tests/test_host_reboot.py new file mode 100644 index 0000000..ec12da3 --- /dev/null +++ b/tests/test_host_reboot.py @@ -0,0 +1,29 @@ +"""reboot_host: the contract for restarting the device itself. + +netOrk used to restart a host by sending ``/sbin/reboot`` through a driver's +private ``_send_command``. A driver that talks to an API instead has no such +method, and the caller swallowed the resulting AttributeError, so the reboot +"succeeded" without happening. A declared contract lets netOrk ask first. +""" + +from __future__ import annotations + +import inspect + +from napalm_device_types import DeviceTypeDriver, HostRebootMixin + + +def test_every_device_type_driver_carries_the_declaration(): + assert issubclass(DeviceTypeDriver, HostRebootMixin) + + +def test_declared_not_implemented(): + """hasattr is netOrk's capability probe; only a driver that implements it + may answer True.""" + assert not hasattr(DeviceTypeDriver, "reboot_host") + + +def test_declaration_documents_the_contract(): + source = inspect.getsource(HostRebootMixin) + assert "def reboot_host(self) -> None" in source + assert "RuntimeError" in source diff --git a/tests/test_vm_identifiers.py b/tests/test_vm_identifiers.py index d38f6e2..ec64e39 100644 --- a/tests/test_vm_identifiers.py +++ b/tests/test_vm_identifiers.py @@ -51,3 +51,20 @@ class TestVMConfigCarriesWhatAHardwareViewShows: from napalm_device_types.models import VMPassthroughDict assert get_type_hints(VMPassthroughDict) == {"slot": str, "kind": str, "config": str} + + +class TestGuestAgentDeclaration: + """netOrk's cloud-init installed qemu-guest-agent on every new VM. A VMware + guest reports its IP through open-vm-tools instead; the hypervisor says + which, and netOrk stops hard-coding one of them.""" + + def test_default_is_qemu_guest_agent(self): + from napalm_device_types import HypervisorDriver + + assert HypervisorDriver.GUEST_AGENT_PACKAGES == ("qemu-guest-agent",) + assert HypervisorDriver.GUEST_AGENT_RUNCMD == ("systemctl enable --now qemu-guest-agent",) + + def test_attributes_are_not_methods(self): + from napalm_device_types import HypervisorDriver + + assert not callable(vars(HypervisorDriver)["GUEST_AGENT_PACKAGES"]) -- 2.54.0