diff --git a/napalm_qnap_qts/qnap_qts.py b/napalm_qnap_qts/qnap_qts.py index 22bbc78..6a9ee2e 100644 --- a/napalm_qnap_qts/qnap_qts.py +++ b/napalm_qnap_qts/qnap_qts.py @@ -24,7 +24,12 @@ from __future__ import annotations import re from typing import Any -from napalm_device_types import FingerprintRule, PortSpec, StorageDriver +from napalm_device_types import ( + FingerprintRule, + HypervisorDriver, + PortSpec, + StorageDriver, +) from napalm_linux.linux import LinuxDriver #: QNAP Systems' IANA enterprise number. Used by discovery to recognise a NAS @@ -42,27 +47,22 @@ _DOCKER_GLOBS = ( _VERSION_RE = re.compile(r"(\d+)\.") -class QnapQtsDriver(StorageDriver, LinuxDriver): +class QnapQtsDriver(StorageDriver, HypervisorDriver, LinuxDriver): """NAPALM driver for QNAP NAS systems running QTS. - **Inheritance order matters.** ``StorageDriver`` precedes ``LinuxDriver`` in - the MRO, so its ``NotImplementedError`` stubs shadow LinuxDriver's working - implementations wherever the names collide — ``get_services``, - ``get_packages`` and ``install_package``. Each collision is resolved - explicitly below rather than left to the MRO; see ``TestMroForwarding``. + Declares all three roles it fills. The role bases carry no implementations, + so listing them alongside ``LinuxDriver`` costs nothing and shadows nothing; + what the driver can actually do is whatever it implements below. - NAS services are exposed as ``get_storage_services()``, not - ``get_services()``, because netOrk's poller reads the former for the storage - snapshot and the latter for the OS service list. Same convention as - napalm-openmediavault. + NAS services are ``get_storage_services()``; the OS service list inherited + from ``LinuxDriver`` stays ``get_services()``. Two names because they are two + different things with different return shapes, not because they collided. """ + # A QNAP is three things at once, and the order says which one leads: + # storage first, so netOrk shows it as a NAS. Nothing has to restate that + # -- device_class is read from this line. TYPE_LABEL = "Storage" - # Declared outright: the driver inherits LinuxDriver for the OS surface, so - # netOrk's issubclass chain would reach OSDriver first and classify a QNAP - # as "linux", hiding its Storage tab. VMs and containers stay visible - # through capability introspection, not through this key. - DEVICE_CLASS = "storage" VENDOR = "QNAP" DRIVER_NAME = "qnap_qts" driver_name = "qnap_qts" @@ -143,15 +143,22 @@ class QnapQtsDriver(StorageDriver, LinuxDriver): """Override LinuxDriver's hook: docker is not on PATH under QTS.""" return getattr(self, "_docker_path", None) or "docker" - # ── MRO collision resolution ────────────────────────────────────────────── + # ── QPKG, not apt ───────────────────────────────────────────────────────── # - # StorageDriver comes first in the MRO and its stubs would otherwise win. + # QTS is Linux, so LinuxDriver's package methods are in the MRO and would + # run happily -- against a package manager QTS does not have. They used to + # be shadowed by StorageDriver's stubs, which made a QNAP refuse them by + # accident. Now that role bases implement nothing, the refusal has to be + # deliberate. QPKG parsing lands with the device harvest; until then, + # refusing is the only honest answer. - def get_services(self) -> Any: - """OS services, not NAS services — this is what netOrk's poller reads. + def get_packages(self) -> Any: + raise NotImplementedError( + "QTS uses QPKG, not a Linux package manager; QPKG parsing is not implemented yet" + ) - Forwarded explicitly past ``StorageDriver.get_services``, which shadows - it and returns a different shape (dict of NAS services vs. list of OS - services). - """ - return LinuxDriver.get_services(self) + def install_package(self, name: str, version: str = "") -> None: + raise NotImplementedError("QPKG install is not implemented yet") + + def uninstall_package(self, name: str) -> None: + raise NotImplementedError("QPKG removal is not implemented yet") diff --git a/tests/test_qnap_qts.py b/tests/test_qnap_qts.py index eef7d5b..9a204f6 100644 --- a/tests/test_qnap_qts.py +++ b/tests/test_qnap_qts.py @@ -11,6 +11,7 @@ from unittest.mock import MagicMock, patch import pytest +from napalm_device_types import primary_role_of, role_keys_of from napalm_qnap_qts import QnapQtsDriver #: Parsers need real command output to be written against. These tests are the @@ -62,10 +63,14 @@ class TestDriverIdentity: def test_type_label_is_storage(self): assert QnapQtsDriver.TYPE_LABEL == "Storage" - def test_declares_device_class_explicitly(self): - """Inheriting LinuxDriver would otherwise get it classified as "linux", - and netOrk would hide the Storage tab.""" - assert QnapQtsDriver.DEVICE_CLASS == "storage" + def test_declares_every_role_it_fills(self): + """A QNAP is a NAS, a hypervisor and a Linux host at once.""" + assert role_keys_of(QnapQtsDriver) == ["storage", "hypervisor", "linux"] + + def test_storage_leads_because_it_is_listed_first(self): + """device_class comes from the order of the role bases, not from an + attribute restating it.""" + assert primary_role_of(QnapQtsDriver) == "storage" def test_declares_at_least_one_fingerprint_source(self): """Discovery silently skips a driver that declares no fingerprint data.""" @@ -83,42 +88,51 @@ class TestDriverIdentity: assert patterns["qnap"].mandatory is True -class TestMroForwarding: - """StorageDriver precedes LinuxDriver in the MRO, so its NotImplementedError - stubs shadow LinuxDriver's working implementations. Every collision has to be - resolved deliberately — this is the class of bug that makes a driver look - fine until it runs against hardware. +class TestRolesDoNotShadowLinux: + """The role bases declare their methods; they implement none. + + Before that change, ``StorageDriver`` preceded ``LinuxDriver`` in the MRO and + its ``NotImplementedError`` stubs replaced LinuxDriver's working + implementations, so this driver carried a hand-written forwarder for every + collision. There is nothing left to collide with. """ - def test_get_services_returns_the_linux_os_service_list(self, driver): - """netOrk's poller expects a list here (OS services). StorageDriver's - stub would return a dict of NAS services, if it returned anything.""" + @pytest.mark.parametrize("method", ["get_services", "get_users", "get_docker_info"]) + def test_os_surface_resolves_to_linux(self, method): + """QTS really is Linux for these, so inheriting them is correct.""" from napalm_linux.linux import LinuxDriver - with patch.object(LinuxDriver, "get_services", return_value=[{"name": "sshd"}]) as m: - result = driver.get_services() + owner = next(k for k in QnapQtsDriver.__mro__ if method in k.__dict__) + assert owner is LinuxDriver - assert m.called - assert result == [{"name": "sshd"}] + @pytest.mark.parametrize( + "method", ["get_packages", "install_package", "uninstall_package"] + ) + def test_package_surface_is_refused_deliberately(self, method): + """QTS has no apt/dnf, so LinuxDriver's versions must not be inherited + silently -- this driver overrides them to refuse.""" + owner = next(k for k in QnapQtsDriver.__mro__ if method in k.__dict__) + assert owner is QnapQtsDriver + + def test_no_forwarding_methods_remain(self): + """A forwarder here would mean the shadowing came back.""" + own = { + name for name, val in vars(QnapQtsDriver).items() + if callable(val) and not name.startswith("__") + } + assert "get_services" not in own @pytest.mark.xfail(strict=True, reason=_PENDING_HARVEST) def test_nas_services_live_under_a_separate_name(self): - """get_storage_services is what netOrk's _collect.py actually reads for - the storage snapshot — get_services is the OS list.""" + """get_storage_services is what netOrk's _collect.py reads for the + storage snapshot — get_services is the OS list.""" assert hasattr(QnapQtsDriver, "get_storage_services") - @pytest.mark.xfail(strict=True, reason=_PENDING_HARVEST) - def test_get_packages_is_not_the_storage_stub(self): - from napalm_device_types import StorageDriver - - assert QnapQtsDriver.get_packages is not StorageDriver.get_packages - @pytest.mark.parametrize( ("method", "args"), [ ("install_package", ("qpkg-name",)), - ("remove_package", ("qpkg-name",)), - ("snapshot_create", ("DataVol1", "snap1")), + ("uninstall_package", ("qpkg-name",)), ], ) def test_out_of_scope_writers_still_raise(self, driver, method, args): @@ -127,6 +141,11 @@ class TestMroForwarding: with pytest.raises(NotImplementedError): getattr(driver, method)(*args) + def test_volume_snapshot_writer_is_not_implemented_yet(self): + """Declared on StorageDriver for type checkers only, so it does not + exist until the harvest supplies a real implementation.""" + assert not hasattr(QnapQtsDriver, "create_volume_snapshot") + class TestQtsVersionDetection: def test_parses_major_version(self, driver):