refactor: add ProxmoxClient to handle pve api interactions (#196)

prometheuspvesd/client.py

@@ -0,0 +1,90 @@
+"""Proxmox Client."""
+import requests
+from prometheus_client import Counter
+
+from prometheuspvesd.config import SingleConfig
+from prometheuspvesd.exception import APIError
+from prometheuspvesd.logger import SingleLog
+from prometheuspvesd.model import HostList
+from prometheuspvesd.utils import to_bool
+
+try:
+    from proxmoxer import ProxmoxAPI
+    HAS_PROXMOXER = True
+except ImportError:
+    HAS_PROXMOXER = False
+
+PVE_REQUEST_COUNT_TOTAL = Counter("pve_sd_requests_total", "Total count of requests to PVE API")
+PVE_REQUEST_COUNT_ERROR_TOTAL = Counter(
+    "pve_sd_requests_error_total", "Total count of failed requests to PVE API"
+)
+
+
+class ProxmoxClient:
+    """Proxmox API Client."""
+
+    def __init__(self):
+        if not HAS_PROXMOXER:
+            self.log.sysexit_with_message(
+                "The Proxmox VE Prometheus SD requires proxmoxer: "
+                "https://pypi.org/project/proxmoxer/"
+            )
+
+        self.config = SingleConfig()
+        self.log = SingleLog()
+        self.logger = SingleLog().logger
+        self.client = self._auth()
+        self.logger.debug("Successfully authenticated")
+        self.host_list = HostList()
+
+    def _auth(self):
+        try:
+            self.logger.debug(
+                "Trying to authenticate against {} as user {}".format(
+                    self.config.config["pve"]["server"], self.config.config["pve"]["user"]
+                )
+            )
+            return ProxmoxAPI(
+                self.config.config["pve"]["server"],
+                user=self.config.config["pve"]["user"],
+                password=self.config.config["pve"]["password"],
+                verify_ssl=to_bool(self.config.config["pve"]["verify_ssl"]),
+                timeout=self.config.config["pve"]["auth_timeout"]
+            )
+        except requests.RequestException as e:
+            PVE_REQUEST_COUNT_ERROR_TOTAL.inc()
+            raise APIError(str(e))
+
+    def _do_request(self, *args):
+        PVE_REQUEST_COUNT_TOTAL.inc()
+        try:
+            # create a new tuple containing nodes and unpack it again for client.get
+            return self.client.get(*("nodes", *args))
+        except requests.RequestException as e:
+            PVE_REQUEST_COUNT_ERROR_TOTAL.inc()
+            raise APIError(str(e))
+
+    def get_nodes(self):
+        self.logger.debug("fetching all nodes")
+        return self._do_request()
+
+    def get_all_vms(self, pve_node):
+        self.logger.debug("fetching all vms on node {}".format(pve_node))
+        return self._do_request(pve_node, "qemu")
+
+    def get_all_containers(self, pve_node):
+        self.logger.debug("fetching all containers on node {}".format(pve_node))
+        return self._do_request(pve_node, "lxc")
+
+    def get_instance_config(self, pve_node, pve_type, vmid):
+        self.logger.debug("fetching instance config for {} on {}".format(vmid, pve_node))
+        return self._do_request(pve_node, pve_type, vmid, "config")
+
+    def get_agent_info(self, pve_node, pve_type, vmid):
+        self.logger.debug("fetching agent info for {} on {}".format(vmid, pve_node))
+        return self._do_request(pve_node, pve_type, vmid, "agent", "info")["result"]
+
+    def get_network_interfaces(self, pve_node, vmid):
+        self.logger.debug("fetching network interfaces for {} on {}".format(vmid, pve_node))
+        return self._do_request(pve_node, "qemu", vmid, "agent",
+                                "network-get-interfaces")["result"]

prometheuspvesd/discovery.py

@@ -6,63 +6,32 @@ import json
 import re
 from collections import defaultdict
 
-import requests
-from prometheus_client import Counter
 from prometheus_client import Gauge
 from prometheus_client import Summary
 
+from prometheuspvesd.client import ProxmoxClient
 from prometheuspvesd.config import SingleConfig
 from prometheuspvesd.exception import APIError
 from prometheuspvesd.logger import SingleLog
 from prometheuspvesd.model import Host
 from prometheuspvesd.model import HostList
-from prometheuspvesd.utils import to_bool
-
-try:
-    from proxmoxer import ProxmoxAPI
-    HAS_PROXMOXER = True
-except ImportError:
-    HAS_PROXMOXER = False
 
 PROPAGATION_TIME = Summary(
     "pve_sd_propagate_seconds", "Time spent propagating the inventory from PVE"
 )
 HOST_GAUGE = Gauge("pve_sd_hosts", "Number of hosts discovered by PVE SD")
-PVE_REQUEST_COUNT_TOTAL = Counter("pve_sd_requests_total", "Total count of requests to PVE API")
-PVE_REQUEST_COUNT_ERROR_TOTAL = Counter(
-    "pve_sd_requests_error_total", "Total count of failed requests to PVE API"
-)
 
 
 class Discovery():
     """Prometheus PVE Service Discovery."""
 
     def __init__(self):
-        if not HAS_PROXMOXER:
-            self.log.sysexit_with_message(
-                "The Proxmox VE Prometheus SD requires proxmoxer: "
-                "https://pypi.org/project/proxmoxer/"
-            )
-
         self.config = SingleConfig()
         self.log = SingleLog()
         self.logger = SingleLog().logger
-        self.client = self._auth()
+        self.client = ProxmoxClient()
         self.host_list = HostList()
 
-    def _auth(self):
-        try:
-            return ProxmoxAPI(
-                self.config.config["pve"]["server"],
-                user=self.config.config["pve"]["user"],
-                password=self.config.config["pve"]["password"],
-                verify_ssl=to_bool(self.config.config["pve"]["verify_ssl"]),
-                timeout=self.config.config["pve"]["auth_timeout"]
-            )
-        except requests.RequestException as e:
-            PVE_REQUEST_COUNT_ERROR_TOTAL.inc()
-            raise APIError(str(e))
-
     def _get_names(self, pve_list, pve_type):
         names = []
 
@@ -92,11 +61,8 @@ class Discovery():
         if pve_type == "qemu":
             # If qemu agent is enabled, try to gather the IP address
             try:
-                PVE_REQUEST_COUNT_TOTAL.inc()
+                if self.client.get_agent_info(pve_node, pve_type, vmid) is not None:
-                if self.client.get("nodes", pve_node, pve_type, vmid, "agent", "info") is not None:
+                    networks = self.client.get_network_interfaces(pve_node, vmid)
-                    networks = self.client.get(
-                        "nodes", pve_node, "qemu", vmid, "agent", "network-get-interfaces"
-                    )["result"]
             except Exception:  # noqa  # nosec
                 pass
 
@@ -108,10 +74,9 @@ class Discovery():
                         elif ip_address["ip-address-type"] == "ipv6" and not ipv6_address:
                             ipv6_address = self._validate_ip(ip_address["ip-address"])
 
-        if not ipv4_address:
+        config = self.client.get_instance_config(pve_node, pve_type, vmid)
+        if config and not ipv4_address:
             try:
-                PVE_REQUEST_COUNT_TOTAL.inc()
-                config = self.client.get("nodes", pve_node, pve_type, vmid, "config")
                 if "ipconfig0" in config.keys():
                     sources = [config["net0"], config["ipconfig0"]]
                 else:
@@ -125,17 +90,17 @@ class Discovery():
             except Exception:  # noqa  # nosec
                 pass
 
-        if not ipv6_address:
+        if config and not ipv6_address:
             try:
-                PVE_REQUEST_COUNT_TOTAL.inc()
-                config = self.client.get("nodes", pve_node, pve_type, vmid, "config")
                 if "ipconfig0" in config.keys():
                     sources = [config["net0"], config["ipconfig0"]]
                 else:
                     sources = [config["net0"]]
 
                 for s in sources:
-                    find = re.search(r"ip=(\d*:\d*:\d*:\d*:\d*:\d*)", str(s))
+                    find = re.search(
+                        r"ip=(([a-fA-F0-9]{0,4}:{0,2}){0,7}:[0-9a-fA-F]{1,4})", str(s)
+                    )
                     if find and find.group(1):
                         ipv6_address = find.group(1)
                         break
@@ -194,15 +159,11 @@ class Discovery():
     def propagate(self):
         self.host_list.clear()
 
-        PVE_REQUEST_COUNT_TOTAL.inc()
+        for node in self._get_names(self.client.get_nodes(), "node"):
-        for node in self._get_names(self.client.get("nodes"), "node"):
             try:
-                PVE_REQUEST_COUNT_TOTAL.inc()
+                qemu_list = self._filter(self.client.get_all_vms(node))
-                qemu_list = self._filter(self.client.get("nodes", node, "qemu"))
+                container_list = self._filter(self.client.get_all_containers(node))
-                PVE_REQUEST_COUNT_TOTAL.inc()
-                container_list = self._filter(self.client.get("nodes", node, "lxc"))
             except Exception as e:  # noqa
-                PVE_REQUEST_COUNT_ERROR_TOTAL.inc()
                 raise APIError(str(e))
 
             # Merge QEMU and Containers lists from this node
@@ -220,15 +181,13 @@ class Discovery():
                 except KeyError:
                     pve_type = "qemu"
 
-                PVE_REQUEST_COUNT_TOTAL.inc()
+                config = self.client.get_instance_config(node, pve_type, vmid)
-                config = self.client.get("nodes", node, pve_type, vmid, "config")
 
                 try:
                     description = (config["description"])
                 except KeyError:
                     description = None
                 except Exception as e:  # noqa
-                    PVE_REQUEST_COUNT_ERROR_TOTAL.inc()
                     raise APIError(str(e))
 
                 try:

prometheuspvesd/model.py

@@ -36,6 +36,20 @@ class HostList:
     def __init__(self):
         self.hosts = []
 
+    def __eq__(self, other):
+        if not isinstance(other, HostList):
+            return False
+
+        if len(other.hosts) != len(self.hosts):
+            return False
+
+        for host in self.hosts:
+            if other.host_exists(host):
+                continue
+            return False
+
+        return True
+
     def clear(self):
         self.hosts = []
 

prometheuspvesd/test/fixtures/fixtures.py

@@ -160,6 +160,24 @@ def defaults():
     }
 
 
+@pytest.fixture
+def nodes():
+    return [{
+        "level": "",
+        "id": "node/example-node",
+        "disk": 4783488,
+        "cpu": 0.0935113631167406,
+        "maxcpu": 24,
+        "maxmem": 142073272990,
+        "mem": 135884478304,
+        "node": "example-node",
+        "type": "node",
+        "status": "online",
+        "maxdisk": 504209920,
+        "uptime": 200
+    }]
+
+
 @pytest.fixture
 def qemus():
     return [
@@ -222,12 +240,35 @@ def qemus():
     ]
 
 
+@pytest.fixture
+def instance_config():
+    return {
+        "name": "102.example.com",
+        "description": '{"groups": "test-group"}',
+        "net0": "virtio=D8-85-75-47-2E-8D,bridge=vmbr122,ip=192.0.2.25,ip=2001:db8::666:77:8888",
+        "cpu": 2,
+        "cores": 2
+    }
+
+
+@pytest.fixture
+def agent_info():
+    return {
+        "supported_commands": [{
+            "name": "guest-network-get-interfaces",
+            "enabled": True,
+            "success-response": True
+        }],
+        "version": "5.2.0"
+    }
+
+
 @pytest.fixture
 def addresses():
     return {
         "ipv4_valid": [
-            "192.168.0.1",
+            "192.0.2.1",
-            "10.0.0.1",
+            "198.51.100.1",
         ],
         "ipv4_invalid": [
             "127.0.0.1",
@@ -282,17 +323,17 @@ def networks():
             "hardware-address": "92:0b:bd:c1:f8:39",
             "ip-addresses": [
                 {
-                    "ip-address": "10.168.0.1",
+                    "ip-address": "192.0.2.1",
                     "ip-address-type": "ipv4",
                     "prefix": 32
                 },
                 {
-                    "ip-address": "10.168.0.2",
+                    "ip-address": "192.0.2.4",
                     "ip-address-type": "ipv4",
                     "prefix": 32
                 },
                 {
-                    "ip-address": "2001:cdba:3333:4444:5555:6666:7777:8888",
+                    "ip-address": "2001:db8:3333:4444:5555:6666:7777:8888",
                     "ip-address-type": "ipv6",
                     "prefix": 64
                 },
@@ -315,8 +356,9 @@ def networks():
 @pytest.fixture
 def inventory():
     hostlist = HostList()
-    hostlist.add_host(Host("101", "host1", "129.168.0.1", False, "qemu"))
+    hostlist.add_host(Host("100", "100.example.com", "192.0.2.1", False, "qemu"))
-    hostlist.add_host(Host("202", "host2", "129.168.0.2", False, "qemu"))
+    hostlist.add_host(Host("101", "101.example.com", "192.0.2.2", False, "qemu"))
+    hostlist.add_host(Host("102", "102.example.com", "192.0.2.3", False, "qemu"))
 
     return hostlist
 
@@ -324,21 +366,30 @@ def inventory():
 @pytest.fixture
 def labels():
     return [{
-        "targets": ["host1"],
+        "targets": ["100.example.com"],
         "labels": {
-            "__meta_pve_ipv4": "129.168.0.1",
+            "__meta_pve_ipv4": "192.0.2.1",
             "__meta_pve_ipv6": "False",
-            "__meta_pve_name": "host1",
+            "__meta_pve_name": "100.example.com",
+            "__meta_pve_type": "qemu",
+            "__meta_pve_vmid": "100"
+        }
+    }, {
+        "targets": ["101.example.com"],
+        "labels": {
+            "__meta_pve_ipv4": "192.0.2.2",
+            "__meta_pve_ipv6": "False",
+            "__meta_pve_name": "101.example.com",
             "__meta_pve_type": "qemu",
             "__meta_pve_vmid": "101"
         }
     }, {
-        "targets": ["host2"],
+        "targets": ["102.example.com"],
         "labels": {
-            "__meta_pve_ipv4": "129.168.0.2",
+            "__meta_pve_ipv4": "192.0.2.3",
             "__meta_pve_ipv6": "False",
-            "__meta_pve_name": "host2",
+            "__meta_pve_name": "102.example.com",
             "__meta_pve_type": "qemu",
-            "__meta_pve_vmid": "202"
+            "__meta_pve_vmid": "102"
         }
     }]

prometheuspvesd/test/unit/test_cli.py

@@ -6,6 +6,7 @@ from proxmoxer import ProxmoxAPI
 
 import prometheuspvesd.exception
 from prometheuspvesd.cli import PrometheusSD
+from prometheuspvesd.client import ProxmoxClient
 from prometheuspvesd.config import Config
 from prometheuspvesd.discovery import Discovery
 from prometheuspvesd.exception import APIError
@@ -17,7 +18,7 @@ pytest_plugins = [
 
 
 def test_cli_required_error(mocker, capsys):
-    mocker.patch.object(Discovery, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
+    mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
     mocker.patch.object(PrometheusSD, "_fetch", return_value=True)
 
     with pytest.raises(SystemExit) as e:
@@ -33,7 +34,7 @@ def test_cli_config_error(mocker, capsys):
         "prometheuspvesd.config.SingleConfig.__init__",
         side_effect=prometheuspvesd.exception.ConfigError("Dummy Config Exception")
     )
-    mocker.patch.object(Discovery, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
+    mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
     mocker.patch.object(PrometheusSD, "_fetch", return_value=True)
 
     with pytest.raises(SystemExit) as e:
@@ -45,21 +46,21 @@ def test_cli_config_error(mocker, capsys):
 
 
 def test_cli_log_error(mocker, capsys):
-    mocker.patch.object(Log, "update_logger", side_effect=ValueError("Dummy Logleve Exception"))
+    mocker.patch.object(Log, "update_logger", side_effect=ValueError("Dummy Loglevel Exception"))
-    mocker.patch.object(Discovery, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
+    mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
     mocker.patch.object(PrometheusSD, "_fetch", return_value=True)
 
     with pytest.raises(SystemExit) as e:
         PrometheusSD()
 
     stdout, stderr = capsys.readouterr()
-    assert "Dummy Logleve Exception" in stderr
+    assert "Dummy Loglevel Exception" in stderr
     assert e.value.code == 1
 
 
 def test_cli_api_error(mocker, builtins, capsys):
     mocker.patch.dict(Config.SETTINGS, builtins)
-    mocker.patch.object(Discovery, "_auth", side_effect=APIError("Dummy API Exception"))
+    mocker.patch.object(ProxmoxClient, "_auth", side_effect=APIError("Dummy API Exception"))
     mocker.patch.object(PrometheusSD, "_fetch", return_value=True)
 
     with pytest.raises(SystemExit) as e:
@@ -77,7 +78,7 @@ def test_cli_write(mocker, tmp_path, builtins, inventory, labels):
     builtins["output_file"]["default"] = out.as_posix()
 
     mocker.patch.dict(Config.SETTINGS, builtins)
-    mocker.patch.object(Discovery, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
+    mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
     mocker.patch.object(Discovery, "propagate", return_value=inventory)
     mocker.patch("tempfile.NamedTemporaryFile", return_value=temp.open("w"))
 

prometheuspvesd/test/unit/test_discovery.py

@@ -3,6 +3,7 @@
 import pytest
 from proxmoxer import ProxmoxAPI
 
+from prometheuspvesd.client import ProxmoxClient
 from prometheuspvesd.discovery import Discovery
 
 pytest_plugins = [
@@ -12,23 +13,11 @@ pytest_plugins = [
 
 @pytest.fixture
 def discovery(mocker):
-    mocker.patch.object(Discovery, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
+    mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI))
 
     return Discovery()
 
 
-def get_mock(*args):
-    networks = args[0]
-    args = args[1:]
-
-    if "info" in args:
-        return True
-    if "network-get-interfaces" in args:
-        return {"result": networks}
-
-    return False
-
-
 def test_exclude_vmid(discovery, qemus):
     discovery.config.config["exclude_vmid"] = ["100", "101", "102"]
     filtered = discovery._filter(qemus)
@@ -98,9 +87,31 @@ def test_validate_ip(discovery, addresses):
 
 
 def test_get_ip_addresses(mocker, discovery, networks):
-    discovery.client.get.side_effect = lambda *args: get_mock(networks, *args)
+    mocker.patch.object(ProxmoxClient, "get_network_interfaces", return_value=networks)
 
     assert discovery._get_ip_addresses("qemu", "dummy", "dummy") == (
         networks[1]["ip-addresses"][0]["ip-address"],
         networks[1]["ip-addresses"][2]["ip-address"],
     )
+
+
+def test_get_ip_addresses_from_instance_config(mocker, discovery, instance_config):
+    mocker.patch.object(ProxmoxClient, "get_network_interfaces", return_value=[])
+    mocker.patch.object(ProxmoxClient, "get_instance_config", return_value=instance_config)
+
+    assert discovery._get_ip_addresses("qemu", "dummy", "dummy") == (
+        "192.0.2.25",
+        "2001:db8::666:77:8888",
+    )
+
+
+def test_propagate(
+    mocker, discovery, nodes, qemus, instance_config, agent_info, networks, inventory
+):
+    mocker.patch.object(ProxmoxClient, "get_nodes", return_value=nodes)
+    mocker.patch.object(ProxmoxClient, "get_all_vms", return_value=qemus)
+    mocker.patch.object(ProxmoxClient, "get_instance_config", return_value=instance_config)
+    mocker.patch.object(ProxmoxClient, "get_agent_info", return_value=agent_info)
+    mocker.patch.object(ProxmoxClient, "get_network_interfaces", return_value=networks)
+
+    assert discovery.propagate() == inventory

prometheuspvesd/test/unit/test_model.py

@@ -38,7 +38,7 @@ pytest_plugins = [
         }),
     ]
 )
-def test_host(mocker, testinput, expected):
+def test_host(testinput, expected):
     host = Host(
         testinput["vmid"],
         testinput["hostname"],