fix(cluster): remove marked resources together with marks

[mkoura]

Problem

Follow-up to #3566, last finding from the independent review. The
scheduler removes "current mark" records when marks become invalid -
before a cluster respin (_init_respin) and when cleaning up a dead
cluster instance (_cleanup_dead_clusters). The mark's resource
records were left behind on those paths. With the "current mark"
records gone, the mark staleness handling could not see the marks
anymore, so the leftover resource records were unreachable by every
deletion path: pools locked by a marked group of tests stayed locked
for the rest of the testrun, and tests that wanted them waited out
the whole grace period and failed.

Fix

New _rm_marks helper removes the marked resource records of the
cluster instance together with its "current mark" and "respin after
mark" records; used by both paths above and by _on_marked_test_stop
(which converts "respin after mark" to "needs respin" beforehand).
Unmarked resource records (owned by running tests, cleaned by
on_test_stop) and other instances are untouched. An empty mark is
rejected - it would match the unmarked records of running tests.

Scheduler behavior changes

Two behavior changes fell out of the review-fix iterations and are
worth calling out:

• A non-initial marked test that initiates a respin is re-evaluated
  as the initial test of its mark. Its own mark records - including
  the group's resource records - are removed by the respin init, so
  the test must resolve its resources anew. Previously it would have
  kept the cluster instance for its group while holding no resource
  records at all, letting e.g. a singleton test lock the instance
  while the marked group is still mid-group. A "needs respin" record
  is created at that point, so the respin stays scheduled even when
  the re-evaluation is delayed by a resource conflict.

• A worker respinning its instance is not blocked by its mark
  appearing on another instance. The re-evaluation happens after
  the global lock was released, so another worker of the group (e.g.
  after the original worker died and xdist re-queued the remaining
  tests) can claim the mark elsewhere in that window. The pinned
  respinner now proceeds as the first test of the mark instead of
  waiting forever. The mark can then exist on two instances - a state
  the framework already tolerates; the cost is a duplicated expensive
  setup.

Verification

• New framework_tests/test_cluster_getter.py - first unit tests that
  exercise ClusterGetter directly (10 tests): _rm_marks semantics
  (all-marks/single-mark scoping, other workers' records, unmarked and
  other-instance records preserved, empty-mark rejection), both
  _init_respin paths including the own-mark re-evaluation contract
  and the no-double-wipe short-circuit, the running-tests guard,
  _on_marked_test_stop conversion ordering, and the respinner
  bypass in _marked_select_instance. The shared db_dir fixture
  moved to framework_tests/conftest.py.
• All 50 framework tests pass, make lint clean.
• Reviewed in a 4-round review-fix loop (code review, test coverage,
  comment accuracy per round) until convergence; the re-evaluation
  path was verified by independent traces of the scheduling loop,
  including cross-worker interleavings.

[Copilot]

Pull request overview

Fixes a status-db cleanup gap in the cluster scheduler where deleting invalid “current mark” records could leave behind marked resource records (locks/uses) that then became unreachable by all cleanup paths, causing pools/instances to remain blocked for the rest of the test run.

Changes:

• Adds a new ClusterGetter._rm_marks helper to remove mark-related status records together with marked resource records, and uses it from respin/dead-cluster cleanup paths.
• Adjusts marked scheduling to allow a respinning worker to proceed even if its mark is observed on another instance, and forces re-evaluation when a marked test initiates a respin that wipes its own mark/resource records.
• Adds new direct unit tests for ClusterGetter and centralizes the shared db_dir fixture in framework_tests/conftest.py.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
framework_tests/test_status_db.py	Removes the local db_dir fixture (moved to shared conftest).
framework_tests/test_cluster_getter.py	Adds focused unit tests covering _rm_marks, respin-init behaviors, and marked-instance selection edge cases.
framework_tests/conftest.py	Introduces shared db_dir fixture to reset status DB connection and point it at a temp dir.
cardano_node_tests/cluster_management/status_db.py	Updates GC docstring to reflect that marked resource records can also be removed during respin/dead-instance handling.
cardano_node_tests/cluster_management/cluster_getter.py	Adds _rm_marks and integrates it into respin/dead-instance cleanup; adjusts marked selection for respinning workers.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.