diff --git a/CHANGELOG.md b/CHANGELOG.md index b0e1dfd..29492c8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/running.md b/docs/running.md index 0220607..d56d965 100644 --- a/docs/running.md +++ b/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: [""]` where `` 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: "; 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. diff --git a/docs/upgrading.md b/docs/upgrading.md index 635aa49..05ced8d 100644 --- a/docs/upgrading.md +++ b/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 diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 1f45bea..1663423 100644 --- a/internal/engine/engine.go +++ b/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 { diff --git a/internal/engine/engine_m2b_test.go b/internal/engine/engine_m2b_test.go index 5108616..a19e7f6 100644 --- a/internal/engine/engine_m2b_test.go +++ b/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"}) diff --git a/internal/engine/persist.go b/internal/engine/persist.go index 44e865d..8842458 100644 --- a/internal/engine/persist.go +++ b/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). diff --git a/internal/store/store.go b/internal/store/store.go index 6100ca6..c5d6c33 100644 --- a/internal/store/store.go +++ b/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 } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index ee46142..3094ad3 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -31,6 +31,7 @@ func sampleKey() engine.KeyRecord { Incident: &engine.IncidentRecord{ Opened: t0, Class: "unhealthy", Rung: 2, CooldownUntil: t0.Add(time.Minute), Spent: true, SpentReason: "ladder", Suspended: true, SuspendedAt: t0.Add(30 * time.Second), Recovering: true, RecoverySince: t0.Add(time.Hour), RecoveryCount: 7, + Parent: "abc123@2026-10-17T12:00:00Z", }, } } @@ -246,6 +247,7 @@ func TestSchemaOneDatabaseIsMigrated(t *testing.T) { s.db.Exec(`DROP TABLE notified`) s.db.Exec(`DROP TABLE events`) s.db.Exec(`DROP INDEX incidents_closed`) + s.db.Exec(`ALTER TABLE incidents DROP COLUMN parent`) // a database of that schema has no such column s.db.Exec("PRAGMA user_version = 1") s.Close() @@ -269,6 +271,7 @@ func TestSchemaTwoDatabaseIsMigratedToEvents(t *testing.T) { s.SaveNotified("x", "T", "k", t0) s.db.Exec(`DROP TABLE events`) s.db.Exec(`DROP INDEX incidents_closed`) + s.db.Exec(`ALTER TABLE incidents DROP COLUMN parent`) // a database of that schema has no such column s.db.Exec("PRAGMA user_version = 2") s.Close() @@ -485,13 +488,14 @@ func TestSchemaThreeDatabaseIsMigratedToTheTypeIndex(t *testing.T) { s.CloseIncident(engine.ClosedIncident{Key: "h/p/web/1", Opened: t0, Closed: t0.Add(time.Minute), Class: "unhealthy", Outcome: "recovered"}) s.db.Exec(`DROP INDEX events_type`) s.db.Exec(`DROP INDEX incidents_closed`) + s.db.Exec(`ALTER TABLE incidents DROP COLUMN parent`) // a database of that schema has no such column s.db.Exec("PRAGMA user_version = 3") s.Close() s = open(t, dir) defer s.Close() - if v, err := userVersion(s); err != nil || v != 4 { - t.Fatalf("user_version %d %v, want 4", v, err) + if v, err := userVersion(s); err != nil || v != schemaVersion { + t.Fatalf("user_version %d %v, want %d", v, err, schemaVersion) } snap, err := s.Load(t0) if err != nil || len(snap.Keys) != 1 { @@ -550,3 +554,36 @@ func TestHistoryLimitKeepsTheNewestClosedIncidents(t *testing.T) { t.Fatalf("limit 0 must mean no limit, got %d", len(all)) } } + +// A schema-4 database (no parent column on incidents) gets the column, keeps its open incident, and the parent can then +// be stored and read. +func TestSchemaFourDatabaseIsMigratedToTheIncidentParent(t *testing.T) { + dir := t.TempDir() + s := open(t, dir) + k := sampleKey() + k.Incident.Parent = "" + s.SaveKey(k) + if _, err := s.db.Exec(`ALTER TABLE incidents DROP COLUMN parent`); err != nil { + t.Fatal(err) + } + s.db.Exec("PRAGMA user_version = 4") + s.Close() + + s = open(t, dir) + defer s.Close() + if v, err := userVersion(s); err != nil || v != schemaVersion { + t.Fatalf("user_version %d %v, want %d", v, err, schemaVersion) + } + snap, err := s.Load(t0) + if err != nil || len(snap.Keys) != 1 || snap.Keys[0].Incident == nil { + t.Fatalf("%v %v", snap.Keys, err) + } + if got := snap.Keys[0].Incident.Parent; got != "" { + t.Fatalf("an incident from before the column has no parent: %q", got) + } + k.Incident.Parent = "gone:A" + s.SaveKey(k) + if snap, err = s.Load(t0); err != nil || snap.Keys[0].Incident.Parent != "gone:A" { + t.Fatalf("%+v %v", snap.Keys, err) + } +}