mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 02:27:01 +09:00
main
4678
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a2cc9a3016 |
fix(dashboard): link a tenant row to its detail page (#4004)
## What this PR does Fixes #3822 Tenant list showed name as plain text and the only link in a row was Edit button, so tenant detail page with its tabs and Delete was reachable only by opening edit form and cancelling out of it. Name cell is a link to that page now, it appears exactly where Edit appears and points to the same CR. Plain click switches selected tenant to the row's parent first, same as Edit does, because detail page takes namespace from selected tenant. Ctrl, cmd and shift click don't run that switch, so the tab you open a tenant from stays where it was. First commit fixes what the link depends on. Console recovers a tenant's parent from namespace name (`tenant-a-b` sits under `tenant-a`), but root is the one level that naming skips: tenant `acme` ordered in `tenant-root` owns `tenant-acme`, not `tenant-root-acme`. So `realParentNamespace` returned undefined for every top level tenant, which is most tenants on a normal cluster, and Edit button has been missing there for as long as it exists. Ancestor labels carry the parent that the name does not. Root level tenant named `root-x` resolves to CR `root-x` now, not `x`. ### Notes I first tried to make links carry their tenant in the url (`?tenant=`), so a detail page opened in new tab resolves on its own. That turned into questions about tenant context itself: what to do with a tenant in a link the user can't see, what localStorage may hold, and TenantNamespace LIST ignoring `labelSelector`. Detail route addresses a Tenant CR by relative name and takes namespace from selected tenant, and making that robust is a refactor of tenant context and routing, which should start with study and planning, not with patches on patches. So this PR only closes the UX gap (no way from tenant list into a tenant's page), going deeper is out of scope. Known limit that comes with it: link has no tenant in it, so opening it in a new tab (ctrl/cmd/middle click, open in new tab, copied url) resolves against the tenant last selected in any tab, because selection lives in shared localStorage. For a row directly under that tenant it opens the right page. For a row deeper in the tree the page says `Application <name> not found.` and there is no Delete. If that tenant also has a direct child tenant with the same relative name, new tab opens that other tenant's page with working Edit and Delete, and delete confirm says only `Delete Tenant "<name>"?`. Plain click in the same tab is not affected. Detail page's own tabs and Edit link already behave this way, it's how detail route resolves its namespace today and fixing it properly is the refactor above. A row links, and offers Edit, exactly when namespace holding its Tenant CR is one the apiserver returns to this user. It returns a tenant namespace only if the user is subject of a RoleBinding in it, so sub-tenant admin that cant act in `tenant-root` sees no link there. No row level Delete button. Issue allows either that or a route to detail page, I took the route so confirm and mutation stay in one place. Covered by tests, every source change here fails a test when reverted: - top level tenant, nested one, root level tenant named `root-x`, forest root without ancestor labels - bridged row whose real parent is invisible gets neither link nor Edit - plain click switches to the row's parent and opens detail page - ctrl click leaves selection and location alone ### Screenshots Tenant list under root on a dev cluster. Every row name is a link to tenant page now (`pr4078` is hovered), top level tenants had neither link nor Edit before. Host column redacted. <!-- SCREENSHOT: drag in 01-tenant-list-links.png here --> <img width="1440" height="900" alt="01-tenant-list-links" src="https://github.com/user-attachments/assets/39cd723a-1ac5-4d3b-8c36-b70ddc626961" /> ### 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. --> - [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: All changed files are under `packages/system/dashboard/images/console`, console has no downstream copy (standalone UI repo is archived). ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(dashboard): tenant name in the tenant list links to its detail page now (where Delete is), and top level tenants get their Edit button back. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Root-level tenant names are now displayed without incorrectly removing prefixes, and tenants ordered directly under the root are correctly recognized in the hierarchy. * Tenant rows link to their detail pages when the parent tenant is accessible. Same-tab navigation updates the active tenant, while opening a link in another tab leaves the current selection unchanged. * Edit links and navigation now use consistent tenant routes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
7f55ff5d85 |
fix(dashboard): link a tenant row to its detail page
The tenant list rendered the name as plain text and put the row's only link on an Edit button, so nothing in the list reached the detail page that every other kind gets, tabs and a Delete action included. The only way there was to open the edit form and cancel out of it. The name cell becomes a link wherever Edit is offered, to the same CR. The detail page reads the namespace from the tenant context, so a plain click aligns the context to the row's parent first, the way Edit already does. React runs that handler for a ctrl-, cmd- or shift-click too, which the browser turns into a new tab or window, so it runs only for the click that navigates its own tab; otherwise opening a tenant beside the current one would move the tab left behind as well. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
e44e41d307 |
fix(dashboard): resolve the Tenant CR of a tenant ordered in the root
A tenant's CR lives in its parent's namespace, and the console recovered the parent from the namespace name: `tenant-a-b` sits under `tenant-a`. The root is the one level that naming skips. A tenant `acme` ordered in `tenant-root` owns `tenant-acme`, not `tenant-root-acme`, so the prefix test never matched a top-level tenant and `realParentNamespace` returned undefined for it. That is most tenants on a normal cluster, and the Edit button, offered only where the CR's namespace resolves, was missing for all of them. The ancestor labels carry the parent the name does not, so the lookup falls back to `tenant-root` when the chain names it. Deriving the CR name keys on the parent being the root rather than on the child's name happening to start with it, which keeps a root-level tenant called `root-x` from resolving to the CR `x`, and the row label agrees with it rather than showing `x`. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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 --> |
||
|
|
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` (
|
||
|
|
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 |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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 |
||
|
|
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 --> |
||
|
|
0b3f5dd7e1 |
fix(keycloak)!: make the KMS proxy keep its keys across restarts (#4818)
## What this PR does Two fixes for the optional KMS proxy of the platform Keycloak (`encryption.*`). The default static backend never started. The chart stored 32 alphanumeric characters as the KEK, but the proxy wants base64 of 32 bytes and exits with `config: KKP_KEK must decode to 32 bytes (got 24)`. The KEK is now generated with `randBytes`, and a stored key of the old form is replaced. The proxy never ran with such a key, so nothing was encrypted under it. Any other stored key is kept if the proxy accepts it and fails the render otherwise, so the chart never overwrites key material. The KEK Secret now carries `helm.sh/resource-policy: keep`: the DEK set is wrapped with it, so it has to survive turning encryption off or reinstalling the package. Without `deksetSecretName` the proxy mints a new DEK set on every start (`buildCipher` in `cmd/proxy/main.go` of keycloak-kms-proxy) and keeps it only in memory. After the first restart of the proxy pod it cannot decrypt anything it wrote before. The values comment said this is safe on a fresh DB, it is not. The chart now refuses to render the proxy without a DEK set unless `fields: disabled`. Passthrough still works without one, so the static backend can get its KEK Secret created before the DEK set is minted with it. Breaking: a release with encryption on and no `deksetSecretName` fails to render on upgrade until a DEK set is configured. Such a setup loses its data on the next proxy restart anyway, so failing loudly is the point. A stored static KEK the proxy cannot decode fails the render too, instead of being replaced. Checked with `helm unittest` (100 tests, the new ones fail on the old template) and by starting keycloak-kms-proxy v0.2.3 with the KEK the chart renders now. With the KEK from the old template it fails as quoted above. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/742 documents the setup with a DEK set - [ ] [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 other repositories do not touch the keycloak chart values. ### Release note ```release-note fix(keycloak)!: The KMS proxy of the platform Keycloak now requires `encryption.deksetSecretName` unless `encryption.fields` is `disabled`. Without it the proxy generated a new data key on every start and could not read data encrypted before a restart. The static backend now generates a KEK the proxy accepts; before, the proxy could not start with it. The KEK Secret is no longer deleted when encryption is turned off or the package is removed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Configuration** * Enabling field encryption requires a persistent DEK set. Chart rendering reports an error if it is missing; setting fields to `disabled` does not require one. * For the static backend, a valid stored KEK is reused, and a missing or legacy-format KEK is replaced. An unusable stored KEK causes rendering to fail. * The KEK Secret is retained when encryption is disabled or the package is removed. * **Documentation** * Clarified DEK set requirements and sharing across starts and replicas, and updated guidance on configuring multiple proxy replicas. * Warns against deleting the retained KEK while encrypted values remain. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
b36628661f |
fix(ingress-nginx): drop the admission-webhook policy a rebase brought back (#5088)
## What this PR does Rebase behind [#3931](https://github.com/cozystack/cozystack/pull/3931) brought back namespaced `CiliumNetworkPolicy` for admission webhook, which [#3997](https://github.com/cozystack/cozystack/pull/3997) deleted in favour of clusterwide policy. It renders only on `admissionWebhookPolicy.enabled` (true by default) and never checks that Cilium API exists, so on cluster without Cilium every tenant with `ingress: true` fails to install with `no matches for kind "CiliumNetworkPolicy"`. On cluster with Cilium it repeats deny that clusterwide policy already has, and also renders for child tenants where webhook is disabled. This PR removes the template with its test and adds suite that renders whole package without Cilium API and fails if any Cilium resource comes out. Existing capability suite selects only clusterwide template, so it could not catch this file coming back. Clusterwide policy and `admissionWebhookPolicy` value are unchanged. Fixes #4706 On cluster without Cilium nothing in this chart restricts webhook port. Removed policy could not do it either because it needs Cilium CRD to install at all, and clusterwide policy already renders only where Cilium API is served, so no protection is lost, such cluster now just installs ingress instead of failing. Only `tenant-root` runs webhook, child tenants set `admissionWebhooks.enabled: false`. Restricting webhook port on non-Cilium clusters is separate design question, not part of this PR. Checked `helm template` with and without `cilium.io/v2/CiliumClusterwideNetworkPolicy` in api versions: without it no Cilium object renders, with it only clusterwide policy, same as before. New suite fails on tree before this change. ### Screenshots Not a UI change. ### Downstream repositories Only template and tests under `packages/system/ingress-nginx` changed, `admissionWebhookPolicy` value kept, nothing from trigger map in `docs/agents/contributing.md` matches. - [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(ingress-nginx): remove the namespaced admission-webhook CiliumNetworkPolicy that was re-added by mistake. It was rendered without checking that the Cilium API exists, so on clusters without Cilium every tenant ingress failed to install with `no matches for kind "CiliumNetworkPolicy"`. The clusterwide admission-webhook policy is unchanged and still renders only where the Cilium API is served. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Configuration** * The ingress-nginx chart no longer generates an admission webhook network policy. Deployments that relied on this policy to restrict access to the webhook will need to configure equivalent traffic controls separately. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
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 --> |
||
|
|
7dc5c0b5e6 |
fix(marketplace): do not install a connected repository's components on connect (#5071)
## What this PR does Connecting a marketplace repository materialized its `PackageSource`s and then auto-created a tap-managed `Package` per source. The Package reconciler installs a `HelmRelease` for every install-marked component of the default variant, and Flux runs those under its `flux` service account, which is bound to `cluster-admin` with no impersonation (`--default-service-account` is not set, and neither `internal/operator` nor `pkg/registry/apps` set `serviceAccountName` on the HelmRelease). A publisher's install chart therefore ran as `cluster-admin` on connect, with no operator step, contradicting the documented behaviour that tapping installs nothing. The `privileged` flag is a prompt to the admin and does not protect against a publisher acting in bad faith. This removes the on-connect auto-registration (`ensureRegistration`). Connecting now only materializes `PackageSource`s. Installing an app is an explicit `cozypkg add`, which becomes the sole creator of a `Package`; every other `Package` reference in the operator is a read or a watch. Removing the call also fixes the second effect of the same path: a `Package` the admin removed with `cozypkg del` is no longer re-created on the next source revision. A second commit reconciles the texts this behaviour change made wrong: they promised that tapping registers a repository's apps in the catalog and runs its charts. Reworded the `cozypkg tap`/`untap` help, the tap success output (it now prints a `cozypkg add <name>` line per materialized PackageSource), the `cozypkg init` scaffold README, the packages-index trust model, and the dashboard Repositories banner and per-repository empty state. Accepted consequence: a connected repository's apps appear in the catalog only after an admin runs `cozypkg add`, since the dashboard catalog is computed from `ApplicationDefinition`s. Install from a connected repository is CLI-only for now; the console connects and disconnects repositories but does not create a `Package`. Populating the catalog from `PackageSource` metadata without an install is the long-term shape in #4359 and is out of scope here. The privileged-confirmation gaps #4358 and #4359 stay open. Upgrade note: a `Package` that Cozystack 1.7.0 auto-created on connect stays after the upgrade, and the privileged-confirmation gate still applies to it, so this is not a regression for existing installs. For a newly connected repository no tap-labelled `Package` is created, so the gate does not come into play on that path. An operator who does not want a 1.7.0 auto-created package can remove it with `cozypkg del`. Addresses #4978. ### Screenshots Dashboard text-only change (Repositories banner and per-repository empty state). No layout change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The diff is operator logic plus CLI/dashboard/doc text; it matches no concrete trigger (no package, API, schema, tooling, annotation, metric or commit-convention change). The dashboard console is vendored in this repo (`packages/system/dashboard`), so its text change travels in this PR. Leaving every box empty rather than ticking "No downstream repository is affected". ### Release note ```release-note fix(marketplace): connecting a marketplace repository no longer installs its components. A connected repository's apps are materialized as PackageSources but not installed until an explicit `cozypkg add`; install from a connected repository is CLI-only for now. Previously, connecting auto-created a Package whose install-marked components were deployed by Flux under its cluster-admin service account. A Package that Cozystack 1.7.0 auto-created on connect stays after upgrade and can be removed with `cozypkg del`. ``` |
||
|
|
7215b02d38 |
feat(keycloak): load additional CA files from Secret or ConfigMap keys (#5039)
## What this PR does Closes #3823. With the stock Cozystack chart, Keycloak cannot connect to an LDAPS server, external identity provider or SMTP server signed by a private CA because the chart cannot deliver a CA file to its truststore. Add `truststores`, a list of existing Secret/ConfigMap references in the release namespace, with one selected key per entry (`ca.crt` by default). ```yaml truststores: - configMapName: freeipa-ca key: ca.crt - secretName: partner-ca key: certificates.pem ``` Each key is mounted read-only in a separate `cozystack-<index>` subdirectory of `/opt/keycloak/conf/truststores`. Keycloak discovers it automatically; the chart adds no environment override, Kubernetes API permission or arbitrary volume path. Existing image certificates, Java default roots, themes and `extraEnv` keep their behavior. References are mandatory: a missing object or selected key prevents the pod from starting. Helm validates the reference shape and safe key paths, but cannot inspect external certificate contents. The trust decision belongs to platform administrators and applies to the entire Keycloak server, across all realms. Write access to a referenced object therefore permits changing that server's trust. If HTTPS client authentication is set to `request` or `required` without a separate `https-trust-store-file`, these CAs also verify incoming client certificates; configure that file explicitly when incoming mTLS requires a different trust boundary. Use public certificates only (PEM bundles or unencrypted PKCS12 without private keys). This does not disable TLS hostname verification. Updating an object's contents does not restart Keycloak or reload its truststore: restart both pods after rotation, or use a versioned object name to trigger a rollout. Validation: - All 113 Keycloak Helm tests pass, including 21 new cases for reference wiring, read-only mounts, selected-key isolation, themes, PKCS12 filenames, defaults and invalid input. - Boundary tests reject source names and keys of 254 characters and the single-dot key, and accept a source name and key of exactly 253 characters. Removing any of the three guards makes its regression test fail; changing either length comparison to reject 253 also fails the boundary test. - Empty-default render matches a clean upstream/main worktree after normalizing the existing randomly generated bootstrap password. Four mutations (wrong mount path, writable mount, wrong selected key and removed validation) each turn the new suite red. - Enabled and default manifests render successfully; enabled YAML parses with yq. `git diff --check` passes. - Runtime check with official Keycloak 26.6.3 in isolated Docker: synthetic LDAPS succeeds with a CA loaded automatically through an emulated Kubernetes AtomicWriter symlink layout (HTTP 204 from the LDAP connection-test Admin API); no custom CA fails with PKIX, and a trusted CA with a mismatched SAN fails hostname verification (both HTTP 400). KC_TRUSTSTORE_PATHS was unset. This checks TLS establishment, not a complete federation bind/failover or deployment to a Kubernetes cluster. - `make generate` has no target in this system chart; no generated schema/API files are involved. `make show` requires the unavailable cozyhr CLI and a live release, so rendering uses `helm template`. `helm lint` reports the same pre-existing undeclared cozy-lib dependency on both this branch and clean main. ### Screenshots Not applicable; no UI changes. ### Downstream repositories The website guide currently says CA mounts are unavailable, so its `next` documentation needs updating once this ships. Other downstream trigger-map entries do not apply: this only adds an opt-in value to the system Keycloak chart, with no app schema, platform configuration, CRD, tooling, naming or node-contract changes. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/765 - [ ] [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 feat(keycloak): load additional CA files from existing Secret or ConfigMap keys through the new truststores value, enabling TLS to private LDAPS, identity-provider and SMTP endpoints. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit - **New Features** - Added support for configuring Keycloak truststores from existing Secrets or ConfigMaps. - Supports PEM bundles and unencrypted PKCS12 certificates, with optional key selection and read-only mounts. - Truststore certificates apply server-wide, including incoming client-certificate validation when no separate HTTPS truststore is configured. - Invalid truststore configurations produce clear errors. Missing objects or keys prevent startup; certificate rotations require pod restarts unless the referenced object name changes. - **Tests** - Added coverage for default behavior, multiple truststore sources, theme mounts, explicit paths, and invalid configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
2f39e304f0 |
fix(marketplace): reconcile tap texts with no-install-on-connect
Removing on-connect auto-registration made several user-facing and doc texts wrong: they promised that tapping registers a repository's apps in the catalog and runs its charts. Tapping now installs nothing, so the texts told operators to wait for apps that never appear. Reword the `cozypkg tap`/`untap` help, the tap success output (now printing a `cozypkg add <name>` line per materialized PackageSource), the `cozypkg init` scaffold README, the packages-index trust model, the dashboard Repositories banner and per-repository empty state, and the stale re-registration code comments. No behaviour changes beyond the added `cozypkg add` hint in the tap output. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
|
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
|
||
|
|
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 -->
|
||
|
|
432d02e20c |
fix(etcd): stop counting canceled watch streams as failed gRPC requests (#4985)
## What this PR does
The tenant etcd chart has its own copy of
`etcdHighNumberOfFailedGRPCRequests`, and both of its rules count every
`grpc_code` other than `OK` as a failure. `grpc_server_handled_total`
counts a Watch stream once, when it ends, and the streams of a
kube-apiserver that goes away end with `Canceled`. So whenever an API
server on a tenant etcd stops, the Watch method of every member reads as
100 percent failed and the critical rule fires on all of them.
Both rules now count only
`Unknown|FailedPrecondition|ResourceExhausted|Internal|Unavailable|DataLoss|DeadlineExceeded`,
the list the etcd mixin has used since etcd-io/etcd#13127 and the one
the platform copy in
`packages/system/monitoring-agents/alerts/etcd.yaml` already uses. The
grouping `BY (job, instance, grpc_service, grpc_method)`, the
thresholds, `for` and the annotations stay as they are, so the alerts
keep their labels.
Seen on a v1.6.5 cluster while a Kubernetes cluster on a tenant etcd was
created and deleted a few minutes later. vmalert had the critical alert
firing for `grpc_method="Watch"` on all three members with the value
100. At that point
`rate(grpc_server_handled_total{grpc_code!="OK"}[10m])` was 0.63 for
Watch with `Canceled`, and the only other non-OK codes were
`FailedPrecondition` on `RoleGet` and `UserGet` at 0.0017 each, which is
one call each. The same query an hour earlier returned nothing. The
platform copy runs in the same vmalert over the same series, and none of
the firing alerts came from it. Each alert also showed up seven times
there, once per tenant copy of these rules, because the root tenant's
vmalert loads the rules of every tenant namespace. This PR does not
change that.
`hack/etcd-grpc-alerts.bats` renders the chart's PrometheusRule with
`helm template` and replays it through `vmalert-tool` v1.136.0 pinned by
digest, the same image #4963 uses.
`hack/testdata/etcd-grpc-alerts/unittest.yaml` has three cases with
seven checks. Watch streams that end only with `Canceled` must not fire
at 6, 12 and 30 minutes. `Range` calls failing with `Unavailable` at 25
percent must fire the critical rule at 7 minutes and both rules at 12,
with the labels and the summary pinned. Both together must fire only for
`Range`. Against the rules on main the suite fails 4 of the 7 checks,
all of them on Watch alerts with the value 100. Dropping `Unavailable`
from the list, or grouping by `job` and `instance` only, fails 3 checks
each.
```console
$ bats hack/etcd-grpc-alerts.bats
1..1
ok 1 tenant etcd failed gRPC request alerts pass their vmalert-tool cases
```
The same run on the rules from main prints `not ok 1` and the four
failing checks. With Bats 1.14.0 at the commit CI pins, the suite passes
together with `bats-strict-setup`, `bats-no-negated-assert`,
`bats-no-exit-trap`, `bats-runner-coverage`, `bats-runner-contract`,
`alert-severity-contract` and `image-pin-consistency`, 81 of 81. Like
the migration suites it needs docker, and only the image pull touches
the network. Helm 3.17.3 and 4.2.4 render the same rules. I have not run
the new rules in vmalert on a live cluster.
The rules file is the same on release-1.6 and release-1.7. The commit
cherry-picks onto release-1.6 without conflicts, and that backport is a
separate PR with one more commit that drops the `load test_helper` line,
because release-1.6 runs `hack/*.bats` through `hack/cozytest.sh`.
### Screenshots
Not a UI change.
### Downstream repositories
A rule template in `packages/extra/etcd`, a Bats suite under `hack/` and
its test data. No values, schema, README, image or
`ApplicationDefinition` change, and the website does not mention these
rules, 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(etcd): the tenant etcd alert etcdHighNumberOfFailedGRPCRequests now counts only the gRPC codes that the etcd mixin and the platform copy of the alert count, so the Canceled watch streams of a Kubernetes API server that stops no longer fire it on every etcd member.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Refined etcd gRPC failure alerts to target specific failure status
codes, reducing alerts for non-failure responses while preserving
existing thresholds and timing.
- Added coverage for canceled watch streams, unavailable range requests,
and combined failure scenarios, including warning and critical alert
evaluations.
- **Tests**
- Added integration validation for rendering etcd alert rules and
running them through the alert-testing tool, including failure handling
when prerequisites are unavailable.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
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 --> |
||
|
|
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> |
||
|
|
59932e5098 |
fix(platform): report FoundationDB patch drift in migration dry runs
The carry path skipped diagnostics in dry-run mode, so its summary could report no releases needing attention even when the upgrade would lower a FoundationDB patch version. Skip only the write and share the warning and summary bookkeeping with a real run. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
4cb505a512 |
fix(platform): record 7.1 FoundationDB clusters instead of carrying them
The operator no longer carries the 7.1 client, and the chart no longer offers v7.1, so a 7.1 cluster has no line to be carried to. Migration 59 now records it, together with any release whose version already names a line that is no longer offered; both have to be upgraded or repointed before the chart will render them. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
d5ebb62632 |
fix(platform): keep migration 60's record whole across a retry
A failed patch stops the migration before the stamp, and the retry finds version already written on the releases it carried, so it never reached the step that records a patch-level move. The move is now worked out from the values alone, so a retried run records it too. An OpenSearch release storing version: null rendered the old chart's v2 default and is refused by the new one; it now goes on the record before it turns up as a stuck release. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
8c2e20539d |
fix(platform): record every release migration 60 moves
A FoundationDB release carried to its line still moves to the one tag the chart ships for it, which can be a step below what it runs, and that showed up only in the Job log. Kubernetes and KubernetesNodes releases pointing the removed image overrides at a mirror were not listed at all. Both now go into the needs-attention record next to the OpenSearch ones. The migrations image carries no charts, so the tag map is repeated in the migration, and the bats suite fails when it drifts from the chart's versions.yaml. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
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> |
||
|
|
033bec74fb |
fix(platform): keep one bad release from stopping migration 60
spec.values on a HelmRelease takes any shape, so a release holding a scalar there, or a cluster.version the apiserver stored as a number, crashed jq and stopped the carry-over for the whole management cluster: every healthy FoundationDB release then went into the upgrade without its version. Those releases are now reported and recorded like any other value that cannot be carried over. The same record covers OpenSearch releases that still set images.opensearch. Nothing can be carried over for them, so what the operator needs is the list. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
f3058d2353 |
fix(platform): guard the FoundationDB version carry-over on all values
Migration 60 tested only cluster.version before adding version, so a version set on the release between the scan and the patch would have been overwritten. The patch now tests the whole values object it read. resourceVersion is not used for this: helm-controller keeps writing the release status, and a mismatch there would abort the platform upgrade for no change to the values. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
930051280b |
fix(platform): carry FoundationDB cluster.version over to version
The foundationdb chart no longer reads cluster.version, so a cluster that set it to 7.1 or 7.4 would move to the v7.3 default on the first reconcile after the upgrade: a full-bounce upgrade from 7.1, or a downgrade from 7.4. Migration 60 sets version to the release line of each cluster.version it finds. It keeps cluster.version, because the old chart still renders the release between the migration and the chart upgrade and would fall back to its own default without it. A value outside the supported lines is recorded on the cozystack-version ConfigMap and does not block the upgrade. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
873bd6712d |
fix(platform): stop carrying images.kubectl into adopted node pools
Migration 56 copied the parent cluster's kubectl image override into each kubernetes-nodes release it creates. Neither chart reads an images map any more, so the copy only planted an inert key in the pool values. Migration 56 has not shipped in a stable release, so no cluster has run the old mapping outside a pre-release. Assisted-by: LLM Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
d607b68a0f |
test(keycloak): pin truststore name and key constraints
Boundary cases must catch removal of the constraints and the error at 253 characters. Assisted-by: LLM Signed-off-by: Федор Батоногов <f.batonogov@yandex.ru> |
||
|
|
8c1d2dbee2 |
fix(ingress-nginx): drop the admission-webhook policy a rebase brought back
A rebase re-applied the namespaced CiliumNetworkPolicy that the move to source-side clusterwide policies had deleted, so two policies now guard the same webhook port and the older one is the worse of the two. It is gated only on admissionWebhookPolicy.enabled, which defaults to true, and never checks that the Cilium API exists. On a cluster without Cilium every tenant ingress release therefore fails to install with `no matches for kind "CiliumNetworkPolicy"`. On a Cilium cluster it repeats the target-side deny the clusterwide policy already carries. It also renders for child tenants, whose controller runs with the admission webhook disabled and for which the clusterwide policy renders nothing, so there it denies a port nothing listens on and is the only thing that puts a cluster-spanning peer onto the controller endpoints. That peer is enumerated per pod identity, the coupling to namespace churn the clusterwide policy was written to contain. The clusterwide policy and the admissionWebhookPolicy value it reads are left as they are. The existing capability suite selects only the clusterwide template, so it could not have noticed this file coming back; a suite with no template filter now fails if any Cilium kind renders while the Cilium API is absent. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
04a3f4be56 |
docs(keycloak): clarify incoming mTLS trust scope
The system truststore also verifies HTTPS client certificates unless a separate HTTPS truststore is configured. Operators adding an LDAP CA need to know that this can extend inbound TLS trust as well. Assisted-by: LLM Signed-off-by: Федор Батоногов <f.batonogov@yandex.ru> |
||
|
|
8bc8e429e6 |
feat(keycloak): trust additional CA files from existing objects
Private LDAPS, identity-provider and SMTP endpoints need their CA in the Keycloak truststore. Environment variables alone cannot supply that file, and manual StatefulSet patches are overwritten by Flux reconciliation. Assisted-by: LLM Signed-off-by: Федор Батоногов <f.batonogov@yandex.ru> |