carlos / platform Public

Pin the two decline-but-keep invariants, and name the second removal authority

Paul Campbell pushed by paul@keymail.dev 2 months ago 65c0f217e11e690421eead0c88979096f4ac7443 parent 04c886f

2 files changed, +75 −0

internal/provision/sidecar_reconcile.go +21 −0
internal/provision/sidecar_reconcile_test.go +54 −0

diff --git a/internal/provision/sidecar_reconcile.go b/internal/provision/sidecar_reconcile.go

index c458f43d0..5297cddef 100644

--- a/internal/provision/sidecar_reconcile.go

+++ b/internal/provision/sidecar_reconcile.go

@@ -20,6 +20,17 @@ package provision

// removal authority is therefore scoped to its OWN Kind, asserted from both

// sides (TestInstanceReconcileNeverRemovesSidecarRows,

// TestSidecarReconcileRemovesOnlyItsOwnKind).

+//

+// ONE EXCEPTION, and it is deliberate: domains.Reconcile drops EVERY route of

+// a trashed app (internal/domains/reconcile.go, the trashed-apps loop) with no

+// Kind filter at all, sidecar rows included. So removal authority over a

+// sidecar row is held by TWO passes, not one. That is not a leak — it is the

+// only path by which a trashed app's sidecars stop running, it carries its own

+// production-flag guard, and it is precisely what makes THIS pass's

+// decline-but-keep response to a trashed app correct rather than a way for

+// rows to outlive their app. Anyone tightening the rule stated above should

+// know where its exception lives before assuming a sidecar row can only be

+// deleted here.

import (

"context"

@@ -345,6 +356,16 @@ func ReconcileSidecars(ctx context.Context, s release.Store, reg *registry.Regis

// file-staged co-spawn must not ALSO get a native sidecar row, or

// the same workload runs twice under two owners that know nothing

// of each other.

+ //

+ // This is a MINT-TIME check and nothing more. A -sidecar.env

+ // staged AFTER this row exists is not detected, and that host then

+ // carries both mechanisms with no complaint from this pass. It is

+ // left that way deliberately: the reverse — a converge-time

+ // refusal — would mean a file dropped by hand could stop an

+ // already-provisioned sidecar converging, which hands anything

+ // that can write the data dir a way to freeze a member's row. The

+ // direction that matters is the one guarded: the file came first,

+ // so the file keeps the host.

if coSpawnStaged(cfg.DataDir, rec.Host) {

log.Printf("provision: REFUSING to mint sidecar %s: %s%s is staged on this box — "+

"the legacy file co-spawn and a native sidecar row are two mechanisms for one host; "+

diff --git a/internal/provision/sidecar_reconcile_test.go b/internal/provision/sidecar_reconcile_test.go

index 2eeceb26f..cd996e5b6 100644

--- a/internal/provision/sidecar_reconcile_test.go

+++ b/internal/provision/sidecar_reconcile_test.go

@@ -7,6 +7,7 @@ import (

"path/filepath"

"strings"

"testing"

+ "time"

"github.com/carlosframework/platform/internal/registry"

"github.com/carlosframework/platform/internal/release"

@@ -338,6 +339,59 @@ func TestSidecarReconcileProtectsAChannellessRecord(t *testing.T) {

noRoute(t, f.reg, old, "protect is narrow: an unusable record holds back ITS OWN host only, and the pass's removal phase still runs")

}

+// --- decline-but-keep -------------------------------------------------------------

+// TestSidecarReconcileKeepsRowsWhenTheManifestIsAbsent pins the invariant that

+// stands between "an app's sidecars.json was deleted" and "every sidecar row

+// of that app is retired". An absent manifest is a confirmed answer, not a

+// blip, so it does NOT arm the pass-wide fail-safe — which is exactly why the

+// record must stay in the record set on its own account. Declining to mint is

+// the whole of the response.

+//

+// The hazard is one line away: manifestFor is consulted in the record scan

+// (placement needs it), so a natural-looking `if !mKnown { continue }` there

+// would convert a missing manifest into a fleet evacuation.

+func TestSidecarReconcileKeepsRowsWhenTheManifestIsAbsent(t *testing.T) {

+ f := sidecarSetup(t)

+ host := f.host("bot", "eleven")

+ // No sidecars.json for this app at all — the feature is not enabled.

+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})

+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "new", App: "eleven", Channel: "stable"})

+ f.mustUpsert(t, registry.Route{Host: host, Kind: "sidecar", Slug: "bot", App: "eleven",

+ Account: f.sqid, Channel: "stable", Provisioned: true, Args: `["sidecar","run"]`})

+ f.reconcile(t, ReconcileConfig{})

+ rt := hasRoute(t, f.reg, host, "an absent sidecars manifest must not RETIRE a running sidecar — it is the app's opt-in door, not a statement that this row is gone")

+ if rt.Args != `["sidecar","run"]` {

+ t.Fatalf("Args = %q — with no manifest there is nothing authoritative to converge from, so the row must be left exactly as it was", rt.Args)

+ }

+ noRoute(t, f.reg, f.host("new", "eleven"), "with no manifest there is no argv to run, so nothing may be minted either")

+// TestSidecarReconcileKeepsRowsOfATrashedApp: a trashed app mints nothing

+// here, and its rows are not this pass's to retire either. domains.Reconcile

+// drops every route of a trashed app on its own authority (with its own

+// production-flag guard) — this pass merely declines to create, or the two

+// loops would fight forever, one re-raising what the other removes.

+func TestSidecarReconcileKeepsRowsOfATrashedApp(t *testing.T) {

+ f := sidecarSetup(t)

+ host := f.host("bot", "eleven")

+ f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})

+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})

+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "new", App: "eleven", Channel: "stable"})

+ f.mustUpsert(t, registry.Route{Host: host, Kind: "sidecar", Slug: "bot", App: "eleven",

+ Account: f.sqid, Channel: "stable", Provisioned: true})

+ if err := release.TrashApp(ctx, f.store, f.sqid, "eleven", "someone@example.com", time.Now()); err != nil {

+ t.Fatal(err)

+ }

+ f.reconcile(t, ReconcileConfig{})

+ hasRoute(t, f.reg, host, "dropping a trashed app's routes is domains.Reconcile's call, with its production-flag guard — this pass only declines to create")

+ noRoute(t, f.reg, f.host("new", "eleven"), "a trashed app provisions nothing")

// --- placement -------------------------------------------------------------------

// TestSidecarReconcilePlacementGate pins §8's second half: a no-inbound