Skip to content

Give Dgraph's only pod a disruption budget, so node scale-down can't delete it - #4076

Merged
saadahmed95 merged 2 commits into
mainfrom
fix/4066-dgraph-disruption-budget
Nov 1, 2026
Merged

saadahmed95 merged 2 commits into
mainfrom
fix/4066-dgraph-disruption-budget

Conversation

@saadahmed95

Copy link
Copy Markdown
Contributor

Why

Refs #4066. I'll close it once this is applied, because merging alone changes nothing on the cluster.

lore-dgraph-alpha is the one stateful service on the cluster without a PodDisruptionBudget: lore-api, lore-db-primary, lore-stations, lore-ui and utopia-db-primary all have one at minAvailable: 1. Node consolidation deleted its only pod 4 times in the 30 days to 2026-11-01. Twice its disk stayed attached to the old node, and the graph was down for 38m30s and 42m42s, failing ingests and impact reads across repos.

What Changed

  • infra/terraform/lore-dgraph.tf: a PodDisruptionBudget of minAvailable: 1 selecting the Dgraph pod, as a kubectl_manifest like the Service and StatefulSet beside it.
  • libs/shared/src/domain/infra-contract/security-posture.test.ts: a test that the budget exists, holds minAvailable: 1 and selects that pod, appended so no cited line moves.
  • specs/1-lore-platform/spec.md: an NFR-2 statement citing that test.

It needs a manual apply. CI never runs terraform (docs/managing-secrets.md), so someone with access to Lore's terraform state has to run this after merging:

cd infra/terraform && terraform apply -target=kubectl_manifest.lore_dgraph_disruption_budget

It doesn't restart Dgraph, because it adds a new object and leaves the StatefulSet alone.

Alternatives Considered

  • The cluster-autoscaler.kubernetes.io/safe-to-evict: "false" annotation. It changes the pod template, so applying it restarts Dgraph and risks the same slow disk attach this is meant to prevent.
  • Doing nothing, as headlamp.tf chooses for a dashboard. That file accepts an occasional restart because the dashboard is stateless. Here a restart can cost 40 minutes.
  • Running Dgraph with more replicas. That's the real fix for availability, but lore-dgraph.tf already defers it as a split zero/alpha HA setup, and it's a much bigger change.
  • kubernetes_pod_disruption_budget_v1. It would work too, but this file declares its workloads as kubectl_manifest.

The cost: the node holding this pod won't be consolidated, and a node upgrade that reaches it waits up to an hour before GKE evicts the pod anyway, so an upgrade can still cause one slow restart. Autopilot bills pod requests rather than nodes, as headlamp.tf notes, so holding the node shouldn't add cost.

ADRs & Architecture

None. This follows what lore-api's values file already does for the same reason.

Testing

  • Against the cluster, without persisting anything: kubectl apply --dry-run=server accepts this exact manifest, its selector matches the one pod lore-dgraph-alpha-0, and the namespace holds no budget today.

  • Test: LORE_SKIP_DGRAPH_TESTS=1 npx vitest run src/domain/infra-contract/ in libs/shared passes, and with minAvailable set to 0 the new test fails.

  • Terraform: terraform fmt -check passes. terraform validate wasn't run locally, because the terraform-validate CI job does it on this PR.

  • GKE's documented behaviour, not tested here: scale-down is blocked when a node's deletion would violate a budget, and upgrades honour one for up to an hour, then evict.

  • After applying: kubectl -n lore-dgraph get pdb should show ALLOWED DISRUPTIONS 0, and ScaleDown events on this pod should stop.

  • Lint passes (npx eslint on the changed test)

  • Types check (npx tsc --noEmit -p tsconfig.json in libs/shared)

  • Tests pass for changed packages (src/domain/infra-contract/ in libs/shared)

  • No secrets, tokens, API keys, or credentials in the diff

  • Migrations are idempotent (N/A, no ui-helm/migrations/ change)

  • Commit trailers present if this is a Lore-managed branch (N/A, a human branch)

  • ADR created or updated if an architectural decision was made (N/A)

🤖 Generated with Claude Code

Node consolidation deleted the pod 4 times in 30 days, and twice its disk
stayed attached to the old node for about 40 minutes. A budget stops the
autoscaler removing that node, and every other stateful service on the
cluster already has one. A node upgrade still finishes, because GKE honours
the budget for an hour.

Refs #4066

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@saadahmed95 saadahmed95 self-assigned this Nov 1, 2026
@lore-agent

lore-agent Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Lore is reviewing this PR — 7f45050d-605d-4387-82c2-39d61dcecdbf.

Submit a review that requests changes to have Lore push a fix for it. Comment @lore review to re-run the review.

@lore-agent

lore-agent Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Nothing blocks this PR from merging now. Lore updates this comment when the reason changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

🔍 Lore Spec Impact — advisory

This PR touches 5 statement(s) across 2 spec(s), and changes the validating tests alongside every one of them.

Weaker signals (5) — linked by a spec, not proven by a test run

Project Constitution · 2 statement(s)

.specify/memory/constitution.md

Principle 2: Zero Stored Credentials
✓ this PR also changes the tests that validate it

This is a hard requirement with no exceptions.

validated by libs/shared/src/domain/infra-contract/security-posture.test.ts:96
via changed file libs/shared/src/domain/infra-contract/security-posture.test.ts

Principle 2: Zero Stored Credentials
✓ this PR also changes the tests that validate it

GitHub Actions: Workload Identity Federation.

validated by libs/shared/src/domain/infra-contract/security-posture.test.ts:96
via changed file libs/shared/src/domain/infra-contract/security-posture.test.ts

Feature Specification: Lore — Shared Context Infrastructure · 3 statement(s)

specs/1-lore-platform/spec.md

NFR-1: Security
✓ this PR also changes the tests that validate it

No long-lived credentials anywhere in the system.

validated by libs/shared/src/domain/infra-contract/security-posture.test.ts:96
via changed file libs/shared/src/domain/infra-contract/security-posture.test.ts

NFR-1: Security
✓ this PR also changes the tests that validate it

Workload Identity for all GKE workloads.

validated by libs/shared/src/domain/infra-contract/security-posture.test.ts:96
via changed file libs/shared/src/domain/infra-contract/security-posture.test.ts

NFR-1: Security
✓ this PR also changes the tests that validate it

Workload Identity Federation for GitHub Actions.

validated by libs/shared/src/domain/infra-contract/security-posture.test.ts:96
via changed file libs/shared/src/domain/infra-contract/security-posture.test.ts

1 new statement(s) have no test link yet.

Deterministic · graph @ 242f133 (projected 2026-10-31) · 1 file(s) skipped: changed since the graph last saw them, so their line numbers no longer line up · no tests run by this check

@saadahmed95
saadahmed95 merged commit 0b4eb40 into main Nov 1, 2026
38 checks passed
@saadahmed95
saadahmed95 deleted the fix/4066-dgraph-disruption-budget branch November 1, 2026 20:54
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.

1 participant