test(state): trim pid-namespace lease tests to the two invariants
Keep one behavioural test per probe site: a sibling-namespace session turn lease is not stolen (hermes_state lease/lock gate), and a foreign-namespace flock holder record is not broken while same-ns and unstamped dead records still are (hermes_state_common). The local namespace is pinned via _LOCAL_PID_NS so both run on every host rather than only on Linux. The matrix/parse/helper tests re-asserted the same rule through private seams.
This commit is contained in:
@@ -1,34 +1,9 @@
|
||||
"""PID-namespace qualification for state.db lock/lease holder probes.
|
||||
|
||||
A ``pid=<n>`` holder is namespace-relative: siblings sharing one ``state.db``
|
||||
from different PID namespaces (the standard gateway + webui container layout
|
||||
on one volume, systemd ``PrivatePIDs=``) see disjoint PID sets, so a live
|
||||
sibling's holder reads as absent to a namespace-blind ``psutil.pid_exists()``
|
||||
probe — and the unexpired row is reclaimed under the sibling's feet. For a
|
||||
session turn lease that ends the sibling's in-flight turn ("another Hermes
|
||||
process took over this session"); for a compression lock it splits the
|
||||
compression lineage. For the flock holder records (``hermes_state_common``) it
|
||||
breaks a live holder's lock.
|
||||
|
||||
``hermes_state_pidns`` stamps every structured holder with the writer's PID
|
||||
namespace (``pidns=``, the ``/proc/self/ns/pid`` inode) and qualifies each
|
||||
probe on it. Two policies, one rule:
|
||||
|
||||
* :func:`holder_pid_checkable` — STRICT, the policy for TTL-bounded rows
|
||||
(compression locks, session turn leases). A same-namespace reading is still
|
||||
kernel proof and crash cleanup is unchanged; a foreign-namespace OR unstamped
|
||||
holder is never probed — it defers to the row's own expiry (at most the
|
||||
remaining TTL; a false defer self-heals, a false reclaim loses a reply).
|
||||
Strictness is also what stops a steal during a mixed-version rollout as soon
|
||||
as the reader restarts, before every writer has.
|
||||
|
||||
* :func:`persistent_record_pidns_checkable` — LEGACY ROLLOUT, the policy for
|
||||
records with no expiry (the flock holder records). A foreign-namespace
|
||||
record is never probed; an unstamped one keeps main's behavior (refusing to
|
||||
probe it would permanently disable orphaned-lock cleanup).
|
||||
|
||||
These tests pin the qualification matrix and the behavioral halves, plus the
|
||||
sibling-shape regression that motivated the stamp.
|
||||
A ``pid=<n>`` holder is namespace-relative: a sibling sharing one ``state.db``
|
||||
from another PID namespace (gateway + webui containers on one volume) reads as
|
||||
dead to a namespace-blind probe, so its live lease/lock must not be reclaimed.
|
||||
The local namespace is pinned so the invariants hold on every host.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -41,268 +16,50 @@ import time
|
||||
import psutil
|
||||
import pytest
|
||||
|
||||
import hermes_state
|
||||
import hermes_state_common
|
||||
import hermes_state_pidns
|
||||
from hermes_state import SessionDB
|
||||
from hermes_state_pidns import (
|
||||
LocalPidNamespace,
|
||||
_parse_pid_namespace_link,
|
||||
holder_namespace_token,
|
||||
holder_pid_checkable,
|
||||
persistent_record_pidns_checkable,
|
||||
pid_namespace_id,
|
||||
recorded_namespace,
|
||||
)
|
||||
from hermes_state_pidns import LocalPidNamespace
|
||||
|
||||
_OURS, _SIBLING = "111", "222"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _pinned_namespace(monkeypatch):
|
||||
monkeypatch.setattr(hermes_state_pidns, "_LOCAL_PID_NS", LocalPidNamespace(_OURS, True))
|
||||
|
||||
|
||||
def _dead_pid() -> int:
|
||||
"""A PID that is provably gone: spawn, reap, verify absence."""
|
||||
proc = subprocess.Popen([sys.executable, "-c", "pass"])
|
||||
proc.wait(timeout=30)
|
||||
pid = proc.pid
|
||||
assert not psutil.pid_exists(pid), "reaped pid unexpectedly reused"
|
||||
return pid
|
||||
assert not psutil.pid_exists(proc.pid), "reaped pid unexpectedly reused"
|
||||
return proc.pid
|
||||
|
||||
|
||||
def _sibling_pidns() -> str:
|
||||
"""A namespace id that is not ours (linux-gated callers only)."""
|
||||
return str(int(pid_namespace_id()) + 12345)
|
||||
|
||||
|
||||
def _ttl_holder(pid, pidns=None, *, suffix="turn=t:platform=test") -> str:
|
||||
"""A structured TTL-row holder string in the production shape."""
|
||||
stamp = f":pidns={pidns}" if pidns is not None else ""
|
||||
return f"pid={pid}{stamp}:{suffix}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Identity resolution (pure parts)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_parse_pid_namespace_link() -> None:
|
||||
assert _parse_pid_namespace_link("pid:[4026532534]") == "4026532534"
|
||||
assert _parse_pid_namespace_link("pid:[123]") == "123"
|
||||
assert _parse_pid_namespace_link("") is None
|
||||
assert _parse_pid_namespace_link("pid:no-brackets") is None
|
||||
|
||||
|
||||
def test_recorded_namespace_extracts_only_a_segment_bounded_token() -> None:
|
||||
assert recorded_namespace("pid=1:pidns=42:turn=x") == "42"
|
||||
assert recorded_namespace("pidns=42") == "42"
|
||||
assert recorded_namespace("pid=1:tid=2:agent=ab:nonce=cd") is None
|
||||
assert recorded_namespace("") is None
|
||||
assert recorded_namespace(None) is None
|
||||
# The token must start at a segment boundary — not some other key's suffix.
|
||||
assert recorded_namespace("apidns=7") is None
|
||||
assert recorded_namespace("x-pidns=7") is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("local", "recorded", "strict", "legacy"),
|
||||
[
|
||||
# Platform has no namespace concept (macOS/Windows): a pid is still
|
||||
# evidence and both predicates keep main's semantics.
|
||||
(LocalPidNamespace(None, False), None, True, True),
|
||||
(LocalPidNamespace(None, False), "123", True, True),
|
||||
# The diverging row: unstamped on Linux — strict defers (TTL-bounded
|
||||
# rows must be protected during mixed-version rollouts), legacy probes
|
||||
# (no-expiry records must not become permanently unverifiable).
|
||||
(LocalPidNamespace("111", True), None, False, True),
|
||||
# Linux: our own namespace verifies.
|
||||
(LocalPidNamespace("111", True), "111", True, True),
|
||||
# Linux: a sibling's namespace is never probed, under either policy.
|
||||
(LocalPidNamespace("111", True), "222", False, False),
|
||||
# Linux with a failed lookup: no authority — nothing is verifiable.
|
||||
(LocalPidNamespace(None, True), None, False, False),
|
||||
(LocalPidNamespace(None, True), "111", False, False),
|
||||
],
|
||||
)
|
||||
def test_namespace_qualification_matrix(monkeypatch, local, recorded, strict, legacy) -> None:
|
||||
monkeypatch.setattr(hermes_state_pidns, "_LOCAL_PID_NS", local)
|
||||
assert holder_pid_checkable(_ttl_holder(123, recorded)) is strict
|
||||
assert persistent_record_pidns_checkable(recorded) is legacy
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_holder_namespace_token_round_trips_on_this_host() -> None:
|
||||
token = holder_namespace_token()
|
||||
assert token.startswith(":pidns=") and token[len(":pidns="):].isdigit()
|
||||
holder = f"pid=123{token}:tid=1:agent=abc:nonce=deadbeef"
|
||||
assert recorded_namespace(holder) == pid_namespace_id()
|
||||
assert holder_pid_checkable(holder) is True
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_failed_namespace_lookup_is_not_cached_and_defers(monkeypatch) -> None:
|
||||
"""A transient /proc failure must not be cached, and while it lasts every
|
||||
structured holder is unverifiable (defer, never probe)."""
|
||||
calls: list[str] = []
|
||||
real_readlink = os.readlink
|
||||
|
||||
def readlink(path):
|
||||
if path == "/proc/self/ns/pid":
|
||||
calls.append(path)
|
||||
raise OSError("simulated readlink failure")
|
||||
return real_readlink(path)
|
||||
|
||||
monkeypatch.setattr(hermes_state_pidns, "_LOCAL_PID_NS", None)
|
||||
monkeypatch.setattr(hermes_state_pidns.os, "readlink", readlink)
|
||||
|
||||
assert hermes_state_pidns.pid_namespace_id() is None
|
||||
assert hermes_state_pidns.pid_namespace_id() is None
|
||||
assert len(calls) == 2, "a failed lookup must be retried, not cached"
|
||||
assert holder_pid_checkable(_ttl_holder(1, "111")) is False
|
||||
assert holder_pid_checkable(_ttl_holder(1)) is False
|
||||
assert persistent_record_pidns_checkable("111") is False
|
||||
assert persistent_record_pidns_checkable(None) is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Probe gate (the choke-point both lease kinds share)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_probe_gate_matrix() -> None:
|
||||
dead = _dead_pid()
|
||||
probe = hermes_state._compression_lock_holder_process_is_dead
|
||||
|
||||
# Same namespace + provably gone: still reclaimable on kernel proof.
|
||||
assert probe(_ttl_holder(dead, pid_namespace_id())) is True
|
||||
# Foreign namespace: our absence reading says nothing — defer (the bug).
|
||||
assert probe(_ttl_holder(dead, _sibling_pidns())) is False
|
||||
# Unstamped (pre-upgrade writer): TTL rows defer — see the module docstring.
|
||||
assert probe(_ttl_holder(dead)) is False
|
||||
# Unstructured and own-pid holders: never probed (existing contract).
|
||||
assert probe("legacy_holder") is False
|
||||
assert probe(_ttl_holder(os.getpid(), _sibling_pidns())) is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioral: compression locks
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_sibling_namespace_holder_survives_dead_local_pid(tmp_path) -> None:
|
||||
"""A sibling container's holder is invisible to our PID probe — the lock
|
||||
must be kept (TTL still bounds it) instead of stolen."""
|
||||
db = SessionDB(tmp_path / "state.db")
|
||||
dead = _dead_pid()
|
||||
sibling = f"pid={dead}:pidns={_sibling_pidns()}:tid=1:agent=sibling:nonce=aaaaaaaa"
|
||||
assert db.try_acquire_compression_lock("sess1", sibling, ttl_seconds=300) is True
|
||||
|
||||
contender = f"pid={os.getpid()}:pidns={pid_namespace_id()}:tid=2:agent=us:nonce=bbbbbbbb"
|
||||
assert db.try_acquire_compression_lock("sess1", contender, ttl_seconds=300) is False
|
||||
assert db.get_compression_lock_holder("sess1") == sibling
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_unstamped_legacy_holder_defers_to_ttl(tmp_path) -> None:
|
||||
"""TTL rows written before the stamp cannot be told apart from a live
|
||||
sibling's — defer, never steal; the row's own expiry is the release path."""
|
||||
db = SessionDB(tmp_path / "state.db")
|
||||
dead = _dead_pid()
|
||||
legacy = f"pid={dead}:tid=1:agent=old:nonce=cccccccc"
|
||||
assert db.try_acquire_compression_lock("sess1", legacy, ttl_seconds=0.2) is True
|
||||
|
||||
contender = f"pid={os.getpid()}:pidns={pid_namespace_id()}:tid=2:agent=us:nonce=dddddddd"
|
||||
assert db.try_acquire_compression_lock("sess1", contender, ttl_seconds=300) is False
|
||||
assert db.get_compression_lock_holder("sess1") == legacy
|
||||
|
||||
time.sleep(0.35)
|
||||
assert db.try_acquire_compression_lock("sess1", contender, ttl_seconds=300) is True
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_same_namespace_dead_holder_is_still_reclaimed(tmp_path) -> None:
|
||||
"""The conservative rule must not regress crash cleanup: a holder that
|
||||
died in OUR namespace is still reclaimed ahead of TTL."""
|
||||
db = SessionDB(tmp_path / "state.db")
|
||||
dead = _dead_pid()
|
||||
holder = f"pid={dead}:pidns={pid_namespace_id()}:tid=1:agent=crash:nonce=eeeeeeee"
|
||||
assert db.try_acquire_compression_lock("sess1", holder, ttl_seconds=300) is True
|
||||
|
||||
contender = f"pid={os.getpid()}:pidns={pid_namespace_id()}:tid=2:agent=us:nonce=ffffffff"
|
||||
assert db.try_acquire_compression_lock("sess1", contender, ttl_seconds=300) is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioral: session turn leases (the reported incident shape)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_turn_lease_of_sibling_namespace_is_not_stolen(tmp_path) -> None:
|
||||
"""The incident: the sibling's turn is still in flight; a dead local
|
||||
reading must not end it early. The lease releases at TTL."""
|
||||
"""The sibling's turn is still in flight; a dead local reading must not
|
||||
end it early. The lease releases at TTL."""
|
||||
db = SessionDB(tmp_path / "state.db")
|
||||
db.create_session("shared", source="test")
|
||||
dead = _dead_pid()
|
||||
|
||||
sibling = f"pid={dead}:pidns={_sibling_pidns()}:turn=webui-turn:platform=webui"
|
||||
sibling = f"pid={_dead_pid()}:pidns={_SIBLING}:turn=webui-turn:platform=webui"
|
||||
assert db.try_acquire_session_turn_lease("shared", sibling, ttl_seconds=0.3) is True
|
||||
|
||||
contender = f"pid={os.getpid()}:pidns={pid_namespace_id()}:turn=gw-turn:platform=cli"
|
||||
contender = f"pid={os.getpid()}:pidns={_OURS}:turn=gw-turn:platform=cli"
|
||||
assert db.try_acquire_session_turn_lease("shared", contender, ttl_seconds=300) is False
|
||||
|
||||
time.sleep(0.4)
|
||||
assert db.try_acquire_session_turn_lease("shared", contender, ttl_seconds=300) is True
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_turn_lease_same_namespace_dead_holder_is_reclaimed(tmp_path) -> None:
|
||||
"""A turn owner that crashed in OUR namespace is reclaimed ahead of TTL —
|
||||
the e2e crash-resume path (test_compaction_kill9) depends on this."""
|
||||
db = SessionDB(tmp_path / "state.db")
|
||||
db.create_session("shared", source="test")
|
||||
dead = _dead_pid()
|
||||
|
||||
crashed = f"pid={dead}:pidns={pid_namespace_id()}:turn=crashed:platform=cli"
|
||||
assert db.try_acquire_session_turn_lease("shared", crashed, ttl_seconds=300) is True
|
||||
|
||||
contender = f"pid={os.getpid()}:pidns={pid_namespace_id()}:turn=resumer:platform=cli"
|
||||
assert db.try_acquire_session_turn_lease("shared", contender, ttl_seconds=300) is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioral: flock holder records (hermes_state_common)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.platforms("linux")
|
||||
def test_flock_holder_record_qualifies_pid_namespaces() -> None:
|
||||
dead = _dead_pid()
|
||||
provably_dead = hermes_state_common._lock_holder_provably_dead
|
||||
|
||||
# Same namespace + provably gone: break the orphaned lock (unchanged).
|
||||
assert provably_dead(
|
||||
{"pid": dead, "pidns": pid_namespace_id(), "start_ticks": 1, "acquired_at": 0.0}
|
||||
) is True
|
||||
assert provably_dead({"pid": dead, "pidns": _OURS, "start_ticks": 1, "acquired_at": 0.0}) is True
|
||||
# Foreign namespace: a local absence is not proof — defer.
|
||||
assert provably_dead(
|
||||
{"pid": dead, "pidns": _sibling_pidns(), "start_ticks": 1, "acquired_at": 0.0}
|
||||
) is False
|
||||
# Unstamped (pre-upgrade record): LEGACY rollout — these records never
|
||||
# expire, so refusing to probe would permanently disable cleanup.
|
||||
assert provably_dead({"pid": dead, "pidns": _SIBLING, "start_ticks": 1, "acquired_at": 0.0}) is False
|
||||
# Unstamped (pre-upgrade) record: these never expire, so keep probing them.
|
||||
assert provably_dead({"pid": dead, "start_ticks": 1, "acquired_at": 0.0}) is True
|
||||
# No/malformed record: defer (existing contract).
|
||||
assert provably_dead(None) is False
|
||||
|
||||
# A live same-namespace holder stays unbroken; a recycled reading (its
|
||||
# start ticks disagree) still breaks — both proofs survive the stamp.
|
||||
child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"])
|
||||
try:
|
||||
ticks = hermes_state_common._proc_start_ticks(child.pid)
|
||||
assert ticks is not None
|
||||
assert provably_dead(
|
||||
{"pid": child.pid, "pidns": pid_namespace_id(), "start_ticks": ticks, "acquired_at": 0.0}
|
||||
) is False
|
||||
assert provably_dead(
|
||||
{"pid": child.pid, "pidns": pid_namespace_id(), "start_ticks": ticks + 1, "acquired_at": 0.0}
|
||||
) is True
|
||||
finally:
|
||||
child.kill()
|
||||
child.wait(timeout=30)
|
||||
|
||||
Reference in New Issue
Block a user