Repository navigation
[proposal] Per-cluster etcd: give each Kubernetes control plane its own datastore - #25
myasnikovdaniil wants to merge 2 commits into
Conversation
Fold etcd into the kubernetes app so each tenant Kubernetes cluster provisions and owns its own etcd and Kamaji DataStore. End state is one etcd per one Kubernetes cluster: consumers can start a cluster with no admin pre-step, control-plane state is physically isolated per cluster, and no etcd runs unless a cluster needs it. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
📝 WalkthroughWalkthroughA new design proposal document is added at ChangesPer-cluster etcd design proposal
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal to retire the shared tenant etcd module and bind a dedicated etcd cluster directly to each Kubernetes app, improving self-service, performance isolation, and security. The review feedback highlights two important considerations: first, the risk of DNS label length violations (exceeding 63 characters) if the release name is too long, and second, a potential race condition during deletion where the etcd datastore might be reaped before the KamajiControlPlane finishes finalization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| endpoints: | ||
| - {{ .Release.Name }}-etcd.{{ .Release.Namespace }}.svc:2379 | ||
| ``` |
There was a problem hiding this comment.
If .Release.Name is long, the service endpoint {{ .Release.Name }}-etcd might exceed the 63-character limit for Kubernetes Service names and DNS labels. To prevent deployment failures or DNS resolution issues, consider using a helper function (such as a truncated/hashed name helper) to format the etcd service name and endpoint, ensuring it strictly adheres to the 63-character limit.
|
|
||
| - **Two `Kubernetes` apps in one namespace** → each renders its own `<release>-etcd` and `<namespace>-<release>` DataStore; no name collision (the previous single shared `DataStore: <namespace>` could not represent two clusters distinctly). | ||
| - **DataStore name exceeds the cluster-scoped name length limit** → fall back to `<namespace>-<release>-<hash>`; the helper enforces the limit deterministically. | ||
| - **Cluster deletion** → the delete hook (`packages/apps/kubernetes/templates/delete.yaml`) must additionally delete the `EtcdCluster`, the cluster-scoped `DataStore`, and the per-cluster cert Secrets, *after* the existing `<release>-datastore-config` finalizer cleanup (delete.yaml already handles Kamaji's datastore-secret finalizer, issue #3062). Ordering: drain the control plane → strip the datastore-config finalizer → remove EtcdCluster + DataStore. |
There was a problem hiding this comment.
If the EtcdCluster or DataStore is deleted at the same time as the KamajiControlPlane (which is the default behavior when they are part of the same Helm release), a race condition can occur. If the etcd pods terminate before the KamajiControlPlane finishes its finalization, the Kamaji controller may fail to reconcile the control plane due to datastore connection timeouts, potentially blocking the deletion process or causing error storms in the operator logs. Consider implementing a mechanism (such as a finalizer on the DataStore or EtcdCluster that waits for the KamajiControlPlane to be gone, or explicit ordering in the deletion hook) to ensure the datastore is only reaped after the control plane has been completely deleted.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
design-proposals/etcd-per-cluster/README.md (2)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFuture date in a draft proposal.
The date
2026-06-30is at the end of the current month. If this is intentional as a target date, no action needed; if it's a placeholder, consider using the draft creation date or removing it until finalized.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/etcd-per-cluster/README.md` at line 5, The README draft currently uses a future placeholder date, so update the date field in the proposal header to a non-future value if this is meant to be a draft, or remove it until finalized. Locate the date entry in the proposal metadata section and replace the hardcoded `Date` value with the draft creation date or omit it entirely if the target date is not confirmed.
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider rephrasing "exactly" per style guidance.
Static analysis flagged this as overused. Alternatives: "precisely", "in effect", or restructure the sentence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design-proposals/etcd-per-cluster/README.md` at line 35, The sentence in the proposal body uses the overused word “exactly”; rephrase that line in the README text to avoid the flagged wording while keeping the same meaning. Update the prose near the per-cluster etcd description, using the existing surrounding sentence structure and terms like “per-cluster etcd” and “tenant-sprawl ceremony” to locate it, but replace “exactly” with a cleaner alternative such as “precisely” or “in effect.”Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design-proposals/etcd-per-cluster/README.md`:
- Line 190: The Phase 4 removal instructions use the wrong key name for the
legacy etcd namespace propagation, which can mislead implementers. Update the
wording in the README section describing removal of the shared-etcd plumbing to
refer to the exact propagated Secret key and helper name, using _namespace.etcd
and cozy-lib.ns-etcd consistently with the existing references in the document.
- Line 173: Clarify the delete hook behavior in the cluster deletion section:
the current delete flow in the delete hook should only remove the per-cluster
EtcdCluster, DataStore, and cert Secrets for new per-cluster etcd clusters, and
must skip those resources for legacy shared-etcd clusters. Update the guidance
around packages/apps/kubernetes/templates/delete.yaml to explain how the hook
determines the mode (for example, by checking for the per-cluster DataStore or a
dedicated annotation) so the legacy shared EtcdCluster/DataStore owned by the
tenant module is not deleted.
---
Nitpick comments:
In `@design-proposals/etcd-per-cluster/README.md`:
- Line 5: The README draft currently uses a future placeholder date, so update
the date field in the proposal header to a non-future value if this is meant to
be a draft, or remove it until finalized. Locate the date entry in the proposal
metadata section and replace the hardcoded `Date` value with the draft creation
date or omit it entirely if the target date is not confirmed.
- Line 35: The sentence in the proposal body uses the overused word “exactly”;
rephrase that line in the README text to avoid the flagged wording while keeping
the same meaning. Update the prose near the per-cluster etcd description, using
the existing surrounding sentence structure and terms like “per-cluster etcd”
and “tenant-sprawl ceremony” to locate it, but replace “exactly” with a cleaner
alternative such as “precisely” or “in effect.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d31dff8a-6a76-4ef4-9d32-ca60354f0c77
📒 Files selected for processing (1)
design-proposals/etcd-per-cluster/README.md
|
|
||
| - **Two `Kubernetes` apps in one namespace** → each renders its own `<release>-etcd` and `<namespace>-<release>` DataStore; no name collision (the previous single shared `DataStore: <namespace>` could not represent two clusters distinctly). | ||
| - **DataStore name exceeds the cluster-scoped name length limit** → fall back to `<namespace>-<release>-<hash>`; the helper enforces the limit deterministically. | ||
| - **Cluster deletion** → the delete hook (`packages/apps/kubernetes/templates/delete.yaml`) must additionally delete the `EtcdCluster`, the cluster-scoped `DataStore`, and the per-cluster cert Secrets, *after* the existing `<release>-datastore-config` finalizer cleanup (delete.yaml already handles Kamaji's datastore-secret finalizer, issue #3062). Ordering: drain the control plane → strip the datastore-config finalizer → remove EtcdCluster + DataStore. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Explicitly address delete hook behavior for legacy vs. new clusters.
The delete hook must distinguish between:
- Per-cluster etcd clusters: delete the
EtcdCluster,DataStore, and cert Secrets. - Legacy shared-etcd clusters: do not delete the shared
EtcdClusterorDataStore(owned by the tenant module/ancestor).
Without this distinction, deleting a legacy cluster could destroy the shared etcd and break all other clusters in the subtree. Add a sentence clarifying how the hook detects which mode applies (e.g., by the presence of the per-cluster DataStore or an annotation).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-proposals/etcd-per-cluster/README.md` at line 173, Clarify the delete
hook behavior in the cluster deletion section: the current delete flow in the
delete hook should only remove the per-cluster EtcdCluster, DataStore, and cert
Secrets for new per-cluster etcd clusters, and must skip those resources for
legacy shared-etcd clusters. Update the guidance around
packages/apps/kubernetes/templates/delete.yaml to explain how the hook
determines the mode (for example, by checking for the per-cluster DataStore or a
dedicated annotation) so the legacy shared EtcdCluster/DataStore owned by the
tenant module is not deleted.
| 1. **Phase 1 — self-provisioned etcd, opt-in.** Add the `etcd` values block and per-cluster etcd/DataStore rendering to the `kubernetes` app behind a default that self-provisions for new clusters while still honoring a legacy `_namespace.etcd` reference for existing ones. Extend `delete.yaml` to reap the per-cluster etcd/DataStore. Ship docs for the new sizing knobs. | ||
| 2. **Phase 2 — migration path.** Land and prove the shared→dedicated data migration (Kamaji datastore migration or snapshot/restore), as a numbered-migration hook where idempotent or a gated runbook otherwise. Provide a dashboard/CLI signal of which clusters are still on the shared etcd. | ||
| 3. **Phase 3 — deprecate the tenant module.** Mark `Tenant.spec.etcd` and the etcd tenant-module catalog entry deprecated; new tenants no longer offer it. Stop hardcoding `etcd: tenant-root`. | ||
| 4. **Phase 4 — remove plumbing.** Once telemetry shows no cluster on the shared etcd, delete `apps/tenant/templates/etcd.yaml`, the `etcd` tenant value, the `_namespace.etcd` propagation and `cozy-lib.ns-etcd` helper, and the `namespace.cozystack.io/etcd` label. Keep the etcd-operator and the etcd chart. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the legacy reference key name.
Line 190 says "Stop hardcoding etcd: tenant-root" but the actual value propagated is _namespace.etcd: tenant-root (line 26). Dropping the _namespace. prefix could confuse implementers about which Secret key and helper to remove. Align with the exact key name used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design-proposals/etcd-per-cluster/README.md` at line 190, The Phase 4 removal
instructions use the wrong key name for the legacy etcd namespace propagation,
which can mislead implementers. Update the wording in the README section
describing removal of the shared-etcd plumbing to refer to the exact propagated
Secret key and helper name, using _namespace.etcd and cozy-lib.ns-etcd
consistently with the existing references in the document.
|
+1 on the core direction — folding etcd into the Writing as an operator of a CozyStack-based managed platform, where we run per-tenant Kubernetes clusters at scale. On the "per-cluster etcd multiplies overhead" concernIn practice, per-cluster etcd is a modest addition on top of what a
Per-cluster etcd adds ~18% to requests CPU and ~28% to CPU/memory limits on Small aside: On the proposed
|
|
Filed a scoped tracking issue for Sergei Makarov (@SerjioTT)'s minimal-API point (expose only It captures the reasoning from the review comment above — matching the existing satellite convention (hardcoded infra resources + VPA), keeping the Note it's explicitly gated on this proposal: there's nothing to expose/hide until per-cluster etcd is folded into the |
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Requesting changes — the topology change is well-motivated and the isolation win is real, but a few things should be reconciled before implementation.
- The proposal is written against
etcd.aenix.io/v1alpha1, while in-flight #2859 migrates the platform toetcd-operator.cozystack.io/v1alpha2, renames the CRDs, and switches readiness fromReadytoAvailable. The EtcdCluster apiVersion, the "wait for Ready" semantics, and the reusedextra/etcdchart shape will all be stale once #2859 lands. Please declare an explicit dependency / rebase-onto-#2859 and target v1alpha2 +Available. - Migration-number collision: the proposal targets migration 49, but 49 is already taken on main (ancestor-label backfill) and #2859 also adds its own migration 49. Any numbered migration here must claim 50+.
- Post-migration rollback is effectively one-way and paired with a stateful freeze-mode migration. Kamaji's live datastore migration puts the TenantControlPlane in read-only/freezing mode and requires restarting kubelet on every tenant worker node afterward — that worker-node kubelet restart is a real availability impact and isn't in the proposal; it belongs in the migration runbook and Testing.
- The live datastore switch is triggered by patching
.spec.dataStoreon the nativeTenantControlPlane, but cozystack uses the CAPIKamajiControlPlane, for which upstream docs don't cover a live switch. Open Question 1 is a genuine blocker, and #2859 choosing snapshot/restore is a strong signal the live CAPI path isn't available — please reconcile.
Minor: several code citations have drifted line numbers (the constructs still exist), and §2's full etcd values block should shrink to the minimal etcd.replicas surface per the direction agreed in #3179.
Dismissing the review verdict: converting this to non-blocking feedback since this is a design proposal, not code. The technical points in the review comment still stand as suggestions to reconcile before implementation.
…se all open questions Second draft of the per-cluster etcd proposal. The framing changes and every open question from the first review round is answered. Reframing: the proposal is about completing packages/apps/kubernetes, which since the control-plane / node-pool split (community#8) is the control-plane app. Every control-plane component is rendered by that chart except the one holding all of its state. Retiring the etcd tenant-module wiring is a consequence of fixing that, not the goal. This makes the relationship to community#39 explicit: #39 owns where packages live and how modules declare themselves, this owns what the control-plane app contains. Disjoint files, neither blocks the other. Resolved questions, replacing the five open ones: - Migration: the CAPI provider does re-project dataStoreName onto TenantControlPlane.spec.dataStore on every reconcile, so a live switch works, but it costs a read-only freeze window and a kubelet restart on every tenant worker node. Migration is therefore an operator-initiated maintenance runbook, never an unattended platform migration. - Standalone etcd: survives. Only the Tenant.spec.etcd bool and the _namespace.etcd propagation retire; the chart and its ApplicationDefinition stay. - Default replicas: 3. - Reuse mechanism: shared named templates in cozy-lib, not a subchart. The etcd chart hard-fails unless the release is named etcd, a subchart inherits the parent release name, and the literal etcd-*-tls names are pinned by an immutable spec.tls on adopted clusters. - Footprint: surfaced in NOTES.txt and the app README. Also folded in: the minimal values surface from cozystack#3179 (only etcd.replicas, with the StorageClass derived from it at platform level), etcd-operator v1alpha2 and the Available condition throughout, a name-length section with the real 53-character release cap and the operator's derived names, a delete hook that reaps a per-cluster etcd but never a shared one, corrected source citations, and a migration slot of 56+ rather than the stale 49. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
myasnikovdaniil
changed the title
|
IvanHunters second draft pushed. Framing changed too, it is now about completing
Citations fixed, values block cut down to |
|
Sergei Makarov (@SerjioTT) took your minimal surface as is. Only
On Q1, taking you up on that. Mechanism is settled now (kamaji live switch works, costs a read only freeze plus the kubelet restart on every tenant worker), what nobody has measured is how it behaves on talos workers at fleet scale. Migration e2e gates phase 2, so that is exactly the run that would help. |
|
scooby87 #3179 adopted. The values block now exposes only Framing changed as well, it is a companion to #39 now rather than a competing packaging change. #39 owns where packages live, this one owns what the control plane app contains. |
|
Following up on Q1 — ran the live datastore switch on a real cluster with Talos workers. Short version: the Kamaji migration mechanism itself works and is fast (~20 s data copy, ~43 s patch-to-Ready), but in a CozyStack tenant topology the dominant cost is everything around it, and the run surfaced three findings that I think belong in the Migration/Testing sections of this proposal. Environment
Reads and node heartbeats were never interrupted at any point: 0 read failures over the whole multi-hour session, and 0 write failures after the freeze webhook was removed (including all three worker reboots). Data integrity: every ConfigMap written before each freeze window survived the migrations, the canary's write log shows clean OK→FAIL→OK boundaries with no writes leaking through either freeze window, and the writer kept appending on the new datastore overnight without a single failure. Finding 1 — migration direction is constrained by tenant network topologyThe migrate job runs from the Kamaji namespace and can reach any tenant's etcd, so the data copy succeeds regardless of the target. But the tenant CP pods live in the cluster's namespace, and OVN isolation only permits reaching an ancestor's etcd, not a sibling's. Result of attempt 1: migration "completes", the new CP generation rolls out pointing at For this proposal that's mostly good news — per-cluster etcd lives in the same namespace as the CP, which is trivially reachable, and the ancestor path needed for rollback is open as well — but the migration runbook (shared root etcd → per-cluster etcd, and any rollback) must state the reachability rule explicitly, and ideally the flow should pre-flight it. A Finding 2 — the freeze webhook can outlive a successful migration, leaving a Ready-but-write-locked clusterMechanics first, since they weren't obvious to me from the docs: Kamaji freezes the cluster by installing a
In attempt 2 the migration finished, the TCP went Ready on the new datastore, the CP pods rolled out cleanly — and the webhook was never removed. Kamaji logged no attempt to remove it in the following ~15 minutes (likely related to a leftover migrate Job from attempt 1: the reconcile hit Runbook implication: after any datastore switch, verify Finding 3 — kubelets go deaf to new pod assignments until restarted; the restart itself is cheapOn the new datastore with untouched kubelets: existing pods keep running, nodes stay Ready, and the scheduler works — a new pod got bound to a node (nodeName assigned). But the node's kubelet never picked it up: the pod sat Restarting the worker (VM reboot in our case) fixed it instantly: the pending pod started within seconds of the node coming back. The reboots themselves were cheaper than expected: Talos does a graceful cordon ( One operational gap worth closing as part of this work — there is currently no operator path to the tenant workers' Talos API at all. The chart provisions a per-cluster Suggested runbook/e2e additions
|
… storage Etcd, ingress and seaweedfs modules are exempt as platform overhead, and etcd stays exempt once per-cluster etcd (cozystack#25) moves it into the Kubernetes app. Monitoring reserves its declared storage only, which is the one module figure that still needs a gate when the flag is enabled. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
… storage Etcd, ingress and seaweedfs modules are exempt as platform overhead, and etcd stays exempt once per-cluster etcd (cozystack#25) moves it into the Kubernetes app. Monitoring reserves its declared storage only, which is the one module figure that still needs a gate when the flag is enabled. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <[email protected]>
… storage Etcd, ingress and seaweedfs modules are exempt as platform overhead, and etcd stays exempt once per-cluster etcd (cozystack#25) moves it into the Kubernetes app. Monitoring reserves its declared storage only, which is the one module figure that still needs a gate when the flag is enabled. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <[email protected]>
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
myasnikovdaniil the direction is right, and cozystack/cozystack#4817 already implements phases 1 and 2. Its description has a section of deviations from this document, and each one matches a gap I found here independently against cozystack main (785cbc391b7c). The proposal should carry those decisions back, otherwise the accepted design and the code disagree from day one.
-
_namespace.etcdcan't tell an existing cluster from a new one.packages/system/cozystack-basics/templates/cozystack-values-secret.yaml:17hardcodesetcd: tenant-root, andpackages/apps/tenant/templates/namespace.yaml:28-31passes the parent's value down unchanged unless a tenant setsetcd: true. So every tenant namespace resolves it to at leasttenant-root, whether or not an etcd runs there. Section 4 says the chart honours an explicitly set_namespace.etcdand also that new clusters under a shared etcd get their own, and both can't hold when the value is always present. The delete hook in section 5 reuses the same undefined condition. The liveKamajiControlPlane'sspec.dataStoreNameworks as the discriminator: it exists only for running clusters and records the datastore they really use. The same fact makes the Context bullet and the first problem story inaccurate, because in a default install a new cluster doesn't wait inawaiting-etcd, it points at atenant-rootDataStore that may not exist. -
A
DataStorenamed<namespace>-<release>isn't unique, because both parts contain dashes (child namespaces are<parent>-<name>,packages/apps/tenant/templates/_helpers.tpl:11). Kubernetesfoointenant-agetstenant-a-kubernetes-foo, which is also the namespace of tenantfoounder tenantkubernetesundertenant-a. DataStore is cluster-scoped, so the second install fails on ownership. A namespace can't contain., so<namespace>.<release>fixes it. -
Per-cluster etcd has no platform backup. The Etcd strategy is keyed on the
Etcdapplication kind and the fixedEtcdClusternameetcd(internal/backupcontroller/etcdstrategy_controller.go:36-44), and nothing routesKubernetesto it. The document doesn't say this, yet it cites the backup integration as a reason to reject inlining. Please state it as an accepted gap or design backup and restore. -
The network-policy claim doesn't hold.
policy.cozystack.io/allow-to-etcdis read only by<tenant>-ingress-etcd(packages/apps/tenant/templates/networkpolicy.yaml:129-144), which renders only when the tenant'setcdis set, and section 5 removes that.allow-internal-communicationin the same file admits every pod in the namespace to every other pod, and Cilium allow rules only add. A per-cluster etcd in the tenant namespace is therefore reachable from every pod there, including other clusters' control planes. Keeping it tight needs a deny rule or something equivalent, and the document should say which. -
Phase 4 would switch off every addon. Besides the beacon consumers section 4 lists, 26 templates in
packages/apps/kubernetes/templatesread_namespace.etcd, and the addon HelmReleases, CSI, KCCM and the autoscaler gate on it. Removing the propagation before those gates stops cilium, coredns, CSI and the rest from rendering. -
SerjioTT's live-switch run from 3 September isn't reflected in the runbook yet: reachability of the target has to be checked from the cluster namespace, the freeze can outlive a Ready control plane so step 5 should verify a write rather than a read, and step 4 needs a way to reach Talos workers. Please fold these in or reject each with a reason.
Smaller points:
- The Kubernetes name is capped at 32 now (
packages/apps/kubernetes/values.yaml:2), so truncate-and-hash isn't needed. The operator also creates<cluster>-client, which sets the real ceiling, and the table omits the CA Secret the DataStore needs. cozystack.kubernetes-applicationdoesn't depend oncozystack.etcd-operatortoday (packages/core/platform/sources/kubernetes-application.yaml). Phase 2 needs that edge.- The document should stay state-free: the revision note on line 8, "a review round raised it" on line 160, and "main currently ships through 55" (main is at
targetVersion: 60) describe history, not design.
Both commits land in main as written, since community merges with rebase. 35ad494 has Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> and a [proposal] subject, and 6e1ceef has Assisted-By: Claude <[email protected]> and opens with "Second draft ... every open question from the first review round is answered". Please squash into one docs(etcd-per-cluster): commit with Assisted-by: LLM and a body that says why.
IvanHunters' review is dismissed, and his points (v1alpha2 and Available, the kubelet restart, the live-switch evidence) are in this revision.
| - **etcd is a tenant module.** `packages/apps/tenant/values.yaml:8-9` exposes `etcd: false`. When a `Tenant` sets it true, `packages/apps/tenant/templates/etcd.yaml` renders a `HelmRelease` for the etcd chart in that tenant's namespace, labelled `internal.cozystack.io/tenantmodule: "true"`. | ||
| - **The etcd chart owns the datastore.** `packages/extra/etcd` renders an `EtcdCluster` (`etcd-operator.cozystack.io/v1alpha2`, `templates/etcd-cluster.yaml:11`), its cert-manager CA/issuers/certificates, and a Kamaji `DataStore` named after the **namespace** pointing at `etcd.<namespace>.svc:2379` (`templates/datastore.yaml:5-8`). `DataStore` is **cluster-scoped** (`packages/system/kamaji/charts/kamaji/crds/kamaji.clastix.io_datastores.yaml:16`). | ||
| - **Every name in that chart is the literal string `etcd`.** `EtcdCluster/etcd`; the Secrets `etcd-ca-tls`, `etcd-client-tls`, `etcd-server-tls`, `etcd-peer-tls`; the operator-owned Service `etcd`. The chart enforces this: `templates/check-release-name.yaml` calls `fail` unless `.Release.Name == .Chart.Name`. These names are load-bearing rather than cosmetic — `spec.tls` is immutable in v1alpha2 (CRD CEL `self.tls == oldSelf.tls`) and `etcd-migrate` adopts legacy clusters into exactly this `secretRef` shape, so renaming them would make every post-adoption reconcile fail. Design §1 works with this constraint instead of against it. | ||
| - **The reference propagates down the tenant tree.** `tenant-root` ships a hardcoded `_namespace.etcd: tenant-root` in its `cozystack-values` Secret (`packages/system/cozystack-basics/templates/cozystack-values-secret.yaml:17`). For nested tenants, `packages/apps/tenant/templates/namespace.yaml:28-30` inherits `_namespace.etcd` from the parent and overrides it with the current tenant only when *this* tenant sets `spec.etcd: true`; the resolved value is written to the namespace label `namespace.cozystack.io/etcd` (`namespace.yaml:86`) and into descendants' `_namespace.etcd` (`namespace.yaml:120`). Consumers read it through `cozy-lib.ns-etcd` (`packages/library/cozy-lib/templates/_cozyconfig.tpl:116-118`). So every tenant in a subtree resolves `_namespace.etcd` to the nearest ancestor that owns an etcd. |
There was a problem hiding this comment.
Every tenant resolves _namespace.etcd to at least tenant-root (cozystack-values-secret.yaml:17, inherited by namespace.yaml:28), even when no etcd runs there. See point 1.
| | Native Service (client + peer endpoint) | `<release>-etcd` | etcd-operator | | ||
| | Member pods | `<release>-etcd-<suffix>` | etcd-operator | | ||
| | TLS Secrets | `<release>-etcd-{server,operator-client,peer}-tls` | etcd-operator / cert-manager | | ||
| | `DataStore` (cluster-scoped) | `<namespace>-<release>` | this chart | |
There was a problem hiding this comment.
<namespace>-<release> collides because both parts contain dashes: Kubernetes foo in tenant-a and tenant foo under tenant-a-kubernetes both give tenant-a-kubernetes-foo. A . separator can't collide.
|
|
||
| `$etcd` and the `awaiting-etcd` beacon leave the normal path. The chart no longer waits for an external reference, because it renders the datastore itself. | ||
|
|
||
| The beacon and the `_namespace.etcd` read are not deleted immediately. Through the compatibility window (Rollout Phase 1-2) the chart honours an explicitly-set `_namespace.etcd` and keeps using the shared `DataStore` when it finds one, so a cluster that exists today continues exactly as it is, indefinitely, until an operator migrates it. New clusters — including any created in a namespace whose ancestors do provide a shared etcd — provision their own. Three other consumers key off the beacon and are updated with it: `packages/apps/kubernetes/templates/ingress.yaml`, `hack/admin-kubeconfig-invariant.bats`, and the ComputePlane README's description of the wait state. |
There was a problem hiding this comment.
What counts as "explicitly set"? _namespace.etcd is non-empty in every tenant. The delete hook at line 215 depends on the same answer.
|
|
||
| ## Security | ||
|
|
||
| - **Removes a shared trust boundary.** Each cluster gets its own etcd CA, Service, certificates, and PVCs. Today every control plane in a subtree dials the same `etcd.<ns>.svc` — frequently cross-namespace, because the `DataStore` is cluster-scoped and its owner is an ancestor tenant. Afterwards, a control plane dials only its own in-namespace etcd, and the `policy.cozystack.io/allow-to-etcd` pod label (`cluster.yaml:464`) retargets to it so the NetworkPolicy stays tight rather than subtree-wide. |
There was a problem hiding this comment.
allow-to-etcd is read only by <tenant>-ingress-etcd, which section 5 removes, and allow-internal-communication already admits the whole namespace.
| 2. Take a snapshot of the source etcd. This is the rollback path and is not optional. | ||
| 3. Flip the cluster's `dataStoreName` to its own `DataStore`. Kamaji puts the `TenantControlPlane` into **read-only / freezing mode** while it copies, rejecting writes through an admission controller with an explicit "in freezing mode due to a maintenance mode" message. The default budget is five minutes, adjustable via the `kamaji.clastix.io/migration-timeout` annotation. | ||
| 4. **Restart `kubelet.service` on every worker node of that tenant cluster.** Kamaji's own guide requires this to complete the procedure. It is the single largest cost in the runbook and the reason this is a maintenance operation rather than a platform migration: rolling a tenant's kubelets is a tenant-visible availability event that the platform must not perform unannounced. Plan it as a rolling restart within a maintenance window. | ||
| 5. Verify a canary object written before the switch is readable after it, then retain the source prefix and the snapshot for a defined cool-off period before reclaiming. |
There was a problem hiding this comment.
Per SerjioTT's run: verify a write, not a read, since the freeze can stay behind after the control plane is Ready.
| 1. **Phase 1 — extract, no behaviour change.** Move the `EtcdCluster`, certificate, and `DataStore` shapes into `cozy-lib` named templates; `packages/extra/etcd` calls them and renders byte-identical output, proven by its existing test suite. Nothing user-visible ships. | ||
| 2. **Phase 2 — per-cluster etcd for new clusters.** `apps/kubernetes` renders its own etcd, certificates, and `DataStore`, adds `etcd.replicas` and the platform-level StorageClass keys, extends the delete hook with both branches, and keeps honouring an explicit `_namespace.etcd` for clusters that already have one. Ship the sizing and quota notes. Land and prove the migration runbook, including the kubelet-restart step, before documenting it as supported. | ||
| 3. **Phase 3 — deprecate.** Mark `Tenant.spec.etcd` and the etcd tenant-module catalog entry deprecated; new tenants stop offering it. Stop shipping the hardcoded `_namespace.etcd: tenant-root`. Update the ComputePlane docs and anything else describing `awaiting-etcd`. Optionally add the advisory migration that reports clusters still on a shared etcd. | ||
| 4. **Phase 4 — remove plumbing.** Once that signal shows no cluster on a shared etcd, delete `apps/tenant/templates/etcd.yaml`, the `etcd` tenant value, the `_namespace.etcd` propagation, the `cozy-lib.ns-etcd` helper, and the `namespace.cozystack.io/etcd` label. Keep the etcd-operator, the etcd chart, and the standalone etcd app. |
There was a problem hiding this comment.
26 templates in packages/apps/kubernetes/templates read _namespace.etcd, including the addon HelmReleases, CSI, KCCM and the autoscaler gates. Those gates have to go before the propagation does.
5 participants
Summary
Design proposal for
packages/apps/kubernetesto render its ownEtcdClusterand its own KamajiDataStore, one per cluster.Since the control plane / node pool split (#8)
apps/kubernetesis the control plane app. It renders kamaji, the certs, konnectivity, csi, kccm, the oidc wiring. The one component it does not render is the one that holds all of the control plane's state, etcd comes from outside instead, from an admin settingspec.etcd: trueon an ancestorTenant. This proposal closes that gap, so a cluster's datastore is as much part of the cluster as its api server.Two things follow from it. A consumer who can create a
Kubernetesapp gets a working cluster with no admin pre step, noawaiting-etcd. And each cluster's control plane state is physically isolated, own raft group, own disk, own quota, own CA, instead of N control planes multiplexed onto one etcd by key prefix.Relation to #39
Companion, not competitor. #39 answers where packages live and how a module declares itself. This one answers what the Kubernetes control plane app is made of, it adds one component to one chart. Disjoint files and neither blocks the other. etcd is the single tenant module whose sharing is the defect, which is why #39 leaves it out and this one picks it up.
Contents
design-proposals/etcd-per-cluster/README.md, filling the template.Second draft. Every open question from the first round is answered rather than deferred, the values surface is cut to
etcd.replicasper cozystack#3179, and the whole document is retargeted atetcd-operator.cozystack.io/v1alpha2after cozystack#2859 and cozystack#3270 merged.Status: Review