reviewer(ET): auto-commit from reviewer run_id=493
This commit is contained in:
95
docs/work-items/ORCH-062/12-review.md
Normal file
95
docs/work-items/ORCH-062/12-review.md
Normal file
@@ -0,0 +1,95 @@
|
||||
---
|
||||
verdict: APPROVED
|
||||
work_item: ORCH-062
|
||||
stage: review
|
||||
author_agent: reviewer
|
||||
status: approved
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
type: review
|
||||
work_item_id: ORCH-062
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-062 — INFRA: авто-prune docker build cache на mva154
|
||||
|
||||
## Summary
|
||||
|
||||
PR вводит фоновый daemon-поток `src/build_cache_pruner.py` («вторая половина» disk-watchdog
|
||||
ORCH-063): периодически выполняет **строго `docker builder prune -f --filter until=<until>`** на
|
||||
хосте через ssh, устраняя корень инцидента 07.06.2026 (build cache → 100% диска) автоматически.
|
||||
|
||||
Проверены все 4 оси. Реализация **точно** соответствует ADR-001 (D1…D7) и закрывает все 9 критериев
|
||||
приёмки. Полный регресс зелёный (`pytest tests/ -q` → **1319 passed**); новый модуль покрыт
|
||||
`tests/test_build_cache_pruner.py` (TC-01…TC-12, 23 кейса, docker замокан — ни один тест не трогает
|
||||
реальный docker/диск). Реестр QG, переходы стадий и схема БД **не тронуты** (проверено `git diff`:
|
||||
`src/stages.py`/`src/stage_engine.py`/`src/qg/`/`src/db.py` без изменений). Документация (golden
|
||||
source) обновлена в том же PR. **Findings P0/P1 отсутствуют.**
|
||||
|
||||
### Соответствие ТЗ / Acceptance Criteria
|
||||
- **AC-1** (авто-уборка без оператора): ✅ тик каждые `interval_s` (дефолт 6ч), pure-функция
|
||||
`decide_prune`.
|
||||
- **AC-2** (тёплый кэш удерживается): ✅ дефолт `until=24h`; `-a` добавляется **только в паре** с
|
||||
`until` (`build_prune_command`, TC-03).
|
||||
- **AC-3** (self-hosting безопасность): ✅ строго `docker builder prune`; в коде **нет**
|
||||
`image prune`/`system prune`/удаления контейнеров/рестарта прода (TC-03 ассертит явно).
|
||||
- **AC-4** (never-raise): ✅ per-команда + per-tick `try/except` (TC-04/05/06).
|
||||
- **AC-5** (kill-switch): ✅ гард в `main.lifespan` + `start()` (TC-07).
|
||||
- **AC-6** (конфигурируемость + валидаторы): ✅ `_bcp_positive_int`/`_bcp_until`/`_bcp_notify_min_gb`
|
||||
деградируют на безопасный дефолт + warning, старт не падает (TC-08).
|
||||
- **AC-7** (наблюдаемость): ✅ read-only блок `build_cache_prune` в `GET /queue`, `status()`
|
||||
never-raise (TC-09/TC-12).
|
||||
- **AC-8** (изоляция от QG/БД): ✅ leaf-модуль (TC-10 AST-проверка импортов); `STAGE_TRANSITIONS`/
|
||||
`QG_CHECKS`/схема БД не тронуты (проверено diff).
|
||||
- **AC-9** (документация + регресс): ✅ см. раздел «Документация»; регресс зелёный.
|
||||
|
||||
### Соответствие ADR
|
||||
- **ADR-001 D1** (leaf-демон, не host-инфра B/C): ✅ модуль leaf, каркас `disk_watchdog`.
|
||||
- **D2** (команда + удержание): ✅ строго BuildKit GC, `-a` только с `until`.
|
||||
- **D3** (ssh-канал, no-op без таргета): ✅ `_ssh_target()`, пустой `deploy_ssh_host` → no-op
|
||||
(TC-05).
|
||||
- **D4** (конфиг/дефолты/валидаторы): ✅ 6 флагов и дефолты (`enabled=True`, `interval=21600`,
|
||||
`until=24h`, `all=False`, `timeout=120`, `notify_min_gb=0`) совпадают с таблицей ADR.
|
||||
- **D5** (наблюдаемость): ✅ форма `status()` соответствует.
|
||||
- **D6** (инварианты/never-raise/без миграции): ✅ in-memory state, два уровня never-raise.
|
||||
- **D7** (lifecycle): ✅ старт последним после `disk_watchdog.start()`, стоп первым в reverse.
|
||||
- **Трассировка маркеров:** правки в `main.py`/`config.py`/`INFRA.md` аддитивны рядом с маркерами
|
||||
ORCH-063; инвариант disk-watchdog (порядок старт/стоп демонов) сохранён — стоп идёт строго в
|
||||
reverse (`build_cache_pruner.stop()` → `disk_watchdog.stop()`). Нарушений нет.
|
||||
|
||||
### Качество кода
|
||||
- Docstrings на всех публичных функциях/методах; модульный docstring фиксирует инварианты.
|
||||
- `shlex.quote` на `until` (защита remote-shell) поверх regex-валидации `^\d+[smhdw]?$` —
|
||||
двойная защита от инъекции.
|
||||
- `decide_prune` вынесена в чистую функцию → детерминированно тестируема без потока/таймера.
|
||||
- Тесты содержательные: проверяют поведение (no-op без таргета, запись `last_error`, парсинг
|
||||
reclaimed, изоляция от QG через AST), а не тривиальные ассерты.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- Нет.
|
||||
|
||||
### P1 — Must fix
|
||||
- Нет.
|
||||
|
||||
### P2 — Should fix
|
||||
- Нет (опционально, не блокирует): `decide_prune(interval_s)` и `_stop.wait(interval_s)` дважды
|
||||
гейтят один интервал — это осознанный belt-and-braces (помечено в docstring), регрессом не
|
||||
является.
|
||||
|
||||
## Документация
|
||||
|
||||
Документация обновлена в том же PR — ось пройдена (golden source = код):
|
||||
- **`docs/operations/INFRA.md`**: добавлена секция «Build-cache-pruner (ORCH-062)» + 6 строк в
|
||||
карте env; **снята** формулировка ORCH-063 «освобождение build cache — ручная операция» в части
|
||||
build cache (требование AC-9 / TRZ §7 выполнено буквально).
|
||||
- **`docs/architecture/README.md`**: новый компонент в ряду фоновых демонов.
|
||||
- **`docs/architecture/adr/README.md`**: индекс adr-0025 (+ комплементарность adr-0024).
|
||||
- **`docs/architecture/adr/adr-0025-build-cache-pruner.md`**: сквозной ADR.
|
||||
- **`.env.example`**: 6 новых ключей `ORCH_BUILD_CACHE_PRUNE_*` (канон).
|
||||
- **`CHANGELOG.md`**: запись в `## [Unreleased]`.
|
||||
- **Артефакты задачи**: `06-adr/ADR-001`, `07-infra-requirements.md`, `10-tech-risks.md` заполнены.
|
||||
|
||||
Изменений в `README.md` «Известные ограничения» (ORCH-079) данный PR не закрывает — обзорная витрина
|
||||
обновления не требует.
|
||||
Reference in New Issue
Block a user