vvzvlad/portainer
1
0
Fork 0
Code Issues 5 Pull Requests 4 Packages Projects Releases Wiki Activity

feat(automation): health-gated rollback + per-endpoint + notify hook (#12, epic #3 M5) #18

Closed
agent_coder wants to merge 2 commits from feat/12-hardening into feat/11-autoupdate
pull from: feat/12-hardening
merge into: vvzvlad:feat/11-autoupdate
Conversation 2 Commits 2 Files Changed 20 +1069 -15
agent_coder commented 2026-06-29 10:42:40 +03:00
Collaborator
Copy Link

Summary

Хардненинг авто-апдейта: health-gated rollback (standalone), per-endpoint enable, notifier-seam. Closes #12 (эпик #3, финальный M5). Бэкенд Go + фронт React. Стек поверх M4 (#16): base = feat/11-autoupdate, ретаргет на develop после мержа цепочки. Расширяет M4, не форкает.

P0 — health-gated rollback (standalone-путь): updateStandalone переписан: (1) перед recreate захватывает старый image id (sha256), оригинальный ref (Config.Image), наличие healthcheck; гейт только если rollback включён, healthcheck есть (nil/empty/NONE → skip, лог) и id+ref разрешены; (2) обычный Recreate(pull=true); (3) health-poll (healthGate, новый rollback.go) каждые 3с по State.Health.Status/Running, контекст от baseCtx; (4) rollback: ImageTag(oldImageID, originalRef) возвращает старый образ на ref (новый становится dangling, остаётся для разбора) → Recreate(pull=false) пересоздаёт на старом (реюз сохранения конфига); при сбое re-tag/recreate — unhealthy-контейнер не уничтожается, лог+update-failed; (5) cleanup перенесён за гейт — старый образ удаляется только после confirmed-healthy (цель отката не пропадает); при выключенном rollback поведение M4 без изменений.

  • чистый decideRollback(health, now, deadline) → healthy/trigger/continue (юнит-тест без Docker). Stack-rollback вне scope (документировано как будущее).

P1 — per-endpoint enable: Endpoint.ContainerAutomationDisabled bool (zero-value участвует → миграция не нужна), оба демона пропускают через AutomationEnabledForEndpoint; задаётся в endpoint update API. UI-контрол отложен (форма env большая; issue явно разрешает минимальную/daemon-side реализацию — бэкенд готов и покрыт тестами).

P2 — notifier-seam: Notifier интерфейс + logNotifier (структурный zerolog), Event{Kind,EndpointID,ContainerID,StackID,Image,Message,Err}, kinds updated/rollback/update-failed/heal-restarted; вшит в update/redeploy/rollback/recreate-fail/heal-restart. Без внешних интеграций — это seam под будущее.

Настройки: AutoUpdate.{RollbackOnFailure bool, RollbackTimeout string="120s"} — init-дефолты, backfill в M1/M4-миграции, golden, settings_update валидация, AutoUpdatePanel (тоггл+таймаут, тултип «standalone-only»), TS-типы.

How verified

  • go build ./api/... — чисто; go vet (containerautomation/datastore/settings/cmd/endpoints) — чисто; go test -race ./api/containerautomation/... — ok; go test ./api/datastore/ ./api/http/handler/settings/... — ok; gofmt -l — пусто. Тесты: TestDecideRollback, TestHasHealthGate, TestParseRollbackTimeout, TestLogNotifierDoesNotPanic, TestRecordingNotifierCapturesEvents, TestAutomationEnabledForEndpoint, TestSettingsUpdatePayloadValidateRollbackTimeout.
  • pnpm typecheck — чисто; pnpm lint — 0 ошибок; vitest (AutoUpdatePanel+validation) — зелёные.

Только ручная проверка: реальный rollback против живого Docker (unhealthy после апдейта → откат на старый образ), re-tag side-effect, cleanup-ordering, per-endpoint disable, Swarm.

Известные компромиссы (документированы)

  • No-healthcheck контейнер: гейт пропускается, апдейт без отката (лог).
  • Re-tag: rollback переносит originalRef со нового образа на старый; новый (плохой) образ остаётся dangling для пост-мортема.
  • Rollback-fail: unhealthy-контейнер оставляем на месте (не уничтожаем), громкий лог + событие.
  • P1 UI отложен (бэкенд готов).

Review checklist

  • критерии приёмки из #12 выполнены
  • вне scope ничего; EE/isBE не тронуты; стек-контейнер не отрывается; cleanup не удаляет цель отката до confirmed-healthy
## Summary Хардненинг авто-апдейта: health-gated rollback (standalone), per-endpoint enable, notifier-seam. Closes #12 (эпик #3, финальный M5). Бэкенд Go + фронт React. **Стек поверх M4 (#16): base = `feat/11-autoupdate`, ретаргет на `develop` после мержа цепочки.** Расширяет M4, не форкает. **P0 — health-gated rollback (standalone-путь):** `updateStandalone` переписан: (1) перед recreate захватывает старый image id (sha256), оригинальный ref (`Config.Image`), наличие healthcheck; гейт только если rollback включён, healthcheck есть (nil/empty/NONE → skip, лог) и id+ref разрешены; (2) обычный `Recreate(pull=true)`; (3) health-poll (`healthGate`, новый `rollback.go`) каждые 3с по `State.Health.Status`/`Running`, контекст от `baseCtx`; (4) rollback: `ImageTag(oldImageID, originalRef)` возвращает старый образ на ref (новый становится dangling, остаётся для разбора) → `Recreate(pull=false)` пересоздаёт на старом (реюз сохранения конфига); при сбое re-tag/recreate — unhealthy-контейнер не уничтожается, лог+`update-failed`; (5) **cleanup перенесён за гейт** — старый образ удаляется только после confirmed-healthy (цель отката не пропадает); при выключенном rollback поведение M4 без изменений. - чистый `decideRollback(health, now, deadline)` → `healthy/trigger/continue` (юнит-тест без Docker). Stack-rollback вне scope (документировано как будущее). **P1 — per-endpoint enable:** `Endpoint.ContainerAutomationDisabled bool` (zero-value участвует → миграция не нужна), оба демона пропускают через `AutomationEnabledForEndpoint`; задаётся в endpoint update API. UI-контрол отложен (форма env большая; issue явно разрешает минимальную/daemon-side реализацию — бэкенд готов и покрыт тестами). **P2 — notifier-seam:** `Notifier` интерфейс + `logNotifier` (структурный zerolog), `Event{Kind,EndpointID,ContainerID,StackID,Image,Message,Err}`, kinds `updated`/`rollback`/`update-failed`/`heal-restarted`; вшит в update/redeploy/rollback/recreate-fail/heal-restart. Без внешних интеграций — это seam под будущее. **Настройки:** `AutoUpdate.{RollbackOnFailure bool, RollbackTimeout string="120s"}` — init-дефолты, backfill в M1/M4-миграции, golden, settings_update валидация, `AutoUpdatePanel` (тоггл+таймаут, тултип «standalone-only»), TS-типы. ## How verified - `go build ./api/...` — чисто; `go vet` (containerautomation/datastore/settings/cmd/endpoints) — чисто; `go test -race ./api/containerautomation/...` — ok; `go test ./api/datastore/ ./api/http/handler/settings/...` — ok; `gofmt -l` — пусто. Тесты: `TestDecideRollback`, `TestHasHealthGate`, `TestParseRollbackTimeout`, `TestLogNotifierDoesNotPanic`, `TestRecordingNotifierCapturesEvents`, `TestAutomationEnabledForEndpoint`, `TestSettingsUpdatePayloadValidateRollbackTimeout`. - `pnpm typecheck` — чисто; `pnpm lint` — 0 ошибок; vitest (AutoUpdatePanel+validation) — зелёные. **Только ручная проверка:** реальный rollback против живого Docker (unhealthy после апдейта → откат на старый образ), re-tag side-effect, cleanup-ordering, per-endpoint disable, Swarm. ## Известные компромиссы (документированы) - No-healthcheck контейнер: гейт пропускается, апдейт без отката (лог). - Re-tag: rollback переносит `originalRef` со нового образа на старый; новый (плохой) образ остаётся dangling для пост-мортема. - Rollback-fail: unhealthy-контейнер оставляем на месте (не уничтожаем), громкий лог + событие. - P1 UI отложен (бэкенд готов). ## Review checklist - [ ] критерии приёмки из #12 выполнены - [ ] вне scope ничего; EE/isBE не тронуты; стек-контейнер не отрывается; cleanup не удаляет цель отката до confirmed-healthy <!-- state:review reviewed_head: cdf17d904d9bc6798f50503f3c3f0240f56a72ec baseline_head: cdf17d904d9bc6798f50503f3c3f0240f56a72ec verdict: CHANGES round: 1 max_rounds: 6 open_findings: [F1,F2] reopened: {} reviewer: agent_reviewer -->
agent_coder added 1 commit 2026-06-29 10:42:44 +03:00
feat(automation): health-gated rollback + per-endpoint + notify hook (#12, epic #3 M5) 32a2b7a9ae 
P0 Health-gated rollback (standalone auto-update path): capture the previous
image id + reference + healthcheck before the recreate, then poll the new
container's health over a configurable window. On healthy proceed (and only
then clean up the old image); on unhealthy/exit/timeout re-tag the old image
back onto the original reference and Recreate (no pull) to restore it, reusing
Recreate's config preservation. The decision is a pure decideRollback() helper.

P1 Per-endpoint enable: ContainerAutomationDisabled flag on Endpoint (zero value
participates, no migration churn), checked by both daemons; settable via the
endpoint update API. UI control deferred (see report).

P2 Notifier seam: minimal Notifier interface + logNotifier, emitting structured
updated/rollback/update-failed/heal-restarted events from the daemon.

Settings: RollbackOnFailure + RollbackTimeout (default 120s) added to
ContainerAutomation.AutoUpdate, wired through defaults/migration/golden,
settings_update validation, the AutoUpdatePanel and the TS types.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder added the review/needs label 2026-06-29 10:44:16 +03:00
agent_coder added 1 commit 2026-06-29 10:58:13 +03:00
fix(automation): rollback robustness — transient inspect, start_period, digest images, shutdown, event order (#12 review) cdf17d904d 
F1: tolerate up to 3 consecutive health-gate inspect failures (reset on
success) before declaring an update failed, so a transient Docker API blip no
longer triggers a false rollback.

F2: detect baseCtx cancellation during the gate and abort without rolling back
or emitting update-failed (debug log only), instead of a misleading
"rollback failed" event on every shutdown mid-gate.

F3: derive the gate deadline as start + max(RollbackTimeout, StartPeriod+buffer)
via effectiveRollbackDeadline, reading the container's healthcheck StartPeriod
so a legitimately slow-starting container is not rolled back while starting.

F4: only enable the gate when the original reference is a proper tag (new
isTagReference helper); skip with a log line for digest-pinned / bare-image-id
containers that cannot be re-tagged.

F5: document the sequential-tick delay limitation of the gate poll.

F6: emit EventUpdated only after the gate confirms healthy (or immediately when
no gate is active); the rollback path emits only EventRollback, so the event
sequence is truthful.

F7: floor RollbackTimeout at 10s in backend and frontend validation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_reviewer commented 2026-06-29 11:18:40 +03:00
Collaborator
Copy Link

Ревью M5 «hardening: health-gated rollback + per-endpoint enable + notifier-seam» — agent_reviewer

Полный веер 8 аспектов против базы стека origin/feat/11-autoupdate... (head cdf17d9).

Очень крепкий PR. security / regressions / test-coverage / documentation / conventions / architecture — LGTM: чистое ядро отката (decideRollback/effectiveRollbackDeadline/isTagReference/inspectErrorTolerated) образцово отделено от Docker-IO и покрыто юнит-тестами с границами (deadline, start_period, transient-inspect ≤3); rollback не подтягивает чужой образ (ImageTag+Recreate(pull=false), digest-pinned/bare-id отсекаются); cleanup корректно перенесён ЗА health-gate (цель отката не удаляется до confirmed-healthy); при сбое re-tag/recreate unhealthy-контейнер не уничтожается; per-endpoint opt-out на верном уровне с обратно-совместимым zero-value (без миграции); путь при rollback=off идентичен M4; дефолт 120s/floor 10s согласованы во всех местах вкл. golden/swagger.

verdict: CHANGES — 1 warning + 1 suggestion.

F1 [warning] pre-update inspect на неограниченном baseCtx может навсегда подвесить авто-апдейт

Новый pre-update cli.ContainerInspect(s.baseCtx, c.ID) (autoupdate.go:246) вызывается на голом s.baseCtx (контекст жизни приложения, без per-operation таймаута). Все остальные Docker-вызовы пакета ограничены: листинг/heal — endpointTimeout, status-check — statusCheckTimeout, recreate — recreateTimeout, health-poll — context.WithDeadline (autoupdate.go:127,152,274,339,374, rollback.go:212, autoheal.go:136,174). Этот единственный inspect — исключение.
Последствие: если Docker API подвиснет на этом inspect (реалистично для remote-TCP эндпоинта при сетевом partition / неотвечающем демоне), горутина авто-апдейта повиснет до самого shutdown, а overlap-guard (updateRunning.CompareAndSwap) превратит ВСЕ последующие тики в no-op — авто-апдейт молча умрёт до перезапуска Portainer, без таймаут-ошибки в логах. Регресс внесён именно M5 (раньше этого inspect не было). Проверено чтением кода.
DO: обернуть в ограниченный контекст, как остальные inspect-вызовы: inspectCtx, cancel := context.WithTimeout(s.baseCtx, statusCheckTimeout); defer cancel() и передать inspectCtx. Поведение при ошибке уже корректное (лог + продолжение без health-gate).

F2 [suggestion] мёртвый guard newContainer != nil

Recreate (api/docker/container.go) при err==nil всегда возвращает не-nil *ContainerJSON; после if err != nil { return } в updateStandalone newContainer не может быть nil. Поэтому блок autoupdate.go:292-294 (newImage := ""; if newContainer != nil { newImage = newContainer.Config.Image }) содержит недостижимую ветку — и это противоречит коду ниже, где newContainer.ID разыменовывается безусловно (:304,:310,:320). Та же лишняя проверка осталась в условии cleanup (:324).
DO: заменить блок :292-294 на newImage := newContainer.Config.Image и убрать newContainer != nil из условия :324.

DROP (в калибровку): notifier-seam (Notifier/Event/logNotifier) — по факту один внутренний потребитель и нет точки инъекции (YAGNI), плюс дублирует существующие log.Warn/Info на тех же call-site. Но это намеренный deliverable милстоуна M5, дёшево и обратимо — не DO и не ESCALATE.
reviewer: agent_reviewer · LGTM: security, regressions, test-coverage, documentation, conventions, architecture.

## Ревью M5 «hardening: health-gated rollback + per-endpoint enable + notifier-seam» — agent_reviewer Полный веер 8 аспектов против базы стека `origin/feat/11-autoupdate...` (head `cdf17d9`). Очень крепкий PR. **security / regressions / test-coverage / documentation / conventions / architecture — LGTM**: чистое ядро отката (`decideRollback`/`effectiveRollbackDeadline`/`isTagReference`/`inspectErrorTolerated`) образцово отделено от Docker-IO и покрыто юнит-тестами с границами (deadline, start_period, transient-inspect ≤3); rollback не подтягивает чужой образ (`ImageTag`+`Recreate(pull=false)`, digest-pinned/bare-id отсекаются); cleanup корректно перенесён ЗА health-gate (цель отката не удаляется до confirmed-healthy); при сбое re-tag/recreate unhealthy-контейнер не уничтожается; per-endpoint opt-out на верном уровне с обратно-совместимым zero-value (без миграции); путь при rollback=off идентичен M4; дефолт `120s`/floor `10s` согласованы во всех местах вкл. golden/swagger. **verdict: CHANGES** — 1 warning + 1 suggestion. --- ### F1 [warning] pre-update inspect на неограниченном `baseCtx` может навсегда подвесить авто-апдейт Новый pre-update `cli.ContainerInspect(s.baseCtx, c.ID)` (`autoupdate.go:246`) вызывается на голом `s.baseCtx` (контекст жизни приложения, без per-operation таймаута). Все остальные Docker-вызовы пакета ограничены: листинг/heal — `endpointTimeout`, status-check — `statusCheckTimeout`, recreate — `recreateTimeout`, health-poll — `context.WithDeadline` (`autoupdate.go:127,152,274,339,374`, `rollback.go:212`, `autoheal.go:136,174`). Этот единственный inspect — исключение. **Последствие:** если Docker API подвиснет на этом inspect (реалистично для remote-TCP эндпоинта при сетевом partition / неотвечающем демоне), горутина авто-апдейта повиснет до самого shutdown, а overlap-guard (`updateRunning.CompareAndSwap`) превратит ВСЕ последующие тики в no-op — авто-апдейт молча умрёт до перезапуска Portainer, без таймаут-ошибки в логах. Регресс внесён именно M5 (раньше этого inspect не было). Проверено чтением кода. **DO:** обернуть в ограниченный контекст, как остальные inspect-вызовы: `inspectCtx, cancel := context.WithTimeout(s.baseCtx, statusCheckTimeout); defer cancel()` и передать `inspectCtx`. Поведение при ошибке уже корректное (лог + продолжение без health-gate). ### F2 [suggestion] мёртвый guard `newContainer != nil` `Recreate` (`api/docker/container.go`) при `err==nil` всегда возвращает не-nil `*ContainerJSON`; после `if err != nil { return }` в `updateStandalone` `newContainer` не может быть nil. Поэтому блок `autoupdate.go:292-294` (`newImage := ""; if newContainer != nil { newImage = newContainer.Config.Image }`) содержит недостижимую ветку — и это противоречит коду ниже, где `newContainer.ID` разыменовывается безусловно (`:304,:310,:320`). Та же лишняя проверка осталась в условии cleanup (`:324`). **DO:** заменить блок `:292-294` на `newImage := newContainer.Config.Image` и убрать `newContainer != nil` из условия `:324`. --- *DROP (в калибровку): notifier-seam (`Notifier`/`Event`/`logNotifier`) — по факту один внутренний потребитель и нет точки инъекции (YAGNI), плюс дублирует существующие `log.Warn/Info` на тех же call-site. Но это намеренный deliverable милстоуна M5, дёшево и обратимо — не DO и не ESCALATE.* *reviewer: agent_reviewer · LGTM: security, regressions, test-coverage, documentation, conventions, architecture.*
agent_reviewer added review/changes-requested and removed review/needs labels 2026-06-29 11:18:41 +03:00
agent_coder commented 2026-06-29 12:55:28 +03:00
Author
Collaborator
Copy Link

Объединено в один PR на эпик #3 → #19 (M5 вошёл целиком). Этот PR закрываю как superseded; ветка будет удалена (все коммиты в feat/3-auto-update).

Объединено в один PR на эпик #3 → #19 (M5 вошёл целиком). Этот PR закрываю как superseded; ветка будет удалена (все коммиты в feat/3-auto-update).
agent_coder closed this pull request 2026-06-29 12:55:28 +03:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
needs-human
эскалация: нужно решение человека
 review/approved
в последнем ревью нет открытых blocking
 review/changes-requested
последнее ревью оставило открытые DO/blocking
 review/needs
head не ревьюился (head != reviewed_head)
No Label review/changes-requested
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/portainer#18
Delete Branch "feat/12-hardening"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?