feat: wire the EtcdMirror CRD, sample and end-user roles into kustomize#414
Open
xrl wants to merge 17 commits into
Open
feat: wire the EtcdMirror CRD, sample and end-user roles into kustomize#414xrl wants to merge 17 commits into
xrl wants to merge 17 commits into
Conversation
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>
…pshot JSON-tag Snapshot/CutoverStatus/Drift (lowerCamel, omitzero on times) as the /statusz wire contract, and add two additive fields the agent binary's metrics need: KeysAppliedTotal (data ops committed in fenced Txns) and ForcedResyncCountByReason (labeled-counter split of ForcedResyncCount). No engine semantics change; no field renames. Signed-off-by: Xavier Lange <xrlange@gmail.com>
cmd/mirror-agent wires pkg/mirroragent to real config, TLS and auth:
- Flags-only config (operator convention); secrets arrive as mounted-file
paths, never flag values or env. Quantities parse as resource.Quantity;
zero/unset selects engine defaults; the reconciliation enabled+interval
-> ReconcileInterval translation (1h default) lands here as assigned.
- TLS per side via transport.TLSInfo file paths: the client leaf pair is
re-read on every handshake, so certificate rotation needs no restart;
--<side>-ca-bundle-file wins over --<side>-ca-file (caBundleRef
precedence). Schemeless endpoints get the scheme implied by
--<side>-tls; a contradicting scheme is an error (the CEL rules'
binary mirror). RBAC auth reads username/password files once; the
token identity wins over the certificate CN.
- One listener serves /statusz (JSON of the tagged mirroragent.Snapshot
- the wire contract the controller decodes), /healthz (always 200),
/readyz (ready = past Connecting and not Failed; Degraded and Drained
are ready), /metrics (standalone Prometheus registry;
etcd_mirror_agent_* incl. tls_cert_expiry_timestamp_seconds{side,kind}
refreshed from the mounted files every 5m).
- On terminal engine states (drained / permanent failure) the process
lingers serving status instead of exiting: a crashloop would hide the
terminal state and re-run genesis under the same epoch. Exit 0 = clean
signal shutdown, 1 = startup or permanent engine failure.
- Packaging: both binaries in the one existing image (ENTRYPOINT stays
/manager; the rendered Deployment overrides command).
Signed-off-by: Xavier Lange <xrlange@gmail.com>
- Pre-initialize forced_resync_total at 0 for every known reason so rate()/increase() can see the first resync (a counter's first sample yields no delta); unknown future reasons still pass through. - Serialize Snapshot cluster IDs as JSON strings: random uint64s exceed 2^53 and round through float64-based consumers (jq, JavaScript), matching the checkpoint's existing convention. - Reject TLS material flags (cert/key/ca/bundle/server-name/ insecure-skip-verify) without --<side>-tls instead of silently dialing cleartext; gate the cert-expiry gauge on tls too. - Fail startup on empty credential files: clientv3 silently skips authentication unless both username and password are non-empty. - Don't log "engine failed permanently" (or linger) when Run's return raced a shutdown signal; classify cancellation once in one place. - Restore default signal disposition after the first signal so a second SIGINT/SIGTERM kills a wedged shutdown, like the manager's ctrl.SetupSignalHandler. - Make the live test's timeout diagnostic read the snapshot after polling (require.Eventually copies msgAndArgs before it). - Drop PR-ladder "rung" vocabulary from doc comments. Signed-off-by: Xavier Lange <xrlange@gmail.com>
Two size-1 EtcdClusters plus a mirror-agent pod wired source->target in a dedicated namespace. Proves put/delete replication within a polled 10s ceiling (250ms interval, convergence time logged) and that /statusz (keysAppliedTotal, watermark, lastProgressTime) and the /metrics keys_applied_total counter advance past baseline. The agent image is distroless, so statusz/metrics are reached via the API-server pod proxy. Signed-off-by: Xavier Lange <xrlange@gmail.com>
status.scanRestartCount is the persisted high-water mark the controller dedups InitialSyncCompactionRaced events against (forcedResyncCount precedent). CertificateExpiringSoon / InsecureSkipVerifyEnabled are the controller-side Warning events promised by the certexpiry gauge and the EtcdMirrorTLS.InsecureSkipVerify contract. ReplicationLagExceeded's threshold constants live controller-side, not in the agent — the doc comment now says so. Signed-off-by: Xavier Lange <xrlange@gmail.com>
…shot ReasonFor is the exported accessor form of Classify's errors.As ladder: Snapshot.LastErrorReason gives the controller typed Failed reasons (UnsupportedVersion, CheckpointInvalid, PrefixConflict, ...) without string-matching error messages. SourceLearner/TargetLearner surface IsLearner from the connect-time Status probe — previously dropped — as the LearnerEndpoint condition's input. Signed-off-by: Xavier Lange <xrlange@gmail.com>
Reconcile each EtcdMirror into a size-1 Recreate Deployment running /mirror-agent (command overrides the /manager ENTRYPOINT; image from the new --mirror-agent-image manager flag). Args derive deterministically from the spec: --link-uid=UID, --epoch=generation, TLS Secrets mounted directly for in-place rotation, auth Secrets hash-rolled via a resourceVersion pod annotation, serviceRef named ports resolved to numeric dial endpoints. Status is fed by polling the agent's /statusz through an injectable AgentStatusClient seam (envtest has no kubelet, so tests fake it): snapshot fields map to phase/watermark/versions/hex cluster IDs/cutover, and to the condition contract — Available, ReplicationLagExceeded (5000 revs sustained 5m, controller-internal), Compacted/ForcedResync, TargetThrottled, TargetQuotaExhausted, ResyncLoopDetected, DriftDetected, InitialSyncComplete, EmptyTargetViolation (message carries the exact etcdctl del range), CutoverReady, LearnerEndpoint, and InvariantsHeld (lag ok + key counts equal + no drift + verification pass fresh within 2x the reconciliation interval). Unreachable or undecodable /statusz degrades with a reason instead of crashing; stale fields are retained and LastStatusSyncTime is not advanced. Guards stop the newer CR (Pending, replicas 0) on PrefixConflict (overlapping effective destination ranges on the same target cluster; runtime cluster IDs authoritative over spec identity) and DirectionConflict (inverted source/target cluster-ID or spec pairs). Events use the API constants with persisted-status dedup ledgers: ForcedResyncStarted/Completed, CheckpointInvalidated, InitialSyncCompactionRaced, EmptyTargetViolation, cert-expiry lead window (14d, 12h damped) and InsecureSkipVerifyEnabled. The operator.etcd.io/checkpoint-cleanup finalizer deletes the reserved checkpoint key from the target via a seamed short-lived client after the agent pods are gone; bounded-backoff retries forever, with the skip-checkpoint-cleanup annotation as the documented escape hatch. Credential reads follow the objectstore never-log pattern in mirror_creds.go. RBAC regenerated from the controller markers; no NetworkPolicy (repo ships none controller-managed; the manager policy is opt-in under config/network-policy). Signed-off-by: Xavier Lange <xrlange@gmail.com>
Blocking: - Finalizer checkpoint delete is now ownership-checked: decode the stored fence, delete (mod-revision-guarded Txn) only when LinkUID matches the CR; foreign or undecodable fences are left in place with a Normal event (deleting a PrefixConflict loser previously destroyed the winner's live fence via the shared default key). - Filter self-inflicted watch events (generation/annotation predicates): every status write re-enqueued the CR, turning the 15s poll cadence into a hot loop. - InitialSyncComplete and the initialSync*/lastReconciliation/lastProgress fields no longer regress when the agent pod restarts (resume-from- checkpoint never sets the process-local scan fields); the condition resets only on checkpoint invalidation (ClusterIDMismatch resync or CheckpointInvalid), keeping it durable across Compacted forced resyncs. Important: - Poll failure retains the prior phase instead of fabricating Degraded (reserved for the agent's own backoff loop). - Cluster IDs are latched: a Connecting snapshot's zero IDs no longer blank the persisted identities and disarm the runtime guards. - forcedResyncCount/scanRestartCount rebase onto persisted status across pod restarts (monotonic, never reset) via a per-pod base ledger. Minor: - types.go names the drained carve-out (Available=True/DrainComplete, phase stays Syncing) in the Available and Syncing contracts. - Guards evaluate both conditions every reconcile (stale DirectionConflict cleared for prefix losers); the prefix winner's False honestly names the parked sibling (reason ConflictWinner). - Intra-cluster (source==target) mirrors are excluded from the direction guard: cluster-level inversion is trivially true yet no loop exists. - warnedAt/counter ledger entries are pruned on finalizer removal; status.cutover clears once spec.mode leaves Drain. - Spec warnings (cert expiry, insecureSkipVerify) emit on every reconcile, not only after a successful poll. - Validation failure with an existing Deployment retains the prior phase (Pending claims the workload does not exist) and says the agent keeps running on mounted material. - Agent clears LastErrorReason alongside LastError/LastErrorClass on a successful fenced commit. Signed-off-by: Xavier Lange <xrlange@gmail.com>
|
[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 8, 2026
This was referenced Jul 8, 2026
…8s.io RBAC Secret volumes mounted 0400 are root-owned and unreadable by the distroless agent user (uid 65532): the pod now sets fsGroup 65532 and mounts at 0440. The manager's recorder emits events.k8s.io/v1 Events, which the core-group grant alone silently drops on a real apiserver. Both found by the TLS and forced-resync kind e2e scenarios; neither is visible in envtest. Signed-off-by: Xavier Lange <xrlange@gmail.com>
Signed-off-by: Xavier Lange <xrlange@gmail.com>
Kubebuilder scaffolding (create webhook, edit) refuses or mis-scaffolds for a kind absent from the project config. Signed-off-by: Xavier Lange <xrlange@gmail.com>
xrl
force-pushed
the
pr/etcdmirror-rbac-kustomize
branch
from
bf8eea2 to
9a1909f
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 EtcdMirror feature into the deployment surface: the CRD base joins
config/crd/kustomization.yaml, the sample joinsconfig/samples, and end-useretcdmirror-editor-role/etcdmirror-viewer-roleaggregation roles land inconfig/rbac(byte-for-byte models of the existing etcdcluster editor/viewer roles). Also records the EtcdMirror resource inPROJECT.Depends on: #413 (EtcdMirror controller — previous rung of this series).
Two deliberate omissions, with the reasoning: the controller's own
role.yamlrules already landed with #413's kubebuilder markers, so no RBAC regeneration is needed here; and there is no agent ServiceAccount — the mirror-agent Deployment runs on the namespace default ServiceAccount withautomountServiceAccountToken: falseand needs zero Kubernetes API access, so installing an SA/Role/RoleBinding for it would be pure surface (now stated indocs/etcdmirror.mdprerequisites). A static SA underconfig/rbacwould also land in the operator's namespace while agent Deployments live in each CR's namespace.Note for reviewers: with the CRD wired into
config/crd,make install(used by the e2e suite) now installs the EtcdMirror CRD.Verified with
make test,make verify, andkustomize buildoverconfig/defaultandconfig/samples(CRD, both roles, and the sample all present in the rendered output;make manifests generateproduces no drift).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