From 1c3e4fc7e784afc311cbe0f47fc4939a0862f107 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 24 Jan 2024 12:50:39 +0100 Subject: [PATCH] fix: split tag string at semicolon instead of comma (#532) --- prometheuspvesd/discovery.py | 17 +- prometheuspvesd/test/fixtures/fixtures.py | 222 ++++++++------------ prometheuspvesd/test/unit/conftest.py | 39 +++- prometheuspvesd/test/unit/test_cli.py | 55 +++-- prometheuspvesd/test/unit/test_config.py | 2 +- prometheuspvesd/test/unit/test_discovery.py | 33 ++- prometheuspvesd/test/unit/test_model.py | 63 +++--- pyproject.toml | 4 +- 8 files changed, 235 insertions(+), 200 deletions(-) diff --git a/prometheuspvesd/discovery.py b/prometheuspvesd/discovery.py index 4f71ac7..7528a54 100644 --- a/prometheuspvesd/discovery.py +++ b/prometheuspvesd/discovery.py @@ -111,6 +111,13 @@ class Discovery: filtered = [] for item in pve_list: obj = defaultdict(dict, item) + tags = [] + tags_excl = self.config.config["exclude_tags"] + + if isinstance(obj["tags"], str): + tags = obj["tags"].split(";") + self.logger.debug(f"vmid {obj['vmid']}: discovered tags: {tags}") + if ( len(self.config.config["include_vmid"]) > 0 and str(obj["vmid"]) not in self.config.config["include_vmid"] @@ -119,7 +126,7 @@ class Discovery: if len(self.config.config["include_tags"]) > 0 and ( bool(obj["tags"]) is False # continue if tags is not set - or set(obj["tags"].split(",")).isdisjoint(self.config.config["include_tags"]) + or set(tags).isdisjoint(self.config.config["include_tags"]) ): continue @@ -132,9 +139,11 @@ class Discovery: if str(obj["vmid"]) in self.config.config["exclude_vmid"]: continue - if isinstance(obj["tags"], str) and not set(obj["tags"].split(",")).isdisjoint( - self.config.config["exclude_tags"] - ): + if isinstance(obj["tags"], str) and not set(tags).isdisjoint(tags_excl): + self.logger.debug( + f"vmid {obj['vmid']}: " + f"excluded by tags: {list(set(tags).intersection(tags_excl))}" + ) continue filtered.append(item.copy()) diff --git a/prometheuspvesd/test/fixtures/fixtures.py b/prometheuspvesd/test/fixtures/fixtures.py index bed5716..ba7a7ef 100644 --- a/prometheuspvesd/test/fixtures/fixtures.py +++ b/prometheuspvesd/test/fixtures/fixtures.py @@ -3,8 +3,7 @@ import environs import pytest -from prometheuspvesd.model import Host -from prometheuspvesd.model import HostList +from prometheuspvesd.model import Host, HostList @pytest.fixture @@ -13,131 +12,118 @@ def builtins(): "metrics.enabled": { "default": True, "env": "METRICS_ENABLED", - "type": environs.Env().bool + "type": environs.Env().bool, }, "metrics.address": { "default": "127.0.0.1", "env": "METRICS_ADDRESS", - "type": environs.Env().str - }, - "metrics.port": { - "default": 8000, - "env": "METRICS_PORT", - "type": environs.Env().int - }, - "config_file": { - "default": "", - "env": "CONFIG_FILE", - "type": environs.Env().str + "type": environs.Env().str, }, + "metrics.port": {"default": 8000, "env": "METRICS_PORT", "type": environs.Env().int}, + "config_file": {"default": "", "env": "CONFIG_FILE", "type": environs.Env().str}, "logging.level": { "default": "WARNING", "env": "LOG_LEVEL", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "logging.format": { "default": "console", "env": "LOG_FORMAT", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "output_file": { "default": "dummy", "env": "OUTPUT_FILE", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "output_file_mode": { "default": "0640", "env": "OUTPUT_FILE_MODE", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "loop_delay": { "default": 300, "env": "LOOP_DELAY", "file": True, - "type": environs.Env().int - }, - "service": { - "default": False, - "env": "SERVICE", - "file": True, - "type": environs.Env().bool + "type": environs.Env().int, }, + "service": {"default": False, "env": "SERVICE", "file": True, "type": environs.Env().bool}, "exclude_state": { "default": [], "env": "EXCLUDE_STATE", "file": True, - "type": environs.Env().list + "type": environs.Env().list, }, "exclude_vmid": { "default": [], "env": "EXCLUDE_VMID", "file": True, - "type": environs.Env().list + "type": environs.Env().list, }, "exclude_tags": { "default": [], "env": "EXCLUDE_TAGS", "file": True, - "type": environs.Env().list + "type": environs.Env().list, }, "include_vmid": { "default": [], "env": "INCLUDE_VMID", "file": True, - "type": environs.Env().list + "type": environs.Env().list, }, "include_tags": { "default": [], "env": "INCLUDE_TAGS", "file": True, - "type": environs.Env().list + "type": environs.Env().list, }, "pve.server": { "default": "dummyserver", "env": "PVE_SERVER", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "pve.user": { "default": "dummyuser", "env": "PVE_USER", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "pve.password": { "default": "dummypass", "env": "PVE_PASSWORD", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "pve.token_name": { "default": "dummyname", "env": "PVE_TOKEN_NAME", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "pve.token_value": { "default": "dummyvalue", "env": "PVE_TOKEN_VALUE", "file": True, - "type": environs.Env().str + "type": environs.Env().str, }, "pve.auth_timeout": { "default": 5, "env": "PVE_AUTH_TIMEOUT", "file": True, - "type": environs.Env().int + "type": environs.Env().int, }, "pve.verify_ssl": { "default": True, "env": "PVE_VERIFY_SSL", "file": True, - "type": environs.Env().bool - } + "type": environs.Env().bool, + }, } @@ -149,16 +135,9 @@ def defaults(): "exclude_vmid": [], "include_tags": [], "include_vmid": [], - "logging": { - "format": "console", - "level": "WARNING" - }, + "logging": {"format": "console", "level": "WARNING"}, "loop_delay": 300, - "metrics": { - "address": "127.0.0.1", - "enabled": True, - "port": 8000 - }, + "metrics": {"address": "127.0.0.1", "enabled": True, "port": 8000}, "output_file": "dummy", "output_file_mode": "0640", "pve": { @@ -168,7 +147,7 @@ def defaults(): "user": "", "token_name": "", "token_value": "", - "verify_ssl": True + "verify_ssl": True, }, "service": True, } @@ -176,20 +155,22 @@ 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 - }] + 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 @@ -212,7 +193,7 @@ def qemus(): "status": "running", "netout": 12159205236, "mem": 496179157, - "tags": "unmonitored,excluded,postgres" + "tags": "unmonitored;excluded;postgres", }, { "diskwrite": 0, @@ -230,7 +211,7 @@ def qemus(): "disk": 0, "status": "running", "netout": 12159205236, - "mem": 496179157 + "mem": 496179157, }, { "diskwrite": 0, @@ -249,7 +230,7 @@ def qemus(): "status": "prelaunch", "netout": 12159205236, "mem": 496179157, - "tags": "monitored" + "tags": "monitored", }, ] @@ -261,19 +242,17 @@ def instance_config(): "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 + "cores": 2, } @pytest.fixture def agent_info(): return { - "supported_commands": [{ - "name": "guest-network-get-interfaces", - "enabled": True, - "success-response": True - }], - "version": "5.2.0" + "supported_commands": [ + {"name": "guest-network-get-interfaces", "enabled": True, "success-response": True} + ], + "version": "5.2.0", } @@ -310,16 +289,8 @@ def networks(): { "hardware-address": "00:00:00:00:00:00", "ip-addresses": [ - { - "ip-address": "127.0.0.1", - "ip-address-type": "ipv4", - "prefix": 8 - }, - { - "ip-address": "::1", - "ip-address-type": "ipv6", - "prefix": 128 - }, + {"ip-address": "127.0.0.1", "ip-address-type": "ipv4", "prefix": 8}, + {"ip-address": "::1", "ip-address-type": "ipv6", "prefix": 128}, ], "name": "lo", "statistics": { @@ -330,26 +301,18 @@ def networks(): "tx-bytes": 9280, "tx-dropped": 0, "tx-errs": 0, - "tx-packets": 92 - } + "tx-packets": 92, + }, }, { "hardware-address": "92:0b:bd:c1:f8:39", "ip-addresses": [ - { - "ip-address": "192.0.2.1", - "ip-address-type": "ipv4", - "prefix": 32 - }, - { - "ip-address": "192.0.2.4", - "ip-address-type": "ipv4", - "prefix": 32 - }, + {"ip-address": "192.0.2.1", "ip-address-type": "ipv4", "prefix": 32}, + {"ip-address": "192.0.2.4", "ip-address-type": "ipv4", "prefix": 32}, { "ip-address": "2001:db8:3333:4444:5555:6666:7777:8888", "ip-address-type": "ipv6", - "prefix": 64 + "prefix": 64, }, ], "name": "eth0", @@ -361,13 +324,10 @@ def networks(): "tx-bytes": 12185866619, "tx-dropped": 0, "tx-errs": 0, - "tx-packets": 14423878 - } - }, - { - "hardware-address": "ba:97:85:bd:9a:a5", - "name": "eth1" + "tx-packets": 14423878, + }, }, + {"hardware-address": "ba:97:85:bd:9a:a5", "name": "eth1"}, ] @@ -383,31 +343,35 @@ def inventory(): @pytest.fixture def labels(): - return [{ - "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": "192.0.2.2", - "__meta_pve_ipv6": "False", - "__meta_pve_name": "101.example.com", - "__meta_pve_type": "qemu", - "__meta_pve_vmid": "101" - } - }, { - "targets": ["102.example.com"], - "labels": { - "__meta_pve_ipv4": "192.0.2.3", - "__meta_pve_ipv6": "False", - "__meta_pve_name": "102.example.com", - "__meta_pve_type": "qemu", - "__meta_pve_vmid": "102" - } - }] + return [ + { + "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": "192.0.2.2", + "__meta_pve_ipv6": "False", + "__meta_pve_name": "101.example.com", + "__meta_pve_type": "qemu", + "__meta_pve_vmid": "101", + }, + }, + { + "targets": ["102.example.com"], + "labels": { + "__meta_pve_ipv4": "192.0.2.3", + "__meta_pve_ipv6": "False", + "__meta_pve_name": "102.example.com", + "__meta_pve_type": "qemu", + "__meta_pve_vmid": "102", + }, + }, + ] diff --git a/prometheuspvesd/test/unit/conftest.py b/prometheuspvesd/test/unit/conftest.py index e24b42a..18dafd2 100644 --- a/prometheuspvesd/test/unit/conftest.py +++ b/prometheuspvesd/test/unit/conftest.py @@ -1,8 +1,11 @@ """Pytest conftest fixtures.""" +import logging import os import sys +from contextlib import contextmanager import pytest +from _pytest.logging import LogCaptureHandler from prometheuspvesd.utils import Singleton @@ -14,9 +17,43 @@ def reset_singletons(): @pytest.fixture(autouse=True) def reset_os_environment(): - os.environ = {} + os.environ.clear() @pytest.fixture(autouse=True) def reset_sys_argv(): sys.argv = ["prometheus-pve-sd"] + + +@contextmanager +def local_caplog_fn(level=logging.INFO, name="prometheuspvesd"): + """ + Context manager that captures records from non-propagating loggers. + + After the end of the 'with' statement, the log level is restored to its original + value. Code adapted from https://github.com/pytest-dev/pytest/issues/3697#issuecomment-790925527. + + :param int level: The level. + :param logging.Logger logger: The logger to update. + """ + + logger = logging.getLogger(name) + + old_level = logger.level + logger.setLevel(level) + + handler = LogCaptureHandler() + logger.addHandler(handler) + + try: + yield handler + finally: + logger.setLevel(old_level) + logger.removeHandler(handler) + + +@pytest.fixture +def local_caplog(): + """Fixture that yields a context manager for capturing records from non-propagating loggers.""" + + yield local_caplog_fn diff --git a/prometheuspvesd/test/unit/test_cli.py b/prometheuspvesd/test/unit/test_cli.py index 6133028..af2f08f 100644 --- a/prometheuspvesd/test/unit/test_cli.py +++ b/prometheuspvesd/test/unit/test_cli.py @@ -30,22 +30,22 @@ def test_cli_required_error(mocker, capsys): @pytest.mark.parametrize( - "testinput", [{ - "pve.user": "dummy", - "pve.password": "", - "pve.token_name": "", - "pve.token_value": "" - }, { - "pve.user": "dummy", - "pve.password": "", - "pve.token_name": "dummy", - "pve.token_value": "" - }, { - "pve.user": "dummy", - "pve.password": "", - "pve.token_name": "", - "pve.token_value": "dummy" - }] + "testinput", + [ + {"pve.user": "dummy", "pve.password": "", "pve.token_name": "", "pve.token_value": ""}, + { + "pve.user": "dummy", + "pve.password": "", + "pve.token_name": "dummy", + "pve.token_value": "", + }, + { + "pve.user": "dummy", + "pve.password": "", + "pve.token_name": "", + "pve.token_value": "dummy", + }, + ], ) def test_cli_auth_required_error(mocker, capsys, builtins, testinput): for key, value in testinput.items(): @@ -59,22 +59,21 @@ def test_cli_auth_required_error(mocker, capsys, builtins, testinput): PrometheusSD() stdout, stderr = capsys.readouterr() - assert "Either 'pve.password' or 'pve.token_name' and 'pve.token_value' are required but not set" in stderr + assert ( + "Either 'pve.password' or 'pve.token_name' and 'pve.token_value' are required but not set" + in stderr + ) assert e.value.code == 1 @pytest.mark.parametrize( - "testinput", [{ - "pve.password": "dummy", - "pve.token_name": "", - "pve.token_value": "" - }, { - "pve.password": "", - "pve.token_name": "dummy", - "pve.token_value": "dummy" - }] + "testinput", + [ + {"pve.password": "dummy", "pve.token_name": "", "pve.token_value": ""}, + {"pve.password": "", "pve.token_name": "dummy", "pve.token_value": "dummy"}, + ], ) -def test_cli_auth_no_error(mocker, capsys, builtins, testinput): +def test_cli_auth_no_error(mocker, builtins, testinput): for key, value in testinput.items(): builtins[key]["default"] = value @@ -91,7 +90,7 @@ def test_cli_auth_no_error(mocker, capsys, builtins, testinput): def test_cli_config_error(mocker, capsys): mocker.patch( "prometheuspvesd.config.SingleConfig.__init__", - side_effect=prometheuspvesd.exception.ConfigError("Dummy Config Exception") + side_effect=prometheuspvesd.exception.ConfigError("Dummy Config Exception"), ) mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI)) mocker.patch.object(PrometheusSD, "_fetch", return_value=True) diff --git a/prometheuspvesd/test/unit/test_config.py b/prometheuspvesd/test/unit/test_config.py index 9d99f25..835cd5f 100644 --- a/prometheuspvesd/test/unit/test_config.py +++ b/prometheuspvesd/test/unit/test_config.py @@ -25,7 +25,7 @@ def test_yaml_config(mocker, defaults): assert config.config == defaults -def test_yaml_config_error(mocker, capsys): +def test_yaml_config_error(mocker): mocker.patch( "prometheuspvesd.config.default_config_file", "./prometheuspvesd/test/data/config.yml" ) diff --git a/prometheuspvesd/test/unit/test_discovery.py b/prometheuspvesd/test/unit/test_discovery.py index 649249d..865a714 100644 --- a/prometheuspvesd/test/unit/test_discovery.py +++ b/prometheuspvesd/test/unit/test_discovery.py @@ -1,5 +1,7 @@ """Test Discovery class.""" +import logging + import pytest from proxmoxer import ProxmoxAPI @@ -11,6 +13,10 @@ pytest_plugins = [ ] +def records_to_messages(records): + return [r.getMessage() for r in records] + + @pytest.fixture def discovery(mocker): mocker.patch.object(ProxmoxClient, "_auth", return_value=mocker.create_autospec(ProxmoxAPI)) @@ -32,19 +38,27 @@ def test_exclude_state(discovery, qemus): assert len(filtered) == 2 -def test_exclude_tags(discovery, qemus): +def test_exclude_tags(discovery, qemus, local_caplog): discovery.config.config["exclude_tags"] = ["unmonitored"] - filtered = discovery._filter(qemus) + with local_caplog(level=logging.DEBUG) as caplog: + filtered = discovery._filter(qemus) + + assert ( + "vmid 100: discovered tags: ['unmonitored', 'excluded', 'postgres']" + in records_to_messages(caplog.records) + ) + assert "vmid 100: excluded by tags: ['unmonitored']" assert len(filtered) == 2 @pytest.mark.parametrize( - "testinput,expected", [ + "testinput,expected", + [ (["monitored"], 1), (["monitored", "postgres"], 2), ([], 3), - ] + ], ) def test_include_tags(discovery, qemus, testinput, expected): discovery.config.config["include_tags"] = testinput @@ -53,10 +67,13 @@ def test_include_tags(discovery, qemus, testinput, expected): assert len(filtered) == expected -@pytest.mark.parametrize("testinput,expected", [ - (["101", "100"], 2), - ([], 3), -]) +@pytest.mark.parametrize( + "testinput,expected", + [ + (["101", "100"], 2), + ([], 3), + ], +) def test_include_vmid(discovery, qemus, testinput, expected): discovery.config.config["include_vmid"] = testinput filtered = discovery._filter(qemus) diff --git a/prometheuspvesd/test/unit/test_model.py b/prometheuspvesd/test/unit/test_model.py index 75cf634..7ab6c4a 100644 --- a/prometheuspvesd/test/unit/test_model.py +++ b/prometheuspvesd/test/unit/test_model.py @@ -9,34 +9,41 @@ pytest_plugins = [ @pytest.mark.parametrize( - "testinput,expected", [ - ({ - "vmid": 101, - "hostname": "host1", - "ipv4_address": False, - "ipv6_address": False, - "pve_type": "qemu", - }, { - "__meta_pve_vmid": "101", - "__meta_pve_name": "host1", - "__meta_pve_ipv4": "False", - "__meta_pve_ipv6": "False", - "__meta_pve_type": "qemu", - }), - ({ - "vmid": "202", - "hostname": "host2", - "ipv4_address": "129.168.0.1", - "ipv6_address": "2001:db8:3333:4444:5555:6666:7777:8888", - "pve_type": "qemu", - }, { - "__meta_pve_vmid": "202", - "__meta_pve_name": "host2", - "__meta_pve_ipv4": "129.168.0.1", - "__meta_pve_ipv6": "2001:db8:3333:4444:5555:6666:7777:8888", - "__meta_pve_type": "qemu", - }), - ] + "testinput,expected", + [ + ( + { + "vmid": 101, + "hostname": "host1", + "ipv4_address": False, + "ipv6_address": False, + "pve_type": "qemu", + }, + { + "__meta_pve_vmid": "101", + "__meta_pve_name": "host1", + "__meta_pve_ipv4": "False", + "__meta_pve_ipv6": "False", + "__meta_pve_type": "qemu", + }, + ), + ( + { + "vmid": "202", + "hostname": "host2", + "ipv4_address": "129.168.0.1", + "ipv6_address": "2001:db8:3333:4444:5555:6666:7777:8888", + "pve_type": "qemu", + }, + { + "__meta_pve_vmid": "202", + "__meta_pve_name": "host2", + "__meta_pve_ipv4": "129.168.0.1", + "__meta_pve_ipv6": "2001:db8:3333:4444:5555:6666:7777:8888", + "__meta_pve_type": "qemu", + }, + ), + ], ) def test_host(testinput, expected): host = Host( diff --git a/pyproject.toml b/pyproject.toml index d97a8f4..7e2c1ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,7 +78,6 @@ exclude = [ "__pycache__", "build", "dist", - "test", "*.pyc", "*.egg-info", ".cache", @@ -131,6 +130,9 @@ select = [ "RUF", ] +[tool.ruff.per-file-ignores] +"test_*.py" = ["S"] + [tool.ruff.format] quote-style = "double" indent-style = "space"