fix(lease-broker): no lease held is a no-op success, not a denied transition
ci/woodpecker/pr/ci Pipeline failed
ci/woodpecker/pr/ci Pipeline failed
revoke-lease.py read MOSAIC_LEASE_BROKER_SOCKET and MOSAIC_LEASE_SESSION_ID with direct dict access inside the try block. A session that never held a lease raised KeyError, fell into the fail-closed handler, and returned rc=2 on every lifecycle transition -- including compaction. Denying compaction to a session that holds no lease protects nothing; it converts a recoverable context limit into a lost session. That is D29. Absence must be TOTAL to qualify for the no-op. If exactly one variable is present the session is half-provisioned, which is real misconfiguration, and it still takes the fail-closed path. An exported-but-empty variable counts as absent. Five unit tests cover the contract: no-op success, vacuous no-op (the broker must not be contacted), both half-provisioned mirrors still rc=2, and empty string as absent. Red control: 2 of the 5 fail without the guard. The test is enumerated in test:framework-shell. Every other src/lease-broker/*_unittest.py is enumerated there, and the #1017 membership guard's population is *test*.sh under framework/tools/, so it does not see python suites -- an unenumerated python test is silently never run. Still open, not fixed here: why seat panes carry no MOSAIC_LEASE_* variables at all.
This commit is contained in:
@@ -54,6 +54,22 @@ def main(
|
||||
arguments = parser.parse_args(argv)
|
||||
source_environment = os.environ if environ is None else environ
|
||||
|
||||
# D29: a session that never held a lease has nothing to revoke, and that is a
|
||||
# SUCCESS, not a failed revocation. The block below is deliberately fail-closed
|
||||
# for a broker that is unreachable, which is right — but it cannot distinguish
|
||||
# "the broker is down" from "there was never a lease", so a bare-launched
|
||||
# session was denied every lifecycle transition, including compaction. Denying
|
||||
# compaction protects nothing there; it converts a recoverable context limit
|
||||
# into a lost session.
|
||||
#
|
||||
# Absence must be TOTAL to qualify. If exactly one variable is present the
|
||||
# session is half-provisioned, which is real misconfiguration, and it still
|
||||
# takes the fail-closed path below.
|
||||
lease_variables = ("MOSAIC_LEASE_BROKER_SOCKET", "MOSAIC_LEASE_SESSION_ID")
|
||||
present = [name for name in lease_variables if source_environment.get(name)]
|
||||
if not present:
|
||||
return 0
|
||||
|
||||
try:
|
||||
if not arguments.reason or len(arguments.reason) > 128:
|
||||
raise ValueError("invalid revoke reason")
|
||||
|
||||
Reference in New Issue
Block a user