chore(openspec): archive completed changes
Move finished change directories to openspec/changes/archive/: - configurable-dashboard-widgets - decommission-monitoring-poller - service-registry - unify-tasks-on-services All associated implementation has been merged to main.
This commit is contained in:
@@ -0,0 +1,101 @@
|
||||
# Plan — decommission-monitoring-poller
|
||||
|
||||
> Status: **DRAFT — awaiting user approval before implementation.**
|
||||
> Scope: a focused backend+frontend decommission, not a full SDD change. Plan-then-implement (user-approved 2026-06-17).
|
||||
> Root cause this addresses: the 2026-06-16/17 observability update externalised metrics to Prometheus+Grafana+Loki+Alertmanager, but the *legacy Manage-side SSH-scraping monitor* (the `MonitoringPoller`, `/monitoring/disk`, `/monitoring/machines/{id}/actions`, and the `monitoring_machine_actions` SQLite table) was never removed. It duplicates the new stack, drains SSH budget every 300s, and feeds nothing (its UI was deleted in `e2ad731`).
|
||||
|
||||
## 1. Problem
|
||||
|
||||
Manage runs a background thread (`MonitoringPoller`) that, every 300s, SSHes into every configured machine, runs `df`, and stores the result in its own SQLite table (`monitoring_machine_actions`, 30-day retention). After the observability update, **Prometheus already scrapes node_exporter on these machines and Grafana already owns the dashboards**. The poller is pure duplication: more SSH sessions, more stale state, a second source of truth for "disk usage," and a SQLite table that nothing reads.
|
||||
|
||||
The alerting side (Alertmanager proxy + `/alerts` + `/alertmanager-status` + `/alertmanager-webhook` + `/prometheus-targets` + `/machines`) already fits the new model and is untouched by this change.
|
||||
|
||||
## 2. Goals / non-goals
|
||||
|
||||
**Goals**
|
||||
|
||||
- Stop the duplicated SSH-scraping of system metrics.
|
||||
- Remove the dead `/disk`, `/poller`, `/machines/{id}/actions` surface and the SQLite history that fed it.
|
||||
- Remove the now-orphaned frontend `DiskSpaceCard` + `DiskSpace` type.
|
||||
- Leave Manage a clean thin-dashboard: Alertmanager alerts + Prometheus target health + Grafana deep-links.
|
||||
|
||||
**Non-goals**
|
||||
|
||||
- Do NOT touch the Alertmanager proxy, `/prometheus-targets`, `/machines`, or `/alertmanager-webhook` — they fit the model.
|
||||
- Do NOT remove the `disk_usage` **job template** in `jobs.py` (user-approved: it is a manual on-demand Actions job, not monitoring).
|
||||
- Do NOT remove `node_exporter_*` fields on `MonitoringMachine` — they configure where Prometheus scrapes; that is correct and stays.
|
||||
- Do NOT introduce a Prometheus query proxy / PromQL reader in this change (that was the alternative the user did not pick).
|
||||
- Do NOT add new features. This is a removal.
|
||||
|
||||
## 3. Exact removal map (verified against source)
|
||||
|
||||
### Backend — delete entirely
|
||||
|
||||
- `backend/src/media_library_viewer_api/services/monitoring_poller.py` (the `MonitoringPoller` class, `PollerConfig`, `_MONITORING_POLLER`, `get_monitoring_poller`).
|
||||
- **Verified sole callers:** `main.py` lifespan, `dependencies.py` wrapper, `routers/monitoring.py:/poller`, `routers/settings.py` (machine save → `poller.start()/kick()`).
|
||||
- `backend/src/media_library_viewer_api/services/monitoring_actions.py` (the whole file: `build_machine_client`, `disk_space`, `summarize_operation_result`, `json_compact`, `run_machine_operation`, `poll_machine_snapshot`).
|
||||
- **Verified:** `run_machine_operation` has exactly 2 callers (`poll_machine_snapshot` here, and `/monitoring/disk`) — both going. `tasks.py` does NOT use it. Nothing else imports the module.
|
||||
- `backend/tests/test_monitoring_actions.py` (36 lines, tests `poll_machine_snapshot`).
|
||||
|
||||
### Backend — edit in place
|
||||
|
||||
- `backend/src/media_library_viewer_api/main.py` lifespan (lines ~48–56): remove `monitoring_poller = get_monitoring_poller()`, `monitoring_poller.start()`, `monitoring_poller.stop()`, and the `get_monitoring_poller` import on line 16. Keep `backup_poller` and `mail_queue` intact.
|
||||
- `backend/src/media_library_viewer_api/dependencies.py`: remove the `MonitoringPoller` import block (lines 24–29) and the `get_monitoring_poller` wrapper (lines 254–256).
|
||||
- `backend/src/media_library_viewer_api/routers/monitoring.py`: remove imports of `disk_space`, `run_machine_operation`, `poll_machine_snapshot`; remove the three endpoints `/poller` (99), `/machines/{machine_id}/actions` (113), `/disk` (127). Keep `/machines`, `/prometheus-targets`, `/alerts`, `/alertmanager-status`, `/alertmanager-webhook`. Also drop the now-unused `_resolve_machine` helper if it becomes unreferenced after `/disk` and `/actions` removal (verify during impl — `/machines` does not use it).
|
||||
- `backend/src/media_library_viewer_api/routers/settings.py` (lines 198–204 and 218–224): remove the `get_monitoring_poller()` + `poller.start()` + `poller.kick()` calls from `post_machine` and `put_machine`. Keep `write_prometheus_targets(store)` (that is the new-model target generation).
|
||||
- `backend/src/media_library_viewer_api/services/settings_store.py`:
|
||||
- Remove `CREATE TABLE IF NOT EXISTS monitoring_machine_actions` (lines ~90) and its two indexes (`idx_monitoring_machine_actions_machine_time`, `idx_monitoring_machine_actions_action_status`, lines ~183–190) from `init_schema`.
|
||||
- Remove methods `record_machine_action` (566), `list_machine_actions` (610), `prune_machine_actions` (638).
|
||||
- Note: existing databases will keep the orphaned `monitoring_machine_actions` table harmlessly (no migration framework here — `init_schema` is `CREATE TABLE IF NOT EXISTS` + ad-hoc `ALTER`). A one-line `DROP TABLE IF EXISTS` can be added to `init_schema` for cleanliness; decide at impl time.
|
||||
- `backend/src/media_library_viewer_api/config.py`: remove `monitoring_poll_interval_seconds` (56), `monitoring_poll_initial_delay_seconds` (57), `monitoring_action_retention_days` (58).
|
||||
|
||||
### Backend — tests to fix
|
||||
|
||||
- `backend/tests/test_api.py`:
|
||||
- `TestMonitoring.test_disk` (line 615) — **remove** (tests `/api/monitoring/disk`).
|
||||
- `TestMonitoring.test_prometheus_targets_empty` and `..._returns_enabled_ssh_node_exporter` — **keep** (test the surviving `/prometheus-targets`).
|
||||
- `TestSettingsMachines` — **keep** but verify they still pass after the `poller` calls are removed from `post/put_machine`.
|
||||
- `TestAlertmanager` — **keep** (untouched).
|
||||
- The `disk_usage` reference at line 576/586 is the **Jobs** test (`/api/jobs/run`), NOT the monitoring poller — **keep** (the job template stays).
|
||||
|
||||
### Frontend — delete
|
||||
|
||||
- `frontend/src/components/DiskSpaceCard.tsx` — **verified orphaned** (only `__tests__/DiskSpaceCard.test.tsx` imports it; no page uses it).
|
||||
- `frontend/src/components/__tests__/DiskSpaceCard.test.tsx`.
|
||||
- `frontend/src/types/index.ts` `DiskSpace` interface (line 279) — remove after confirming no importer (grep shows none outside the type file).
|
||||
|
||||
### Docs
|
||||
|
||||
- `AGENTS.md` line 25 ("starts the mail queue and monitoring poller") → "...starts the mail queue and backup alert poller."
|
||||
- `docs/monitoring-logging-design.md` line 65 (describes the poller) → update or strike the poller paragraph.
|
||||
- `docs/MIGRATION_PLAN.md` line 110 (`/api/monitoring/disk` row) → remove the row.
|
||||
- `docs/REQUIREMENTS.md` → add a note that Manage-side system-metric scraping is retired in favour of the external observability stack.
|
||||
- `docs/superpowers/specs/2026-05-11-backup-monitoring-design.md` is a historical spec; leave as-is (it is an archived design doc).
|
||||
|
||||
## 4. Slice plan (≤400 lines each, build+pytest green per slice)
|
||||
|
||||
1. **Slice 1 — Backend removal (endpoints + poller + actions + store + config).** Delete `monitoring_poller.py`, `monitoring_actions.py`, `test_monitoring_actions.py`; edit `main.py`, `dependencies.py`, `routers/monitoring.py`, `routers/settings.py`, `settings_store.py`, `config.py`; fix `test_api.py` (`test_disk` removed, `TestSettingsMachines` re-checked). Gate: `cd backend && PYTHONPATH=src pytest`.
|
||||
2. **Slice 2 — Frontend orphan removal.** Delete `DiskSpaceCard.tsx` + its test + `DiskSpace` type. Gate: `cd frontend && npm run build && npm run lint && npm test`.
|
||||
3. **Slice 3 — Docs.** `AGENTS.md`, `docs/monitoring-logging-design.md`, `docs/MIGRATION_PLAN.md`, `docs/REQUIREMENTS.md`. Gate: none (docs); commit standalone.
|
||||
|
||||
Estimated total: ~500–700 lines deleted, ~50–100 added (edits). Each slice well under 400.
|
||||
|
||||
## 5. Risks & verification
|
||||
|
||||
- **Hidden caller of `run_machine_operation` / `poll_machine_snapshot`**: mitigated — grep shows exactly the callers listed; re-grep at slice-1 start.
|
||||
- **`TestSettingsMachines` breakage** once `poller.start()/kick()` is removed from `post/put_machine`: those tests mock `write_prometheus_targets` and don't assert on the poller; should pass. If they reference `get_monitoring_poller`, fix by dropping the assertion.
|
||||
- **Orphaned SQLite table on existing DBs**: harmless (empty, unused). Optional `DROP TABLE IF EXISTS monitoring_machine_actions` in `init_schema` for cleanliness.
|
||||
- **No browser smoke**: same caveat as the UI rework; backend covered by pytest.
|
||||
- **`_resolve_machine` in monitoring.py** may become unused after `/disk` + `/actions` removal; remove if so.
|
||||
|
||||
## 6. Acceptance
|
||||
|
||||
- `cd backend && PYTHONPATH=src pytest` green (with `test_disk` + `test_monitoring_actions.py` removed).
|
||||
- `grep -rnE 'MonitoringPoller|poll_machine_snapshot|/monitoring/disk|monitoring_machine_actions|monitoring_poll_interval_seconds|DiskSpaceCard' backend/ frontend/src/` → only historical/docs hits (spec.md archive is fine).
|
||||
- `cd frontend && npm run build && npm run lint && npm test` green.
|
||||
- Docs updated to reflect Manage no longer scrapes its own metrics.
|
||||
|
||||
## 7. Open questions for the user (none blocking, defaults shown)
|
||||
|
||||
- Q1. Existing DBs' orphaned `monitoring_machine_actions` table — (a) add `DROP TABLE IF EXISTS` to `init_schema` for a clean slate [default], or (b) leave it harmless?
|
||||
- Q2. Commit/PR mechanics — same as the UI rework (commit per slice, no push until you say)?
|
||||
Reference in New Issue
Block a user