fix(jobs): harden resume locks and durability
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import multiprocessing
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -46,6 +47,36 @@ def test_lock_keys_cannot_escape_lock_directory(tmp_path):
|
||||
WorkspaceJobLock(tmp_path, "demo", "../evidence")
|
||||
|
||||
|
||||
def test_preexisting_lock_symlink_is_rejected(tmp_path):
|
||||
lock = WorkspaceJobLock(tmp_path, "demo", "evidence")
|
||||
lock.path.parent.mkdir(parents=True)
|
||||
target = tmp_path / "target"
|
||||
target.write_text("do not modify")
|
||||
lock.path.symlink_to(target)
|
||||
with pytest.raises(OSError):
|
||||
lock.acquire()
|
||||
assert target.read_text() == "do not modify"
|
||||
|
||||
|
||||
def test_preexisting_locks_directory_symlink_is_rejected(tmp_path):
|
||||
jobs = tmp_path / ".tht-jobs"
|
||||
jobs.mkdir()
|
||||
outside = tmp_path / "outside"
|
||||
outside.mkdir()
|
||||
(jobs / ".locks").symlink_to(outside, target_is_directory=True)
|
||||
with pytest.raises(OSError):
|
||||
WorkspaceJobLock(tmp_path, "demo", "evidence").acquire()
|
||||
assert list(outside.iterdir()) == []
|
||||
|
||||
|
||||
def test_lock_file_is_owner_only_regular_single_link(tmp_path):
|
||||
with WorkspaceJobLock(tmp_path, "demo", "evidence") as lock:
|
||||
stat = os.stat(lock.path, follow_symlinks=False)
|
||||
assert stat.st_uid == os.getuid()
|
||||
assert stat.st_nlink == 1
|
||||
assert stat.st_mode & 0o777 == 0o600
|
||||
|
||||
|
||||
def test_lock_is_recoverable_after_process_crash_without_stale_deletion(tmp_path):
|
||||
context = multiprocessing.get_context("spawn")
|
||||
ready = context.Event()
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import json
|
||||
import os
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from tht.jobs.models import JobSpec
|
||||
from tht.jobs.runner import CorruptCheckpointError, run_job
|
||||
import tht.jobs.runner as runner_module
|
||||
|
||||
|
||||
def _spec(tmp_path, **updates):
|
||||
@@ -11,6 +13,11 @@ def _spec(tmp_path, **updates):
|
||||
"workspace_id": "demo",
|
||||
"job_type": "evidence",
|
||||
"workspace_root": tmp_path,
|
||||
"spec_version": "jobs-v1",
|
||||
"pipeline_version": "evidence-v1",
|
||||
"config_fingerprint": "sha256:" + "1" * 64,
|
||||
"input_fingerprint": "sha256:" + "2" * 64,
|
||||
"stage_ids": ("stage",),
|
||||
}
|
||||
values.update(updates)
|
||||
return JobSpec(**values)
|
||||
@@ -36,11 +43,12 @@ def test_failed_stage_is_resumable_and_skips_completed_stage(tmp_path):
|
||||
calls.append(("acquire", False))
|
||||
raise RuntimeError("source /customer/alice token=secret unavailable")
|
||||
|
||||
first = run_job(_spec(tmp_path), [discover, acquire])
|
||||
first = run_job(_spec(tmp_path, stage_ids=("discover", "acquire")), [discover, acquire])
|
||||
assert first.status == "failed"
|
||||
assert [stage.status for stage in first.stages] == ["succeeded", "failed"]
|
||||
assert first.stages[1].error.model_dump() == {
|
||||
"category": "RuntimeError",
|
||||
"category": "internal",
|
||||
"code": "stage_exception",
|
||||
"message": "stage execution failed",
|
||||
}
|
||||
|
||||
@@ -48,7 +56,7 @@ def test_failed_stage_is_resumable_and_skips_completed_stage(tmp_path):
|
||||
calls.append(("recovered", False))
|
||||
|
||||
second = run_job(
|
||||
_spec(tmp_path, resume_run_id=first.run_id),
|
||||
_spec(tmp_path, resume_run_id=first.run_id, stage_ids=("discover", "acquire")),
|
||||
[discover, acquire],
|
||||
)
|
||||
assert second.resumed_from == first.run_id
|
||||
@@ -125,3 +133,103 @@ def test_stage_timestamps_are_aware_and_ordered(tmp_path):
|
||||
assert stage.finished_at.tzinfo is not None
|
||||
assert stage.started_at <= stage.finished_at
|
||||
assert report.started_at <= stage.started_at <= report.finished_at
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("update", "replacement"),
|
||||
[
|
||||
("dry_run", True),
|
||||
("spec_version", "jobs-v2"),
|
||||
("pipeline_version", "evidence-v2"),
|
||||
("config_fingerprint", "sha256:" + "3" * 64),
|
||||
("input_fingerprint", "sha256:" + "4" * 64),
|
||||
("workspace_id", "other"),
|
||||
("job_type", "dwh"),
|
||||
],
|
||||
)
|
||||
def test_resume_rejects_changed_identity_or_inputs_before_stage_execution(
|
||||
tmp_path, update, replacement
|
||||
):
|
||||
first = run_job(_spec(tmp_path), [lambda _context: None])
|
||||
called = False
|
||||
|
||||
def stage(_context):
|
||||
nonlocal called
|
||||
called = True
|
||||
|
||||
values = {update: replacement, "resume_run_id": first.run_id}
|
||||
with pytest.raises(CorruptCheckpointError, match="incompatible"):
|
||||
run_job(_spec(tmp_path, **values), [stage])
|
||||
assert called is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("stages", [[], [lambda _context: None, lambda _context: None]])
|
||||
def test_resume_rejects_removed_or_inserted_stages(tmp_path, stages):
|
||||
def first_stage(_context):
|
||||
pass
|
||||
|
||||
first = run_job(_spec(tmp_path), [first_stage])
|
||||
with pytest.raises(CorruptCheckpointError, match="incompatible"):
|
||||
run_job(_spec(tmp_path, resume_run_id=first.run_id), stages)
|
||||
|
||||
|
||||
def test_resume_rejects_reordered_stages(tmp_path):
|
||||
def one(_context):
|
||||
pass
|
||||
|
||||
def two(_context):
|
||||
pass
|
||||
|
||||
first = run_job(_spec(tmp_path, stage_ids=("one", "two")), [one, two])
|
||||
with pytest.raises(CorruptCheckpointError, match="incompatible"):
|
||||
run_job(
|
||||
_spec(tmp_path, resume_run_id=first.run_id, stage_ids=("two", "one")),
|
||||
[two, one],
|
||||
)
|
||||
|
||||
|
||||
def test_hostile_exception_identity_never_enters_terminal_report(tmp_path):
|
||||
Hostile = type("ApiKey_secret_/customer/alice", (Exception,), {})
|
||||
|
||||
def fail(_context):
|
||||
raise Hostile("password=hunter2")
|
||||
|
||||
report = run_job(_spec(tmp_path), [fail])
|
||||
payload = report.model_dump_json()
|
||||
assert report.status == "failed"
|
||||
assert report.stages[0].error.model_dump() == {
|
||||
"category": "internal",
|
||||
"code": "stage_exception",
|
||||
"message": "stage execution failed",
|
||||
}
|
||||
assert "secret" not in payload
|
||||
assert "alice" not in payload
|
||||
assert "hunter2" not in payload
|
||||
|
||||
|
||||
def test_new_run_without_resume_allows_intentional_spec_change(tmp_path):
|
||||
first = run_job(_spec(tmp_path), [lambda _context: None])
|
||||
second = run_job(_spec(tmp_path, input_fingerprint="sha256:" + "9" * 64), [lambda _context: None])
|
||||
assert second.status == "succeeded"
|
||||
assert second.run_id != first.run_id
|
||||
assert second.resumed_from is None
|
||||
|
||||
|
||||
def test_run_directories_are_private_and_fsynced_before_atomic_replace(tmp_path, monkeypatch):
|
||||
events = []
|
||||
real_replace = os.replace
|
||||
|
||||
monkeypatch.setattr(runner_module.os, "fsync", lambda _fd: events.append("fsync"))
|
||||
|
||||
def tracked_replace(source, destination):
|
||||
events.append("replace")
|
||||
real_replace(source, destination)
|
||||
|
||||
monkeypatch.setattr(runner_module.os, "replace", tracked_replace)
|
||||
report = run_job(_spec(tmp_path), [lambda _context: None])
|
||||
run_dir = tmp_path / ".tht-jobs" / "evidence" / "runs" / report.run_id
|
||||
|
||||
assert run_dir.stat().st_mode & 0o777 == 0o700
|
||||
first_replace = events.index("replace")
|
||||
assert "fsync" in events[:first_replace]
|
||||
assert "fsync" in events[first_replace + 1 :]
|
||||
|
||||
Reference in New Issue
Block a user