Repository navigation
feat(kubernetes): give each tenant Kubernetes cluster its own etcd - #4817
Timofei Larkin (lllamnyp) wants to merge 3 commits into
Conversation
The kubernetes chart is about to render an etcd of its own per cluster, and it needs the same EtcdCluster, PKI, DataStore, defrag, VPA and scrape shapes the etcd chart renders. It cannot take the etcd chart as a subchart: the chart refuses any release not named "etcd", and the names it renders are pinned by the immutable spec.tls of adopted clusters. The shapes therefore move into cozy-lib, parameterised by the cluster name, and the etcd chart calls them with "etcd". Its rendered output is byte-identical before and after. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>
A tenant Kubernetes cluster could only start once an ancestor Tenant had an etcd module turned on, which nobody creating a cluster may do, and every cluster below that tenant then shared one etcd, one CA and one disk. Anything that orders clusters on a tenant's behalf had to flip the tenant's etcd on first. A new cluster now renders its own EtcdCluster and DataStore, from the cozy-lib shapes the etcd chart uses, and is created and deleted with them. The awaiting-etcd state goes, since a datastore always exists. An existing cluster stays on the DataStore its live KamajiControlPlane names. Changing it starts a Kamaji datastore migration, which freezes the tenant's writes and leaves every worker kubelet needing a restart, so no upgrade may do it. _namespace.etcd cannot decide this: tenant-root publishes one to every tenant whether or not an etcd runs behind it. The pre-delete hook now fails while the TenantControlPlane remains, because Kamaji cannot finalise one whose etcd Helm has already removed. A Cilium deny policy keeps the rest of the tenant namespace off the etcd ports, and the platform waits for etcd-operator before offering the application. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>
Existing tenant clusters keep the shared tenant etcd they run on, and moving one is a per-cluster operator decision that the platform never takes on an upgrade. Retiring Tenant.spec.etcd has to wait until no cluster uses it, so the upgrade lists the clusters that still do. It writes nothing beyond the version stamp. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
github-actions
Bot
added
area/kubernetes
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) NOT LGTM. The design holds up, but the branch conflicts with main, two existing bats suites fail because of this change, long cluster names cannot install, and a tenant that keeps its shared etcd gets its quorum alert diluted.
Business context: a tenant Kubernetes cluster gets its own etcd and Kamaji DataStore instead of waiting for an ancestor Tenant to turn on a shared one, while existing clusters stay on the shared datastore they already use.
I tried hardest to break the parts that touch existing control planes, and they hold:
- Upgrade.
kubernetes.etcd.sharedDataStorekeys on the liveKamajiControlPlane, so a cluster already ontenant-rootkeeps thatdataStoreNameand renders no etcd of its own. When I forced that helper to return empty, five cases inetcd_test.yamlfailed, so the stickiness is pinned. Migration 60 only reads and stamps, and its bats pass. - Kamaji wiring. The TenantControlPlane webhook only requires the DataStore to exist, and the release creates it next to the
KamajiControlPlane, so the provider's retry covers the ordering. - Deletion. Member PVCs are controller-owned by the
EtcdMembers in etcd-operator, so they go with theEtcdCluster. The five cert-manager Secrets go through the post-delete Job. When I disabled the hook gate,etcd_test.yamlcaught it.
On the PR head helm unittest passes for kubernetes (359), extra/etcd (22), platform (258), tenant (76), computeplane (13) and cozy-lib-tests (62), and select-install_test.bats passes.
Blockers
B1: conflicts with main
packages/apps/kubernetes/templates/delete.yaml, packages/extra/etcd/templates/etcd-defrag.yaml and packages/system/kubernetes-rd/cozyrds/kubernetes.yaml conflict, and GitHub reports CONFLICTING. One of them needs more than a textual fix. fe59a6b added --compaction=false and --etcd-storage-quota-bytes to the etcd chart's defrag CronJob, and that CronJob now lives in cozy-lib.etcd.defrag. The flags have to go into the cozy-lib template so both charts get them, with the quota computed from each caller's size (4Gi for a cluster's own etcd). Otherwise the extraction silently reverts that fix. The defrag_test.yaml that main added will catch the etcd chart side, but nothing checks the per-cluster side. 904bc38 also changed the same pre-delete script this PR edits.
B2: two existing bats suites fail on this head
Unit & controller tests is red (run of 2026-10-08T10:30Z, attempt 1), so E2E (in-tree) was skipped. That means none of the "Needs a live cluster" items has run, the upgrade case included. I reproduced both failures locally:
hack/kubernetes-pre-delete-hook.batscases 5, 6, 8 and 10 still expect exit 0 when the TenantControlPlane outlives step 4 or its probe fails. Case 10 says "an all-failures run exited 1, which would wedge the release". The newexit 1is the opposite contract, so those cases have to be split into a shared-datastore case (still exits 0) and an own-etcd case (fails closed). This file is not in the test plan.hack/check-render-matrix.bats"fixture states enable OIDC groups and shared-service resources" expects thefreshfixture, which has_namespace.etcd: "", to render nokubernetes-check-metrics-server. With the etcd gates gone, it does render. The test and the fixture's meaning need updating, not just the expected count.
B3: the defrag CronJob name exceeds 52 characters for long cluster names
cozy-lib.etcd.defrag names the CronJob <etcd name>-defrag, which for this chart is kubernetes-<name>-etcd-defrag. The app name allows 32 characters, so the CronJob name reaches 55. The apiserver rejects CronJob names longer than 52, so any cluster named with 30 to 32 characters fails to install. I rendered kubernetes-abcdefghijklmnopqrstuvwxyz0123 and got the 53-character kubernetes-abcdefghijklmnopqrstuvwxyz0123-etcd-defrag. The 56-character guard in kubernetes.etcd.name does not cover this. Either shorten the CronJob name (truncate plus hash) or lower the guard, and add a test at the maximum name length.
B4: a tenant's shared etcd scrapes the cluster etcds in its namespace
packages/extra/etcd/templates/podscrape.yaml selects app.kubernetes.io/name: etcd, and etcd-operator puts that label on every member (clusterLabels in controllers/helpers.go). In a tenant with etcd: true that creates a cluster in the same namespace, etcd-pod-scrape now also scrapes kubernetes-<name>-etcd. Those members are then scraped twice, and the shared etcd's job counts six members instead of three. etcdInsufficientMembers sums up by job, so with two of the three shared members down it sees 4 of 6 up and stays quiet. Before this PR nothing else in that namespace carried the label. Selecting on app.kubernetes.io/instance: etcd (or etcd-operator.cozystack.io/cluster: etcd) fixes it.
Decisions to confirm, not blocking
- New clusters lose platform backup. Before, a new cluster's control plane lived in the tenant etcd, which an
EtcdBackupJob covers. After this PR it lives in an etcd no strategy routes to. The README and the release note say so. I'd still like a maintainer to accept that explicitly before this leaves draft, because it is a regression for anyone who backs up tenant etcd today. - The new
exit 1wedges the uninstall whenever the TenantControlPlane cannot finalize, including when the cluster's own etcd never became Available, for example because of the quota case in your test plan. Whether Kamaji can finalize a TenantControlPlane whose datastore was never reachable decides whether that deletion ever finishes without manual work. That case belongs in the live test list. kubernetesEtcd.storageClassLocaldefaults to empty, so on a default install three members land on the namespace default class. That is usually the replicated one, which is the double replication the values comment says the local class avoids.
| apiVersion: batch/v1 | ||
| kind: CronJob | ||
| metadata: | ||
| name: {{ $name }}-defrag |
There was a problem hiding this comment.
B3: for this chart this is kubernetes-<name>-etcd-defrag, which is 53 to 55 characters when the app name is 30 to 32. The apiserver caps CronJob names at 52.
| selector: | ||
| matchLabels: | ||
| app.kubernetes.io/name: etcd | ||
| {{ include "cozy-lib.etcd.podScrape" (dict "name" "etcd" "selector" (dict "app.kubernetes.io/name" "etcd")) }} |
There was a problem hiding this comment.
B4: every etcd-operator member carries app.kubernetes.io/name: etcd, so this now also matches kubernetes-<name>-etcd pods in the same namespace. app.kubernetes.io/instance: etcd keeps the job to this cluster.
| TenantControlPlane whose datastore no longer answers: its | ||
| finalizer then stays until removed by hand. Failing here | ||
| keeps the etcd up and makes Flux retry the uninstall. */}} | ||
| if [ "$tcp_lookup" != 0 ] || [ -n "$tcp" ]; then |
There was a problem hiding this comment.
B2: hack/kubernetes-pre-delete-hook.bats cases 5, 6, 8 and 10 still expect exit 0 on this path and fail.
2 participants
What this PR does
A tenant Kubernetes cluster could not start until an ancestor
Tenanthadspec.etcd: true. The chart read the datastore name from_namespace.etcdand, with none, rendered only a<release>-awaiting-etcdConfigMap. Every cluster below that tenant then shared one etcd, one CA and one disk, separated by key prefix only. Whoever creates a cluster usually may not edit theTenant, so anything ordering clusters on a tenant's behalf (the portal's controller, for one) had to flipetcd: truefirst.This PR makes the
Kubernetesapplication carry its own etcd. It implements phases 1 and 2 of the per-cluster etcd design proposal, cozystack/community#25, as three commits that can be split later:refactor(etcd): the etcd chart'sEtcdCluster, cert-manager PKI,DataStore,WorkloadMonitor, defragCronJob, VPA andVMPodScrapemove into named templates incozy-lib(packages/library/cozy-lib/templates/_etcd.tpl), parameterised by the cluster name.packages/extra/etcdcalls them with"etcd", and its rendered output is byte-identical before and after (see Verification).feat(kubernetes): a new cluster renders its ownEtcdClusterandDataStorefrom those templates, and a cluster that already runs on a shared tenant etcd keeps it.feat(platform): migration 60 → 61 reports which clusters stay on a shared etcd and changes nothing.Phases 3 and 4 (deprecating, then removing
Tenant.spec.etcd,_namespace.etcdandcozy-lib.ns-etcd) are left for later.Design
Names. For release
kubernetes-<name>in namespace<ns>:EtcdCluster, its headless Service, members<…>-xxxxx,<…>-clientServicekubernetes-<name>-etcdkubernetes-<name>-etcd-{ca,client,server,peer,peer-ca}-tlsDataStore(cluster-scoped)<ns>.kubernetes-<name>kubernetes-<name>-etcd.<ns>.svc:2379Values. One new field,
etcd.replicas:3(default) or1. The schema bounds it to 1..3 and the chart refuses 2 at render time, becausecozyvalues-gencannot express an integer enum (it emits string members for one). Size (4Gi), resources (1000m / 512Mi limits) and the etcd version (3.6.11, the etcd chart's default) are chart-internal.StorageClass. Two new platform values,
kubernetesEtcd.storageClassLocalandkubernetesEtcd.storageClassReplicated, reach the chart as_cluster.etcd-storage-class-local(used with 3 members) and_cluster.etcd-storage-class-replicated(used with 1 member). Empty keys fall back to the namespace default StorageClass.spec.storage.storageClassNameis immutable on anEtcdCluster, so the chart reads the live object's class on every render after the first and never re-derives it; scaling 1 → 3 keeps the class the cluster started with.New clusters vs existing ones. The chart decides from the live
KamajiControlPlane, looked up by release name:KamajiControlPlane, or one whosespec.dataStoreNameis the cluster's own name: own etcd;DataStore: thatDataStoreis kept, and the chart renders no etcd of its own.Clusters stuck in the old awaiting-etcd state have no
KamajiControlPlane, so they get their own etcd on their next reconcile._namespace.etcdis deliberately not the signal.packages/system/cozystack-basics/templates/cozystack-values-secret.yamlhardcodesetcd: tenant-root, andpackages/apps/tenant/templates/namespace.yamlpasses it down, so every tenant publishes a value whether or not an etcd runs behind it. Honouring it for new clusters would change nothing for anyone. Following it for existing clusters is also wrong: today, switching onspec.etcdfor a mid-level tenant silently moves every cluster below it to a new datastore, which is a Kamaji datastore migration. With this PR, the live binding sticks.An existing cluster renders exactly what it rendered before, apart from randomly generated Talos tokens, so the upgrade rolls no control-plane pods. The new pod label goes on new clusters only.
What goes away. The
awaiting-etcdbeacon goes, along with the_namespace.etcdgate on the ~25 addon and satellite templates (helmreleases/*,csi/deploy.yaml,kccm/manager.yaml,cluster-autoscaler/deployment.yaml,ingress.yaml, the Proxmox CCM and CSI). A datastore now always exists, so these gates were always true.Deletion. The existing pre-delete hook deletes the
TenantControlPlaneand waits up to 120s. Before this PR it exited 0 even when theTenantControlPlanewas still there. On a cluster that owns its etcd, Helm deletes the etcd right after the hook, and Kamaji then cannot finish its finalizer, because it needs the datastore to drop the tenant's role, prefix and user. So on such a cluster the hook now fails while theTenantControlPlaneremains, keeping the etcd up and making Flux retry the uninstall. After it:EtcdCluster,DataStoreand Certificates;EtcdMembers;The hook never touches a shared etcd, and the Secret cleanup Job is not rendered for a cluster on one.
Deviations from cozystack/community#25
<namespace>.<release>, not<namespace>-<release>. Tenant names are alphanumeric and nest with dashes, so the dashed form collides: Kubernetesfoointenant-agivestenant-a-kubernetes-foo, which is also the namespace, and therefore the sharedDataStorename, of tenantfoounder tenantkubernetesundertenant-a. A namespace cannot contain a dot. Kamaji copies theDataStorename into the pod labelcomponent.kamaji.clastix.io/datastore, so the name is capped at 63 characters, truncated with an 8-character hash past that.kubernetes.etcdName. The app name is now capped at 32 characters (@maxLength 32, added with the nodes split), sokubernetes-<name>-etcdis at most 48 characters. That is inside the 56 the operator needs: the<name>-clientService and the generated member names must fit in 63. The helper fails the render past 56 rather than rename anything._namespace.etcd" means keeping a cluster on theDataStoreits live control plane names, for the reason above. It does not mean following_namespace.etcdfor new clusters.etcd.replicasvalidation is a schema range of 1..3 plus a render-time refusal of 2, rather than a schema enum.KamajiControlPlanecondition as the render, as Release 0.1.0 #25 asks, but it fails the uninstall rather than ordering deletions inside the hook. Helm deletes release resources only after a successful pre-delete hook, so that ordering already holds once the hook refuses to pass early.secretRefshape (chart-issued certificates) rather than the operator'scertManagermode. Kamaji'sDataStoreneeds the CA private key to sign each tenant's client certificate, andsecretRefkeeps that key in a Secret the chart controls. The server certificate also carries the operator's<name>-clientService names, which the etcd chart's does not.PrometheusRulestays in the etcd chart only: its expressions match every etcd job, so one copy per cluster would duplicate every alert.VMPodScrape, VPA and theCiliumNetworkPolicyrender only where their APIs are served.talosconfigis issued. The numbered migration only reports, as Release 0.1.0 #25 suggests.Gaps #25 did not cover
Etcdbackup strategy (internal/backupcontroller/etcdstrategy_controller.go,packages/system/backupstrategy-controller/templates/strategy-etcd-default.yaml) is keyed on theapps.cozystack.io/Etcdapplication and on the fixedEtcdClusternameetcd, andcozy-defaultroutes noKuberneteskind. A cluster's own etcd is therefore not backed up by the platform. A cluster still on a shared tenant etcd keeps exactly the coverage it had. The guest-side Velero addon covers workloads, not control-plane state. An operator can snapshot a cluster's etcd by hand with anEtcdSnapshotagainstkubernetes-<name>-etcd; the server certificate covers the-clientService the snapshot Job dials. Backup alone could routeKubernetesto the existing driver with a name lookup. Restore is the hard part: it means recreating theEtcdClusterunder a live KamajiTenantControlPlane, and needs its own design before anything is wired. The README states the gap.CiliumNetworkPolicynamed after the etcd selects its member pods withingressDenyon 2379/2380 against two sets of pods. The first is pods in the tenant namespace that are neither members nor carrypolicy.cozystack.io/allow-to-etcd-cluster: <etcd name>, a label set on the control-plane pods and the defrag Job. The second is pods in any other namespace exceptkube-systemand namespaces labelledcozystack.io/system: "true", which is where Kamaji, its migrate Job and etcd-operator run. Deny rules are needed because the tenant's own policies allow all traffic within a namespace and from ancestor tenants, and allow rules only add. Metrics on 2381 stay open. The pods also carrycozystack.io/service: kubernetes-etcdrather thancozystack.io/service: etcd, so the tenant-wide<tenant>-ingress-etcdpolicy does not admit child tenants' control planes to them. The policy renders only where Cilium servescilium.io/v2/CiliumNetworkPolicy.cozystack.kubernetes-applicationnowdependsOncozystack.etcd-operator. Both come with the isp-full variants, the only ones the iaas bundle allows. The selective-install fixture is unchanged (hack/select-install.sh --disabled postgresproduces the same set). The same edge makes an etcd-operator change select thekubernetes-latest,kubernetes-previousandcomputeplanee2e suites besidesetcd, andhack/select-e2e_test.batsnow expects that.hack/e2e-install-cozystack.batsno longer turns on tenant-root's etcd, which only Kubernetes clusters used. A cluster that fell back on a shared etcd would therefore not come up.run_kubernetes_testasserts the control plane namestenant-test.kubernetes-<name>and thatkubernetes-<name>-etcdreachesAvailable.cozy_cleanupwaits until theEtcdCluster,DataStore, member PVCs and Secrets are gone.etcd: trueand the comment calling it mandatory.hack/admin-kubeconfig-invariant.batsdrops its two awaiting-etcd cases, and the now-unusedtests/values-ci-no-etcd.yamlgoes with them.make -C packages/apps/kubernetes generateproducedvalues.schema.json, the README table,api/apps/v1alpha1/kubernetes/types.goandpackages/system/kubernetes-rd/cozyrds/kubernetes.yaml. Rootmake generateregeneratedzz_generated.deepcopy.go, andpkg/generated/openapidid not change.make generatein every touched package leaves the tree clean.KamajiControlPlanewhoseDataStoreits own release did not render, judged by Helm's release annotations on theDataStore, with thatDataStore's endpoints. It writes nothing but the version stamp. Slot check on 2026-10-08:mainships through 59 withtargetVersion: 60;release-1.6ships through 53 (target 54);release-1.7.0-alpha.3ships through 58 (target 59);packages/apps/kubernetes/README.mdclaimed a dedicated etcd per cluster while clusters shared the tenant's. It now describes what this PR ships, including the legacy case and the backup gap. The ComputePlane README no longer lists a tenant etcd as a prerequisite.Migrating an existing cluster (designed, not implemented)
Nothing here moves a cluster, and nothing should until the runbook below has an e2e. It combines #25's runbook with SerjioTT's Talos measurements of 2026-09-03 (about 20s copy, 43s from patch to Ready, reads uninterrupted) and with what Kamaji 26.9.4-edge's code does.
dataStoreName. The likely shape is a chart-internal value, or an annotation the chart reads, that the operator sets. Wait forAvailable=True.nc -zv <target> 2379from a pod carrying the control plane's labels in the cluster namespace. For the per-cluster target this holds by construction (same namespace, and the deny policy admits the control plane), but the check stays because rollback goes the other way.KamajiControlPlane.spec.dataStoreNameat<ns>.<release>, with the CAPI provider re-projecting it onto theTenantControlPlane.kamaji.clastix.io/migration-timeoutcan go on theKamajiControlPlane, because the provider copies annotations to theTenantControlPlane. Before switching, delete any leftovermigrate-<TCP UID>Job in Kamaji's namespace:kamaji-freezeValidatingWebhookConfiguration in the guest, and only its soot controller removes it, only when theTenantControlPlanestatus is exactlyReady. A pausedTenantControlPlane, or one sitting inWriteLimited,SleepingorUpgrading, keeps the freeze. Node leases are exempt and reads are untouched, so nodes andkubectl getlook healthy. The success criterion is a canary write through the guest API; ifkamaji-freezeis still in the guest after theTenantControlPlaneisReady, delete it.talos-caIssuer but issues no client credentials from it, so the fallback isvirtctl restartper VM.talosconfigfrom that CA alongside the admin kubeconfig is the follow-up that makes this step cheap. It is not in this PR: it is a separate credential with its own exposure question.Test plan
Done here, no cluster involved:
helm templateofpackages/extra/etcdbefore and after the extraction is byte-identical: 12 renders (default; 1 member + local SC + 10Gi; resources with CPU and memory allocation ratios + another version; a global topology spread; a hard affinity with 5 members and 512Mi; leftoverbackup.*values with 0 members), each intenant-rootandtenant-abc, all identical by sha256.helm templateofpackages/apps/kuberneteswith Cilium served. With_namespace.etcdset totenant-root, or empty, both with 3 members:EtcdCluster/kubernetes-demo-etcdreplicas=3 on the namespace default StorageClass, andDataStore/tenant-demo.kubernetes-demopointing atkubernetes-demo-etcd.tenant-demo.svc:2379. TheKamajiControlPlanenames thatDataStore, and the policy renders.replicatedand 3 members uselocal. 2 members is refused at render time; 5 is refused by the schema.KamajiControlPlanelookup stubbed to returntenant-root) renders identically tomainwithvalues-ci.yaml, apart from random Talos tokens. A new cluster's render differs frommainonly by the old shareddataStoreNameline and the added etcd objects.packages/apps/kubernetes: 359 tests, including a new 19-casetests/etcd_test.yaml. It covers names, the second cluster in one namespace, the 63-characterDataStorecap, StorageClass choice and stickiness, refusal of 2, the policy, the Secret cleanup and the delete-hook gate. For an existing cluster it covers staying on the sharedDataStore, ignoring a changed_namespace.etcd, no own etcd, no Secret cleanup and no gate.packages/extra/etcd: 22.packages/core/platform: 258, including a new wiring test for the StorageClass keys.cozy-lib-tests,packages/apps/tenantandpackages/extra/computeplane: green.hack/migration-60-shared-etcd-report.bats(new, 4 cases);hack/run-kubernetes*_test.bats,admin-kubeconfig-invariant,check-hook-request-timeout,app-cleanup-hooks_test,bats-no-negated-assert,bats-no-exit-trap,select-install_test,select-e2e_test,helmrelease-readiness,proxmox-e2e-fixture_test,talos-image-cache_test,linstor-storage-mode_testandcozystack-version-stamp.go build ./...,go test ./pkg/registry/...and theapi/apps/v1alpha1module.make generatein every touched package and at the root leaves no diff;hack/check-commit-trailers.sh origin/main..HEADpasses.Needs a live cluster:
kubernetes-latest,kubernetes-previousandcomputeplane, now without tenant-root's etcd. The Proxmox lane on its self-hosted runner.mainwith an existing cluster on tenant-root's etcd. It must stay ontenant-rootwith no control-plane rollout, and migration 60 must list it.nc -zv kubernetes-<name>-etcd.<ns>.svc 2379must be refused. The control plane, the defrag Job, anEtcdSnapshotJob, Kamaji and etcd-operator must still connect.TenantControlPlaneis held past the 120s wait. The uninstall must fail and retry rather than leave aTenantControlPlanestuck on its finalizer.ResourceQuotacannot fit 3 × 1000m/512Mi limits and 3 × 4Gi requests must surface the quota error on the etcd member pods.Follow-ups, not opened
These are listed here at the maintainer's request rather than filed:
content/en/docs/next/operations/configuration/platform-package.mdgainskubernetesEtcd.storageClassLocalandkubernetesEtcd.storageClassReplicated;content/en/docs/next/kubernetes/relocate-etcd.mddescribes the shared tenant etcd as what a cluster uses;etcd: truefor a cluster, which is now an obsolete workaround;internal/provider/kubernetes_schema.goandkubernetes_model.goneedetcd.replicas, with the default of 3, a 1..3 range and the expand/flatten pair.talosconfig;Screenshots
Not a UI change.
Downstream repositories
Follow-ups are listed above under "Follow-ups, not opened". By the maintainer's decision they are not filed from this draft, so no box is ticked.
Release note