Skip to content

feat(kv-cache): elect Mooncake leaders with Kubernetes Lease by default - #697

Merged
thxCode merged 2 commits into
mainfrom
feat/mooncake-default-lease-election
Sep 29, 2026
Merged

thxCode merged 2 commits into
mainfrom
feat/mooncake-default-lease-election

Conversation

@thxCode

@thxCode thxCode commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind enhancement
/kind api-change
/area worker

What this PR does / why we need it:

  • Add leader.electionBackend with Kubernetes as the default and None for single-replica images without Lease support. Reject None when leader.replicas > 1.
  • Run the Kubernetes Lease election at one replica so scaling an already elected leader adds standbys without replacing the first leader Pod or rolling members. Keep the guarded two-step transition for a live unelected template.
  • Remove the unused leader.highAvailability wrapper and expose its only setting as leader.memberAddressing. Drop the obsolete unknown snapshot field checks; retain the snapshot flag safety checks. The API is unreleased, so no migration is required.
  • Align leader RBAC, flags, member addressing, status, webhook validation, generated API, unit tests, e2e cases, and KV cache documentation with the new default.
  • Document that leader restart loses DRAM-only key metadata, while disk key recovery depends on successful re-registration. An observed service gap does not bound the first hit time of every disk key.

Which issue(s) this PR links to:

NONE

Special notes for your reviewer:

  • A Mooncake image without the Kubernetes Lease backend requires leader.electionBackend: None when creating a single-replica backend. The API has not been released yet, so this PR has no migration requirement for existing CR objects.
  • Validation: RACE=false make test, make lint, make lint docs, make lint agents-shell, make generate, and git diff --check.
  • The default race-enabled make test found an independently reproducible race in the unchanged pkg/utils/certs/cache test Test_k8sCache_TamperedSecretIsLoggedAsKeyValuePairs: its cleanup clears the global klog logger while its informer is still running. This package is outside the PR scope.
  • Cluster e2e was not run; the updated e2e scripts passed the repository's shell lint gate.
  • The local .claude/reports/llm-pd-disaggregation-kvcache-cr-design/v0.9-guide.md was updated but is ignored by Git and is outside this PR.

Does this PR introduce a user-facing change?

Kubernetes Lease election now runs by default even with one Mooncake leader replica, so later scale-out does not restart the first leader solely to enable election. Images without Lease support can use leader.electionBackend: None for a single-replica backend.

@gpustack-code-review

gpustack-code-review Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

✅ OpenCodeReview: Review complete: 0 finding(s) across 15 selected item(s).

@thxCode

thxCode commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

Addressed all three findings in the review summary:

  • Wrapped the ElectionObserved and TierWasEmpty sentence in docs/kv-cache/backend.md. make lint docs passes.
  • Documented that electionBackend: None makes members use the Service even when memberAddressing: Lease is set, in both the API field and the leader guide. A focused test verifies that rendered address; the Mooncake and API package tests pass.
  • Kept protobuf field number 2. This API is unreleased, and the Kubernetes CR is stored as JSON, so there is no released protobuf wire contract to migrate. make generate passed and produced only the expected documentation changes in generated artifacts.

make lint, git diff --check, and the commit lint also pass. The review changes were folded into their owning commits because the commit lint rejects fixup! subjects.

@thxCode
thxCode force-pushed the feat/mooncake-default-lease-election branch from 262896d to 23f6853 Compare September 29, 2026 04:38
@thxCode

thxCode commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up on the two findings in the latest review summary:

  • The project owner confirmed that this API has not been released, so this PR intentionally replaces leader.highAvailability without a migration path. No versioned release tag in this checkout contains the commit that introduced the field. The generated CRD, OpenAPI, proto, and apply-configuration changes are included; the PR's verify-generated check passed on the previous head.
  • Shortened the walkthrough's repeated election-switch explanation to a link to the leader page, which owns the restart and DRAM cache details. make lint docs passes.

make generate after folding the documentation change left a clean tree, and the commit lint passes. No inline threads were created for these summary findings.

@thxCode
thxCode force-pushed the feat/mooncake-default-lease-election branch from 23f6853 to 4a64304 Compare September 29, 2026 04:48
@thxCode
thxCode merged commit 180cfbc into main Sep 29, 2026
11 checks passed
@thxCode
thxCode deleted the feat/mooncake-default-lease-election branch September 29, 2026 05:12
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