Repository navigation
feat(platform): add slim installation variants - #4595
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe platform now supports ChangesSlim platform variants
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Operator as cozystack-operator
participant Profile as profile values
participant System as system bundle
participant Helpers as package helpers
Operator->>Profile: Select values.yaml and variant values
Profile->>System: Set bundles and system variant
System->>Helpers: Render packages for the selected variant
Helpers-->>System: Apply slim or full package selection
Merge Risk: ⚪ Minimal · up to The slim profiles have consistent tenant dependencies and standalone networking configuration. No actionable merge-blocking risk remains; installations must satisfy the documented node-CIDR prerequisites. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The managed slim profiles reject unsupported encryption rather than silently dropping it, and existing full-profile encryption behavior is preserved. No introduced security weakness was established. Permissions over platform configuration and recovery from live profile changes remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (5 skipped: 5 unsupported.)
✨ 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 |
…riants Small installs such as arm64 labs had to take the whole isp-full set: KubeVirt, backups, monitoring, etcd, SeaweedFS and every paas/naas operator. The slim variants keep only the engine, tenants, ingress, gateway, LINSTOR storage and networking, and turn everything else into opt-in packages listed in bundles.enabledPackages. Full variants render the same Packages as before. Networking on isp-slim and isp-slim-generic is Cilium alone: without VMs nothing needs KubeOVN, and Cilium L2 announcements take the place of MetalLB, which stays available as an opt-in package. KubeOVN IPsec is what networking.encryption drives, so the slim variants refuse it rather than ignore it. isp-hosted-slim keeps the noop datapath. tenant-application gains a slim variant whose dependsOn drops the monitoring, etcd and seaweedfs applications; the default variant waits on them, which would leave cozystack-basics and tenant-root never Ready. The iaas bundle stays limited to the isp-full variants. objectstorage-controller stays on everywhere: cozystack-controller watches BucketClaim unconditionally, so without that CRD its manager never syncs its caches and crash-loops. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
Add isp-slim, isp-slim-generic and isp-hosted-slim to the platform PackageSource so a Package can select the minimal presets shipped by the platform chart. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
f89e4ac to
465195c
Compare
|
Aleksei Sviridkin (@lexfrei) where did request for slim variants come from? Description motivates it with one line (small installs like arm64 labs where isp-full is too heavy), there is no linked issue, and I didn't find any request for slim or minimal install in cozystack or community repos. I had minimal install request myself, but it is covered by Yes, variant is cheap, it is mostly config, but without concrete user or use case I don't understand motivation. arm64 by itself doesn't look like reason either - #1961 reports isp-full-generic running fully on 3-node arm64 k3s, and PR has numbers only for slim, not for isp-full on same hardware. |
IvanHunters
left a comment
There was a problem hiding this comment.
Verdict
LGTM with non-blocking notes
Upgrade-safe: every existing variant renders byte-identical to main except for one additive, unselected PackageSource block. The three slim variants render a coherent, dependency-closed package set, and the new tests actually guard the behaviour rather than just rendering it. Two notes below, neither blocks.
Claim mismatches
[PARTIAL] The PR body says "the slim variants refuse [networking.encryption] instead of silently ignoring it". Only isp-slim and isp-slim-generic do: they fail at render time. isp-hosted-slim selects the noop networking variant, so there the toggle is silently ignored, exactly like isp-hosted. Rendering isp-hosted-slim with networking.encryption.enabled=true produces no error and a cozystack.networking Package on variant noop. The in-code values.yaml comment is accurate; only the PR-body summary overstates it. No code change needed.
Caveats
- Opt-in gating on slim is a two-key gate. An opt-in paas/naas package renders only when
bundles.{paas,naas}.enabled: trueAND it is listed inbundles.enabledPackages. Setpaas.enabled=falseand putcozystack.postgres-applicationinenabledPackagesand you get nothing, with no error. The shipped slim presets set both flags true so the default path works; a doc line saying the bundle flag is itself the opt-in gate would save someone the silent no-op. golangci-lintwould not run locally here (its binary was built with go1.26, the repo targets go1.27.1), so I leaned on the green CI "Analyze (go)" job for lint.go build,go vetandgo test ./cmd/cozystack-operator/pass locally.- The new
slimvariant on thecozystack.tenant-applicationPackageSource is only exercised by render in this review. How the operator reconciles a newly-added-but-unselected variant on a live cluster is not something I can test hermetically. The change is additive and existing variants still selectdefault, so the blast radius is small. - No e2e covers a slim install. The
cilium/cilium-genericnetworking variants were dormant before this PR (the system bundle never selected them), and this change turns them on together with Cilium L2 announcements standing in for MetalLB. Whether a LoadBalancer Service actually gets an address from the admin-createdCiliumLoadBalancerIPPoolplusCiliumL2AnnouncementPolicy, and the host ingress comes up, is only answerable on a cluster.hack/e2e-platform-packages.shdoes not exercise slim. Not a defect, a gap worth a follow-up e2e. - [NIT]
packages/core/platform/templates/_helpers.tpl:124:set $components "cilium" (dict "values" (dict "cilium" $cilium))rebuildscomponents.cilium.valueswholesale, so any future sibling key next tociliumundervalueswould be dropped. Harmless today, since bothsystem.yamlbranches put onlyciliumthere. Flagging it so the line is not copied as-is into a path that carries more.
Recommended follow-ups
- Satellites (ccp#23, ansible-cozystack#79, website#718) carry the downstream variant wiring. Not reviewed here, and no desync with them is visible in this diff.
This documents the three slim platform variants added in cozystack/cozystack#4595: `isp-slim`, `isp-slim-generic` and `isp-hosted-slim`. The variants page gets them in the comparison table and one section each. It also explains the Cilium load balancer setup that replaces MetalLB there, and has a table of which packages to add to `bundles.enabledPackages` to bring back applications, monitoring, etcd, SeaweedFS and backups. The platform package, generic install, distribution and Ansible pages list the new names. Only `docs/next` is touched, since the variants are not in a release yet.
What this PR does
This adds three minimal platform variants for small installs, such as arm64 labs, where isp-full is too heavy. They are
isp-slimfor Talos,isp-slim-genericfor k3s, kubeadm or RKE2, andisp-hosted-slimfor clusters where the host provides CNI and storage.A slim variant installs only the base platform: the engine, API and dashboard, tenants, ingress and Gateway API. The two non-hosted variants also get LINSTOR. objectstorage-controller stays on too, because cozystack-controller watches BucketClaim unconditionally and crash-loops without that CRD. Everything else is opt-in through
bundles.enabledPackages, including every paas and naas application and its operator, monitoring, backups, etcd, SeaweedFS, metrics-server, VPA, Multus and MetalLB. The iaas bundle is refused, so there are no VMs and no managed Kubernetes. With the stock preset a slim variant emits 20 Packages (15 for hosted), where isp-full emits 77.On the two non-hosted slim variants, networking is Cilium alone. Without VMs nothing needs Kube-OVN, and Cilium L2 announcements take the place of MetalLB. The admin creates a
CiliumLoadBalancerIPPooland aCiliumL2AnnouncementPolicy, or usespublishing.externalIPs. Pod ranges come fromnode.spec.podCIDR, which Talos and k3s allocate by default.networking.encryptiononly drives Kube-OVN IPsec, so the slim variants refuse it instead of silently ignoring it.The tenant application needs its own slim variant. The default one waits on the monitoring, etcd and SeaweedFS applications, so on slim cozystack-basics and tenant-root would never become ready. tenant-rd and cozystack-basics still reference the default variant's artifacts by name. Those artifacts exist anyway, because the PackageSource builds them for every variant.
The full variants don't change. I rendered isp-full, isp-full-generic and isp-hosted against main and got byte-identical Packages. The only new output there is the slim variant in the tenant-application PackageSource.
Things to know before using it:
DependenciesNotReady.kubeovn-ciliumtocilium, the operator removes the Kube-OVN release, and running pods lose networking. The other Packages are kept byhelm.sh/resource-policy: keep, so the switch doesn't make the cluster smaller either.I installed isp-slim from a build of this branch on a fresh three-node Talos 1.13 cluster (amd64). All Packages and HelmReleases went Ready, with no Kube-OVN, MetalLB, monitoring, backup or KubeVirt pods. From outside, the dashboard and the API answered by name with valid certificates. A LoadBalancer Service got an address from a Cilium pool and answered through L2 announcements. Pod-to-pod traffic across nodes, cluster DNS and a replicated LINSTOR volume worked. Opting in postgres-operator and postgres-application gave a Ready Postgres. That run found the BucketClaim crash-loop above, which is fixed here. The base platform used about 215m CPU and 3.2 GiB of memory, not counting the Kubernetes control plane.
Screenshots
Not a UI change.
Downstream repositories
Release note
Summary by CodeRabbit
isp-slim,isp-slim-generic, andisp-hosted-slimplatform variants, with support for PaaS and NaaS bundles.