From bf233b19af3670d99f3e4022ec1d50acfe3c748c Mon Sep 17 00:00:00 2001 From: Doug Holt Date: Thu, 1 Oct 2026 15:54:58 -0600 Subject: [PATCH] fix(mig): make the unmanaged hooks file opt-in --- config.example/group_vars/all.yml | 5 ++- docs/slurm-cluster/nvml.md | 2 + playbooks/nvidia-software/nvidia-mig.yml | 6 ++- tests/mig-hooks/test_hooks.py | 49 ++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 tests/mig-hooks/test_hooks.py diff --git a/config.example/group_vars/all.yml b/config.example/group_vars/all.yml index 1b2d2da43..afb79e26f 100644 --- a/config.example/group_vars/all.yml +++ b/config.example/group_vars/all.yml @@ -283,7 +283,10 @@ deepops_disable_cloud_init: true # Default profile when using NVIDIA MIG Manager: https://github.com/NVIDIA/mig-parted mig_manager_profile: all-disabled mig_manager_config: /etc/nvidia-mig-manager/config.yml -mig_manager_hooks: /etc/nvidia-mig-manager/hooks.yaml +# Optional, site-managed hooks file passed to `nvidia-mig-parted apply -k`. +# DeepOps does not create this file. Leave empty to apply without custom hooks; +# set a path only after provisioning and reviewing the hooks on every target. +mig_manager_hooks: "" ################################################################################ # Container registry # diff --git a/docs/slurm-cluster/nvml.md b/docs/slurm-cluster/nvml.md index 605753315..632f3e3e1 100644 --- a/docs/slurm-cluster/nvml.md +++ b/docs/slurm-cluster/nvml.md @@ -74,6 +74,8 @@ ansible-playbook -l slurm-node -e mig_manager_profile="all-1g.10gb" playbooks/nv Where `mig_manager_profile` is a configuration profile for the NVIDIA `mig-parted` tool. +DeepOps does not create a MIG hooks file. The example configuration leaves `mig_manager_hooks: ""`, so the playbook omits the `-k` option. To use custom hooks, provision a reviewed hooks file on every target and set `mig_manager_hooks` to its path. An explicitly configured missing or invalid file remains an error; it is never silently ignored. Existing inventories that still set `/etc/nvidia-mig-manager/hooks.yaml` must either provide that file or clear the variable. Drain GPU workloads and stop GPU clients before changing MIG configuration, or provide site-specific hooks to manage those clients; omitting hooks does not quiesce them. + For more information on configuring MIG, see the documentation for [NVIDIA mig-parted](https://github.com/NVIDIA/mig-parted). ### MIG configuration example diff --git a/playbooks/nvidia-software/nvidia-mig.yml b/playbooks/nvidia-software/nvidia-mig.yml index a77bda84c..37299c226 100644 --- a/playbooks/nvidia-software/nvidia-mig.yml +++ b/playbooks/nvidia-software/nvidia-mig.yml @@ -58,13 +58,14 @@ when: - has_mig.rc == 0 - # TODO: Consider storing a custom copy of the hooks.yaml configuration alongside the config.yaml - name: copy cluster-wide mig config file copy: src: "../../config/nvidia-mig-config.yml" dest: "{{ mig_manager_config }}" when: has_mig.rc == 0 + # DeepOps does not manage a hooks file. Only request hooks when the site + # explicitly configures them; a missing configured file must still fail. # nvidia-mig-parted drives the same GPUs as the probe, so it needs the same # isolation from the ssh cgroup. A failure here is left unhandled on purpose: # a MIG layout that could not be applied must not pass as applied. @@ -72,7 +73,8 @@ command: >- systemd-run --wait --pipe --collect --quiet -- nvidia-mig-parted apply -f {{ mig_manager_config }} - -c {{ mig_manager_profile }} -k {{ mig_manager_hooks }} + -c {{ mig_manager_profile }} + {{ ('-k ' ~ (mig_manager_hooks | quote)) if mig_manager_hooks | default('') | length > 0 else '' }} when: has_mig.rc == 0 # Reboot nodes if necessary and poll for them to come up diff --git a/tests/mig-hooks/test_hooks.py b/tests/mig-hooks/test_hooks.py new file mode 100644 index 000000000..d2f639706 --- /dev/null +++ b/tests/mig-hooks/test_hooks.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +"""Render the actual MIG apply command without running GPU operations. + +Run using the project's Ansible Python environment: + python tests/mig-hooks/test_hooks.py +""" +import os +from pathlib import Path +import shlex +import unittest + +from ansible.parsing.dataloader import DataLoader +from ansible.template import Templar +import yaml + +ROOT = Path(os.environ.get("DEEPOPS_TEST_ROOT", Path(__file__).resolve().parents[2])) +PLAY = yaml.safe_load((ROOT / "playbooks/nvidia-software/nvidia-mig.yml").read_text())[0] +APPLY = next(task for task in PLAY["tasks"] if task.get("name") == "Apply MIG configuration") +DEFAULTS = yaml.safe_load((ROOT / "config.example/group_vars/all.yml").read_text()) + + +class Hooks(unittest.TestCase): + def render(self, values): + variables = {"mig_manager_config": "/etc/mig/config.yml", + "mig_manager_profile": "all-disabled", **values} + rendered = Templar(loader=DataLoader(), variables=variables).template(APPLY["command"]) + return shlex.split(rendered) + + def test_example_defaults_do_not_require_an_unmanaged_file(self): + self.assertNotIn("-k", self.render(DEFAULTS)) + + def test_unset_or_empty_hooks_omit_flag(self): + for values in ({}, {"mig_manager_hooks": ""}): + with self.subTest(values=values): + self.assertNotIn("-k", self.render(values)) + + def test_explicit_hooks_are_preserved_as_one_argument(self): + # Includes a missing path: do not silently bypass an operator's hooks. + for path in ("/etc/mig/custom-hooks.yaml", "/missing/hooks file.yaml"): + with self.subTest(path=path): + args = self.render({"mig_manager_hooks": path}) + self.assertEqual(args[args.index("-k") + 1:], [path]) + self.assertEqual(args[:6], ["systemd-run", "--wait", "--pipe", + "--collect", "--quiet", "--"]) + self.assertIn("apply", args) + + +if __name__ == "__main__": + unittest.main()