diff --git a/.gitignore b/.gitignore index 6fe70d6..b951f5d 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,4 @@ pip-delete-this-directory.txt .vscode/ .pytest* +CLAUDE.md diff --git a/src/pvecontrol/__init__.py b/src/pvecontrol/__init__.py index c39f142..64b77bb 100644 --- a/src/pvecontrol/__init__.py +++ b/src/pvecontrol/__init__.py @@ -11,6 +11,7 @@ import urllib3 from pvecontrol import actions +from pvecontrol.config import config, list_clusters from pvecontrol.utils import OutputFormats @@ -57,8 +58,8 @@ def _is_defaulting_to_help(self, ctx, args): return False def parse_args(self, ctx, args): - if self._is_defaulting_to_help(ctx, args): - self.ignoring = True + self.ignoring = self._is_defaulting_to_help(ctx, args) + if self.ignoring: for param in self.params: param.required = False @@ -113,7 +114,7 @@ def format_commands(self, ctx, formatter) -> None: "--cluster", metavar="NAME", envvar="CLUSTER", - required=True, + default=None, help="Proxmox cluster name as defined in configuration", ) @click.option( @@ -135,11 +136,32 @@ def pvecontrol(ctx, debug, output, cluster, unicode, color): urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) if not ctx.command.ignoring: - # get cli arguments - args = SimpleNamespace(output=output, cluster=cluster, unicode=unicode, color=color) - # configure logging logging.basicConfig(encoding="utf-8", level=logging.DEBUG if debug else logging.INFO) + + if cluster is None: + clusters = list_clusters() + logging.debug("No cluster specified, found %d cluster(s) in config: %s", len(clusters), clusters) + if len(clusters) == 1: + cluster = clusters[0] + 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()) + ctx.exit(1) + else: + available = "\n".join(f" {name}" for name in clusters) + logging.error( + "No cluster specified. Available clusters:\n%s\n\n" + "Use -c / --cluster NAME or set the PVECONTROL_CLUSTER environment variable.", + available, + ) + ctx.exit(1) + else: + logging.debug("Cluster specified: %s", cluster) + + # get cli arguments + args = SimpleNamespace(output=output, cluster=cluster, unicode=unicode, color=color) logging.debug("Arguments: %s", args) ctx.ensure_object(dict) diff --git a/src/pvecontrol/config.py b/src/pvecontrol/config.py index 9022be2..0f026ac 100644 --- a/src/pvecontrol/config.py +++ b/src/pvecontrol/config.py @@ -48,19 +48,40 @@ config = confuse.LazyConfig("pvecontrol", __name__) +def _load_config(): + try: + validconfig = config.get(configtemplate) + except confuse.ConfigReadError as e: + logging.error("Cannot read configuration file: %s", e) + sys.exit(1) + except confuse.NotFoundError as e: + logging.error("Missing required configuration key: %s", e) + sys.exit(1) + except confuse.ConfigError as e: + logging.error("Invalid configuration: %s", e) + sys.exit(1) + return validconfig + + +def list_clusters(): + validconfig = _load_config() + return [c.name for c in validconfig.clusters] + + def set_config(cluster_name): - validconfig = config.get(configtemplate) + validconfig = _load_config() logging.debug("configuration is %s", validconfig) # FIXME trouver une methode plus clean pour recuperer la configuration du bon cluster # Peut etre rework la configuration completement avec un dict - clusterconfig = False - for c in validconfig.clusters: - if c.name == cluster_name: - clusterconfig = c - if not clusterconfig: + matches = [c for c in validconfig.clusters if c.name.lower() == cluster_name.lower()] + if not matches: logging.error('No such cluster "%s"', cluster_name) sys.exit(1) + if len(matches) > 1: + logging.error('Ambiguous cluster name "%s": matches %s', cluster_name, [c.name for c in matches]) + sys.exit(1) + clusterconfig = matches[0] logging.debug("clusterconfig is %s", clusterconfig) for k, v in validconfig.node.items(): diff --git a/src/pvecontrol/config_default.yaml b/src/pvecontrol/config_default.yaml index 4233271..402028d 100644 --- a/src/pvecontrol/config_default.yaml +++ b/src/pvecontrol/config_default.yaml @@ -1,14 +1,6 @@ --- -clusters: - - name: cluster1 - host: 127.0.0.1 - user: pvecontrol@pve - timeout: 60 - ssl_verify: false - # password: somerandomsecret - # token_name: mytokenname - # token_value: somemorerandomsecret +clusters: [] node: cpufactor: 2.5 diff --git a/src/tests/sanitycheck/test_vm_disks.py b/src/tests/sanitycheck/test_vm_disks.py index ba0fdae..c81d4df 100644 --- a/src/tests/sanitycheck/test_vm_disks.py +++ b/src/tests/sanitycheck/test_vm_disks.py @@ -1,3 +1,4 @@ +# 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 new file mode 100644 index 0000000..47e4eca --- /dev/null +++ b/src/tests/test_config.py @@ -0,0 +1,117 @@ +import importlib +import unittest +from unittest.mock import MagicMock, patch + +import confuse + +from pvecontrol.config import 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 +# instance. importlib.import_module resolves via sys.modules directly, bypassing +# that shadowing, and returns the actual module. +config_module = importlib.import_module("pvecontrol.config") + + +def _make_cluster(name, node=None, vm=None): + c = MagicMock() + c.name = name + c.node = node if node is not None else {} + c.vm = vm if vm is not None else {} + return c + + +def _make_validconfig(cluster_names): + vc = MagicMock() + vc.clusters = [_make_cluster(n) for n in cluster_names] + vc.node = {"cpufactor": 2.5, "memoryminimum": 8589934592} + vc.vm = {"max_last_backup": 1500} + return vc + + +class TestLoadConfigErrors(unittest.TestCase): + + def test_config_read_error(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.side_effect = confuse.ConfigReadError("config.yaml") + with self.assertRaises(SystemExit) as cm: + list_clusters() + self.assertEqual(cm.exception.code, 1) + + def test_not_found_error(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.side_effect = confuse.NotFoundError() + with self.assertRaises(SystemExit) as cm: + list_clusters() + self.assertEqual(cm.exception.code, 1) + + def test_config_type_error(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.side_effect = confuse.ConfigTypeError() + with self.assertRaises(SystemExit) as cm: + list_clusters() + self.assertEqual(cm.exception.code, 1) + + +class TestListClusters(unittest.TestCase): + + def test_empty(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = _make_validconfig([]) + self.assertEqual(list_clusters(), []) + + def test_returns_names(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = _make_validconfig(["prod", "staging"]) + self.assertEqual(list_clusters(), ["prod", "staging"]) + + +class TestSetConfig(unittest.TestCase): + + def test_case_insensitive_lower_input(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = _make_validconfig(["PROD"]) + result = set_config("prod") + self.assertEqual(result.name, "PROD") + + def test_case_insensitive_upper_input(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = _make_validconfig(["prod"]) + result = set_config("PROD") + self.assertEqual(result.name, "prod") + + def test_exact_match(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = _make_validconfig(["prod"]) + result = set_config("prod") + self.assertEqual(result.name, "prod") + + def test_not_found(self): + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = _make_validconfig(["prod", "staging"]) + with self.assertRaises(SystemExit) as cm: + set_config("unknown") + self.assertEqual(cm.exception.code, 1) + + def test_ambiguous_names(self): + vc = MagicMock() + vc.clusters = [_make_cluster("prod"), _make_cluster("PROD")] + vc.node = {} + vc.vm = {} + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = vc + with self.assertRaises(SystemExit) as cm: + set_config("prod") + self.assertEqual(cm.exception.code, 1) + + def test_ambiguous_names_error_lists_conflicts(self): + vc = MagicMock() + vc.clusters = [_make_cluster("prod"), _make_cluster("PROD")] + vc.node = {} + vc.vm = {} + with patch.object(config_module, "config") as mock_config: + mock_config.get.return_value = vc + with self.assertLogs("root", level="ERROR") as log: + with self.assertRaises(SystemExit): + set_config("prod") + self.assertTrue(any("prod" in msg and "PROD" in msg for msg in log.output)) diff --git a/src/tests/test_pvecontrol.py b/src/tests/test_pvecontrol.py index be380f4..4a5da0d 100644 --- a/src/tests/test_pvecontrol.py +++ b/src/tests/test_pvecontrol.py @@ -1,3 +1,7 @@ +from unittest.mock import patch + +from click.testing import CliRunner + from pvecontrol import pvecontrol, get_leaf_command from pvecontrol.utils import reorder_keys from pvecontrol.actions.node import root as node, evacuate @@ -26,3 +30,37 @@ def test_get_leaf_command(): leaf_cmd, leaf_args = get_leaf_command(pvecontrol, ctx, testcase[1]) assert leaf_cmd == testcase[0] assert leaf_args == testcase[2] + + +@patch("pvecontrol.list_clusters", return_value=[]) +def test_no_cluster_configured(_, caplog): + result = CliRunner().invoke(pvecontrol, ["status"]) + assert result.exit_code == 1 + assert "No cluster configured" in caplog.text + + +@patch("pvecontrol.list_clusters", return_value=[]) +def test_no_cluster_configured_shows_config_path(_, caplog): + CliRunner().invoke(pvecontrol, ["status"]) + assert "Configuration file:" in caplog.text + + +@patch("pvecontrol.list_clusters", return_value=["prod", "staging"]) +def test_multiple_clusters_lists_names(_, caplog): + result = CliRunner().invoke(pvecontrol, ["status"]) + assert result.exit_code == 1 + assert "prod" in caplog.text + assert "staging" in caplog.text + + +@patch("pvecontrol.list_clusters", return_value=["prod", "staging"]) +def test_multiple_clusters_shows_usage_hint(_, caplog): + CliRunner().invoke(pvecontrol, ["status"]) + assert "--cluster" in caplog.text + + +@patch("pvecontrol.actions.cluster.PVECluster.create_from_config") +@patch("pvecontrol.list_clusters", return_value=["prod"]) +def test_auto_select_single_cluster(_, mock_create): + CliRunner().invoke(pvecontrol, ["status"]) + mock_create.assert_called_once_with("prod")