From 9179fa29b06106a0428430fe1e14c488443f5ee4 Mon Sep 17 00:00:00 2001 From: Mathias Petermann Date: Mon, 4 Apr 2022 22:43:00 +0200 Subject: [PATCH] refactor: add ProxmoxClient to handle pve api interactions (#196) --- prometheuspvesd/client.py | 90 +++++++++++++++++++++ prometheuspvesd/discovery.py | 69 ++++------------ prometheuspvesd/model.py | 14 ++++ prometheuspvesd/test/fixtures/fixtures.py | 79 ++++++++++++++---- prometheuspvesd/test/unit/test_cli.py | 15 ++-- prometheuspvesd/test/unit/test_discovery.py | 39 +++++---- prometheuspvesd/test/unit/test_model.py | 2 +- 7 files changed, 217 insertions(+), 91 deletions(-) create mode 100644 prometheuspvesd/client.py diff --git a/prometheuspvesd/client.py b/prometheuspvesd/client.py new file mode 100644 index 0000000..2209a72 --- /dev/null +++ b/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"] diff --git a/prometheuspvesd/discovery.py b/prometheuspvesd/discovery.py index a69dce7..e778fa5 100644 --- a/prometheuspvesd/discovery.py +++ b/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("nodes", pve_node, pve_type, vmid, "agent", "info") is not None: - networks = self.client.get( - "nodes", pve_node, "qemu", vmid, "agent", "network-get-interfaces" - )["result"] + if self.client.get_agent_info(pve_node, pve_type, vmid) is not None: + networks = self.client.get_network_interfaces(pve_node, vmid) 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("nodes", node, "qemu")) - PVE_REQUEST_COUNT_TOTAL.inc() - container_list = self._filter(self.client.get("nodes", node, "lxc")) + qemu_list = self._filter(self.client.get_all_vms(node)) + container_list = self._filter(self.client.get_all_containers(node)) 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("nodes", node, pve_type, vmid, "config") + config = self.client.get_instance_config(node, pve_type, vmid) try: description = (config["description"]) except KeyError: description = None except Exception as e: # noqa - PVE_REQUEST_COUNT_ERROR_TOTAL.inc() raise APIError(str(e)) try: diff --git a/prometheuspvesd/model.py b/prometheuspvesd/model.py index edcaa7f..e2ee8b6 100644 --- a/prometheuspvesd/model.py +++ b/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 = [] diff --git a/prometheuspvesd/test/fixtures/fixtures.py b/prometheuspvesd/test/fixtures/fixtures.py index 2540403..f6e0370 100644 --- a/prometheuspvesd/test/fixtures/fixtures.py +++ b/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", - "10.0.0.1", + "192.0.2.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("202", "host2", "129.168.0.2", False, "qemu")) + hostlist.add_host(Host("100", "100.example.com", "192.0.2.1", 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": "192.0.2.1", + "__meta_pve_ipv6": "False", + "__meta_pve_name": "100.example.com", + "__meta_pve_type": "qemu", + "__meta_pve_vmid": "100" + } + }, { + "targets": ["101.example.com"], "labels": { - "__meta_pve_ipv4": "129.168.0.1", + "__meta_pve_ipv4": "192.0.2.2", "__meta_pve_ipv6": "False", - "__meta_pve_name": "host1", + "__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" } }] diff --git a/prometheuspvesd/test/unit/test_cli.py b/prometheuspvesd/test/unit/test_cli.py index b65775c..ccf234e 100644 --- a/prometheuspvesd/test/unit/test_cli.py +++ b/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(Discovery, "_auth", return_value=mocker.create_autospec(ProxmoxAPI)) + mocker.patch.object(Log, "update_logger", side_effect=ValueError("Dummy Loglevel Exception")) + 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")) diff --git a/prometheuspvesd/test/unit/test_discovery.py b/prometheuspvesd/test/unit/test_discovery.py index 26696ce..649249d 100644 --- a/prometheuspvesd/test/unit/test_discovery.py +++ b/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 diff --git a/prometheuspvesd/test/unit/test_model.py b/prometheuspvesd/test/unit/test_model.py index 7d4be90..75cf634 100644 --- a/prometheuspvesd/test/unit/test_model.py +++ b/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"],