mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-12 23:54:01 +09:00
f48a94de6d53d23801f4a27b016090e03ff24632
1869
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 ZverGuy
|
b703a54cb4 |
fix(tenant): drop the tenant egress rule to the observability proxy
The rule was added for a tenant Grafana reading through the proxy, but the tenant Grafana datasources still point straight at vlselect and vmselect, and the console reaches the proxy from cozy-dashboard. The rule therefore served no caller and only let every pod of every tenant reach the proxy directly, outside the console route, where each request with a fresh token costs a TokenReview. It can come back together with a tenant client that needs it. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
f7719cf6e2 |
feat(tenant): allow egress to the observability proxy
A tenant's Grafana reads the tenant's logs and metrics through the observability proxy, and tenant egress is default-deny with one allow-to-* policy per platform service. The rule is narrowed to the proxy pods and their port, since the proxy's namespace holds nothing else a tenant should reach. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
myasnikovdaniil
|
4087218a02 |
docs: move the repo-only guides onto the website (#5034)
## What this PR does Seven guides under `docs/` were operator facing and written alongside the code, and left in the source tree where no user looks. They are on the website now, so this deletes them here and repoints every reference. ``` docs/operations/backup-classes.md -> operations/services/backup-classes docs/operations/resource-presets.md -> guides/resource-management docs/storage-immutability.md -> operations/storage-class-immutability docs/operations/system-memory-limits.md -> operations/configuration/system-memory-limits docs/gateway-classes.md -> networking/gateway-classes docs/observability/distributed-tracing.md -> operations/services/monitoring/distributed-tracing docs/operations/seaweedfs-capacity-for-backups.md -> operations/services/object-storage/capacity-for-backups ``` `docs/release.md`, `docs/deprecating-applications.md` and the two SeaweedFS runbooks in `docs/operations/` stay. A workflow, `AGENTS.md`, a migration and a bats suite read them by path, which is the test from #3777. References point at `https://cozystack.io/docs/v1/...`. The site rewrites that short prefix to the latest version in `layouts/404.html`, so the links follow releases and pin none. Backup Classes, resource management and `storageClass` immutability exist in the latest version already. The other four pages are only in `next/` until 1.7.0 is promoted, and their links start to resolve then. The generated mongodb files come from `values.yaml` through `make generate` with cozyvalues-gen v1.7.0, the version pinned in `.github/workflows/pre-commit.yml`. Tests: helm unit tests for mongodb (151), postgres (209), vm-disk (114) and kubernetes-nodes (223), go tests for backupcontroller, presets and the operator, rd-presets-check, the migration-50 bats suite. ### Downstream repositories - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#763 Walked the rest of the trigger map against the diff. No provider follow-up: the mongodb `values.schema.json` diff is one `description` string, with no field added, removed or renamed, no enum, no default, and no `kind`, `plural` or `release.prefix` change. `packages/core/platform/values.yaml` changed in comments only and no key, so the hand written platform-package table on the site is unaffected. Please merge after cozystack/website#763, the links in the code only resolve once those pages deploy. ### Release note ```release-note docs: seven guides move from the docs/ directory of this repository to cozystack.io (Backup Classes, resource presets, storageClass immutability, system component memory limits, GatewayClass selection, distributed tracing, SeaweedFS capacity for backups), and in-repo references point there. ``` |
||
|
IvanHunters
|
8d96c46994 |
fix(monitoring-agents): isolate merged log payload under log_processed
Fluent Bit's kubernetes filter runs with Merge_Log On but without Merge_Log_Key, so a parsed JSON log line is merged into the record root. An application log line can thereby forge the service fields that later pipeline stages rely on for routing and tenant isolation (kubernetes_namespace_name, kubernetes.labels, tenant, cluster, log_source). Setting Merge_Log_Key log_processed nests the parsed payload under a dedicated key so application content can no longer override top-level service fields. Applies to both the platform (system/monitoring-agents) and guest (apps/kubernetes) Fluent Bit configurations. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
3abdff5900 |
fix(kubernetes-nodes): refuse a partial resources spec on every pool (#4973)
The `resources.cpu`/`resources.memory` pair check in `nodegroup.yaml` ran only when an `instanceType` was set, while the field description (values.yaml, README, schema, the KubernetesNodes OpenAPI schema) promised a render-time error for a partial spec on any pool. Without an instancetype a cpu-only pool renders a VM with no memory, which KubeVirt refuses when the Machine is created, and a memory-only pool silently sizes the guest at KubeVirt's default of one vCPU. - The check now runs on every kubevirt pool. The proxmox branch already required both, so this is the kubevirt half of one rule, and the field descriptions are true as written (no doc change). On a pool with no `instanceType` the message ends in "or neither and set instanceType", so it does not send the user straight into the next refusal. - A kubevirt pool with neither an `instanceType` nor both resources fails at render with a message naming both, the way `vm-instance` does. No such pool could have passed admission (no memory), so this refusal breaks nothing that runs today. This is the decision the issue asked for; if the gate should stay narrower, this commit can be dropped on its own. - **Upgrade impact.** One shape that renders on main stops rendering: `instanceType: ""` with only `resources.memory`. On main it gets `domain.memory.guest` and no `domain.cpu`, so KubeVirt runs the worker with one vCPU; with this PR it fails with the partial-spec message, and the pool's HelmRelease sits in a failed upgrade until the values are edited. To keep the current size such a pool needs `resources.cpu: "1"`. Adding that line renames the machine template (for the test values, `kubernetes-myk8s-md0-28d812` becomes `kubernetes-myk8s-md0-1ce103`, the same name on main and on this branch), so the pool's workers are replaced once. cpu-only with `instanceType: ""` is not in this class: it renders a VM with no memory, which KubeVirt refuses, so it never ran. - `cores:` printed an unquoted (float64) core count of one million or more in exponent form (`1e+06`). It is printed as an integer now, with the same bytes for every smaller whole number. Tests: `tests/resources_partial_test.yaml` covers both halves with and without an instancetype, the no-sizing case, both fields set, and the float64 formatting. The existing unquoted-zero case in `resources_cpu_test.yaml` now reads the field as unset on an instancetype-sized pool (with a `kubernetesProvider` stub for the lookup), and zero beside memory is pinned as a partial spec. Byte stability, checked by snapshotting the `KubevirtMachineTemplate` on main for nine spellings of `resources.cpu` and replaying the snapshots on this branch: `"2"`, `2`, `2.0`, `"2.0"`, `"+2"`, `"1e1"`, `"010"`, `999999` render identical bytes and template names. Only unquoted `4294967295` changes, from `cores: 4.294967295e+09` to `cores: 4294967295`. Verified with `helm unittest .` in `packages/apps/kubernetes-nodes` (38 suites, 233 tests, 12 snapshots), and on a trial merge with current main (39 suites, 237 tests). Closes #4546. The leading-zero inconsistency from the issue's comment is addressed separately, together with #4554. ### Release note ```release-note fix(kubernetes-nodes): a partial `resources` spec (only `cpu` or only `memory`) is now refused at render time on every kubevirt worker pool, not only on a pool with an `instanceType`, and a pool with neither an `instanceType` nor both resources is refused as well. Action required: a pool with `instanceType: ""` and only `resources.memory` rendered before (KubeVirt ran it with one vCPU) and now fails to upgrade; add `resources.cpu: "1"` to keep its current size. Adding that line renames the pool's machine template, so its workers are replaced once. Whole-number core counts of one million or more are now printed as integers instead of in exponent form. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * KubeVirt worker pools now require either an instance type or both CPU and memory values. Configurations with only one resource or no sizing information are rejected. * Whole-number CPU values render without exponent notation, including values written with a decimal fraction. * An unquoted zero CPU value is treated as unset when an instance type is provided; without an instance type, pairing zero CPU with memory fails validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
60d1f53e82 |
fix(kubernetes-nodes): plan autoscaler capacity on the cores the VM actually gets (#4974)
On the kubevirt substrate `resources.cpu` is written into `domain.cpu.cores` as spelled and decoded as YAML, so `"010"` gives the VM eight cores, while the cluster-autoscaler capacity annotation parsed the same string as decimal ten (`capacity.cluster-autoscaler.kubernetes.io/cpu: "9000m"` on a node that offers 7000m). This is the inconsistency reported in #4554 and in the comment on #4546. - The YAML-decoded value the range check already computes is kept, and on kubevirt the capacity annotation and the reserved-CPU check start from it. - The kubelet reservations are not touched. The worker machineconfig (`_talosconfigtemplate.tpl`) sizes them from the raw string read as a decimal, and the TalosConfigTemplate name is a hash of that config, so reading the decoded value there would roll every leading-zero pool once. Instead the annotation subtracts the reservations the machineconfig really carries from the cores the VM really has: for `"010"` that is 8000m − 2 × 500m = `"7000m"`, the node's real allocatable, while `"8"` keeps `"7200m"` (2 × 400m). Tests pin both sides: the annotation and the bootstrap `configRef` name in `resources_cpu_test.yaml`, and `systemReserved.cpu` / `kubeReserved.cpu` in the machineconfig in `talos_worker_config_test.yaml`. - No pool rolls. Checked by rendering `nodegroup.yaml` on main and on this branch for nine spellings (`"2"`, `2`, `2.0`, `"2.0"`, `"+2"`, `"1e1"`, `"010"`, `999999`, `4294967295`): the only difference is the capacity annotation of the `"010"` case, so the `KubevirtMachineTemplate` name and the TalosConfigTemplate `configRef` stay as they are. The machineconfig template itself is not changed. - Proxmox parses the quantity as decimal for the machine template and for the reservations alike, so it never had the inconsistency and is unchanged (pinned by a test). - The `resources` description says the value is decoded as YAML, what that means for a leading zero, and that the reservations still read it as a decimal. README, schema, the RD and the Go type are regenerated from it. This closes #4554 without the migration and the render-time refusal it proposes. A refusal at render breaks a stored pool on its next upgrade, a pattern in the OpenAPI schema breaks every update of an existing CR, and a platform-wide migration over every `KubernetesNodes` and parent `nodeGroups` entry is a lot of blast radius for a spelling nobody has reported in a live cluster. With this change the autoscaler plans for the node the VM really is. What is left of a leading zero is the octal reading itself and reservations sized from the decimal value, and both are documented. The unquoted zero of the issue is already harmless: it is falsy, counts as unset, and renders the same bytes as an absent field. If a migration is still wanted, please reopen. Verified with `helm unittest .` in `packages/apps/kubernetes-nodes` (38 suites, 233 tests, 12 snapshots; pointing the annotation back at the decimal string turns the `"010"` case red with `9000m`), `make generate` (no diff), and a trial merge with #4973 (clean, 243 tests green). Closes #4554 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * KubeVirt VM sizing and autoscaler capacity now reflect YAML-decoded CPU values. For example, `"010"` represents eight VM cores, while kubelet reservations interpret it as decimal ten. `"8"` and `"10"` yield `7200m` and `9000m` of autoscaler capacity, respectively. Proxmox continues to interpret `"010"` as ten cores, yielding `9000m` of capacity. * **Documentation** * Clarified explicit worker CPU and memory sizing behavior, including how leading-zero CPU values affect VM sizing, reservations, and capacity calculations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
44848b9c9d |
fix(kubernetes): run Proxmox CCM v0.16.1 and pin the Proxmox images by digest (#4917)
## What this PR does
`v1.7.0-rc.1` and `release-1.7` ship the Proxmox
cloud-controller-manager as
`ghcr.io/sergelogvinov/proxmox-cloud-controller-manager:v0.15.0`, a bare
tag. That tag was re-pushed upstream on 2026-09-27: it used to name the
2026-08-06 build (`sha256:fbc553921145…`) and now names
`sha256:d81702998ae1…`, a binary whose gitVersion is `0.15.0` without
the leading `v`. component-base v0.37 parses that string in an `init()`
and panics:
```
panic: version string "0.15.0" doesn't match expected regular expression: "^v(\d+\.\d+\.\d+)"
```
The CCM Deployment crash-loops, no new Node loses
`node.cloudprovider.kubernetes.io/uninitialized`, and workloads on new
workers stay Pending. On a live Proxmox cluster this went unnoticed for
five days: the existing Nodes were already initialized, and the failure
only showed when the next new worker came up. Every fresh install pulls
the broken build, so as it stands v1.7.0 would be the first stable
release with the Proxmox substrate, shipping a CCM that cannot start.
Two commits, both cherry-pick onto `release-1.7` unchanged:
1. **`fix(kubernetes)`**: run v0.16.1, pinned as tag plus digest in
`images/proxmox-cloud-controller-manager.tag`. That is the same shape
the chart already uses for the KubeVirt cloud provider. v0.16 builds
with goreleaser and stamps the version with its `v`. The config the init
container writes still parses: the cluster entry moved to
go-proxmox-pool's `ClusterConfig` with the same `url`, `insecure`,
`token_id`, `token_secret` and `region` keys, and `features.provider:
capmox` still writes `proxmox://<SMBIOS UUID>`, the providerID capmox
puts on the Machine.
2. **`fix(kubernetes, proxmox-csi)`**: pin the CSI driver from the same
registry at its current v0.20.0, with no version change. The
management-side controller goes through
`images/proxmox-csi-controller.tag`. The tenant-side node plugin goes
through `repository` + `tag` in
`packages/system/proxmox-csi/values.yaml`, which also puts it in
`hack/lib/image-refs.sh` and so in front of the release multi-arch gate.
Pinning takes the version out of the vendored chart's hands, so
`hack/proxmox-csi-driver-pin_test.bats` now fails unless the chart's
`appVersion` and both pins name the same release.
All three digests are OCI indexes for `linux/amd64` and `linux/arm64`.
The sig-storage sidecars stay on tags, as they do for the KubeVirt CSI
driver: registry.k8s.io publishes through image promotion, which does
not move a tag.
### Testing
- `helm unittest`: `packages/apps/kubernetes` 31 suites / 346 tests,
`packages/apps/kubernetes-nodes` 37 / 223, all passing.
- bats: the new test, plus `image-pin-consistency`,
`promote-rewrite-tags_test`, `overlay-main-images_test`,
`nightly-mirror_test`, `promote-retag_test`, `stitch-multiarch_test`,
`bats-strict-setup`, `bats-runner-coverage`: 107/107. The new test was
checked to fail on a node tag at another version.
- `git cherry-pick -x` of both commits onto `release-1.7` (
|
||
|
Marian Koreniuk
|
f7913329b2 |
fix(kubernetes-nodes): plan autoscaler capacity on the cores the VM actually gets
On the kubevirt substrate resources.cpu is written into domain.cpu.cores as spelled and decoded as YAML, so "010" gives the VM eight cores, while the cluster-autoscaler capacity annotation parsed the same string as decimal ten. The autoscaler then planned 9000m on a node that offers 7000m. Keep the YAML-decoded value the range check already computes and start the capacity annotation and the reserved-CPU check from it on kubevirt. The kubelet reservations stay as they are: the worker machineconfig sizes them from the decimal string, and the TalosConfigTemplate name is a hash of that config, so changing them would roll every leading-zero pool once. The annotation therefore subtracts the reservations the machineconfig really carries (2 x 500m for "010") from the cores the VM really has, which is the node's real allocatable. Neither template name changes for any spelling, so no pool rolls. Proxmox parses the quantity as decimal on both sides and is unchanged. The field description says the value is decoded as YAML and that the reservations still read it as a decimal. This closes #4554 without the migration and the render-time refusal it proposes: a refusal breaks stored pools on their next upgrade, a schema pattern breaks updates of existing CRs, and a platform-wide migration of every KubernetesNodes and nodeGroups entry is a lot of blast radius for a spelling nobody has reported in a live cluster. Closes #4554 Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
780b068df0 |
fix(kubernetes, proxmox-csi): pin the Proxmox CSI driver images by digest
The CSI controller in the management cluster was referenced by the bare tag ghcr.io/sergelogvinov/proxmox-csi-controller:v0.20.0, and the node plugin in the tenant by the vendored chart's default, its appVersion as a bare tag. That registry is the one whose proxmox-cloud-controller- manager:v0.15.0 was re-pushed to a build that panics, so a tag there is not a statement of what runs. Both now carry the v0.20.0 digest: the controller through images/proxmox-csi-controller.tag, like the cloud-controller-manager, and the node plugin through repository and tag in the proxmox-csi values, which also makes it visible to hack/lib/image-refs.sh and therefore to the release multi-arch gate. Both digests are indexes for linux/amd64 and linux/arm64. Pinning takes the version out of the vendored chart's hands: a chart bump would bring a new appVersion while both pins kept naming the old release, and the controller and the node plugin would drift apart from the templates and from each other without anything noticing. hack/proxmox-csi-driver-pin_test.bats fails unless the chart's appVersion and both pins name the same release. The sig-storage sidecars stay on tags, as they do for the KubeVirt CSI driver: registry.k8s.io publishes them through image promotion, which does not overwrite a tag. Assisted-by: LLM Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> |
||
|
Marian Koreniuk
|
05a347e6f6 |
fix(kubernetes): run proxmox-cloud-controller-manager v0.16.1, pinned by digest
The Proxmox CCM was referenced by its bare v0.15.0 tag, and that tag no
longer names the image it named when the template was written. It was
re-pushed on 2026-09-27 (index sha256:d81702998ae1, replacing the
2026-08-06 build sha256:fbc553921145) to a binary whose gitVersion is
"0.15.0" without the leading v. component-base v0.37 parses that string
in the init() of its legacy metrics registry and panics:
panic: version string "0.15.0" doesn't match expected regular
expression: "^v(\d+\.\d+\.\d+)"
so the Deployment crash-loops, no Node ever loses the
node.cloudprovider.kubernetes.io/uninitialized taint, and workloads on a
new worker stay Pending. Any node that pulls the tag fresh gets it, which
is every fresh install.
v0.16 builds with goreleaser and stamps gitVersion as v{{.Version}}. The
configuration this template writes is read unchanged: the cluster entry
moved to go-proxmox-pool's ClusterConfig with the same url, insecure,
token_id, token_secret and region keys, and features.provider: capmox
still writes proxmox://<SMBIOS UUID>, the providerID capmox puts on the
Machine.
The reference now lives in images/proxmox-cloud-controller-manager.tag
as tag plus digest, the shape the chart already uses for the KubeVirt
cloud provider, so a moving tag can no longer change what runs.
Assisted-by: LLM
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
|
||
|
Marian Koreniuk
|
0e77e2b10f |
fix(kubernetes-nodes): name instanceType in the partial-spec refusal
On a pool with no instanceType, "set both resources.cpu and resources.memory, or neither" sent the user straight into the next refusal, since neither sizing source is refused as well. The message now ends in "or neither and set instanceType" on such a pool; a pool sized by an instancetype keeps the old wording. The tests anchor both spellings, so the suffix cannot leak onto, or drop off, either kind of pool. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
ae9e7c4c64 |
test(kubernetes): pin RetryOnFailure on the Proxmox csi release (#4969)
The KubeVirt csi and cilium releases carry a helm-unittest pin on `strategy.name: RetryOnFailure` for both actions, and the e2e HelmRelease guard in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` names those two tests as load-bearing. `helmreleases/csi-proxmox.yaml` sets the same strategy for the same reason and had no pin. This adds `packages/apps/kubernetes/tests/csi_proxmox_install_retry_test.yaml`, a mirror of `csi_install_retry_test.yaml` rendered on the proxmox substrate, and updates the guard comment that pointed at the gap. No template change. Verified with `helm unittest .` in `packages/apps/kubernetes` (32 suites, 349 tests). Closes #4731 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Expanded automated coverage for Proxmox CSI installation and upgrade retry behavior, including the retry interval and remediation retry limits. The checks verify that retries continue according to the configured settings during installation and upgrades. This update does not change user-facing functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
1ac83aebd9 |
fix(tenant): stop the gateway release waiting on TenantGateway listeners and certificates (#5086)
## What this PR does After platform upgrade the tenant `gateway` HelmRelease fails with `timeout waiting for: [TenantGateway/<namespace>/cozystack status: 'InProgress']`, retries every 30s without end (`RetryOnFailure`) while holding a helm-controller worker, and parent tenant release that waits on it fails too. helm-controller waits on every object of the release with kstatus on install and on every upgrade, and kstatus reads TenantGateway `Ready=False/ListenersNotReady` as InProgress. TenantGateway stays there while any listener certificate is pending, which depends on ACME or DNS and on routes owned by other releases, so waiting longer doesn't help. It passed before only because the wait runs inside a Helm action and the install happened before those listeners showed up. I added `healthCheckExprs` for `TenantGateway` with `current: "true"` and pinned `waitStrategy: poller`, because expressions are evaluated only under poller. Wait stays on. Flux returns InProgress while `status.observedGeneration` lags `metadata.generation` before it reads any expression, so release still waits until controller observed the new TenantGateway spec, but not for listeners or certificates. tenant-root renders same template and gets the change too. Gateway release that is already stuck picks this up on its next attempt, after the current one times out. New helm-unittest suite (`packages/apps/tenant/tests/gateway_wait_test.yaml`) pins the strategy and the whole expression list, checks there is no `disableWait` on install or upgrade, and that no gateway release is rendered when `gateway` is unset. ### 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(tenant): the tenant gateway release no longer times out on upgrade while TenantGateway listener certificates are pending, it waits only until the controller has observed the current TenantGateway spec. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Gateway releases now determine readiness from the gateway resource’s current status, rather than waiting for its listeners and certificates to become ready. Releases still wait for the latest resource update to be observed. * **Tests** * Added checks for gateway release readiness behavior and for tenants configured without a gateway. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
749d401899 |
fix(kubernetes, opensearch, foundationdb)!: restrict managed application images (#5085)
## What this PR does
Image overrides let tenants run arbitrary containers in management
cluster, including helpers with access to Talos CA secrets and
Kubernetes API credentials. This PR removes those overrides and uses
images shipped by the platform.
Replaces [the original
PR](https://github.com/cozystack/cozystack/pull/4287) for 1.7. Andrei
Kvapil's commits keep his authorship.
- Kubernetes and KubernetesNodes helpers use the bundled image
references through `cozy-lib.image`. OpenSearch uses its version map.
Talos installer settings for tenant worker VMs stay available.
- FoundationDB uses `version: v7.3` or `v7.4`, mapped to `7.3.63` and
`7.4.1`. Migration 60 carries the old `cluster.version` over to its
release line, records releases needing attention and preserves the
legacy value while the old chart can still render it.
- The FoundationDB chart refuses a render when `cluster.version` and
`version` name different release lines. This also covers variants
without platform migrations.
- OpenSearch and FoundationDB get version-map update scripts. They
retain the current default while it remains supported.
Application types and schemas are still generated from the charts.
### Upgrade notes
Stored image overrides are accepted without new admission warnings and
ignored by the charts. On the first reconcile, OpenSearch nodes move to
the image for their selected version, and Kubernetes components whose
images changed restart. Check custom OpenSearch images before upgrading:
if one runs a newer version than the chart's map, switching back can
leave nodes unable to read their data. Air-gapped installations need
`docker.io` and `ghcr.io` mirrored on management-cluster nodes.
FoundationDB preserves the release line, not an arbitrary patch: 7.3.x
moves to 7.3.63 and 7.4.x to 7.4.1. Check these versions before
upgrading. A cluster on 7.1 needs an upgrade to 7.3 first. On variants
without migrations, set `version` to the line the cluster runs. To
change release lines later, remove the leftover `cluster.version` as
directed by the chart error.
Migration 60 records affected releases in the
`cozystack.io/migration-60-needs-attention` annotation on the
`cozystack-version` ConfigMap. Dry-run reports patch changes without
patching, annotating or advancing the migration stamp. Migration 56 no
longer carries the removed kubectl override into adopted node pools.
An explicit OpenSearch `version: null` now fails rendering instead of
falling back to `v2`.
### Validation
- `make unit-tests` passed, including migrations 56 and 60.
- Four-chart regression run: 750 tests passed.
- API Create/Update, admission, defaulting and existing warning
regression tests passed. `cmd/`, `internal/` and `pkg/` match main.
- Shell test lanes passed with `/bin/sh` as dash throughout the process
tree.
- `make generate manifests` passed with no generated drift, using
cozyvalues-gen v1.7.0.
E2E passed on
|
||
|
myasnikovdaniil
|
55aad6f914 |
fix(tcp-balancer): cap HAProxy fds and threads by the pod's own limits (#5005)
## What this PR does
On a v1.6.5 cluster a TCPBalancer created with the chart defaults
(`t1.nano`, limits 250m CPU and 128Mi memory, `haproxy:3.4.4`)
crash-looped. Both pods were OOMKilled with a memory limit of 134217728,
7 restarts each in about 15 minutes, and HAProxy logged `[NOTICE] (8) :
Automatically setting global.maxconn to 524126.` On another cluster the
same chart picked maxconn 524251 and the pods kept running.
`haproxy.cfg` has no `global` section, so HAProxy sizes itself from the
node. maxconn comes from the nofile limit capped at the built-in
`fd-hard-limit` of 1048576 (`DEFAULT_MAXFD`), and the thread count is
the number of CPUs the process may run on, whatever its CPU limit is.
Both are paid for at startup, before any traffic, and the thread part
grows with the node.
This PR adds a `global` section sized from the replica's own limits,
read through the same `cozy-lib.resources.defaultingSanitize` helper the
Deployment uses, so `resources` and `resourcesPreset` resolve the same
way. For `t1.nano` it renders
```
global
fd-hard-limit 4096
thread-hard-limit 1
```
`fd-hard-limit` is 32 per MiB of the memory limit and HAProxy derives
maxconn from it, 2035 for `t1.nano`, which is about 64 KiB per
connection. For comparison `haproxy -m 128` picks maxconn 3600 on 3.4.4,
about 35 KiB per connection, and the difference is left for the process
itself and for the kernel socket buffers the cgroup also charges. An
explicit `maxconn` would read simpler, but `strict-limits` is on by
default and HAProxy refuses to start when it cannot raise nofile to
about twice that value. With 1024 soft and 4096 hard nofile `maxconn
4096` exits with `Cannot raise FD limit to 8251`, while `fd-hard-limit
8192` starts with maxconn 2018. `thread-hard-limit` is the CPU limit
rounded up, at least 1. It caps the detected thread count and never
forces more threads than the node has.
Reproduced with Docker Desktop on arm64 (12 vCPU, kernel 6.12) and the
same image, running the rendered config the way the Deployment mounts
it. A larger node is stood in for by adding `nbthread 96` or `nbthread
128` to the `global` section. Memory is `memory.peak` of the container
cgroup.
```sh
cd packages/apps/tcp-balancer && mkdir -p /tmp/cfg
helm template tb . --show-only templates/configmap.yaml | yq '.data["haproxy.cfg"]' > /tmp/cfg/haproxy.cfg
docker run --rm --memory 128m --memory-swap 128m --ulimit nofile=1048576:1048576 -v /tmp/cfg:/usr/local/etc/haproxy:ro haproxy:3.4.4
```
| run | main | this PR |
| --- | --- | --- |
| idle, 12 threads | maxconn 524258, peak 89.6 MiB | maxconn 2035, 1
thread, peak 18.7 MiB |
| idle, `nbthread 96` | maxconn 524127, worker exited with code 137,
OOMKilled | 1 thread, peak 20.8 MiB |
| idle, `nbthread 128` | worker exited with code 137, OOMKilled | 1
thread, peak 22.9 MiB |
| 2035 clients echoing 16 KiB through a backend for 25 s, 12 threads |
worker exited with code 137 about a second after the clients started |
running, peak 83.7 MiB |
One case this PR does not cover is clients and servers that both stop
reading. They fill kernel socket buffers that the cgroup charges, and
with this PR the 128Mi container was still OOMKilled after 1225 such
connections, which maxconn does not bound.
`helm unittest .` passes 13 of 13 with helm-unittest 1.1.2 as CI pins
it. Against the template on main the four new cases fail and the nine
existing ones pass, and each of six template mutations fails at least
one case. `haproxy -c` accepts the render for `t1.nano`, `t1.4xlarge`,
`m1.4xlarge`, a zero CPU limit, the whitelist, `kubernetesAndTalos` and
`tcp-with-proxy` inputs. A full `helm template` differs from main only
in the `global` section and the `checksum/config` annotation, so the
pods roll once on upgrade. I have not run the change on a live cluster.
The template is the same on release-1.6, and that backport is a separate
PR.
### Screenshots
Not a UI change.
### Downstream repositories
One chart template and its unittest suite in
`packages/apps/tcp-balancer`. No values key, schema, default, README or
package change, so no trigger map entry applies.
- [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(tcp-balancer): HAProxy now takes its fd and thread caps from the replica's memory and CPU limits instead of the node, so a t1.nano balancer is no longer OOMKilled on nodes with many CPUs. maxconn follows the memory limit at about 16 connections per MiB, 2035 on t1.nano, and the pods roll once on upgrade.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- TCP Balancer now automatically configures HAProxy file-descriptor and
thread limits based on the selected or explicitly defined resource
settings.
- CPU-based thread limits are rounded up and always maintain a minimum
of one thread.
- Memory-based file-descriptor limits scale automatically with the
configured memory capacity.
- **Tests**
- Added coverage for default, preset-based, explicit resource, rounding,
and minimum-limit scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
myasnikovdaniil
|
614847976f |
test(harbor): pin the resources of the nginx proxy and the portal (#5025)
## What this PR does #4095 gave the Harbor nginx proxy the `t1.small` preset and moved the portal from `t1.nano` to `t1.micro`, so neither falls back to the LimitRange default of a tenant namespace, 250m CPU and 128Mi. It came without a test. This adds a helm-unittest suite that pins both presets in the nested HelmRelease and checks that they stay the same when the other components are resized, so dropping them or tying them to another component's values fails the suite. Both containers run nginx with `worker_processes auto`, which starts one worker per CPU of the node whatever the container's CPU limit is. With the proxy config the chart renders and `goharbor/nginx-photon:v2.15.1`, 96 workers idle at about 205 MiB, so on a node with many CPUs the proxy is OOMKilled at 128Mi before it serves a request. That is what happened on a v1.6.5 cluster, where both nginx pods of a default Harbor restarted 5 and 6 times within minutes, because `release-1.6` never got #4095. The backport, with this suite and the measurements, is a separate PR. `helm unittest .` in `packages/apps/harbor` passes 47 of 47 with helm-unittest 1.1.2. With the #4095 hunk reverted the three new cases fail and the 44 existing ones pass. Each of seven template mutations fails at least one new case. They drop the nginx block, give nginx `t1.micro` or the portal `t1.nano`, tie nginx to the core values or the portal to the jobservice values, misspell the `resources` key under nginx, and make core ignore its preset, which the resize case checks so that it cannot pass on overrides that never reach the release. ### Screenshots Not a UI change. ### Downstream repositories One new helm-unittest suite in `packages/apps/harbor/tests`. No template, values key, schema, default, README or package change, so no trigger map entry applies. - [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 NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Tests** * Added Helm coverage verifying default resource settings for Harbor’s nginx and portal components. * Added coverage confirming nginx and portal memory allocations remain consistent when related service resources are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
5265ee662c |
fix(tenant): stop the gateway release waiting on TenantGateway listeners and certificates
helm-controller waits on every object of a release with the kstatus poller on install and on every upgrade. kstatus reads a TenantGateway at Ready=False/ListenersNotReady as InProgress, and a TenantGateway sits there whenever one listener certificate is pending, which depends on ACME or DNS and, under HTTP-01, on routes that other releases own. After a platform upgrade the gateway release therefore times out, retries without end under RetryOnFailure, holds a helm-controller worker meanwhile, and fails the tenant release that waits on it. It used to pass because health is judged only inside a Helm action: the release was installed while the TenantGateway had no status yet or was still Ready, and the listeners whose certificates do not issue came later with other releases' routes. An always-true expression replaces kstatus for TenantGateway and keeps the wait on. Flux checks status.observedGeneration against the generation before it evaluates any expression, so the release still waits until the controller has observed the new spec, but not for listener or certificate health. The poller strategy is set explicitly because expressions are only evaluated under it. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
639ede20be |
fix(api): leave FoundationDB version checks in the chart
The chart still reads cluster.version to guard release-line changes, so calling it ignored in an admission warning contradicts the render error. Keep that decision in the chart, which can explain the version mismatch, and limit removed-field warnings to inert values. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrei Kvapil
|
b43196e549 |
fix(kubernetes): pin the Proxmox cloud-config builders to the bundled image
The Proxmox substrate added two init containers that build the cloud config from the hypervisor token Secret, and both still took their image from images.waitForKubeconfig. With that key gone the render failed on a nil map; with it set, a tenant chose the image that reads the token. They now run the bundled busybox through cozy-lib.image, like the wait-for-kubeconfig container next to them. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
9775628cc8 |
fix(foundationdb): refuse a render that would move the release line
Migration 60 carries cluster.version over to version, but platform migrations run only on the variants that enable them. On any other the new chart ignored cluster.version and rendered the v7.3 default, so a 7.4 cluster was downgraded and a 7.1 one moved onto a line nothing had upgraded it to. The render now fails when a stored cluster.version names another release line than version selects, and says how to proceed, so the release stays on the revision it runs until someone picks the line. Where the migration ran the two agree and nothing changes. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
448b12657b |
fix(foundationdb): stop offering the 7.1 line
The FoundationDB operator no longer carries the 7.1 client, so it can neither manage nor upgrade a 7.1 cluster. The version map offered v7.1 all the same; it now lists the lines the operator carries, 7.3 and 7.4, which is also what hack/update-versions.sh reads from the operator chart. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
1636349a63 |
fix(api): point the removed image overrides at node registry mirrors
The warning for a removed image override told an air-gapped install to use the platform-wide registry, but nothing in the platform writes _cluster.images-registry, so there is nothing to set. What serves these images offline is what serves every other platform image: the registry mirrors configured on the nodes, as the air-gapped install guide sets up. The tests that hold the cozy-lib.image routing now say what they check rather than promising a platform knob. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
dfff5c2dac |
test(kubernetes): hold the routing of the busybox and unpin images
The busybox helper shared by four workloads and the pool's pre-delete Job had no test holding their route through cozy-lib.image, so reverting either line left both suites green. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
a41015e309 |
fix(api): warn when a release still carries a field nothing reads
The apps API accepts and stores a values key the chart has stopped reading, so a kubectl apply or a terraform apply carrying one succeeds and changes nothing. The Phase 2 warning for the Kubernetes worker-pool keys now covers the image overrides and the FoundationDB cluster.version as well, one table per kind. The version helpers name the value and the versions on offer when the release values carry an explicit null, instead of failing on a template type error that names neither. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
a887125ee5 |
build(apps): keep the version default out of the tag refresh
Both generators wrote the newest line as the default, so a run made to pick up a patch tag also moved every release that never set a version onto another major line. The default now stays where it is while it is still supported, and a line that leaves the supported set is reported instead of quietly leaving the enum. FoundationDB takes the patch from the operator chart rather than from the newest tag in the registry: the operator only carries the client library of that build, and the generator now reproduces the committed map exactly. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
d99a1b9f2f |
fix(kubernetes): route the helper images through cozy-lib.image
These images now go through cozy-lib.image like the harbor, kafka and mariadb hook images, so they follow _cluster.images-registry wherever that is set rather than any per-application value. An air-gapped install serves them the way it serves every other platform image, through the registry mirrors configured on its nodes. The talos-csr-signer sidecar and the OIDC Jobs had no test holding their image, which is where an override could come back unnoticed. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
46a9e12578 |
build(opensearch): generate the version map the way postgres does
hack/update-versions.sh listed the majors by hand and only rewrote files/versions.yaml, so the Version enum and the default in values.yaml drifted from the map on every refresh. It now takes the tested range from the compatibility table of the pinned operator release, picks the newest tag per major that has a matching Dashboards image, and rewrites the enum and default along with the map, like postgres and mariadb. The committed map keeps its current tags; the helper follows the postgres idiom and no longer falls back to v2 on an empty version. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
d62056ef03 |
fix(foundationdb)!: pick the version from a fixed list
cluster.version took any string and passed it to the operator as the FoundationDB version, so a tenant chose the image tag rather than a supported release line. The application now takes version (v7.4, v7.3, v7.1) and maps it to a tag through files/versions.yaml, the same way postgres and mariadb do. The lines are the ones the pinned operator ships client libraries for, which hack/update-versions.sh reads from the operator chart. The map keeps v7.3 on 7.3.63, the previous default, so a cluster that never set the version renders exactly as before. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
941b50c675 |
fix(kubernetes-nodes)!: drop the kubectl image override
images.kubectl picked the image of the talos-reconcile and pre-delete unpin Jobs. Both run in the management cluster under service accounts that read the pool's Talos and cluster CA secrets and patch its CAPI objects, so a tenant-chosen image ran with access the apps API never grants. The image now always comes from images/kubectl.tag. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
af20694359 |
fix(kubernetes)!: drop the helper image overrides
images.waitForKubeconfig, images.kubectl and images.talosCsrSigner ran whatever image a tenant named inside the management cluster: the csr signer as a sidecar of the tenant control plane with the Talos CA key mounted, the other two in pods holding service account tokens for CAPI, KubeVirt and Cilium objects in the tenant namespace. A tenant never gets that access through the apps API, and the platform should not hand it over through an image field. The images now always come from the bundled tag files. A release that still carries the keys renders exactly as one without them. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Andrei Kvapil
|
6f8a04c11a |
fix(opensearch)!: drop the custom image override
images.opensearch put any image a tenant named into the OpenSearchCluster, so the platform ran that binary in its own cluster and the version map stood behind nothing. The application offers a choice of version; the image now always comes from that version. A release that still carries the key renders as if it were absent: the operator rolls the nodes onto the image of the selected version. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
myasnikovdaniil
|
a26c441d78 |
fix(vm-disk): let CDI adopt the PVC a Velero restore recreates (#4955)
## What this PR does After a Velero restore of a VMDisk or of a VMInstance (`RestoreJob` with the Velero strategy), the disk's HelmRelease fails on its next upgrade and stays failed: ``` Helm upgrade failed for release <ns>/vm-disk-<name> with chart vm-disk@...: failed to create resource: admission webhook "datavolume-validate.cdi.kubevirt.io" denied the request: Destination PVC <ns>/vm-disk-<name> already exists ``` The VMInstance release that references the disk then fails in turn (`Specified disk not exists in cluster`). Seen on a v1.6.5 cluster with CDI v1.66.1, where each failed upgrade is retried and the release history of one such disk had grown past 46,000 revisions. ### Cause The restore controller deletes the disk's DataVolume (`prepareForRestore`) and Velero restores the PVC under the same name, with no owner. The restored release matches its chart and values, so Flux does not upgrade it and the DataVolume is not recreated right away; the next Helm upgrade (a platform update, an edit of the VMDisk) does recreate it, and CDI's webhook refuses it. In CDI v1.66.1 the webhook admits a DataVolume over an existing claim only when `ClaimMayExistBeforeDataVolume` is true (`pkg/controller/common/util.go`): either the PVC carries `cdi.kubevirt.io/storage.populatedFor: <dv name>`, or `AllowClaimAdoption` returns true, which reads `cdi.kubevirt.io/allowClaimAdoption` **from the DataVolume** and otherwise falls back to the `DataVolumeClaimAdoption` feature gate. The Velero strategy's resource modifiers (`createResourceModifiersConfigMap`) put `allowClaimAdoption=true` on the restored **PVC**, where CDI never reads it, and the `CDI` CR in `packages/system/kubevirt-cdi` enables neither the gate nor anything equivalent. Migration 29 creates its PVCs with the same PVC-side annotation. ### Fix The vm-disk chart sets `cdi.kubevirt.io/allowClaimAdoption: "true"` on the DataVolume it renders, on the creation path and on the existing-DataVolume path alike, so the annotation does not flap between upgrades. - A fresh disk is unaffected: no PVC exists when the DataVolume is admitted, and the PVC CDI then creates carries `cdi.kubevirt.io/createdForDataVolume: <dv uid>`, for which `AllowClaimAdoption` returns false, so CDI imports, clones or uploads as before. - An existing DataVolume already controls its PVC, so the annotation changes nothing there; a Helm upgrade only adds it to the DataVolume's metadata, which CDI allows (only the spec is immutable). - After a restore, the webhook admits the recreated DataVolume and the DataVolume controller adopts the claim: `pvcRequiresWork` is false, it takes controller ownership of the PVC and the DataVolume goes `Succeeded` without populating it again, so the restored data is kept. Checked against a v1.6.5 cluster carrying the broken state, by rendering the chart for an affected disk and running `kubectl create --dry-run=server`: the DataVolume without the annotation is refused with the error above, the same one with the annotation is admitted. On that cluster two affected disks were later recovered through the claim-adoption path (the same `AllowClaimAdoption` check): the PVC kept its uid, became controlled by the new DataVolume, and the DataVolume reached `Succeeded` with no import. ### Alternatives considered - **Enable the `DataVolumeClaimAdoption` feature gate in the `CDI` CR.** It works too, but for every DataVolume in the cluster (tenant Kubernetes worker disks, golden images, KubeVirt `dataVolumeTemplates`), each of which would then silently take over any unowned PVC of its name. The per-DataVolume annotation keeps the behaviour to the one chart whose DataVolumes are expected to meet a restored claim, and CDI has said it means to retire the gate (kubevirt/containerized-data-importer#3480) while the annotation is checked first and independently of it. - **Stamp `cdi.kubevirt.io/storage.populatedFor` on the restored PVC** from the resource modifiers. That would also pass the webhook, but only for disks restored by the Velero strategy, not for claims created by migration 29 or by hand, and it would need a controller image change where a chart change suffices. The PVC-side `allowClaimAdoption` rule in `createResourceModifiersConfigMap` (and the one in migration 29) is inert under CDI v1.66.1. This PR leaves the rule and its unit-test assertions in place, and corrects the comments that credited it with the adoption (the collector and `createResourceModifiersConfigMap` in `velerostrategy_controller.go`, and step 5 of `examples/backups/vmi/scenario-user-restore.md`): they now say CDI adopts the restored PVC through the annotation the vm-disk chart sets on the DataVolume. The backports carry only the template and the unittest suite, so these comment and doc edits do not reach them. One behaviour change to be aware of: a PVC named `vm-disk-<name>` left behind without an owner (a restored disk deleted before this fix, whose PVC Velero restored without an owner) is now adopted by a new VMDisk of the same name, with its old contents, instead of blocking that VMDisk forever. That PVC lives in the same tenant namespace. ### Tests - `packages/apps/vm-disk/tests/dv_claim_adoption_test.yaml`: the post-restore state (a restored PVC, no DataVolume) renders the DataVolume through the creation path with the annotation, and the existing-DataVolume path keeps it. Both fail with the template change reverted. `helm unittest packages/apps/vm-disk`: 116 passed. - `examples/backups/vmdisk/05-restore-in-place.sh` (run by the `vmdisk-2-backup-roundtrip` Chainsaw suite): after the restore, the HelmRelease was already reported Ready because no upgrade ran, which is why CI did not catch this. The script now forces a Helm upgrade (`reconcile.fluxcd.io/forceAt` + `requestedAt`), then requires the release Ready, the DataVolume `Succeeded`, and the PVC to keep its restored uid and be controlled by the DataVolume. ### Backports Opened by hand as #4956 (`release-1.6`) and #4957 (`release-1.7`) rather than through the `kind/backport` labels: `examples/backups/vmdisk/` does not exist on either branch, so the bot's cherry-pick would conflict. They carry the template change and the helm unittest suite. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff: a vm-disk template change (no `values.yaml`, schema, `README.md` or `Chart.yaml` change), a new helm unittest file, and a backup walkthrough script. No app, field, default, CRD, platform value or tooling changes, and no documented workaround on the website is made obsolete (the only mention of `allowClaimAdoption` there is in the archived v0 docs, about a different manual procedure). - [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(vm-disk): a VMDisk restored by Velero, alone or as part of a VMInstance, no longer fails its next upgrade with "Destination PVC ... already exists": the DataVolume now carries cdi.kubevirt.io/allowClaimAdoption, so CDI adopts the restored PVC and keeps its data. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Improved in-place VM disk restores so restored storage is correctly adopted when the disk resource is recreated. * The restore process now waits for the disk to become ready and reports an error if it fails or the restored storage is not retained. * Clarified the adoption behavior in restore documentation and ensured recreated disks use the required adoption setting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
b83cc087cc |
docs: move the repo-only guides onto the website
Seven guides in docs/ were written next to the code and left in the source tree, where no operator looks. The website carries them now, so a second copy here only lets the two drift again: the Backup Classes page had already fallen behind its original. docs/release.md, docs/deprecating-applications.md and the two SeaweedFS runbooks in docs/operations stay. A migration, a bats suite, a workflow or AGENTS.md reads each of them by path, which is the test for keeping a document in this repository. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Yan Bondarenko
|
1d8bea039b |
test(harbor): check that the resized components reach the release
The case that resizes the other components looked only at nginx and the portal, so it would keep passing if those overrides stopped reaching the nested release, and then it could not catch nginx or the portal following another component's values. It now checks the core, registry and jobservice limits it sets and leaves trivy alone, as nothing in the case reads it. The portal case also checks its memory request, as the nginx case does. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
7c0634586b |
test(harbor): pin the resources of the nginx proxy and the portal
Both containers run nginx with worker_processes auto, so their idle memory follows the CPU count of the node, whatever their own CPU limit is. In a tenant namespace a container without resources gets the LimitRange default of 128Mi, which the proxy outgrows on a node with many CPUs before it serves a request. The presets that keep both out of that default have no test, so dropping them or tying them to another component's values would go unnoticed. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
20545d00a5 |
fix(tcp-balancer): cap HAProxy fds and threads by the pod's own limits
Without a global section HAProxy 3.4 sizes itself from the node. maxconn comes from the nofile limit, cut to the built-in fd-hard-limit of 1048576, so about 524k, and the thread count is the number of CPUs the process may run on, whatever the CPU limit says. With one thread that alone idles at about 85 MiB, and every thread adds about 0.6 MiB, so a t1.nano replica with its 128Mi limit runs on a small node and is OOMKilled at startup on a node with many CPUs. fd-hard-limit is now 32 per MiB of the memory limit, from which HAProxy derives about 16 connections per MiB, 2035 for the t1.nano defaults. That leaves 64 KiB per connection against the roughly 35 KiB HAProxy budgets per connection itself when it is given a memory limit with -m, and the rest covers the process and the kernel socket buffers charged to the same cgroup. It caps the derived value instead of setting maxconn, because an explicit maxconn above half the hard nofile limit stops HAProxy at startup (strict-limits is on by default), while fd-hard-limit still follows a lower system limit. thread-hard-limit is the CPU limit rounded up, at least 1. It only caps the detected thread count, so a node with fewer CPUs is not forced to run more threads than it has. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Marian Koreniuk
|
1211095693 |
fix(kubernetes-nodes): refuse a partial resources spec on every pool
The resources.cpu/resources.memory pair check ran only when an instanceType was set, while the field description promised a render-time error for a partial spec on any pool. Without an instancetype a cpu-only pool renders a VM with no memory, which KubeVirt refuses when the Machine is created, and a memory-only pool silently sizes the guest at one vCPU. Run the check on every kubevirt pool; the proxmox branch already requires both. A kubevirt pool with neither an instanceType nor both resources now fails at render with a message naming both, the way vm-instance does; no such pool ever passed admission, so nothing that runs is affected. A float64 core count of one million or more printed in exponent form, which the CRD's integer field cannot take. Print it as an integer; every smaller whole number prints the same bytes, so no template name moves. Closes #4546 Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
98d0402096 |
test(kubernetes): pin RetryOnFailure on the Proxmox csi release
The KubeVirt csi and cilium releases carry a helm-unittest pin on strategy.name: RetryOnFailure for both actions, and the e2e HelmRelease guard names those two tests as load-bearing. helmreleases/csi-proxmox.yaml sets the same strategy for the same reason and had no pin. Add the mirror test and update the guard comment that pointed at the gap. No template change. Closes #4731 Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Mattia Eleuteri
|
b4f6fb67db |
fix(vm-disk): let CDI adopt the PVC a Velero restore recreates
A Velero restore of a VMDisk or a VMInstance deletes the disk's
DataVolume and restores the PVC under its name, with no owner. The
restored release is in sync, so nothing recreates the DataVolume until
the next Helm upgrade, typically a platform update. That upgrade is
refused by CDI's admission webhook ("Destination PVC ... already
exists") and the release stays failed, followed by the VMInstance
release that looks the disk up.
CDI admits a DataVolume over an existing claim only when the
DataVolume carries cdi.kubevirt.io/allowClaimAdoption or the
DataVolumeClaimAdoption feature gate is on. The restore controller
puts the annotation on the restored PVC, where CDI never reads it, and
the CDI CR does not enable the gate. Setting it on the DataVolume
keeps the scope to vm-disk instead of every DataVolume in the cluster,
and does not depend on a gate CDI intends to remove. A claim CDI
created for this DataVolume carries its uid in createdForDataVolume
and is never treated as adoptable, so a fresh import is unaffected.
The VMDisk backup round-trip now forces an upgrade after the restore,
which is what reaches the failure, and checks that the DataVolume
adopted the restored PVC instead of replacing it.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-by: LLM
|
||
|
Myasnikov Daniil
|
f3486f4e3a |
docs: move the operator guides out of docs/ to where readers find them
Five guides in docs/ were written next to the feature code that needed them and left in the source tree, where no operator looks. They are on the website, so a second copy here only drifts: edits kept landing in docs/ after the website copy was made. docs/vm-external-vlan.md is split rather than moved. Its section on staging the reference CNI plugins is a contract that hack/cni-plugins-staging-contract.bats asserts against the multus Dockerfile, the daemonset manifest and the platform values, so that half is package documentation and goes to packages/system/multus/README.md, which the suite now reads. The operator half goes to the website. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.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
|
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
|
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
|
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 --> |
||
|
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
|
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 --> |
||
|
Yasar Rahman
|
8be7fb2e66 |
feat(redis): expose min-replicas-to-write and min-replicas-max-lag
Lets an operator bound the writes lost when an isolated Redis master is failed over: the master refuses writes while fewer replicas than configured are connected and recent. Both values are unset by default, so the rendered Redis config is unchanged. Redis treats 0 in either directive as disabling the check, so the chart refuses minReplicasMaxLag 0 next to a replica requirement instead of silently turning the protection off. Negative values are refused by the schema (minimum 0), since Redis rejects them at CONFIG SET and the operator would stop healing the Sentinels of that instance. A requirement of replicas or more is refused at render: a master with replicas pods has replicas-1 replicas, so it would refuse every write for good. Assisted-by: LLM Signed-off-by: Yasar Rahman <yasar.rehman@gmail.com> |
||
|
Aleksei Sviridkin
|
33ca8b67bb |
feat(redis): run Sentinel on the same image as the Redis pods (#4831)
## What this PR does
The Redis application pins its Sentinels to `redis:6.2.6-alpine`
(`packages/apps/redis/templates/redisfailover.yaml`), while the data
pods run the version selected by `version` (`redis:8.4.0` for `v8`,
`redis:7.4.7` for `v7`). The pin was added in
|
||
|
Ray Low
|
904bc38fc3 |
fix(kubernetes): delete the Talos PKI and oidc-bootstrap Job on uninstall
cert-manager runs with enableCertificateOwnerRef: false, so the <release>-talos-ca and <release>-talos-tls-cert Secrets carry no ownerReference to their Certificates and outlive the release, keeping the CA private key of a cluster that no longer exists. A new cluster created under the same name then adopts that stale CA instead of issuing its own trust root. The <release>-oidc-bootstrap Job is a hook whose only delete policy is before-hook-creation. Hooks are not part of the release manifest, so uninstall never removes it. The pre-delete hook deletes the two Certificates before their Secrets, because deleting a Secret while its Certificate exists makes cert-manager re-issue it. It runs after the TenantControlPlane is gone, since the control plane's CSR signer mounts the CA. The Role grants each delete by resourceName only. Assisted-by: LLM Signed-off-by: Ray Low <crossrt@gmail.com> |
||
|
Aleksei Sviridkin
|
6ba440f7a3 |
fix(kafka): skip the OpenAPI patch lookup in the migration hook
The pre-install/pre-upgrade hook applies the node pools and the drain KafkaRebalance with kubectl, and its container has no resources of its own, so in a tenant with resource quotas it runs under the LimitRange default of 128Mi. When it applies an object that already exists, kubectl downloads the cluster's whole OpenAPI v2 document only to decide that a custom resource takes a JSON merge patch: with the 374 CRDs of an e2e cluster that peaks at about 300Mi, so every upgrade of a Kafka in such a tenant OOM-kills the hook and fails with "pre-upgrade hooks failed". --openapi-patch=false skips that lookup and builds the same three-way merge patch from the last-applied annotation, peaking at about 35Mi on the same cluster. A server-side apply would be as small but changes what gets removed: the "kafka" pool is created with "kubectl create", whose manager keeps co-owning every field it set, so a later server-side apply leaves those fields in place when the chart drops them. A larger memory limit would charge every upgrade against the tenant quota instead. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |