From bfdd1cec7b522e6a3ac55c2098bd6c80ecc91d3b Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Mon, 31 Aug 2026 23:39:39 +0500 Subject: [PATCH] fix(placement): a drained member is where maintenance goes, not where it is refused A build requires its member to be empty, and the only way to empty one is drain. Drain closes that member's pressure gate. The gate then removed it from every candidate list, so the build reported "no fleet member has room" about a member with zero occupants. Observed live: gha-runner-2 drained to zero, and reconcile-image with --apply --stage-only refused with insufficient-memory naming an empty machine. The orchestrator does pass --target, but the placement scriptlet runs anyway and vetoes what the target selected. Two correct guards, jointly unsatisfiable. The gate exists to keep new work off a busy member; an image builder is not new work, it is the maintenance the member was emptied for. The scriptlet now exempts exactly the two instance names imageplan generates -- gha-image-builder- and gha-image-smoke- -- from the gate, and from nothing else. Memory headroom, the storage pool check and the disk reserve still apply, because a build placed where it does not fit fails later and more expensively. The test asserts both halves: the exemption is present, and the capacity checks it must not weaken are still there. Verified by mutation -- removing the maintenance condition fails it. --- internal/incusplacement/scriptlet.go | 15 ++++++++- internal/incusplacement/scriptlet_test.go | 38 +++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/internal/incusplacement/scriptlet.go b/internal/incusplacement/scriptlet.go index 9407fb10..1d55f509 100644 --- a/internal/incusplacement/scriptlet.go +++ b/internal/incusplacement/scriptlet.go @@ -97,6 +97,8 @@ RESERVATION_BY_LIMIT_MIB = {{RESERVATION_BY_LIMIT}} CPU_ALLOWANCE_BY_LIMIT_MIB = {{CPU_BY_LIMIT}} PRESSURE_SCHEMA = {{PRESSURE_SCHEMA}} PRESSURE_OPEN = {{PRESSURE_OPEN}} +# Exactly the two names internal/imageplan gives a build's instances. +MAINTENANCE_PREFIXES = ("gha-image-builder-", "gha-image-smoke-") LOAD_TIE_EPSILON = 0.05 def memory_reserve_bytes(total): @@ -167,11 +169,22 @@ def instance_placement(request, candidate_members): chosen_projected_cpu = -1.0 chosen_score = -1.0 + # An image builder is maintenance, not a job, and it is placed deliberately + # on a member that was drained for it. The pressure gate exists to keep new + # work off a busy member; applying it here made drain and build jointly + # unsatisfiable -- drain empties the member by closing its gate, the closed + # gate removes it from every candidate list, and the build then reports + # "no fleet member has room" about a member that is completely empty. + # + # Observed: gha-runner-2 drained to zero occupants, then a staged image + # build refused with insufficient-memory naming an empty member. + maintenance = request.name.startswith(MAINTENANCE_PREFIXES) + for member in candidate_members: name = member.server_name if member.config.get("user.gha_pressure.schema", "") != PRESSURE_SCHEMA: continue - if member.config.get("user.gha_pressure.state", "") != PRESSURE_OPEN: + if not maintenance and member.config.get("user.gha_pressure.state", "") != PRESSURE_OPEN: continue state = get_cluster_member_state(name) diff --git a/internal/incusplacement/scriptlet_test.go b/internal/incusplacement/scriptlet_test.go index 169f3d2f..e5e86316 100644 --- a/internal/incusplacement/scriptlet_test.go +++ b/internal/incusplacement/scriptlet_test.go @@ -64,3 +64,41 @@ func TestRenderRejectsAmbiguousCPUAllowanceForOneMemoryClass(t *testing.T) { t.Fatalf("ambiguous memory-to-CPU class was accepted: %v", err) } } + +// TestMaintenanceInstancesIgnoreTheClosedGate pins the pair that was jointly +// unsatisfiable. A build requires its member to be empty, and the only way to +// empty one is drain -- which closes the member's pressure gate. The gate then +// removed that member from every candidate list, so the build reported +// "no fleet member has room" about a member with zero occupants. +// +// Observed live: gha-runner-2 drained to zero, staged build refused with +// insufficient-memory. +func TestMaintenanceInstancesIgnoreTheClosedGate(t *testing.T) { + cfg, err := config.Load("../../config/example-runner-1.yaml") + if err != nil { + t.Fatal(err) + } + script, err := Render(cfg) + if err != nil { + t.Fatal(err) + } + for _, required := range []string{ + `MAINTENANCE_PREFIXES = ("gha-image-builder-", "gha-image-smoke-")`, + "maintenance = request.name.startswith(MAINTENANCE_PREFIXES)", + `if not maintenance and member.config.get("user.gha_pressure.state", "") != PRESSURE_OPEN:`, + } { + if !strings.Contains(script, required) { + t.Fatalf("rendered scriptlet does not exempt maintenance placement: missing %q", required) + } + } + // The exemption is for the gate only. Memory, disk and the pool check still + // apply, or a build would be placed where it cannot fit. + for _, stillEnforced := range []string{ + "if remaining < 0:", + "if pool.space.total - pool.space.used < want.root_disk_size:", + } { + if !strings.Contains(script, stillEnforced) { + t.Fatalf("maintenance exemption removed a real capacity check: missing %q", stillEnforced) + } + } +}