feat: wire the periodic reconciliation pass into the EtcdMirror engine#408
Open
xrl wants to merge 6 commits into
Open
feat: wire the periodic reconciliation pass into the EtcdMirror engine#408xrl wants to merge 6 commits into
xrl wants to merge 6 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xrl The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @xrl. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This was referenced Jul 7, 2026
xrl
force-pushed
the
pr/etcdmirror-reconciliation-pass
branch
from
4f3ad2c to
adcdde4
Compare
This was referenced Jul 8, 2026
Introduces the EtcdMirror CRD (schema only, no controller/agent) that will describe a continuous one-way key-range sync from a source etcd cluster to a target etcd cluster. Adds EtcdMirrorSpec/Status, the EtcdMirrorEndpoint/TLS/Auth/Sync/Checkpoint/Reconciliation sub-specs, phase and condition constants (including TargetThrottled and ReplicationLagExceeded), printer columns, and CEL XValidation rules for the destPrefix/noDestPrefix mutual exclusion, the endpointList/serviceRef oneOf, and the insecureSkipVerify/ insecureSkipVerifyAcknowledgeRisk companion-field requirement. Registers EtcdMirror/EtcdMirrorList in the scheme, regenerates zz_generated.deepcopy.go and the CRD manifest via controller-gen, and adds a realistic sample CR. CEL rules are covered by a table-driven envtest suite in etcdmirror_cel_test.go, following the tls_cel_test.go pattern, exercised against a real envtest apiserver. The design's Metrics field (reusing EtcdClusterSpec's MetricsSpec) is intentionally omitted: MetricsSpec does not exist on this branch yet and will land with pr/domain-metrics; it will be added in a later PR once that type is importable. No controller or agent logic in this PR -- types, generated code, and CRD schema only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Xavier Lange <xrlange@gmail.com>
Reshape the EtcdMirror API for the Design-3 engine (checkpoint stored in
the target etcd, stateless agent) per the 48-finding cross-cloud review.
Deletions:
- checkpoint.storageSpec (no PVC; checkpoint lives in the target at a
reserved fenced key)
- expectEmptyPrefix (superseded by initialSync.mode)
- noDestPrefix (redundant; one anchored rewrite formula
key' = target.prefix + destPrefix + TrimPrefix(key, source.prefix))
- syncInterval
Additions/renames:
- spec.mode: Sync|Drain; status.cutover{drainTargetRevision,
drainedRevision, verifiedTime, counts, leasedKeyCount}
- initialSync.mode: RequireEmpty|Overwrite|OverwriteAndPrune,
startRevision
- sync: requestTimeout, excludePrefixes, pageKeyLimit,
watchBufferBytes; batching flushes only at source-revision boundaries
- TLS: secretRef pointerized (nil = system trust roots), caBundleRef
- pod resources; configurable reserved checkpoint key
- watermark-derived lag/liveness semantics; frozen condition vocabulary
(TargetQuotaExhausted, ResyncLoopDetected, CutoverReady,
InvariantsHeld, LearnerEndpoint, Prefix/DirectionConflict) and Event
Reason constants as API contract
- status: source/target versions and cluster IDs, leaseBackedKeyCount,
always-on source/target key counts, initialSyncTotalKeyCount
- CEL: scheme-vs-TLS rules, prefix/rewrite immutability
- docs/etcdmirror.md (fidelity caveats, one-way contract) and
regenerated API reference
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Xavier Lange <xrlange@gmail.com>
Purpose-built replication engine, zero k8s dependency; replaces the
clientv3 mirror.Syncer plan.
Architecture: unpinned chunked scan + watch-replay from R0 (reflector
pattern) — the watch starts at the revision observed before the scan
and events are buffered/replayed over the scanned base, eliminating
mid-scan compaction as a failure class. Pull-based single-page scan
with byte-bounded pages keeps agent memory bounded; the source watch
is cancelled on sustained target backoff and resumed from checkpoint.
Fence protocol: checkpoint/watermark stored in the target etcd at a
reserved key (\x00-after-prefix convention, exact-match excluded from
scans/counts/prune/RequireEmpty), written in the same Txn as each
applied batch and guarded by a mod_revision compare on every write
path (applies, reconciliation repairs, prune deletes). Fence value
carries {linkUID, epoch, role}; role flips to Primary at cutover so
straggler applies fail their compare loudly. Undecodable checkpoints
fail closed; prune-pending is durable across restarts.
Batching: flush only at source-revision boundaries (whole revisions
coalesced, never split), ~1MiB byte watermark, one MaxTxnOps slot
reserved for the checkpoint write.
Error taxonomy: ErrCompacted classified distinctly;
rpctypes.ErrNoSpace -> TargetQuotaExhausted (permanent until operator
acts); oversized-Txn InvalidArgument and >2MiB client send-cap
ResourceExhausted both permanent with the offending key surfaced
redacted; throttle-distinct backoff; N-consecutive-forced-resync
livelock detector.
Liveness: watermark-derived progress via WithProgressNotify +
WithRequireLeader, client-driven RequestProgress, gRPC keepalives on
both clients, per-unary-RPC deadlines (watch excluded). Version probe
at connect enforces the >=3.4 source floor and gates progress-notify
trust on 3.4.25/3.5.8. Anchored prefix rewrite via the single
documented formula. Lease-backed keys detected and counted
(kv.Lease != 0).
Tests: unit coverage per module plus an embedded-etcd integration
suite covering scan/tail convergence, fence overlap and epoch/role
rejection, compaction during scan and drain, forced-resync
mark-and-sweep, checkpoint resume, prune, and error classification.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Xavier Lange <xrlange@gmail.com>
A single post-heal put can race the recovery attempt's restart backoff and land below the scan's R0: the scan applies it, the live tail has nothing to deliver, and no live apply ever clears the latch. Write a fresh key per poll tick so one is guaranteed to arrive via the tail. Signed-off-by: Xavier Lange <xrlange@gmail.com>
Deadline-driven pass on the steady-state tail loop: consume arms a timer from a Run-goroutine-owned deadline and runs the diff-and-repair pass inline, so it can never overlap the genesis scan, a forced-resync sweep (which re-arms the deadline — it just produced the same signal), a drain (whose own verification supersedes it), or itself. The drain repair branch now records its drift, recordKeyCounts stamps the pass-completion time on every counts-producing pass — including the count-only drain verify — keeping the always-on counts contract behind InvariantsHeld, and Validate rejects negative intervals. Signed-off-by: Xavier Lange <xrlange@gmail.com>
- publish counts, pass timestamp, and drift in one Snapshot update inside reconcilePass; drop the torn caller-side recordDrift - cancel the live source watch once a periodic/drain repair pass queues more than a few seconds of MaxOpsPerSecond pacing, bounding clientv3's unbounded per-watcher buffer during a long healthy pass - correct the ReconcileInterval re-arm, key-count population, and maybeReconcile error-flow comments to match actual behavior - damage the target atomically (single Txn) in the drift test so a pass boundary cannot split the damages across two drifts Signed-off-by: Xavier Lange <xrlange@gmail.com>
xrl
force-pushed
the
pr/etcdmirror-reconciliation-pass
branch
from
adcdde4 to
43e41de
Compare
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.
Wires the periodic reconciliation pass into the EtcdMirror engine's steady-state loop, taking
Config.ReconcileInterval/Config.ReconcileDeleteOrphansfrom documented no-ops ("NOT YET WIRED") to live behavior. The diff-and-repair pass itself shipped with the engine (it already backs the forced-resync mark-and-sweep, the OverwriteAndPrune genesis pass, and the Drain verification); this PR promotes it to a schedulable, continuous integrity signal.Depends on: #407 (the
pkg/mirroragentengine — previous rung of this series).Scheduler design
Deadline-driven and single-goroutine: the steady-state
consumeloop gains a fourth select case on a timer armed from a persistentnextReconciledeadline (nil channel when disabled, so the case never fires). The pass runs inline on the Run goroutine withrepair=trueand orphan deletion gated onReconcileDeleteOrphans. Non-overlap is structural, not locked:consumeisn't running);Every counts-producing pass — periodic or mandatory — re-arms the deadline, so a forced-resync sweep pushes the next periodic pass out a full interval (it just produced the same signal). Errors escaping the pass ride the existing classification taxonomy untouched: transient/throttle are absorbed inside by the retry/backoff machinery,
ErrCompactedpropagates to the forced-resync arm, and permanent errors (PrefixConflictError,FenceError) fail the agent. Every repair and prune delete rides the same fenced mod-revision-compare Txn path as ordinary applies, so a concurrent fence bump aborts the pass loudly.Always-on key counts
The per-side key counts (
Snapshot.SourceKeyCount/TargetKeyCount) are the input to the controller'sInvariantsHeldcondition and must stay meaningful even when periodic reconciliation is disabled — sorecordKeyCountsnow stampsLastReconcileTimealongside the counts, and every mandatory pass (forced-resync sweep, OverwriteAndPrune, drain verification) dates them. Counts, timestamp, and drift are published in a single snapshot update so no reader can observe fresh counts with stale drift.Bounded memory during a paced repair
A long, healthy, rate-limited repair pass would previously let the source watch buffer grow for the duration of the pass (the existing watch-cancel fired only from backoff/quota branches, which a healthy pass never enters). The pass now cancels the source watch once queued repair work exceeds a few seconds' worth of
MaxOpsPerSecondpacing and resumes from the watermark through the normal transient re-watch path — the same bounded-retry posture the engine takes everywhere else. The genesis sweep keeps its watch: that one drains into the byte-bounded replay buffer by design.Validation and tests
Config.Validate()rejects a negativeReconcileInterval(0 = disabled). New unit tests cover the gating table (reconcileDue), deadline re-arm, validation, and the watch-cancel threshold; new embedded-etcd integration tests cover: injected divergence repaired by the periodic pass with drift reported; orphan policy in both report-only and delete modes; the pass never firing before steady state; a fence bump aborting the pass cleanly; mandatory passes keeping counts fresh with periodic reconciliation disabled; and a paced 100-key repair that forces a mid-pass watch cancel, then converges and still replicates fresh writes. The new tests are race-detector-clean over repeated runs.Verified with
make testandmake verify.PR series — operability fixes, TLS & EtcdMirror
Small single-purpose PRs from live kind-cluster testing of the operator. Each stands alone unless an After is listed. → = this PR.
events.k8s.ioRBAC so operator Events are actually recordedmembers[]/leaderIDfrom one health snapshot (consistent leader)altNames.ipAddressesinto certificatesvalidityDuration(365d,100d12h) as documentedDegradedcondition (was empty status)--max-concurrent-reconciles)podTemplate.specscheduling fields: topologySpread, resources, priorityClass, schedulerNameconfig/surfacespec.tls.{peer,client}surfaces (breaking alpha API)TLSReadycondition + TLS lifecycle EventsPeerCANotSharedEtcdMirrorCRD: one-way cross-cluster replication API (types + CEL)pkg/mirroragentreplication engine (fenced checkpoint-in-target)mirror-agentbinary: config/TLS-reload/statusz/metrics + kind e2e/metricsendpointEtcdBackupCR → object storage (S3/GCS)🟢 ready · ⚪ draft