105 lines
10 KiB
Markdown
105 lines
10 KiB
Markdown
# Master/Agent Phase 5 — Drift and Merge Implementation Plan
|
|
|
|
> This plan documents what was built for issue #10 (backend only) and where it deviates from the spec. Code in "Frozen contracts" is final; tasks give the contract and the test cases.
|
|
|
|
**Goal:** The master knows when a service runs something other than the version the last job deployed (drift), never corrects it by itself, and offers two ways out: restore the desired version or accept the running one. When a Compose service disappears from one agent and appears on another, the master suggests that it moved, and a merge moves the old row (history, mode, desired ref) to the new location. Merging can also be done by hand.
|
|
|
|
**Architecture:**
|
|
- Model: `Service.Drifted()` plus `RunsRef` and `RunningRef` decide drift and what accept writes. `Move` carries a suggestion.
|
|
- Store: migration 0005 adds `services.awaiting_inventory`; `FinishJob` sets it, every inventory upsert clears it. New `AcceptRunning`, `MoveCandidates`, `MergeSources`, `MergeService` (the only code that deletes a `services` row).
|
|
- Jobs: `Start` refuses poll and webhook jobs for a drifted service. `Restorable` and `RestoreDesired` deploy the recorded ref again as a manual job.
|
|
- Web: three POST routes (`/services/{id}/restore`, `/accept`, `/merge`) and new `pageData` fields. No template or CSS changes: a parallel session rewrites the UI (issue #18), which adds the buttons.
|
|
|
|
**Tech Stack:** no new dependencies.
|
|
|
|
**Spec:** `docs/superpowers/specs/2026-10-12-master-agent-design.md`, sections "Drift" and "Service identity and moves". Phase 4 plan conventions still apply.
|
|
|
|
## Global Constraints
|
|
|
|
- `go test ./...` passes with no Docker and no Postgres. Postgres tests carry `//go:build integration`.
|
|
- No changes to `internal/master/web/templates/*`, `static/*`, `layout.html` or `groupServices`.
|
|
- Interfaces declared at the consumer (`jobs.Store`, `web.Store`, `web.Jobs`). Hand-written fakes and testify, table-driven where there is more than one case.
|
|
- GoDoc on every exported identifier. Conventional Commits, each ending with ` (#10)`.
|
|
- Every state-changing request is a POST with a CSRF token. Refusals render the service page with the reason (409), not a bare error.
|
|
- The invariants of earlier phases hold: one active job per service (`update_jobs_one_active`), the webhook and poller never create services, the updater's own services are never updated or merged.
|
|
|
|
## Deliberate deviations from the spec in this phase
|
|
|
|
1. **Drift compares what runs, not reference strings.** The spec says a service drifts when `actual_ref` differs from `desired_ref`. `desired_ref` is always `repo:tag@digest` while `actual_ref` is the raw image string, so a string comparison would flag every healthy service. `RunsRef` compares the repo exactly (no `docker.io` normalisation), then the digest when both sides know one, else the followed tag. The same digest under a different tag is therefore not drift.
|
|
2. **`awaiting_inventory`.** The agent's upsert arrives a debounce interval after `job.result`; until then `desired_ref` is newer than `actual_ref`. The flag, set by `FinishJob` and cleared by the next upsert of the row, keeps that window from counting as drift.
|
|
3. **Poll and webhook jobs are refused for a drifted service** (`ReasonDrifted`). Manual jobs are allowed, so a person can still update or roll back.
|
|
4. **Move suggestion is stricter than "exactly one appears".** Besides one removed and one live row on each side, the live row must be newer, have no desired ref and no jobs, and the removed row must have something worth moving (desired ref, auto mode or jobs). Otherwise every restarted stack would produce a suggestion.
|
|
5. **Manual merge is allowed for Compose and Swarm of the same kind**, names may differ, same agent allowed. Plain containers and the updater itself are refused.
|
|
6. **Accept does not clear `available_*`**, and the form's `actual` field is only a guard; the value written comes from the row.
|
|
7. **The overview does not show moves or drift yet.** `MoveCandidates` is only read on the service page. `Service.Drifted()` is a model method, so the UI follow-up needs no further wiring.
|
|
8. **`CreateJob` maps the foreign key violation on `update_jobs_service_id_fkey` to `ErrNotFound`**, so a job for a service that a merge just deleted fails cleanly.
|
|
|
|
## Review Focus
|
|
|
|
| Failure case | Test that covers it |
|
|
|---|---|
|
|
| Drift by digest, not string: tag+digest vs tag and reported digest, bare image, tagless, port colon, other repo, case. | model `TestRunsRef`, `TestDrifted` |
|
|
| Accept writes `repo:tag@digest` from the running version. | model `TestRunningRef` |
|
|
| A fresh job result is not drift until the inventory arrives. | store `TestFinishJob_SetsAwaitingInventoryUntilTheNextUpsert`; e2e `TestNoDriftAfterASuccessfulJob` |
|
|
| Accept is guarded by the running version, refuses removed/never-updated, leaves `last_tag_digest`, `available_*`, mode alone. | store `TestAcceptRunning_*` |
|
|
| Poll and webhook start no job for a drifted service; manual does. | jobs `TestStart_RefusesPollAndWebhookWhileDrifted`; e2e `TestDriftedAutoServiceIsNotUpdatedByPollOrWebhook` |
|
|
| Restore deploys the recorded ref and remembers the drifted digest of the same tag. | jobs `TestRestoreDesired_*`; e2e `TestRestoreDeploysThePinnedRefOnTheAgent` |
|
|
| Move suggestion only for a unique, new, history-less target and a removed source with history. | store `TestMoveCandidates` |
|
|
| Merge keeps id, mode, desired ref, first seen; takes location and inventory; moves jobs; deletes the duplicate. | store `TestMergeService_MovesHistoryModeAndDesiredToTheNewLocation` |
|
|
| Merge refusals change nothing (active job, kinds, container, self, live old, removed new, revoked agent, unknown). | store `TestMergeService_Refusals` |
|
|
| Inventory after a merge updates the survivor; the old agent returning creates a new row. | store `TestMergeService_InventoryAfterTheMerge` |
|
|
| A job for a deleted service is `ErrNotFound`. | store `TestCreateJob_UnknownServiceIsNotFound` |
|
|
| Handlers: redirects, 400/404/409, signals published, survivor id in the redirect. | web `TestRestoreDesired`, `TestAcceptRunning_*`, `TestMergeService_*`, `TestServiceDetail_DriftAndMoveData` |
|
|
|
|
## Frozen contracts
|
|
|
|
### C1. Migration `0005_drift_merge.sql`
|
|
`services.awaiting_inventory boolean NOT NULL DEFAULT false`; index `services_by_compose_name` on `(compose_project, compose_service) WHERE kind = 'compose'`.
|
|
|
|
### C2. `internal/master/model`
|
|
`Service.AwaitingInventory`; `Service.Drifted()`; `RunsRef(desired, actualRef, actualDigest)`; `RunningRef(actualRef, actualDigest)`; `Move{FromID, ToID, FromAgent, ToAgent}` (FromID is the removed row). `ReasonDrifted`, `ReasonNotDrifted`; `MergeRefusedError{Reason}` with the `ReasonMerge*` constants (`ReasonSelf` is reused).
|
|
|
|
### C3. Store
|
|
- `upsertService` sets `awaiting_inventory = false`; `FinishJob` success sets it to true.
|
|
- `AcceptRunning(ctx, id, actualRef, actualDigest, desired)`: updates only a live, previously updated row that still reports that actual ref and digest. `ErrNotFound` for unknown or removed, `ErrConflict` otherwise.
|
|
- `MoveCandidates(ctx) ([]model.Move, error)`: Compose only, not self, other agent, unique on both sides before the extra filters (live row newer, no desired ref, no jobs; removed row has a desired ref, auto mode or jobs). Agent names by `LEFT JOIN`, so a revoked agent keeps its name.
|
|
- `MergeSources(ctx, toID)`: removed rows of the same kind (not container, not self), same name first, then `removed_at DESC`, at most 50.
|
|
- `MergeService(ctx, fromID, toID)`: one transaction. Lock order: the live row's agent (`FOR SHARE`, `ErrRevoked`), then both rows `ORDER BY id FOR UPDATE`. Validates (`MergeRefusedError`), refuses an active job on either row (`ErrJobActive`), moves the duplicate's jobs, reads its location and inventory, **deletes it**, then updates the old row (keeps id, kind, `first_seen_at`, `update_mode`, `desired_ref`; takes agent, location, inventory columns, available suggestion and poll error; `last_tag_digest` only if the old row has none; `removed_at = NULL`, `awaiting_inventory = false`). The delete comes first because `location_key` is unique and not deferrable.
|
|
- `CreateJob`: foreign key violation `update_jobs_service_id_fkey` becomes `ErrNotFound`.
|
|
|
|
### C4. Jobs
|
|
`Start` returns `NotUpdatableError{ReasonDrifted}` for a non-manual trigger on a drifted service. `Restorable(svc)`: drifted, then `updatable(svc, true)`, and the desired ref's repo is not the agent image repo. `RestoreDesired(ctx, id, by)`: `Start` with `Ref: svc.DesiredRef`, manual. After a created job, `SetLastTagDigest` to the drifted digest when the actual tag equals the desired tag, the digest is well formed, differs from the desired digest and from `last_tag_digest`.
|
|
|
|
### C5. Web
|
|
Routes `POST /services/{id}/restore`, `/accept` (form `actual` required; optional `return=overview`), `/merge` (form `from` = removed row, `{id}` = live row; redirects to the surviving id). `pageData` gains `RestoreBlocked`, `MoveFrom`, `MoveTo`, `MergeSources`. Log events `drift_accepted` and `service_merged`.
|
|
|
|
### C6. Concurrency
|
|
An upsert for the live row's location waits on its lock, then updates the survivor. A snapshot after the merge lists the survivor's key, so it is not marked removed. A removal racing a merge can be lost; the next snapshot fixes it. `CreateJob` on the deleted row gets `ErrNotFound`; the poller and webhook re-read and log it.
|
|
|
|
## Tasks
|
|
|
|
### Task 1: Store and model (C1, C2, C3)
|
|
Migration, `imageref.IsDigest` (replaces the local pattern in the store), model types and functions, store methods. Tests: model tables; integration tests listed in Review Focus.
|
|
|
|
### Task 2: Jobs (C4)
|
|
`Store.SetLastTagDigest` on the consumer interface, `Start` refusal, `Restorable`, `RestoreDesired`. Tests: refusal table, manual job to the desired ref with protocol 3, active job, old agent, last-tag-digest table.
|
|
|
|
### Task 3: Web (C5)
|
|
Handlers, interfaces, `pageData`. Tests with fakes.
|
|
|
|
### Task 4: End-to-end
|
|
`memStore.FinishJob` sets `AwaitingInventory`. `internal/e2e/drift_test.go`.
|
|
|
|
### Task 5: Documentation
|
|
README section "Drift and moved services", webhook note, CLAUDE.md design intent and gotchas, this plan.
|
|
|
|
## Final verification
|
|
|
|
```bash
|
|
gofmt -l . ; go vet ./...
|
|
go build ./cmd/master ./cmd/agent && rm -f master agent
|
|
go test ./...
|
|
go test -race ./internal/agent/... ./internal/master/... ./internal/e2e/
|
|
DATABASE_URL=postgres://... go test -tags integration ./internal/master/store/
|
|
```
|