From 07fbd47c035b180a2af503f6d21d704179c36efc Mon Sep 17 00:00:00 2001 From: Laurent Corbes Date: Wed, 13 May 2026 11:38:57 +0200 Subject: [PATCH 1/3] feat(config): Add option --config Add --config option to specify a custom configuration file in a different path chore(doc): Update README fix(config): Potential empty filename Fix issue when the filename of a config il confuse can be None --- README.md | 3 ++ src/pvecontrol/__init__.py | 24 ++++++++- src/pvecontrol/config.py | 3 ++ src/tests/test_config.py | 100 ++++++++++++++++++++++++++++++++++++- 4 files changed, 127 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 57db16e..3999b49 100644 --- a/README.md +++ b/README.md @@ -203,6 +203,8 @@ Options: configuration [required] --unicode / --no-unicode Use unicode characters for output --color / --no-color Use colorized output + --config PATH Path to configuration file (overrides + default ~/.config/pvecontrol/config.yaml) --help Show this message and exit. Commands: @@ -260,6 +262,7 @@ If this works, we're good to go! - `PVECONTROL_CLUSTER`: the default cluster to use when no `-c` or `--cluster` option is specified. - `PVECONTROL_COLOR`: if set to `False`, it will disable all colorized output. - `PVECONTROL_UNICODE`: if set to `False`, it will disable all unicode output. +- `PVECONTROL_CONFIG`: path to the configuration file to use instead of the default `~/.config/pvecontrol/config.yaml`. The specified file is merged with the defaults, not a full replacement. ## Shell completion diff --git a/src/pvecontrol/__init__.py b/src/pvecontrol/__init__.py index 64b77bb..1464de3 100644 --- a/src/pvecontrol/__init__.py +++ b/src/pvecontrol/__init__.py @@ -129,13 +129,33 @@ def format_commands(self, ctx, formatter) -> None: default=True, help="Use colorized output", ) +@click.option( + "--config", + "config_file", + metavar="PATH", + envvar="CONFIG", + type=click.Path(exists=True, dir_okay=False, readable=True), + default=None, + help=f"Path to configuration file (overrides default {config.user_config_path()})", +) @click.pass_context -def pvecontrol(ctx, debug, output, cluster, unicode, color): +def pvecontrol(ctx, debug, output, cluster, unicode, color, config_file): signal.signal(signal.SIGINT, lambda *_: sys.exit(130)) # Disable urllib3 warnings about invalid certs urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) if not ctx.command.ignoring: + if config_file: + # Use the given file authoritatively: layer it on top of the packaged + # defaults only, skipping the user config (~/.config/pvecontrol/config.yaml). + # confuse merges sources key-by-key, so without this any key absent from + # the given file (proxy_certificate, timeout, extra clusters, ...) would + # leak in from the user config. And since the config is a list and not + # a dict, items of the list will be merged based on their index in the + # config file, producing very weird behaviors. + config.read(user=False, defaults=True) + config.set_file(config_file) + # configure logging logging.basicConfig(encoding="utf-8", level=logging.DEBUG if debug else logging.INFO) @@ -147,7 +167,7 @@ def pvecontrol(ctx, debug, output, cluster, unicode, color): logging.debug("Auto-selected single cluster: %s", cluster) elif len(clusters) == 0: logging.error("No cluster configured. Please add a cluster to your configuration file.") - logging.error("Configuration file: %s", config.user_config_path()) + logging.error("Configuration file: %s", config_file or config.user_config_path()) ctx.exit(1) else: available = "\n".join(f" {name}" for name in clusters) diff --git a/src/pvecontrol/config.py b/src/pvecontrol/config.py index 0f026ac..37cc248 100644 --- a/src/pvecontrol/config.py +++ b/src/pvecontrol/config.py @@ -70,6 +70,9 @@ def list_clusters(): def set_config(cluster_name): validconfig = _load_config() + for source in config.sources: + if source.filename: + logging.debug("configuration file: %s", source.filename) logging.debug("configuration is %s", validconfig) # FIXME trouver une methode plus clean pour recuperer la configuration du bon cluster diff --git a/src/tests/test_config.py b/src/tests/test_config.py index 47e4eca..eef32df 100644 --- a/src/tests/test_config.py +++ b/src/tests/test_config.py @@ -1,10 +1,16 @@ import importlib +import os +import tempfile import unittest from unittest.mock import MagicMock, patch import confuse +import yaml +from click.testing import CliRunner -from pvecontrol.config import list_clusters, set_config +import pvecontrol as pvecontrol_module +from pvecontrol import pvecontrol +from pvecontrol.config import configtemplate, list_clusters, set_config # pvecontrol/__init__.py does `from pvecontrol.config import config`, which # overwrites the `config` attribute on the pvecontrol package with the LazyConfig @@ -115,3 +121,95 @@ def test_ambiguous_names_error_lists_conflicts(self): with self.assertRaises(SystemExit): set_config("prod") self.assertTrue(any("prod" in msg and "PROD" in msg for msg in log.output)) + + +def _make_temp_config(overrides=None): + content = { + "clusters": [{"name": "test", "host": "127.0.0.1", "user": "root@pam", "password": "secret"}], + "node": {"cpufactor": 2.5, "memoryminimum": 8589934592}, + "vm": {"max_last_backup": 1500}, + } + if overrides: + content.update(overrides) + f = tempfile.NamedTemporaryFile(suffix=".yaml", mode="w", delete=False) + yaml.dump(content, f) + f.close() + return f.name + + +class TestConfigFileOption(unittest.TestCase): + + def setUp(self): + # test_pvecontrol.py::test_get_leaf_command calls make_context with --help args, + # which sets ignoring=True on the shared pvecontrol group instance and never resets it. + pvecontrol.ignoring = False + + def test_config_option_calls_set_file(self): + config_path = _make_temp_config() + try: + runner = CliRunner() + with ( + patch.object(pvecontrol_module, "config") as mock_config, + patch("pvecontrol.actions.cluster.PVECluster.create_from_config", side_effect=SystemExit(0)), + ): + runner.invoke(pvecontrol, ["--config", config_path, "--cluster", "test", "status"]) + mock_config.set_file.assert_called_once_with(config_path) + finally: + os.unlink(config_path) + + def test_config_option_skips_user_config(self): + # The given file must be layered on the packaged defaults only (user=False), + # otherwise confuse merges the user config underneath and keys absent from the + # given file leak in from ~/.config/pvecontrol/config.yaml. + config_path = _make_temp_config() + try: + runner = CliRunner() + with ( + patch.object(pvecontrol_module, "config") as mock_config, + patch("pvecontrol.actions.cluster.PVECluster.create_from_config", side_effect=SystemExit(0)), + ): + runner.invoke(pvecontrol, ["--config", config_path, "--cluster", "test", "status"]) + mock_config.read.assert_called_once_with(user=False, defaults=True) + finally: + os.unlink(config_path) + + def test_no_config_option_skips_set_file(self): + runner = CliRunner() + with ( + patch.object(pvecontrol_module, "config") as mock_config, + patch("pvecontrol.actions.cluster.PVECluster.create_from_config", side_effect=SystemExit(0)), + ): + runner.invoke(pvecontrol, ["--cluster", "test", "status"]) + mock_config.set_file.assert_not_called() + mock_config.read.assert_not_called() + + def test_autodetect_no_cluster_error_references_custom_config(self): + # When autodetection finds no cluster, the error must point at the file + # passed with --config, not at the default user config path. + config_path = _make_temp_config({"clusters": []}) + try: + runner = CliRunner() + with self.assertLogs("root", level="ERROR") as log: + runner.invoke(pvecontrol, ["--config", config_path, "status"], env={"PVECONTROL_CLUSTER": None}) + assert any(config_path in msg for msg in log.output) + finally: + os.unlink(config_path) + + def test_custom_config_file_has_highest_priority(self): + config_path = _make_temp_config() + try: + cfg = confuse.Configuration("pvecontrol_test", read=False) + cfg.set_file(config_path) + assert cfg.sources[0].filename == config_path + finally: + os.unlink(config_path) + + def test_custom_config_values_are_loaded(self): + config_path = _make_temp_config({"node": {"cpufactor": 99.0, "memoryminimum": 8589934592}}) + try: + cfg = confuse.Configuration("pvecontrol_test", read=False) + cfg.set_file(config_path) + result = cfg.get(configtemplate) + assert result.node["cpufactor"] == 99.0 + finally: + os.unlink(config_path) From ed2889535b914be7d6f3131ada19f92305fb7f8c Mon Sep 17 00:00:00 2001 From: Laurent Corbes Date: Tue, 19 May 2026 14:00:56 +0200 Subject: [PATCH 2/3] chore(README): Update PVECONTROL_CONFIG variable description --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3999b49..12b1d64 100644 --- a/README.md +++ b/README.md @@ -262,7 +262,7 @@ If this works, we're good to go! - `PVECONTROL_CLUSTER`: the default cluster to use when no `-c` or `--cluster` option is specified. - `PVECONTROL_COLOR`: if set to `False`, it will disable all colorized output. - `PVECONTROL_UNICODE`: if set to `False`, it will disable all unicode output. -- `PVECONTROL_CONFIG`: path to the configuration file to use instead of the default `~/.config/pvecontrol/config.yaml`. The specified file is merged with the defaults, not a full replacement. +- `PVECONTROL_CONFIG`: path to the configuration file to use instead of the default `~/.config/pvecontrol/config.yaml`. ## Shell completion From effcebd0cf4e47df8a6fdb750b9e187434c25102 Mon Sep 17 00:00:00 2001 From: Paul Laffitte Date: Fri, 5 Jun 2026 17:08:58 +0200 Subject: [PATCH 3/3] tests: fix pylint issue (consider-using-with) --- src/pvecontrol/actions/vm.py | 1 - src/tests/sanitycheck/test_vm_disks.py | 1 - src/tests/test_config.py | 7 +++---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/pvecontrol/actions/vm.py b/src/pvecontrol/actions/vm.py index 7a2804e..21b450a 100644 --- a/src/pvecontrol/actions/vm.py +++ b/src/pvecontrol/actions/vm.py @@ -76,7 +76,6 @@ def migrate(ctx, vmid, target, online, follow, wait, dry_run): # Lancer tache de migration upid = proxmox.api.nodes(node.node).qemu(vmid).migrate.post(**options) # Suivre la task cree - # pylint: disable=duplicate-code proxmox.refresh() print_task(proxmox, upid, follow, wait) else: diff --git a/src/tests/sanitycheck/test_vm_disks.py b/src/tests/sanitycheck/test_vm_disks.py index c81d4df..ba0fdae 100644 --- a/src/tests/sanitycheck/test_vm_disks.py +++ b/src/tests/sanitycheck/test_vm_disks.py @@ -1,4 +1,3 @@ -# pylint: disable=duplicate-code from unittest.mock import patch from pvecontrol.models.cluster import PVECluster from pvecontrol.sanitycheck.tests.vm import DiskUnused diff --git a/src/tests/test_config.py b/src/tests/test_config.py index eef32df..4bd66ab 100644 --- a/src/tests/test_config.py +++ b/src/tests/test_config.py @@ -131,10 +131,9 @@ def _make_temp_config(overrides=None): } if overrides: content.update(overrides) - f = tempfile.NamedTemporaryFile(suffix=".yaml", mode="w", delete=False) - yaml.dump(content, f) - f.close() - return f.name + with tempfile.NamedTemporaryFile(suffix=".yaml", mode="w", delete=False) as f: + yaml.dump(content, f) + return f.name class TestConfigFileOption(unittest.TestCase):