From 18db3f60dacd68cbdafe7b725ded95034d18ba53 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Fri, 6 Dec 2024 15:23:40 -0500 Subject: [PATCH] fix(slurm_ops): manadate that config server is provided in service file Require the `--conf-server` to be provided for start-up. Variable substitution cannot be used in environment files. Declared environment variables are treated as independent key-value pairs. Signed-off-by: Jason C. Nucciarone --- lib/charms/hpc_libs/v0/slurm_ops.py | 37 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/charms/hpc_libs/v0/slurm_ops.py b/lib/charms/hpc_libs/v0/slurm_ops.py index cb60de3..5652db6 100644 --- a/lib/charms/hpc_libs/v0/slurm_ops.py +++ b/lib/charms/hpc_libs/v0/slurm_ops.py @@ -657,19 +657,26 @@ def _apply_overrides(self) -> None: """Override defaults supplied provided by Slurm Debian packages.""" match self._service_name: case "sackd": - # TODO: https://github.com/charmed-hpc/hpc-libs/issues/54 - - # Make `sackd` create its service environment file so that we - # aren't required to manually create it here. - _logger.debug("creating sackd environment file") - self._env_file.touch(mode=0o644, exist_ok=True) - self._env_file.write_text( + _logger.debug("overriding default sackd service configuration") + config_override = Path( + "/etc/systemd/system/sackd.service.d/10-sackd-config-server.conf" + ) + config_override.parent.mkdir(parents=True, exist_ok=True) + config_override.write_text( textwrap.dedent( """ - SACKD_CONFIG_SERVER='' - SACKD_OPTIONS="${SACKD_CONFIG_SERVER:+--conf-server $SACKD_CONFIG_SERVER}" + [Service] + ExecStart= + ExecStart=/usr/sbin/sackd --systemd --conf-server $SACKD_CONFIG_SERVER """ ) ) + + # TODO: https://github.com/charmed-hpc/hpc-libs/issues/54 - + # Make `sackd` create its service environment file so that we + # aren't required to manually create it here. + _logger.debug("creating sackd environment file") + self._env_file.touch(mode=0o644, exist_ok=True) case "slurmctld": _logger.debug("overriding default slurmctld service configuration") self._set_ulimit() @@ -914,8 +921,7 @@ def __init__(self, *args, **kwargs) -> None: @property def config_server(self) -> str | None: """Get the configuration server address of this `sackd` node.""" - result = self._env_manager.get("SACKD_CONFIG_SERVER") - return None if result == "" else result + return self._env_manager.get("SACKD_CONFIG_SERVER") @config_server.setter def config_server(self, addr: str) -> None: @@ -927,15 +933,8 @@ def config_server(self, addr: str) -> None: @config_server.deleter def config_server(self) -> None: - """Unset the configuration server address of this `sackd` node. - - Warnings: - The `SACKD_CONFIG_SERVER` is set to an emtpy string rather than - deleted from the environment file to preserve the line order necessary - to successfully use variable substitution within the `SACKD_OPTIONS` - environment variable. - """ - self._env_manager.set({"SACKD_CONFIG_SERVER": ""}) + """Unset the configuration server address of this `sackd` node.""" + self._env_manager.unset("SACKD_CONFIG_SERVER") class SlurmctldManager(_SlurmManagerBase):