feat(register): expose Backend wrapper for registry (KPEP-0001 phase 2)#1
Closed
zachsmith1 wants to merge 2 commits into
Closed
feat(register): expose Backend wrapper for registry (KPEP-0001 phase 2)#1zachsmith1 wants to merge 2 commits into
zachsmith1 wants to merge 2 commits into
Conversation
…y (KPEP-0001 phase 2) Adds register.go: an Options struct that implements registry.Backend so the kplane apiserver can register Spanner against a *registry.Backends in its Phase 3 dispatch swap, without the apiserver source needing to know anything Spanner-specific. The wrapper is thin: NewOptions() returns an Options whose Build() calls the existing NewBackendFactory. Identical store/broadcaster/watcher behavior to the legacy 'if opts.SpannerProject != ""' branch in the apiserver, so the Phase 3 cutover is behavior-preserving. Bumps go.mod fork pin to the cherry-pick branch (kplane-dev/kubernetes#3) so 'storage.WithDecodeCallback' resolves at the storage package — that's the symbol watcher.go/store.go have been calling since they were written, but no merged fork commit exposed it publicly until the cherry-pick. Also adds the github.com/kplane-dev/storage require (registry/ subpackage) pinned to the kpep-0001/add-registry branch head. Once both prerequisite PRs merge, a follow-up bumps these pins to whichever main SHAs end up containing them. Tested: - go build ./... clean (was previously broken at HEAD). - go test -run 'TestOptions|TestAddFlags|TestValidate|TestRegister' ./... passes against the published storage branch. - Existing emulator-backed store tests still gated on SPANNER_EMULATOR_HOST. See KPEP-0001 in kplane-dev/enhancements for the design rationale, and the chain: - kplane-dev/kubernetes#3 (fork: move DecodeCallback to storage pkg) - kplane-dev/storage#2 (registry types) - this PR (Spanner Register wrapper) - kplane-dev/apiserver#? (TBD: dispatch swap)
3 tasks
…-0001 head Aligns with the consolidated KPEP-0001 fork branch. Both prerequisite features (DecodeCallback move + per-cluster allocators) live there; the former is what unblocks store.go/watcher.go from using storage.WithDecodeCallback. Apiserver Phase 3 will pin the same fork SHA.
zachsmith1
added a commit
to kplane-dev/storage
that referenced
this pull request
Jun 17, 2026
…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.
Contributor
Author
|
Closing. KPEP-0001 collapses kplane-dev/spanner into kplane-dev/storage/backends/spanner/ — see kplane-dev/storage#2 for the migrated source. The standalone repo's BackendFactory duplication ('to avoid an import cycle') goes away when Spanner lives as a subpackage of storage. |
zachsmith1
added a commit
to kplane-dev/storage
that referenced
this pull request
Jun 30, 2026
…nce + TTL (#3) * feat(registry): add storage backend registry (KPEP-0001 phase 1) 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. * feat(decorator): add BackendFactory hook on DecoratorConfig (KPEP-0001) 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. * chore: pin fork to feat/per-cluster-allocators (KPEP-0001 prereqs) 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. * feat(backends): migrate Spanner backend in-tree + add aggregator (KPEP-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. * chore: bump go directive to 1.25.8 (cloud.google.com/go/spanner v1.92.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. * feat(spanner): auto-apply schema in Options.Build() 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. * feat(spanner): add factory.Backend adapter (KPEP-0001 Option C) 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 * chore: bump kubernetes fork pin to merged K0.5 (DecodeCallback at storage 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. * feat(spanner): production-ready storage.Interface (conformance + TTL + 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 * docs: add storage package README and ignore coverage artifacts
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
1 participant
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
Phase 2 of KPEP-0001 (pluggable storage backends). Adds a thin `Options` struct that implements `registry.Backend` from `kplane-dev/storage/registry` so the kplane apiserver can register Spanner against a `*registry.Backends` without the apiserver source needing to know anything Spanner-specific.
What's in the package
Two go.mod bumps
This PR pins:
Once both prerequisite PRs merge, a follow-up bumps these pins to whichever `main` SHAs end up containing them.
Merge chain
This PR is the third link:
Test plan
See KPEP-0001 in kplane-dev/enhancements for the design rationale.