mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
2fa7fea29b8b9562a2c9e42d2e53ca9fa72fb01d
6979
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Yan Bondarenko
|
2fa7fea29b |
fix(dashboard): show ConfigMaps listed in the application resource map
FoundationDB keeps its connection string in a ConfigMap that its operator writes, and tenants may read it, but the console had no view for ConfigMaps. Tenant roles grant ConfigMaps by name only, so the tab takes the names from the application's resource map and reads each one with a metadata.name field selector, which RBAC checks against resourceNames. An application without a readable resource map shows no tab, while a listed ConfigMap that cannot be read is reported instead of looking empty. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
e3ae8033e6 |
fix(foundationdb): let tenants read the dashboard resource map
The resource map is the only object that names the connection ConfigMap for a client such as the dashboard. It was rendered only with monitoring enabled, and the tenant role could not read it, so a tenant session had no way to find the ConfigMap it is allowed to read. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
4f043b572e |
fix(api): pass the stored object to UpdatedObject in tenantsecret Update (#4603)
## What this PR does A PATCH against an existing TenantSecret returned 404 because `REST.Update` passed `nil` as the old object to `objInfo.UpdatedObject`, and the apiserver patch handler builds the patched object from that old object alone. `Update` now passes the stored Secret (converted with `secretToTenant`) and keeps `nil` only for the `forceCreate` path. The nil also made the patch reach admission as CREATE instead of UPDATE. Validation: the new `TestUpdate_PassesStoredObjectToUpdatedObjectInfo` fails before the change and passes after; `go test ./pkg/registry/...` and `go build ./pkg/... ./cmd/...` pass. I did not run a live `kubectl patch` or the real apiserver patch handler. Known limitation, unchanged by this PR: `tenantToSecret` prefers `Data` over `StringData` when `Data` is non-empty, so a patch that only sets `stringData` on a secret that already has data is a no-op, as a GET-then-PUT edit of `stringData` already is. Patching `data` or metadata works. Fixes #3601 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(api): PATCH, `kubectl label` and `kubectl edit` on an existing TenantSecret no longer fail with 404 NotFound. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Tenant secret updates now correctly apply changes using the existing stored value, including label updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3d610b7c31 |
feat(monitoring): add optional email receiver to alertmanager (#3800)
## What this PR does Adds an optional email receiver configured through `alerta.alerts.email`. It is disabled by default. Alerts carrying `notify_email="true"` are emailed when their severity is in `severities`, which defaults to critical and major. `alertnames` also selects platform alerts without changing their rules. Earlier heartbeat and blackhole routes keep precedence. Email routes preserve the Alerta copy through a preceding route with `continue: true`, and they inherit the root `group_by`, so Alerta and email agree on how many notifications one alert produces. The SMTP password is read from a Secret with a fixed name, `alertmanager-email-password`, mounted through `VMAlertmanager.spec.secrets` and referenced by `auth_password_file` when `authUsername` is set. The platform operator creates that Secret. The name is not configurable, because a tenant-chosen name would let a `Monitoring` writer mount any Secret of the namespace into Alertmanager and send its `password` key to an SMTP host of their choosing, and no tenant role can read those Secrets by any other route. `to` takes an address or a comma-separated list, and `from` takes one address, each with an optional display name. The accepted form is narrower than RFC 5322 on purpose, so that a bad address fails the render instead of being dropped when the first alert fires. The local part must be a dot-atom. A display name is a run of atoms or quoted strings without the specials `net/mail` rejects in a phrase. A domain literal must hold a dotted-quad IPv4 address. IPv6 literals are rejected because their syntax differs across Go mail parser versions. Template actions are rejected, because Alertmanager expands both fields as templates before it parses them. Quoted local parts, comments and a pasted `mailto:` prefix fail the render as well. `smarthost` must be `host:port`, where the host is a host name or a bracketed IP address. A bracketed value must be a valid IPv4 or IPv6 address, so `[deadbeef]:25` and `[1:2:3:4::5:6:7:8]:25` fail the render. The port must be between 1 and 65535 and must not have leading zeros, so `smtp.example.org:0465` fails too. `repeatInterval` is checked with Alertmanager's own duration grammar and limits. Units go in descending order, each at most once, and the value must be non-zero and below the int64 overflow, so what renders is what Alertmanager loads. Partial configuration fails the render and names the fields still set, so clearing the addresses to turn email off tells the operator which leftovers to clear. Subject and body templates are not exposed. Alertmanager parses them per notification, and a broken one surfaces only in a log the tenant cannot read. Checks run for this change. - The monitoring chart suites pass with 228 tests in `packages/system/monitoring` and 16 in `packages/extra/monitoring`. 117 of them are in the two email test files. - 234 address inputs were rendered through the chart. 91 were accepted and 143 rejected. Every accepted input executes as a template and passes `net/mail.ParseAddressList`, including the one-address check for `from`, on Go 1.25.8, 1.26.4 and 1.27.1. - Widening the IPv4 octet from `25[0-5]` to `25[0-9]` fails 4 tests. Putting back the loose `\[[0-9A-Fa-f:.]+\]` domain literal fails 19 tests. Replacing the group-count guard for compressed IPv6 smarthosts with `true` fails the `[1:2:3:4::5:6:7:8]:25` test. - `make generate` with cozyvalues-gen v1.6.0 leaves the tree unchanged. SMTP delivery against a real relay has not been tested. ### Screenshots No dashboard/UI change. ### Downstream repositories Walked the trigger map against the diff. `packages/extra/monitoring` gains values fields, so its README and schema changed. `monitoring` is in the website Makefile `SERVICES` list, and that reference page regenerates from `README.md` automatically on the next stable tag, so there is no manual follow-up to link. The terraform provider does not model the monitoring service, as there is no such resource under `internal/provider`, so there is nothing to update there either. No package is added or renamed, and no CRD, tooling or node contract is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(monitoring): add optional email alerting through alerta.alerts.email, with configurable severity and alert-name selection and an SMTP password read from the fixed Secret alertmanager-email-password ``` |
||
|
Yan Bondarenko
|
17aaf3e038 |
feat(monitoring): add optional email alert delivery
Platform alerts need an email delivery path alongside Alerta without exposing arbitrary namespace Secrets through tenant-controlled values. SMTP credentials therefore use a fixed Secret name. Alertmanager parses the addresses and connects to the SMTP host only when it sends a notification, so a malformed value there drops alerts instead of failing the release. The chart validates these fields when it renders. Addresses must be literal dot-atom mailboxes, and a domain literal must hold a dotted-quad IPv4 address, because IPv6 literal syntax differs across the supported Go mail parsers. A bracketed smarthost must be a valid IPv4 or IPv6 address, and its port must be between 1 and 65535 without leading zeros. The repeat interval follows the Alertmanager duration grammar and range, so a value that renders is one Alertmanager loads. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Pujitha Paladugu
|
e11e239044 |
fix(api): pass the stored object to UpdatedObject in tenantsecret Update
Motivation: a PATCH (kubectl patch, label, edit) against an existing
TenantSecret answered 404 NotFound while GET on the same name succeeded.
REST.Update called objInfo.UpdatedObject(ctx, nil). The apiserver patch
handler builds the patched object from that old object alone, so with nil it
takes its create-new-object branch and returns NotFound from inside the
handler. The same nil also made the patch reach admission as CREATE rather
than UPDATE.
Approach: hand UpdatedObject the stored Secret converted with
secretToTenant when it exists, and nil only when it does not (the
forceCreate path, unchanged), matching what the generic store and the
sibling application and securitygroup registries do.
Validation: added TestUpdate_PassesStoredObjectToUpdatedObjectInfo, which
drives Update with a patch-style UpdatedObjectInfo that fails on a nil old
object. It fails before the change ("UpdatedObject received a nil old
object") and passes after. Ran `go test ./pkg/registry/...` and
`go build ./pkg/... ./cmd/...`, all passing. Not run: a live cluster
kubectl patch, or the real apiserver patch handler.
Known limitation, unchanged by this fix: tenantToSecret prefers Data over
StringData when Data is non-empty, so a patch that only sets stringData on a
secret that already has data is a no-op, exactly as a GET-then-PUT edit of
stringData already is. Patching data or metadata works. That precedence is a
separate design question and is left alone here.
Report: https://github.com/cozystack/cozystack/issues/3601
Assisted-by: LLM
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
||
|
Andrey Kolkov
|
f6a615f693 |
fix(backups): scrub legacy plaintext passwords from CNPG Backup snapshots (#4558)
## What this PR does A CNPG `Backup` taken while the Postgres app still accepted `users[].password` copied that password into `status.underlyingResources`, where anyone with `get backups` in the namespace can read it. #4078 stopped new snapshots from carrying the field but left every existing `Backup` as it was (#4179). This adds `LegacyPasswordScrubber`, a leader-elected runnable in the backup strategy controller that, on start, lists `Backup` objects and removes `users.*.password` from every snapshot whose `kind`/`apiVersion` is the CNPG snapshot's. - **Why on every leader start and not a platform migration.** The migration hook runs pre-upgrade, while the old controller is still leader, so a `Backup` the old replica writes during the rollout would keep its password and the migration never re-runs. With `--leader-elect` on, the old replicas stop writing before the new leader's scrub runs, and a `Backup` brought back from an object-level export is cleaned on the next start too. On a clean cluster the whole pass is one List from the manager cache, which `BackupReconciler` already populates. - **Write shape.** The patch is a merge patch computed from the diff, so the request body only nulls the `password` keys and cannot overwrite status another writer changed since the List. The snapshot is handled as generic JSON (numbers kept as `json.Number`), so fields the current snapshot type does not know survive. - **Failure handling.** A `Backup` that cannot be decoded or patched is logged and skipped, and a failed List is logged too; `Start` always returns nil, because a returned error would stop the manager and take every backup down over one object. - **Restore compatibility.** Restore already ignored the key; a test decodes a scrubbed snapshot through `unmarshalCNPGBackupSnapshot` to pin that. - RBAC: `list` and `patch` on `backups.cozystack.io/backups` are already granted to the controller, so the chart is unchanged. - `docs/operations/backup-classes.md` no longer tells operators to delete pre-upgrade `Backup` objects, and says what the scrub does not reach (audit logs, etcd backups, external exports of the objects, Helm history of the init-script Secret), so a password once set through `users[].password` is still treated as exposed until rotated. The runnable carries a `TODO(#4179)` to remove it once upgrading from a release that accepted `users[].password` is no longer supported. Verification: `go test ./internal/backupcontroller/...` and `go build ./cmd/backupstrategy-controller/` are green. The new tests cover scrubbing with everything else kept (including an unknown field with a large integer), leaving non-CNPG, other-apiVersion, clean and snapshot-less Backups unpatched, and continuing past a Backup that fails to decode or patch. Disabling the delete, or returning on the first patch error, makes the tests fail. Not exercised on a live cluster yet. Fixes #4179 ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map against the diff: it touches only `internal/backupcontroller`, `cmd/backupstrategy-controller` and the in-repo `docs/operations/backup-classes.md`, with no package, values, schema, CRD, variant or asset change. The website carries no "delete pre-upgrade Backups" workaround this would make obsolete; its stale `users[name].password` rows are the #4078 follow-up already noted there. - [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(backups): the backup strategy controller now removes plaintext `users[].password` values left in `status.underlyingResources` of CNPG `Backup` objects taken before managed Postgres stopped accepting user passwords. It runs on every controller start and needs no action. It does not reach copies outside the cluster (audit logs, etcd backups, exported `Backup` objects), so a password once set through `users[].password` should still be treated as exposed until it is rotated. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * At controller startup, plaintext user passwords are removed from matching CNPG backup snapshots. Other snapshot data is preserved. * This cleanup does not affect external copies or remove older init-script manifests from Helm history. Treat passwords previously set through `users[].password` as exposed until rotated. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b86fd7aa5b |
fix(platform): make mongodb-application depend on its operator (#3845)
The mongodb chart renders `kind: PerconaServerMongoDB` from `psmdb.percona.com/v1`, but `cozystack.mongodb-application` depended only on `cozystack.networking` and `cozystack.cozystack-engine`. So `mongodb-rd` registers the ApplicationDefinition as soon as the engine is up, a tenant can create a MongoDB before the operator exists, and its HelmRelease then fails with `no matches for kind "PerconaServerMongoDB"`. Every other operator-backed application here already carries the edge. This is the same defect #3817 fixes for etcd. It also closes a test-selection gap. Without the edge `cozystack.mongodb-operator` reaches no runnable suite, so every change to the operator escalates to a run of all suites instead of selecting `mongodb`. See `src_to_suites()` in `hack/select-e2e.sh`. ## Checks `packages/core/platform/tests/sources_mongodb_application_dependson_test.yaml` pins the edge alongside the baseline ones, mirroring the etcd guard. Mutation checked: removing the edge turns it red, restoring it turns it green. `hack/select-install.sh --validate` reports the graph still has no cycle, and all platform helm unit tests pass. ## The exposure this adds An administrator who lists `cozystack.mongodb-operator` in `bundles.disabledPackages` while keeping the application now leaves it `DependenciesNotReady`. That is the same exposure postgres, mariadb, kafka, redis and etcd already carry, with `Package.spec.ignoreDependencies` as the escape hatch. ### Release note ```release-note fix(platform): the mongodb application now waits for the mongodb operator, so a tenant can no longer create a MongoDB before the CRD that serves it exists ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured the MongoDB operator is available before MongoDB application components are registered or deployed. * Preserved required dependencies on networking and the platform engine. * **Tests** * Added coverage verifying the MongoDB application dependency configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Closes #3850 |
||
|
Myasnikov Daniil
|
b8bc0dfd5f |
fix(platform): make mongodb-application depend on its operator
The mongodb chart renders kind: PerconaServerMongoDB from psmdb.percona.com/v1, but cozystack.mongodb-application depended only on networking and cozystack-engine. So mongodb-rd registers the ApplicationDefinition as soon as the engine is up, and a tenant can create a MongoDB before the operator exists; its HelmRelease then fails with no matches for kind "PerconaServerMongoDB". This is the same defect #3817 fixed for etcd. Every other operator-backed app here already carries the edge. It also closes a test-selection gap: without it cozystack.mongodb-operator reaches no runnable suite, so every change to the operator escalates to a run of all suites instead of selecting mongodb. select-install.sh --validate reports the graph still has no cycle. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d6b38f0a79 |
fix(kubernetes): fail the OIDC bootstrap Job when the tenant CA is missing (#4232)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does In `spec.oidc.mode: System`, the post-install/post-upgrade bootstrap Job patches the chart-owned `<release>-oidc-kubeconfig` Secret with a real kubeconfig built from the tenant admin kubeconfig's CA data. When that CA data was empty, the script logged a WARNING and continued, then exited 0. The hook was satisfied, the HelmRelease reported `Ready`, and the Secret was left holding the install-time placeholder kubeconfig, which cannot authenticate. This PR makes the Job fail (`exit 1`) instead, so the failure surfaces as a red install and can be retried (the Job already has `backoffLimit: 6` and `restartPolicy: OnFailure`) rather than reported as a falsely healthy release. This is the smallest of the three directions raised on the linked issue, and matches how the neighbouring failure mode in the same script already behaves (a failed `kubectl apply` of the ClusterRoleBindings already fails the Job under `set -euo pipefail`). Behaviour for `mode: None` and `mode: CustomConfig` is unchanged, and the common case — CA data present — runs the identical patch commands as before. A regression test was added to `packages/apps/kubernetes/tests/oidc_test.yaml` asserting the rendered Job script contains the `exit 1` on empty `CA_DATA` and no longer contains the old warn-and-continue text. Confirmed it fails against the pre-fix template and passes against the fix. Validation: - `helm unittest .` in `packages/apps/kubernetes` — 150/150 tests pass (up from 149 before the new test). - `helm template ... --show-only templates/oidc-rbac-job.yaml` with representative `System`-mode values renders valid YAML; the extracted bootstrap script passes `bash -n` and `shellcheck -s sh` introduces no new warnings versus the pre-change script. - `make generate` was not run/needed — this change touches neither `values.yaml`, `values.schema.json`, `Chart.yaml`, nor `README.md`. - Not run: a live cluster reproduction (Kamaji-issued admin kubeconfig actually missing CA data at Job run time). The defect is provable from the script logic itself and the added unit test demonstrates the fail-then-pass behaviour; there is no live cluster available in this environment. ### Screenshots Not applicable — no UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [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: This change is confined to the shell logic inside `packages/apps/kubernetes/templates/oidc-rbac-job.yaml`'s post-install hook Job (a `Values.oidc.mode: System`-only failure path) and its accompanying helm-unittest test. It does not add, rename or remove a package, a `values.yaml` key, a CRD, a namespace, a release-owned object name, or any other surface the trigger map lists, so no downstream repository is affected. ### 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(kubernetes): a System-mode OIDC install no longer reports Ready while the tenant OIDC kubeconfig Secret still holds the install-time placeholder — the bootstrap Job now fails (and retries) instead of only logging a warning when the tenant cluster's CA data isn't available yet. ``` Fixes #3765 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * OIDC setup now fails with an error if the tenant admin kubeconfig is missing certificate authority data, rather than leaving the OIDC kubeconfig Secret at its install-time placeholder. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6580d2eeb0 |
feat(kubernetes): storage and cloud-controller integration for Proxmox-backed tenants (#4089)
## What this PR does **Series behind #3481** — three separable pull requests: 1. #4087 — the Cluster API providers (capmox + in-cluster IPAM) 2. #4088 — the `substrate` switch in `kubernetes` and `kubernetes-nodes` 3. #4089 — the Proxmox CSI driver and cloud-controller-manager #4088 has merged, so this diff is the eight commits on top of it: `proxmox.pool`, `csi-proxmox`, `ccm-proxmox`, `delete.yaml`, the tests, the e2e suite, one README sentence on token privileges, and the reconcile Job's replace-on-immutable step for existing clusters. The `substrate` value this gates on is on `main`. ### One chain, not two components Storage and the cloud-controller-manager are one chain, not two components: ``` kubelet --cloud-provider=external → taint node.cloudprovider.kubernetes.io/uninitialized → CCM applies topology.kubernetes.io/region|zone + providerID → CSI node plugin starts → PVC binds ``` The CSI node plugin reads region and zone off its own Node and exits fatally without them: ``` F main.go:126] Failed to get region or zone for node: …, region: , zone: , see documentation about topology labels ``` Those labels come from the cloud-controller-manager, and the CCM ignores a Node until the kubelet's `--cloud-provider=external` has tainted it. Shipping the CSI driver alone gives a tenant a StorageClass, a healthy controller, and a node plugin in CrashLoopBackOff. The CSI driver has no equivalent knob for this. Its `features.provider` accepts `default` and `capmox` and validates neither, so an unrecognised value falls through to the default path — the key is therefore not set at all. The cloud-controller-manager is the opposite case: `capmox` is a real mode there and is set, because the default one writes a Node providerID of `proxmox://<node>/<vmid>` while capmox has already written `proxmox://<SMBIOS UUID>` on the Machine. So the CCM has no `enabled` switch and its credentials reference is required: on this substrate, a cluster without a CCM is a cluster where every worker keeps the `uninitialized` taint and nothing schedules. A chart that cannot name the Secret should stop rather than render that. The CSI release depends on cilium only; the CCM is a Deployment of this chart, not a HelmRelease, so nothing in the tenant can depend on it — the node plugin waits on the labels the CCM applies, which is the ordering that matters. ### Packaging | Package | Chart | App | |---|---|---| | `packages/system/proxmox-csi` | proxmox-csi-plugin 0.5.12 | v0.20.0 | The vendored chart is the tenant-side half only. It **drops the upstream `values.talos.yaml` preset**, which pins the workloads onto `node-role.kubernetes.io/control-plane` and `node.cloudprovider.kubernetes.io/platform: nocloud`: a Kamaji tenant has no control-plane nodes at all — its control plane is a Deployment in the management cluster — so that preset leaves the components unschedulable. It is published as a component of the existing `kubevirt` variant of `kubernetes-application` rather than in a new variant: a second variant would rename the artifacts of all eighteen tenant addons, and exactly one component differs between the substrates. The cloud-controller-manager is not a vendored chart. It is a Deployment the `kubernetes` chart renders itself (`templates/ccm-proxmox/controller.yaml`), because it runs here, in the management cluster, and there is nothing of it to install in the tenant. Credentials never reach the tenant. Each controller names a Secret in the tenant's namespace of this cluster (`proxmox.csi.credentialsSecretName`, `proxmox.ccm.credentialsSecretName`), an init container composes the config file from it into a memory-backed emptyDir at start-up, and the `kubernetes` release stores only the Secret's name. They are separate knobs from the capmox credentials, and from each other, so an operator can scope them: attaching disks is a different blast radius from creating VMs, and the CCM only reads inventory. `proxmox.insecure` defaults to `false`; a stand whose Proxmox serves a self-signed certificate sets it to `true`. `proxmox.csi.enabled: false` switches off the controller as well as the tenant release, and needs no CSI token. ### Where the hypervisor token lives, and why it decides the shape Both controllers run in the tenant's namespace of **this** cluster and drive the tenant over its admin kubeconfig. Only the CSI node plugin goes into the tenant, and it mounts no credentials. Installing them into the tenant would hand the token to the tenant. The resource definition lists the admin kubeconfig in `secrets.include`, so a tenant is cluster-admin of its own cluster, and the vendored chart writes the driver's `config.yaml` — token included — wherever it is installed. The privileges the driver documents are granted on `/`, so such a token reaches every VM and every volume on the Proxmox cluster. The KubeVirt path already has the shape that works, in `templates/csi/deploy.yaml`, and the upstream components support it: the CSI controller's `--kubeconfig` is documented for running out of cluster. Three consequences of the split are handled rather than left to be discovered: the vendored chart has no switch for its controller, so the tenant release sets `replicaCount: 0`; its Secret template is gated on a non-empty `config.clusters`, so an empty list keeps the credential out; and `CSIStorageCapacity` is published by the provisioner sidecar, which now owns those objects through a Pod that does not exist in the tenant, so `CSIDriver.storageCapacity` is patched to false by a kustomize postRenderer — the chart hardcodes it, and left true the scheduler refuses every Pod with "node(s) did not have enough free storage". Volumes are owned per cluster. The driver names them `vm-<controllerVmID>-pvc-…` and owns every volume as VMID 9999 unless `controllerVmID` is set, which leaves different clusters' volumes indistinguishable on shared storage. Each cluster sets its own, derived from its tenant namespace and release name, so two tenants' clusters with the same name still get different owners. **This does not make one tenant unable to reach another's volumes.** A tenant can still create a static PV naming a foreign `volumeHandle`, and the controller will attach it, detach it or delete it: the driver checks neither the VMID a node ID names nor the volume a handle names against the cluster it serves. Closing that needs a per-tenant Proxmox scope, and the charts can now produce one: `proxmox.pool` on the `kubernetes-nodes` chart puts every worker VM into a Proxmox resource pool as capmox clones it, replacements included, so one ACL on `/pool/<name>` follows every VM without being granted again. The README states the scope where an operator configuring the chart reads: a storage of the tenant's own (`Datastore.Allocate` on a storage reaches every volume on it), `VM.Audit`/`VM.Config.Disk` on that pool for the CSI controller, `VM.Audit` and `VM.GuestAgent.Audit` on it for the CCM — a NotReady Node whose VM the token cannot see is reported as gone and deleted — and what that scope still leaves open: the tenant's own disks, which a cluster-admin of the tenant could reach anyway. ### Deleting a cluster frees its disks The pre-delete hook deletes the CSI driver in Step 2, and a volume is freed only by the driver answering DeleteVolume, so a Step 1b runs first: it deletes the tenant's claims (`--wait=false`; a claim in use carries `kubernetes.io/pvc-protection`, so it clears once nothing mounts it), then the Pods holding each claim, then waits for the PVs rather than the PVCs. The Pods are found by listing every Pod with the claims it mounts and matching in the shell: a JSONPath filter on the claim name fails for a whole namespace as soon as one Pod there mounts two claims, and would find no Pod at all. The claims go first because a claim being deleted is one the scheduler places no new Pod on, so a Pod that a StatefulSet brings back cannot mount it again. Without the step the disks stayed on the hypervisor with the tenant's data in them and no PV object left to name their owner. A tenant apiserver that does not answer is told apart from a tenant with no claims: the first goes through the hook's backstop and shows in its summary, the second is the clean case. The probe is bounded with `timeout`, not `--request-timeout`, which the repo's hook check refuses because a non-zero value diverts an in-cluster kubectl call to localhost:8080. On the KubeVirt path two things change. The pre-delete hook's Step 5 checks that the DataVolume CRD exists before deleting DataVolumes, so a hook that runs where CDI is not installed no longer fails after every earlier step has run; the hook runs only at deletion. The worker pool's talos-reconcile Job changes at upgrade: its Role's TalosConfigTemplate rights are narrowed to the pool's own template and gain a named `delete`, and its script gains the replace step, which moves the Job's content-hash name. Every existing worker pool therefore gets a new reconcile Job when it upgrades. Where a pool's live TalosConfigTemplate still matches the render, that Job changes nothing; where it has drifted, the Job now deletes and recreates it instead of failing. Machines already running keep the bootstrap data they joined with; Machines created afterwards boot from the current template. ### Existing clusters: the TalosConfigTemplate CABPT will not let us patch CABPT's validating webhook rejects any change to `TalosConfigTemplate.spec` (`TalosConfigTemplate.Spec is immutable`). The kubelet flag this PR adds changes that spec, so on a cluster created before it the reconcile Job's `kubectl apply` fails with `is immutable` on every run, and a pool whose `ProxmoxMachineTemplate` also changed (say, `proxmox.pool` set) rolls its workers off the old template: they join without `--cloud-provider=external`, the CCM leaves them unmanaged, and the CSI node plugin exits on the missing zone label. The last commit makes the Job replace the object when apply reports the spec immutable: delete, apply again, log both. Existing Machines keep their bootstrap data; new ones are built from the replaced template. Pinned by `talos_tct_immutable_test.yaml`, and the RBAC the Job runs with covers exactly that delete (`resourceNames` of its own template). ### Evidence ``` $ KUBECONFIG=<tenant> kubectl get node -o custom-columns=… kubernetes-px3-md0-rp4s5-rgr48 prox-home prox-home proxmox://b903f905-7b4f-4048-b1b6-3e8f58530d8f $ kubectl -n tenant-proxmox3 get deploy kubernetes-px3-pcsi-controller 1/1 kubernetes-px3-pccm 1/1 $ KUBECONFIG=<tenant> kubectl get pods -n csi-proxmox csi-plugin-node 3/3 Running (the controller Deployment has 0 replicas) $ KUBECONFIG=<tenant> kubectl get secret -A | grep -c token_secret 0 $ KUBECONFIG=<tenant> kubectl get pvc csi-probe csi-probe Bound pvc-… 1Gi RWO proxmox ``` That the volume is real, not merely `Bound` in the API, and that it is this tenant's: ``` $ qm config <vmid> scsi1: main-pool:vm-798497-pvc-…,size=1G ``` Deleting the PVC frees the disk from the storage; the e2e probe checks that too. ### Known limitation The CCM logs every five minutes on PVE 9: ``` error get ha-groups 500 cannot index groups: ha groups have been migrated to rules ``` Proxmox 9 changed the HA-groups API. Node initialization is unaffected — labels and providerID are applied — but the CCM image v0.15.0's HA-groups lookup is not compatible with PVE 9. ### Testing `make unit-tests` — exit 0. `apps/kubernetes` 28 suites / 338 tests; `apps/kubernetes-nodes` 33 suites / 182 tests / 12 snapshots. Each of the eight commits passes both suites on its own. What the new asserts pin, each shown to go red on the mutation it exists for: the tenant release carries no credentials and no `valuesFrom`; the controllers run here with the Secret mounted and the tenant as their target; `csi.enabled: false` renders no CSI controller and needs no CSI token; the kubelet runs with `cloud-provider: external` on proxmox and without it on kubevirt; Step 1b and the cleanup Role's admin-kubeconfig entry render on proxmox and not on kubevirt, and Step 1b deletes the claims before the Pods; `proxmox.pool` reaches the `ProxmoxMachineTemplate` and an empty one renders no `pool`; each tenant gets its own volume owner id; the CCM runs in capmox mode; the CCM tolerates exactly two taints. `proxmox.ccm`, `proxmox.csi` and `proxmox.insecure` are optional in the schema — each has a default, and the render enforces what the substrate needs — so a `Kubernetes` object written against `main`'s `proxmox` block stays valid; the API review gate reports no sizeable change at any of the commits. The e2e suite ships `.disabled` and no CI lane runs it, so the substrate has no automated coverage until a runner with the `proxmox` label exists. What is in the tree is runnable: the fixture is rendered by `run-proxmox.sh` with explicit defaults (GNU `envsubst` does not expand `${VAR:-default}`, so the defaults live in the script and the render refuses a fixture with a variable left in it — pinned by `hack/proxmox-e2e-fixture_test.bats`), the runner reads the `.conf` kubeconfig a runner outside the management cluster can reach, the StorageClass and the PVC probe read the same storage variable, the probe checks the disk is gone after the claim is deleted, `keep_tenant` is honoured by the suite's own teardown, the workflow fails on an unset stand variable instead of falling back to a hardcoded address, and its schedule runs only where `vars.PROXMOX_NIGHTLY` is set, since the job needs a self-hosted runner with the `proxmox` label. The fixture takes the stand's resource pool through `COZY_PVE_POOL`. ### Screenshots Not applicable — no UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against this diff, file by file. What it matches is below. **No box is ticked**, because every follow-up it points at depends on a decision that is not mine to make: #3481 asks whether Proxmox becomes a supported platform at all, and writing its documentation or its Terraform schema before that is answered would be work built on an assumption. I have not filed speculative PRs or issues in those repositories for the same reason. If the direction is accepted, I open them; a human should decide which. **cozystack/terraform-provider-cozystack — matches, no follow-up yet.** - `packages/apps/kubernetes/values.schema.json` gains `proxmox.ccm.credentialsSecretName`, `proxmox.csi` (`enabled`, `credentialsSecretName`, `storageClasses[]`) and `proxmox.insecure`, with defaults in `values.yaml`. Same coupling as the substrate PR: hand-written schema, model and expand/flatten, and defaults the provider restates and sends itself. **cozystack/website — unclear, deliberately left unticked.** This PR adds one package under `packages/system/`, not under `packages/apps/` or `packages/extra/`, so the app-list trigger does not fire. Whether `proxmox-csi` counts as a "platform component" for `guides/platform-stack/_index.md` is a judgement about that page's scope: it is not part of the platform stack — it is an addon installed *into* a tenant cluster, like the KubeVirt CSI driver, which has no entry there either. I read that as no, but I am not confident enough to decide it for you, so the box stays empty. The README section on token privileges and per-tenant scope is the kind of text `operations/` pages carry; that follows the decision #3481 asks for. **Everything else: no match.** No app package added or renamed; no version enum, `kind`, `plural` or `release.prefix` change; `packages/core/platform/values.yaml` untouched; `packages/core/platform/sources/kubernetes-application.yaml` gains one component but is not a renamed `*-rd` file (ccp); `hack/` gains `e2e-chainsaw/_lib/run-proxmox.sh`, a suite directory shipped `.disabled`, and `proxmox-e2e-fixture_test.bats`, which runs with the other bats files; no `ApplicationDefinition` semantics or `release.chartRef.kind` enum change (external-apps-example); no node prerequisites; no telemetry metric; no commit-convention change. - [ ] 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 feat(kubernetes): add the Proxmox CSI driver and cloud-controller-manager for tenant clusters on the `proxmox` substrate, giving them PersistentVolumes backed by Proxmox storage. The cloud-controller-manager is not optional there: worker kubelets run with `--cloud-provider=external`, so without it every node keeps the `uninitialized` taint and the CSI node plugin cannot start. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## New Features - Added Proxmox VE as an infrastructure option alongside KubeVirt for Kubernetes clusters and worker pools. - Added configuration for Proxmox networking, DNS, VM templates, storage, node placement, credentials, IPv4 allocation, and storage classes. - Added Proxmox cloud-controller-manager and CSI integrations. - Added validation for required Proxmox settings and unsupported combinations. ## Bug Fixes - Improved recovery when Talos configuration templates cannot be updated because they are immutable. ## Documentation - Documented Proxmox configuration, requirements, and supported parameters. ## Tests - Added coverage for Proxmox rendering, sizing, integrations, validation, and KubeVirt compatibility. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ### Verified on a live stand Run against three throwaway tenants on Proxmox VE 9.2, each built and destroyed: - every Secret in the tenant cluster scanned for the token — none found, while the PVC round-trip works; - a PVC bound, mounted, written and read back, its disk visible on the hypervisor as `vm-798497-pvc-…` rather than `vm-9999-…`; - `helm uninstall` with the volume still mounted by a running Pod, after which the disk is gone from the storage with nothing done by hand. What that run pinned: the CCM's capmox mode, capacity-aware scheduling off with `CSIDriver.storageCapacity` patched, and the teardown Role's admin-kubeconfig entry. A second run, on the head of this PR, used a pool per tenant and two privilege-separated tokens holding exactly the README's rows, on a throwaway tenant and then on the stand's long-lived cluster upgraded in place: - the worker VM lands in the pool as capmox clones it, and so does its replacement after the Machine is deleted — the CCM initialises the new node and the CSI attaches a disk to it with no ACL touched; - no `401` or `403` from the hypervisor in the CSI controller or the CCM over the whole run, once the roles were granted to the user as well as the token; - the PVC round-trip on both clusters, the disk gone from the storage after the claim; `helm uninstall` with a claim held by a running Pod deletes the claim before the Pod and waits for the volume to be released; - the in-place upgrade replaced the immutable template on the first apply conflict and the re-created worker came up with `--cloud-provider=external`. |
||
|
Pujitha Paladugu
|
4fe45eba32 |
fix(kubernetes): fail the OIDC bootstrap Job when the tenant CA is missing
In mode=System the post-install hook Job patches the chart-owned <release>-oidc-kubeconfig Secret with a real kubeconfig built from the tenant admin kubeconfig's CA data. When that CA data was empty the script only warned and exited 0, so the hook was satisfied, the HelmRelease reported Ready, and the Secret kept the install-time placeholder kubeconfig, which cannot authenticate. The failure only showed up when a user tried to use the downloaded kubeconfig. Exit 1 instead, so the install fails visibly and is retried. The Job already has backoffLimit: 6 and restartPolicy: OnFailure, and a failed ClusterRoleBinding apply in the same script already fails the Job the same way. Assisted-by: LLM Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> |
||
|
Marian Koreniuk
|
e5dc086338 |
fix(kubernetes-nodes): replace the TalosConfigTemplate CABPT's webhook will not let us patch
CABPT's validating webhook rejects any change to TalosConfigTemplate.spec
(TalosConfigTemplate.Spec is immutable), so the worker reconcile Job's
`kubectl apply` fails the moment any input the render depends on genuinely
changes. The apiserver address is the one that moves in practice: the Job
writes it into certSANs and into the worker's endpoint, and a cluster that
gets a new address leaves the Job dead with
TalosConfigTemplate.bootstrap.cluster.x-k8s.io "…" is invalid:
spec: Invalid value: …: Spec is immutable
from then on, until somebody deletes the object by hand. Nothing in the
cluster says that is what has to happen.
The Job now recognises that one failure and replaces the object: delete, then
apply the same manifest. Every other apply error still stops the Job, because a
delete followed by a create that fails for the same reason — an apply rejected
for its content, say — would leave the pool with no template, unable to scale
until the render is fixed. The replace itself does not roll the pool: Machines
reference the template by name, and the name does not change. The match is on
the message for that reason, and the Role's new `delete` is bound to this pool's
own template by name, so the Job cannot reach a sibling pool's.
While the Role is being touched anyway, its other TalosConfigTemplate rights are
pinned to the same name. `get`, `patch` and `update` were unnamed, and several
clusters can share a tenant namespace: a Job that can patch a sibling pool's
template can rewrite the machineconfig that pool's next workers boot. `create`
stays unnamed because RBAC matches resourceNames only on verbs that name an
object.
The manifest is captured into a variable rather than written to a file: the
container runs with readOnlyRootFilesystem, and one variable also means the
replace applies exactly what the first attempt did.
Both applies stay server-side, as the Job's single apply already is: a
client-side merge pulls the full OpenAPI schema and runs the Job out of its
256Mi. The webhook rejects a spec change the same way under server-side apply,
so the `is immutable` match still fires.
UPGRADE NOTE. This changes the KubeVirt path as well, and the render with it:
the talos-reconcile Role gains the `delete` rule, its TalosConfigTemplate rights
are narrowed to this pool's own object, and the Job's script changes,
which moves the Job's content-hash name. Every existing worker pool therefore
gets a new reconcile Job on upgrade. On a pool whose live TalosConfigTemplate
no longer matches the current render, that Job will now delete and recreate it
instead of failing. Machines already running are unaffected — their bootstrap
data was rendered when they joined — but Machines created afterwards boot the
new machineconfig. On a pool where template and render agree, nothing happens.
Both new asserts were checked against the mutation they exist for: deleting the
`exit 1` from the `*)` branch and pointing the delete at `${RELEASE}` each turn
the matching test red. A bare `exit 1` pattern does not — the input-timeout path
near the top of the script already has one.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
5be222ad75 |
docs(kubernetes): say the token's user needs the pool privileges too
A privilege-separated Proxmox token never exceeds its user's permissions. Granting the README's roles to the token alone left the CSI and CCM tokens with an empty effective set on the pool and the storage; granting the same roles to the user made the token's own ACL the effective one. Seen on Proxmox VE 9.2.20 while running the storage pair against a pool-scoped user, and worth one sentence next to the table so nobody repeats it. Assisted-by: LLM Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> |
||
|
Marian Koreniuk
|
7500a9fbae |
test(kubernetes): add the Proxmox e2e suite, parked disabled
Upstream CI has no hypervisor, so this suite cannot run there; it ships
`.disabled` and is run by a lane that owns a Proxmox stand.
Named for what the chart actually creates. The resource definition prefixes the
release, so a Kubernetes CR called k8s-e2e produces
ProxmoxCluster/kubernetes-k8s-e2e in the tenant's own namespace, and the admin
kubeconfig Secret is kubernetes-k8s-e2e-admin-kubeconfig.
Stand-specific values are variables rather than hardcoded. Every one of them is
a bare ${COZY_PVE_*} that run-proxmox.sh expands with envsubst, supplying the
defaults itself — GNU envsubst leaves a ${VAR:-default} in the text exactly as
it is, set or not, and that reaches the apiserver as a string where the schema
wants an integer — and refusing a fixture with a variable still in it. The
StorageClass and the PVC probe read the same COZY_PROXMOX_STORAGE, so the volume
lands where the probe looks. The runner reads the tenant kubeconfig's .conf key,
the address a runner outside the management cluster can reach.
The stand's resource pool comes in through COZY_PVE_POOL, empty by default.
hack/proxmox-e2e-fixture_test.bats pins the shape of the fixture and the render.
The PVC probe reads the storage again after the claim is deleted: a deleted
claim whose disk is still there is the leak the pre-delete hook's Step 1b
exists to prevent. keep_tenant is honoured by the suite's own teardown, not
only by the workflow's. The workflow carries no fallback for the stand's
variables: an unset one fails the job before a key is sent anywhere. The
schedule runs only where vars.PROXMOX_NIGHTLY is true: the job targets a
self-hosted runner with the `proxmox` label, and a scheduled run on a repository
without one would sit in the queue until GitHub cancels it.
The suite also asserts the half of the deployment that lives in the management
cluster — the CSI controller and the cloud-controller-manager — because that is
the property the pair is designed around and a tenant-only assertion would pass
with the token in the wrong place.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
36b04218d7 |
test(kubernetes): assert where the hypervisor token is and where it is not
Every assert here was written against the mutation it exists for, and each was checked to turn red on it and green without: putting the controller back in the tenant, giving the tenant a non-empty config.clusters, pointing a sidecar at the wrong kubeconfig, restoring the blanket toleration, hardcoding the driver's default owner id, dropping capmox mode, and letting the controller ignore proxmox.csi.enabled. The negations carry `any: true`. Without it a negated containsDocument passes whether or not the document is in the render. Two of the cases are about objects rather than values: the tenant release must carry no valuesFrom at all, and two clusters must derive different volume owner ids. Both are properties a reader can check against the rendered output, which is why they are asserted that way rather than by reading the templates. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
5ab36023e0 |
fix(kubernetes): release Proxmox volumes before the driver that frees them goes
Deleting a tenant cluster left its PVC disks on the hypervisor. The pre-delete
hook deletes every HelmRelease labelled with the cluster in Step 2, the CSI
driver among them, and nothing before that deleted the tenant's PVCs — a volume
is freed only by the driver answering DeleteVolume. capmox then destroys the
worker VMs, and Proxmox frees only what the destroyed VMID owns, which these
volumes are not. They stayed on storage with the tenant's data in them, and the
PV objects that could have named their owner went with the control plane.
Step 1b deletes the claims while the driver is still up, and it deletes the
claims before the Pods that hold them: a claim being deleted is one the
scheduler places no new Pod on, so a Pod that a StatefulSet or Deployment brings
back cannot mount it again and hold it to the backstop. The Pods go second: a
claim in use carries the kubernetes.io/pvc-protection finalizer, so it clears
only once nothing mounts it, and the Pods are going with the cluster in any
case. Then it waits for the PVs rather than the PVCs, because the claim
disappears as soon as its finalizer clears while the disk is freed only when the
PV is gone.
The Pods holding a claim are found by listing every Pod with the claims it
mounts and matching in the shell. A JSONPath filter on the claim name cannot do
it: with a Pod that mounts two claims its left side yields two values, client-go
refuses the whole expression ("can only compare one element at a time"), and no
Pod in that namespace would be found for any claim.
A kubeconfig that exists is not an apiserver that answers, and listing claims
against one that does not comes back empty — the clean case. Step 1b asks
/readyz first and sends an apiserver that stays silent through the backstop, so
the summary carries it. The probe is bounded by `timeout` rather than by
--request-timeout: a non-zero --request-timeout on an in-cluster kubectl call
diverts it to localhost:8080, which is why hack/check-hook-request-timeout.bats
refuses the flag in hook templates, and the check cannot tell this call, which
carries its own kubeconfig, from one that does not.
Best effort, and loud about it: a tenant that never came up has no apiserver to
ask, and a teardown must not wedge on that, but a timeout is reported through
the same backstop the other steps use.
The hook's Role gains one Secret by name — the tenant's admin kubeconfig, which
this step needs. Without it the step takes its no-kubeconfig branch and reports
that there is nothing to release while the volumes stay behind.
Step 5 no longer fails the whole hook where CDI is not installed. It deletes
DataVolumes, which are a KubeVirt-path component, and kubectl exits non-zero on
an unknown resource type; under set -e that failed the teardown after every
earlier step had already run, leaving the cluster half removed and the hook
reporting failure.
Verified on a throwaway tenant: a PVC bound and mounted by a running Pod, then
`helm uninstall`, and the volume freed by the driver rather than by hand.
delete_hook_test.yaml pins Step 1b and the Role's admin-kubeconfig entry on
proxmox and their absence on kubevirt, the /readyz probe, and the order of the
claim and Pod deletes.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
d902421f47 |
feat(kubernetes): deploy the Proxmox cloud-controller-manager with the cluster
On this substrate the worker kubelet runs with --cloud-provider=external, which taints every node node.cloudprovider.kubernetes.io/uninitialized until a cloud provider clears it. Without a cloud-controller-manager the taint is permanent: nothing but tolerating DaemonSets schedules, and the CSI node plugin — which reads topology.kubernetes.io/region and zone off its own Node and exits fatally without them — never starts. It runs in the management cluster for the same reason as the CSI controller beside it: it authenticates to the hypervisor, and the tenant is cluster-admin of its own cluster. Its whole job is to write on the tenant's Nodes, and it does that over the tenant's admin kubeconfig from out of cluster, which also means there is no tenant-side RBAC left to install. capmox mode, not the default one. The default resolves a node to its VM by name and writes providerID as proxmox://<node>/<vmid>, while capmox reads the CAPI Machine and writes proxmox://<SMBIOS UUID> — the form capmox itself puts on the Machine. With the default the Node and its Machine carry two different identifiers for the same VM. The tolerations are spelled out rather than a blanket `operator: Exists`. That one also matches node.kubernetes.io/unreachable:NoExecute with no tolerationSeconds, which stops admission adding the bounded 300s one and lets the Pod outlive the Node it runs on. `proxmox.insecure` defaults to false: the controllers verify the hypervisor's certificate unless an operator turns that off, and both run here, so the value describes what the management cluster trusts, not the tenant. The README gains a section on what the two tokens can reach — the controller acts on whatever VMID and volume handle the tenant's objects name — and on the per-tenant Proxmox scope that contains it: a storage of the tenant's own and one ACL on the resource pool proxmox.pool puts its workers in, which follows every VM capmox creates — VM.Audit there for the CCM too, which deletes a NotReady Node whose VM it cannot see. ccm, csi and insecure are optional in the schema. Each has a default, and what the substrate needs is enforced at render time — a proxmox cluster without ccm.credentialsSecretName does not render — so a Kubernetes object written before these fields existed stays valid, and adding them is not a breaking change to the API. The kubelet flag is asserted on both substrates: present on proxmox, absent on kubevirt, where nothing would ever clear the taint it causes. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
93c89c8bf6 |
feat(kubernetes): add the Proxmox CSI driver, with its controller in this cluster
Tenant clusters on the proxmox substrate get persistent volumes backed by the
hypervisor's storage. The driver is sergelogvinov/proxmox-csi-plugin, vendored
as a chart that reproduces byte-for-byte from `helm pull`.
It is deployed in halves, and which half goes where is the whole design. The
controller authenticates to Proxmox, so it runs HERE, in the tenant's namespace
of the management cluster, and drives the tenant over its admin kubeconfig —
the same shape templates/csi/deploy.yaml already uses for kubevirt-csi. The
tenant gets the node plugin, the CSIDriver object and the storage classes; its
DaemonSet mounts no credentials.
Putting the controller in the tenant would hand the token to the tenant. The
resource definition lists the admin kubeconfig in secrets.include, so the
tenant is cluster-admin of its own cluster and can read any Secret there, and
the chart writes the driver's config.yaml — token included — wherever it is
installed. The privileges the driver documents are granted on `/`, so that
token reaches every VM and every volume on the Proxmox cluster, not just this
tenant's.
Three consequences of the split are handled here rather than discovered later:
- The vendored chart has no switch for its controller, so the tenant release
sets replicaCount: 0. The Deployment object still renders; no Pod does.
- Its Secret template is gated on a non-empty config.clusters, so an empty
list is what keeps the credential out.
- CSIStorageCapacity is published by the provisioner sidecar, which now runs
here and owns those objects through a Pod that does not exist in the
tenant. Left on, CSIDriver.storageCapacity=true and the scheduler refuses
every Pod with "node(s) did not have enough free storage". The chart
hardcodes that field, so a kustomize postRenderer patches it rather than
forking the chart.
The controller follows proxmox.csi.enabled, like the tenant release: with the
driver switched off there is nothing for it to serve and no token to hold.
Volumes are owned per tenant. The driver names them vm-<controllerVmID>-pvc-…
and defaults the id to 9999; on shared storage that made every tenant's volumes
indistinguishable to an operator and to a VM-level ACL. Each cluster derives its
own from its release name.
What this does not do is make one tenant unable to reach another's volumes: a
tenant can still create a static PV naming a foreign volumeHandle. That needs a
per-tenant Proxmox scope — a dedicated storage and a VM ACL per tenant — which
is a deployment requirement rather than a chart change.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
fc729299f4 |
feat(kubernetes-nodes): put Proxmox workers into a resource pool
The privileges a Proxmox token needs — VM.Audit and VM.Config.Disk for the CSI controller, VM.Audit and VM.GuestAgent.Audit for the cloud-controller-manager — are granted on an ACL path, and the only path that follows every VM capmox creates, replacements included, is a resource pool. Without one the choices are `/`, which reaches every VM on the hypervisor, or each VMID, which has to be granted again after every scale-up and every Machine replacement, and until it is the new worker gets no disks and no providerID. proxmox.pool names the pool; capmox adds each worker to it as it clones the VM (ProxmoxMachineTemplate.spec.template.spec.pool). The pool has to exist on the hypervisor — capmox does not create it — and every node pool of a tenant cluster sets the same one, or a worker outside it is a worker the tenant's tokens cannot reach. Empty adds the workers to no pool, which is what the chart did before. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
7d44372a16 |
fix(postgres): read the live Cluster when deciding the WAL-archive prefix (#4299)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Fixes #4298. `applyClusterPluginBackup` preserves the `serverName` already on the Cluster so that a backup does not flip the WAL-archive prefix mid-stream, which would split the archive across two prefixes. That rule is right; the read it rests on is not reliable in one window. The value was read through the manager's cached client, and `BackupJobReconciler.SetupWithManager` registers `For(&BackupJob{})` and nothing else, so a Cluster only reaches the cache through a lazily started informer with no resync tied to the Cluster's own changes. An in-place `RestoreJob` deletes the Cluster and the chart re-renders it with `bootstrap.newServerName` seconds later, so a read served during that window describes the object that was purged — and the source's `serverName` is written back over the freshly rendered one. `barman-cloud-check-wal-archive` then refuses the destination with `Expected empty archive` and the `RestoreJob` ends on its 30-minute deadline, which is the failure `newServerName` was added in #3988 to prevent. The same stale read carries a stale `UID`, and that `UID` owns the `ObjectStore` this function applies, so the store is garbage-collected as soon as it is written. Reading through the dynamic client this reconciler already carries keeps the preservation rule and makes the value it rests on truthful. There is precedent in the same file: the HelmRelease suspend/resume path reads that way for the same reason. Observed on a v1.6.3 cluster, twice on separate runs with the same four-second gap between the chart's apply and the driver's. The regression test makes the two clients disagree the way they disagree in that window and asserts the driver believes the live one; reverting the fix fails it on the serverName assertion. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- 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: ### 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(postgres): an in-place restore no longer loses its fresh WAL-archive prefix to a stale Cluster read, which left the restored cluster archiving onto the source's prefix and the RestoreJob failing on its deadline ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backup processing now uses the latest CNPG Cluster state rather than potentially outdated cached information. This ensures the cluster’s current server name and identity are used when associating backup ObjectStore resources, including when the cached and live cluster details differ. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
08c344ab5f |
feat(vpc): expose allowSubnets per subnet (#4478)
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->
## What this PR does
Adds an optional `allowSubnets` list to each entry of `subnets` in the
VirtualPrivateCloud chart, rendered into `spec.allowSubnets` of the
kube-ovn `Subnet`.
Every VPC subnet is rendered `private: true`, so kube-ovn drops any
packet whose source is outside the subnet's own CIDR. A tenant who runs
a VPN or router VM on a subnet already gets the forward path through
`routes` (a static route with the VM as `nextHopIP`), but traffic
arriving from the remote network is dropped by the private ACL. Today
the only fix is an operator patching the cluster-scoped `Subnet` by
hand: the tenant cannot do it, no application value records it, and it
is lost whenever the subnet is recreated.
```yaml
spec:
subnets:
- name: gateway
cidr: 172.16.1.0/24
allowSubnets:
- 192.168.1.0/24
routes:
- cidr: 192.168.1.0/24
nextHopIP: 172.16.1.10
```
The subnet stays private; the list only widens the ingress ACL of the
subnet that declares it, inside the tenant's own VPC. An absent or empty
list renders nothing, so existing releases produce the same manifests.
The chart had no helm-unittest suite, so this adds a `test` target and
`tests/subnet_test.yaml`, covering a subnet without the field, one with
it, the field staying scoped to its own subnet, and an empty list. Two
of the four cases fail against the template on `main`.
`allowSubnets` is a kube-ovn field. #4034 teaches this chart to render
Cozyplane objects when kube-ovn is absent, so whichever of the two lands
second has to decide how the other backend treats the list.
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up: cozystack/terraform-provider-cozystack#61 (the provider
models `subnets` by hand in `vpc_schema.go` and `vpc_model.go`, so the
new nested field needs a schema and expand/flatten change there; an
issue rather than a PR, since it should follow this one merging)
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
feat(vpc): subnets accept an optional `allowSubnets` list of CIDRs admitted into the private subnet, so a VM on the subnet can act as a VPN or router gateway without a manual patch of the kube-ovn Subnet.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Private subnets can now allow additional source CIDR ranges, such as
remote networks routed through a gateway. This setting is optional and
can be configured for individual subnets.
* **Tests**
* Added checks for rendering subnet CIDRs, including cases where the
setting is omitted or applied to only one subnet.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
3e4030d4d9 |
feat(kubernetes): per-pool kernel modules for Talos worker nodes (#3571)
## What this PR does Implements gap 1 of #3563: a pool-scoped `kernelModules` field on `KubernetesNodes`, emitted as Talos `machine.kernel.modules`. A Talos system extension installs a kernel module but does not load it — loading is `machine.kernel.modules`' job. The worker machine config had no such block and no values key could add one, so a GPU pool on Talos workers was unusable as shipped: the extension supplied the NVIDIA module, nothing loaded it, and the failure was silent end to end. The VM held the PCI device, the node advertised no GPU, and no component logged an error. Found in a production 1.6.0 environment, where the worker rollover took the 4 GPUs of a tenant cluster out of service with no error anywhere. ### Design: three states, and no default The shape was arbitrated in #3563 (option 3 of the three proposed there). - **absent** — the chart chooses. A pool holding at least one `nvidia.com/*` GPU gets `nvidia` (with `NVreg_NvLinkDisable=1`), `nvidia_uvm`, `nvidia_drm`, `nvidia_modeset`; any other pool gets no `kernel` block at all. - **non-empty list** — taken verbatim, replacing the chart's choice outright. - **`[]`** — explicit opt-out: no modules even on a GPU pool. The order is not cosmetic: Talos loads the list in sequence, `nvidia` has to come first because the other three depend on it, and `nvidia_uvm` is what CUDA unified memory needs. This is the order validated against a production GB202 passthrough node. `NVreg_NvLinkDisable=1` is what the gpu-operator driver container applies on non-Talos workers; on Talos that container has to be off, so nothing else would carry it, and without it the driver waits forever for an NVLink fabric that the passed-through GPU does not have. The three states are distinguishable only because the field carries no default — no entry in `values.yaml`, no `default` in `values.schema.json`. A default of `[]` would collapse "absent" into "opted out" for every pool and make the automatic set unreachable. The generated parameter table prints `[]` in the Default column for any array with no default; the field description says so explicitly. For the same reason `keysOrder` does not list the field, so the dashboard renders it at the bottom of the form. Automatic emission is keyed on the `nvidia.com/` resource prefix, not on the mere presence of `gpus`, so an AMD pool is not given NVIDIA modules on a guess. The field loads modules; it does not put them in the image. Which extensions the image carries is the pool's schematic, so a GPU pool points `osImage.factory.schematicID` (or `osImage.builtin.schematicID`) at a schematic carrying the NVIDIA extensions. Module names and parameters reach the reconcile Job's unquoted heredoc, so both are validated at render, and the emitted list is rebuilt from those two fields alone: the schema does not prune unknown keys, and any other key passed through `toYaml` would reach the shell unchecked. A parameter must be printable ASCII with no whitespace, and must not contain `$`, a backtick, a backslash or quotes. The ASCII half is there because RE2's `\s` and `[:cntrl:]` are ASCII-only: without it U+2028 and U+2029 pass, `toYaml` writes them raw, and Helm's YAML parser reads them as line breaks that end the Job's `command` block scalar early. Inside ASCII the guard denies what the shell expands rather than allowing a fixed set, because NVIDIA's documented multi-value parameter is semicolon-separated. ### Rollout, and #3523 A pool that resolves to no modules renders the machine config it rendered before, byte for byte: `tests/render_snapshot_test.yaml` and its committed golden snapshot pass unmodified, including the `talos-reconcile` Job's content-hash name. For a pool that does get modules, what happens depends on how its `TalosConfigTemplate` is named, and the field description states both cases: - **On main today** the template has a fixed name and an immutable spec, so the reconcile Job's apply of a changed machine config is refused (#3515). Running workers keep what they booted with, and new workers boot from the old template too. This covers the upgrade itself: every existing pool holding an `nvidia.com/*` GPU with the field unset gets the automatic set, and its apply is refused the same way. Only pools created afterwards pick the modules up. - **Once #3523 lands** the template is named by its content, so the same change renames it and CAPI rolls the pool. On the upgrade that brings both, every existing pool holding an `nvidia.com/*` GPU with the field unset rolls onto the automatic set. That roll is the intended outcome — those pools have no working driver without it — but it is a roll, and the release note says so. ### Tests - `packages/apps/kubernetes-nodes/tests/kernel_modules_test.yaml`, 12 cases: the NVIDIA set and its order for an `nvidia.com/*` pool, no `kernel` block for a pool with neither GPUs nor modules, an explicit list taken verbatim including `parameters`, an explicit list replacing the NVIDIA default, `[]` opting out while leaving the `gpu=on` label intact, no NVIDIA assumption for another vendor, a semicolon-separated parameter accepted, a hostile module name and a hostile parameter each rejected, a parameter carrying U+2028 and one carrying U+2029 each rejected with the kernelModules message rather than a YAML parse error, and an unvalidated extra key on an item dropped. The patterns pin absolute indentation, because `modules:` one level out is still valid YAML and silently loads nothing. - `hack/talos-reconcile-heredoc_test.bats` lists `kernelModules` in its validated arm and gains two execution-level cases: a hostile name and a hostile parameter each refused at render, and a semicolon-separated parameter surviving the real shell literally. Loosening the guards to `^.+$` makes the refusal case fail. - `helm unittest packages/apps/kubernetes-nodes`: 31 suites, 174 tests, 12 snapshots, all passing. `hack/cozytest.sh hack/talos-reconcile-heredoc_test.bats`: 8 passed. - `make generate` in the package, and `controller-gen object` for the `api/apps/v1alpha1` submodule. ### Not in scope The rest of #3563: the `nvidia-operator-validator` → device-plugin chain that cannot validate an OS-provided driver, CDI spec generation and the `nvidia` containerd runtime default, and the `maxSurge` / `nodeStartupTimeout` defaults. ### 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/issues/643 - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/20 - [ ] [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: Both follow-ups are issues rather than PRs. `terraform-provider-cozystack` is hand-written, so the field needs a schema entry, a model entry and an expand/flatten pair on `KubernetesNodes`; the undefaulted three-state behaviour is exactly where that provider's habit of sending its own defaults would break users silently, since materialising `kernel_modules = []` would disable the automatic NVIDIA set for every GPU pool managed through Terraform. `website` needs the three-state behaviour written down, the GPU pool example pointed at `osImage.factory.schematicID`, and the Blackwell constraint documented: on GB202 the schematic must carry `siderolabs/nvidia-open-gpu-kernel-modules-production`, because with the proprietary extension the module loads, `/dev/nvidia0` appears, and `nvidia-smi -L` then reports `No devices found`. The `hack/talos-reconcile-heredoc_test.bats` change adds test cases and does not change node prerequisites, so `ansible-cozystack` and `talm` are not reached. No other repository in the map is touched: no package, platform component, bundle, variant or release asset is added, and `ApplicationDefinition` semantics are unchanged. ### Release note ```release-note feat(kubernetes): Worker node pools (`KubernetesNodes`) accept a `kernelModules` field, emitted as Talos `machine.kernel.modules`. A Talos system extension installs a kernel module without loading it, so a GPU pool previously came up with no working driver and no way to fix it through values. Left unset, a pool holding at least one `nvidia.com/*` GPU now loads `nvidia` (with `NVreg_NvLinkDisable=1`), `nvidia_uvm`, `nvidia_drm` and `nvidia_modeset` automatically; an explicit list replaces that set, and an explicit `[]` opts out. The modules still have to be in the image: point the GPU pool's `osImage.factory.schematicID` at a schematic carrying the NVIDIA extensions, and on Blackwell (GB202) use the open kernel modules extension, not the proprietary one. Pools without GPUs and without the field render exactly what they rendered before. NOTE: while the worker `TalosConfigTemplate` keeps a fixed name (#3515), a changed module list does not reach a pool that already exists — including the automatic set on existing GPU pools — and only pools created afterwards pick it up. Once that template is named by its content (#3523), the same change rolls the pool, so existing GPU pools that leave the field unset are rolled onto the automatic set. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Worker node pools can now configure Talos kernel modules, including optional module parameters. * When no list is specified, NVIDIA GPU pools automatically receive NVIDIA modules; other pools receive none. A custom list replaces this selection, while an empty list disables modules. * Invalid module names or parameters are rejected during configuration rendering. * **Documentation** * Added guidance on module selection, parameters, image-extension requirements, and pool rollout behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8ed34166cf |
perf(lineage): cache owner GETs and short-circuit re-admission (#2667)
## What this PR does The lineage mutating webhook walks the owner chain (Pod, StatefulSet, HelmRelease, application CR) on every admission, with one uncached dynamic GET per level, and sibling Pods of the same parent repeat the same lookups. On a multi-tenant cluster this put `lineage.cozystack.io` at 300-500ms p99 in `apiserver_admission_webhook_admission_duration_seconds`, the largest share of admission latency. Owners are now cached in-process for 60 seconds, and only successful lookups are stored. A failed lookup (RESTMapping, NotFound, timeout) leaves the walk without an owner, so the object is admitted with no lineage labels and stays unlabeled until its next UPDATE runs the walk again. Secrets, Services and PVCs can go a long time between updates, so a cached failure would leave every object admitted within the TTL unlabeled for that long. The aggregated cozystack-api makes such failures routine, since discovery of `apps.cozystack.io` fails while it rolls. The one staleness left is the scheduling class, which is read from the application's spec: a change to it reaches newly admitted Pods up to 60 seconds later. The cache is not swept below 4096 entries, so up to that many owner objects stay in memory for the life of the process. Above that, each insert drops the expired entries among a random sample of 64, which keeps the write lock short. Non-Pod objects that already carry every lineage label, `internal.cozystack.io/tenantresource` included, skip the walk, because the objectSelector still matches an UPDATE whose old object lacks the label. Pods go through regardless, as each Pod admission also sets the scheduling class. ### Screenshots Not a UI change. ### Downstream repositories The diff is Go under `pkg/lineage` and `internal/lineagecontrollerwebhook` only: no label, annotation, CRD, values key, `hack/` file or telemetry metric is added or renamed, so no entry of the trigger map 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 perf(lineage): the lineage mutating webhook caches owner lookups for 60 seconds and skips the owner walk for non-Pod objects that already carry lineage labels, cutting admission latency on clusters with many tenants. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Lineage and owner lookups now use a time-limited in-memory cache, reducing repeated API requests during admission processing. Scheduling-class updates may take up to 60 seconds to appear on new Pods. * Non-Pod objects with complete lineage labels are allowed without additional owner lookup or mutation. * **Tests** * Added coverage for cache hits, misses, expiration, and retrying lookups after errors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
877d2ff43c |
fix(linstor): build piraeus-server's protobuf code on an arm64 builder (#4576)
## What this PR does The arm64 half of a release fails to build piraeus-server. linstor-server's build always downloads the `linux-x86_64` protoc, and the builder stage runs on the build platform. Since the release builds arm64 on a native arm64 runner, that protoc cannot execute there, and `./gradlew generateJava` fails with `Syntax error: ")" unexpected`. The first multi-arch pre-release run, v1.7.0-alpha.2, stopped on it. A new patch to linstor-server picks the protoc build for the host's architecture. An amd64 builder still gets the same x86_64 protoc as before. The generated code is Java, so one native builder serves both target platforms. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Builds on ARM-based hosts now select a compatible Protocol Buffers compiler download, avoiding failures caused by using an x86_64-only build. Builds on other supported hosts continue to use the x86_64 compiler. This improves compatibility across build architectures without changing the resulting application behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->v1.7.0-alpha.3 |
||
|
Aleksei Sviridkin
|
e56478cf2f |
feat(keycloak): expose organizationsEnabled realm toggle (#3031)
## What this PR does - Adds an `organizationsEnabled` value to the `cozy-keycloak-configure` chart (default `false`, no behaviour change). - Maps it to `ClusterKeycloakRealm.spec.organizationsEnabled` so the Keycloak [Organizations](https://www.keycloak.org/docs/latest/server_admin/#_managing_organizations) feature can be turned on declaratively for the platform realm. ### Why The chart hardcoded the realm without exposing this field. Enabling Organizations through the Keycloak admin console is not durable: the EDP keycloak-operator reconciles `ClusterKeycloakRealm` by `PUT`ting the chart-rendered spec back to the realm, so the console change is reverted on the next reconcile (operator restart, periodic resync, or any CR change). Verified on Keycloak 26.5.2. ### Release note ```release-note feat(keycloak): add `organizationsEnabled` value to the keycloak-configure chart to enable the Keycloak Organizations feature on the platform realm (default: false) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a configuration option to enable or disable organizations in the platform-managed Keycloak realm. * Organizations remain disabled by default. Set the option to `true` to enable them. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
018b49929f |
feat(monitoring): auto-route tenant logs using namespace labels (#2195)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does A tenant that runs its own monitoring stack gets a VictoriaLogs instance, but the node fluent-bit in `cozy-monitoring` sends every container log to `global.target` (`tenant-root`), so that instance stays empty. This PR routes each namespace's container logs to the VictoriaLogs named by its `namespace.cozystack.io/monitoring` label, the label the tenant chart already sets and the per-tenant vmagent already selects on. **Chart (`packages/system/monitoring-agents`).** `templates/_tenant-log-routes.tpl` looks up the namespaces and keeps those whose label is set and names a tenant other than `global.target`. For those, fluent-bit gets one `rewrite_tag` filter with a `Rule` per target (a single emitter however many tenants are routed), one `http` output per target to `vlinsert-generic.<target>.svc:9481`, and a per-target `modify Set tenant <target>`, because the re-tagged record re-enters the filter chain where the `Match *` `Add tenant` cannot overwrite the key. Unlabelled namespaces, empty labels and labels equal to `global.target` stay on the default outputs. **Controller (`internal/controller/tenantlogrouting`).** `lookup` results are not part of the digest helm-controller compares, so the chart alone would not pick up a tenant that turns monitoring on, or drop one that is removed, until an unrelated upgrade. The new reconciler in cozystack-controller digests the monitoring label across all namespaces and, when the digest moves, sets `reconcile.fluxcd.io/requestedAt` and `reconcile.fluxcd.io/forceAt` on `cozy-monitoring/monitoring-agents`, which forces an upgrade and re-runs the lookup. It digests every labelled namespace rather than repeating the chart's selection, so it cannot drift from the chart; the cost is an occasional upgrade that renders the same config. The digest it acted on is kept in its own annotation, so a manual `flux reconcile` does not make it force another upgrade. The existing ClusterRole already grants the verbs it needs. **Behaviour change for the platform operator.** Routed records are re-tagged with `keep false`: once a tenant runs its own monitoring, its namespaces' container logs go to that tenant's VictoriaLogs and no longer to the root one. Events and audit logs are not routed and stay on the root store. **Why not a per-tenant collector.** An earlier comment on this PR proposed a fluent-bit per tenant, following the per-tenant vmagent and the OTLP collector from #3763. That collector receives OTLP over the network; a log collector has to tail `/var/log` through `hostPath`, which the PodSecurity level of tenant namespaces rejects, and running it in a privileged system namespace instead would mean one DaemonSet per tenant tailing every node's logs. The routing therefore stays in the one node DaemonSet, with the reconciler closing the reconcile gap. Tests: `tests/fluent-bit_tenant_log_routing_test.yaml` mocks the namespaces with `kubernetesProvider` and pins which get a rule and an output, which are skipped (unlabelled, empty label, label equal to `global.target`), and that a routed record carries the target tenant. Three of its cases fail against the previous revision of this branch. `tests/fluent-bit_tenant_log_routing_offline_test.yaml` pins that an offline render routes nothing. `internal/controller/tenantlogrouting/reconciler_test.go` covers first sight, no-op while labels are unchanged (including after a manual reconcile), a tenant turning monitoring on, a routed namespace being deleted, and a missing release. Refs #2194 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> No entry of the trigger map matches the diff: it touches `packages/system/monitoring-agents` (values, a template helper, tests), a new reconciler in `internal/controller`, its registration in `cmd/cozystack-controller`, and a comment in the cozystack-controller ClusterRole. The one open question is `cozystack/website`: `content/en/docs/next/operations/services/monitoring/logs.md` describes log collection in general terms and does not say where a tenant's container logs land, so the behaviour change above could deserve a sentence there. I have left every box empty for a maintainer to decide. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(monitoring): container logs from a tenant that runs its own monitoring stack now go to that tenant's VictoriaLogs instead of the root one. The node fluent-bit routes them by the namespace.cozystack.io/monitoring label, and cozystack-controller re-renders the node agents when that label changes on any namespace. ``` |
||
|
Aleksei Sviridkin
|
811f030f15 |
fix(cozy-lib): round the sanitized CPU request up to a whole milli-CPU (#3522)
## What this PR does Fixes #2917. `cozy-lib.resources.sanitize` divides the CPU limit by `cpuAllocationRatio` and rendered the quotient as a bare float. When the ratio does not divide the value evenly the result is finer than a milli-CPU, and the VPA admission webhook rejects the object outright: ``` admission webhook "vpa.k8s.io" denied the request: minAllowed: CPU [62500u] must be a whole number of milli CPUs ``` `packages/extra/etcd/templates/vpa.yaml` routes its VPA `minAllowed`/`maxAllowed` through the helper, so with `cpuAllocationRatio: 4` its floor renders as `250m / 4 = 62.5m` and the `etcd` HelmRelease loops `UpgradeFailed` → `RollbackFailed` forever: the rollback target renders the same invalid value. Since v1.6.0 HelmReleases wait on each other, so that one stuck app also blocks `tenant-root` → `cozystack-basics` → `kubevirt-cdi-operator`, and without CDI the Talos worker rollover cannot import the worker OS image. Only the divisors of 250 worked as ratios, which is an artefact of one chart's hardcoded `250m` floor rather than a real constraint on the operator. - `packages/library/cozy-lib/templates/_resources.tpl`: round the sanitized CPU request **up** to the next whole milli-CPU. Up, never down, so the request stays at or above the intended fraction of the limit. And only when rounding is actually needed: an exactly divisible value keeps its current float rendering, so nothing changes on the default ratio. - The whole-milli test is exact, with no tolerance around it. sprig's `divf`/`mulf` run on `shopspring/decimal` rather than float64, so `mulf (divf 0.25 10.0) 1000.0` renders exactly `25` in Helm and there is no binary-float noise on this codepath to smooth over. A tolerance would in fact reintroduce the bug: a value that is whole-milli only to within `5e-7` would read as exact and render verbatim, so `cpu: 100.0000004m` at ratio 1 would render `0.1000000004`, rejected for the same reason `62.5m` is. There is a test for that case. - `packages/tests/cozy-lib-tests/`: new `resources` template plus a `resources_test.yaml` suite covering the indivisible ratio, the round-up direction, the two unchanged-rendering cases, a whole-milli-only-within-a-tolerance value, sub-milli input at an ordinary and at an extreme ratio, a genuinely zero request, and that memory is untouched. - `packages/extra/etcd/tests/vpa_test.yaml`: regression guard asserting the chart's rendered VPA `minAllowed.cpu` is a whole milli-CPU (`63m`) with `cpu-allocation-ratio: 4`, and that the default ratio still renders `0.025` / `0.5`. ### Why in `sanitize` and not in the etcd chart The issue also suggests taking `minAllowed`/`maxAllowed` out of `sanitize` in the etcd chart, on the grounds that VPA policy bounds are not workload requests. I looked at that first and it is the wrong fix here: - `minAllowed`/`maxAllowed` bound the **request** the VPA writes, and in Cozystack every request is `limit / cpuAllocationRatio` — `packages/extra/etcd/templates/etcd-cluster.yaml` sanitizes `.Values.resources` the same way. On the default ratio the etcd request is `1000m / 10 = 100m`, bracketed by `minAllowed 25m` and `maxAllowed 500m`. Dropping `sanitize` would make the floor a literal `250m`, i.e. **10x** the intended request floor, and let the ceiling reach `5000m`. It would also change rendered output on the default ratio, for every existing cluster. - It fixes one chart. The same helper feeds every app's requests, and at `cpuAllocationRatio: 4` seven other charts (`http-cache`, `mariadb`, `nats`, `rabbitmq`, `redis`, `tcp-balancer`, `valkey` — all from the `250m` nano presets) also render `cpu: "0.0625"`. Those go into pod requests rather than a VPA object so they are not rejected, but they are still not quantities we should be emitting. - No other chart routes `minAllowed` through `sanitize`, so there is no consistency argument either way: the other VPA templates (`etcd-operator`, `monitoring`, `monitoring-agents`, `vpa-for-vpa`) use hand-picked literals in system charts where no allocation ratio applies. ### How it was validated - `hack/helm-unit-tests.sh`, full run: 81 packages green, before and after. The only difference between the two runs is the two new suites appearing as `PASS`; no existing suite changed status. - Both new suites were confirmed to **fail** against `main`'s helper (5 tests in `resources_test.yaml`, 1 in the etcd `vpa_test.yaml`), so they guard the regression rather than merely describing the new behaviour. - Breadth of the bug: over the 905000 combinations of a whole-milli limit `1m`..`1000m` against an integer ratio `1`..`905`, `main` renders a quantity finer than 1m for **898026** of them (99.2%) and this branch for **none**. - Rendered-output check on the default ratio: `helm template` over all 164 chart directories under `packages/{apps,extra,system,core}`, before and after, byte-diffed. 129 render standalone; all 129 are identical apart from 8 whose diff is only their per-render random secrets and self-signed certs (no `cpu` line differs in any of them). - Same sweep with `--set _cluster.cpu-allocation-ratio=4`: `main` emits 11 sub-milli `cpu: "0.0625"` requests across 7 charts, this branch emits **0**. - The quoted webhook error was checked against the VPA we actually ship (1.5.1): `pkg/admission-controller/resource/vpa/handler.go` wraps the quantity error as lowercase `minAllowed:`/`maxAllowed:`, and `62.5m` canonicalises to `62500u`. - Not verified: nothing from this branch was applied to a live cluster, so the end of the chain (the webhook accepting the object, the etcd HelmRelease converging, CDI unblocking) is inferred from the rendered manifests and the webhook's own error message, not observed. ### Screenshots Not applicable, no UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The diff is one Helm library template plus test files: no package added, renamed or removed; no `values.yaml`, `values.schema.json`, version enum or default changed; no `ApplicationDefinition`, CRD, namespace, `hack/` file, package layout or release asset touched. The terraform provider restates app defaults, but no default moves here — the rendered value only changes for a non-default `cpuAllocationRatio`, which the provider does not model. - [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: ### Release note ```release-note fix(cozy-lib): round the sanitized CPU request up to a whole milli-CPU. A `cpuAllocationRatio` that does not divide a chart's CPU value evenly (for example `4` against the `250m` VPA floor of the etcd chart) produced a quantity finer than 1m, which the VPA admission webhook rejects, leaving the etcd HelmRelease looping UpgradeFailed/RollbackFailed and — since v1.6.0, where HelmReleases wait on each other — blocking tenant-root, cozystack-basics and CDI behind it. Any allocation ratio now renders a valid quantity; rendering is unchanged whenever the ratio divides evenly, including on the default ratio of 10. Where a ratio does not divide a configured CPU value evenly, the rendered request changes once on upgrade: for example `cpu: 125m` at the default ratio goes from `0.0125` to `13m`, and so does every nano preset at a ratio like 4. The API server already rounds pod requests up the same way, but the string stored in operator-managed resources (CNPG clusters, KubeVirt VMs and so on) changes, and some operators may roll their pods for it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - CPU resource requests are now sanitized to avoid floating-point precision issues. - Fractional milli-CPU requests are rounded up to the nearest whole milli-CPU, with positive sub-milli requests clamped to `1m`. - Exact and zero CPU values remain unchanged. - CPU limits and memory requests continue to be preserved without modification. - **Documentation** - Updated resource guidance to describe CPU request rounding behavior. - **Tests** - Added coverage for allocation ratios, default bounds, rounding, precision, and resource preservation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6ab3e0be78 |
feat(postgres): allow preloading an allowlist of shipped modules (#3506)
## What this PR does - Adds `postgresql.sharedPreloadLibraries`, a closed enum of four module names that already ship in the CloudNativePG PostgreSQL image, rendered onto the CNPG `Cluster` field of the same meaning. - Leaves the `shared_preload_libraries` entry in the `postgresql.parameters` blocklist exactly as it is. - Adds `tests/shared_preload_libraries_test.yaml`: absent by default, single and multiple libraries, no interference with `parameters`, schema rejection of a non-allowlisted name and of an absolute path, and the parameter blocklist still firing when the allowlist is in use. - Adds `hack/check-postgres-preload-allowlist.bats`, which renders the chart with `--skip-schema-validation` to exercise the template-side allowlist that helm-unittest never reaches, and checks that it matches the schema enum. `shared_preload_libraries` is blocked in `postgresql.parameters`, and rightly so: the parameter is free text and accepts a path, so it can point at a writable location such as the PGDATA volume. The block does not make every preloadable module unreachable, though. CloudNativePG carries a managed extensions list (`ManagedExtensions` in `pkg/postgres/configuration.go`, checked on v1.30.0, the operator version in this repo): as soon as a parameter under the `pg_stat_statements.`, `auto_explain.` or `pgaudit.` prefix is set, the operator adds that library to `shared_preload_libraries` itself, and for `pg_stat_statements` and `pgaudit` it also runs `CREATE EXTENSION`. The chart already passes such keys through, so `postgresql.parameters."pg_stat_statements.track": all` works on main today. What this PR adds is narrower than a missing capability. `pg_prewarm` has no route at all, and its autoprewarm worker only runs when the module is preloaded. For the other three, the field is an explicit switch that does not require setting one of their tuning parameters, even to its default value, just to get the library loaded. The two routes combine rather than override: the operator merges this list with the libraries it preloads on its own, so removing a module from `sharedPreloadLibraries` does not unload it while a parameter under its prefix is still set. The field description says so. The distinction this PR draws is between a library *name* and a library *path*. Nothing needs to be injected to preload these four: `pg_stat_statements.so`, `auto_explain.so`, `pgaudit.so` and `pg_prewarm.so` are already in `$libdir` in the image, and that directory is on the read-only root filesystem, so it cannot be written to. `dynamic_library_path` stays blocked, so `$libdir` remains the only search path. A bare name from a fixed enum therefore has no way to reach code that is not already in the image, which is the property the blanket block was protecting. The enum in `values.schema.json` is the primary guard and rejects a bad value before rendering starts. The template carries the same list as a backstop for renders that bypass schema validation, using the existing fail-fast `SECURITY:` idiom from the parameter blocklist a few lines below it. helm-unittest validates the schema on every render, so it never reaches that backstop; the bats suite renders with `--skip-schema-validation` instead. I checked that it fails when the template condition is replaced with `false`, and when the template list gains a name the enum does not have. I verified the mechanism end to end on a live CloudNativePG 1.27.3 cluster rather than assuming it: with `spec.postgresql.shared_preload_libraries` set, the operator writes `shared_preload_libraries = 'pg_stat_statements'` into `postgresql.conf`, `SHOW shared_preload_libraries` reports it, and `SELECT ... FROM pg_stat_statements` returns rows. Adding a second module to the list took effect too, and the CNPG instance manager restarts PostgreSQL in place for it, with no pod restart. That restart covers every instance, the primary included, so writes pause briefly. The CRD documents the field as additive to the operator's own defaults, so this does not clobber anything CNPG needs to preload itself. Scope note: the four names are the modules in the image that are useful to a tenant and safe to preload. Anything requiring a background worker or a shipped-but-privileged module was deliberately left out, and the enum is the single place to extend if that changes. ### Screenshots No UI change. ### Downstream repositories `packages/apps/postgres/values.schema.json` gains a field, which per the trigger map reaches `cozystack/terraform-provider-cozystack`: its postgres resource carries a hand-maintained schema and its own expand/flatten pair, with no codegen from `values.schema.json`, so `sharedPreloadLibraries` will be invisible to Terraform users until it is added there by hand. The enum matters twice over there, because the provider restates enums as `stringvalidator.OneOf` lists. I searched that repository and found no open PR or issue covering it (the only open PR is a dependabot Go module bump). I have not filed the follow-up, so I am leaving the box unticked rather than claiming work that does not exist. The same gap applies to `users.<name>.createRole` in #3505; one follow-up there can cover both fields. I walked the rest of the map against the diff and found nothing else: no package is added, renamed or removed, no existing version enum changes, no default changes, `release.prefix` and the output Secret and Service names are untouched, and no CRD outside the regenerated `packages/system/postgres-rd/cozyrds/postgres.yaml` is affected. The `README.md` parameter table is regenerated here and postgres is already in the website generator's app list, so the website path stays automated. - [ ] 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: not filed yet, see the note above - [ ] [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: ### Release note ```release-note feat(postgres): add `postgresql.sharedPreloadLibraries`, a closed allowlist of modules shipped in the PostgreSQL image (`pg_stat_statements`, `auto_explain`, `pgaudit`, `pg_prewarm`) that are preloaded at server start. It is the first way to preload `pg_prewarm`, and lets the other three be preloaded without setting one of their parameters, which CloudNativePG already treats as a request to preload them. The `shared_preload_libraries` parameter stays blocked: only bare names from the allowlist are accepted, never a path. Changing the list restarts PostgreSQL on every instance, the primary included. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional PostgreSQL shared preload library configuration for `pg_stat_statements`, `auto_explain`, `pgaudit`, and `pg_prewarm`. * Preserves the configured library order. CloudNativePG may automatically preload the first three libraries when their related parameters are set; `pg_prewarm` must be enabled through this list. * Changing the list restarts every PostgreSQL instance, including the primary. * **Validation** * Rejects unsupported modules, absolute paths, and attempts to set preload libraries through free-form parameters. * **Documentation** * Added configuration guidance covering supported libraries and restart behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
e126158404 |
fix(ci): keep the controller Go tests running when a unit test fails first (#4233)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The `checks` job in `.github/workflows/pull-requests.yaml` runs `make unit-tests test-controllers -j4 --output-sync=target` as a single step. `unit-tests` and `test-controllers` are independent goals on that command line, and without `-k` GNU Make stops handing out new work the instant any earlier-processed goal's prerequisite fails — a goal that had not yet been scheduled never starts. So whenever a BATS file inside `unit-tests` fails before `test-controllers` gets a job slot, the entire Go controller suite (`go test ./internal/...`) silently never runs, and the PR page shows only the unit-test failure with no signal at all about the controllers. This adds `-k` so already-schedulable goals keep running after an earlier one fails; the job still exits non-zero and still fails whenever anything failed, `-k` only stops make from abandoning independent work. This addresses the second half of https://github.com/cozystack/cozystack/issues/3776. The first half of that report — whether the job's comment overclaims that it gates the merge, since `Unit & controller tests` is not currently in `main`'s required status checks — needs a maintainer's call between two mutually exclusive fixes (weaken the comment, or add the job to branch protection), and is deliberately left untouched here; the issue itself frames the two as independent and separately useful. Note: `hack/unit-test-parallelism.bats`, which pins the literal invocation string in a test, was updated to match. ### Screenshots N/A — CI workflow and test-script change only, no UI. ### 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: I walked the trigger map: this PR only touches CI scheduling flags inside this repo's own `.github/workflows/pull-requests.yaml` and a `hack/*.bats` meta-test that asserts against that same file. Nothing in the trigger map names either file, and the change has no effect on any package, API, chart, or contract another repository consumes. ### Validation Reproduced the exact scheduling gap in an isolated Makefile mirroring this repo's real dependency shape (a `unit-tests`-like goal depending on several independent prerequisites, one of which fails immediately, plus a sibling goal with no dependency on it), under the same GNU Make 4.4.1 `-j4 --output-sync=target` invocation used here. Without `-k` the sibling goal never ran, across 5 repeated trials; with `-k` added it ran to completion every time across 3 repeated trials, and the overall exit code stayed non-zero either way. `bats hack/unit-test-parallelism.bats` passes locally against the real files. I also ran `make unit-tests test-controllers -j4 -k --output-sync=target` against this repo directly: local disk space (a limit of my sandbox, unrelated to this change) made `go test ./internal/...` fail to compile with "no space left on device", so I could not confirm a clean end-to-end pass of `test-controllers` itself in this environment — but the run still exercised the fix: after that build failure, unrelated `hack/*.bats` prerequisites kept being scheduled and ran to completion instead of the whole invocation aborting, which is exactly the behavior `-k` adds over the prior command line. I could not run this repo's actual CI runners to confirm disk is not similarly constrained there; nothing in the workflow or runner spec suggests it would be. ### 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(ci): the `Unit & controller tests` PR check no longer silently skips the Go controller test suite when a Helm-unittest or BATS unit test fails first; both suites now always run and report. ``` --- Part of #3776 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated automated test execution so unit and controller test suites continue running when one test target fails, while the job still reports failures. * Updated workflow validation to reflect the revised test command. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
53829ea109 |
fix(kubernetes): reconcile KCCM EndpointSlices when tenant node VMIs change (#3500)
## What this PR does Adds `patches/vmi-informer.diff` to the `kubevirt-cloud-provider` image of the `kubernetes` package. The file is named after what it does: no upstream pull request carries this change yet (kubevirt/cloud-provider-kubevirt#382 covered the same desync and was closed as rotten), so a number would point at an unrelated upstream PR and invite someone bumping the pin to drop it. The `kubevirteps` controller derives the infra EndpointSlices of a LoadBalancer Service from the VMIs backing the tenant nodes (`getDesiredEndpoints`, and the all-VMIs fallback added by `379.diff`). But it registers informers only on infra Services, tenant EndpointSlices and infra EndpointSlices, and both factories are built with a resync period of `0`. VMI addresses are resolved with a one-off `Get` during a reconcile. Consequently a VMI appearing, disappearing or changing its address triggers no reconciliation, and no periodic relist ever catches up. The EndpointSlices keep pointing at VMIs that no longer exist and the LoadBalancer blackholes the share of the inbound connections hashed onto those dead addresses — sporadic inbound timeouts, with nothing wrong on the tenant side. Services whose tenant EndpointSlice churns along with its pods recover on their own, because that churn produces an event. Cilium Gateway API Services do not: their tenant slice is a static sentinel (`192.192.192.192`, no `NodeName`), so nothing ever forces a recompute. Seen in production on a cluster whose GPU MachineDeployment is rolled by the cluster-autoscaler: 5 of the 9 endpoints of each of the 3 gateways pointed at destroyed VMs (13 failures out of 20 requests). Worth noting for anyone debugging this: the KCCM looks dead (no log for hours) while its lease is still being renewed every second. The process is healthy, it is simply never woken up. The patch: - adds an informer on the infra `VirtualMachineInstances` that requeues the LoadBalancer Services of this cluster (those labelled `cluster.x-k8s.io/cluster-name=<clusterName>`, the ones `reconcile` does not skip) on add/delete, and on updates filtered by a fingerprint of what actually shapes an endpoint: the VMI's cluster label, `Status.NodeName`, `Status.Phase`, and the address of the `default` interface; - gives both informer factories a 5 minute resync, as a safety net if an event is ever missed; - scopes the all-VMIs fallback to the VMIs labelled `cluster.x-k8s.io/cluster-name=<clusterName>`. An infra namespace can hold several tenant clusters as well as standalone `VirtualMachines`, and their VMIs do not serve this Service's NodePort, so selecting them adds endpoints that blackhole traffic. The fallback widens to every VMI only when none of them carries the label, so an environment that does not label them does not lose all of its endpoints; once any VMI is labelled, an empty selection means this cluster has no node right now and is not widened to another cluster's VMIs. No RBAC change: the KCCM role already grants `watch` on `virtualmachineinstances`. On `main`, the Gateway scenario reaches the all-VMIs fallback without any other KCCM patch: Cilium 1.19.5 (shipped since v1.6.0) sets `kubernetes.io/service-name` on the tenant slice of a `cilium-gateway-*` Service (cilium/cilium#46242), so the existing lookup finds the sentinel slice and falls back on its missing `NodeName`. This is why #3374, which added that lookup to the KCCM, was closed. On v1.5.x and earlier, which ship Cilium 1.19.3, a Gateway Service finds no tenant slice at all, so it has no endpoints to go stale in the first place. ## Testing - `341 + 379 + vmi-informer` applies onto the pinned `a0acf33`; the Dockerfile steps (`go get`, `go mod tidy`, `go mod vendor`, `-mod=vendor` build of the manager) pass, `go vet` and `gofmt` are clean. - The patch adds 6 specs, and the suite runs 21 of 21 green: fallback scoped to the cluster, fallback preserved when no VMI is labelled, no widening when only another cluster's VMIs are present, unlabelled VMIs ignored once some are labelled, VMI add/delete requeues the Service of this cluster and not the one of another cluster, and VMI updates driven through the fake dynamic client: an annotation-only update does not requeue, while a change of phase, node, address, or cluster label does. - Mutation-tested, each one turns the suite red: always requeuing on update; dropping the phase, the node, the address or the cluster label from the fingerprint; requeuing every Service of the namespace; dropping the delete requeue. - Under `-race`, the suite reports an intermittent data race at `kubevirteps_controller_test.go:820` (a `PrependReactor` racing an informer list). It reproduces identically on `341 + 379` without this patch, and the new specs pass on every `-race` run. - Validated on a live cluster with an image built from an earlier revision of this patch (same informer and fallback, before the cluster-scoped requeue and the label in the fingerprint): endpoint added in under 2s when a VMI appears, dead address removed in 33s when it is deleted. Before the patch the dead address stayed forever. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff is one patch file of the KCCM image: it touches no values, schema, CRD, `hack/` tooling, namespace, telemetry metric or node prerequisite, so the trigger map comes out empty. ### Release note ```release-note fix(kubernetes): the KCCM now reconciles the LoadBalancer EndpointSlices when the VMIs backing the tenant nodes change, instead of keeping stale addresses that blackhole part of the inbound traffic ``` |
||
|
Aleksei Sviridkin
|
924d28cf3d |
feat(kubernetes): default gpu-operator to an OS-provided driver on Talos (#3585)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Closes the gpu-operator half of #3563 — gap 3 (the operator cannot validate a driver it did not install) and gap 4 (nothing routes CUDA libraries to a tenant pod). Tenant clusters have run Talos workers since 1.6.0, where the NVIDIA driver comes from a system extension baked into the worker image rather than from the operator's driver container. The addon default still described the Ubuntu shape, so a GPU node group came up with the operator fighting the OS for the driver and no path at all for the CUDA libraries. Three changes, each closing one silent failure: | Change | Without it | |---|---| | `driver.enabled: false`, `toolkit.enabled: false` | the driver container loads its own module and clashes with the extension's; the toolkit container rewrites `/etc/containerd/config.toml`, which Talos owns and which already registers the `nvidia` runtime with `enable_cdi = true` and `cdi_spec_dirs = ['/run/cdi']` | | `validator.driver.env`: `DRIVER_ROOT=/`, `NVIDIA_DRIVER_ROOT=/`, `DISABLE_DEV_CHAR_SYMLINK_CREATION=true` | the driver validator looks for a driver-container install, finds none, never writes its sentinel, and the device plugin stays blocked behind `toolkit-validation` — the loop on `Attempting to validate a driver container installation` (**gap 3**) | | `devicePlugin.env`: `DEVICE_LIST_STRATEGY=cdi-annotations`, `NVIDIA_CTK_PATH=/usr/local/bin/nvidia-ctk`, plus `DEVICE_ID_STRATEGY`, `NVIDIA_DRIVER_ROOT`, `CDI_ANNOTATION_PREFIX` | tenant pods carry no `runtimeClassName` and did not need one on Ubuntu, where the toolkit made `nvidia` the default containerd runtime. Talos does not, so CDI is the only route: the pod gets the device and no CUDA libraries (**gap 4**) | Every value is the configuration validated on a production Talos GPU node group, minus the parts that only mattered while the driver container was in play. No DaemonSet, no new image, no new package — the fix is values. ### The removed default The previous default fed `NVreg_NvLinkDisable=1` to the driver container through the `nvidia-kernel-module-params` ConfigMap. With that container off the ConfigMap is mounted by nothing, so this removes the default. On Talos the parameter belongs to the worker machine config, which the `kernelModules` field of the worker pool writes: that field is top-level in the KubernetesNodes app (`packages/apps/kubernetes-nodes`), added by #3571, and its automatic set for a pool holding an `nvidia.com/*` GPU already loads `nvidia` with `NVreg_NvLinkDisable=1`. The wrapper chart keeps the ConfigMap feature for an operator who re-enables the driver container through `valuesOverride`. A test pins the absence so the dead default cannot come back and offer a second, inert source for the same setting. ### Relationship to #3571 Independent files, no conflict, but only useful together: this makes `driver.enabled: false` correct, and #3571 is what actually loads the modules the extension ships. Merged alone, this changes a GPU node group from "operator fights the OS" to "operator stays out of the way and the modules still are not loaded" — better, but not yet working. #3571 alone leaves the operator fighting for the driver. Either order is safe; both are needed for a GPU node group to work. ### What is verified, and what is not Verified here: the rendered HelmRelease values, that `valuesOverride` still wins over each new key, and that the removed default is gone. The individual values are the ones running in production today, applied by hand through `addons.gpuOperator.valuesOverride`. Not verified here: this exact default exercised end-to-end as the chart's own output. That needs a Talos GPU node group whose modules load, which needs #3571 released. I would rather say so than imply a green e2e that does not exist — happy to hold this as draft until I have run a single-node L40S cluster against the chart default, and I will report the result on this PR either way. ### Not covered HAMi. When `addons.hami.enabled` the operator's device plugin stays off and HAMi serves the GPUs; HAMi's own plugin needs equivalent CDI wiring on Talos, which this default does not attempt. Flagging rather than guessing — the production cluster this was derived from runs HAMi disabled, so I have no validated configuration for that path. Worth its own issue if a maintainer confirms the shape. Also not in scope, and unchanged from the merge base: `talos.installerRepository` and `talos.version` reach the worker machine config unvalidated (being tracked separately by @lexfrei), and the `nvidia-operator-validator` chain for the *management* cluster's own passthrough/vgpu variants, which never load the host driver at all. ### Screenshots No UI changes. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/643 - [ ] [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: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(kubernetes): The GPU Operator addon of tenant Kubernetes clusters now defaults to a driver provided by the operating system, which is the shape of the Talos workers these clusters have run since 1.6.0. `driver` and `toolkit` are disabled (the driver comes from a Talos system extension, and containerd configuration belongs to the machine config), the driver validator is pointed at the host root so it stops looping on "Attempting to validate a driver container installation", and the device plugin is switched to CDI (`DEVICE_LIST_STRATEGY=cdi-annotations`) so tenant pods receive the CUDA libraries — previously they received the GPU device and nothing else, since Talos does not make `nvidia` the default containerd runtime. The previous default, which fed `NVreg_NvLinkDisable=1` to the operator's driver container through a ConfigMap, is removed because that container is disabled; on Talos the parameter is carried by the worker machine config, through the `kernelModules` field of the worker pool in the KubernetesNodes app (`packages/apps/kubernetes-nodes`), which loads `nvidia` with `NVreg_NvLinkDisable=1` by default on a pool holding an `nvidia.com/*` GPU. Operators who overrode `addons.gpuOperator.valuesOverride` to work around this by hand can drop those overrides. NOTE: an override that sets `devicePlugin.env` replaces the list rather than merging it, so restate the full list when overriding it or CDI stops working. HAMi is not covered: when `addons.hami.enabled` the operator's device plugin stays off and HAMi's own plugin needs equivalent CDI wiring, which this change does not attempt. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Configuration** * GPU workloads on Talos workers now use host-provided NVIDIA drivers and tooling by default; the GPU Operator’s driver and toolkit are disabled. * Device discovery uses CDI annotations and UUID-based device IDs. When HAMi is enabled, the Operator’s device plugin is disabled. * Host-root validation no longer creates `/dev/char` symlinks. * Driver kernel-module configuration is no longer created by default. * **Documentation** * Clarified when kernel-module configuration applies and what settings are needed to re-enable the driver. * Noted that `valuesOverride` replaces lists wholesale. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d466fd96b3 |
fix(rabbitmq): bound the topology operator Secret cache (#4130)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Fixes #4128. `messaging-topology-operator` 1.14.2 caches **every Secret in the cluster** — its `ClusterRole` grants `secrets: list, watch` cluster-wide with no namespace or label restriction — and the vendored manifest limits it to `500Mi`. On a cluster that has grown enough Secrets it OOMKills and never recovers, because the initial `LIST` alone no longer fits. While it is down, no topology CR reconciles for any tenant. Measured on the affected cluster: 8 211 Secrets across 291 namespaces, 293 MB of Secret payload (87 % of it Helm release history), against 21 topology CRs and 1 `RabbitmqCluster`. Working set sat flat at 331–335 Mi for 30+ hours, crossed 500Mi, then crashlooped 154 times. The operator was caching 8 211 Secrets in order to read 2. Upstream fixed exactly this, and it is not a bigger limit — their default is still `512Mi`: - `329aa9e4e` (2026-06-30, released in v1.20.0) gates the Secret informer on `rabbitmq.com/topology-operator: "true"`, to bound "memory usage on large clusters". - `0850c3ae8` (2026-07-22, released in v1.20.1) bypasses the cache for `Service` reads. v1.20.2 is the current patch release carrying both; v1.19.x and earlier carry neither. This PR refreshes the vendored manifest to v1.20.2 through the package's existing `make update` pipeline (fetch, drop the `Namespace` object, rewrite `rabbitmq-system` → `cozy-rabbitmq-operator`). **The bump alone would break every install**, which is why the app chart moves with it. The app chart's credentials Secret is referenced by `User.spec.importCredentialsSecret`, and from 1.20 the admission webhook rejects a user-provided Secret without the label: ``` secret %q must have label rabbitmq.com/topology-operator=true to be used by the Topology Operator ``` Verified against the 1.20.2 source, two things deliberately do **not** change: - The `RabbitmqCluster` `default-user` Secret needs no label — `rabbitmqclient/cluster_reference.go` reads it through `apiReader.Get`, uncached. - The `Status.Binding` contract is unchanged since 1.14.2, so the vendored `cluster-operator` 2.9.0 stays compatible and is left alone to keep this PR focused. The v1.20.2 manifest is self-contained: it ships its own `Issuer` and both `Certificate` objects (webhook + metrics), and adds probes, a `securityContext` that satisfies `restricted` PodSecurity, a defaulting `MutatingWebhookConfiguration` and secure metrics. Same 13 CRDs, none added or removed. ### Tests - `packages/apps/rabbitmq/tests/user_credentials_test.yaml` renders two users and asserts that each `User` imports its own `<release>-<user>-credentials` Secret and that every credentials Secret carries `rabbitmq.com/topology-operator: "true"`. Deleting the label line from the template fails it. - The rabbitmq e2e suite now creates its cluster with one user and asserts `User/rabbitmq-test-app` reaches `Ready`, so the webhook and the User reconcile run against the real 1.20.2 operator. Its `catch` gains a describe of that `User` and the topology operator's logs. ### Upgrade path Existing installs carry unlabeled credentials Secrets until each tenant's `rabbitmq-*` HelmRelease re-renders. No migration is added: in 1.20.2 the User controller watches Secrets and maps an import Secret back to its Users through the `.spec.importCredentialsSecret.name` field index (`secretToUserRequests`), so relabelling the Secret requeues the User immediately rather than on the next interval, and nothing is removed from the broker in between. The import path no longer writes `<user>-user-credentials`, and nothing in this repo reads the ones 1.14.2 left behind. ### Verification - `make generate` in both touched packages — no diff beyond the intentional changes - `helm unittest packages/apps/rabbitmq` — 103/103, and the new suite fails with the label removed ### Screenshots No UI changes. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> Walked the trigger map against the diff, file by file. `packages/apps/rabbitmq/templates/rabbitmq.yaml` adds one label to a rendered Secret: no package added or renamed, no `values.schema.json` field, no version enum, no `values.yaml` default, no `release.prefix`, and the Secret's *name* is unchanged, so the provider's literal-name lookups are unaffected. `packages/system/rabbitmq-operator/templates/messaging-topology-operator.yml` is a vendored upstream manifest for an already-present component: no package layout change, no `cozyrds` rename, no namespace rename, no installer or platform values key. `packages/apps/rabbitmq/tests/` and `hack/e2e-chainsaw/rabbitmq/` are test-only and appear nowhere in the map. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(rabbitmq): update messaging-topology-operator to 1.20.2, which bounds its Secret cache to Secrets labeled `rabbitmq.com/topology-operator=true` instead of caching every Secret in the cluster. Before this, the operator's memory grew with the cluster's total Secret count and OOMKilled against its 500Mi limit on large clusters, stopping reconciliation of all Queue, Exchange, Binding, User, Permission, Vhost and Policy resources for every tenant. The managed RabbitMQ chart now labels the credentials Secret it creates, so existing users need no action. If you manage topology resources by hand and point `connectionSecret`, `importCredentialsSecret`, `uriSecret` or `upstreamSecret` at your own Secret, add the label `rabbitmq.com/topology-operator: "true"` to it or the admission webhook will reject the resource. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configuration options for resource cleanup, queue types, connection and channel limits, shovel queue arguments, and federation settings. * Added metrics support, health checks, leader-election configuration, and queue webhook capabilities. * Improved compatibility of generated credential Secrets with the RabbitMQ Topology Operator. * **Bug Fixes** * Improved validation and documentation for topology resources, including requiring an exchange for topic permissions. * Updated operator components and Kubernetes resource configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
3c657a4cc1 |
feat(vpc): expose allowSubnets per subnet
Every VPC subnet is rendered private, so kube-ovn drops any packet whose source lies outside the subnet's own CIDR. A tenant that runs a VPN or router VM on a subnet and points a static route at it gets the forward path from the existing routes value, but traffic arriving from the remote network is then dropped by the private ACL. Subnet is cluster-scoped, so the only workaround so far has been an operator patching spec.allowSubnets by hand: something the tenant cannot do, that no application value records, and that is lost whenever the subnet is recreated. Declaring it per subnet keeps the widening in the tenant's own VPC and scoped to the one subnet that needs it. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
91c87b84cd |
fix(kubernetes): refuse non-ASCII kernelModules parameters
RE2's \s and [:cntrl:] only cover ASCII, so the parameter deny class passed U+2028 and U+2029. toYaml writes either one raw, and Helm's YAML parser reads it as a line break that ends the reconcile Job's command block scalar early. The render then failed on an unparseable manifest rather than on the guard, which is the last check before the unquoted heredoc. Pinning parameters to printable ASCII closes every non-ASCII line break at once, and the deny class keeps refusing what the heredoc expands. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
ca24d8f5f6 |
perf(lineage): cache owner GETs and short-circuit re-admission
Every admission walks the owner chain (Pod, StatefulSet, HelmRelease, application CR) with one uncached GET per level, and sibling Pods repeat the same lookups. On a cluster with many tenants that put the lineage webhook at 300-500ms p99, the largest share of apiserver admission latency. Owners are now reused for 60 seconds, successful lookups only. A failed lookup, whether RESTMapping, NotFound or a timeout, leaves the walk without an owner, so the object is admitted with no lineage labels and stays unlabeled until its next UPDATE runs the walk again. Secrets, Services and PVCs can go a long time between updates, so a cached failure would leave every object admitted within the TTL unlabeled for that long, and the aggregated cozystack-api makes such failures routine: discovery of apps.cozystack.io fails while it rolls. Non-Pod objects that already carry every lineage label, tenantresource included, skip the walk, since the objectSelector still matches an UPDATE whose old object lacks the label. Pods go through it regardless, as each Pod admission also sets the scheduling class. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
d6e5c1bf2b |
fix(api): preserve foreign HelmRelease metadata on Application update (#2914)
## What this PR does
The aggregated `apps.cozystack.io` API rebuilds the HelmRelease from the
Application on `Update` and issues a full-replace PUT. This strips
metadata owned by other controllers:
- `metadata.finalizers` — including helm-controller's
`finalizers.fluxcd.io`
- `metadata.ownerReferences`
- labels and annotations outside the `apps.cozystack.io-` prefix (e.g.
`reconcile.fluxcd.io/requestedAt`)
Losing the Flux finalizer is the harmful case: helm-controller re-adds
it on the next reconcile, but a delete landing in that window removes
the HelmRelease **without running `helm uninstall`**, orphaning every
resource of the release (VMs, databases, PVCs...). A label patch
immediately followed by a delete — a common client pattern — hits that
window almost every time. We traced ~106 orphaned releases across 39
tenants on a production cluster back to this.
Reproduction:
1. `kubectl patch <any apps.cozystack.io resource> --type merge -p
'{"metadata":{"labels":{"foo":"bar"}}}'` → the backing HelmRelease loses
`finalizers.fluxcd.io`
2. `kubectl delete` the resource right after → HelmRelease is removed
instantly, Helm uninstall never runs, child resources stay alive
Changes:
- `Update` now fetches the current HelmRelease and carries over
`finalizers`, `ownerReferences`, and non-prefixed labels/annotations
before the PUT
- Keys under the `apps.cozystack.io-` prefix remain owned by the API and
are recomputed from the Application, so label/annotation deletions still
propagate
- Regression test `TestUpdate_PreservesForeignHelmReleaseMetadata` pins
the carry-over contract (fails on the previous behavior with exactly the
stripped fields)
## Release note
```release-note
fix(api): updating an Application no longer strips finalizers, ownerReferences, and foreign labels/annotations from the backing HelmRelease, which could cause deletion to skip helm uninstall and orphan the release's resources
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Application updates now preserve metadata managed by external
controllers, including finalizers, owner references, labels,
annotations, and shard information, when synchronizing related releases.
* Conflict retries now retain the latest release metadata, preventing
changes made between attempts from being lost.
* Creating an application without labels no longer alters shared release
configuration labels.
* **Tests**
* Added coverage for metadata preservation during updates and conflict
retries, and for label handling during application creation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
6c7d22edab |
fix(linstor): build piraeus-server's protobuf code on an arm64 builder
linstor-server's build always downloads the linux-x86_64 protoc, and the builder stage runs on the build platform. The release now builds the arm64 images on a native arm64 runner, where that protoc cannot execute and generateJava fails. A patch picks the protoc build for the host's architecture. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
1f9b1b85db |
fix(kubernetes): reconcile KCCM EndpointSlices when tenant node VMIs change
The kubevirteps controller derives the infra EndpointSlices of a LoadBalancer Service from the VMIs backing the tenant nodes, but it watches no VMI and its informers had no resync period. A VMI appearing, disappearing or changing its address therefore triggered no reconciliation, and no periodic relist ever caught up: the EndpointSlices kept pointing at VMIs that no longer existed and the LoadBalancer blackholed the share of the inbound connections hashed onto those dead addresses. Services backed by a tenant EndpointSlice that churns with its pods recovered on their own. Cilium Gateway API Services did not: their tenant slice is a static sentinel, so nothing ever forced a recompute. Observed in production on a cluster whose GPU MachineDeployment is rolled by the cluster-autoscaler: 5 of the 9 endpoints of every gateway pointed at destroyed VMs. Add an informer on the infra VirtualMachineInstances that requeues the LoadBalancer Services of this cluster, ignoring the updates that cannot change an endpoint, and give the informers a 5 minute resync as a safety net. Services of other tenant clusters sharing the namespace are not requeued, since reconcile would skip them. Also scope the all-VMIs fallback to the VMIs labelled with the name of the cluster being reconciled: an infra namespace can hold several tenant clusters and standalone VirtualMachines, whose VMIs do not serve the Service NodePort. The fallback widens to every VMI only when none of them carries the label, so that environments that do not label them keep their endpoints. Once any VMI is labelled, an empty selection means this cluster has no node right now, and widening it would publish the addresses of another cluster. The patch is named after what it does rather than after an upstream pull request, because none carries this change yet. Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
9b56e70ca6 |
feat(mongodb): support the system-bucket backup flow (useSystemBucket) (#3967)
## What this PR does MongoDB was the only `cozy-default` app with **no `useSystemBucket` flow**: its backup storage was always static on the `PerconaServerMongoDB` CR (bucket/endpoint/credentials from `backup.*` values plus a chart-emitted `<release>-s3-creds` Secret), so a tenant could not back up to the platform system bucket out of the box the way Postgres/ClickHouse/MariaDB/etcd do. This closes that gap so the default MongoDB backup-restore works OOTB. PBM takes `bucket`/`endpointUrl`/`region`/`prefix` as **static CR fields** (not secret refs), and the platform bucket name/endpoint are only known at BackupJob time — so, unlike ClickHouse's runtime `secretKeyRef` env, the chart cannot render them. The implementation mirrors the CNPG driver's pattern: - **Chart** (`packages/apps/mongodb`): a new `backup.useSystemBucket` flag keeps `spec.backup.enabled` (so the pbm agents run) but omits the static `storages`/`tasks`/`pitr` and the `<release>-s3-creds` Secret. Helm never owns `spec.backup.storages`, so a Flux re-render can't fight the driver. A `bootstrap.enabled + useSystemBucket` render guard mirrors Postgres, and `useSystemBucket: true` with `backup.enabled: false` is refused both at render time and by an `x-kubernetes-validations` rule on `backup` in the schema (the same mechanism as the `storageClass` immutability rule). Note on the schema rule: the aggregated apiserver does not evaluate CEL on application-backed resources yet (#2657), so today it is a UI/schema-level marker and the render-time refusal is what stops a Flux release; it also trips the API review gate (“validation rule added”), so this PR needs an API-owner approval on top of the code review. - **Strategy** (`cozy-default-mongodb`): now carries the S3 coordinates (`endpointUrl` already scheme'd by the `backupstrategy-controller.endpoint` helper, `credentialsSecret: cozy-backups-creds`, `prefix: <namespace>/<application>`). - **Driver** (`psmdbstrategy_controller.go`): SSA-injects `spec.backup.storages[s3-storage]` onto the live `PerconaServerMongoDB` at BackupJob time under a dedicated field manager (`cozystack-psmdb-backup-driver`) with `ForceOwnership`. Injection is **gated on the app's `backup.useSystemBucket` flag** (a legacy app leaves it false and is never touched; a useSystemBucket app is re-injected once per mint (once per BackupJob, plus once per storage-race re-mint), before its operator CR is created, so a later coordinate change catches up on the next job while a streaming dump is never patched under). The backup CR is minted only once the operator reports the storage pushed into pbm (`status.observedGeneration` caught up and `PBMReady` True): pbm writes to what its own config says, not to the spec, so a CR minted before that push would land at the previous coordinates while its Backup recorded the new ones; the job waits at `PerconaServerMongoDBPBMConfigPending` meanwhile, bounded by the backup deadline. Once a job has its CR, none of the mint-time reads or holds can fail it: the strategy and app reads the artifact still needs are bounded by the 24h running ceiling instead, and the cluster is not read at all **and on the release having rendered that flag**: the app CR is projected from the HelmRelease's desired values, so the flag reads true before helm-controller renders the revision, while the tenant's own storage, scheduled tasks and PITR stream from the previous render still sit on the cluster. As long as the cluster carries `spec.backup.tasks` or `spec.backup.pitr`, the driver does not inject and holds the BackupJob at `Ready=False PerconaServerMongoDBLegacyRender` (failing it at the deadline), so a wedged legacy revision never has its nightly dump and oplog redirected into the shared bucket. The backup snapshot falls back to the strategy coordinates when the operator status echo is empty; that fallback is gated on the CR's own `percona.com/delete-backup` finalizer (the flow the CR was minted on), not on the app flag re-read at completion, so a legacy backup never records the platform bucket and a flag flipped mid-dump does not change the artifact. The restore path rebuilds `backupSource` from it (the `cozy-backups-creds` reference is projected into the restore namespace before dispatch). - **Retention/cleanup**: on the `useSystemBucket` flow the driver owns the shared-bucket archive (the chart's psmdb `tasks[].keep` retention is gated out, and an on-demand backup is not covered by task retention). It stamps the operator's `percona.com/delete-backup` finalizer on each backup it mints **against a storage that carries the platform coordinates** — the strategy's bucket and `credentialsSecret`, read back from the live cluster after the injection — never on the flag alone: a strategy without an `s3` block, or a storage that points anywhere else, holds the job (`MongoDBStrategyHasNoS3` / `PerconaServerMongoDBStorageForeign`) instead of minting a finalized dump against a bucket the platform does not own. On Cozystack `Backup` deletion (by hand today; by platform retention once it lands, since a `Plan` only creates BackupJobs and never deletes a `Backup`) the driver deletes the operator `PerconaServerMongoDBBackup` CR so that finalizer frees the object — the finalizer's presence is the ownership marker cleanup reads back. That reading is unambiguous because cleanup resolves the CR by the name recorded in `driverMetadata` (the one the driver minted), not by scanning: the psmdb operator does stamp `percona.com/delete-backup` on its own `cron-*` task-minted backups, but those are never in scope. A legacy backup keeps the pre-existing hands-off contract, decided from the snapshot before any read of the operator CR. A terminating namespace or the `backups.cozystack.io/skip-artifact-cleanup: "true"` annotation strips the driver's finalizer and releases; a `Get` of the operator CR that fails with anything other than NotFound/NoMatchError still requeues, and the operator's own `release-lock` finalizer can hold the CR while the operator is down, so those two are what can keep a namespace in `Terminating`. `schedule` and `retentionPolicy` are inert on this flow (there is no engine-side retention and a `Plan` has no retention field yet); platform-level retention is tracked in [#4192](https://github.com/cozystack/cozystack/pull/4192) and teardown GC of orphaned backups in [#3966](https://github.com/cozystack/cozystack/issues/3966), so until those land bound `cozy-backups` with an S3 lifecycle policy. - **Bounds on the waits**: the injection race with the operator's cluster cache (an `unable to get storage` error on a freshly injected storage) is retried by deleting the errored CR and minting a fresh one, with the retry count kept on the BackupJob (`backups.cozystack.io/mongodb-storage-race-retries`) because the deleted CR is reaped before the next poll; after three retries the operator's error is terminal. A system-bucket dump that has started streaming is not cancelled on the 30-minute deadline, but fails after 24 hours in `running` (`BackupAbandoned` warning naming the operator CR, which is left in place as the handle to whatever pbm still writes). The deadline cancel of a not-yet-started CR pins its delete to the resourceVersion the live read answered for. - **RBAC**: `patch` on `perconaservermongodbs` for the injection (server-side apply), and `create`/`delete`/`patch` on `perconaservermongodbbackups` — `delete` to prune the shared-bucket archive on `Backup` deletion, `patch` to strip the driver's finalizer when releasing a wedged cleanup. Coverage: Go unit tests over `internal/backupcontroller` — the injection gate keyed off the app flag (legacy cluster never patched) and off the applied render (a cluster still carrying tasks or pitr is held, untouched), the SSA-injection precondition with the whole entry (per-application prefix included) asserted on the cluster, strategy templating per application, the finalizer refused against a storage without platform coordinates (strategy without `s3` over a legacy entry, foreign coordinates on the merged entry), the retry budget driven with the operator simulated reaping the deleted CR between polls, the restore snapshot fallback and credential swap across all four legacy/system flow combinations, the finalizer-owned cleanup with its escape hatches (skip annotation before any read, `NoMatchError` treated as gone, terminating-namespace release using the pre-read CR, a failed strip surfaced as an Event, a legacy CR never deleted, a legacy snapshot released without a read, a system-bucket snapshot reaching the prune), the deadline cancelling a not-yet-running operator backup only on a positive live read (including no live client at all, and a CR that moved since the read), and the running ceiling. Plus helm-unittest on both charts (chart skips the creds Secret and omits `spec.backup.storages`/`tasks`/`pitr` on `useSystemBucket` and keeps them on the legacy flow; the strategy renders the S3 coordinates and the `insecureSkipTLSVerify` knob). A live `useSystemBucket` backup+restore e2e against the platform bucket is a follow-up (needs the system bucket wired in CI, same status as the Postgres/ClickHouse useSystemBucket e2e). One item to confirm in e2e: the psmdb operator accepting `spec.backup.enabled=true` with an empty `storages` between install and the first BackupJob. ### Screenshots Not applicable (no UI changes). ### Downstream repositories The tenant-facing schema surface is one new field, `spec.backup.useSystemBucket`, on the MongoDB `values.schema.json` / CRD (regenerated), plus a validation rule on `spec.backup`. `terraform-provider-cozystack` hand-models MongoDB and may want to expose it, but it is a new optional boolean that defaults false (no drift for existing users), and the chart `README.md` regenerates on release. Leaving every box unticked per the trigger-map guidance so a maintainer decides on the provider follow-up. - [ ] 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: ### Release note ```release-note feat(mongodb): MongoDB backups now support the platform system bucket via backup.useSystemBucket, so the default cozy-default BackupClass works without per-app S3 credentials — the backup driver injects the system-bucket storage onto the cluster at backup time, once the release has rendered the flag. On this flow nothing prunes the shared bucket automatically yet (a Plan carries no retention field; tracked in #4192 and #3966), so an S3 lifecycle policy on cozy-backups is a prerequisite for enabling it, not a suggestion. useSystemBucket requires backup.enabled=true; the chart refuses the combination with backups disabled at render time, and the schema carries the same rule as a marker until the apiserver enforces CEL (#2657). A system-bucket dump still running after 24 hours fails its BackupJob with the operator backup left in place. A MongoDB restore now adopts the target cluster's S3 credentials only for a storage on the same bucket as the archive (it used to adopt them unconditionally); a legacy restore into a target on a different bucket that relied on one key pair covering both must keep or re-create the source release's -s3-creds Secret by name, and is held at Ready=False RestoreCredentialsMissing until it exists. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added opt-in system bucket mode for MongoDB backups. * Automatically injects platform bucket storage without application-managed S3 credentials. * Supports endpoint, region, path prefix, addressing, and TLS settings. * Manages shared-bucket backup artifacts when system bucket backups are deleted. * **Bug Fixes** * Improved restore credential selection to avoid cross-bucket mismatches. * Preserved backup metadata when storage details are unavailable. * **Documentation** * Documented configuration, scheduling, retention, PITR limitations, and restore guidance. * **Validation** * Prevented incompatible bootstrap and system bucket restore configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
e510d49b3d |
feat(monitoring): re-render node agents when a monitoring label changes
The node fluent-bit builds its tenant routes from a lookup of namespace labels, and helm-controller upgrades a release only when its chart or values digest changes. A tenant turning monitoring on therefore got no route, and a removed one kept its route, until an unrelated change upgraded monitoring-agents. The reconciler digests the label across all namespaces and requests a forced upgrade of the release when the digest moves. It digests every labelled namespace rather than repeating the chart's selection, so it cannot drift from the chart; the cost is an occasional upgrade that renders the same config. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
cdff5ed380 |
feat(monitoring): route tenant logs to the tenant's own VictoriaLogs
A tenant that runs its own monitoring stack gets a VictoriaLogs instance, but the node fluent-bit sends every container log to global.target, so that instance stays empty. The tenant chart already names the stack a namespace reports to in namespace.cozystack.io/monitoring, the label the per-tenant vmagent selects on, so the log path keys off the same label. The routed record leaves the default outputs (keep false): a tenant with its own monitoring holds its logs, the root store no longer does. The re-tagged record re-enters the filter chain, where the Match * "Add tenant" cannot overwrite the key, so a per-target Set labels it with the tenant it is sent to. One rewrite_tag carries every rule, keeping a single emitter however many tenants are routed. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
874590e200 |
test(keycloak): pin organizationsEnabled rendering in configure-kk
Cover both paths of the new value: the chart default renders spec.organizationsEnabled: false (no behaviour change), and setting organizationsEnabled: true renders through to the ClusterKeycloakRealm. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
mattia-eleuteri
|
adb2918daf |
feat(keycloak): expose organizationsEnabled realm toggle
The keycloak-configure chart hardcoded the realm with the Organizations feature off, with no value to enable it. Operators who flip organizationsEnabled in the Keycloak admin console see it reverted on the next ClusterKeycloakRealm reconcile, since the EDP operator pushes the chart-rendered spec back to the realm. Expose organizationsEnabled in values.yaml (default false, no behaviour change) and map it to ClusterKeycloakRealm.spec.organizationsEnabled so the feature can be enabled declaratively and survive reconciles. Assisted-by: LLM Signed-off-by: mattia-eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
f7c04b7a39 |
feat(kubernetes): expose kernel modules for Talos worker pools
A Talos system extension installs a kernel module but does not load it; loading is machine.kernel.modules' job. The worker machine config had no such block and no values key could add one, so a GPU pool on Talos workers was unusable as shipped: the extension supplied the NVIDIA module, nothing loaded it, and the failure was silent end to end. The VM held the PCI device, the node advertised no GPU, and no component logged an error. The field is three-state, and the states are distinguishable because it carries no default: absent lets the chart choose (a pool holding at least one nvidia.com/* GPU gets nvidia, nvidia_uvm, nvidia_drm, nvidia_modeset, in that order, since Talos loads the list in sequence and the last three depend on the first; any other pool gets nothing), a non-empty list replaces that choice outright, and an explicit [] opts out even on a GPU pool. A default of [] would collapse absent into opted-out for every pool and make the automatic set unreachable. The automatic nvidia entry carries NVreg_NvLinkDisable=1. Individual GPUs are passed into worker VMs without their NVSwitches, and on non-Talos workers the gpu-operator driver container applies that parameter; on Talos that container has to be off, so nothing else would. Names and parameters reach the reconcile Job's unquoted heredoc, so they are validated at render and the emitted list is rebuilt from those two fields alone: the schema does not prune unknown keys, and any other key passed through toYaml would reach the shell unchecked. A pool that resolves to no modules renders the machine config it rendered before, byte for byte, so its content-hash Job name does not rotate. Refs: #3563 Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
669fb2416a |
feat(kubernetes): default gpu-operator to an OS-provided driver on Talos
Closes the gpu-operator half of #3563. Tenant clusters have run Talos workers since 1.6.0, where the NVIDIA driver comes from a system extension baked into the worker image rather than from the operator's driver container. The addon default still described the Ubuntu shape, so a GPU node group came up with the operator fighting the OS for the driver and no path at all for CUDA libraries to reach a tenant pod. Three things change, each fixing one silent failure: driver.enabled and toolkit.enabled go false. The driver container would load its own module and clash with the extension's; the toolkit container would rewrite /etc/containerd/config.toml, which Talos owns and which already registers the nvidia runtime with enable_cdi = true and cdi_spec_dirs = ['/run/cdi']. validator.driver.env points DRIVER_ROOT and NVIDIA_DRIVER_ROOT at / and disables /dev/char symlink creation. The driver validator otherwise looks for a driver container install, finds none, never writes its sentinel, and leaves the device plugin blocked behind toolkit-validation — the loop on "Attempting to validate a driver container installation". devicePlugin.env selects cdi-annotations and points NVIDIA_CTK_PATH at the host's nvidia-ctk. Tenant pods carry no runtimeClassName and did not need one on Ubuntu, where the toolkit made nvidia the default containerd runtime; Talos does not, so CDI is the only route for the CUDA libraries. DEVICE_ID_STRATEGY, NVIDIA_DRIVER_ROOT and CDI_ANNOTATION_PREFIX are set and tested alongside it: CDI device names key on the UUID strategy, the plugin resolves the driver from NVIDIA_DRIVER_ROOT, and the annotation prefix has to be the one containerd reads, so a regression in any of them breaks device resolution on its own. The previous default fed NVreg_NvLinkDisable=1 to the driver container through the nvidia-kernel-module-params ConfigMap. With that container off the ConfigMap is mounted by nothing, so the default is removed. On Talos a module parameter belongs to the worker machine config, which the kernelModules field of the KubernetesNodes app (packages/apps/kubernetes-nodes, #3571) writes. The wrapper chart keeps the ConfigMap for an operator who re-enables the driver container. Every value here is the configuration validated on a production Talos GPU node group, minus the parts that only mattered while the driver container was in play. Not covered: HAMi. When addons.hami.enabled the operator's device plugin stays off and HAMi serves the GPUs, and HAMi's own plugin needs equivalent CDI wiring on Talos that this default does not attempt. Refs: #3563 Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
bb3ddee233 |
fix(rabbitmq): bound the topology operator Secret cache
messaging-topology-operator 1.14.2 caches every Secret in the cluster: its ClusterRole grants secrets list/watch cluster-wide with no namespace or label restriction, so the controller-runtime cache holds the whole set. Against the vendored 500Mi limit this OOMKills on any cluster that has grown enough Secrets, and it never recovers, because the initial LIST alone no longer fits. While it is down no topology CR reconciles, for any tenant. Observed on a cluster with 8211 Secrets across 291 namespaces (293 MB, 87% of it Helm release history): a flat 333Mi working set for days, then the ceiling, then a permanent crashloop. The operator was caching 8211 Secrets to read 2. Upstream fixed this by gating the Secret informer on a label, released in v1.20.0, and by bypassing the cache for Service reads, released in v1.20.1. Refresh the vendored manifest to v1.20.2, the current patch release carrying both, through the package's existing make update pipeline. The bump alone would break every install: the app chart's credentials Secret is referenced by User.spec.importCredentialsSecret, and from 1.20 the admission webhook rejects a user-provided Secret that does not carry rabbitmq.com/topology-operator=true. Label it in the same change, pin the label in the chart's unit tests, and give the e2e RabbitMQ a user so the webhook and the User reconcile are exercised against the real operator. The RabbitmqCluster default-user Secret needs no label, it is read uncached via apiReader, and the Status.Binding contract is unchanged since 1.14.2, so the vendored cluster-operator 2.9.0 stays compatible and does not move here. Refs #4128 Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-by: LLM |
||
|
Mattia Eleuteri
|
67bf9ca317 |
fix(postgres): read the live Cluster when deciding the WAL-archive prefix
applyClusterPluginBackup preserves the serverName already on the Cluster, so
that a backup does not flip the archive prefix mid-stream. It read that value
through the manager's cached client, and this controller registers
For(&BackupJob{}) and nothing else, so a Cluster only reaches the cache
through a lazily started informer with no resync tied to its own changes.
An in-place RestoreJob deletes the Cluster and the chart re-renders it with
bootstrap.newServerName seconds later. A read served during that window
describes the object that was purged, so the source's serverName is written
back over the freshly rendered one and the restored cluster archives onto the
source's WAL prefix. barman-cloud-check-wal-archive then refuses to start with
"Expected empty archive" and the RestoreJob ends on its deadline -- the
failure newServerName was added to prevent.
The stale read carries a stale UID too, and that UID owns the ObjectStore this
function applies, so the store is garbage-collected as soon as it is written.
Reading through the dynamic client keeps the preservation rule and makes the
value it rests on truthful.
Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
|
||
|
Mattia Eleuteri
|
7c9ab0e28e |
fix(cozy-lib): round the sanitized CPU request up to a whole milli-CPU
cozy-lib.resources.sanitize divides the CPU limit by cpuAllocationRatio and rendered the quotient as a bare float. When the ratio does not divide the value evenly the result is finer than a milli-CPU, and the VPA admission webhook rejects it outright: admission webhook "vpa.k8s.io" denied the request: minAllowed: CPU [62500u] must be a whole number of milli CPUs packages/extra/etcd routes its VPA minAllowed/maxAllowed through the helper, so 250m / 4 = 62.5m left the etcd HelmRelease looping UpgradeFailed -> RollbackFailed forever: the rollback target renders the same invalid value. Since v1.6.0 HelmReleases wait on each other, so that one stuck app also blocks tenant-root, cozystack-basics and kubevirt-cdi-operator, and without CDI the Talos worker rollover cannot import the worker OS image. Only the divisors of 250 worked as ratios, which is an artefact of one chart's hardcoded floor rather than a real constraint on the operator. The request is rounded up, never down, so it stays at or above the intended fraction of the limit. It is rounded only when it has to be: if the milli-CPU value equals its own ceiling, the float keeps its current rendering, so nothing changes on the default ratio of 10 or on any ratio that divides evenly; otherwise it renders as the ceiling in m. The comparison is exact, with no tolerance. sprig's divf and mulf run on shopspring/decimal rather than float64, so mulf (divf 0.25 10.0) 1000.0 renders exactly 25 and there is no binary-float noise on this codepath to absorb. A tolerance would reintroduce the bug: a value whole-milli only to within it, such as 100.0000004m at ratio 1, would read as exact and render "0.1000000004", rejected for the same reason 62.5m is. The same predicate covers sub-milli input, since ceil carries any positive request to at least 1m, and a zero request stays "0". Fixes #2917 Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
25cdd51881 |
feat(postgres): allow preloading an allowlist of shipped modules
shared_preload_libraries is blocked in postgresql.parameters, and rightly so: the parameter is free text and accepts a path, so it can point at a writable location such as the PGDATA volume. What the block leaves is an indirect route. CloudNativePG preloads pg_stat_statements, auto_explain and pgaudit by itself as soon as a parameter under their prefix is set, such as pg_stat_statements.track, so enabling one of them today means setting one of its tuning parameters, even to its default value. pg_prewarm has no route at all, and its autoprewarm worker only runs when the module is preloaded. postgresql.sharedPreloadLibraries is the explicit switch. It is a closed enum of four module names that already ship in the CloudNativePG PostgreSQL image, rendered onto the CNPG Cluster field of the same meaning, which the operator merges with the libraries it preloads on its own. A bare name can only resolve inside $libdir, which sits on the read-only image filesystem, and dynamic_library_path stays blocked, so no path can be introduced. The blocklist on postgresql.parameters is left untouched. The template repeats the enum as a backstop for renders that skip schema validation. helm-unittest always validates the schema and never reaches it, so a bats suite renders with --skip-schema-validation to keep the backstop, and its agreement with the enum, under test. Verified on a live CloudNativePG 1.27.3 cluster: with the field set, the operator writes shared_preload_libraries into postgresql.conf, SHOW reports the modules, and pg_stat_statements returns rows. Changing the list restarts PostgreSQL on every instance, the primary included, through the instance manager and without restarting the pod. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |