feat(spanner): KPEP-0001 backend with full storage.Interface conformance + TTL#3
Merged
Conversation
Adds the registry/ subpackage hosting the Backend interface, Factory
type, and Backends instance-scoped registry. This is the first
implementation step for KPEP-0001 (pluggable storage backends).
The package is intentionally additive — nothing in the rest of
kplane-dev/storage imports it, so existing consumers (the apiserver's
StorageWithClusterIdentity decorator path) continue to work unchanged.
The wiring lands in later PRs:
- kplane-dev/spanner adds a Register() that satisfies registry.Backend.
- kplane-dev/apiserver constructs *Backends in main and dispatches via
--storage-backend (alongside the existing hardcoded if/else for
one release, then the hardcoded branch is removed).
The Factory signature matches upstream storagebackend/factory.Create
exactly so existing backend implementations (the BackendFactory type
that kplane-dev/spanner already exposes) plug in without adaptation.
Tested: go test ./registry/... covers Register/Get/Names/AddFlags
fan-out + duplicate-panic behavior. Existing tests (e2e_test.go,
identity_test.go, keylayout_test.go) unaffected since nothing in those
paths references the new package.
See KPEP-0001 in kplane-dev/enhancements for the design rationale.
Adds the seam the apiserver's RESTOptionsDecorator needs to install a registry-selected backend in place of the upstream etcd3 path. When DecoratorConfig.BackendFactory is non-nil, StorageWithClusterIdentity calls it instead of generic.NewRawStorage; the cacher wrapping above is unchanged. The BackendFactory type is declared at the top level (mirror of registry.Factory) so consumers of DecoratorConfig don't need a transitive import of the registry subpackage. Signature is 1-to-1 with upstream factory.Create — any backend already at that shape (etcd3, Spanner, future postgres) plugs in without adaptation. Nil preserves the pre-registry behavior so callers that haven't switched yet continue to hit etcd3 unchanged.
That fork branch is the consolidated source of two prerequisite features
for KPEP-0001:
- 32f5e9075db: move DecodeCallback to storage package (lets non-etcd
backends like Spanner honor cacher-installed decode callbacks).
- 8744b93de42: per-cluster service allocator support (apiserver's
multi-cluster bootstrap needs this when it threads BackendFactory
through DecoratorConfig).
Both consumers downstream of this PR (kplane-dev/spanner and
kplane-dev/apiserver) pin against the same fork commit so the chain
agrees on which symbols exist. When feat/per-cluster-allocators
eventually merges, every consumer moves to the resulting main SHA in a
follow-up bump.
…P-0001)
Brings the Spanner backend into kplane-dev/storage as the first in-tree
implementation, matching the KPEP-0001 end-state layout:
kplane-dev/storage/
registry/ (added in previous commits)
decorator.go BackendFactory = registry.Factory alias
backends/
register.go RegisterBuiltin(b) aggregator
spanner/
broadcast.go, config.go,
factory.go, register.go,
store.go, watcher.go,
store_test.go, register_test.go
What this collapses:
- Drops the standalone kplane-dev/spanner repo entirely. It existed only
because BackendFactory needed to be declared somewhere without
importing kplane-dev/storage (the 'avoid an import cycle' comment in
the old factory.go). Now that Spanner is a subpackage of
kplane-dev/storage, it references storage.BackendFactory directly.
- BackendFactory is now a type alias to registry.Factory, so the
apiserver can pass registry.Backend.Build()'s return value into
DecoratorConfig without a conversion.
- The aggregator (backends/register.go) is one import + one Register()
call per backend. Adding postgres or kine in the future means editing
one file here, not the apiserver.
Tested:
- go test ./registry/... ./backends/... clean.
- Spanner emulator-backed tests pass.
- decorator.go alias compiles cleanly; the BackendFactory hook path in
StorageWithClusterIdentity is unchanged.
Follow-up: archive kplane-dev/spanner (or leave as a one-release re-export
shim). kplane-dev/spanner#1 will be closed since its work is now here.
See KPEP-0001 in kplane-dev/enhancements for the design.
….0 requirement) Auto-bumped by go mod tidy after pulling in Spanner deps from the backends/spanner migration. Local dev needs Go 1.25.8 (or auto-toolchain download). CI bumps will follow.
Wires EnsureSchema into Options.Build() so a fresh Spanner backend doesn't require an out-of-band schema-apply step. EnsureSchema is now idempotent (gRPC AlreadyExists is treated as success) so the call is safe on every apiserver startup, not just first-run. Adds a 30s timeout context so a misconfigured emulator endpoint can't hang apiserver startup indefinitely. Operator UX is now: ./kplane-apiserver --storage-backend=spanner --spanner-* ... instead of: go run ./cmd/spanner-schema --... # one-time ./kplane-apiserver --storage-backend=spanner --spanner-* ... KPEP-0001 local e2e recipe in kplane-dev/apiserver no longer needs the 'apply the schema manually' caveat — Build() handles it.
Adds BuildFactoryBackend() to registry.Backend so the apiserver can plug the same Spanner backend into the upstream factory.Register hook. - registry.Backend gains BuildFactoryBackend() (factory.Backend, error) - spanner.FactoryBackend implements factory.Backend over a shared spanner.Client: Create dispatches to the existing NewStore; the four health/ready/prober/monitor methods are thin SELECT-1 probes. - spanner.Options.Build and .BuildFactoryBackend share one FactoryBackend (one client) per process so CR storage and internal-state callsites (master/peer endpoint leases, service IP/NodePort allocators) reuse the same Spanner session pool. Also lands two store fixes uncovered by exercising the internal-state callers: - store.go: tolerate nil newFunc in GuaranteedUpdate/Delete/GetList by reflecting on the destination (or list element type) the same way upstream's etcd3 store does. serviceallocator.NewEtcd and reconcilers.NewLeases both pass nil for newFunc and previously panicked. - store.go: trim a leading "/" off keys before joining with pathPrefix in prepareKey, and trim the same in Stats's STARTS_WITH query. Previously produced "/registry//apiregistration..." (double slash) at the join between pathPrefix and the cluster-rewritten key. Refs: KPEP-0001
…rage package) Picks up github.com/kplane-dev/kubernetes#5, which relocates DecodeCallback to the storage package and adds SetFeatureSupported. Required for the Spanner backend's decode-callback wiring and feature-checker registration to compile against origin/main.
…+ ordered watch) Brings the Spanner backend to 24/24 upstream storage.Interface conformance and adds TTL eviction. Three independently-motivated changes that overlap in the same files and ship as one bundle. ### Broadcaster — Ticket+heap dispatcher (broadcast.go) Replaces the channel-fanout broadcaster with a per-write Ticket abstraction plus a single-goroutine heap dispatcher. Why: Spanner commit timestamps come from TrueTime and assigned in commit order, but concurrent Apply() calls return out-of-order at the client. Publishing in arrival order produced non-monotonic event RVs to watchers, violating the cacher watchCache's binary-search invariant (watch_cache.go:982). How: AcquireWrite() reserves a Ticket BEFORE Apply; defer Cancel covers every error path; Publish on success enqueues the event in a min-heap. The dispatcher waits until pendingTickets==0 before flushing, at which point TrueTime guarantees no future Apply can produce an RV below anything buffered. Tested in broadcast_order_test.go (concurrent-write monotonicity) and cacher_list_test.go (cacher integration). ### storage.Interface conformance (store.go, watcher.go, conformance_test.go) Brings the backend to the upstream RunTest* contract. - storageKeyFromSpannerKey: preserve leading slash (was stripping '/registry/' AND the leading '/', so the cacher's ListPrefix lookup missed every item it indexed — the load-bearing bug behind 'empty LIST' - Get + GetList: guard too-high RVs with TooLargeResourceVersionError instead of letting Spanner return DeadlineExceeded - Watch: honor ctx.Done() in both the pre-loop and streaming-loop paths - Watch: for legacy RV=0/empty (SendInitialEvents nil), send initial events as ADDED to match etcd's behavior; skip the trailing bookmark (which belongs to the explicit WatchList contract) - Watch: advance startRev to max(initialItemRVs) for the legacy path (advancing to bookmarkRV races concurrent writes that committed just before GetCurrentResourceVersion and silently filters them out) - GetList: ensure non-nil empty slice when predicate doesn't match - GetList: one resolveListRV() helper for the three callsites that need a guaranteed-nonzero list RV; returns an explicit error instead of silently substituting 0 - conformance_test.go: wraps the upstream RunTest* battery with Pod codec, /pods/ prefix, identity transformer Existing watch tests updated to pass the post-create RV — the previous expectation (no initial events on a watch with no RV specified) only held because of the bug being fixed. ### TTL eviction — three independent layers (config.go, store.go, broadcast.go) - schema: expire_at TIMESTAMP, kv_by_expire_at STORING index, ROW DELETION POLICY (OLDER_THAN(expire_at, INTERVAL 0 DAY)) - write paths: Create / GuaranteedUpdate compute expire_at client-side when ttl > 0 - read filter: Get / GetList SELECTs include (expire_at IS NULL OR expire_at > CURRENT_TIMESTAMP()) so expired rows are invisible to reads even before Spanner's row deletion policy physically removes them - watch emission: per-broadcaster TTL scanner publishes synthetic Delete events for newly-expired rows. CAS-verifies under a write Ticket before publishing (closes the refresh race where a concurrent Update refreshes expire_at between the scan and the publish). Per-replica scanner, no cross-replica coordination needed — broadcasters are disjoint and watchers are local. Same shape from the watcher's POV as a real Delete: processEvent decodes prevValue, emits watch.Deleted. ### Test results - 24/24 upstream conformance tests pass (storagetesting.RunTest* battery) - All existing backend tests pass
4 tasks
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.
Summary
Stands up the in-tree Spanner backend for KPEP-0001 (pluggable storage backends) and brings it to a production-ready state: 24/24 upstream
storage.Interfaceconformance, ordered watch delivery, and a three-layer TTL eviction design.Depends on kplane-dev/kubernetes#4 (factory.Backend registry seam) and #5 (DecodeCallback at storage package), both merged.
What lands
Phase 1 — KPEP-0001 backend wiring (earlier commits on this branch)
registry/registry.go) with aBackendinterface andBuildFactoryBackend()hookbackends/spanner/) with the aggregator patternBackendFactoryhook onDecoratorConfigso the multicluster decorator dispatches through itfactory.Backendadapter (backends/spanner/factory_backend.go) so a single registration covers everyfactory.Createcallsite in the apiserver — CR storage, master/peer endpoint leases, service IP/NodePort allocatorsOptions.Build()auto-applies the schema on first use, so a fresh database doesn't require a separate schema-apply stepPhase 2 — Ticket+heap broadcaster (broadcast.go)
Replaces the channel-fanout broadcaster with a per-write
Ticketabstraction plus a single-goroutine heap dispatcher.Why: Spanner commit timestamps come from TrueTime and are assigned in commit order, but concurrent
Apply()calls return out-of-order at the client. Publishing in arrival order produced non-monotonic event RVs to watchers, violating the cacherwatchCache's binary-search invariant.How:
AcquireWrite()reserves a Ticket BEFORE Apply;defer Cancelcovers every error path;Publishon success enqueues the event in a min-heap. The dispatcher waits untilpendingTickets==0before flushing, at which point TrueTime guarantees no future Apply can produce an RV below anything buffered.Verified by
broadcast_order_test.go(concurrent-write monotonicity) andcacher_list_test.go(cacher integration).Phase 3 — storage.Interface conformance
Brings the backend to the upstream
RunTest*contract fromstaging/.../pkg/storage/testing. Hooked up inconformance_test.go.Substantive fixes:
storageKeyFromSpannerKey: preserve the leading slash. Was stripping/registry/AND the leading/, so the cacher'sListPrefixlookup missed every item it indexed — the load-bearing bug behind "empty LIST returns".TooLargeResourceVersionErrorinstead of letting Spanner returnDeadlineExceeded.ctx.Done()in both the pre-loop and streaming-loop paths.RV=0): send initial events as ADDED to match etcd'sareInitialEventsRequiredbehavior; skip the trailing bookmark (which belongs to the explicitWatchListcontract); advancestartRevtomax(initialItemRVs)(advancing tobookmarkRVraces concurrent writes that committed just beforeGetCurrentResourceVersionand silently filters them out).resolveListRV(): one helper for the three callsites that need a guaranteed-nonzero list RV — returns an explicit error instead of silently substituting 0.Existing watch tests updated to pass the post-create RV; the previous expectation (no initial events on a watch with no RV specified) only held because of the bug being fixed.
Phase 4 — TTL eviction (config.go, store.go, broadcast.go)
Three independent layers, each doing one thing well:
expire_at IS NULL OR expire_at > CURRENT_TIMESTAMP()predicate on every Get/List SELECT — reads never return expired rowsROW DELETION POLICY (OLDER_THAN(expire_at, INTERVAL 0 DAY))— Google-managed background deletion, zero client costPer-replica scanner; no cross-replica coordination needed because broadcasters are disjoint and watchers are local. From the watcher's POV, a synthetic Delete is identical to a real Delete — same path through
processEvent, sameprevValuedecoding, samewatch.Deletedemission.Test plan
go build ./...cleango test ./backends/spanner/ -run "^TestConformance_"— 24/24 PASS (upstreamRunTest*battery)go test ./backends/spanner/ -run "^Test[^C]"— all existing tests PASSWhy one PR
The four phases overlap heavily in
broadcast.go/store.go/watcher.go— splitting them would create PRs that individually break tests until the others land. Commits within the PR are organized by phase for review.