mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-02 10:06:53 +09:00
feat/cozystack-controller-gpu-only
4629
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
a522d58968 |
feat(cozystack-controller): add a GPU-only mode and a Lease name flag
A cluster whose cozystack-controller predates the GPU binder cannot pick up GPUDevice by starting a second, newer instance: every reconciler is registered unconditionally, so the second instance runs its own copy of the workload, tenant, gateway and secret reconcilers next to the first. --gpu-only (with --enable-gpu) registers only the GPU binder, its webhooks and the probes; nothing else is set up, so no other informer or cache exists in that process. It is not meant to run beside a controller from this tree, which runs a binder of its own. --leader-election-id names the Lease instead of the fixed ID, whose default stays the historical one. A GPU-only instance that elects must name its own Lease: on the default one it would take leadership from the main controller in a shared namespace and stop all of its reconcilers while holding it. The logger is now set right after flag parsing, so this and the other flag refusals reach the pod log instead of exiting silently. Known limit of the pairing: the older controller's WorkloadMonitor still counts the GPUs KubeVirt requests on a virt-launcher pod, while the binder writes a usage record per device, so a VM covered by an application's WorkloadMonitor is counted twice until the main controller is upgraded. The manager wiring is now a list of named steps, so a test can assert which steps each mode runs without starting a manager. The image builds the command as a package, since it no longer fits in main.go. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2aaf7f6872 |
feat(gpu): count the KubeVirt table's GPU names as GPU resource names
The NVIDIA and mediated entries of the KubeVirt permittedHostDevices table are GPU resource names besides the hoster's gpu.resourceNames list, whether the hoster sets one or not. Without that, a GPU name only the table carries, on a node whose agent reports nothing and with no class of its own, would not be a GPU resource name: under forbidden the raw-name policy would let a VM or a pod request it directly, and the VMI webhook would take it for a non-GPU host device. The binder adds those names to the effective set it publishes in the settings ConfigMap, which the policy and cozystack-api read, and the VMI webhook adds them when it looks for a VMI's own GPU entries. A PCI entry of another vendor, such as a ConnectX VF, stays out. Whether a class's resource name is in the table at all is still checked against every PCI and mediated entry. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9fc57e7e07 |
fix(gpu): count a pod as a VMI's launcher only when the VMI lists it
The binder, the VMI webhook, the catalog option source and the usage records each leave out of a pod's GPU usage the units its VMI's devices already hold. They matched the pod to the VMI by its owner reference, the usage records also by the device-reference label, and whoever creates the pod writes both, so a pod naming a VMI with attached devices could hide its GPUs from capacity, raw-key budgets and the usage records. A pod now counts as a VMI's launcher only when that VMI lists the pod's UID in status.activePods; any other pod's GPUs count as raw-name usage. One shared reader decides it for all four. The binder takes the active pods from the VMIs it already lists, the webhook from the VMI informer every replica warms, the usage records from a read of the named VMI, and cozystack-api reads the named VMI only for a pod that requests a raw GPU name, for which it gains get on virtualmachineinstances. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e488a7ad78 |
feat(api): add gpuclass and gpudevice option sources
The dashboard orders GPUs by class and attaches them by device name, so it needs the catalog and the tenant's own devices without reading GPUClass or GPUDevice objects itself. The gpuclass source lists every class that still takes orders, a deleted one excluded, with its display name, a short description of the offer, the free units and the most free units on one node from the class status, the per-VM maximum, and the requesting tenant's remaining budget. That budget comes from the shared model over the tenant and each ancestor's subtree, from their declared resourceQuotas and every device, and is 0 for a vGPU class without the entitlement. Pods are read only when an ancestor limits a class's resource name with a raw key, one list per namespace of the outermost such ancestor's subtree, and a virt-launcher pod does not count the units its attached devices already cover. No field names a node. The gpudevice source lists the devices in the request namespace. cozystack-api gets read access to both kinds, and get and list on pods for the raw-key case. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3648bc39ae |
docs(cozy-lib): reflow the barman sidecar comment
The wording change for GPU-only tenants left one line of the comment well past the width of its block. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ca74e5b750 |
feat(vm-instance): attach GPUDevice objects through device and deviceTemplate
A gpus entry can now reference a held GPUDevice by name or ask for a device of a class for as long as the VM runs, next to the existing raw resource name. References are rendered only as the device-reference label and one JSON list in the devices annotation on the VMI template. The GPU webhook resolves them when the VMI is created, and the label is what selects the VMI and its virt-launcher pod for it. The schema cannot say "exactly one of", so name becomes optional and the template refuses an entry that sets none or several of the three, a list mixing raw names with references, a device listed twice, and a deviceTemplate without a class or with a count below one. A zero deviceTemplate counts as unset, since a typed client sends one on every entry. The dashboard offers the tenant's devices and the GPU catalog through the gpudevice and gpuclass option sources. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
45c66d217f |
feat(cozystack-basics): let tenant admins order and release GPUDevice objects
Tenant admins and above create, change and delete GPUDevice objects in their namespaces where GPUs are on. The grant renders under the same conditions as the policy that keeps tenants off a device's finalizers and template mark, so no tenant can order without it; the GPU webhook checks each order's budget and entitlement. Status holds a device's binding and attachment and stays a subresource no tenant role grants. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e03e14aa14 |
feat(platform): thread the GPU switch and settings to the controller
cozystack-controller runs on every cluster, and its GPU paths watch KubeVirt objects, so the chart's gpu.enabled is set only where the GPU admission policies apply: where gpu-operator serves GPUs to VMs. monitoring-agents scrapes the metrics those paths serve under the same condition. gpu.resourceNames and gpu.rawResourceNames reach the controller, which publishes them in its settings ConfigMap for the policies and cozystack-api. rawResourceNames also reaches cozystack-basics, whose raw-name policy renders only under forbidden. Both are validated on every render, so a typo fails before anyone turns GPUs on. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7d1d238f4d |
feat(cozystack-basics): enforce gpu.rawResourceNames forbidden
With raw names forbidden a GPU reaches a VM only through a GPUDevice reference, which makes a held device a hard guarantee. A VMI carrying a GPU entry (any gpus entry, a hostDevices entry or a resource of a GPU resource name) without the device-reference label is refused, which covers a VM written by a tenant super-admin, a VMIReplicaSet and a worker pool. A pod requesting a GPU resource name is refused unless kubevirt-controller creates it with a VMI owner, as it does for virt-launcher and migration target pods. The GPU resource names come from the settings ConfigMap the controller publishes. The binding fails closed without it, and since the parameter is resolved before any expression runs, the bindings select tenant namespaces only; anything wider would block every pod in the cluster, the controller's own included, until the ConfigMap exists. The usage-monitor label policy now selects tenant namespaces the same way, so the two cannot disagree on what a tenant namespace is, and its ratchet also refuses changing the label's value, which could make an object labelled before the policy start matching the monitor. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
36eac696a9 |
feat(monitoring-agents): scrape the GPU binder's metrics
The class capacity, bound units and Lost count are kept out of the world-readable GPUClass status and exported as cozystack-controller metrics instead, so platform monitoring has to scrape them. The metrics port is authenticated, so vmagent also needs the right to GET /metrics. Both render only with GPU enabled, as the port does. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
91f922d380 |
feat(cozystack-controller): serve the GPU webhooks with GPU enabled
The GPU admission webhooks need a Service, a serving certificate and webhook configurations, and the binder's metrics need a port a scrape can reach. All of it renders only with GPU enabled, which the platform sets where KubeVirt serves GPUs, so clusters without KubeVirt change nothing. The webhooks fail closed. The two mutating ones select only VMIs and virt-launcher pods carrying the device-reference label, the pod webhook also requiring the virt-launcher label, so of VMIs and pods a controller outage blocks only the start of VMs with device references; the GPUDevice webhook blocks new orders as well. The controller then runs two replicas behind a PDB, since a single fail-closed replica would block those starts on every rollout. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ccaef2f877 |
feat(gpu-node-agent): restrict the agent to its own node's GPUNode
RBAC cannot name the caller's node, so the agent's role lets every agent write every GPUNode, and a compromised node could rewrite the inventory, and through resourceNames the cluster's GPU resource-name set, of any other node. The pod-bound service account token carries the node name as a user extra, which a ValidatingAdmissionPolicy can compare with the object name. A token without that extra is refused rather than let through: the policy matches only the agent's service account, so failing closed stops the agent and nobody else. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b9c1d465aa |
feat(platform): run the GPU node agent with gpu-operator
GPUNode inventory is needed exactly where GPUs are served, so the agent ships as a component of every gpu-operator variant, in the cozy-gpu-operator namespace. It is marked privileged because its hostPath mounts are refused below the privileged Pod Security level, and the operator labels a namespace privileged only for privileged components. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0cb44dc919 |
feat(gpu-node-agent): add the agent command, image and chart
The inventory has to run next to kubelet: a DaemonSet reading the host's sysfs and kubelet's pod-resources socket, both mounted read-only. Connecting to the socket needs root, so the agent runs as root but with no capabilities and no privilege escalation. The manager's cached client would watch every pod in the cluster, so the agent uses a direct client and lists only its own node's pods. RBAC cannot name the caller's node, so the role is cluster-wide; it holds only the verbs the agent uses, and the DaemonSet selects the nodes node-feature-discovery marks as carrying an NVIDIA device, so the token does not sit on nodes with nothing to report. The image is in the root build list so that every build path stamps its reference; the committed tag is replaced by the first build. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f736950fcd |
feat(cozystack-controller): grant the GPU controller its writes
The binder writes finalizers and status on GPU classes, devices and nodes, deletes template devices with their VMI, and deletes the unscheduled VMI of a device that must leave its node. The create right on GPU devices is for the template devices made on behalf of a VMI. The settings ConfigMap is the only ConfigMap the controller writes, so its rights live in a Role in cozy-system rather than the ClusterRole: get, update and patch are scoped to that object by name, and create, which RBAC cannot scope by name, is scoped by the namespace. With --metrics-secure the metrics endpoint authenticates and authorizes each scrape through TokenReview and SubjectAccessReview, so the controller needs create on both for the GPU metrics to be scraped. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
e983911bae |
docs(tenant): say which resourceQuotas bring the tenant LimitRange
The tenant chart now renders its ResourceQuota and the default LimitRange only for keys other than the gpu.cozystack.io/* GPU budgets, so a tenant with only GPU budgets has no LimitRange. The comments and the system memory limits page that explain sidecar sizing and system defaults by that LimitRange said any declared resourceQuotas brings it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9a3262a8af |
test(cozystack-basics): pin when the GPU admission policies render
The class and device policies render only where the platform switched GPUs for VMs on, the usage-monitor label reservation always, and none of them where the API server does not serve ValidatingAdmissionPolicy. A string "false" or a null gpu block must not switch the gated ones on. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cede4f7dc9 |
feat(cozystack-basics): reserve the GPU usage-monitor label
The GPU controller writes the usage records of held devices under a WorkloadMonitor whose selector is a reserved label key, so the monitor must match nothing and tenant labels cannot shape GPU usage records. A write that adds the key to any object or subresource in a tenant namespace is refused, whoever makes it, which also covers the virt-launcher pods that copy a VM template's labels and the DataVolumes created from a VM's dataVolumeTemplates. The policy renders whether or not GPUs are on, so no tenant can place the label before the hoster turns GPUs on. It ratchets: an object that already carries the key can still be updated, lose its finalizers and drop the key, so nothing labelled before the policy can get stuck. It sees every write in those namespaces, so its expression reads only metadata and guards each step. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
32fc6114e9 |
feat(cozystack-basics): keep tenants off GPUDevice finalizers and mark
Whoever orders GPUDevice objects must not touch two parts of a device. The device finalizer keeps a released device, its binding and its usage record until no VMI uses it, so a tenant who could drop it could take a held device out of the usage records. The template-device label is the mark the VMI webhook is to set on devices it creates from a deviceTemplate, and the GPU controller deletes devices so marked with their VMI, so on a tenant's own order it would get that order deleted. Both are refused for everyone but platform callers, matched by group like the other cozystack-basics policies: cozy-system and kube-system service accounts and cluster admins, plus kube-controller-manager, which has no group of its own. The orphan and foregroundDeletion finalizers belong to the API server and the garbage collector and stay free. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
30de93c031 |
feat(cozystack-basics): freeze a held GPUClass, refuse deleted orders
Held devices and their usage records name their class, so while the class carries its protection finalizer, what it sells (vendor, model, mode, profile, memory, maxCount and the kind of source) cannot change. The policy relies on the GPU controller setting the finalizer on every class, so a changed offer is a new class. resourceName and nodeSelector stay open for host-layer changes and for moving the offer. CRD rules cannot see finalizers, so this is an admission policy, and it reads the stored object so the write that drops the finalizer cannot change the spec as well. A class being deleted waits only for the devices that already hold it, so a second policy, evaluated once per class, refuses new orders of it. The policies render only where the platform switched GPUs for VMs on. Their tests evaluate the expressions read from the chart, compiled for both the current library and the oldest served Kubernetes minor. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6a0a446f7f |
feat(cozystack-basics): let tenants read GPUDevice, everyone GPUClass
Every tenant role reads the GPUDevice objects in its namespace. No tenant role writes them yet: nothing checks an order's budget or vGPU entitlement until the admission webhooks exist, so the write ships with them. GPUClass is the hoster's published catalog and is cluster-scoped, so tenant roles, which are bound per namespace, cannot carry its read; a ClusterRoleBinding gives it to every authenticated user. GPUNode names physical devices and stays out of every tenant role. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7045899800 |
feat(platform): gate the GPU policies on gpu-operator, add gpu.vgpu
The GPU class and device admission policies in cozystack-basics apply only where gpu-operator serves GPUs to VMs: emitted by the iaas bundle in the default or vgpu variant. A helper reads the same bundle keys as the iaas bundle's own gpu-operator gate to decide that. gpu.vgpu entitles the root tenant through _cluster.gpu-vgpu, written only when on because every release in every tenant reads that Secret. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
933f58c1d6 |
feat(cozystack-basics): take the root tenant vGPU flag from the platform
A tenant is entitled to vGPU only when its parent is, and the root tenant's namespace and _namespace values are rendered here rather than by the tenant chart, so the chain has to start here. The value comes from _cluster.gpu-vgpu, which the platform fills from its gpu.vgpu. Like a tenant namespace, the root namespace always carries the label, and _namespace.vgpu appears only when entitled. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f7b12cf63a |
feat(tenant): add the vGPU entitlement, keep GPU budgets out of quotas
A vGPU class may be ordered only by an entitled tenant, and no tenant may grant itself the entitlement. The Tenant field gpu.vgpu is set by the parent's administrator, so the effective value is the field and the parent's value together, and it reaches the namespace as the namespace.cozystack.io/vgpu label. The label is always written, so only "true" entitles and a missing label cannot be read as consent. _namespace.vgpu is written only when entitled, since every app in the tenant reads that Secret and an always-present key would upgrade them all. gpu.cozystack.io/* budget keys are checked against GPUDevice orders, not pods, so they are dropped before the ResourceQuota render, and a tenant with only GPU budgets gets no ResourceQuota or LimitRange. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
18a3ce5a24 |
feat(api): add gpu.cozystack.io GPUClass, GPUDevice and GPUNode
GPUs become a catalog of hoster offers, tenant reservations against them and a per-node inventory, so a tenant can hold a GPU while its VM is off and an order that does not fit is refused at create time. The CRDs enforce what the apiserver alone can: a class name is a DNS label other than total because it becomes part of a quota key, the source is a union with dra reserved for a later member, a device's spec is immutable and orders at least one GPU, and bound devices are listable per node through a field selector on status.nodeName. A class resource name must be a valid extended resource outside gpu.cozystack.io, where it would read as a GPU budget key. The CRDs are kept on chart uninstall, since they ship as plain templates and deleting GPUDevice would drop every hold and usage record. maxCount carries no schema default: it defaults to 1 for vgpu classes and to no limit for passthrough, which a structural default cannot express, so readers resolve it through the shared model. A GPUNode device reports how many device-plugin devices kubelet counts as allocatable for it, because an mdev parent serves several from one PCI function and one healthy entry per function would undercount the node. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
8bd6d91dbb |
fix(backups): gate every default Strategy CR on its CRD (#4929)
## What this PR does Upgrade from v1.6.x to v1.7 leaves `backupstrategy-controller` HelmRelease in `UpgradeFailed`: `no matches for kind "Kafka" in version "strategy.backups.cozystack.io/v1alpha1"`, same for Rabbitmq and Redis, and none of the new strategy CRDs get created. CRDs of this chart are ordinary templates, and Helm maps every object of a release through the RESTMapper before applying any of them. Default Strategy CRs for kafka, rabbitmq and redis were gated only on the bucket name, so they render next to their not yet served CRDs, Build() fails, nothing is applied and every retry renders the same manifest. Same happens on a first install with external S3 (`provisionBucket: false`) for all older default strategies, bucket name comes from values there. MongoDB default strategy already has the `crdEstablished` gate, this PR adds it to all other default strategies. On an installed cluster nothing changes. Otherwise first revision creates the CRDs and skips the CRs, then DefaultObjectsGate forces the follow-up upgrade that renders them. CRDs stay templates so later CRD changes still reach the cluster on upgrade. Short explanation is added to `docs/operations/backup-classes.md`. To keep this from coming back there is `hack/backupstrategy-crd-gate.bats`: it renders the chart with the bucket gate open and no CRD served and fails on any Strategy CR from any template, so a new `strategy-*-default.yaml` without the gate goes red without anyone updating a list. It also checks that every CR gates on the CRD this chart ships for its kind. helm-unittest covers both halves of the gate for all 11 templates. Checked on a throwaway kind cluster: - v1.6.4 installed, then upgraded: current main fails with the error above, this branch passes with all 11 CRDs Established and the follow-up upgrade creates the rest - modified CRD is applied on a later upgrade - fresh install converges, and fresh install with external S3 converges here while main fails with "no matches for kind" for eight kinds Fixes #4925. Needs backport to release-1.7, both commits apply there cleanly. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note fix(backups): backupstrategy-controller release no longer gets stuck in UpgradeFailed on upgrade from v1.6 and no longer fails a fresh install with external S3, strategy CRDs are created first and default strategies follow on the next reconcile ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Default backup strategies are now created only after their required resource types are available and the backup bucket is resolved. This helps prevent installation and upgrade issues, including when using externally managed S3 storage. * **Documentation** * Clarified when default backup strategies are created during installation and upgrades. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
f47bab7280 |
test(backups): fail CI on any default Strategy CR rendered without its CRD gate
The CRD gate has now been missed twice: MongoDB shipped without it, and Kafka, RabbitMQ and Redis followed after it existed. helm-unittest only checks the templates a case lists, so a new strategy-*-default.yaml that forgets the gate stays green until an upgrade wedges. Render the chart with the bucket gate open and no CRD served, and fail on any strategy.backups.cozystack.io object from any template. Also check that every rendered Strategy CR gates on the CRD this chart ships for its kind, because a gate naming the wrong CRD passes the first check and keeps the CR out of every live render. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
7204992452 |
fix(backups): gate every default Strategy CR on its CRD
The chart ships its strategy CRDs as ordinary templates, and Helm maps every document of a release through the RESTMapper before it applies any. A default Strategy CR gated only on the bucket name therefore fails Build() with "no matches for kind" whenever the bucket name resolves before the CRD is served, the release applies nothing, the CRDs are never created, and every retry renders the same manifest. That happens on the upgrade that first ships a kind: the Kafka, Rabbitmq and Redis CRs leave a v1.6 cluster's HelmRelease stuck in UpgradeFailed on the way to v1.7. It also happens on the first render of an external-S3 install (provisionBucket=false), where the bucket name comes from values and every older default CR renders next to its absent CRD. MongoDB already gates on crdEstablished for this reason. Give every default Strategy the same gate. On an installed cluster the CRDs are established and the render is unchanged; otherwise the first revision creates the CRDs and skips the CRs, and DefaultObjectsGate, which reads the cozy-default BackupClass routes, forces the follow-up upgrade that renders them once the kinds are served. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
11b19635e0 |
fix(monitoring): send alerts to every Alertmanager replica (#4889)
## What this PR does vmalert sends every alert to every Alertmanager replica instead of to the shared Service. The chart gave vmalert a single notifier, `http://vmalertmanager-alertmanager.<namespace>.svc:9093`. That Service is headless and resolves to the three Alertmanager pods, but each alert reached one replica at a time and moved to another one whenever vmalert reconnected. The Alertmanager README asks for the opposite. It says "Do not load balance traffic between Prometheus and its Alertmanagers" and to point the sender at every instance, because the cluster deduplicates notifications only when each replica receives each alert. What this looked like on three clusters running 1.6.4, read through the Alertmanager API of each pod. - On all three, every active alert sat on one replica and the other two had none. The counts were 0, 0 and 10 on the first, 0, 0 and 23 on the second, and 1, 0 and 9 on the third. - On the third one, which also has an email receiver, the active alert count of each replica changed 163 to 180 times in 24 hours. The three replicas sent 63, 67 and 62 emails in that window. Over the same week only three alert names with a severity routed to email fired there. An alert that leaves a replica expires there and goes out as resolved, and the replica that receives it next sends it as a new firing notification. So a single long-running alert keeps producing firing and resolved notifications, and a receiver that rejects concurrent duplicates sees them as conflicts. The change. - `vm/vmalert.yaml` renders one notifier per Alertmanager pod, `http://vmalertmanager-alertmanager-<i>.vmalertmanager-alertmanager.<namespace>.svc:9093`. That is the address the operator builds for StatefulSet pods (`VMAlertmanager.asPodFQDN`), and the replicas already reach each other through the same per-pod names in their `--cluster.peer` arguments. - The replica count comes from a new `monitoring.alertmanager.replicas` helper, which now also sets `spec.replicaCount` of the VMAlertmanager. The notifier list cannot drift from the number of pods. Why not the operator's notifier selector. `VMAlertNotifierSpec.selector` expands matched VMAlertmanagers into the same per-pod addresses, but only when the VMAlert is reconciled. The VMAlert controller does not watch VMAlertmanager objects, so a VMAlert reconciled before its VMAlertmanager exists, which can happen on a fresh install, would get no notifier until something else triggers a reconcile. A static list renders the same addresses without depending on that order. Upgrade impact. The VMAlertmanager is unchanged, it renders `replicaCount: 3` as before. The VMAlert gets new notifier arguments, so the operator restarts the vmalert pod once. Testing. - `helm unittest` passes for the monitoring chart, 24 suites and 304 tests. The new suite `tests/vmalert_notifiers_test.yaml` checks the three notifier addresses, the replica count of the VMAlertmanager and the release namespace in the addresses. - Each of these breakages turns at least one case of the new suite red. The notifier back on the Service address, the helper set to 2, a hardcoded namespace in the address, and a hardcoded `replicaCount` that no longer follows the helper. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: Walked the trigger map in `docs/agents/contributing.md` against the diff. It changes two templates and the helpers of `packages/system/monitoring` and adds a test file. No values, schema, README, package name, make target or `ApplicationDefinition` changes, so no entry applies. ### Release note ```release-note fix(monitoring): vmalert now sends alerts to every Alertmanager replica instead of the shared Service, so the Alertmanager cluster deduplicates them and a long-running alert no longer produces repeated firing and resolved notifications. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * vmalert now configures a notifier URL for each of the three Alertmanager replicas, rather than using a single shared URL. * Alertmanager’s replica count is managed consistently with the notifier configuration. * **Tests** * Added checks for the replica count, all three notifier URLs, and the configured namespace. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
e8635f6e4b |
build(seaweedfs): stamp the re-pinned in-tree image
The builder, the compile flags and the version stamp changed, so the image built from them has a new digest, and values.yaml has to name it for the components to run that build. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
5592a3f65d |
build(seaweedfs): re-pin the in-tree build from make update
The in-tree build pinned its source, base image and toolchain by hand, and nothing moved them when `make update` vendored a new chart, so a bump would build one release's weed into another release's image, or keep patching a release that already carries the fix. Pin the source archive by checksum and the upstream image by digest as build arguments, compile with the toolchain upstream's own builder uses for the tag, and let `make update` re-pin all of them from the vendored chart and fail when the patches no longer apply. The tests of the patched packages now run once rather than once per target platform, and `weed version` names the build. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
d2c6b85cac |
fix(seaweedfs): run the in-tree SeaweedFS image in every component
With the image keys unset the vendored chart renders chrislusf/seaweedfs at its appVersion, so the patched build from images/seaweedfs would be built and never run, and the S3 gateway would keep being OOM-killed on large GETs. Point every component at the in-tree image, pinned by digest like every first-party reference, and hold that in a unit test so a values change cannot quietly fall back to the upstream binary. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
1913104bd3 |
build(seaweedfs): build SeaweedFS in-tree with a bounded read-ahead
The S3 gateway is OOM-killed while serving a large object. SeaweedFS starts read-ahead downloads after every read of a sequential stream and counts only the downloads it newly starts toward its prefetch window, so a GET reading 256 KiB at a time out of 4 MiB chunks pulls up to about a gigabyte of the object into memory, past the 512Mi limit of the S3 deployment. No SeaweedFS release carries a fix yet, so there is no release to move to instead. Build weed from the 4.45 sources with a patch that keeps the window at the next prefetchCount chunks, and lay it over the upstream image of the same tag so the rest of the image stays upstream's own. The image build runs the tests of the packages the patch touches, including the regression test the patch adds, because go build never compiles them. Assisted-by: LLM Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Andrey Kolkov
|
0c2f131192 |
feat(backups): back up Valkey with the Redis strategy
Valkey runs on the same spotahome RedisFailover operator as Redis and speaks the same replication protocol and RDB format, so it needs no strategy kind of its own. The only difference the backup Job sees is the chart's resource-name prefix: valkey-<app> instead of redis-<app> for the sentinel Service and the auth Secret. The Redis driver now accepts applicationRef kind Valkey as well as Redis and hands the template .Release.Prefix, derived from the app's kind, which cozy-default-redis uses for every name it builds. A kind with no known prefix fails the render instead of producing an empty one. cozy-default routes apps.cozystack.io/Valkey to cozy-default-redis. A restore must target an app of the same kind as the backup's source: replaying a Redis dump into Valkey, or the reverse, is not something the strategy can promise across engine releases. The shipped template reads the prefix as `.Release.Prefix | default "redis-"`: during an upgrade the previous controller, which has no Prefix in its render context and serves only Redis, renders the new CR, and a bare reference would print "<no value>" into the names. The redis:8 image already loads RDB v11, which both shipped Valkey versions (7.2, 8.1) write. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Yan Bondarenko
|
9d4221b890 |
fix(monitoring): send alerts to every Alertmanager replica
vmalert had a single notifier, the headless vmalertmanager-alertmanager Service. It resolves to all three pods, but each alert reached one replica at a time and moved to another when vmalert reconnected. On clusters running 1.6.4 all active alerts sat on one replica while the other two had none, and on one of them the alert count of each replica changed about 170 times a day. The Alertmanager cluster deduplicates notifications only when every replica receives every alert. The replica an alert left let it expire and sent a resolved notification, and the replica it reached sent a new firing one. vmalert now lists each Alertmanager pod as its own notifier, using the <name>-<i>.<service>.<namespace>.svc address the operator itself gives StatefulSet pods. The replica count comes from one helper that also sets spec.replicaCount of the VMAlertmanager, so the two cannot drift. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
3a463609e4 |
feat(bucket): add a ui flag to skip the s3manager web UI (#4881)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Adds a `ui` flag to the Bucket application. Every bucket currently ships an s3manager Deployment, its Service, and an Ingress or HTTPRoute on `<bucket>.<tenant host>`. Tenants that only use the S3 API pay for a pod per bucket and for one DNS name and TLS certificate per bucket, none of which they need. - `packages/apps/bucket`: new `ui` value (default `true`), passed through to the `bucket-system` HelmRelease. The dashboard Role drops its Ingress rule when the UI is off, since an empty `resourceNames` would widen it to every Ingress in the namespace. - `packages/system/bucket`: the Deployment, Service, Ingress and HTTPRoute render only when `ui` is true. With the UI off, the chart no longer requires the tenant apex, as the UI hostname was its only consumer there. - Regenerated `values.schema.json`, `README.md`, `types.go` and the `bucket-rd` ApplicationDefinition. The default keeps existing buckets unchanged. Setting `ui: false` on an existing bucket makes Helm delete the UI objects on the next upgrade; the cert-manager Certificate for the UI host is owned by the Ingress through ingress-shim and goes with it. Tested with `helm unittest` on both charts: new suites cover the default render, `ui: false` behind an Ingress and behind a Gateway, the empty-apex case, the value passthrough and the dashboard Role. The system-chart suite fails against the previous templates. ### Screenshots Not applicable: no dashboard change, only a new field in the Bucket form generated from the schema. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> Walked the trigger map against the diff. A new field in `packages/apps/bucket/values.schema.json` reaches the Terraform provider (schema, model, expand/flatten) and the website (the Bucket page is hand-written). Both follow-ups are issues rather than PRs, since they apply only once this PR is merged. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#750 - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#71 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(bucket): add a `ui` flag to the Bucket application. Set it to `false` to skip the s3manager web UI and its Service, Ingress or HTTPRoute, DNS name and TLS certificate. Defaults to `true`, so existing buckets are unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a `ui` option for bucket deployments. It is enabled by default and provides the S3 manager web UI at the tenant host. * Set `ui` to `false` to run a bucket without its web UI or the associated Service and Ingress or HTTPRoute. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
cfa9c5a5c7 |
fix(e2e): keep chainsaw asserts polling through transient API errors (#4867)
## What this PR does Chainsaw asserts in the e2e suites can fail long before their timeout. Chainsaw v0.2.15 keeps polling only on NotFound and stops on any other API error. When etcd changes its leader, kube-apiserver answers the GET with a 500, and whichever suite happens to be polling goes red. Between 2026-09-27 and 10-11 this hit 6 of the 397 e2e attempts that reached chainsaw, which is 6 of the 23 assert failures in that window. Each one has `etcdserver: leader changed` in the apiserver logs within 0.2 s of the failure. Five of the six came after 10-09, and four of them were the computeplane ingress-nginx assert. The fix is kyverno/chainsaw#2761, which is still open and not in any release. Until it is released, the e2e sandbox image builds chainsaw v0.2.15 from source with that PR's commit applied unchanged, using the same Go minor and build flags as the v0.2.15 release binary. The build runs the tests of the patched packages. It also runs chainsaw against a fake apiserver that answers 500: a short run of 500s must not fail the assert, and an endless one must fail it at its timeout. Plain v0.2.15 fails both cases after the first GET. The Dockerfile header lists what to remove once a chainsaw release carries the fix. The README sentence added here has to go too, and so does the Renovate rule that keeps the builder on Go 1.26. The upstream PR also breaks one chainsaw test outside the packages it touches, `Test_runE` in `pkg/commands/assert`. Its expected string lacks the new `; operation terminated with error: context deadline exceeded` suffix. I found it with a full `go test ./...` on v0.2.15 with and without the patch. The image build tests only the patched packages, so that test does not run here. One trade-off: when the API stays broken, an assert or `error` step now runs to its timeout instead of failing within seconds, so a red run caused by a dead API gets longer. The proxmox e2e lane runs the chainsaw installed on its runner, not the sandbox image, so it does not get this fix. Known gaps, left for later: the new test covers only an assert on a named object, not one that lists objects or an `error` operation, and its skip guard does not notice if no test runs at all. The patch file has no `-whitespace` entry in `.gitattributes`, so `git diff --check` reports whitespace inside the upstream patch. A multi-platform build of the image would run the builder tests once per platform; the image is built for amd64 only. Closes #4720 ### Screenshots Not a UI change. ### Downstream repositories None of them builds or uses the e2e sandbox image; the change stays inside this repo's CI. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): a chainsaw assert in the e2e suites keeps polling through a transient API error such as an etcd leader change, instead of failing the suite long before its timeout. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * End-to-end sandbox assertions now retry after transient Kubernetes API errors instead of failing immediately. Persistent errors still cause assertions to fail when their timeout expires. * **Testing** * Added checks that verify assertions recover after temporary API errors and fail within the expected timeout when errors persist. * **Maintenance** * Updated sandbox image build and dependency update settings for the Chainsaw testing tool. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
a3470391e2 |
fix(linstor): mark volumes restored from a snapshot DRBD-initialized (#4866)
## What this PR does Restoring a PVC from a LINSTOR VolumeSnapshot can leave one replica with an empty volume that DRBD marks UpToDate. A pod that lands on that replica sees an empty filesystem. linstor-csi restores the snapshot on one node and autoplaces the other replicas. LINSTOR v1.33.3 copies `Linstor/Drbd/InitialUptoDateOn` from the source volume into the restored volume definition and leaves the definition without `DRBD_INITIALIZED`. When autoplace puts a replica on the node that name points to, that empty new volume wins the initial UpToDate race, gets a random current UUID and a fresh filesystem, and goes StandAlone against the restored replica. LINBIT fixed this in v1.34.0. This PR backports that commit, LINBIT/linstor-server@9d8cdf5, to our v1.33.3 build as a patch. The README entry says to drop the patch with the move to v1.34.0 or later. Tested on a three-node cluster with a thick ZFS pool and `placementCount: 3`. Before the change the reproduction failed every time. With the patch I ran 5 restores, 4 of them with the source volume's winner on a different node than the restore. I also restored a snapshot taken before the change, and a 10Gi volume. Every time all replicas came up connected and UpToDate, a pod on each node read the same checksum, and the backing zvols were byte-identical when read with `O_DIRECT`. The resync after a restore moves about the written data, around 40 s for 1Gi and 134 s for 7.5 GiB on 10Gi. CSI clone PVC-to-PVC was checked too and is not affected. There is no e2e test. Every e2e lane runs without DRBD (`COZY_LINSTOR_DRBD_ENABLED=false`, StorageClass `local`), so a suite for replicated restores would never run in CI. Volumes restored before this fix are not repaired by it. How to find and fix them is in [this comment](https://github.com/cozystack/cozystack/issues/4862#issuecomment-6106661441). Known limitation, shared by upstream v1.34 since that code did not change: the autoplaced replicas skip the initial sync and rely on the source volume's day0 UUID still being in the restored replica's history. If that ever fails, DRBD goes StandAlone with "unrelated data" instead of syncing, which is visible, not silent. I could not reproduce it. Closes #4862 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(linstor): a PVC restored from a VolumeSnapshot no longer gets a replica with an empty volume marked UpToDate. Volumes restored before this release may still have such a replica; https://github.com/cozystack/cozystack/issues/4862#issuecomment-6106661441 explains how to check and repair them. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Restored snapshots with diskful DRBD resources are now recognized as initialized by default, helping them resume without unnecessary metadata recreation. * When force-initial-sync is enabled, restored volumes continue through the existing metadata-recreation path. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d060c1dbb0 |
fix(kafka): pass the container limits to the Kafka JVMs (#4865)
## What this PR does The Kafka JVMs we ship size themselves from host memory instead of their container limits, and get OOMKilled. The Strimzi 0.45.1-rc1 images carry JDK 17.0.15, which reads `/proc/cgroups` to decide whether it runs in a container. Kernels 6.12 and newer built without `CONFIG_MEMCG_V1` no longer list the memory controller there, so the JVM turns container support off. The JDK fix is JDK-8347811, backported to 17.0.20 and 21.0.10. In CI this shows up in two places. The cluster operator gets a 25 GiB max heap inside its 512Mi limit. In 107 of the last 150 e2e reports the kernel OOM-killed a Java process at about 500M, the limit the cluster operator shares with the topic and user operators. Brokers and controllers start with a 5 GiB heap inside a 1Gi limit, because Strimzi asks the same JDK for the container memory and takes half of what it reports. This PR passes the limits to every Kafka JVM explicitly: - the cluster operator gets `-XX:MaxRAM` and `-XX:ActiveProcessorCount` from its own limits through the downward API; - brokers, controllers, the topic and user operators and the drain-time Cruise Control get `JAVA_TOOL_OPTIONS` with `-XX:MaxRAM` set to their memory limit, plus `-XX:ActiveProcessorCount` where the chart sets a CPU limit. The chart sets one only for brokers and controllers. A LimitRange default on the entity operators or Cruise Control is invisible to the chart, and Strimzi template env takes no `resourceFieldRef`, so those JVMs still count host CPUs. Every java in a container reads `JAVA_TOOL_OPTIONS`, including the heap probe Strimzi runs before starting Kafka. So Strimzi's own heap rules and Kafka's G1 defaults apply to the real memory limit, and the chart does no heap arithmetic. `jvmOptions` with `-XX` would not work here: Strimzi turns those into `KAFKA_JVM_PERFORMANCE_OPTS`, and Kafka skips its G1 settings when that is set. Each JVM now prints a "Picked up JAVA_TOOL_OPTIONS" line on start. In a kind cluster on such a kernel the operator then runs Serial GC with a heap of about 100 MiB, and peaks at 249Mi while creating a Kafka, against 440Mi before. A broker with a 1Gi limit and one CPU gets the same `-Xmx518982205` that Strimzi computes when the JDK sees the limit, with G1 and one concurrent GC thread. Existing Kafka clusters roll once on upgrade, Talos ones included, because the pod environment changes. Their heap does not change on a kernel where the JDK already sees the limit. The chart has no values key for JVM options or heap, so there is no user setting this could override. The Kafka migration hook now applies with `--openapi-patch=false`. Its container runs under a tenant LimitRange default of 128Mi, and a plain `kubectl apply` of an existing pool downloads the cluster's whole OpenAPI document, about 300Mi with the platform's CRDs, so every upgrade in such a tenant failed on an OOM-killed hook. With the flag kubectl builds the same merge patch from the last-applied annotation and peaks at about 35Mi. A larger memory limit would count against the tenant quota, and a server-side apply would stop removing fields that `kubectl create` set on the `kafka` pool. Other Jobs without resources are tracked in #4868. The operator OOMKill is what triggered the e2e failure in #4855. The other half is a Strimzi race. After a restart, the PodSet controller drops the ADD event of a PodSet that has no pods yet and only picks it up at the 5-minute resync. That one is upstream and is described in the issue. The other JVMs we ship are tracked in #4864. Talos nodes are not affected, the Talos kernel keeps `CONFIG_MEMCG_V1=y`. Generic-variant clusters on newer distribution kernels are. CI is affected even though it runs Talos, because there the Talos nodes are containers on the runner's Ubuntu 6.17 kernel. Closes #4855 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(kafka): the Strimzi cluster operator, Kafka brokers and controllers, the entity operators and the drain-time Cruise Control no longer size their JVM heap from host memory on kernels built without cgroup v1 memory support, which got them OOMKilled. The Kafka migration hook no longer runs out of memory on upgrade in a tenant with resource quotas. Existing Kafka clusters roll once on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Kafka components now size JVM memory and processor settings based on their configured resource limits, including during migrations. - The Kafka cluster operator now receives container resource limits for JVM sizing. - **Bug Fixes** - Kafka migration operations avoid OpenAPI patch lookups that could use excessive memory. - **Tests** - Added coverage for JVM sizing, resource limits, and migration behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
1f74ad4ee7 |
fix(multus): drop the CPU limit and make resources configurable (#4830)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The multus daemon was too small to recover a node after a CNI outage, and its limits could not be raised from values. The 100m CPU limit and the 1Gi memory limit of `kube-multus` were hardcoded in the patched upstream template. On a node with about 600 dead sandboxes and 220 pods waiting, multus was throttled in every CFS period. Its calls took longer than kubelet's two-minute CRI deadline, so every call was retried and the queue only grew. With the CPU limit lifted it OOM-looped at 1Gi. With 2 CPU and 4Gi it cleared the backlog in about three minutes. Now the resources come from a new `resources` value. The default keeps the same requests (100m CPU, 100Mi memory), has no CPU limit, and raises the memory limit to 4Gi. The memory limit stays on purpose. Without it the pod cgroup has no `memory.max`, and multus becomes a candidate for the Talos userspace OOM handler again. The chart now refuses to render when an override removes the memory limit or the memory request. Without a request Kubernetes uses the limit as the request, so a node with less allocatable memory than the limit would be left without multus. The change is in `patches/customize-deployment.patch`, so `make update` keeps it. I applied the patch to upstream v4.3.0 and got the committed template, except the image references. The platform reverts a direct edit of the child `cozystack.multus` Package, so a running cluster sets the value through the new `networking.multus.resources` key of the platform Package. The key is passed on only when set, and a value that is not a map stops the render. The system-defaults LimitRange in the namespace sets memory only, so it does not bring a CPU limit back. Closes #4829 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> Not a UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/747 - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff adds a platform value, so the hand-written key table on the website needs it. The follow-up PR there is a draft until this one merges. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(multus): the kube-multus container no longer has a CPU limit, and its memory limit is raised from 1Gi to 4Gi, so a node with hundreds of pods can recover after a CNI outage. Its resources can be overridden through the new `networking.multus.resources` platform value. An override that removes the memory limit or the memory request now fails the render. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional CNI plugin staging during Multus startup, configurable through chart values. Staging skips when the plugin directory is unavailable and reports individual copy failures while continuing. * Made Multus resource settings configurable, with defaults of 100m CPU and 100Mi memory requests and a 4Gi memory limit. Rendering requires memory requests and limits. * Updated the Multus deployment to use the `cozy-multus` namespace and DaemonSet name, and Cilium as the master CNI configuration. * **Bug Fixes** * Added a 128Mi memory limit to the CNI installation step. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
fa9f09e3d8 |
fix(vm-disk)!: refuse a golden-image clone CDI would leave unbound (#4796)
## What this PR does A VM disk created from a golden image into a StorageClass other than the image's one can hang forever: its temporary clone PVC stays unbound, and the user sees nothing. With the default `csi-clone` strategy, CDI checks the source and target classes for a common provisioner, a matching volume mode and room to expand. It never compares class names. So a target class backed by the same provisioner gets no host-assisted fallback, and the clone is left to the CSI driver. LINSTOR creates it next to the source data, so on a target class that does not allow remote volume access from every node (a node-local class, for example) the PVC may never bind. The vm-disk chart now refuses such a disk at render time and tells the user which StorageClass to set. The check follows CDI's own clone planner. The effective strategy comes from the CDI override, or else from the target StorageProfile. It has to be `csi-clone`, or `snapshot` when CDI gets a VolumeSnapshotClass of the provisioner. Once the snapshot path has such a class it runs the same checks as csi-clone, and LINSTOR ships one. Both classes have to share a driver (the golden's is read from its PV, as CDI does), the volume modes have to match, and the target class has to allow expansion. On top of that the chart refuses only a LINSTOR target whose `allowRemoteVolumeAccess`, read the way linstor-csi reads it, is not true; without the parameter that means a layer list with no DRBD or NVMe layer. A remote-accessible LINSTOR class and other drivers render. When the provisioner or the volume mode differs, CDI falls back to a host-assisted copy, which completes, so the disk renders as before. A target class without expansion is left alone too, see the follow-ups. The class check compares nothing when a lookup it needs returns nothing: under `helm template`, before the golden PVC exists, or without the CDI resource. An empty `storageClass` is compared as the class CDI would pick for a kubevirt DataVolume, which is the default virt class, otherwise the Kubernetes default class. Existing disks are exempt, because their DataVolume spec is immutable. A disk counts as existing when its DataVolume or its PVC exists, since CDI can garbage-collect a completed DataVolume. Under `snapshot`, a snapshot class that both StorageProfiles name but that does not exist is refused too, for any driver, because CDI then fails to plan the clone instead of falling back. This is a breaking change: a new disk in a LINSTOR class without remote access from every node rendered before and is refused now, and so is one whose StorageProfiles name a snapshot class that does not exist. The first case can include a clone that would happen to work: on a class with `Immediate` binding, which I have not verified, or when the node the clone is placed on already holds a replica of the golden image. The way out is the golden image's class or the `copy` clone strategy. How this covers the three fix directions in the issue: 1. Omitting `cloneStrategyOverride` when it is empty already landed on main in #4138: the template skips the field for an empty or null value, the values schema accepts the empty string, and the chart tests pin both. This PR does not touch it. 2. I kept `csi-clone` as the default on purpose. Same-class clones are the common case, and `csi-clone` is the fast path for them. Changing the default is a separate decision, tracked in #4850. 3. vm-disk refuses the mismatch where the clone can hang, as described above. That is narrower than the issue proposed, which was to refuse every mismatch: every case CDI falls back on completes, and a clone into a remote-accessible LINSTOR class should complete too. The last part comes from reading the CDI and linstor-csi sources, I have not run it on a cluster. Follow-ups this PR does not cover: - When several classes claim the default, CDI picks the newest one. The chart does not reproduce that tie-break and compares nothing in that case. - The chart does not compare sizes. A target class without volume expansion is never refused for remote access, even when the disk is no larger than the golden image and CDI would attempt the clone. - Under `snapshot` on a cluster that does not serve the snapshot API, StorageProfiles naming a snapshot class still make CDI fail, and the chart does not look that class up. - A disk cloned from another disk (`source.disk`) into another class of the same provisioner can hang the same way, and nothing refuses it yet. - Whether `csi-clone` should remain the platform default now that it can be turned off: #4850. Closes #2908 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: No values, schema or ApplicationDefinition change. Re-check notes for the next CDI and linstor-csi bumps sit next to the version pins in the kubevirt-cdi-operator and linstor Makefiles. The new note in the vm-disk README reaches the website through the regular reference-page regeneration. ### Release note ```release-note fix(vm-disk)!: a new disk from a golden image is now refused with a clear error when CDI would CSI-clone it into another LINSTOR StorageClass that does not allow remote volume access from every node. Before, such a disk could stay pending forever with no error. A clone that would have landed on a node already holding a replica of the golden image is refused as well; use the golden image's StorageClass or the copy clone strategy. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented image-backed disks from being rendered when CDI’s selected clone strategy cannot safely clone across differing storage classes, including applicable LINSTOR remote-access and snapshot-class checks. * Existing DataVolumes and same-namespace PVCs are not blocked by these checks. Disk creation can proceed when required information is unavailable or the selected strategy supports the operation. * **Documentation** * Clarified when image-backed disks may be refused and recommended using the golden image’s storage class or the copy strategy as alternatives. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
938abdc4d2 |
fix(platform)!: refuse a networking variant change on a live cluster (#4790)
## What this PR does Changing the platform variant on a running cluster can delete its CNI, and nothing stopped that before. A new platform variant (`spec.variant` of the `cozystack.cozystack-platform` Package) can give the `cozystack.networking` Package a different variant, and the operator then deletes every HelmRelease of that Package the new variant does not install. Moving from `isp-full` to `isp-hosted` deletes Cilium and Kube-OVN, so every pod loses networking. Moving from `isp-full` to `isp-slim` deletes Kube-OVN. With this PR the platform render looks up the live `cozystack.networking` Package. If its variant differs from the one about to be rendered, the render fails, and the error names the value that lets it through: `networking.acknowledgeVariantChange: "<new variant>"`. I made the acknowledgement a variant name instead of a boolean like `publishing.proxyProtocolAcknowledgeUnclean`. A leftover `true` would let every later switch through unchecked, while a variant name covers only the switch it names. The guard does nothing when there is no live Package to compare against: `helm template`, a client-side dry-run, or a first install. It is also skipped when `cozystack.networking` is listed in `bundles.disabledPackages`, because the platform does not render the Package then. It lives in the shared Package helper and not in `bundles/system.yaml`, so it covers both places that emit the networking Package, including the `noop` call in the `isp-hosted` branch. The other Packages whose variant follows the bundle, `cozystack-scheduler` and `tenant-application`, keep the same release names in every variant, so changing their variant deletes nothing and they need no guard. The new helm-unittest cases cover the refused switches, an unchanged variant, a matching and a mismatching acknowledgement, a bare boolean, `disabledPackages`, an offline render, and a live cluster with no networking Package yet. A cluster where someone set the Helm-owned networking Package to `cilium-kilo` by hand now fails its platform upgrade, and the error points it at `bundles.disabledPackages`. Acknowledging does not help there, because the render writes the bundle's variant over `cilium-kilo`. helm-controller v1.5.0 installs a release with server-side apply by default, which takes over every rendered field. On a release it installed, the upgrade replaces the variant and the operator deletes Kilo. A release first installed with Helm 3 keeps the apply method recorded in its history, so there the outcome depends on that history. Known gap, left for a follow-up: the guard fires on any variant change, including Talos to generic (`kubeovn-cilium` to `kubeovn-cilium-generic`). Those pairs share release names, so nothing is deleted and only values change. Refusing is safe, and the error does not claim a deletion for such a switch. Closes #4596 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories This PR adds a key to `packages/core/platform/values.yaml`, and the website keeps a hand-written table of those keys in `operations/configuration/platform-package.md`. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/746 - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(platform): the platform render now refuses to change the variant of a live cozystack.networking Package, because the operator would then delete the CNI HelmReleases the new variant does not install (isp-full to isp-hosted removes Cilium and Kube-OVN). To go ahead with such a change, set networking.acknowledgeVariantChange to the new networking variant named in the error. A cluster whose cozystack.networking variant was set by hand (for example cilium-kilo) needs cozystack.networking in bundles.disabledPackages before the upgrade, otherwise the platform render refuses. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Changing the networking variant on a running cluster now requires explicit confirmation naming the requested variant. A change without a matching confirmation is blocked. * The confirmation applies only to the specified switch and must be updated for a later variant change. If the running package has no recorded variant, it is treated as using the default variant. * New installations without an existing networking package are unaffected, and networking can still be disabled through its package setting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
953b965e66 |
fix(kubernetes): keep NFS publish from hanging on a dead server (#4791)
## What this PR does When the kubevirt-csi node plugin publishes an NFS volume, it first mounts the export root into an `nfs-init-*` temp dir to create `/data` and move old files into it. That temp mount was hard. If the NFS server stopped answering at that point, the publish call never returned. kubelet gives up after 2m and retries, each retry blocked the same way, and goroutines and temp dirs piled up until the server came back or the plugin restarted. The fix does three things: 1. The temp root is now mounted `soft` with `retry=0`. It only does metadata work that a retried publish redoes, so an error there is harmless, and a stuck call now ends on its own once the connection to the server is declared dead. The mount the pod gets stays hard, because a soft mount can report a failed write that the server did apply. 2. A publish to a target path that already has one running now returns `Aborted` instead of starting another. This is what stops the pile-up: at most one call per target path is stuck at a time. 3. Moving old files into `/data` now checks the destination with `lstat` and fails the publish on any error other than a missing file. With the soft mount, the check can fail with EIO on a file that `/data` already holds, and a rename issued after the server comes back would replace that file. `lstat` keeps a user's symlink in `/data` that loops or dangles from failing every publish. Such a symlink counts as present and is left alone, while before this change the rename replaced a dangling one. I kept the default retry timeouts on purpose, although the issue suggested lowering `timeo` and `retrans`. The Linux client shares one connection per server for all of that server's mounts on the node, and the first mount sets the connection's TCP timeouts. Lowering them on the temp mount would have changed the timeouts of the pods' hard mounts too. Over NFSv4, a soft RPC fails only after the connection is declared dead, so a stuck call takes minutes to end, longer than kubelet's 2m deadline. The guard keeps kubelet's retries from adding new calls during that time. The guard works per target path, not per volume. Two pods on one node that use the same RWX volume still publish in parallel, and each pod can have at most one stuck call. The unit tests pin the mount options on both mounts, the guard behaviour including its release after a failed publish, and the destination check: a failed check stops the move, and a symlink in `/data` stays in place. They use the fake mounter. I did not test against a real NFS server that went away. Not covered here, and tracked in #4814: - If the server's TCP connection stays up while the NFS service hangs, a soft NFSv4 RPC does not time out. The guard still limits it to one stuck call per target. - `Mount` from mount-utils runs `mount` with no deadline. This predates this change, and the guard bounds it the same way. - If the target is already hard-mounted from the dead server, the check of the target at the start of a publish blocks for good. The guard keeps this to one call per target but does not end it. Unpublish is not coordinated with a publish that is still running either. Both predate this change. - A follow-up comment on the issue reported two leftovers of the bounded unmount: an orphaned `umount.nfs` after a timeout, and a possible `stat` before `umount2`. Both may end sooner now that the temp mount is soft, but I have not verified either. Refs #4581 ### Screenshots Not a UI change. ### Downstream repositories The change is limited to the kubevirt-csi node plugin's Go code. None of the triggers in the contributing guide's map apply: no package added or renamed, no platform values, variants, release assets, ApplicationDefinition semantics or developer tooling touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(kubernetes): an NFS volume publish in tenant Kubernetes clusters no longer hangs forever when the NFS server goes away during the publish, and kubelet retries no longer pile up while a publish is stuck. The plugin's temporary mount of the export root is now soft, and a second publish to the same target path is refused while the first is still running. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Concurrent NFS publish requests for the same target now return an `Aborted` error. Different targets remain available, and a target can be used again after a request finishes, including after a failed attempt. * Migration now stops if checking a destination encounters an unexpected error. Existing destinations, including symlinks, are left in place. * Temporary-directory creation and mount errors now identify the affected volume. * **Updates** * Temporary NFS root mounts now use soft behavior with retries disabled. The final data mount remains hard. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
785cbc391b |
fix(fluxcd): keep Helm uninstall working during flux-plunger recovery (#4293)
## What this PR does Fixes #4060. `flux-plunger` recovers a HelmRelease stuck with `has no deployed releases` by briefly suspending it, deleting the stale Helm storage Secret, recording a processed-version annotation, and unsuspending. helm-controller v1.5.x skips the Helm uninstall and drops its finalizer when it handles a delete on a suspended HelmRelease, and it wakes on the same delete event flux-plunger does, so a delete arriving inside that window orphaned every resource the release owns (for a VMDisk: DataVolume, PVCs, the CDI importer pod, RBAC, WorkloadMonitor and the storage Secret itself). Lifting the suspension on the delete event cannot win that race reliably. The bug is not specific to VMDisk. The change makes such a delete reach the uninstall: - **An uninstall fence.** The suspension is applied together with a flux-plunger finalizer, `flux-plunger.cozystack.io/uninstall-fence`, in one server-side apply, so flux-plunger never holds a suspension without it. If a delete arrives while the release is suspended, the fence keeps the object whether helm-controller or flux-plunger handles the delete first: flux-plunger lifts its suspension and keeps the fence, helm-controller reconciles the unsuspended generation and runs the uninstall, and flux-plunger removes the fence once helm-controller's finalizer is gone and it has observed that generation. A recovery that completes lifts the suspension and the fence together. A terminating HelmRelease never begins recovery, and termination is re-checked immediately before deleting the storage Secret. - **Only flux-plunger's own suspension is acted on.** Ownership is decided by which field manager owns `spec.suspend` in `metadata.managedFields`: the apiserver transfers the field away the moment an operator (or `cozyhr resume`, or a backup/restore controller) writes a different value to it, so a later suspension is never misread as flux-plunger's. A suspension set through the shared `flux-client-side-apply` manager (platform migrations, the tenant delete hook, an operator's manual `kubectl apply`) is never claimed. A release deleted under another manager's suspension is that manager's decision: flux-plunger does not hold it with the fence, and helm-controller leaves its resources in place, as the migrations that suspend and then delete rely on. - **No suspension taken over in a race.** Acquiring applies without forcing first and, on a conflict, re-fetches and forces only if `spec.suspend` is still `false`: a backup controller owns the field at `false` via a dynamic-client `Update`, which a non-forced apply can never take. Every apply flux-plunger sends, acquiring as well as lifting the suspension or the fence, is preconditioned on the `resourceVersion` and `uid` of the read that decided on it. The `resourceVersion` makes a write landing in between fail the apply instead of being absorbed: acquire then defers rather than adopt or take over a suspension set meanwhile, and lifting the suspension and the fence from a release read as live cannot delete it without an uninstall if helm-controller has handled a delete in the meantime. The `uid` stops an apply to a release that has gone from creating an empty one, since server-side apply creates what does not exist. A conflict is retried through the requeue. The apply carries only `spec.suspend` and the fence, so it cannot leak the required `spec.interval` and conflict on it. - **Fresh reads.** The release, every re-check and the storage Secret list are read from the apiserver (an uncached `APIReader`), not the informer cache. A lagging cache would let the processed-version guard delete the revision below the one just reclaimed, and the ClusterRole grants secrets `get`/`list` but not `watch`, which an informer needs to stay synced; flux-plunger deliberately does not cache Secrets. Every step requeues with backoff on failure. - **Events.** A Warning when flux-plunger holds a suspension it failed to clear, and a Warning when a delete has been held by the fence for more than ten minutes waiting for helm-controller (the held state is re-checked on a timer, not only on helm-controller's next status write). A release deleted under another manager's suspension gets a Normal event when that is a migration's choice, and a Warning when the release carries flux-plunger's processed-version annotation, because then the skipped uninstall is an accident, not a decision. Normal events also mark an externally-owned suspension left alone and a recovery deferred because the release changed between read and write; an external suspension on a release flux-plunger has recovered before is a Warning instead, because it is most likely one an earlier flux-plunger left mid-recovery (see Operating notes). Events from the shutdown drain are written straight to the apiserver, since the manager's recorder stops with the manager. - **The fence is released when flux-plunger goes.** Once its manager has stopped, and with it every reconcile, flux-plunger drains its fences under a deadline inside the pod's termination grace period; a stop that overran its 10s bound is drained too and still reported: a recovery in progress is aborted, the suspension and the fence lifted together for the next flux-plunger to start again, and a release being deleted has flux-plunger's suspension lifted and keeps the fence until helm-controller has run its uninstall. A release helm-controller does not handle in time keeps its fence. The drain lives in the controller, so the envtest suites hold it against a real apiserver's generations and finalizers. A pre-delete hook on the chart is the backstop for what the drain could not finish, so uninstalling the component never leaves a release it cannot finish deleting: it stops flux-plunger, waits out its grace period, then lifts any remaining suspension and fence without waiting again, naming each release it lets go of before helm-controller uninstalled it. It lifts a suspension only where flux-plunger alone holds it, the outcome the controller's own relinquish has, with a JSON patch of `spec.suspend` preconditioned on the `resourceVersion` and `uid` it read; not with `kubectl apply --server-side`, which on a release last written by a client-side `kubectl apply` moves that apply's fields to the applying manager and strips every one the body leaves out. helm-controller runs a pre-delete hook under the release's uninstall timeout, 300s by default and not set by the operator, so the Job's deadline sits under it. A Deployment that cannot be stopped or a listing that never succeeds ends the hook before any fence is touched, and a fence it cannot remove ends it after the sweep; either way it exits non-zero, because a failed pre-delete hook leaves the release untouched and helm-controller retries the uninstall, hook included, whereas exiting 0 would let the component go and strand the fence. A flux-plunger pod still there after its grace period, typically one stuck `Terminating` on a `NotReady` node, is only reported, and the pod wait is cut to what is left of the hook's retry window; because such a pod may still be setting fences, the fences are listed again after the sweep and any found fails the hook. The Job runs once and is kept after success so its report stays readable until its TTL; `helm.sh/hook-output-log-policy` is deliberately not used, because helm-controller never sets Helm's hook output writer and calls a nil one for it (fluxcd/helm-controller#1562). The hook's kubectl image is digest-pinned in the chart values, and its RBAC is only what `kubectl scale`, the listing and the patches issue. The chart's README documents the finalizer and how to clear it by hand. A single writer is kept by leader election, not by the rollout. A second process acting under the same field manager could clear a suspension the first is still recovering, and no Deployment strategy prevents one: the next pod starts as soon as the old one is terminating (Kubernetes v1.35 has no pod-replacement policy for Deployments), and an eviction or a node drain replaces a pod the same way, while the old one stops and drains. The chart now passes `--leader-elect` with a Role on `leases` in the release namespace. The Lease is not given up on stop and outlasts the last renewal, the renew deadline, the manager's bounded stop (10s) and the drain, and only a flux-plunger that led drains at all, so the next one acts only once the old one is done; the price is that a new pod waits for the Lease to run out, a little under two minutes at the default. The timings derive from one value, `shutdownGracePeriodSeconds` (90 by default, refused outside 40 to 140 at render): the pod's termination grace period, the drain deadline (25s less: the manager's 10s stop plus a 15s margin before the kubelet's SIGKILL, both tied to the Go constants by a contract test) and the hook's pod wait. The strategy keeps `maxSurge: 0` and `maxUnavailable: 1` and stays `RollingUpdate` rather than `Recreate` on purpose: the chart shipped without a strategy block, so every existing install's live Deployment carries the apiserver-defaulted `rollingUpdate` object and helm-controller applies server-side, where laying `type: Recreate` over that populated block is rejected on upgrade. ### Operating notes A HelmRelease carries the fence only while flux-plunger is recovering it or waiting for helm-controller to uninstall it. A fenced release whose uninstall keeps failing stays `Terminating` exactly as any HelmRelease does under helm-controller's own finalizer, and raises the Warning above. Stopping flux-plunger drains its fences, and uninstalling it releases whatever the drain left through the pre-delete hook; a fenced release that is deleted while flux-plunger is merely scaled to zero, or killed without a graceful stop, and that helm-controller has not uninstalled, stays `Terminating` until flux-plunger runs again. To release one by hand, after confirming its uninstall has run or is not wanted: ```bash kubectl patch helmrelease NAME --namespace NAMESPACE --type json --patch '[{"op":"test","path":"/metadata/finalizers/INDEX","value":"flux-plunger.cozystack.io/uninstall-fence"},{"op":"remove","path":"/metadata/finalizers/INDEX"}]' ``` where `INDEX` is the position of `flux-plunger.cozystack.io/uninstall-fence` in `metadata.finalizers`; the `test` operation makes the patch fail rather than remove another finalizer if the list has changed. Earlier flux-plunger releases set `spec.suspend` through the shared `flux-client-side-apply` manager, so a suspension one of them left behind cannot be told apart from a migration's or an operator's and is treated as external: it is not cleared automatically. Such a leftover exists only if the old pod stopped in the middle of a recovery, and flux-plunger raises a `SuspensionNotOwned` Warning on it. The chart README carries this section too. To list candidates, suspended releases carrying the flux-plunger annotation whose `spec.suspend` is not owned by flux-plunger's apply: ```bash kubectl get helmreleases.helm.toolkit.fluxcd.io --all-namespaces --output json | jq --raw-output '.items[] | select(.spec.suspend == true) | select(.metadata.annotations["flux-plunger.cozystack.io/last-processed-version"] != null) | select([.metadata.managedFields[]? | select(.manager == "flux-plunger" and .operation == "Apply" and .fieldsV1["f:spec"]["f:suspend"] != null)] | length == 0) | "\(.metadata.namespace)/\(.metadata.name)"' ``` Clearing one is an operator action: confirm nothing else (a migration, a restore) is holding it, then resume it with `flux resume helmrelease NAME --namespace NAMESPACE`. ### Testing The regression the issue asks for is `TestEnvtest_DeleteDuringRecoveryReachesUninstall`: against a real apiserver, a delete lands while flux-plunger holds its recovery suspension, and helm-controller's v1.5.0 delete handling is reproduced step for step (uninstall only when not suspended, finalizer dropped either way, observed generation recorded). It covers both orders, helm-controller handling the delete first and flux-plunger handling it first; without the fence the first order leaves the release gone with no uninstall. The same suite pins the managedFields ownership dynamics a fake client cannot represent: the field transfer, the forced acquire over a dynamic-`Update` owner, the `resourceVersion` precondition rejecting a write that races either apply, the boundary that a `flux-client-side-apply` suspension is never claimed, and interval preservation. The fence mechanics were also checked against a live helm-controller: with another finalizer holding a deleted, suspended release, lifting the suspension made helm-controller run the uninstall and remove the release's resources. The envtest suite runs on every pull request: a new `make test-controllers-envtest` target builds a pinned `setup-envtest` and fetches a kube-apiserver and etcd into `_out/envtest`, which CI caches and the target reuses without asking the release index, takes the HelmRelease CRD from the in-tree Flux manifests, and CI calls it next to `make test-controllers`; the control plane gets a longer start timeout because it starts beside the other test goals. The drain is held there too: it aborts a live recovery, carries a delete through the uninstall and lifts the fence only once helm-controller has observed the unsuspended generation, keeps the fence when helm-controller never answers, releases a fence left without a suspension at once, and leaves another manager's suspension alone. `TestEnvtest_TheNextFluxPlungerLeadsOnlyAfterTheDrain` runs two real managers on one Lease: a clean stop still drains, and the second manager becomes leader only after the first one's drain has ended; it goes red if the Lease is given up on stop. Unit tests cover the controller logic (every branch, ordering, requeue, and the ownership decision through an injected seam), the Lease timing relations, that only a leader drains and through the manager's API reader rather than its stopped cache, that a failed listing is retried, that a stop which overran its grace period is still drained and its events reach the apiserver, and contract tests tie `main.go`, the chart's flags, the drain's arithmetic and the hook's names to the Go code. The pre-delete hook is covered by helm-unittest and by `hack/flux-plunger-fence-hook_test.bats`, which runs its rendered script against a fake `kubectl` that keeps each release's finalizers, suspension owner, generation and observed generation, bumps the generation on a spec write as the apiserver does, answers NotFound without `--ignore-not-found`, spends simulated time on waits, lists a release once however many copies of the fence it carries, refuses `kubectl apply`, lifts a suspension only through the preconditioned JSON patch and never one another manager also holds, reads a negative `kubectl wait` timeout as a week as kubectl does, and applies a finalizer removal only when it is the guarded test-then-remove at an index that holds the fence. Every mutation in a set covering each predicate conjunct, each retry, each exit path, the re-listing, the timings and the selector turns the hook suite red, as does every mutation in the Go set covering the drain's branches, the Lease timings and the events; three drain mutations that still survive are tracked in #4782. The hook script was also run as a Job in the hook's own image on a live cluster, succeeding, with the Deployment already gone, and with patch denied by RBAC, where it exited non-zero and named the release it left; that run caught that a per-call `--request-timeout` makes kubectl drop its in-cluster config, so the hook sets none and the fake rejects it. A later live run caught `kubectl apply --server-side` stripping the finalizers and spec of a release created by a client-side apply; with the JSON patch the same releases kept every field, the suspension flux-plunger alone held was lifted, a co-owned and a foreign one stayed, and every fence went. ### Remaining limitations - A delete that arrives after recovery has deleted the storage Secret of a release whose only revision was that one finds no revision to uninstall; helm-controller then treats the release as already gone. - A writer that sets `spec.suspend: true` while flux-plunger already holds it at `true` does not take the field over, because the value does not change. If such a writer then deletes the release inside the recovery window (the suspend-then-delete that the tenant delete hook and some platform migrations use to skip the uninstall), flux-plunger still sees its own suspension, lifts it, and helm-controller runs the uninstall that writer meant to skip. The same held before this change, when the end of recovery unsuspended unconditionally. - The already-processed guard compares revision numbers only. When Helm re-creates a revision under the same number after recovery deleted it (a release whose only revision fails again the same way), recovery runs again for it. - `listHelmReleaseSecrets` reads the local cluster only, so a HelmRelease that targets a remote cluster via `spec.kubeConfig` (whose storage Secrets live in the remote cluster) is not recovered; it is skipped, never mis-handled. - A new flux-plunger pod waits for the old one's Lease to run out before it acts, on every restart and after a crash: a little under two minutes at the default. Recovery is not latency-sensitive, and this is what keeps a single writer while the old pod drains. - The leader Lease is left in the release namespace after the chart is uninstalled, as with any controller-runtime leader election; it is inert. ### Screenshots Not applicable: no UI change. ### Downstream repositories The trigger map was walked against the diff: it touches `internal/controller/fluxplunger`, `cmd/flux-plunger`, the `packages/system/flux-plunger` chart (templates including RBAC for the leader Lease, a pre-delete hook, values, tests, its package `Makefile`, README), a new bats test under `hack/`, the root `Makefile` (a new `test-controllers-envtest` target; existing targets are unchanged) and the pull-request workflow that calls it and caches its binaries. No row matches: ccp gates on existing `hack/` files and make targets, and nothing there is moved, renamed or changed in behaviour; a new test file and a new target reach none of them. The website's FluxPlunger section describes the recovery steps, which still hold; it does not describe the fence or the drain, which the chart README now documents. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(fluxcd): a HelmRelease deleted while flux-plunger is recovering it now reaches the Helm uninstall instead of leaving its resources (DataVolume, PVCs, RBAC, storage Secret) behind ``` |
||
|
Mattia Eleuteri
|
0c62bee4da |
feat(bucket): add a ui flag to skip the s3manager web UI
Every bucket ships an s3manager Deployment with its Service and an Ingress or HTTPRoute on <bucket>.<tenant host>. Tenants that only use the S3 API pay for a pod per bucket and for one DNS name and TLS certificate per bucket, none of which they need. ui defaults to true, so existing buckets keep their UI. With ui set to false the bucket no longer needs the tenant apex either, since the UI hostname was its only consumer in the bucket-system chart. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
7c46ffff12 |
fix(backupstrategy-controller): run backup clients on a package-owned image (#4756)
## What this PR does The Altinity (ClickHouse) and RabbitMQ backup strategy Pods ran `platform-migrations`: a 349 MB image carrying `kubectl`, `helm`, `git`, `cozyhr` and `etcd-migrate`, whose `ENTRYPOINT` is `run-migrations.sh`. Both pins were stamped from `packages/core/platform/Makefile`, so any migration change re-digested the backup clients too, and only the `command:` override kept the Pods from running platform migrations (#3420). - New `packages/system/backupstrategy-controller/images/backup-http-client/`: `alpine:3.24` (digest-pinned, the same base the tree already uses) + `curl` + `jq` + `ca-certificates`, `USER 65532:65532`, no `ENTRYPOINT`. Everything is installed at build time, so the Pods still never run `apk add` and keep working air-gapped. - New `image-backup-http-client` target in the package's own `Makefile`, chained after `image-redis-backup` like the existing ones so only one `yq -i` writes `values.yaml` at a time. It stamps `chBackupClientImage` and `rabbitmqBackupClientImage` with the same ref. - Removed the cross-package stamps from `packages/core/platform/Makefile`, the WORKAROUND/TODO comments in the chart and the RabbitMQ example, and the stale "live violation" line in `docs/agents/image-refs.md`. - `hack/image-pin-consistency.bats`: dropped the `platform-migrations`-specific test, whose subject no longer exists. The general check (no repository pinned at more than one digest, computed from `hack/promote-retag.sh --dry-run`) still covers the two keys sharing one image. **Why one image for both strategies rather than `ch-backup-client` as the issue named it.** The RabbitMQ strategy had the same reuse and the same cross-package stamp; leaving it would have kept the stamp alive. Its script needs `curl --aws-sigv4` + `ca-certificates`, a subset of the ClickHouse one, so one image serves both. The two values keys stay separate so either strategy can move to its own image later. **Initial pin.** The image does not exist in ghcr until CI builds it, so both keys are committed as `backup-http-client:v1.6.0` without a digest, the same way `redis-backup` was introduced. PR CI rebuilds touched packages and overlays the fresh refs for e2e; release preparation's `make image` stamps tag+digest. Image size: 6.9 MB vs 349 MB. Verified locally: - `docker buildx build` for `linux/amd64,linux/arm64`; in the image as uid 65532: curl 8.22 with `--aws-sigv4`, jq 1.8.2, `date`, `seq`, writes to `/tmp` (the RabbitMQ script's only write), TLS verification against the bundled CA store. - `helm unittest` (23/23), `helm template` renders both strategies with `backup-http-client` and no `platform-migrations` reference. - `hack/cozytest.sh hack/image-pin-consistency.bats` and `hack/build-matrix_test.bats` pass. Not yet run end to end on a cluster; the e2e backup round-trips will exercise it. Fixes #3420 ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map against the diff: it touches `packages/system/backupstrategy-controller` (new image, Makefile target, values comments/pins, template comments), `packages/core/platform/Makefile` (stamp removal only, no `values.yaml` key change), `hack/image-pin-consistency.bats`, `docs/agents/image-refs.md` and a comment in `examples/backups/rabbitmq`. No app package, schema, CRD, variant, asset, platform values key or `hack/package.mk` change. The website does not list first-party image repositories (searched for `platform-migrations` and `redis-backup`), so no mirror list needs the new one. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(backupstrategy-controller): the ClickHouse (Altinity) and RabbitMQ backup strategy Pods now run a dedicated `backup-http-client` image (alpine + curl + jq + ca-certificates, non-root, ~7 MB) instead of the 349 MB `platform-migrations` image. Air-gapped mirrors need the new `ghcr.io/cozystack/cozystack/backup-http-client` repository. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Altinity and RabbitMQ backups now use a dedicated backup HTTP client image, configured separately from the platform migrations image. * The client image includes the tools required for backup operations and runs with a non-root identity. * Both backup configurations receive the same digest-pinned image reference when the image is built. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8deaa8e14c |
fix(vm-disk): name the importer error in the disk's WorkloadsReady message (#4815)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does A VMDisk whose HTTP source answers 404 or does not resolve never finishes importing, and since #4380 its `WorkloadsReady` condition says only `DataVolume vm-disk-x is ImportInProgress`. That reads the same as a large download that is still running. CDI fails an import only on a fatal error (a checksum mismatch) and restarts the importer pod otherwise, so the phase never moves, and the reason the disk will not finish was visible only in the importer logs. CDI already keeps that reason on the DataVolume: the `Running` condition goes `False` with the importer's termination message, and keeps it through the back-off between restarts. The WorkloadMonitor now appends it, so the condition reads `DataVolume vm-disk-x is ImportInProgress: Unable to connect to http data source: expected status code 200, got 404. Status: 404 Not Found`. The console already shows the `WorkloadsReady` message on the disk's overview page, so no UI change is needed. The same text covers an unschedulable importer, a failed image pull and a checksum failure, since CDI reports all of them through `Running=False`. Only `Running=False` is read: `Unknown` is not a failure, and `Bound` repeats the same error behind PVC details. For a host-assisted clone CDI joins the target and source pod messages as `<target> and <source>`, which comes out as ` and ` when both pods are still being created, so empty halves are dropped. Behaviour to be aware of: while the importer pod is briefly running between restarts, CDI flips `Running` to `True` and clears the message, so the condition shows the bare phase for those seconds. This is not a terminal state either: a disk with a broken source still stays `ImportInProgress` and keeps retrying. Deciding when such an import should count as failed is left for a separate change. ### Tests A table test covers a 404, an unresolvable host, an unschedulable importer, a fatal checksum error, a DataVolume without a phase and joined clone messages with one or both halves empty, plus the cases that must not add text: an importer that has not started, a running importer, an `Unknown` condition, a populated disk and an error that only sits on `Bound`. Each conjunct of the condition check and the append itself were mutated and each mutant fails the test. A second test runs against a DataVolume captured from a test cluster after its importer had restarted about two thousand times on a 404, with only names and the URL changed, so the parsing is pinned against what CDI actually writes rather than a hand-built shape. ### Screenshots No UI code changes: the console renders the `WorkloadsReady` message it already receives. A message that carries a long source URL is not wrapped on the overview card; that is a styling follow-up for the console and is left out of this change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The only non-Go change is a sentence in the `packages/apps/vm-disk/README.md` notes; no values, schema, API kind or default changes. The website reference page for vm-disk is regenerated from that README by the release bot. ### Release note ```release-note fix(vm-disk): the WorkloadsReady condition of a VM disk now includes the importer error reported by CDI, such as a 404 or an unresolvable host, so a disk whose source keeps failing shows why it is stuck instead of only ImportInProgress. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Workload status messages now include available DataVolume import failure details, such as HTTP errors or DNS resolution problems, making the cause of an import issue clearer. * **Documentation** * Updated the workload readiness documentation to explain when DataVolume phase and import error details appear in status messages. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
38d6ac7efb |
feat(redis): expose min-replicas-to-write and min-replicas-max-lag (#4848)
## What this PR does Adds two optional values to the Redis app, `minReplicasToWrite` and `minReplicasMaxLag`, rendered as `min-replicas-to-write` and `min-replicas-max-lag` in the RedisFailover `spec.redis.customConfig`. Both are unset by default, so the rendered config and behaviour are unchanged for existing instances. Replication is asynchronous, so a master that is isolated from its replicas keeps accepting writes until Sentinel fails over, and those writes are lost. With `minReplicasToWrite: 1` the isolated master stops accepting them after `minReplicasMaxLag` seconds, which bounds the loss. Trade-off: with the default `replicas: 2` there is a single replica, so the master also refuses writes whenever that replica is down (node drain, restart, upgrade). Users who turn this on accept lower write availability for a bounded loss window. Includes helm-unittest cases and the regenerated schema, README, Go types and ApplicationDefinition. ### Screenshots ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/68 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(redis): add optional minReplicasToWrite and minReplicasMaxLag values to bound writes lost on failover; unset by default ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional Redis settings to require a minimum number of replicas to acknowledge writes and to set the maximum age of those acknowledgements. * Invalid settings are rejected: the required replica count must be nonnegative and below the configured replica count, and the acknowledgement age must be at least 1 when a replica requirement is set. Replication remains asynchronous, so these settings do not eliminate potential write loss during failover. * **Documentation** * Documented the settings, defaults, validation, and behavior in Redis configuration guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
95c2d85b5b |
fix(etcd): leave compaction to etcd and pass the real quota to defrag (#4873)
## What this PR does The hourly defrag CronJob ran `etcd-defrag` with its defaults, and two of them do not fit the cluster the chart creates. - `--compaction` (default `true`) compacts to the latest revision before every defrag. The EtcdCluster already compacts itself (`autoCompactionMode: periodic`, `autoCompactionRetention: 5m`), so the extra compaction either fails with `mvcc: required revision has been compacted` when nothing was written since the last auto-compaction, which is what every defrag Job log shows on our clusters, or, on a busy cluster, drops the history the retention keeps for watchers to resume from. - `--etcd-storage-quota-bytes` defaults to 2 GiB, so `dbQuotaUsage` in the defrag rule is measured against 2 GiB rather than the `quotaBackendBytes` the chart sets (95% of `size`, about 3.8 GiB for the default `4Gi`). The CronJob now passes `--compaction=false` and the chart's own quota through the existing `calculateQuotaBackendBytes` helper. The new unit tests check both arguments and that the defrag quota equals the EtcdCluster's `quotaBackendBytes` for a non-default size. Without the template change all three fail. The same templates are on `release-1.6` and the change applies there cleanly. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff is two template arguments and a unit test in `packages/extra/etcd`, with no change to values, schema, defaults or package names, so nothing in the trigger map applies. ### Release note ```release-note fix(etcd): the defrag job no longer compacts on top of etcd's periodic auto-compaction, and it evaluates its 80% quota threshold against the cluster's real quotaBackendBytes instead of 2 GiB. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * The etcd defragmentation job now uses the cluster’s configured backend storage quota and skips compaction before defragmenting. This keeps the defragmentation operation aligned with the cluster’s storage settings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
647407545d |
fix(backupstrategy-controller): run backup clients on a package-owned image
The Altinity and RabbitMQ strategy Pods ran platform-migrations, a 349 MB image carrying kubectl, helm and etcd-migrate whose ENTRYPOINT is the migration runner, and whose pins were stamped from packages/core/platform so every migration change moved the backup clients too. Build images/backup-http-client (alpine + curl + jq + ca-certificates, non-root) from this package's Makefile and stamp both client keys from it. The cross-package stamp in packages/core/platform/Makefile and the platform-migrations-specific check in hack/image-pin-consistency.bats go away; the general one-digest-per-repository check still covers the two keys sharing one image. Fixes #3420 Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |