Skip to content

feat(omnigraph): hand the cluster lock over in the server entrypoint - #479

Open
blarghmatey wants to merge 3 commits into
mainfrom
omnigraph-013-entrypoint-ledger-v2
Open

feat(omnigraph): hand the cluster lock over in the server entrypoint#479
blarghmatey wants to merge 3 commits into
mainfrom
omnigraph-013-entrypoint-ledger-v2

Conversation

@blarghmatey

@blarghmatey blarghmatey commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What are the relevant tickets?

N/A (witan task tk-rewrite-the-omnigraph-server-entrypoint-and-poli-39b7a8, part of the omnigraph 0.13 upgrade, spec in #461 D3 and D4)

Description (What does it do?)

omnigraph 0.13 has no cluster import or cluster refresh, and every writer of a cluster root (the server, cluster apply, optimize, cleanup) leaves an exclusive lock in the storage root when it exits. The shell entrypoint fails on its first command under 0.13, and without an unlock every pod restart would be refused with state_lock_held.

This replaces docker/omnigraph-server-entrypoint.sh with docker/omnigraph_server_entrypoint.py. It is Python because it has to talk to the Kubernetes API and renew a Lease while a child runs. The boot sequence:

  1. Take the coordination.k8s.io Lease named by OMNIGRAPH_UNLOCK_LEASE, or exit 1 if another holder's is still valid. A holder left by an earlier container of the same pod is taken over.
  2. Wait until no other live pod matches OMNIGRAPH_POD_SELECTOR (Terminating pods count, Failed and Succeeded do not), or exit 1 after OMNIGRAPH_PEER_WAIT_SECONDS (default 50).
  3. Render group membership from the token map (render_groups.py, unchanged).
  4. cluster plan. Exit 1 on an outstanding deployment, on a graph removal not listed in OMNIGRAPH_GRAPH_REMOVAL_ALLOWLIST (0.13 deletes a dropped graph's storage and history), or on any other plan error.
  5. cluster status. Force-unlock a serve or deployment lock, and exit 1 on anything else (e.g. graph_operation, which belongs to a maintenance run).
  6. cluster apply --as $OMNIGRAPH_APPLY_ACTOR, then force-unlock the deployment lock it left.
  7. exec omnigraph-server.

Every force-unlock first proves the Lease with a conditional write and checks that enough of it is left. If no renewal has succeeded for 20s the running child is stopped and the boot exits without unlocking.

Outside Kubernetes (no KUBERNETES_SERVICE_HOST) there is no Lease or pod list to consult, so the entrypoint refuses to force-unlock unless OMNIGRAPH_UNLOCK_WITHOUT_PEERS=1 is set. cluster apply always leaves a lock, so a local docker run needs it on an empty root too. Without the opt-in, a docker run whose cluster.yaml points at a served root would unlock the live server.

Two differences from the sequence written in the spec:

  • The plan runs before the lock is inspected. The spec left unverified which status field reports an outstanding deployment. On the binary, cluster plan reports it as a cluster_deployment_outstanding diagnostic, and it does so while the dead apply's deployment lock is still held. I did not look for an equivalent field in cluster status. test_real_interrupted_apply_is_reported_as_outstanding pins the plan diagnostic by killing an apply mid-run.
  • OMNIGRAPH_APPLY_ACTOR is required when the image carries policy bundles. A no-op apply passes without an actor and the first schema change does not.

Also here: the policy fixture graph code_example is renamed to code-example (0.13 cluster apply accepts the underscore but omnigraph-server refuses to start on it), policy/check.sh drops cluster import, and the image installs python3 in place of python3-minimal, under which urllib.request and secrets do not import (checked in debian:trixie-slim).

This only works on 0.13 and has to merge with the pin bump, not before it. On the pinned 0.11.0 policy/check.sh fails at the converge step, since 0.11's cluster apply needs the ledger that import created, so the witan (Cedar policy bundle) check will be red here until the pin moves.

The ol-infrastructure side is not done yet and the image cannot boot in the cluster without it: the pre-created Lease, get/update on it and list on pods for the server's ServiceAccount, and the OMNIGRAPH_UNLOCK_LEASE, OMNIGRAPH_POD_SELECTOR and OMNIGRAPH_APPLY_ACTOR env vars (tk-ol-infrastructure-omnigraph-stack-for-0-12-clust-2dee94).

docker/README.md links to the spec file, which arrives with #461.

How can this be tested?

  • Against the released 0.13.0 binary (first on PATH, --noconftest because the hermetic conftest puts the pinned binary back in front): test_omnigraph_server_entrypoint.py 54 passed, none skipped. Five of those run the real sequence on a file:// copy of the policy fixture: first boot, boot/serve/SIGTERM/boot again, a schema change, a dropped graph with and without the allow-list, and an apply killed mid-run.
  • policy/check.sh with the 0.13.0 binary passes (80 policy test cases, and the rendered bundles validate).
  • On the pinned 0.11.0: test_omnigraph_server_entrypoint.py 49 passed, 5 skipped (the real-binary cases skip until the pin reports storage format 14). just test-witan-council was 1497 passed, 6 skipped on the first commit, and I did not rerun the full suite after the two fix commits, which touch only the entrypoint, its test file and docker/. policy/check.sh fails there with state_missing ... run cluster import to bootstrap state, as described above.
  • No tests cover KubernetesApi itself (URL building, TLS, error mapping), main, a Lease lost between the first and second unlock, or the renew thread running concurrently with confirm().
  • Not tested: the image build, the Lease and pod-list code against a real Kubernetes API (the tests use a fake), and anything on S3.

https://claude.ai/code/session_011nyp32f2v7efTTBMqnmEuG

@blarghmatey
feat(omnigraph): hand the cluster lock over in the server entrypoint
016dcbc 
omnigraph 0.13 has no `cluster import` or `cluster refresh`, and every
writer of a cluster root (the server, `cluster apply`, `optimize`,
`cleanup`) leaves an exclusive lock behind when it exits. The shell
entrypoint would fail on its first command, and a pod restart would be
refused with `state_lock_held` until someone force-unlocked by hand.

This replaces the shell entrypoint with a Python one that takes a
Kubernetes Lease, waits until no other live pod matches the Deployment's
selector, renders group membership, plans, releases the previous owner's
`serve` or `deployment` lock, applies, releases its own apply's lock and
execs the server. It exits 1 on a `graph_operation` lock, on an
outstanding deployment, and on a cluster.yaml that drops a graph outside
OMNIGRAPH_GRAPH_REMOVAL_ALLOWLIST, because 0.13 deletes a dropped
graph's storage. Losing the Lease stops the running child and unlocks
nothing.

The policy fixture graph `code_example` becomes `code-example`:
0.13's `cluster apply` accepts the underscore but omnigraph-server
refuses to start on it. policy/check.sh drops `cluster import`.

This only works on 0.13. On 0.11 `cluster apply` needs the ledger that
`import` created, so policy/check.sh fails there, and the real-binary
tests skip until the pin reports storage format 14. It has to merge
with the pin bump.

Design: docs/internals/design/omnigraph-0-13-upgrade-spec.md (D3, D4),
#461

Claude-Session: https://claude.ai/code/session_011nyp32f2v7efTTBMqnmEuG
@blarghmatey
fix(omnigraph): keep a metadata-only Lease change from failing a boot
796f5a2 
Three things from review of the entrypoint rewrite.

A renewal that hit a 409 and found the Lease still naming this holder
returned failure, so a confirm() that landed on a relabelled Lease
refused the boot. It now retries once on the new resourceVersion.

A truncated Kubernetes API response raises http.client.HTTPException,
which is neither OSError nor ValueError. It escaped _request and ended
the renew thread, and the running apply was then stopped 20 seconds
later for no reason.

`--cluster=<value>` was not recognised, so a storage-URI root passed
that way would have converged the default directory.

The Dockerfile comment said python3-minimal lacks json and ssl. Both
import there; urllib.request and secrets are the ones that do not
(checked in debian:trixie-slim).

Claude-Session: https://claude.ai/code/session_011nyp32f2v7efTTBMqnmEuG
@blarghmatey
fix(omnigraph): require an opt-in to unlock outside Kubernetes
c1463ca 
Outside Kubernetes the entrypoint has no Lease and no pod list, and it
still force-unlocked whatever `serve` or `deployment` lock it found. A
`docker run` whose cluster.yaml points at a served storage root would
have removed the live server's lock and applied over it.

It now refuses to force-unlock there unless
OMNIGRAPH_UNLOCK_WITHOUT_PEERS=1 is set. `cluster apply` always leaves
a lock, so a local run needs the variable on an empty root too.

Claude-Session: https://claude.ai/code/session_011nyp32f2v7efTTBMqnmEuG
@blarghmatey
blarghmatey marked this pull request as ready for review October 12, 2026 23:24
Copilot AI balanced review requested due to automatic review settings October 12, 2026 23:24
Copilot AI reviewed Oct 12, 2026
View reviewed changes

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.

🔵 Needs a closer look

The safety-critical Kubernetes client lacks direct coverage, and deployment still depends on the 0.13 pin and unfinished infrastructure configuration.

1 open finding
What changed in this PR

Replaces the Omnigraph server’s shell bootstrap with a Kubernetes-aware Python entrypoint for safe lock handoff during the 0.13 upgrade.

Changes:

  • Adds Lease-based coordination, peer detection, guarded planning/apply, and lock release.
  • Adds extensive entrypoint tests and CI coverage.
  • Updates policy fixtures and documentation for Omnigraph 0.13.
File Description
docker/​omnigraph_server_entrypoint.py Implements coordinated boot and lock handoff.
docker/​omnigraph-server-entrypoint.sh Removes the legacy shell bootstrap.
docker/​omnigraph-server.Dockerfile Installs and invokes the Python entrypoint.
docker/​README.md Documents boot behavior and environment variables.
.github/​workflows/​witan-tests.yml Triggers tests for entrypoint changes.
.gitignore Updates cluster-state generation notes.
mcp/​servers/​witan/​tests/​test_omnigraph_server_entrypoint.py Tests Lease, planning, locking, and real-binary behavior.
mcp/​servers/​witan/​policy/​check.sh Adopts the 0.13 direct-apply workflow.
mcp/​servers/​witan/​policy/​cluster.yaml Renames the fixture graph.
mcp/​servers/​witan/​policy/​code-graph.policy.yaml Updates the fixture graph reference.
mcp/​servers/​witan/​policy/​tests/​code-graph.tests.yaml Updates the policy-test command.
mcp/​servers/​witan/​policy/​schema/​stub.pg Updates fixture documentation.
mcp/​servers/​witan/​policy/​render_groups.py References the new entrypoint.
mcp/​servers/​witan/​policy/​README.md Documents renamed fixtures and entrypoint.
mcp/​servers/​witan/​docs/​adr/​0002-witan-cedar-authorization-bundle.md Updates the ADR fixture name.
docs/​explanation/​decisions/​0002-witan-cedar-authorization-bundle.md Updates the mirrored decision document.

🧠 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 docker/omnigraph_server_entrypoint.py
self.status = status


class KubernetesApi:
This was referenced Oct 13, 2026
Merged
Open

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@blarghmatey