From d0db66b6338fa10f3b3d77e955ce5ec170ad3960 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Wed, 10 Jul 2024 16:45:12 -0400 Subject: [PATCH 1/4] feat: enable to bulk get and unset on config object Additional changes: - Fixes bug with `unset`. '!' does not need to be appended to the end of key names if using `snap unset ...` instead of `snap set ...`. Signed-off-by: Jason C. Nucciarone --- lib/charms/hpc_libs/v0/slurm_ops.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/charms/hpc_libs/v0/slurm_ops.py b/lib/charms/hpc_libs/v0/slurm_ops.py index d72c145..26795f6 100644 --- a/lib/charms/hpc_libs/v0/slurm_ops.py +++ b/lib/charms/hpc_libs/v0/slurm_ops.py @@ -202,8 +202,8 @@ def restart(self) -> None: class ConfigurationManager: """Control configuration of a Slurm component.""" - def __init__(self, service: ServiceType) -> None: - self._service = service + def __init__(self, name: str) -> None: + self._name = name def get_options(self, *keys: str) -> Mapping[str, Any]: """Get given configurations values for Slurm component.""" @@ -215,20 +215,20 @@ def get_options(self, *keys: str) -> Mapping[str, Any]: return configs - def get(self, key: str) -> Any: + def get(self, key: Optional[str] = None) -> Any: """Get specific configuration value for Slurm component.""" - key = f"{self._service.config_name}.{key}" + key = f"{self._name}.{key}" if key else self._name config = json.loads(_snap("get", "-d", "slurm", key)) return config[key] def set(self, config: Mapping[str, Any]) -> None: """Set configuration for Slurm component.""" - args = [f"{self._service.config_name}.{k}={json.dumps(v)}" for k, v in config.items()] + args = [f"{self._name}.{k}={json.dumps(v)}" for k, v in config.items()] _snap("set", "slurm", *args) - def unset(self, *keys) -> None: + def unset(self, *keys: str) -> None: """Unset configuration for Slurm component.""" - args = [f"{self._service.config_name}.{k}!" for k in keys] + args = [f"{self._name}.{k}" for k in keys] if len(keys) > 0 else [self._name] _snap("unset", "slurm", *args) @@ -236,8 +236,9 @@ class MungeManager(ServiceManager): """Manage `munged` service operations.""" def __init__(self) -> None: - self._service = ServiceType.MUNGED - self.config = ConfigurationManager(ServiceType.MUNGED) + service = ServiceType.MUNGED + self._service = service + self.config = ConfigurationManager(service.config_name) def get_key(self) -> str: """Get the current munge key. @@ -265,5 +266,5 @@ class SlurmManagerBase(ServiceManager): def __init__(self, service: ServiceType) -> None: self._service = service - self.config = ConfigurationManager(service) + self.config = ConfigurationManager(service.config_name) self.munge = MungeManager() From 378f4e977e654782a51e1558e775db0641676cbb Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Wed, 10 Jul 2024 16:46:58 -0400 Subject: [PATCH 2/4] tests: add unit tests for bulk getting and setting Signed-off-by: Jason C. Nucciarone --- tests/unit/test_slurm_ops.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_slurm_ops.py b/tests/unit/test_slurm_ops.py index 652f02c..3c6900a 100644 --- a/tests/unit/test_slurm_ops.py +++ b/tests/unit/test_slurm_ops.py @@ -79,19 +79,19 @@ def test_config_name(self, *_) -> None: def test_enable(self, subcmd, *_) -> None: """Test that the manager calls the correct enable command.""" self.manager.enable() - calls = [args[0][0] for args in subcmd.call_args_list] + args = subcmd.call_args[0][0] self.assertEqual( - calls[0], ["snap", "start", "--enable", f"slurm.{self.manager._service.value}"] + args, ["snap", "start", "--enable", f"slurm.{self.manager._service.value}"] ) def test_disable(self, subcmd, *_) -> None: """Test that the manager calls the correct disable command.""" self.manager.disable() - calls = [args[0][0] for args in subcmd.call_args_list] + args = subcmd.call_args[0][0] self.assertEqual( - calls[0], ["snap", "stop", "--disable", f"slurm.{self.manager._service.value}"] + args, ["snap", "stop", "--disable", f"slurm.{self.manager._service.value}"] ) def test_restart(self, subcmd, *_) -> None: @@ -120,6 +120,14 @@ def test_get_config(self, subcmd, *_) -> None: self.assertEqual(args, ["snap", "get", "-d", "slurm", f"{self.config_name}.key"]) self.assertEqual(value, "value") + def test_get_config_all(self, subcmd) -> None: + """Test that manager calls the correct `snap get ...` with no arguments given.""" + subcmd.return_value = '{"%s": "value"}' % self.config_name + value = self.manager.config.get() + args = subcmd.call_args[0][0] + self.assertEqual(args, ["snap", "get", "-d", "slurm", self.config_name]) + self.assertEqual(value, "value") + def test_set_config(self, subcmd, *_) -> None: """Test that the manager calls the correct `snap set ...` command.""" self.manager.config.set({"key": "value"}) @@ -130,7 +138,13 @@ def test_unset_config(self, subcmd) -> None: """Test that the manager calls the correct `snap unset ...` command.""" self.manager.config.unset("key") args = subcmd.call_args[0][0] - self.assertEqual(args, ["snap", "unset", "slurm", f"{self.config_name}.key!"]) + self.assertEqual(args, ["snap", "unset", "slurm", f"{self.config_name}.key"]) + + def test_unset_config_all(self, subcmd) -> None: + """Test the manager calls the correct `snap unset ...` with no arguments given.""" + self.manager.config.unset() + args = subcmd.call_args[0][0] + self.assertEqual(args, ["snap", "unset", "slurm", self.config_name]) def test_generate_munge_key(self, subcmd, *_) -> None: """Test that the manager calls the correct `mungectl` command.""" From 74cfcdcfb11b443880b59c31c2f2a298af24302b Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Wed, 10 Jul 2024 16:56:22 -0400 Subject: [PATCH 3/4] feat: enable public export of `ConfigurationManager` Signed-off-by: Jason C. Nucciarone --- lib/charms/hpc_libs/v0/slurm_ops.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/charms/hpc_libs/v0/slurm_ops.py b/lib/charms/hpc_libs/v0/slurm_ops.py index 26795f6..46fe818 100644 --- a/lib/charms/hpc_libs/v0/slurm_ops.py +++ b/lib/charms/hpc_libs/v0/slurm_ops.py @@ -59,6 +59,7 @@ def _on_install(self, _) -> None: "format_key", "install", "version", + "ConfigurationManager", "ServiceType", "SlurmManagerBase", ] From 61959d9e0cc840c8c34f9fd8b754a3e9fd43c202 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Wed, 10 Jul 2024 17:30:37 -0400 Subject: [PATCH 4/4] chore: bump `LIBPATCH` version Signed-off-by: Jason C. Nucciarone --- lib/charms/hpc_libs/v0/slurm_ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/hpc_libs/v0/slurm_ops.py b/lib/charms/hpc_libs/v0/slurm_ops.py index 46fe818..d583d9b 100644 --- a/lib/charms/hpc_libs/v0/slurm_ops.py +++ b/lib/charms/hpc_libs/v0/slurm_ops.py @@ -82,7 +82,7 @@ def _on_install(self, _) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 2 +LIBPATCH = 3 # Charm library dependencies to fetch during `charmcraft pack`. PYDEPS = ["pyyaml>=6.0.1"]