fix: do not preseed slurm and slurmdbd configuration files

src/slurmhelpers/hooks.py

@@ -89,9 +89,7 @@ def install(snap: Snap) -> None:
     """
     setup_logging(snap.paths.common / "hooks.log")
     munge = Munge(snap)
-    slurm = Slurm(snap)
     slurmd = Slurmd(snap)
-    slurmdbd = Slurmdbd(snap)
     slurmrestd = Slurmrestd(snap)
 
     logging.info("Executing snap `install` hook.")
@@ -107,12 +105,6 @@ def install(snap: Snap) -> None:
     logging.info("Generating default munge.key secret.")
     munge.generate_key()
 
-    logging.info("Creating empty `slurm.conf` file.")
-    slurm.config_file.touch(0o644)
-
-    logging.info("Creating empty `slurmdbd.conf` file.")
-    slurmdbd.config_file.touch(0o644)
-
 
 def configure(snap: Snap) -> None:
     """Configure hook for the Slurm snap."""

src/slurmhelpers/models.py

@@ -228,26 +228,27 @@ def __init__(self, *args) -> None:
 
     def update_config(self, config: Dict[str, str]) -> None:
         """Update configuration for the `slurmdbd` service."""
-        # Load current `slurmdbd.conf` configuration file.
-        # Preserve old configuration so that we can determine
-        # if substantial changes were made to the `slurmdbd.conf` file.
-        sconf = slurmdbdconfig.load(self.config_file)
-        old = sconf.dict()
+        with slurmdbdconfig.edit(self.config_file) as sconf:
+            # Preserve old configuration so that we can determine
+            # if substantial changes were made to the `slurmdbd.conf` file.
+            old = sconf.dict()
 
-        # Assemble new `slurmdbd.conf` file.
-        for k, v in config.items():
-            key = k.replace("-", "_")
-            if not hasattr(sconf, key):
-                raise AttributeError(f"`slurmdbd` config file does not support option {key}.")
+            # Assemble new `slurmdbd.conf` file.
+            for k, v in config.items():
+                key = k.replace("-", "_")
+                if not hasattr(sconf, key):
+                    raise AttributeError(f"`slurmdbd` config file does not support option {key}.")
 
-            setattr(sconf, key, _apply_callback(key, v, model=sconf))
+                setattr(sconf, key, _apply_callback(key, v, model=sconf))
 
-        if sconf.dict() == old:
-            logging.debug("No change in `slurmdbd` service configuration. Not updating.")
-            return
+            if sconf.dict() == old:
+                logging.debug("No change in `slurmdbd` service configuration. Not updating.")
+                return
+
+            logging.info("Updating `slurmdbd` configuration file %s.", self.config_file)
+            self._needs_restart(["slurmdbd"])
 
-        slurmdbdconfig.dump(sconf, self.config_file)
-        self._needs_restart(["slurmdbd"])
+        self.config_file.chmod(0o600)
 
 
 class Slurmrestd(_BaseModel):
@@ -402,41 +403,42 @@ def __init__(self, *args) -> None:
 
     def update_config(self, config: Dict[str, Any]) -> None:
         """Update configuration of the Slurm workload manager."""
-        # Load current `slurm.conf` configuration file.
-        # Preserve old configuration so that we can determine
-        # if substantial changes were made to the `slurm.conf` file.
-        sconf = slurmconfig.load(self.config_file)
-        old = sconf.dict()
-
-        # Assemble new `slurm.conf` file.
-        for k, v in config.items():
-            match key := k.replace("-", "_"):
-                case "include":
-                    # Multiline configuration options. Requires special handling.
-                    sconf.include = v.split(",")
-                case "slurmctld_host":
-                    # Multiline configuration options. Requires special handling.
-                    sconf.slurmctld_host = v.split(",")
-                case "nodes":
-                    sconf.nodes = _process_nodes(v)
-                case "frontend_nodes":
-                    sconf.frontend_nodes = _process_frontend_nodes(v)
-                case "down_nodes":
-                    sconf.down_nodes = _process_down_nodes(v)
-                case "node_sets":
-                    sconf.node_sets = _process_node_sets(v)
-                case "partitions":
-                    sconf.partitions = _process_partitions(v)
-                case _:
-                    if not hasattr(sconf, key):
-                        raise AttributeError(f"Slurm config file does not support option {key}.")
-
-                    setattr(sconf, key, _apply_callback(key, v, model=sconf))
-
-        if sconf.dict() == old:
-            logging.debug("No change in Slurm workload manager configuration. Not updating.")
-            return
-
-        logging.info("Updating Slurm workload manager configuration file %s.", self.config_file)
-        slurmconfig.dump(sconf, self.config_file)
-        self._needs_restart(["slurmctld", "slurmd", "slurmdbd", "slurmrestd"])
+        with slurmconfig.edit(self.config_file) as sconf:
+            # Preserve old configuration so that we can determine
+            # if substantial changes were made to the `slurm.conf` file.
+            old = sconf.dict()
+            # Assemble new `slurm.conf` file.
+            for k, v in config.items():
+                match key := k.replace("-", "_"):
+                    case "include":
+                        # Multiline configuration options. Requires special handling.
+                        sconf.include = v.split(",")
+                    case "slurmctld_host":
+                        # Multiline configuration options. Requires special handling.
+                        sconf.slurmctld_host = v.split(",")
+                    case "nodes":
+                        sconf.nodes = _process_nodes(v)
+                    case "frontend_nodes":
+                        sconf.frontend_nodes = _process_frontend_nodes(v)
+                    case "down_nodes":
+                        sconf.down_nodes = _process_down_nodes(v)
+                    case "node_sets":
+                        sconf.node_sets = _process_node_sets(v)
+                    case "partitions":
+                        sconf.partitions = _process_partitions(v)
+                    case _:
+                        if not hasattr(sconf, key):
+                            raise AttributeError(
+                                f"Slurm config file does not support option {key}."
+                            )
+
+                        setattr(sconf, key, _apply_callback(key, v, model=sconf))
+
+            if sconf.dict() == old:
+                logging.debug("No change in Slurm workload manager configuration. Not updating.")
+                return
+
+            logging.info("Updating Slurm configuration file %s.", self.config_file)
+            self._needs_restart(["slurmctld", "slurmd", "slurmdbd", "slurmrestd"])
+
+        self.config_file.chmod(0o600)

tests/unit/conftest.py

@@ -23,6 +23,12 @@
 from snaphelpers._ctl import ServiceInfo
 
 
+@pytest.fixture
+def fake_fs(fs):
+    """Mock filesystem for configuration hook unit tests."""
+    yield fs
+
+
 @pytest.fixture
 def env():
     """Mock the snap runtime environment."""

tests/unit/test_models.py

@@ -143,7 +143,7 @@ def test_update_config(self, mocker, munge) -> None:
 class TestSlurmModel:
     """Test the `Slurm` data model."""
 
-    def test_update_config(self, mocker, slurm) -> None:
+    def test_update_config(self, mocker, fake_fs, slurm) -> None:
         """Test `update_config` method."""
         nodes = NodeMap()
         frontend_nodes = FrontendNodeMap()
@@ -160,9 +160,11 @@ def test_update_config(self, mocker, slurm) -> None:
         config.node_sets = node_sets
         config.partitions = partitions
 
+        # Create fake file for slurm configuration editor.
+        fake_fs.create_file("/var/snap/slurm/common/etc/slurm/slurm.conf")
+
         # Test when there has been no change to slurm configuration.
         mocker.patch("slurmutils.editors.slurmconfig.load", return_value=config)
-        mocker.patch("slurmutils.editors.slurmconfig.dump")
         mocker.patch("slurmhelpers.models._process_nodes", return_value=nodes)
         mocker.patch("slurmhelpers.models._process_frontend_nodes", return_value=frontend_nodes)
         mocker.patch("slurmhelpers.models._process_down_nodes", return_value=down_nodes)
@@ -183,7 +185,6 @@ def test_update_config(self, mocker, slurm) -> None:
 
         # Test when there has been a change made to the slurm configuration.
         mocker.patch("slurmutils.editors.slurmconfig.load", return_value=config)
-        mocker.patch("slurmutils.editors.slurmconfig.dump")
         mocker.patch("slurmhelpers.models._process_nodes", return_value=nodes)
         mocker.patch("slurmhelpers.models._process_frontend_nodes", return_value=frontend_nodes)
         mocker.patch("slurmhelpers.models._process_down_nodes", return_value=down_nodes)
@@ -204,7 +205,6 @@ def test_update_config(self, mocker, slurm) -> None:
 
         # Test when a bad slurmdbd configuration option has been provided.
         mocker.patch("slurmutils.editors.slurmconfig.load", return_value=config)
-        mocker.patch("slurmutils.editors.slurmconfig.dump")
         mocker.patch("slurmhelpers.models._process_nodes", return_value=nodes)
         mocker.patch("slurmhelpers.models._process_frontend_nodes", return_value=frontend_nodes)
         mocker.patch("slurmhelpers.models._process_down_nodes", return_value=down_nodes)
@@ -407,13 +407,16 @@ def test_update_config(self, mocker, slurmd) -> None:
 class TestSlurmdbdModel:
     """Test the `Slurmdbd` data model."""
 
-    def test_update_config(self, mocker, slurmdbd) -> None:
+    def test_update_config(self, mocker, fake_fs, slurmdbd) -> None:
         """Test `update_config` method."""
         config = SlurmdbdConfig()
         config.log_file = "/var/log/slurm/slurmdbd.log"
         config.private_data = ["accounts", "events", "jobs"]
         config.track_slurmctld_down = "no"
 
+        # Create fake file for slurmdbd configuration editor.
+        fake_fs.create_file("/var/snap/slurm/common/etc/slurm/slurmdbd.conf")
+
         # Test when there has been no change to slurmdbd configuration.
         mocker.patch("slurmutils.editors.slurmdbdconfig.load", return_value=config)
         mocker.patch("slurmutils.editors.slurmdbdconfig.dump")
@@ -427,7 +430,6 @@ def test_update_config(self, mocker, slurmdbd) -> None:
 
         # Test when there has been a change to the slurmdbd configuration.
         mocker.patch("slurmutils.editors.slurmdbdconfig.load", return_value=config)
-        mocker.patch("slurmutils.editors.slurmdbdconfig.dump")
         slurmdbd.update_config(
             {
                 "log-file": "/var/log/slurm/slurmdbd.log",
@@ -438,7 +440,6 @@ def test_update_config(self, mocker, slurmdbd) -> None:
 
         # Test when a bad slurmdbd configuration option has been provided.
         mocker.patch("slurmutils.editors.slurmdbdconfig.load", return_value=config)
-        mocker.patch("slurmutils.editors.slurmdbdconfig.dump")
         with pytest.raises(AttributeError):
             slurmdbd.update_config(
                 {

tox.ini

@@ -56,6 +56,7 @@ description = Run unit tests.
 deps =
     pytest
     pytest-mock
+    pyfakefs
     coverage[toml]
     -r{toxinidir}/requirements.txt
 commands =