[B-028] Persist a sidecar incident's parent identity across restarts

CHANGELOG.md

@@ -19,13 +19,17 @@ workflow refuses an undated section.
 
 ### Changed
 
+- **A sidecar's repair remembers its parent across a restart of dokdosta.** The parent identity (container ID and start
+  time) the dependents step planned a repair against is now stored with the incident, so the first parent change after a
+  restart opens a fresh incident at rung 0 instead of being taken for the parent being learned. An incident stored by an
+  older release learns its parent at its first decision, as before.
 - **The host's compose plugin is searched in the Docker CLI's order**: `/usr/local/lib`, `/usr/local/libexec`, `/usr/lib`,
   then `/usr/libexec`. A systemd install with a compose in two of these directories now uses the one `docker compose` uses,
   which may be a different version than before ([docs/upgrading.md](docs/upgrading.md#docker-engine-and-compose),
   [docs/deployment.md](docs/deployment.md)).
-- **The store is schema 4.** Two indexes, `events (type, id)` and `incidents (closed_at)`, so the overview's per-type event
+- **The store is schema 5.** Schema 4 adds two indexes, `events (type, id)` and `incidents (closed_at)`, so the overview's per-type event
   queries and the history list no longer read the whole table at the 100,000-row cap; the incidents list asks the store
-  for only the rows it shows. The first start adds them in place; an older dokdosta refuses the database (back it up
+  for only the rows it shows; schema 5 adds a `parent` column to `incidents`. The first start adds them in place; an older dokdosta refuses the database (back it up
   first, [docs/upgrading.md](docs/upgrading.md#the-store-sqlite-schema)).
 
 ### Fixed

docs/running.md

@@ -75,7 +75,7 @@ A dry run started beside a running dokdosta that shares its control socket (`doc
 
 Mounting `docker.sock` is **root-equivalent on the host**. The image is `gcr.io/distroless/static:nonroot` (uid 65532, no shell), so it cannot open the socket by default and fails with `permission denied while trying to connect to the docker API`. Give it the socket's group: `group_add: ["<gid>"]` where `<gid>` is `stat -c %g /var/run/docker.sock`. Do not run it as root instead: with the recommended `cap_drop: [ALL]`, uid 0 has no `CAP_DAC_OVERRIDE` and can write neither `/data` nor the `/run` tmpfs (both belong to uid 65532), so `dokdosta run` exits with "cannot open the state store".
 
-Run it with a read-only root filesystem. Writable paths: `/run` (a tmpfs for the control socket) and **`/data`, a volume for the state store**. The image creates `/data` owned by uid 65532 and declares it a volume, so a fresh named volume (`data:/data`) is writable by the nonroot user. A bind mount keeps the host directory's owner: `chown 65532:65532` it. `dokdosta run` (not dry-run) refuses to start if it cannot open the store. A store that fails later (a full or read-only `/data`) does not stop healing, which carries on from memory, but what was not written does not survive a restart: `status`, `/api/v1/status` (`store.errors`), `/metrics` (`dokdosta_store_errors_total`) and the overview page say so, with the time and kind of the last failure. `/healthz` stays ok. The store is schema 4: the first start after the upgrade adds two indexes (events by type, closed incidents by close time) in one transaction, which takes no noticeable time; an older dokdosta refuses a schema-4 database, so back up `dokdosta.db*` first if you may go back.
+Run it with a read-only root filesystem. Writable paths: `/run` (a tmpfs for the control socket) and **`/data`, a volume for the state store**. The image creates `/data` owned by uid 65532 and declares it a volume, so a fresh named volume (`data:/data`) is writable by the nonroot user. A bind mount keeps the host directory's owner: `chown 65532:65532` it. `dokdosta run` (not dry-run) refuses to start if it cannot open the store. A store that fails later (a full or read-only `/data`) does not stop healing, which carries on from memory, but what was not written does not survive a restart: `status`, `/api/v1/status` (`store.errors`), `/metrics` (`dokdosta_store_errors_total`) and the overview page say so, with the time and kind of the last failure. `/healthz` stays ok. The store is schema 5: the first start after the upgrade adds two indexes (events by type, closed incidents by close time) and a `parent` column on `incidents` in one transaction each, which takes no noticeable time; an older dokdosta refuses a schema-5 database, so back up `dokdosta.db*` first if you may go back.
 
 Under systemd, run `dokdosta run` as a user in the `docker` group and set `DOKDOSTA_CONTROL_SOCKET` to a path that user can write, and `DOKDOSTA_DATA_DIR` to a directory it owns.
 
@@ -160,7 +160,7 @@ Every variable, with its default, is in [configuration.md](configuration.md) (a
 ## What it does
 
 - **The ladder, one pass per incident.** Rung 1 restarts (`restart` for running containers, `start` for created or exited ones), then the cooldown, then a re-inspect. If the container is still bad, rung 2 recreates it (drift guard first), then the cooldown and a re-inspect again; if it is still bad, the container is **SUSPENDED** (`Suspended`): nothing more is tried. A rung that cannot run (the version gate, no usable compose inputs, a container compose did not make, a service with more than one container) is recorded (`RungUnavailable`) and the ladder goes on. Each rung counts against the rate limit, dependents included. A pre-action check that refuses the same action for the same reason ten times in a row (`ActionRefused`) SUSPENDS the container with "refused 10 times: <reason>"; any action, or a wait for a parent, restarts that count. A failed start whose cause is a network that is gone goes straight to rung 2.
-- **Recovered** means a healthcheck says `healthy`, or, with no healthcheck, the container is running and `RestartCount` has not changed for `RECOVERY_PERIOD`. SUSPENDED clears at that point. The incident itself closes (`Recovered`) only after the container has stayed recovered for `RECOVERY_PERIOD`; if it fails again before that, it is the same incident and the ladder goes on where it was, so a spent ladder means SUSPENDED again. One exception, for a sidecar repaired by the dependents step: when its parent has changed again since that repair (a new container ID, or a new start time), the old incident is closed ("the parent changed again") and a new one opens at rung 0, so a sidecar is repaired after every parent change, also within `RECOVERY_PERIOD`. It does not count toward the flap limit.
+- **Recovered** means a healthcheck says `healthy`, or, with no healthcheck, the container is running and `RestartCount` has not changed for `RECOVERY_PERIOD`. SUSPENDED clears at that point. The incident itself closes (`Recovered`) only after the container has stayed recovered for `RECOVERY_PERIOD`; if it fails again before that, it is the same incident and the ladder goes on where it was, so a spent ladder means SUSPENDED again. One exception, for a sidecar repaired by the dependents step: when its parent has changed again since that repair (a new container ID, or a new start time), the old incident is closed ("the parent changed again") and a new one opens at rung 0, so a sidecar is repaired after every parent change, also within `RECOVERY_PERIOD`. It does not count toward the flap limit. The parent a repair was made against is kept in the store, so this holds across a restart of dokdosta too; an incident stored by an older release learns its parent at the first decision after the restart.
 - **Flapping.** More than `FLAP_LIMIT` incidents within `FLAP_WINDOW` suspend the container the next time an incident opens (nothing is tried), even if every earlier one recovered. Closing an incident does not reset the count; only the window expiring does.
 - **SUSPENDED also clears** on a `create` by someone else, or on a `start` that follows a manual stop (someone stopped it, someone started it). A `start` by the restart policy never clears it. Nor does a `start` inside a restore window, nor one that follows a quarantined shutdown kill (below).
 - **A created container compose is holding back.** `docker compose up` creates every container first and starts each one when its `depends_on` conditions hold. A container that is `created`, has no `State.Error`, and whose `com.docker.compose.depends_on` label names a service of its project with a condition that does not hold yet (`service_healthy` and the service's container is not healthy; `service_completed_successfully` and it has not exited 0) is **held back**: dokdosta does not start it, opens no incident, and looks again at each poll. `status` (under HELD BACK) and the container's page in the dashboard say what it waits for. `service_started` holds nothing. Any other `created` container is a start that never happened: the ladder starts it (rung 1), then recreates it. If the dependency never becomes healthy the container stays held; the dependency's own failure is healed (or reported) as usual.

docs/upgrading.md

@@ -128,10 +128,14 @@ in place, one step per version:
 | 2 | `notified`: the notification dedupe, so a restart does not repeat a message | M3 |
 | 3 | `events`: the persisted event log, the API's `since` cursor and the timelines | M5 |
 | 4 | indexes `events (type, id)` and `incidents (closed_at)`: the overview's event queries and the history list | improvement cycle, B-027 |
-
-v0.1.0 is schema 3, so a v0.1.0 database needs no migration; the steps exist for stores written by development builds. Each step
-is a few `CREATE TABLE` statements and takes no time. Every release that changes the schema says so in the changelog and in this
-table.
+| 5 | column `incidents.parent`: the parent a sidecar's repair was made against, kept across a restart | improvement cycle, B-028 |
+
+Every release up to v0.3.0-rc.1 (v0.1.x and v0.2.0, released as whalenurse) is schema 3. v0.3.0 is schema 5: its first
+start migrates a schema-3 database in two steps, adding the two indexes of schema 4 and the `parent` column of schema 5 (empty for
+the incidents already stored; such an incident learns its parent at its first decision). Steps 1 to 3 exist for stores written by
+development builds. Each step is one transaction of a few `CREATE` or `ALTER TABLE` statements and takes no noticeable time; back
+up `dokdosta.db*` first if you may go back (see *No downgrade* below). Every release that changes the schema says so in the
+changelog and in this table.
 
 - **No downgrade.** A database with a newer schema than the binary knows is refused at start with
   `database schema N is newer than this dokdosta understands (M)`. To go back, [restore the backup](#backing-up-and-restoring-the-store) you took before

internal/engine/engine.go

@@ -184,7 +184,7 @@ func (e *Engine) Restore(s Snapshot) {
 			st.incident = &incident{
 				opened: r.Opened, class: rules.Class(r.Class), rung: r.Rung, cooldownUntil: r.CooldownUntil,
 				spent: r.Spent, spentReason: r.SpentReason, suspended: r.Suspended, suspendedAt: r.SuspendedAt,
-				recovering: r.Recovering, recoverySince: r.RecoverySince, recoveryCount: r.RecoveryCount,
+				recovering: r.Recovering, recoverySince: r.RecoverySince, recoveryCount: r.RecoveryCount, parent: r.Parent,
 			}
 		}
 		e.keys[k.Key] = st
@@ -232,7 +232,7 @@ func (e *Engine) flush(key string, st *keyState) {
 		rec.Incident = &IncidentRecord{
 			Opened: inc.opened, Class: string(inc.class), Rung: inc.rung, CooldownUntil: inc.cooldownUntil,
 			Spent: inc.spent, SpentReason: inc.spentReason, Suspended: inc.suspended, SuspendedAt: inc.suspendedAt,
-			Recovering: inc.recovering, RecoverySince: inc.recoverySince, RecoveryCount: inc.recoveryCount,
+			Recovering: inc.recovering, RecoverySince: inc.recoverySince, RecoveryCount: inc.recoveryCount, Parent: inc.parent,
 		}
 	}
 	e.p.Persist.SaveKey(rec)
@@ -671,8 +671,9 @@ func (e *Engine) settle(t Target, st *keyState, d rules.Decision, cfg config.Con
 // healthy"); the incident closes only after the container has stayed recovered
 // for RECOVERY_PERIOD.
 func (e *Engine) recovering(t Target, st *keyState, inc *incident, o Obs, now time.Time) Plan {
-	if inc.parent == "" {
-		inc.parent = o.Parent // an incident restored after a restart, or opened by the sidecar's own ladder, learns it here
+	if inc.parent == "" && o.Parent != "" {
+		inc.parent = o.Parent // an incident opened by the sidecar's own ladder, or stored by a release before this one, learns it here
+		st.dirty = true
 	}
 	if !inc.recovering || o.RestartCount != inc.recoveryCount {
 		inc.recovering, inc.recoverySince, inc.recoveryCount = true, now, o.RestartCount
@@ -728,6 +729,7 @@ func (e *Engine) parentChanged(t Target, st *keyState, d rules.Decision, o Obs)
 	}
 	if inc.parent == "" {
 		inc.parent = o.Parent
+		st.dirty = true
 		return
 	}
 	if inc.parent == o.Parent {

internal/engine/engine_m2b_test.go

@@ -296,6 +296,39 @@ func TestSameParentKeepsTheOnePassRuleAndUnknownParentChangesNothing(t *testing.
 	}
 }
 
+// The parent a repair was planned against is persisted, so the first parent change after a restart of dokdosta is a
+// change too and not the learning of a parent.
+func TestParentChangeAfterARestartOpensANewIncident(t *testing.T) {
+	ms := newMemStore()
+	e, clk, _ := newEngineWith(t, nil, func(p *Params) { p.Persist = ms })
+	if p := run(e, staleGone, cfg, Obs{Parent: "gone:A"}); p.Act != rules.ActionRecreateStale {
+		t.Fatalf("%+v", p)
+	}
+	flaps := len(ms.keys[tgt.Key].Flaps)
+
+	e2, _, rec2 := newEngineWith(t, clk, func(p *Params) { p.Persist = ms })
+	e2.Restore(ms.snapshot())
+	clk.Advance(5 * time.Second) // inside the cooldown; the parent changed while dokdosta was down
+	if p := run(e2, staleGone, cfg, Obs{Parent: "gone:B"}); p.Act != rules.ActionRecreateStale {
+		t.Fatalf("a new parent is repaired at once, not held by the cooldown of the repair before: %+v / %v", p, rec2.Types())
+	}
+	closed := 0
+	for _, ev := range rec2.Events() {
+		if ev.Type == IncidentClosed && ev.Attrs["reason"] == "parent_changed" {
+			closed++
+		}
+	}
+	if closed != 1 || rec2.Count(IncidentOpened) != 1 || rec2.Count(Suspended) != 0 {
+		t.Fatalf("%v", rec2.Types())
+	}
+	if got := len(ms.keys[tgt.Key].Flaps); got != flaps {
+		t.Fatalf("a dependents-step incident is not counted toward the flap limit: %d flaps, was %d", got, flaps)
+	}
+	if got := ms.keys[tgt.Key].Incident.Parent; got != "gone:B" {
+		t.Fatalf("the new incident persists the new parent: %q", got)
+	}
+}
+
 func TestParentChangeAfterSuspensionUnsuspendsAndRepairsAfresh(t *testing.T) {
 	e, clk, rec := newEngine(t, 0)
 	run(e, staleGone, cfg, Obs{Parent: "gone:A"})

internal/engine/persist.go

@@ -18,7 +18,8 @@ type IncidentRecord struct {
 	SuspendedAt   time.Time
 	Recovering    bool // recovered, waiting out RECOVERY_PERIOD
 	RecoverySince time.Time
-	RecoveryCount int // RestartCount when the recovery period began
+	RecoveryCount int    // RestartCount when the recovery period began
+	Parent        string // the parent (Obs.Parent) a dependents-step repair was planned against; empty when not known
 }
 
 // LoopRecord is the crash-loop tracker's state (rule 3).

internal/store/store.go

@@ -31,7 +31,7 @@ import (
 const FileName = "dokdosta.db"
 
 // schemaVersion is stored in PRAGMA user_version. Bump it with a migration step in migrate.
-const schemaVersion = 4
+const schemaVersion = 5
 
 // Store is an open database.
 type Store struct {
@@ -243,10 +243,20 @@ func (s *Store) migrate() error {
 	if v < 4 {
 		// The overview asks for events of a few types on every refresh and the history page lists closed incidents
 		// newest first; without these both read the whole table at the 100,000-row cap.
-		return s.exec([]string{
+		if err := s.exec([]string{
 			`CREATE INDEX events_type ON events (type, id)`,
 			`CREATE INDEX incidents_closed ON incidents (closed_at)`,
 			"PRAGMA user_version = 4",
+		}); err != nil {
+			return err
+		}
+	}
+	if v < 5 {
+		// The parent a dependents-step repair was planned against, so the first parent change after a restart of
+		// dokdosta is seen as a change. Container IDs and start times only: a re-key leaves it alone.
+		return s.exec([]string{
+			`ALTER TABLE incidents ADD COLUMN parent TEXT NOT NULL DEFAULT ''`,
+			"PRAGMA user_version = 5",
 		})
 	}
 	return nil
@@ -371,14 +381,15 @@ func (s *Store) SaveKey(r engine.KeyRecord) {
 			return err
 		}
 		if i := r.Incident; i != nil {
-			_, err := tx.Exec(`INSERT INTO incidents (key, opened_at, class, rung, cooldown_until, spent, spent_reason, suspended, suspended_at, recovering, recovery_since, recovery_count)
-				VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+			_, err := tx.Exec(`INSERT INTO incidents (key, opened_at, class, rung, cooldown_until, spent, spent_reason, suspended, suspended_at, recovering, recovery_since, recovery_count, parent)
+				VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
 				ON CONFLICT(key, opened_at) DO UPDATE SET class = excluded.class, rung = excluded.rung, cooldown_until = excluded.cooldown_until,
 					spent = excluded.spent, spent_reason = excluded.spent_reason, suspended = excluded.suspended, suspended_at = excluded.suspended_at,
-					recovering = excluded.recovering, recovery_since = excluded.recovery_since, recovery_count = excluded.recovery_count
+					recovering = excluded.recovering, recovery_since = excluded.recovery_since, recovery_count = excluded.recovery_count,
+					parent = excluded.parent
 				WHERE incidents.closed_at IS NULL`,
 				r.Key, tn(i.Opened), i.Class, i.Rung, tn(i.CooldownUntil), b2i(i.Spent), i.SpentReason, b2i(i.Suspended), tn(i.SuspendedAt),
-				b2i(i.Recovering), tn(i.RecoverySince), i.RecoveryCount)
+				b2i(i.Recovering), tn(i.RecoverySince), i.RecoveryCount, i.Parent)
 			return err
 		}
 		return nil
@@ -505,7 +516,7 @@ func (s *Store) Load(now time.Time) (engine.Snapshot, error) {
 	}
 
 	incs := map[string]*engine.IncidentRecord{}
-	rows, err = s.db.Query(`SELECT key, opened_at, class, rung, cooldown_until, spent, spent_reason, suspended, suspended_at, recovering, recovery_since, recovery_count
+	rows, err = s.db.Query(`SELECT key, opened_at, class, rung, cooldown_until, spent, spent_reason, suspended, suspended_at, recovering, recovery_since, recovery_count, parent
 		FROM incidents WHERE closed_at IS NULL`)
 	if err != nil {
 		return snap, err
@@ -515,7 +526,7 @@ func (s *Store) Load(now time.Time) (engine.Snapshot, error) {
 		var opened, cool, susAt, recSince int64
 		var spent, sus, rec int
 		r := &engine.IncidentRecord{}
-		if err := rows.Scan(&k, &opened, &r.Class, &r.Rung, &cool, &spent, &r.SpentReason, &sus, &susAt, &rec, &recSince, &r.RecoveryCount); err != nil {
+		if err := rows.Scan(&k, &opened, &r.Class, &r.Rung, &cool, &spent, &r.SpentReason, &sus, &susAt, &rec, &recSince, &r.RecoveryCount, &r.Parent); err != nil {
 			rows.Close()
 			return snap, err
 		}