Repository navigation
feat(omnigraph): give the 0.13 server what it needs to take over the lock - #6351
Merged
Merged
Conversation
…lock From omnigraph 0.12 a server keeps its storage root's exclusive lock after it exits, and the 0.13 image's entrypoint releases it on the next boot. It needs a Lease to exclude other unlockers and the pod list to show the old pod has exited, and exits before serving without them. Keyed on internal_schema_version >= 14, which each environment reaches in its cutover change, so nothing changes on format 9: - Pre-create the omnigraph-unlock Lease and grant the server get/update on it and list on pods. Pass the entrypoint its Lease, selector, pod name, apply actor and graph-removal allow-list. - Add omnigraph:graph_removal_allowlist. 0.13 deletes a graph that cluster.yaml stops declaring, and the entrypoint refuses unless it is listed. An entry for a graph that is still declared fails the preview. - Stop creating the cluster-apply Job. Beside a serving 0.13 server it is refused, and at cutover its lock would refuse the server's first boot. - Suspend the optimize and cleanup sweeps, which the lock also refuses. Still to come for format 14: the read-only plan gate in the Job's place and the stop-window maintenance CronJob. Claude-Session: https://claude.ai/code/session_01SfdKkJ4YSVXssjVYmBPTWh
Also says in the runbook that the optimize and cleanup sweeps stay suspended on format 14, and drops an ignore_changes that did nothing. Claude-Session: https://claude.ai/code/session_01SfdKkJ4YSVXssjVYmBPTWh
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Falsey malformed allowlist values currently bypass preview-time validation.
2 open findings
What changed in this PR
Adds format-14 infrastructure required for Omnigraph 0.13 lock handoff and safe graph removal.
Changes:
- Adds the unlock Lease, scoped RBAC, and entrypoint environment.
- Disables incompatible apply and maintenance jobs for persisted-lock formats.
- Documents and tests the new cutover behavior.
| File | Description |
|---|---|
lock_handoff.py |
Provisions lock-handoff resources and validation. |
data_tier.py |
Integrates format-gated lock handoff and job suspension. |
__main__.py |
Reads and passes the graph-removal allowlist. |
test_lock_handoff.py |
Tests RBAC, environment, and validation. |
omnigraph-storage-format-upgrade-runbook.md |
Updates format-14 procedures. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Both callers passed the value through `or []`, so a malformed falsey value
(`{}`, `0`, `false`) became an empty allow-list before
`validate_graph_removal_allowlist` could refuse it. In a change that also
removes a graph, the first sign would be the new pod refusing to apply, after
`Recreate` had stopped the old one.
This defaults only a missing key, so any configured value reaches the type
check and fails the preview.
Claude-Session: https://claude.ai/code/session_013DBLiMHBByMfSpbTRLUNXG
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
witan task
tk-ol-infrastructure-omnigraph-stack-for-0-12-clust-2dee94, part of the omnigraph 0.13 / storage format 14 upgrade (D3 and D4 of the spec in mitodl/agent-kit#461). This is the first part of that task. The entrypoint it serves is mitodl/agent-kit#479.Description (What does it do?)
From omnigraph 0.12 a server keeps its storage root's exclusive lock after it exits, and the next start is refused until that lock is force-unlocked. The 0.13 image's entrypoint does that on boot, after taking a Kubernetes Lease and checking that no other server pod is alive. Without the Lease, the RBAC and its env it exits before serving.
Everything here is keyed on
omnigraph:internal_schema_version >= 14. Each environment reaches that in its cutover change, andvalidate_internal_schema_versionties the value to the servedfmt<N>prefix, so it cannot be set early.lock_handoff.py: theomnigraph-unlockLease, pre-created with no holder because RBAC cannot restrictcreateby name, and a Role giving theomnigraph-serverServiceAccountget/updateon that Lease andliston pods. The Deployment getsOMNIGRAPH_UNLOCK_LEASE,OMNIGRAPH_POD_SELECTOR,POD_NAME,OMNIGRAPH_APPLY_ACTORandOMNIGRAPH_GRAPH_REMOVAL_ALLOWLIST, and depends on the Lease and the binding.omnigraph:graph_removal_allowlist. 0.13 deletes a graph's storage and history whencluster.yamlstops declaring it, and the entrypoint refuses that apply unless the graph is listed. An entry for a graph that is still declared fails the preview, so a stale entry cannot let a later accidental removal through.cluster-applyJob is not created at format 14. A no-op direct apply still leaves a lock (measured on 0.13.0), which would refuse the server's first boot after a cutover, and beside a serving server the apply is refused. The entrypoint applies on every boot./readyzcomment is rewritten from a measurement on 0.13.0 with one of four graphs missing: 200,status: degraded,blocked_graph_count: 1.Not in this PR, and needed before any environment cuts over:
cluster planJob in the apply Job's place. Until it exists, a bad schema or policy change is first seen by the restarting pod, afterRecreatehas stopped the old one.WitanDailyMaintenanceStale*(36h) andScheduledJobStaleSlow*(15d) matchomnigraph-optimizeandomnigraph-cleanupby name and do not filter onkube_cronjob_spec_suspend, so they fire for the suspended jobs after a cutover. Until the replacement exists that is accurate, because nothing is compacting. The change that adds it should repoint the rules, or they fire for good.WitanGraphQuarantined*: on 0.13 the startup line'sgraph_countcounts declared graphs, so the rule can no longer fire. The new signal is thegraph blocked during startup graph_id=<id>WARN line.How can this be tested?
pulumi preview --stack CIwith the deployed image digest: 52 unchanged. I could not preview the format-14 branch, since no stack can declare 14 while it servesfmt9.data_tier.pyimportspulumi_aws, so nothing in the suite callscreate_data_tier, and the four gated lines there (no apply Job, suspended sweeps, the env, thedepends_on) are covered by reading only.lock_handoff.pyis tested under Pulumi mocks against the names and verbs the entrypoint uses (one GET and one PUT on the Lease, one pod list).pytest tests/ol_infrastructure/applications/omnigraph/: 227 passed.https://claude.ai/code/session_01SfdKkJ4YSVXssjVYmBPTWh