Skip to content

feat(omnigraph): add the 0.13 plan gate and the nightly stop window - #6355

Merged
blarghmatey merged 2 commits into
mainfrom
omnigraph-013-stack-maintenance
Oct 13, 2026
Merged

blarghmatey merged 2 commits into
mainfrom
omnigraph-013-stack-maintenance

Conversation

@blarghmatey

@blarghmatey blarghmatey commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

What are the relevant tickets?

N/A (witan task tk-ol-infrastructure-omnigraph-stack-for-0-12-clust-2dee94, part of the omnigraph 0.13 upgrade. Spec: mitodl/agent-kit#461 D4 and D5)

Description (What does it do?)

Since #6351, a stack at storage format 14 creates no cluster-apply Job and suspends the optimize and cleanup CronJobs, because both are refused with state_lock_held beside a serving 0.12+ server. That left a format-14 environment with no pre-deploy check on a schema or policy change, and no compaction or version cleanup.

This adds the two replacements, gated on internal_schema_version >= 14 like the lock handoff. Both run commands baked into the omnigraph-server image by mitodl/agent-kit#480.

  • A read-only plan gate (create_plan_gate in lock_handoff.py) in the apply Job's slot. It runs omnigraph_plan_gate.py with the new image against the served root, and the Deployment depends on it. The entrypoint applies only after Recreate has stopped the old pod, so without the gate a change the apply refuses is first seen by a pod that cannot start. Not created while a migration is armed, for the reason the apply Job is not.
  • One omnigraph-maintenance CronJob (create_maintenance_window in maintenance.py) in place of optimize and cleanup, which are deleted at format 14. It runs omnigraph_maintenance.py: scale the server to 0, take over its serve lock, optimize every graph (and cleanup on Sundays), scale back up. 03:20 UTC, omnigraph:maintenance_schedule to move it, omnigraph:cleanup_iso_weekday for the cleanup day.
  • Its own ServiceAccount, omnigraph-maintenance, added to the existing IRSA role for the same S3 access. Its Role allows get/update on the omnigraph-unlock Lease, list on pods, get on the omnigraph-server Deployment and get/update on its scale. Bound to the server's ServiceAccount, those verbs would let the server scale itself.
  • The Deployment carries ol.mit.edu/migration-armed while a migration is armed. The window skips on it and does not scale the server back up if it appears mid-window.
  • The council probe's default schedule at format 14 is */15 0-2,4-23 * * *. The probe has no Kubernetes credential to read a window marker with, so it skips the hour, and its 04:00 run is the check that the window ended. Setting maintenance_schedule without council_probe_schedule fails the preview.
  • The migration Job's writer list comes from whichever maintenance CronJobs exist.
  • The Deployment carries pulumi.com/patchForce at format 14 (see "Unverified").
  • omnigraph-maintenance is added to the daily staleness rule in metric_rules/witan.py.
  • Both runbooks: the stop window and how to recover when it leaves the server down, and the cutover steps that named the two old CronJobs.

The window is a nightly data-tier outage for every witan user. The time budget from 03:20: no new command after 25 minutes, one command stopped after 10, Job deadline at 36, so it ends before the CI indexer's 04:00 run. I sized that against Production's current sweeps (kube_job_status_completion_time - kube_job_status_start_time, 30 days to 2026-10-13, beside a live 0.11 server): optimize 55s to 163s, cleanup 329s to 1552s with the last three runs the longest. So a Sunday window in Production may not reach every graph. The command rotates where the cleanup pass starts each week so no graph is starved, the unreached graphs fail the Job, and nothing is left locked. These need re-measuring on 0.13 in CI and QA before Production's cutover, and the budget constants may have to move.

On format 9 (CI, QA and Production today) this creates the omnigraph-maintenance ServiceAccount and adds it to the IRSA role's trust policy. Nothing else should change, and the preview on this PR is the check.

Not in this PR, and still needed before a cutover:

  • Re-sizing the startup probe budget from a measured 0.13 boot.
  • A second expression for WitanGraphQuarantined* on graph blocked during startup, and an alert on 0 available replicas outside the window. Neither can be validated in Loki until an environment runs 0.13.

Unverified:

  • The format-14 branch of create_data_tier has not been through a pulumi preview. test_data_tier_wiring.py runs the whole function under Pulumi mocks for each format, which checks what is created and not how the provider diffs it.
  • The window writes spec.replicas through the scale subresource, so a later Pulumi update that changes replicas (arming a migration) could be refused with a server-side-apply field conflict. The Deployment therefore carries pulumi.com/patchForce at format 14. I have not observed the conflict or the forced apply. Check kubectl get deploy omnigraph-server --show-managed-fields after the first window in CI.
  • The first apply after this merges should be a full pulumi up. The Deployment and the sweeps gain a depends_on edge to the new ServiceAccount, and Pulumi refuses a --targeted apply that leaves such an edge outside the target list.

How can this be tested?

  • uv run pytest tests/ol_infrastructure/applications/omnigraph: 251 passed. test_data_tier_wiring.py runs create_data_tier under Pulumi mocks for format 9, format 14 and an armed migration from each. test_maintenance_window.py covers the Role's rules and its subject, the CronJob's command, env, deadline and grace period against what the image reads, the time budget including a late start, and the gate's env and mounts.
  • pre-commit run passes on the changed files. mypy on the omnigraph application reports the same two token_sync.py errors as main.
  • A second agent reviewed the diff for format-9 changes and against the image's contract. It found an annotations={} on the Deployment that would have shown as an update on every stack (fixed, now None), and the items under "Unverified".
  • The commands themselves are tested in feat(omnigraph): add the maintenance window and the pre-deploy plan gate agent-kit#480, including a whole window and the gate against the released 0.13.0 binary on a file:// root. Nothing has run against a real cluster or S3. The first run is CI's cutover.

https://claude.ai/code/session_01W4TjT25fD1W4TS1uRUY1hC

At storage format 14 the stack creates no cluster-apply Job and suspends
the optimize and cleanup CronJobs, because both are refused beside a
serving 0.12+ server. That left a format-14 environment with no pre-deploy
check on a schema or policy change and no compaction or version cleanup.

This adds, gated on the served format like the lock handoff:

- a read-only `cluster plan` Job in the apply Job's slot, which the
  Deployment depends on, so a change the entrypoint's apply would refuse
  fails the update while the old pod is still serving;
- one `omnigraph-maintenance` CronJob in place of optimize and cleanup. It
  scales the server to 0 for a nightly window, under its own IRSA
  ServiceAccount with a Role limited to the unlock Lease, the pod list and
  the server Deployment's scale;
- the `ol.mit.edu/migration-armed` annotation on the Deployment, which the
  window reads so it does not scale the server back up under an armed
  migration;
- a council probe schedule that skips the window's hour.

Both commands are in the omnigraph-server image
(mitodl/agent-kit#480).

Environments on format 9 get a second ServiceAccount on the IRSA role and
no other change.

Claude-Session: https://claude.ai/code/session_01W4TjT25fD1W4TS1uRUY1hC
Copilot AI balanced review requested due to automatic review settings October 13, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Replica field ownership, configuration validation, wiring coverage, and alert guidance need correction before rollout.

4 open findings
What changed in this PR

Adds format-14 Omnigraph deployment safeguards and nightly maintenance.

Changes:

  • Adds a pre-deploy plan gate and stop-window CronJob.
  • Adds RBAC, migration coordination, probe scheduling, and monitoring.
  • Adds tests and operational runbooks.
File Description
tests/​ol_infrastructure/​applications/​omnigraph/​test_storage_migration_job.py Updates maintenance writer fixtures.
tests/​ol_infrastructure/​applications/​omnigraph/​test_maintenance_window.py Tests gate and maintenance resources.
src/​ol_infrastructure/​infrastructure/​grafana_alerting/​metric_rules/​witan.py Monitors maintenance staleness.
src/​ol_infrastructure/​applications/​omnigraph/​storage_migration.py Supports variable maintenance writers.
src/​ol_infrastructure/​applications/​omnigraph/​maintenance.py Implements the stop window.
src/​ol_infrastructure/​applications/​omnigraph/​lock_handoff.py Implements the plan gate.
src/​ol_infrastructure/​applications/​omnigraph/​data_tier.py Wires format-dependent resources.
src/​ol_infrastructure/​applications/​omnigraph/​council_probe.py Skips the maintenance hour.
src/​ol_infrastructure/​applications/​omnigraph/​__main__.py Adds configuration and service accounts.
docs/​omnigraph-store-maintenance-runbook.md Documents maintenance and recovery.
docs/​omnigraph-storage-format-upgrade-runbook.md Updates format-14 cutover guidance.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ol_infrastructure/applications/omnigraph/__main__.py
Comment thread src/ol_infrastructure/applications/omnigraph/data_tier.py Outdated
Comment thread src/ol_infrastructure/applications/omnigraph/data_tier.py
…gating

The maintenance window writes `spec.replicas` through the scale
subresource, which makes its client a second field manager of that field.
Server-side apply then refuses a Pulumi update that changes the count, and
the update that does is the one arming a migration. The Deployment now
carries `pulumi.com/patchForce` wherever a window exists.

Nothing ran the format and migration branches of `create_data_tier`, so a
plan gate created during a migration, or one missing at format 14, would
have passed every test. `test_data_tier_wiring.py` runs the whole function
under Pulumi mocks for format 9, format 14 and an armed migration from
each.

Also: `omnigraph:cleanup_iso_weekday: 0` was read as unset and became
Sunday without reaching the range check, and the daily staleness alert's
description now says what `omnigraph-maintenance` is and where its
recovery steps are.

Claude-Session: https://claude.ai/code/session_01W4TjT25fD1W4TS1uRUY1hC
@blarghmatey
blarghmatey merged commit c7fa821 into main Oct 13, 2026
12 checks passed
@blarghmatey
blarghmatey deleted the omnigraph-013-stack-maintenance branch October 13, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants