mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
2fa7fea29b8b9562a2c9e42d2e53ca9fa72fb01d
1257
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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
|
||
|
|
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
|
||
|
|
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
|
||
|
|
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 --> |
||
|
|
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 --> |
||
|
|
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 --> |
||
|
|
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> |
||
|
|
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 |
||
|
|
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> |
||
|
|
afe23a2c3f |
ci(release): pin flux where the release pushes the packages artifact
prepare-release now runs flux push artifact to publish the rc's packages artifact, and it installed whatever flux was latest, while build-amd64 and the candidate checks pin one version. It installs that version now. The arm64 leg and the nightly never run flux, so they no longer install it. The rc-e2e comment gave a stable tag as the reason for its !cancelled(), but the job only runs for an rc; it now says it follows the rule every job below build-arm64 has. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
71144d4fd6 |
ci(release): pin the matchbox rebuild in the release job
tags.yaml only runs on tags, so no pull request exercises it. The matchbox rebuild is the only thing that makes its ref an index, and losing the step, moving it past the tree capture or letting it switch the default builder would surface as a failed gate on the next rc. The comments that called the talos images amd64 only now say they are built for both arches in the amd64 job. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
6ef23da6e7 |
ci(release): publish matchbox as an amd64 and arm64 index
matchbox already carries the kernel and initramfs of both arches, but the release built it for amd64 only: the arm64 leg skips the talos package, so there is no arm64 half to stitch, and the gate let it through on an allowlist entry. Its Dockerfile only copies files, so the amd64 job now rebuilds it for both platforms on a docker-container builder and the stamped ref pins the index. The allowlist entry goes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c740031453 |
Merge remote-tracking branch 'origin/main' into ci/build-arm64-leg
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
5e32ef35b0 |
feat(build): build Talos, Harbor, the Velero KubeVirt plugin and the unlisted images for arm64 (#4552)
## What this PR does This brings together four approved PRs that give the remaining first-party images an arm64 build. Each of them lost its approval when #4498 merged and GitHub retargeted it to main, so I merged them into one branch to review and approve once. Each original PR has the full description and review history, and I'll close them in favour of this one. - #4507 builds the Talos installer and matchbox for arm64. Releases also publish the arm64 ISO, disk images, kernel and initramfs. - #4515 builds the KubeVirt Velero plugin in-tree for amd64 and arm64. - #4525 adds flux-plunger, keycloak-operator, kilo and migration-controller to the root `build:` list, so CI rebuilds them like every other image. A test keeps any new image target from being left out. - #4528 rebuilds the Harbor v2.15.1 images for amd64 and arm64, because upstream publishes v2.15 for amd64 only. Release builds still run with `PLATFORM=linux/amd64`, except the Talos installer, so these images ship arm64 only once #4505 adds the arm64 leg. This PR makes that leg buildable. A few commits are new on top of the originals. The velero test no longer pins the plugin's image string, because the release build stamps its own rebuild over it. It now checks the plugin init container by repository name. A promote-retag test comment now says what it guards, and the image-refs doc no longer calls its fixed gaps open. The merge with main keeps the Harbor portal in the list of stages whose output has no architecture. ### Screenshots Not a UI change. ### Downstream repositories I left every box empty and opened nothing in other repositories. Two repositories are touched, and neither follow-up is needed before this merges: - cozystack/website names the amd64 assets in `content/en/docs/next/install/talos/iso.md` and `install/providers/oracle-cloud.md`. The install pages could offer the arm64 downloads once a release carries them. No asset is renamed, so the current links keep working. - cozystack/talm points at `ghcr.io/cozystack/cozystack/talos:<version>` in `charts/cozystack/values.yaml`. Once a release publishes that tag as a two-arch index, the same reference works on arm64 nodes, so nothing there has to 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(talos): the Talos installer image is built for amd64 and arm64, with arm64 profiles that carry drbd and zfs, and the bootbox matchbox image network-boots arm64 machines as well as amd64 ones. Releases publish the arm64 Talos ISO, disk images, kernel and initramfs. feat(velero): the KubeVirt Velero plugin is now built in-tree for amd64 and arm64, so the velero server starts on arm64 clusters. build: flux-plunger, keycloak-operator, kilo and migration-controller are now rebuilt by CI like every other first-party image, so their pinned images pick up source patches and base-image updates and are published for amd64 and arm64. feat(harbor): the Harbor application now runs on arm64 nodes. Its component images are rebuilt from the pinned upstream v2.15.1 release for amd64 and arm64, because upstream publishes v2.15 images for amd64 only. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Talos images and boot assets now support both AMD64 and ARM64, including architecture-specific installation and cloud images. * Added build support for Harbor components and the KubeVirt Velero plugin. * Nightly builds now include ARM64 release assets. * **Improvements** * Harbor uses pinned chart and image versions, with built image references recorded automatically. * Added checks to catch image packages missing from the main build and to verify multi-architecture installer output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
65b02bf5ac |
fix(ci): keep the controller Go tests running when a unit test fails first
The `checks` job runs `make unit-tests test-controllers -j4` as one step. Without -k, GNU Make stops scheduling new work as soon as any prerequisite fails, so when a BATS file in unit-tests fails first, the independent test-controllers goal never starts and the PR page shows no signal about the Go controller suite at all. With -k make keeps running every goal it still can, and the job stays red on any failure. Report: https://github.com/cozystack/cozystack/issues/3776 Assisted-by: LLM Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> |
||
|
|
8d547be7fe |
fix(tests): reject unreadable RD preset schemas (#4191)
## What this PR does `rd-presets-check` discards `jq` errors, so malformed schema can pass the check or appear as missing presets. It now fails with the schema path and `jq` diagnostic. An absent schema or a readable schema without `resourcesPreset` is still skipped. Regression tests use real `jq` for malformed input and partial output. They also cover the canonical preset set, exact matching and the long-list case fixed by [#4204](https://github.com/cozystack/cozystack/pull/4204). ### Testing - `make unit-tests -j4 --output-sync=target` passed. - Nine cases in `hack/check-rd-presets_test.bats`, run with `bats` and `dash hack/cozytest.sh`. - Both read-failure cases fail against `main`. Eleven deliberate code mutations also fail the expected test. - `shellcheck hack/check-rd-presets.sh hack/check-rd-presets_test.bats`. ### Screenshots Not applicable. ### Downstream repositories - [x] No downstream repository is affected by this change No files or make targets were moved or renamed. The change is limited to the RD checker and its tests. ### Release note ```release-note fix(tests): Fail the RD preset check when a schema cannot be read, with the schema path and jq diagnostic. ``` |
||
|
|
029707f0b2 |
test(hack): say what the promote-retag ownership check guards
The comment above the ownership loop described how both images used to be dropped. Describe the reason the check matters instead: neither image has a release tag to fall back on, so a miss would go unnoticed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
02461acdde |
Merge remote-tracking branch 'origin/main' into feat/arm64-bundle
Signed-off-by: Aleksei Sviridkin <f@lex.la> # Conflicts: # hack/image-builders-cross-compile.bats |
||
|
|
4c945dbc40 |
fix(build): let a builder stage inherit its base stage's platform
The cross-compile check read only the literal FROM line, so a stage built FROM an earlier --platform=$BUILDPLATFORM stage counted as emulated even though it runs natively. The shadowbox webpack stage is such a stage, and it failed the check on main. The check now follows FROM <stage> to its base and inherits its platform. ARCH_NEUTRAL also accepts "<file>:<stage>", so the webpack stage can be marked as producing no architecture without excusing the Go stage in the same Dockerfile. A lookup failure now fails the stage instead of passing it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
8c1266649d |
Merge remote-tracking branch 'origin/feat/harbor-multiarch' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
79854b16fb |
Merge remote-tracking branch 'origin/fix/build-list-orphan-images' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
2f08832392 |
Merge remote-tracking branch 'origin/feat/talos-arm64' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
6d759cc73e |
feat(build): build package images for amd64 and arm64 (#4498)
## What this PR does
Package images can now be built for amd64 and arm64 in one `make image`,
and a published build is multi-arch unless `PLATFORM` says otherwise.
This is the build half of arm64 support from #1961 and #903. CI still
publishes amd64 only until it gets a native arm64 leg, which comes in a
separate PR.
Builder stages now run on the build platform and compile for the target
one, so Go never runs under emulation. Under emulation it is slow and it
crashes: the migration-controller Dockerfile already records a "bad
pointer in Go heap" from exactly that. On top of this, `PLATFORM`
defaults to `linux/amd64,linux/arm64`. A build on an arm64 workstation
can't publish an arm64-only digest any more. That happened once already,
the cilium pin was arm64-only until
|
||
|
|
2e8219fb92 |
fix(build): pass the shared buildx flags from every image target (#4485)
## What this PR does
Five image builds skipped the shared buildx flags in
`hack/common-envs.mk` and wrote their own: flux-plunger,
kubeovn-plunger, both dashboard images, and metallb. The plungers
ignored `PLATFORM`, so on an arm64 workstation they came out as arm64
images that fail on amd64 nodes with `exec format error`. CI builds on
amd64, so it never showed. Dashboard and metallb hardcoded
`--platform=linux/amd64`; for metallb that sat next to `$(BUILDX_ARGS)`,
so setting `PLATFORM` gave buildx two platforms. All of them now build
through `$(BUILDX_ARGS)` only.
How it split: kubeovn-plunger landed two days before
|
||
|
|
ba1e63c665 |
ci(pre-commit): detect schema drift across all packages (#4497)
## What this PR does Extends the `run-make-generate` pre-commit hook to discover package Makefiles with an actual `generate` target across every package category. This preserves existing app and extra coverage while adding system and library packages such as `talos-log-collector`, `kubevirt-cdi`, and `cozy-lib`. A focused Bats test verifies category coverage, skips packages without a generator, excludes nested vendored Makefiles, and confirms generator failures are propagated. The newly covered `cozy-lib` package exposed deterministic schema formatting drift, which is included in this change. Fixes #4416 ### Screenshots N/A — no UI changes. ### Downstream repositories The developer-tooling trigger indicates that the website development documentation should be reviewed. No downstream follow-up was published as part of this PR. - [ ] 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 * **Chores** * Pre-commit package generation now checks eligible package directories across more categories, running generation where a matching target is available. If generation fails, subsequent packages are not processed. * **Tests** * Added checks covering package discovery across categories, exclusion of ineligible directories, and failure handling that prevents later packages from being processed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
47fe04673a |
fix(tests): reject unreadable RD preset schemas
Discarding jq's exit status lets malformed schemas pass the preset check or turns a partial read into a misleading missing-preset report. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
29932ddbb0 |
test(tests): add a render smoke check over every app chart (#4018)
## What this PR does I added an offline render check for application charts because helm-unittest suites can leave templates outside their `templates:` lists unchecked. It runs in `make unit-tests` with fixtures for fresh, configured and wildcard certificate states. The fixtures cover injected cluster and namespace values, with checks for PostgreSQL scheduling, tenant OIDC groups, shared services and Harbor certificate settings. The two charts that require instance-type lookup are skipped only while every fixture fails with the expected lookup error. I also added ClickHouse tests for generating the backup API password and preserving an existing one. The render check uses empty lookup results and checks YAML rendering only. Closes https://github.com/cozystack/cozystack/issues/4057 ### Validation I ran `make unit-tests` and `make test-controllers`. All 16 render-matrix tests passed under bats and the shell runner with dash. Deliberately broken rendering, fixture validation and password generation or reuse made the tests fail. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added automated validation that charts render successfully across fresh, configured, and wildcard cluster states, including checks for conditional resources and scheduling settings. * Added coverage confirming ClickHouse backup API passwords are retained when a Secret already exists. * Expanded checks for invalid fixtures, skipped charts, and consistent rendered output. * **Documentation** * Documented the render scenarios and how fixture values represent cluster configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
11991b9d05 |
feat(harbor): build the Harbor images for amd64 and arm64
Upstream publishes the Harbor v2.15 component images for amd64 only, so every Harbor pod crash-loops with "exec format error" on an arm64 node. The multi-arch build upstream exists on main but is in no release yet, so the package now rebuilds the pinned v2.15.1 release itself: the Go components cross-compile on the build platform, the registry and the Trivy adapter build from the tags upstream's Makefile pins, the Trivy binary is fetched per architecture against its release checksum, and the runtime images keep upstream's photon base, users and entrypoints. The build fails when those registry and Trivy pins, the swagger generator or the golang and node build images no longer match upstream's Makefile at the pinned release, so a version bump cannot pair a new core with an old registry, scanner or toolchain. The portal compiles to static JS, so it lives in its own Dockerfile listed as architecture-neutral in the cross-compile check, and takes the release pin from the main one so both files build the same commit. Because values.yaml now pins the images, `make update` pins the chart release that ships the same Harbor version and fails when its appVersion differs, instead of pulling the newest chart onto old images. Renovate stops proposing tag bumps for the rebuild's build bases, which follow upstream's toolchain for that release. The committed refs stay on the upstream digests until CI stamps the rebuilt ones. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
82aa8a048d |
ci(release): fail an rc on any single-arch image its e2e pulled
The release gate reads only refs the tree pins by digest, so an image a vendored chart renders from its defaults, or one an operator launches, can still be amd64 only. The rc e2e now reads what the three Talos nodes actually pulled from their image stores, each by the digest the pull resolved to, and runs verify-multiarch on that list before the sandbox is torn down. The image store, not the running pods, because the tests delete what they create. collect-images.sh writes that list only when every node answered, so a node that could not be read fails the audit instead of passing the images only it held. A pre-release fails on the audit; a stable tag's run only warns, as it ships its rc's images unchanged. The audit runs in the tree of the tag under test, so a tag cut before it existed has neither the script nor the image list. That run warns and skips instead of failing, so such an rc can still be re-validated from main; a tree that has the script and no list still fails. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
e8d4f757e2 |
ci: build main for arm64 nightly and report its single-arch refs
The arm64 images are built only for a pre-release, so a change that breaks one would first surface as a failed rc. A daily build of main on the arm64 pool surfaces it the next morning instead, and is the one writer of the arm64 build cache that the rc leg only reads. Before building, it prints hack/verify-multiarch.sh's report on main's tree, the list of refs the next rc's gate would fail on unless the stitch turns them into indexes. Nothing consumes its images and it is not a required check. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
67fa638f2a |
ci(release): publish every pre-release image as an amd64 and arm64 index
A release was amd64 only: prepare-release built on an x86 runner and nothing built arm64. A pre-release now builds its images twice, side by side: build-amd64 on x86, build-arm64 on a native arm64 runner under <tag>-arm64, reading the arm64 build cache. Neither waits for the other, so the rc takes as long as the slower build rather than the sum of both. prepare-release runs once both have succeeded. It applies the tree build-amd64 stamped, stitches each image and its arm64 twin into one index under the tags build-amd64 logged, gates on every pinned image being multi-arch, republishes the packages artifact and the installer chart pinned on the index digests, and only then commits, creates and publishes the release and pushes the staging branch. The stamped tree, the tag log and the assets reach it as artifacts that must exist, so a lost hand-off fails loudly. The assets are built in build-amd64, reusing the Talos images make build produced, while the arm64 build is still running. An rc whose arm64 or amd64 build fails is not published at all rather than published amd64 only. A stable tag skips the arm64 build: it never builds, and promotion copies the rc's indexes with every platform. build-amd64 keeps the release-exists no-op and the rejection of a hand-pushed stable tag, and prepare-release no-ops with it. A skipped ancestor makes the implicit success() false for every job below it, so each job below prepare-release states !cancelled() itself; without it the stable-tag changelog backstop would never run. The arm64 build makes the same no-op when the tag's release already exists, so re-running a prepared rc rebuilds nothing. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
144d080342 |
feat(build): gate a release on every pinned image being multi-arch
A release pins one ref per image for clusters of either architecture. A ref that resolves to a single-platform manifest, or to an index missing amd64 or arm64, installs on one and fails to pull on the other, and nothing before an arm64 install would notice. hack/verify-multiarch.sh resolves every ref hack/lib/image-refs.sh enumerates, third-party ones included since an arm64 node pulls them too, and fails naming each ref that is not an index carrying both linux/amd64 and linux/arm64. A ref that cannot be resolved fails as well, rather than passing unchecked. A rate limit or a 5xx is the one answer retried, a bounded number of times, and a ref still refused fails as rate-limited rather than as single-arch. --report prints the same list without failing, for a tree that is not being released. Images that cannot be built for both architectures go in hack/multiarch-allowlist by repository, each with a reason, so a bump needs no edit; they are still listed, and an entry that matches no single-arch ref is named as stale. --refs-file checks a list of refs instead of a tree, a tag-only ref resolved by its tag, with the same allowlist and output. It is for images the tree does not pin by digest, such as the ones an e2e run's nodes actually pulled. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7753394bfe |
build: rebuild every package that has an image target
flux-plunger, keycloak-operator, kilo and migration-controller have image targets but were missing from the root build list, which the build matrix, the main build and release preparation all iterate. No CI path rebuilt them, so their pins stayed on images pushed by hand while Dockerfile, patch and base-image changes never shipped, and the multi-arch pre-release build would find single-arch pins it cannot replace. migration-controller shipped a bare :latest with no digest. Add the four packages, and a test that derives the set from the package Makefiles so a new image target cannot be left out again. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
f02963a2c7 |
fix(keycloak-operator): keep the image host inside repository
The image target stamped only `tag`, leaving the committed `registry: ghcr.io` in place, so a build against the CI registry would render the public host with a digest that exists only in the build registry. Stamping the build host into `registry` instead would split the source registry across two keys, where the nightly mirror's substring host rewrite cannot reach it and the published tree would keep pointing at the private registry. Set `registry: ""` and keep the whole host in `repository`, stamped by the image target. The upstream chart joins the two keys, so the rendered image string is unchanged, and the ref now has the same shape as every other stamped reference the mirror and promotion already handle. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7280e2d7cc |
ci(pre-commit): detect schema drift across all packages
Restricting schema regeneration to application and extra packages left system and library metadata outside the pre-commit drift check. The regression tests must exercise the repository unit-test path; bats-core lifecycle hooks and result helpers are unavailable in that runner and prevent the assertions from executing. Fixes cozystack/cozystack#4416 Assisted-by: LLM Signed-off-by: frolov.so <opsmon.sh@yandex.com> |
||
|
|
769d3b3897 |
fix(build): push the kamaji provider image under the build's IMAGE_TAG
The capi-providers-cpprovider Makefile assigned IMAGE_TAG itself, which beats the value CI puts in the environment. Every PR, main and release line build therefore pushed the same component-version tag and overwrote each other's, and its second tag was built from an undefined TAG variable and ended in a bare "-". The image now goes through image-tags like every other package: the build-unique IMAGE_TAG always, the component version only under PUBLISH_VERSIONED=1. The stamped refs keep the component version as their tag, so the promote, retag and mirror tooling see the same refs as before. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
9a7094427e |
feat(build): stitch amd64 and arm64 builds into multi-arch indexes
The arm64 images are built on native arm64 runners, apart from the amd64 build, so each build pushes a single-platform image. The stitch joins every first-party image with its -arm64 twin into one index, moves every tag this build pushed for it to that index, and repins the tree on the index digest, so one ref serves both architectures. Most refs in the tree name only the build's IMAGE_TAG, while under PUBLISH_VERSIONED=1 the build also pushes the component version, so the refs alone cannot say which tags to move. The image-tags macro therefore appends every tag it pushes to PUSHED_TAGS_LOG when that is set, and the stitch moves exactly those, plus IMAGE_TAG and the tag a ref carries. It reads no other tag: another build's tag that sits on the same amd64 bytes is not this build's to move, and scanning every tag of every repository would cost thousands of registry reads per release. It reads refs through hack/lib/image-refs.sh like every other consumer, rewrites only the files that enumeration names, and then searches the whole tree outside charts/, gzip payloads included, for any old digest it re-pointed. A ref whose digest is also pinned where the rewrite cannot reach is skipped instead of half rewritten. The kamaji control-plane provider's chart ships its ref only inside files/components.gz, built from a plain copy with gzip -n, which no text search sees. The stitch rewrites both copies, recompressing with gzip -n so the result is what the package's image target would have written, rather than leaving that image amd64 only. An image with no arm64 twin (talos, the e2e sandbox), or a pair that is not one amd64 and one arm64 image, is skipped and named. A registry read answered with a rate limit or a 5xx is retried a bounded number of times; any other registry error fails the run at once rather than reading as a missing arm64 image. On a rerun after a stitch that failed partway, the build tag can already sit on an index of the pinned image and an arm64 image, not necessarily this run's if the arm64 leg rebuilt it. The stitch repins on that index instead of skipping the image, and copies it to any of the build's other tags still left on the amd64 image. A tag moved to anything else fails the run, since the tree then no longer matches the registry. MATRIX_ARCH=arm64 makes hack/build-matrix.sh leave out the e2e sandbox, which pins linux/amd64: an arm64 leg building it would push a second amd64 image under its -arm64 tag. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
9872b86772 |
feat(build): keep an arm64 build off the amd64 build cache
A native arm64 leg builds the same images next to the amd64 build and must not overwrite the cache that build owns. CACHE_TAG moves the default cache ref, so the leg can write buildcache-arm64 without every write evicting the amd64 cache layers, and without a change to each package Makefile. An explicit per-call cache tag still wins. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
fd8839aa61 |
fix(ci): verify nightly mirror copies against the raw manifest digest
Without --raw, skopeo inspect resolves a multi-arch index to the child manifest for the host platform and reports that digest. The tree pins the index digest, so once main publishes indexes every mirrored image would fail its own post-copy verification. Hash the raw manifest instead, which is the digest the tree pins for an index and a single manifest alike. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
1bc7effadb |
fix(ci): let the fork e2e lane check out the pull request again
actions/checkout v4.4.0 refuses to check out a fork's pull request ref from a workflow_run workflow unless the step opts in. The Renovate roll-up that moved e2e-fork.yaml to v4.4.0 made the publish job fail at checkout for every fork PR, so the required "E2E Tests" status now goes red on all of them regardless of the change. The lane was built to take fork code this way: publish runs no fork-authored script and e2e holds no push token. Opting in restores the behaviour it had on v4.3.1, and a unit test fails if either pull-request checkout loses the opt-in. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
6bb563f123 |
feat(release): publish the arm64 Talos assets
arm64 users can only take the Talos ISO, disk images, kernel and initramfs from the Image Factory today, because a release carries the amd64 ones only. The tag build now builds the assets for both arches and uploads the arm64 set next to the amd64 one under the same naming. It also pushes the talos installer image as a two-arch index, because a node installed from the arm64 assets pulls its installer from that image. The rest of the build stays amd64. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
|
8b20260105 |
feat(talos): build the installer and matchbox for arm64
The package pinned linux/amd64 because its assets were amd64 only. With arm64 profiles it follows PLATFORM like every other image. The installer is published as one index under every tag. skopeo cannot assemble an index from the imager's docker archives, so the archives go into one OCI layout and a small script writes the index there for skopeo to push. buildx could build it from OCI layout sources, but the release job builds on the classic docker driver, which has no such source. matchbox carries the kernel and initramfs of both arches whatever PLATFORM is, rather than those of TARGETARCH: the node matchbox runs on is not the arch of the machines it boots, so a per-arch image would hand an amd64 machine an arm64 kernel whenever the pod lands on an arm64 node. The amd64 assets stay at their old paths, and a group keyed on the arch iPXE reports sends arm64 machines to the arm64 ones. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
|
084ac47237 |
feat(build): build published images for amd64 and arm64 by default
PLATFORM defaulted to empty, so buildx built for the host alone. On an arm64 workstation `make image` published an arm64-only digest, which amd64 nodes then fail to pull; the cilium pin shipped exactly that until it was rebuilt by hand. A build now targets linux/amd64 and linux/arm64 unless PLATFORM is set, and the digest a package stamps is the index digest. LOAD=1 keeps building for the host only. The classic docker image store cannot load a multi-platform index, and loading is the documented local workflow. This changes local pushes: a `make image` on the default docker driver without the containerd image store now fails with "Multi-platform build is not supported for the docker driver", where it used to build for the host. Such a build needs a docker-container builder (BUILDER), or LOAD=1 or an explicit PLATFORM for a single architecture. talos and testing pin linux/amd64 with `override`, placed before the include because common-envs.mk expands BUILDX_ARGS immediately. The Talos assets are amd64: matchbox bakes the kernel and initramfs into its image, and the e2e sandbox boots the disk under qemu-system-x86_64. An arm64 variant of either would still serve or boot amd64 files. CI pins linux/amd64 on every image build. Nothing there sets up QEMU, fork builds export a single-platform archive, and tags.yaml builds on the default docker driver, so the default would break or emulate those jobs. Native arm64 builds belong in a leg of their own. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
d9cbbec5ff |
fix(build): cross-compile image builders on the build platform
The remaining Go builder stages, plus the kube-ovn, console and piraeus-server builders, ran on the target platform. A multi-platform build therefore ran the whole compiler under QEMU: slow, and for Go unreliable, as the migration-controller Dockerfile already records a "found bad pointer in Go heap" crash from exactly that. The builders now run on $BUILDPLATFORM and produce the target architecture from TARGETARCH. Two builders only produced the right architecture because they were emulated. token-proxy ran `go build` with no GOOS/GOARCH, and kube-ovn calls upstream `make build-go`, which hardcodes GOARCH=amd64 even on an arm64 build. On a native builder both would have shipped host binaries inside the other architecture's image, and nothing fails until a node of that architecture starts the container. token-proxy now passes the target explicitly; kube-ovn picks build-go or its arm64 twin build-go-arm from TARGETARCH. The console builder emits static JS and linstor-server's debian/control declares every package Architecture: all, so both run natively without a target. A bats test requires every stage that compiles to sit on a BUILDPLATFORM FROM and to read TARGETARCH or TARGETPLATFORM on a RUN or ENV line. A bare ARG declaration does not count, which is what let token-proxy through. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
623113f920 |
fix(build): pass the shared buildx flags from every image target
Three package Makefiles spelled out their docker buildx flags instead of using $(BUILDX_ARGS) from hack/common-envs.mk: flux-plunger, kubeovn-plunger and both dashboard images. kubeovn-plunger landed just before the change that moved the other Makefiles onto BUILDX_ARGS, flux-plunger was modelled on it, and dashboard kept its own flags when that change touched it. Those targets ignored PLATFORM, SBOM and BUILDX_EXTRA_ARGS. On an arm64 workstation the plungers came out as arm64 images, which fail on amd64 nodes with "exec format error"; CI builds on amd64 and never saw it. Dashboard and metallb hardcoded --platform=linux/amd64 instead; for metallb that sat next to $(BUILDX_ARGS), so setting PLATFORM gave buildx two platforms and a multi-platform build. All of them now build through $(BUILDX_ARGS) alone. CI sets only BUILDER, which these targets already honoured, and runs on amd64, so its builds are unchanged. Local builds of dashboard and metallb now follow the host architecture like every other package: on an arm64 machine build them for an amd64 cluster with PLATFORM=linux/amd64. A bats test dry-runs every package image target with a probe platform and a marker in BUILDX_EXTRA_ARGS, and requires each buildx call to carry the marker, so $(BUILDX_ARGS) is really passed, and that platform and no other. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7002397e40 |
test(kafka): assert that uninstall leaves no node pool or broker behind
The Kafka suites passed while the node pools of the apps they had deleted, and the brokers those pools own, kept running into the suites that came after. Each suite now checks, after its apps are gone, that no KafkaNodePool and no pod labelled strimzi.io/cluster=<cluster> remains in the namespace. The kafka suite deletes its app explicitly so it can look afterwards. The metadata roundtrip's cleanup does not wait for the uninstalls, so its checks carry a budget sized from the pre-delete Job's deadline. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c64fec66ff |
fix(kafka): delete the release's node pools on uninstall
The migration hook creates the KafkaNodePools with kubectl apply, so they are not in the release manifest and helm uninstall leaves them. Each pool owns the StrimziPodSet that runs its brokers, so after the application is deleted every broker keeps running, and keeps its memory request, with nothing left to remove it. The pre-delete hook now deletes the pools selected by strimzi.io/cluster=<release> once the KafkaTopic delete has finished, including when that wait timed out: the topic operator clears topic finalizers through the brokers, so the brokers have to outlive it. The call does not wait, so the Job's deadline arithmetic is unchanged. An absent CRD is not a failure. Any other failure exits non-zero: once the uninstall completes Helm never runs the hook again, so a retry is the chart's last chance to remove the pools. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
4a2dea2d08 |
feat(cluster-api): add the Proxmox infrastructure provider and in-cluster IPAM as optional packages (#4087)
## What this PR does **Series behind #3481** — three separable pull requests: 1. **#4087 — 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 #4087 and #4088 share no files and can be reviewed in parallel. #4089 builds on #4088. --- This is the provisioning layer #3481 asks to specify first: the part that creates the VMs. Neither of the other two is needed to review it. ### What it adds Two Cluster API providers, packaged the way the existing KubeVirt provider is — vendored components, `gzip -9 -n` for a reproducible artifact, a `make components` target that regenerates it from the upstream release and checks the downloads against the SHA-256 sums the Makefile pins: | Package | Version | Contract | |---|---|---| | `capi-providers-infraprovider-proxmox` | capmox v0.7.7 | v1beta1 | | `capi-providers-ipam-in-cluster` | CAIP v1.0.3 | v1beta1 | capmox v0.7.7 rather than the current v0.9.x on purpose: from v0.8 it declares contract `v1beta2`, which the CAPI core shipped here (v1.10.1) does not serve. Both join the `iaas` bundle as **opt-in** packages, where the KubeVirt infrastructure provider is unconditional. The asymmetry is deliberate: capmox drives a hypervisor outside the cluster and needs a Secret holding a Proxmox API token, which capi-operator reads before it installs anything — with no Secret the provider is never deployed at all. Enabling it by default would ask every Cozystack install that has no Proxmox to supply hypervisor credentials. ```yaml bundles: enabledPackages: - cozystack.capi-provider-infra-proxmox # brings IPAM with it - cozystack.capi-provider-ipam-in-cluster # also enableable on its own ``` Enabling capmox emits the IPAM provider automatically — capmox allocates worker addresses from an `InClusterIPPool` and has no DHCP mode, so on its own it never gets a `ProxmoxCluster` to `Ready`. IPAM stays independently enableable, since assigning addresses is a generic Cluster API concern. Naming capmox in `enabledPackages` while disabling IPAM stops the render. The cluster would catch that combination on its own — the capmox `PackageSource` depends on the IPAM `Package`, so with that `Package` absent capmox goes `Ready=False` with `DependenciesNotReady` and no `HelmRelease` is created — but it would not say why. Failing the render names the two conflicting list entries at the moment someone writes them, instead of leaving a condition to decode. ### Why the earlier attempt did not produce a VM #1927's blocker — "ProxmoxMachine does not produce a VM" — was not a capmox incompatibility. It was four independent causes, each of which only became visible once the previous one was cleared: 1. **`PROXMOX_URL` included `/api2/json`.** capmox appends that itself, so every call went to `/api2/json/api2/json/…` and returned 501. 2. **No IPAM provider.** `no matches for kind "InClusterIPPool"`; the `ProxmoxCluster` never reached `Ready` and every `ProxmoxMachine` sat in `WaitingForClusterInfrastructure`. 3. **`templateSelector.matchTags` is set equality, not containment** (`slices.Equal` after sort). A template carrying four tags is not selected by naming one of them: `found 0 VM templates with tags`. 4. **capmox waits for the qemu-guest-agent, which Talos does not ship.** `error waiting for agent: timed out` — cleared with `checks.skipQemuGuestAgent` and `skipCloudInitStatus`. None of these are in the provider. They are packaging and configuration, which is why the earlier work could look complete and still never boot a machine. ### Evidence Run on Proxmox VE 9.2.11 with Talos v1.13.6 and Cozystack's CAPI stack (core v1.10.1, CABPT v0.6.12, Kamaji control-plane provider v0.19.0): ``` $ kubectl get proxmoxmachine,machine -n tenant-proxmox proxmoxmachine/kubernetes-px-md0-… Ready=True VMProvisioned=True machine/kubernetes-px-md0-… Running proxmox://0ef88cc1-6aea-4563-af64-344d8060d5c0 $ qm list 106 kubernetes-px-md0-jmzds-h9npx running ``` The machine boots Talos from the nocloud ISO, reads the machineconfig capmox writes, and joins. That is the layer #69 called a hard blocker. ### What this PR does not claim - One Proxmox node, one tenant cluster, manual runs — there is no e2e coverage for Proxmox in CI, and this PR does not add any. - It does not make Proxmox a supported platform. It makes the provisioning layer reviewable, which #3481 names as the precondition for that conversation. - It does not touch `capi-providers-core`. The same drift check, pointed at that package, shows that the `startupProbe` from #2946 never reached the shipped blob; that fix changes what every new install runs and is #4483, with its own release note. - Nothing here exposes `ProxmoxCluster.spec.credentialsRef` to a tenant, and nothing should: `capmox-manager-role` grants get/list/watch/patch on Secrets cluster-wide and that field takes a namespace. Whatever renders a `ProxmoxCluster` for a tenant in the rest of the series has to keep the field out of tenant hands. ### Testing `make unit-tests` — exit 0. New suites: `packages/core/platform/tests/sources_capi_provider_proxmox_test.yaml` and `bundles_proxmox_wiring_test.yaml` (both bundle paths, the enable/disable precedence, and the refusal above). Every negated `containsDocument` in the wiring suite carries `any: true`; without it the negation passes whether or not the document is in the render. Three mutations, each reverted afterwards — emitting the capmox `Package` unconditionally, emitting the IPAM `Package` unconditionally, and dropping the `disabledPackages` check from `$proxmoxEnabled` — each turn red the tests named after the property they break, and the unmutated chart stays green (44 suites, 198 tests). `hack/capi-provider-components_test.bats` covers both new provider packages: the blob and the YAML are the same bytes; the Makefile version, the ConfigMap name and `spec.version` are one value — `applied-spec-hash` covers the provider spec and the config Secret but not the ConfigMap, so a bump that regenerates the files while `spec.version` stays behind installs the old provider silently; and the vendored files match the SHA-256 sums the Makefile pins, so the pin holds even when nobody runs `make components`. The render was compared against `main` for five configurations (defaults, `isp-full`, `isp-full-generic` with paas, an unrelated `enabledPackages`, and `disabledPackages` naming IPAM). No line is removed in any of them and the `Package` count is unchanged; the only additions are the two `PackageSource` objects, which the chart renders for every package, optional or not. ### 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/website — matches, no follow-up yet.** - `packages/core/platform/values.yaml` changed. `content/en/docs/next/operations/configuration/platform-package.md` is a hand-written table of the `spec.components.platform.values.*` keys; this PR does not add a key, but it documents two new values that `bundles.enabledPackages` now accepts, and the table is where a reader would look for them. - Two platform components are added (`capi-providers-infraprovider-proxmox`, `capi-providers-ipam-in-cluster`) → `guides/platform-stack/_index.md`, `operations/configuration/licenses.md`. - The `iaas` bundle gains two optional packages → `operations/configuration/variants.md`. **Everything else: no match.** Nothing under `packages/apps/` or `packages/extra/` (terraform-provider, website app lists), no `packages/core/installer/values.yaml` or `networking.*`/`publishing.*` key the Ansible role sets, no `hack/` file or package-layout change (ccp, external-apps-example), no `ApplicationDefinition` semantics, no telemetry metric, no node prerequisites (talm, ansible), no commit-convention change (community). - [ ] 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(cluster-api): add optional Cluster API packages for Proxmox — the capmox infrastructure provider and the in-cluster IPAM provider it requires. Neither is deployed unless named in `bundles.enabledPackages`, since capmox drives a hypervisor outside the cluster and does nothing without Proxmox API credentials. Enabling the Proxmox provider brings the IPAM provider with it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added optional Proxmox infrastructure provider support for IaaS clusters. - Added in-cluster IP address management, usable independently or alongside Proxmox. - Proxmox deployments require a pre-created credentials Secret and automatically include in-cluster IPAM. - Added validation for incompatible Proxmox and IPAM settings. - **Tests** - Added coverage for provider dependencies, package enablement, disabling behavior, invalid configurations, and packaged provider components. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
e883f6f7fc |
fix(capi): ship the core provider manifests the repository actually holds (#4483)
## What this PR does `capi-providers-core` ships the Cluster API core manifests twice: as `files/core-components.yaml`, which a reviewer reads, and as `files/components.gz`, which the chart mounts into the cluster. #2946 added a `startupProbe` to `capi-controller-manager` and edited only the YAML; the blob was never regenerated, so the probe never reached a cluster. This regenerates the blob from the YAML (`gzip -9 -n -c files/core-components.yaml > files/components.gz`), so the decompressed blob matches the YAML byte for byte, and the only change it carries is the six lines of #2946. ### What this reaches, and what it does not New installs only. capi-operator re-applies a provider when the hash of its spec and its config Secret changes (`applied-spec-hash`); the ConfigMap holding the components is in neither, so a cluster that already runs `capi-providers-core` keeps its manager without the probe until the provider's version moves. Whether existing clusters should get the probe, and how, is left open on purpose: moving `spec.version` and the ConfigMap name together would force the re-apply, but that is a version decision for the package, not a side effect of regenerating a blob. ### Also in this diff - `.helmignore`: `files/.*-components.yaml` is a glob, so it matched only names beginning with a dot, and `helm package` shipped the readable YAML alongside the blob that duplicates it. Now `files/*-components.yaml`. - `hack/capi-core-components_test.bats` holds the two files to the same bytes from now on; it goes red on a YAML edited without regenerating the blob. Found by the drift check written for the Proxmox provider packages in #4087, pointed at the package that was already here; split out because it changes what every new install runs and deserves its own release note. ### Testing `bats hack/capi-core-components_test.bats`: 1/1; red with a line appended to the YAML, green again after `gzip -9 -n -c`. `helm package` of the chart now contains `files/components.gz` and `files/metadata.yaml` only. ### Screenshots Not applicable, no UI change. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note fix(cluster-api): ship the `startupProbe` from #2946 in the packaged core provider components. New installs get it; a cluster that already runs the core provider keeps its manager without the probe until the provider's version moves, because capi-operator re-applies components only when the provider spec or its config Secret changes. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added a check that the compressed core components match the YAML source, with a diff provided when they differ. * **Chores** * Updated chart packaging exclusions to cover component YAML files regardless of whether their names begin with a dot. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
a285a5ef36 |
feat(kubernetes): add a proxmox substrate for managed clusters and worker pools (#4088)
## 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 #4087 and #4088 share no files and can be reviewed in parallel. #4089 builds on this one. --- #4087 adds the Cluster API providers; this PR teaches the two managed-cluster charts to point at them. ### The shape A single value, `substrate: kubevirt | proxmox`, on both charts — not a parallel set of charts. The control plane is Kamaji either way; what changes is the infrastructure half: | | `kubevirt` (default) | `proxmox` | |---|---|---| | `Cluster.infrastructureRef` | `KubevirtCluster` | `ProxmoxCluster` | | worker template | `KubevirtMachineTemplate` | `ProxmoxMachineTemplate` | | apiserver Service | `ClusterIP` | `LoadBalancer` | | tenant CCM/CSI | in-cluster (`kccm`, kubevirt-csi) | rendered off; the Proxmox pair is #4089 | The KubeVirt path renders byte-for-byte what it rendered before. Checked by rendering `kubernetes-nodes` with default values at `main` and at this head and diffing the output, not only by the snapshots — the snapshots cover `nodegroup.yaml` alone, and an earlier revision of this branch changed the reconcile Job while they stayed green. The parent chart's default render differs only in the random fields of `talos-secrets`, which differ between two renders of `main` as well. `osImage` (#4171) is refused on the proxmox substrate. Only half of it could be honoured there — `factory.version` reaches the machineconfig's installer image while the worker disk keeps coming from the Proxmox template — and a field that half-works is worse than one that says no. `talos.version` and `talos.schematicID` still set the installer target. ### Three things that only show up once workers are off-cluster **The apiserver endpoint.** A KubeVirt worker runs inside this cluster and reaches the tenant apiserver by ClusterIP. A Proxmox worker does not: it lives on the hypervisor's L2 and cannot route to `10.96.0.0/16`. The machine provisions fine, `VMProvisioned=True`, `providerID` set — and then never joins, with no CSR to show for it. On the proxmox substrate the Kamaji Service is therefore a `LoadBalancer`, and the reconcile Job reads `.status.loadBalancer.ingress[0].ip` instead of `.spec.clusterIP`. **And DNS, for the same reason.** The worker machineconfig carried `nameservers: ${COREDNS_IP}` — the management cluster's CoreDNS ClusterIP, equally unreachable. The proxmox path takes its own resolvers (`proxmox.dnsServers`) and the chart refuses to render without them, rather than writing an address that cannot answer. **`externalManagedControlPlane` and the managed-by annotation.** capmox's webhook nil-derefs on a `ProxmoxCluster` whose control-plane endpoint is empty unless `spec.externalManagedControlPlane` is set, which is correct here — Kamaji owns the endpoint. Separately, the `cluster.x-k8s.io/managed-by: kamaji` annotation that the KubeVirt path carries must **not** be set on the Proxmox object: with it, CAPI never sets an ownerRef and capmox stops at `Waiting for Cluster Controller to set OwnerRef`, leaving the address pool uncreated and every worker waiting on infrastructure. ### Linked clones by default `proxmox.full: false` — worker disks are linked clones of the template rather than full copies. Measured on a ZFS-backed store: **8 KiB per worker instead of 10.2 GiB**. Operators who want independent disks set `full: true`; the value is plumbed straight through to `ProxmoxMachineTemplate`. `proxmox.storage` is a full-clone parameter and is refused with a linked clone: a linked clone always lives on the template's storage, and both Proxmox (`parameter 'storage' not allowed for linked clones`) and capmox's own CRD (`Must set full=true when specifying storage`) reject the pair. The chart says so at render time, naming both values, instead of failing at apply time with an error about a `ProxmoxMachineTemplate` nobody wrote. ### Sizing `ProxmoxMachineTemplate` takes `numCores` and `memoryMiB`, not Kubernetes quantities, so the pool's `resources.cpu`/`memory` are converted (millicores → cores, bytes → MiB, both rounded up). The rounding is pinned in `tests/substrate_branches_test.yaml` with `1500m` and `8G`, where rounding up and down disagree; `tests/substrate_proxmox_sizing_test.yaml` covers the refusal to size a pool that gives neither. ### Known upstream behaviour this surfaces `TalosConfigTemplate.spec` is immutable by schema, so the reconcile Job cannot rewrite an endpoint it has already published — `kubectl apply` fails with `TalosConfigTemplate.Spec is immutable`, and the template has to be deleted for the Job to recreate it. A fresh install never meets it; it appears when the apiserver address changes on an existing cluster. Handling it changes the KubeVirt path too, so it is not in this PR: it is a separate change on `proxmox/tct-replace`, with its own upgrade note. ### Evidence ``` $ KUBECONFIG=<tenant> kubectl get nodes -o wide kubernetes-px-md0-jmzds-ndnl6 Ready <none> 75s v1.35.6 10.0.0.160 Talos (v1.13.6) $ KUBECONFIG=<tenant> kubectl get pods -A cozy-cilium cilium-… Running kube-system konnectivity-agent Running kube-system coredns-… Running ``` No management-cluster address remains in the rendered `TalosConfigTemplate`. ### Testing `make unit-tests` — exit 0. `apps/kubernetes` 26 suites / 314 tests; `apps/kubernetes-nodes` 29 suites / 160 tests / 12 snapshots, all unchanged on the default path. Each of the ten commits passes both suites on its own. Every substrate branch is asserted on both sides, and every assert goes red on the mutation it exists for: inverting the `SVC_IP` condition, inverting the `nameservers` condition, `ceil`→`floor` in the sizing, removing the proxmox gate from each of the kccm and csi templates (the two RBAC bindings included), pinning the retention lookup back to `KubevirtMachineTemplate`, widening the IPv6 class of the `dnsServers` validation, dropping the parent chart's `dnsServers` validation, the storage-with-linked-clone refusal, and the `osImage` refusal. Negated `containsDocument` asserts carry `any: true`. `hack/talos-reconcile-heredoc_test.bats` additionally runs the rendered heredoc through a real shell: a `dnsServers` entry with a substitution is refused and named, an IPv6-shaped one with a substitution is refused too, and a real address survives the shell unchanged. ### 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` and `packages/apps/kubernetes-nodes/values.schema.json` both gain fields: `substrate`, and a `proxmox` object (`allowedNodes`, `dnsServers`, `ipv4Config`, and on the pool side `templateTags`, `network`, `storage`, `full`). The provider hand-maintains a schema, a model and an expand/flatten pair per Kind, with no codegen from these files. - `values.yaml` gains defaults for those fields (`substrate: kubevirt`, `proxmox.full: false`, `prefix: 24`). The provider restates defaults it models and sends its own, so a stale one wins silently rather than showing as a diff. **Everything else: no match.** No app added, renamed or removed (website app lists); no version enum, `kind`, `plural`, `release.prefix` or output Secret/Service name changed; `packages/core/platform/values.yaml` untouched; the `*-rd` files are regenerated, not renamed (ccp); no `hack/` change; no node prerequisites (talm, ansible); no telemetry metric; no commit-convention change (community). - [ ] 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 a `substrate` value to the `kubernetes` and `kubernetes-nodes` applications, selecting whether worker VMs run on KubeVirt in this cluster (`kubevirt`, the default and unchanged) or on an external Proxmox VE cluster through capmox (`proxmox`). On the Proxmox substrate the tenant apiserver is published through a LoadBalancer and worker disks default to linked clones. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for choosing KubeVirt or Proxmox for Kubernetes clusters and node pools. * Added Proxmox configuration for nodes, networking, storage, DNS, templates, cloning, and IPv4 allocation. * Proxmox deployments now use the appropriate cluster, machine, networking, and control-plane resources. * **Validation** * Added checks for supported substrates, required Proxmox settings, sizing, and incompatible options. * **Documentation & Tests** * Documented the new settings and expanded coverage for rendering, validation, cloning, sizing, and template replacement. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |