From 79c781dcd39c4cdd7ade98fe39854e9aac4f4695 Mon Sep 17 00:00:00 2001 From: Azreen Zaman Date: Tue, 30 Jun 2026 11:09:02 -0400 Subject: [PATCH 1/2] Disable sacct metrics if sacct exisits but accounting is disabled --- azure-slurm-exporter/exporter/sacct.py | 12 +++++++++++ azure-slurm-exporter/exporter/util.py | 28 ++++++++++++++++++++++++- azure-slurm-exporter/test/test_sacct.py | 23 ++++++++++++++------ 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/azure-slurm-exporter/exporter/sacct.py b/azure-slurm-exporter/exporter/sacct.py index 635924a5..a137a0d1 100644 --- a/azure-slurm-exporter/exporter/sacct.py +++ b/azure-slurm-exporter/exporter/sacct.py @@ -37,6 +37,9 @@ class Sacct(BaseCollector): "153:0": "SIGXFSZ - File size limit", } + ACCOUNTING_STORAGE_TYPE_KEY = "AccountingStorageType" + ACCOUNTING_STORAGE_DISABLED_VALUES = {"(null)", "accounting_storage/none"} + def __init__(self, binary_path="/usr/bin/sacct", interval=300, timeout=120, starttime=None): self.binary_path = binary_path self.interval = interval @@ -59,6 +62,15 @@ def initialize(self) -> None: if not util.is_file_binary(self.binary_path): log.error(f"{self.binary_path} is not a file or not executable") raise SacctNotAvailException + + storage_type = util.get_scontrol_config_value( + self.ACCOUNTING_STORAGE_TYPE_KEY, + timeout=min(self.timeout, 10), + ) + if storage_type and storage_type.lower() in self.ACCOUNTING_STORAGE_DISABLED_VALUES: + log.warning("Slurm accounting is disabled") + raise SacctNotAvailException + disable_created_metrics() def start(self) -> None: diff --git a/azure-slurm-exporter/exporter/util.py b/azure-slurm-exporter/exporter/util.py index 3c8fe4a9..9a364d5a 100644 --- a/azure-slurm-exporter/exporter/util.py +++ b/azure-slurm-exporter/exporter/util.py @@ -1,5 +1,6 @@ import os import logging +import subprocess log = logging.getLogger(__name__) @@ -29,4 +30,29 @@ def validate_port(port_env_var: str, default_port: int) -> int: ) port = default_port - return port \ No newline at end of file + return port + +def get_scontrol_config_value(key: str, timeout: int = 10) -> str: + """ + Run "scontrol show config" and return the value for a config key. + """ + try: + proc = subprocess.run( + ["scontrol", "show", "config"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + check=False, + timeout=timeout, + ) + except Exception as e: + log.warning("Failed to read scontrol config for %s: %s", key, e) + return "" + + for line in (proc.stdout or "").splitlines(): + stripped = line.strip() + if stripped.startswith(key) and "=" in stripped: + _, value = stripped.split("=", 1) + return value.strip() + + return "" \ No newline at end of file diff --git a/azure-slurm-exporter/test/test_sacct.py b/azure-slurm-exporter/test/test_sacct.py index 4b8fc821..ba180ff7 100644 --- a/azure-slurm-exporter/test/test_sacct.py +++ b/azure-slurm-exporter/test/test_sacct.py @@ -12,10 +12,12 @@ def sacct(self): """Create a Sacct collector instance for testing.""" return Sacct() + @patch("exporter.util.get_scontrol_config_value") @patch("exporter.util.is_file_binary") - def test_initialize_valid_binary(self, mock_is_binary, sacct): + def test_initialize_valid_binary(self, mock_is_binary, mock_get_config, sacct): """Test successful initialization when binary exists.""" mock_is_binary.return_value = True + mock_get_config.return_value = "accounting_storage/slurmdbd" sacct.initialize() @patch("exporter.util.is_file_binary") @@ -25,6 +27,15 @@ def test_initialize_invalid_binary(self, mock_is_binary, sacct): with pytest.raises(SacctNotAvailException): sacct.initialize() + @patch("exporter.util.get_scontrol_config_value") + @patch("exporter.util.is_file_binary") + def test_initialize_accounting_disabled(self, mock_is_binary, mock_get_config, sacct): + """Test initialization fails when scontrol reports accounting storage is disabled.""" + mock_is_binary.return_value = True + mock_get_config.return_value = "(null)" + with pytest.raises(SacctNotAvailException): + sacct.initialize() + class TestSacctParseOutput: @@ -32,7 +43,7 @@ class TestSacctParseOutput: def sacct(self): """Create a Sacct collector instance for testing.""" sacct = Sacct() - with patch("exporter.util.is_file_binary", return_value=True): + with patch("exporter.util.is_file_binary", return_value=True), patch("exporter.util.get_scontrol_config_value", return_value=""): sacct.initialize() return sacct @@ -114,7 +125,7 @@ class TestSacctExportMetrics: def sacct(self): """Create a Sacct collector instance for testing.""" sacct = Sacct() - with patch("exporter.util.is_file_binary", return_value=True): + with patch("exporter.util.is_file_binary", return_value=True), patch("exporter.util.get_scontrol_config_value", return_value=""): sacct.initialize() return sacct @@ -141,7 +152,7 @@ class TestSacctExitCodeMapping: def sacct(self): """Create a Sacct collector instance for testing.""" sacct = Sacct() - with patch("exporter.util.is_file_binary", return_value=True): + with patch("exporter.util.is_file_binary", return_value=True), patch("exporter.util.get_scontrol_config_value", return_value=""): sacct.initialize() return sacct @@ -185,7 +196,7 @@ class TestSacctMetricValues: def sacct(self): """Create a Sacct collector instance for testing.""" sacct = Sacct() - with patch("exporter.util.is_file_binary", return_value=True): + with patch("exporter.util.is_file_binary", return_value=True), patch("exporter.util.get_scontrol_config_value", return_value=""): sacct.initialize() return sacct @@ -269,7 +280,7 @@ async def _sacct_with_one_successful_query() -> Sacct: """Create a Sacct instance with one successful query already executed, so the counter has a known baseline value of 1 for (node1, batch, completed, '', 0:0).""" sacct = Sacct() - with patch("exporter.util.is_file_binary", return_value=True): + with patch("exporter.util.is_file_binary", return_value=True), patch("exporter.util.get_scontrol_config_value", return_value=""): sacct.initialize() mock_proc = MagicMock() mock_proc.stdout = b"1|job1|node1|1|batch|0:0|0:0|COMPLETED|u1|2024-01-01T10:00:00|2024-01-01T09:00:00|2024-01-01T11:00:00|None\n" From a64f5a3bfddddb2f8c250e6843cfb9e998e2e2af Mon Sep 17 00:00:00 2001 From: Azreen Zaman Date: Wed, 15 Jul 2026 14:02:40 -0400 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- azure-slurm-exporter/exporter/util.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/azure-slurm-exporter/exporter/util.py b/azure-slurm-exporter/exporter/util.py index 9a364d5a..c15e9705 100644 --- a/azure-slurm-exporter/exporter/util.py +++ b/azure-slurm-exporter/exporter/util.py @@ -33,9 +33,7 @@ def validate_port(port_env_var: str, default_port: int) -> int: return port def get_scontrol_config_value(key: str, timeout: int = 10) -> str: - """ - Run "scontrol show config" and return the value for a config key. - """ + """Run "scontrol show config" and return the value for a config key.""" try: proc = subprocess.run( ["scontrol", "show", "config"], @@ -49,10 +47,21 @@ def get_scontrol_config_value(key: str, timeout: int = 10) -> str: log.warning("Failed to read scontrol config for %s: %s", key, e) return "" + if proc.returncode != 0: + log.warning( + "scontrol show config failed (rc=%s) while reading %s: %s", + proc.returncode, + key, + (proc.stderr or "").strip(), + ) + return "" + for line in (proc.stdout or "").splitlines(): stripped = line.strip() - if stripped.startswith(key) and "=" in stripped: - _, value = stripped.split("=", 1) - return value.strip() + if "=" not in stripped: + continue + lhs, rhs = stripped.split("=", 1) + if lhs.strip() == key: + return rhs.strip() return "" \ No newline at end of file