diff --git a/app/burnin/stages.py b/app/burnin/stages.py index 13ca8e7..beca66f 100644 --- a/app/burnin/stages.py +++ b/app/burnin/stages.py @@ -24,6 +24,25 @@ class _BadblocksResult(TypedDict): output: str aborted: bool + +def _build_badblocks_cmd(devname: str) -> str: + """Construct the wrapped badblocks command for a given device. + + Wraps badblocks under `sh -c 'echo PID:$$; exec ...'` so we can + capture the remote PID for out-of-band kill -9 (asyncssh's signal + channel is ignored by sshd). Geometry (-b -c -p) is operator-tunable + via Settings → Burn-in; defaults match the Spearfoot disk-burnin.sh + recommendation for large HDDs. + """ + return ( + f"sh -c 'echo PID:$$; exec badblocks " + f"-wsv " + f"-b {settings.surface_validate_block_size} " + f"-c {settings.surface_validate_block_buffer} " + f"-p {settings.surface_validate_passes} " + f"/dev/{devname}'" + ) + from . import kill from ._common import ( POLL_INTERVAL, @@ -404,20 +423,7 @@ async def _stage_surface_validate_ssh(job_id: int, devname: str, drive_id: int) # we need the PID to issue an out-of-band `kill -9` over a fresh # session when we want to abort. # - # Block geometry is operator-tunable (Settings → Burn-in): - # -b N block size in bytes (settings.surface_validate_block_size) - # -c N blocks held per IO (settings.surface_validate_block_buffer) - # -p N pass count (settings.surface_validate_passes) - # Defaults preserve original behavior (-b 4096 -c 64 -p 1). - bb_args = ( - f"-wsv " - f"-b {settings.surface_validate_block_size} " - f"-c {settings.surface_validate_block_buffer} " - f"-p {settings.surface_validate_passes}" - ) - cmd = ( - f"sh -c 'echo PID:$$; exec badblocks {bb_args} /dev/{devname}'" - ) + cmd = _build_badblocks_cmd(devname) async with conn.create_process(cmd) as proc: import re as _re diff --git a/tests/test_badblocks_cmd.py b/tests/test_badblocks_cmd.py new file mode 100644 index 0000000..993a578 --- /dev/null +++ b/tests/test_badblocks_cmd.py @@ -0,0 +1,77 @@ +"""Verifies the Spearfoot tunables (block_size, block_buffer, passes) +actually thread through to the badblocks command line. + +These three settings are exposed in Settings → Burn-in. Without a test, +nothing catches if a future refactor drops one of the flags or reads +from the wrong attribute. The defaults match the Spearfoot disk-burnin.sh +community script; non-defaults can roughly halve runtime on multi-TB +drives at the cost of more RAM. + +Run inside the container image so app deps are present. +""" + +from __future__ import annotations + +import unittest + +from app.burnin.stages import _build_badblocks_cmd +from app.config import settings + + +class TestBadblocksCmd(unittest.TestCase): + + def setUp(self): + # Snapshot defaults so each test can mutate freely without + # polluting siblings or the running process. + self._snap = ( + settings.surface_validate_block_size, + settings.surface_validate_block_buffer, + settings.surface_validate_passes, + ) + + def tearDown(self): + ( + settings.surface_validate_block_size, + settings.surface_validate_block_buffer, + settings.surface_validate_passes, + ) = self._snap + + def test_defaults_match_spearfoot(self): + """Out of the box: -b 4096 -c 64 -p 1 — matches the + disk-burnin.sh community script's recommendation for HDDs.""" + cmd = _build_badblocks_cmd("sda") + self.assertIn("-b 4096", cmd) + self.assertIn("-c 64", cmd) + self.assertIn("-p 1", cmd) + self.assertIn("/dev/sda", cmd) + # Destructive write+verify mode must always be present — anything + # else (read-only, non-destructive) defeats the purpose of burn-in. + self.assertIn("-wsv", cmd) + + def test_tunables_propagate_to_cmd(self): + """Operator-set values (e.g. for paranoid 3-pass burn-in on a + suspect drive, or 8 KiB blocks for faster scan on a 24 TB HDD) + must end up in the shell command.""" + settings.surface_validate_block_size = 8192 + settings.surface_validate_block_buffer = 128 + settings.surface_validate_passes = 3 + cmd = _build_badblocks_cmd("sdb") + self.assertIn("-b 8192", cmd) + self.assertIn("-c 128", cmd) + self.assertIn("-p 3", cmd) + self.assertNotIn("-b 4096", cmd) # no leak from defaults + self.assertNotIn("-c 64", cmd) + self.assertIn("/dev/sdb", cmd) + + def test_pid_capture_wrapper_intact(self): + """The `sh -c 'echo PID:$$; exec ...'` wrapper is what makes + out-of-band kill -9 work over a fresh SSH session — asyncssh's + signal channel is silently ignored by sshd. If a future refactor + drops the wrapper, a cancel won't actually stop the test.""" + cmd = _build_badblocks_cmd("sda") + self.assertTrue(cmd.startswith("sh -c 'echo PID:$$; exec badblocks")) + self.assertTrue(cmd.endswith("'")) + + +if __name__ == "__main__": + unittest.main()