mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-04 02:57:27 +09:00
fix/daemonset-resources
1409
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
d2e3eb4f82 |
fix(kubernetes, e2e): unblock autoscaling below min size, size the computeplane pool, fix the backupcontroller build (#4768)
## What this PR does Makes the backupcontroller test package compile again on main. #4587 added a flavor argument to `createCNPGBackupArtifact` and `marshalCNPGBackupSnapshot`, and #4669, whose CI ran before that, added two test calls with the old argument lists. Both PRs were green on their own, and together they break `go test ./internal/backupcontroller/`. The two calls now pass the same default flavor as the other tests in the file. The second commit fixes an e2e suite that fails the required E2E check here. It is the `computeplane` suite from #4713, and its worker pool was one node with a single vCPU. After the kubelet reservations that node has 900m of CPU to allocate. The tenant addons (cilium, coredns, csi, ingress-nginx, metrics-server, cert-manager) request exactly 900m between them. If cert-manager's startupapicheck hook pod is created last, it can't be scheduled and the install times out after 10m. The reinstall after remediation runs into the same full node. Whether that pod comes last depends on install order, so the suite passed on #4713 and failed here. The pool now has two workers with one vCPU each, the same shape the kubernetes suites use. One vCPU per worker stays on purpose, because of #3513. Two workers exposed a deadlock in the tenant cluster-autoscaler. The suite's pool is created at zero replicas and then gets a min size of 2, which CAPI does not apply to the live replica count on upgrade (#4786). The autoscaler's scale-up for a pending pod asks for one node, and the clusterapi provider refuses it because one is under the min size, although it is an increase. That refusal backs the group off, and the min-size enforcement skips groups in backoff, so the pool never leaves zero. Upstream had the same report in kubernetes/autoscaler#4942, closed as stale, and it is now tracked in kubernetes/autoscaler#10400. Our autoscaler image already patches the mirror case for max size, and the patch now refuses only a decrease below the min size. `go vet` and `go test ./internal/backupcontroller/...` pass, and so do the bats pins for the computeplane suite. The autoscaler patch adds a test for each direction, and the clusterapi package tests pass with it applied to 1.32.0. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved cluster autoscaling for worker groups outside configured size limits: groups below their minimum can scale up toward it, and nodes can be removed when a group exceeds its maximum. Scale-down requests below the minimum remain restricted. * **Tests** * Updated backup and restore test calls with PostgreSQL and empty flavor values. * Updated ComputePlane checks for a two-worker pool and clarified cert-manager installation timing expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
4c3c838a66 |
test(e2e): run the computeplane pool at two workers
A single-vCPU worker has 900m to allocate after the kubelet reservations, and the tenant addons' CPU requests use all of it. When cert-manager's startupapicheck hook pod is the last one created it stays Pending, the install times out, and the reinstall after remediation runs into the same full node. Whether it is last depends on install ordering, so the suite passed or failed on the race. Two workers match the kubernetes suites' pool and leave room for the hook pod. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
bcda5c893a |
feat(kubevirt)!: drop HotplugVolumes for declarative volume hotplug (#4453)
## What this PR does This turns off the deprecated KubeVirt `HotplugVolumes` gate and moves the platform to declarative volume hotplug. It changes how every volume hotplug on the platform works, so the title carries `!`. Closes #4032. This PR is stacked on #4451 and has to merge after it. #4451 adds the kubevirt-csi flag this PR flips. The replacement gate is already on in KubeVirt 1.9, and the old gate is the only thing keeping it off. `DeclarativeHotplugVolumes` is Beta at v1.9.0, and Beta gates are on by default. virt-controller skips `handleDeclarativeVolumeHotplug` while `HotplugVolumes` is on, so dropping the old gate is what switches the new one on. The kubevirt-csi fallback goes off in the same change. With the old gate off, virt-api rejects the VMI-level `removevolume` on a VM-owned VMI, so the chart now passes `--enable-vmi-hotplug-fallback=false`. The comments next to the gate and the flag point at each other. This needs the rebuilt kubevirt-csi image. A driver built before #4451 does not know the flag and exits on it at startup. The image is stamped at release preparation and ships in the same release as this change, so only a `make apply` from main before then would crash-loop the CSI controller. What changes at runtime on an upgrade: - The VM-level `addvolume` and `removevolume` calls now edit `VM.spec` directly. virt-controller then makes the VMI hotplug volumes match it, instead of going through `VM.status.volumeRequests`. - Volumes hot-plugged through the VM subresource are in `VM.spec`, so they stay attached. - During a tenant teardown, when the VM goes before its VMI, the driver no longer calls the VMI subresource. It waits for the VMI status to drop the volume, and the garbage collector deletes the VMI with its VM. Upstream waits up to two minutes. If the VMI is still there after that, the unpublish fails and is retried. A new platform migration keeps old VMI-only volumes attached. On its first pass virt-controller removes every hotpluggable VMI volume that is missing from `VM.spec`. Until July 2025 kubevirt-csi attached volumes at the VMI level (the Role then granted `virtualmachineinstances/addvolume`), and live migration carries such a VMI across KubeVirt updates. Without help, a tenant node not recreated since then would lose a mounted disk on upgrade. Migration 59 copies each such volume and its disk from the VMI into the VM template before anything else changes. Adding a hotplug volume to the VM spec is a valid live update in KubeVirt, so no VM needs a restart. It runs as the platform chart's pre-upgrade hook, before the upgrade re-points `cozystack-packages`. The KubeVirt CR and the tenant kubernetes charts both render from that source, so the gate and the flag cannot change before the migration has finished. It only touches VMs that need it, logs each one, and a second run changes nothing. The migration also makes ephemeral hotplugs permanent. A volume a user added at the VMI level, for example with `virtctl addvolume` without `--persist` from an older virtctl, has the same shape as kubevirt-csi's old attachments, because virt-api builds both the same way. There is nothing reliable to tell them apart by, so both are copied. Leaving the user's volume out would not keep it ephemeral: the first reconcile would unplug it from the running VM. After the upgrade it is part of `VM.spec` and attaches again on a restart, and `virtctl removevolume` drops it. Ephemeral hotplug stops working on VMs. With the old gate off, virt-api refuses VMI-level `addvolume` and `removevolume` on a VMI that a VM owns (`vmiVolumePatch` in v1.9.0). A volume on such a VMI is added and removed through the VM from then on. A VMI without a VM is not affected. Only running VMIs are copied. virt-controller reconciles no other VMI, and a stopped VM builds its next VMI from `VM.spec`, so copying from a stopped one would bring back volumes the restart was going to drop. Migrations run only on the isp-full, isp-full-generic and isp-hosted variants. KubeVirt comes from the platform only on isp-full and isp-full-generic, so those are covered. On the default variant, where packages are installed by hand, nothing runs the migration. This optional check lists the VMIs and volumes the migration would copy. It uses the same rule. On the default variant, run it before upgrading and recreate each tenant node it lists: ```bash kubectl get vm,vmi --all-namespaces --output json | jq --raw-output ' ([.items[] | select(.kind == "VirtualMachine") | {key: "\(.metadata.namespace)/\(.metadata.name)", value: [.spec.template.spec.volumes[]?.name]}] | from_entries) as $vm | .items[] | select(.kind == "VirtualMachineInstance") | ([.metadata.ownerReferences[]? | select(.controller == true and .kind == "VirtualMachine" and (.apiVersion | startswith("kubevirt.io/"))) | .name][0]) as $owner | "\(.metadata.namespace)/\($owner)" as $key | select(.status.phase == "Running" and $owner != null and $vm[$key] != null) | .spec.volumes[]? | select(.dataVolume.hotpluggable == true or .persistentVolumeClaim.hotpluggable == true) | .name as $n | select($vm[$key] | index($n) | not) | "\($key) \($n)"' ``` Empty output means nothing is affected. I checked the query against the migration's test fixtures only. Neither the query nor the migration has run on a live cluster. #4287 also adds a migration 59. If it merges first, this one moves to 60: the file, the version it stamps, `migrations.targetVersion`, the bats file (its `run_migration` argument and expected `STAMP`) and `hack/testdata/migration-59-vmi-hotplug`. A VM with a pending hotplug request at upgrade time may also get stuck. virt-controller still processes `VM.status.volumeRequests` before the declarative handler, and it applies them through the VMI subresource, which virt-api rejects once the old gate is off. The sync then fails before the VM spec is updated, so nothing clears the request, and every later sync of that VM fails the same way. A retry through the VM subresource edits the spec and clears it, which covers kubevirt-csi's own requests. A one-shot request stays stuck until the VM restarts. I read this from the v1.9.0 code and haven't reproduced it. Waiting until this lists nothing before upgrading narrows the window, but a request made during the rollout can still hit it: ```bash kubectl get vm --all-namespaces --output json | jq --raw-output '.items[] | select((.status.volumeRequests // []) | length > 0) | .metadata.namespace + "/" + .metadata.name' ``` The two halves live in different HelmReleases, so there is a short window during the rollout where the driver and the gate disagree. In that window an unpublish fails and is retried until both are updated. A tenant cluster whose `kubernetes` HelmRelease is suspended or failing keeps its old driver for longer. Drivers from before July 2025 attach through the VMI subresource, so on such a tenant every attach is refused until its release reconciles. Such a driver can also attach a volume after the migration has run and before the gate flips. The migration doesn't see that volume, and the first declarative reconcile unplugs it from the running VM. The migration lists the VMs once and patches each one with a `resourceVersion` test, so a VM that changes in between fails the hook Job. The Job has `backoffLimit: 3`, and each retry lists again and skips the VMs already patched. Re-reading only the conflicting VM would avoid the retry; I left that out. The new gate is listed in the CR even though it is on by default. With the old gate gone it is the only hotplug path, and the chart refuses a render that disables it. I found no other user of `HotplugVolumes` in the platform or in the downstream repositories. The VMI-level fallback may have been failing already. The kcsi Role has no rule for the VMI-level `removevolume` (#4452), so that call likely got 403 with or without the gate. I haven't checked that on a cluster. `hack/migration-59-vmi-hotplug-volumes.bats` runs the migration against a fake kubectl: a VM that needs the copy, VMs and VMIs that must be left alone (a VMI that is not Running among them), a second run, a cluster without KubeVirt and the fail-closed paths. The helm-unittests for the KubeVirt CR pin the whole gate list, accept `HotplugVolumes` in `disabledFeatureGates` and refuse `DeclarativeHotplugVolumes` there. The CSI test pins the flag at false, and the `make update` idempotency test still passes. None of that exercises hotplug itself. The real test is e2e with volume attach and detach on a tenant cluster, including a teardown. No CI lane upgrades from the previous release yet, so an upgrade with a live VMI-only volume is untested; #4463 tracks that test. ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(kubevirt)!: KubeVirt now runs with DeclarativeHotplugVolumes instead of the deprecated HotplugVolumes feature gate, and kubevirt-csi no longer uses the VMI-level hot-unplug. Volume hotplug requests are applied through VM.spec. The upgrade copies volumes that older kubevirt-csi versions attached only to the VMI into VM.spec, so they stay attached; the check in the PR description is optional. This also applies to ephemeral volumes added to running user VMs at the VMI level, such as with virtctl addvolume without --persist: they become part of VM.spec and attach again after a VM restart. virt-api now refuses VMI-level addvolume and removevolume on a VMI owned by a VM, so such volumes are added and removed through the VM. A tenant cluster whose kubernetes release is suspended or failing keeps its old kubevirt-csi, and a driver from before July 2025 cannot attach volumes there until that release reconciles, and a volume it attaches during the upgrade itself can be unplugged again. A values override that puts DeclarativeHotplugVolumes into disabledFeatureGates now fails the kubevirt render. Clusters on the default variant do not run platform migrations and should run that check before upgrading. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Declarative hotplug volumes are enabled by default for KubeVirt. * Running virtual machines keep eligible hotplugged volumes and disks when their configuration is migrated. * **Bug Fixes** * The migration avoids adding duplicate volumes or disks and does not mark the migration complete if required operations fail. * The Kubernetes CSI driver no longer uses the VMI hotplug fallback. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
d367846030 |
test(e2e): cover the computeplane module end to end (#4713)
## What this PR does This adds an e2e suite for the ComputePlane tenant module, which had only render tests so far. The suite lives in `hack/e2e-chainsaw/computeplane/`. It turns the module on in its own tenant, `tenant-cplane`, so the shared `tenant-test` is never toggled. That tenant is a child of `tenant-root`, not of `tenant-test`, because a tenant two levels down cannot reach the etcd it inherits from root (#4726). What it checks on a real cluster: - the module release installs, and the cluster release it renders keeps the pinned addons and the `internal.cozystack.io/computeplane` label; - the pool release gets only the keys from the module's per-pool allowlist (the patch sends a `nameOverride` through the ComputePlane API and checks it is dropped); - Kamaji writes `computeplane-cluster-admin-kubeconfig`, and the tenant can't get at it: the Secret is not labelled `tenantresource=true`, `tenantsecrets` does not list it to the tenant's super-admin group, and `kubectl auth can-i get` answers `no` for that group (the read was possible until #4722); - cert-manager and ingress-nginx come up inside the cluster, and the Proxied ingress Service gets a ready endpoint; - the module and cluster releases have no remediation cycle in `status.history`; - on teardown, the in-cluster addon releases are gone before the KamajiControlPlane starts deleting; - turning the module off removes all of it: releases, in-cluster addons, control plane, Secret, worker VMs and disks, and the LINSTOR pools go back to their pre-suite level. The issue names the Secret `computeplane-admin-kubeconfig`. The module writes `computeplane-cluster-admin-kubeconfig`, so the suite checks that name. The issue also lists fluxcd as a pinned addon. That is out of date. The module runs no in-cluster Flux on purpose (`packages/extra/computeplane/templates/cluster.yaml:108-110`), so the suite checks cert-manager and ingress-nginx only. One part of the issue is not covered here. The `placement: ComputePlane` routing in cozystack-api doesn't exist yet, so there is no catalog app to run on the cluster. The issue ties that to "together with, or right after" this test, so I left it for the routing work, tracked in #4715. Things the review found outside this diff, each filed on its own: the module release's 10m install timeout against a slow cluster bringup (#4714), the module docs not matching the per-pool allowlist (#4716), and the LINSTOR helpers in `_lib` running `kubectl exec` without a timeout (#4111). The suite brings its own pool: `md0`, one worker, same sizing as the kubernetes suites. The default `md0` that the module renders for empty `nodeGroups` asks for the `replicated` StorageClass. The container lane doesn't have it, so on this lane the default pool stays covered by the render tests only. ComputePlane workers can't use the in-sandbox Talos image cache. The pool allowlist drops `talos`, so the worker pulls its disk from the public Image Factory (#4700). If the worker join or an addon release times out, the catch block prints the worker image source and points at that issue, so a red run caused by the factory is easy to tell apart. The suite itself needs no entry in `hack/select-e2e.sh`, because `computeplane-application` maps to it by name. That source depends on `kubernetes-application` and re-sources the kubernetes charts. So PRs that touch the kubernetes charts or ingress-nginx now select `computeplane` too, which I estimate adds 12-15 minutes to a green run (kubernetes-previous takes about 8 minutes with two workers, plus teardown). A red run can take much longer, because the waits sit above the release install timeouts. The suite header has the numbers. It is the third suite that brings up a tenant cluster, and the 215m E2E job cap is sized for the red path of the kubernetes pair only. If worker join breaks everywhere, all three go red and can run past the cap, which cancels the job and loses its log. I left that as an accepted risk. `hack/select-e2e_test.bats` now expects `computeplane` in the ingress-nginx selection. The computeplane source also owns `apps/kubernetes-nodes`. Before this PR a pool chart edit reached no suite and ran everything. With the new suite alone it would have selected only `computeplane`, which never creates `KubernetesNodes` natively. So `kubernetes-nodes-application` now maps to `kubernetes-latest kubernetes-previous computeplane` in `hack/select-e2e.sh`, and two new tests pin that for a pool chart edit and a `kubernetes-nodes-rd` edit. The suite is not in the leak-guard allowlist. Its teardown already fails when a TenantControlPlane sticks on its finalizer (#4489), because the admin kubeconfig Secret it owns stays behind, so an exemption would only hide other leaks. The README suite list now names the suite. JFYI, not a defect claim: the Proxied ingress Service from the kubernetes chart has `internal.cozystack.io/tenantresource: "true"`. So the tenant sees it, even though `computeplane-rd` has `services.include: []`. The module doesn't promise to hide it. How I checked it. `chainsaw lint` passes at v0.2.15, the version the sandbox pins. I also ran the suite against an envtest apiserver, with fake controllers applying the real `helm template` output of the tenant and computeplane charts. The intact run is green. Each of these breakages turns it red at the step meant to catch it: label removed, addon `exposeMethod` changed, a key outside the pool allowlist, the allowlist `pick` replaced by the whole group, stale Ready from an older generation, wrong Secret name, Secret labelled `tenantresource`, Secret listed in `tenantsecrets`, a direct RBAC grant on the Secret, ingress-nginx addon not Ready, no ready ingress endpoint, an `uninstalled` history entry, the control plane deleted before the addon releases, an addon release or worker VM left after teardown. Closes #3369 Closes #4747 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note test(e2e): add an end-to-end suite for the ComputePlane tenant module ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added end-to-end coverage for enabling and disabling the ComputePlane tenant module, including generated releases, pool value filtering, admin credential access, addon and cluster readiness, and resource cleanup. * Kubernetes nodes changes now select the ComputePlane suite alongside the latest and previous Kubernetes suites. * **Documentation** * Added the ComputePlane suite to the documented end-to-end test suite list. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9734a16692 |
feat(postgres): add a postgis image flavor (#4587)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Adds a `flavor: postgresql | postgis` enum to the `postgres` app. `postgis` runs the cluster on the CloudNativePG PostGIS operand image, so tenants can enable `postgis`, `postgis_raster`, `postgis_topology`, `postgis_sfcgal`, `postgis_tiger_geocoder` and `address_standardizer` through the existing `databases.<name>.extensions`. The default stays `postgresql` and renders exactly the image it rendered before this change. This picks up #2672 by @matthieu-robin, which the stale bot closed after a rebase request, with these changes: - The postgis tags in #2672 (`17.7-3.5`, `16.11-3.5`, ...) do not exist in `ghcr.io/cloudnative-pg/postgis`; the registry answers 404, so the flavor would have ended in `ImagePullBackOff`. The map now uses tags that exist, `<minor>-<postgis>-standard-trixie`, on the same PostgreSQL minor as `files/versions.yaml`. - v18 is supported: PostGIS images for 18 now exist upstream. - `standard` rather than `system`: the chart archives through the barman-cloud plugin since the backup rework, so the operand no longer needs the barman binaries of the deprecated `system` variant. - `hack/update-versions.sh` derives the postgis map from the minors it picks, and `hack/check-postgres-postgis-versions.bats` fails if the two maps drift to different minors or away from `standard-trixie`. - The chart refuses a flavor change on a live `Cluster`. The default images are built on Debian bullseye (glibc 2.31) and the postgis ones on trixie (glibc 2.41): CNPG would roll the swap out as a plain image update, under an initialised data directory, which silently invalidates every index on collatable text, and the postgis-to-postgresql direction also drops the libraries behind every PostGIS object. Only the image name is compared, so minor bumps and registry mirrors keep passing. - `pgrouting` is not claimed: it is absent from the `18.1-3.6.1` build the chart pins and only appears in later postgis builds. A general mechanism (CNPG ImageVolume extensions) was considered and left out: it needs PostgreSQL 18 and Kubernetes 1.35 or the `ImageVolume` gate, so it cannot serve v13-v17 nor most management clusters today, and PostGIS is the only extension CloudNativePG ships as a full operand image. No other extension request exists in this repository. Testing: - `helm unittest packages/apps/postgres`: 145/145, including 6 new cases in `tests/flavor_test.yaml` (default image, postgis on the Cluster and on the init Job, the guard in both directions with a mocked live Cluster, and a mirrored digest-pinned image taking a new minor). Removing the guard makes exactly the two guard cases fail. - `hack/cozytest.sh hack/check-postgres-postgis-versions.bats` passes, and fails on a shifted minor or a `system` tag. - `make -C packages/apps/postgres generate` leaves no diff. - The rendered `ghcr.io/cloudnative-pg/postgis:18.1-3.6.1-standard-trixie` was started locally: `CREATE EXTENSION` for citext, postgis, postgis_raster and postgis_topology succeeds and `ST_Buffer` answers. A three-instance cluster on the `system` sibling of that image has been running PostGIS in production since 2026-09-29. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#63 (issue: the provider models every field by hand, and the default handling needs a decision there) - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(postgres): add a `flavor` field accepting `postgresql` (default) or `postgis`. `postgis` bootstraps the cluster from the CloudNativePG PostGIS image, so PostGIS extensions can be enabled per database through `databases.<name>.extensions`. The flavor is fixed at creation: changing it on an existing cluster is refused, and so is restoring a backup into a cluster of the other flavor, whether through a RestoreJob or `bootstrap.enabled`; move data across flavors with a logical dump. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a `flavor` option for PostgreSQL clusters. The default `postgresql` flavor remains available, while `postgis` selects an image with PostGIS extensions. - PostGIS images are available for PostgreSQL versions 13–18. - Cluster flavor is fixed at creation. To switch flavors, create a new cluster and migrate data with a logical dump. - **Bug Fixes** - Restores are refused when the backup and target cluster use different flavors. - **Documentation** - Updated PostgreSQL configuration guidance to describe flavor options and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
cc51cac63d |
fix(monitoring): give a central tenant's Grafana a login that cannot write
The collector holds a central tenant to its write rate, but the one login it writes with was also the login of the tenant's Grafana datasource, and it could both read and write. vmauth cleans the request path before it picks a route, so any user the tenant admits to its Grafana, a Viewer included, could POST through the datasource proxy to ../insert and write into the shared store past the limiter. Each central tenant now has two VMUsers on its one account: the collector's, which may only write, and Grafana's, which may only read, each with its own chart-owned Secret and password. A namespace name holds no dot, so the reader's username never matches another tenant's. On upgrade the datasource moves to the new login in the same release that creates it, so reads may fail until vmauth loads the new user. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
474377e8ca |
build(monitoring): build the traces collector with a rate limiter
A shared-central traces store needs each tenant held to a write rate, or one tenant can fill the disk every tenant shares. No upstream OpenTelemetry distribution or processor limits throughput. Elastic's EDOT Collector registers Elastic's ratelimitprocessor among its default components, but it ships under the Elastic License 2.0 and carries far more components than the chart uses. The collector is therefore built here with the OpenTelemetry Collector Builder from the upstream components the chart already uses, at the same v0.136.0, plus that ratelimitprocessor (Apache-2.0). Its v0.8.2 is the release built against v0.135.0, the last before v0.137.0. The image is versioned by the collector release with a cozystack respin counter, runs as the upstream image's user, and is built with the other package images. The builder stage is alpine, which has no git, so a proxy error during its go install would have had neither the direct fallback nor a retry; the stage installs git. The module-fetch contract saw only go build, so it now checks go install too, and takes only git as cover for a versioned go install, which resolves outside the stage's module. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
|
d35493a64d |
test(e2e): cover the computeplane module end to end
The module had render tests only. Nothing checked that turning it on in a tenant yields a working cluster: the Kamaji control plane, a worker pool shaped by the module's allowlist, an admin kubeconfig the tenant can neither list nor read, the pinned cert-manager and ingress-nginx addons running inside the cluster, and a teardown that removes the in-cluster addons before the control plane and leaves nothing behind. The suite runs the module in its own child tenant so the shared tenant-test is never toggled. It supplies its own pool because the default md0 asks for the replicated StorageClass, which the container lane does not have. On failure it prints the worker image source, since the pool allowlist drops talos and a ComputePlane worker cannot use the in-sandbox image cache. The computeplane source depends on the kubernetes application and runs its own ingress-nginx, so an ingress-nginx change now selects this suite too. It also owns apps/kubernetes-nodes, which used to reach no suite and run everything; kubernetes-nodes-application now maps to both kubernetes suites and computeplane, so a pool chart change keeps the suites that create KubernetesNodes natively. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
|
a5d12e78d4 |
fix(ci): reconcile three unit-test contracts broken by adjacent merges (#4751)
## What this PR does Unit tests on main are red, and that blocks E2E on every open PR. Three contracts broke the same way. In each case a check from one PR and a line from another merged a few minutes apart on 2026-10-05, with no textual conflict. Any one of them alone keeps the job red, so they are fixed together. The first is the pin on how a suite reads HelmRelease history. #4689 added it. It accepts `$(kubectl ...` and, since #3594, the bounded `$(_cozy_guard_kubectl ...` wrapper. Nothing in front of kubectl is allowed. The monitoring suite from #4678 bounds that read with `$(timeout 30 kubectl ...`, and the bound is wanted. So the pin now accepts an optional `timeout [-k N] N` in front of kubectl. The prefix is spelled out on purpose. Any text would also let through `$(echo kubectl ...)`, which names the shared expansion without running it, and that form is now a refuse case. I checked it with mutations: the new accept cases fail against the old pattern, and a pin that takes any prefix fails a refuse case. The second is the flux install in workflows. #4645 forbids running fluxcd.io/install.sh and moved every workflow to the SHA-pinned fluxcd/flux2 action. The isp-hosted lane from #4712 still ran the script. It now uses the same action and version. The third is the negated-assertion audit. It reads the find operand in `\( ! -type d` in select-e2e_test.bats as a shell negation and fails. The line now says `-not -type d`. This is the same change as #4748, so this PR supersedes it. The audit bug itself is #4752. After the Go fix, the full log of the unit job had only these two bats failures left. Closes #4750 Closes #4755 ### Screenshots ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(ci): fix three unit-test contracts that went red on main after adjacent merges. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * History checks now recognize direct `kubectl` reads and reads through the supported wrapper, including timeout-bounded commands with an optional kill-grace setting. Commands that only include `kubectl` as an argument, use similarly named commands, or place a timeout before the wrapper are not treated as reads. * **Chores** * The hosted end-to-end workflow now uses a pinned Flux version, and suite selection handles directory exclusions consistently. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
0cae0e87ad |
test(e2e): spell the find negation as -not in the suite-list helper
The negated-assertion audit splits a line on `(` and reads the `!` after `\(` as a shell negation that errexit would exempt, so the helper's find predicate fails the audit and the unit tests on main. `-not` is the same predicate in GNU, BSD and busybox find and does not look like one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7bf6906a80 |
fix(platform): keep the legacy etcd-operator stopped after etcd adoption (#4738)
## What this PR does Migration 50 scales the legacy etcd-operator to 0, adopts every legacy `etcd.aenix.io` cluster with `etcd-migrate`, and then scales the operator back to 1. At that point the Deployment still runs the legacy operator, and after a successful adoption it has nothing left to manage. Bringing it back undoes part of the adoption. Observed on a production cluster upgraded from 1.4.3 to 1.6.4. The API audit log and the Helm history of a tenant's `etcd` release show this sequence, in UTC on the same day. - 13:21:12 migration 50 scales `cozy-etcd-operator/etcd-operator-controller-manager` to 0. - 13:22:59 `etcd-migrate` updates `etcd-headless`, pointing its owner references at the adopted EtcdMembers, and orphan-deletes the legacy EtcdCluster and StatefulSet. - 13:23:04 migration 50 scales the operator back to 1. It is still the legacy operator, the v1alpha2 one replaced it only at 13:40. - 13:23:42 Flux upgrades the tenant's `etcd` release with the old chart (Helm revision 87), because the new chart artifact appeared only at 13:24:40. The old chart re-creates the legacy EtcdCluster at 13:23:43. - 13:23:43 to 13:23:45 the legacy operator reconciles it, updates `etcd-headless` twice and re-creates the legacy StatefulSet `etcd`. - 13:36:49 Flux upgrades the release to the new chart (revision 88), which prunes the legacy EtcdCluster. - 13:36:50 the garbage collector deletes `etcd-headless` and the re-created StatefulSet. The same happened in all five tenants that run an etcd. The pods survived because EtcdMembers own them, but they keep `spec.subdomain: etcd-headless`, so their `etcd-<i>.etcd-headless.<ns>.svc` names stopped resolving. From then on every member logged `no such host` for its peers, the v1alpha2 EtcdCluster had no status conditions, and the `etcd-defrag` CronJob failed every hour with `context deadline exceeded`. The transitional `etcd-headless` Service the etcd chart ships for this window (#3270) does not cover it. Helm patched the existing Service 202 ms before the garbage collector removed it, and did not create it again because it was already part of the release. With this change the operator stays at 0 after a successful adoption. The etcd-operator HelmRelease replaces the Deployment with the v1alpha2 operator later in the same upgrade, and no later migration waits for the operator. A failed or interrupted adoption still restores it to 1 through the existing EXIT trap, so a failed run does not leave the tenant etcds without an operator. If that HelmRelease cannot reconcile, the adopted clusters wait for it with or without this change, because the legacy operator does not reconcile v1alpha2 EtcdClusters. Clusters already upgraded with this bug can be repaired without restarting anything. Creating the Service from the tenant's own `etcd` release is enough. ``` helm -n <ns> get manifest etcd | awk 'BEGIN{RS="\n---\n"} /\nkind: Service\n/ && /\n name: etcd-headless\n/' | kubectl -n <ns> create -f - ``` On the cluster above the v1alpha2 EtcdCluster reported `Available` with reason `QuorumHealthy` within 11 seconds of the Service being created, in every tenant, and the members stopped logging `no such host`. ### Validation - `hack/cozytest.sh hack/migration-50-etcd-adopt.bats` passes all 29 tests. A new test asserts that a successful adoption scales the operator to 0 and never back to 1. Against the current script it fails with `FAIL: the legacy operator was scaled back up after a successful adoption`. - The existing test for a failed adoption still passes, so the EXIT trap still restores the operator to 1. - The new test checks the absence with `if grep ...; then echo "FAIL: ..."; false; fi`, the form the other suites use. A bare `! grep` assertion cannot fail a test under `hack/cozytest.sh`, so it could not have caught this. The other negated assertions in this file had the same weakness, and #3790 has rewritten them on main. With all of them made able to fail in a scratch copy, the suite still passes against this change. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It changes a migration script and its bats test. No package, values, schema, CRD, make target or `hack/` layout changes, so no entry matches. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(platform): keep the legacy etcd-operator stopped after migration 50 adopts the legacy etcd clusters. Scaling it back up let it reconcile the legacy EtcdCluster that Flux re-created from a not yet upgraded etcd chart, re-create the legacy StatefulSet and take the legacy etcd-headless Service away from the adopted members, which the garbage collector then deleted. The adopted members lost their DNS names, the EtcdCluster never reported a status and etcd defrag failed every hour. Clusters already upgraded can recreate the Service from the etcd release manifest. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Migration Updates** * After successful etcd adoption, the legacy operator remains scaled to zero replicas for replacement by the etcd operator; it is no longer scaled back up before the migration is stamped. * If adoption fails or exits before completion, the legacy operator is restored to one replica. * The dry-run message now describes the zero-replica outcome of successful adoption. * Successful adoption continues to record the migration version stamp. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
415430bcc7 |
fix(e2e): fail on a failed release of cilium and csi as on the parent, and report addon remediation (#3594)
## What this PR does The history guard in the tenant Kubernetes e2e suite checked only the parent `kubernetes-<test>` HelmRelease. A failed action on one of the addon releases it installs left nothing the suite looked at, so a run showed only the downstream symptom, usually the node-join deadline, and never named the release behind it. The guard now reads every addon release of the parent. It selects them from the cluster by name prefix, so a release added to the chart later is covered too. The rule: - A `failed` or `uninstalled` Snapshot fails the run on any release whose install strategy is `RetryOnFailure`. That is the parent (the aggregated apiserver stamps it on every Application HelmRelease) and the tenant `cilium` and `csi`. - The same footprint on the other addons, which run the default strategy with `retries: -1`, is printed as a note and the run stays green. For the CRD addons that configuration is tracked in #3555. A transient `cilium` or `csi` failure that later recovers on a retry still turns the run red, by design, as long as its Snapshot still reads `failed`. One recovery leaves no such Snapshot: when an install fails and the very next attempt succeeds, Helm marks the failed install `superseded`, because there is no deployed release to supersede, and the history reads like an ordinary upgrade. The guard cannot see that case from the statuses alone. The code and the docs now say so, a test pins it, and detection is left to a follow-up. An earlier revision of this PR tried to tell a teardown apart from a retried failure and made only `uninstalled` fatal. That split does not hold. In helm-controller v1.5.0, the version the platform installs, a release with an active retry strategy never remediates. `atomic_release.go` returns an upgrade for a failed release before it reaches the remediation switch, and the two other paths that uninstall (a changed release target, and deletion) both clear history afterwards. So on these releases `uninstalled` never shows up, and `failed` is the only signal. The parent keeps the rule it already had on main, and `cilium` and `csi` now follow it too. The failure paths get the same reading. The guard runs after the assertions, while the deadlines a failed addon breaks end the script earlier, so the same classification is also printed, without a verdict, from the node-join collector and from the exit handler. That handler is now armed at the tenant CR apply, so the management-cluster waits before the tenant answers are covered too. Smaller changes that came with this: - Every guard read is bounded, and a failed read is never folded into "no history". - The parent must read back a non-empty strategy. An empty value there means the jsonpath stopped resolving, and without this check every footprint would quietly turn into a note. - The history read uses `HELMRELEASE_HISTORY_JSONPATH`, and the upstream-type pin in `internal/fluxcontract` now accepts the bounded wrapper the read goes through. `docs/agents/e2e-testing.md` §6 and checklist item 8 describe the new rule. On related issues: this adds the `local` that #3774 reports missing in `helmrelease_has_remediation_cycle`, but not the sweep over the other `_lib/` helpers that the issue also asks for. For #3773 it rewrites the guard comment the issue quotes. The waits without a backstop are not touched. Closes #3553. ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map against the diff. It touches `hack/e2e-chainsaw/_lib/`, `hack/*.bats`, one Go test under `internal/fluxcontract`, two spend-order comments in the kubernetes suites, and `docs/agents/e2e-testing.md`. No package, schema, CRD, make target or user-facing document changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): the HelmRelease history guard now also covers the tenant addon releases. A failed or uninstalled Snapshot fails the run on the parent, cilium and csi, which retry in place; other addons get a note. The same classification is printed on the failure paths where the guard itself cannot run. ``` |
||
|
|
2188f16461 |
test(vminstance): cover VMDisk clones from an image and from a disk (#4586)
## What this PR does Closes #3482. The `vminstance` suite now covers both VMDisk clone paths, which had no e2e test at all. One disk is cloned from a golden image with `source.image`, and a second disk is cloned from the first with `source.disk`. For each clone the test checks that the DataVolume points at the expected source PVC and becomes `Ready`, that its PVC is `Bound`, and that the VMDisk reports `Ready` and `WorkloadsReady`, the same way the standalone VMDisk test does. The `Bound` check is separate because CDI derives `Ready` from the DataVolume phase alone, and can reach `Succeeded` from the claim's annotations before the claim binds; a `phase: Succeeded` assert would only repeat `Ready`. The golden image is a fixture the suite creates itself. `vm-default-images` is an opt-in package and the e2e install does not enable it, so `alpine-3.21` from the default collection is not there to clone. The suite imports the same Alpine 3.21 image into `cozy-public` under the name `e2e-alpine-3.21`, so it can never collide with an image the package owns. This is the only download in the new test, and the `source.disk` case reuses the first clone as its base disk. The long wait sits on the DataVolume, not on the HelmRelease. Since #4380 a vm-disk release goes Ready once the DataVolume exists and the clone's progress is reported through the VMDisk `WorkloadsReady` condition, so the test does not assert the HelmRelease at all. Chainsaw deletes the clones before the golden image at cleanup. The clone itself is only proven by this PR's E2E run, on the `local` class of the PR lane. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `hack/e2e-chainsaw/vminstance/`, and nothing in the trigger map covers it. ### Release note ```release-note test(vminstance): e2e now covers VMDisk clones from a golden image (source.image) and from another disk (source.disk). ``` |
||
|
|
4b1a75a332 |
test(vminstance): dry-run evict the VM launcher pod (#4585)
## What this PR does Part of #4422. The e2e suite now asks KubeVirt whether it would let a VM launcher pod be evicted, without evicting it. Until now nothing in e2e evicted a launcher pod, so the drain behaviour set up in #4403 was known only from reading KubeVirt. Once the `vminstance` VM is running, the suite sends a dry-run Eviction (`dryRun=All`) for its virt-launcher pod and checks the answer. Nothing gets deleted. A dry run still goes through the KubeVirt eviction webhook, which is registered with `sideEffects: NoneOnDryRun` in v1.9.0. With `LiveMigrateIfPossible` its answer follows the VMI's `LiveMigratable` condition. That condition depends on storage, so the test reads it instead of assuming it: - `LiveMigratable=True`: the eviction must be refused with the webhook's `Eviction triggered evacuation of VMI "<ns>/<name>"` message (HTTP 429). A drain retries on that while the VM migrates away. - `LiveMigratable=False`: the eviction must be allowed. The old `LiveMigrate` value refused this case with "is not live-migratable". KubeVirt v1.9.0 creates no PodDisruptionBudget for a VMI, so no PDB can refuse it either. - Anything else, including a missing condition, fails the test. The allowed answer alone proves little, because the webhook also lets an eviction through under `None`. So the test also asserts `evictionStrategy: LiveMigrateIfPossible` in the VMI spec, where virt-api copies the cluster value at creation. After the dry run it checks that the launcher pod is the same object and not terminating, and that the VMI has no `evacuationNodeName`. On the PR lane only the `False` branch runs. Post-install prep pins `StorageProfile/local` to RWO because DRBD is off there, and an RWO disk makes the VMI non-migratable. Nightly and tag runs use `replicated`, where I expect the `True` branch, but I have not seen this suite run on that lane yet. A real drain and a workload update after a KubeVirt bump are still open in #4422. I ran the script against a stub `kubectl`. Both expected answers pass, and it fails on every wrong answer I tried, such as a refusal for a non-migratable VMI, a PDB-style 429, a missing condition, two launcher pods, or a replaced pod after the dry run. `chainsaw lint` (v0.2.15) and shellcheck pass. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `hack/e2e-chainsaw/vminstance/` and the suite README, and nothing in the trigger map covers either. ### Release note ```release-note test(vminstance): the e2e suite now dry-run evicts the VM launcher pod and checks the KubeVirt eviction decision against the VMI's LiveMigratable condition. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added end-to-end coverage for dry-run eviction of a running virtual machine’s launcher pod. The test checks the VM’s live-migration policy and `LiveMigratable` condition, expecting eviction to be refused when the VM is live-migratable and accepted otherwise. * Verified that the dry run leaves the launcher pod unchanged and does not record an evacuation node on the VM. * **Documentation** * Documented the test’s behavior and the local `jq` requirement for running it. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
50dec5d46a |
fix(platform): resume a retried migration Job from the stamped version (#4583)
## What this PR does A retried pod of the platform migration Job no longer runs migrations that already finished in the same upgrade. The Job gets `CURRENT_VERSION` once, at render time, and a failed pod is replaced by one with the same environment. By then every finished migration has stamped the `cozystack-version` ConfigMap, but the new pod ignored that and started from the rendered version again. `run-migrations.sh` now starts from the version stamped in the ConfigMap. If reading it fails, the pod fails and the Job retries it, because falling back to the rendered version is exactly the re-run this fixes. The rendered value is used only when the ConfigMap holds no integer version, which is also when the chart itself could not get a real version from it. Without that fallback a value like `v58` gave a green Job that ran nothing. The new `hack/run-migrations.bats` runs the script in the migrations image's alpine base against a fake kubectl that keeps the stamp in a file, so one pod's stamps are what the next pod reads. It needs docker, like the other migration suites. I also found that the ConfigMap existence check at the top of the script treats any API error as a missing ConfigMap and skips every pending migration. That is older than this change and is filed as #4579. Closes #4555 ### Screenshots ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(platform): a retried platform migration Job now resumes from the version stamped in the cozystack-version ConfigMap instead of re-running migrations that already finished in the same upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Migration startup now resumes from a valid stamped version, avoiding reruns of migrations that have already completed. * Empty or non-numeric stamps fall back to the rendered version. Out-of-range stamps or failed stamp reads stop migrations before they run. * If the migration ConfigMap is missing, migrations do not run and the target version is stamped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
2ddd55ce5b |
fix(build): honour PUSH and LOAD from the environment and in image-talos (#4654)
## What this PR does A local build that asks to load images into docker no longer pushes them. `hack/common-envs.mk` set `PUSH` and `LOAD` with plain `:=`, and a makefile assignment beats the environment, so `LOAD=1 PUSH=0 make image` quietly pushed a multi-arch build. Both now take the environment value, and an empty one falls back to the default. I also made `PUSH` default to 0 when `LOAD=1`, so `LOAD=1` alone only loads. Before this, `make image LOAD=1` pushed and loaded at once. An explicit `PUSH` from the environment or the command line still wins, and `OCI_EXPORT_DIR` still forces both off. Every buildx target gets the values through `BUILDX_ARGS`, so the fix lives in that one file. The Talos installer was the one image that ignored `PUSH`. It is published with skopeo, not buildx, so it never saw `--push=$(PUSH)`. Its registry copies now run only when `PUSH=1`. A build that pushes nothing no longer writes `PUSHED_TAGS_LOG`. That log tells the multi-arch stitch which tags to move, so it must hold only tags that were really pushed. The cache helper lost an argument nobody used. No caller passes the second argument of `cache-args`, and an explicit tag there would bypass `CACHE_TAG`, so the amd64 and arm64 builds of that image would share one cache ref. CI sets neither `PUSH` nor `LOAD`, so CI builds do the same thing as before. Each fix has a `make --dry-run` test in `hack/common-envs_test.bats`. Closes #4499 Closes #4504 Closes #4551 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The ccp skills pass `PUSH=1` explicitly, which still works. The website docs do not mention `PUSH`, `LOAD` or `cache-args`. ### Release note ```release-note fix(build): `LOAD` and `PUSH` set in the environment are now honoured, `LOAD=1` no longer pushes unless `PUSH=1` is also set, and the Talos installer is no longer published when `PUSH=0`. ``` |
||
|
|
2af8b0c229 |
ci(build): fall back to direct, retry and cache Go module downloads (#4660)
<!-- 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 One transient error from proxy.golang.org is enough to fail an image build or the `Unit & controller tests` job today. The error is an HTTP/2 stream reset (`stream error ... INTERNAL_ERROR`), and nothing retries or falls back after it. This PR adds three things: a direct fallback, a bounded retry and a module cache. Image builds now fall back to fetching modules from their origin when the proxy fails. They get `GOPROXY=https://proxy.golang.org|direct` as a build arg from `hack/common-envs.mk`, and every Go builder stage declares `ARG GOPROXY`. With a pipe Go goes direct on any proxy error. With Go's default comma it does that only on a 404 or 410. The value comes from `IMAGE_GOPROXY` and not from the host's `GOPROXY`, because buildkit can't reach a mirror on localhost or a `file://` proxy. Every workflow job that runs Go on the runner sets the same value as env. Explicit module downloads in Dockerfiles are retried three times with a growing pause. The loop is inline, in one form that the contract test pins. I tried a shared script from a named build context first, but buildx refuses any `--build-context` for the four Dockerfiles still pinned to the `dockerfile:1.2` frontend. The images also build from different contexts, so no single file can be copied in. Raising those pins changes how those images build and needs a real build to check, so I left it for a follow-up. Two alpine stages, token-proxy and shadowbox's `ss-server`, left the download to `go build`. The alpine golang images have no git, so the direct fallback can't fetch for them, and they now get the same download step. The runner jobs use `hack/go-mod-download.sh` with the same bound. The unit test job now starts from a module cache instead of downloading everything. It restores `GOMODCACHE` with `actions/cache`, keyed on every `go.sum` in the tree, and then prefetches all tracked modules through the retry, so a cache miss still works. A cache saved by a PR run is visible only to that PR, but one saved on the default branch can be restored by any PR ([docs](https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching#restrictions-for-accessing-a-cache)). That is why the build cache warmer on main writes the same key before its image build. This way the first run of a PR, which is the one that flakes, starts warm. The warmer restores only an exact key, so old module versions don't pile up, and its Go steps can't fail the image build. Both runner labels are Linux, so the key matches. Both also check out under `/home/ubuntu/_work`, so `go env GOMODCACHE` should give the same cache path, but I have not seen it on the warmer's runner yet. The release jobs in `tags.yaml` and `promote-rc.yaml` get the GOPROXY env only, no cache. `hack/go-module-fetch-contract.bats` covers all three. Each test is red on main, and I checked each guard with a mutation. Closes #4591 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note ci(build): Go module downloads in image builds and CI jobs now fall back to direct fetching and retry on transient proxy.golang.org errors. The unit test job reuses a module cache written from main. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Reliability** - Go module downloads now fall back to direct access when the public proxy encounters errors. - Builds and automated checks retry failed module downloads, helping them recover from temporary network issues. - **Performance** - CI caches Go modules to reduce repeated downloads and speed up builds and tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9c025fa2b1 |
fix(installer): skip the packages artifact push and stamp with PUSH=0 (#4663)
## What this PR does A local build with `PUSH=0` no longer publishes the packages artifact. Before this change, `make image-packages` in `packages/core/installer` ran `flux push artifact` no matter what `PUSH` was set to, and then wrote the pushed digest into `values.yaml`. The `chart` target in the same Makefile already respects `PUSH`. With `PUSH` set to anything other than 1, the target now skips both the push and the digest stamp. It prints one line saying so and leaves the committed `platformSourceUrl` and `platformSourceRef` as they are. It still writes `packages/core/platform/.build-revision`, which is gitignored. I chose to skip the stamp instead of computing a local digest, because every reader of that value needs a digest that exists in the registry. The chart passes `platformSourceRef` to the operator as `--platform-source-ref`, and Flux then pulls `cozystack-packages@<digest>` from the registry. The release scripts `verify-promoted-packages.sh`, `promote-packages-artifact.sh`, `stitch-multiarch.sh` and `nightly-mirror.sh` also treat it as a registry digest. A digest of an artifact that was never pushed points at nothing. On main an unpushed run would not even get that far, since the recipe dies at `test -n "$DIGEST"`. The gate is a make-level `ifeq ($(PUSH),1)`, so it reads whatever `PUSH` ends up as after `hack/common-envs.mk`. On main that means `PUSH=0` on the command line. I also applied the `common-envs.mk` change from #4654 locally, which takes `PUSH` from the environment and makes `LOAD=1` default it to 0. With that applied, a dry run shows no push and no stamp for `PUSH=0` in the environment, for `LOAD=1`, and for `PUSH=0` on the command line, while `LOAD=1 PUSH=1` and an empty `PUSH` still push. CI is not affected, because no workflow sets `PUSH` or `LOAD` and `PUSH` stays at 1. I dry-ran the three CI invocations of this Makefile before and after the change, on main and again with #4654 applied: - `tags.yaml`, release stitch step: `make -C packages/core/installer image-packages chart` with `REGISTRY`, a release tag as `IMAGE_TAG`, and `PUSHED_TAGS_LOG` - `pull-requests.yaml`, installer step of a same-repo PR: `make -C packages/core/installer image` with a `pr-N-sha` tag, `PLATFORM=linux/amd64`, `BUILDER`, and an empty `OCI_EXPORT_DIR` - `build-main.yaml` and `build-release.yaml`: `make build`, which runs the installer `image` target with defaults ```text $ diff --recursive before/ after/ # tags.txt pr.txt build.txt $ echo $? 0 ``` There is one side effect. `OCI_EXPORT_DIR` forces `PUSH := 0`, so running `image-packages` with it set no longer pushes, where before it would have tried an anonymous push. No CI path does that. The fork PR build already runs only `pre-checks image-operator` for the installer, and the `make -n ... image OCI_EXPORT_DIR=/oci` scan in `e2e-fork.yaml` greps only the `.oci.tar` names, which this change does not touch. The new test dry-runs the target in both modes. With `PUSH=0` there must be no `flux push artifact` and no `platformSourceRef` stamp; with `PUSH` at its default, both must be present. The first test fails on main. I mutated the guard three ways, and each time only the matching test went red: - condition forced true: the `PUSH=0` test fails - compared against 2: the default-path test fails - `endif` moved up so only the push is gated: the `PUSH=0` test fails on the stamp check One limitation is left as a follow-up. The test's `env -u PUSH -u LOAD` only isolates it once #4654 lands, because on main a file assignment wins over the environment anyway. It does not cover `PUSH=0` coming in through `MAKEFLAGS`, so `make bats-unit-tests PUSH=0` would fail the default-path test here and the ordering check in `hack/stitch-multiarch_test.bats`. Nothing runs the suite that way today. If that changes, passing an explicit `PUSH=1` on the default-path `make` lines, or unsetting `MAKEFLAGS` the way `hack/bats-unit-keep-going.bats` does, would fix it. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The website's `development.md` tells developers to run `make image-packages` with `REGISTRY` set to their own registry. That is the default `PUSH=1` path, and it works as before. Neither the website nor ccp mentions `PUSH=0` for this target. Closes #4656 ### Release note ```release-note fix(installer): `make image-packages` no longer pushes the cozystack-packages artifact or stamps its digest into values.yaml when PUSH=0. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * You can run `image-packages` with `PUSH=0` to skip publishing the artifact and keep the committed platform source reference unchanged. * When pushing is enabled, the command publishes the artifact and updates the platform source URL and digest-based reference. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
33cf6972bb |
test(qdrant): exercise the tenant trust anchor end to end (#4701)
## What this PR does The qdrant e2e suite now runs with TLS on and checks the tenant trust anchor end to end. Before this, the fixture left `tls` unset with `external: false`, so the chart rendered no CA and no `TenantProjection`, and nothing in e2e reached the controller or `tenantsecrets`. Setting `tls.enabled: true` turns TLS on without the LoadBalancer that `external: true` would allocate. The first new step checks that the anchor is published. The `qdrant-test-ca` sentinel has to report `Ready=True/Projected`, and `qdrant-test.tenant-ca` has to carry both the `tenant-ca` and `tenantresource=true` labels. The projection must hold only `ca.crt` with the same fingerprint as the chart's CA, and `tenantsecrets` must serve it. The key-bearing `qdrant-test-ca` and `qdrant-test-tls` must answer NotFound there, and any other error fails the step. The second step checks that the anchor is what decides trust. It reads the anchor through `tenantsecrets`, port-forwards the qdrant Service and runs `openssl s_client` with that file as the only CA, hostname check included. A CA with the same subject and another key must fail, and so must the runner's default trust store. A handshake failure counts only when the output shows a verification error, so a refused connection does not pass as a rejection. Plaintext HTTP must get no response. Curl exits 7 and 28 fail the step, and a second verified handshake afterwards proves the forward was still alive. TLS does not break the existing steps, because none of them talk to qdrant. `create-qdrant`, `wait-helmreleases-ready`, `verify-statefulset-and-storage` and `verify-pvc-reclaim-on-delete` only read Kubernetes objects: no `http://`, no port 6333 or 6334, no curl or exec, and the suite has no backup round trip. The release now also renders two Issuers, two Certificates and the sentinel. The chart switches its probes to HTTPS with `enable_tls`, so `readyReplicas: 1` now covers a pod serving TLS. Local checks: `chainsaw lint test` (v0.2.15) says the file is valid. `helm template` with the fixture's spec renders 2 Certificates, 2 Issuers and 1 TenantProjection, and none without TLS. I ran the handshake logic against a local `openssl s_server` with a CA and leaf shaped like the chart's. The anchor passes, the wrong CA and the default store fail on verification, a wrong hostname fails, plaintext gets `000`, and a closed port is not counted as a rejection. A sentinel with a missing source (`SourceNotFound`) is not tested here, the reconciler unit tests cover it. Review notes I left as they are: - Through `kubectl port-forward` the local socket is always accepted, so curl exits 7 and 28 only catch a dead forward process. A false pass would need qdrant to answer plaintext and a pod-side blip on that one request at the same time. - qdrant may reset the socket on the plaintext request, and some kubectl versions drop the whole port-forward on a pod-side error. Then the handshake after it turns the step red, not green. If CI shows `lost connection to pod` there, the plaintext check should get its own port-forward. - The suite's `catch` has no `describe` of the `TenantProjection`. The sentinel assert prints its conditions when it fails, but a red Secret assert or script after it would not. - The vendored chart's `servicemonitor.yaml` hardcodes `scheme: http`, so metrics scraping fails for any qdrant with TLS on. That was already the case before this PR, and the suite does not check metrics. Tracked in #4703. cert-manager does not set owner references here, so `qdrant-test-ca` and `qdrant-test-tls` stay in the namespace with their private keys after the app is deleted. That is tracked in #4704. The nats half of #3706 is tracked in #4702. Closes #3706 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `hack/e2e-chainsaw/qdrant/`, which no entry in the trigger map covers. ### Release note ```release-note test(qdrant): e2e now runs qdrant with TLS on and checks that a client trusting only the tenant-published CA reaches it, while a wrong or missing anchor and plaintext HTTP are refused. ``` |
||
|
|
7ea71042b7 |
ci(e2e): add a daily isp-hosted install lane on kind (#4712)
## What this PR does
This adds a daily CI lane that installs the `isp-hosted` platform
variant through the `hosted` installer variant. No lane ran either of
them before. The PR, nightly and tag lanes all install `isp-full` on
Talos, where Cozystack brings its own CNI and storage, and that is how
#3376 shipped unnoticed.
What `isp-hosted` changes is what it leaves to the host. The system
bundle swaps kube-ovn and Cilium for a `noop` networking Package and
drops multus, LINSTOR, MetalLB, cozy-proxy, snapshot-controller and the
rest of the shared data-plane set. The iaas bundle refuses to render.
Migrations are not a difference, `isp-full` enables them too.
So the lane runs on kind. kindnet, kube-proxy and local-path come from
the cluster, and there is no Cilium, no LINSTOR and no `replicated` or
`local` StorageClass. It installs the latest GHCR nightly instead of
building anything, at 05:00 UTC after the nightly mirror publishes, and
on `workflow_dispatch`. There is no pull request trigger, because a run
would install the nightly images and not the PR's.
A run passes when every HelmRelease is Ready, `cozystack-platform` and
`tenant-root` included. It also checks that the platform emitted the
`noop` networking Package and none of LINSTOR, MetalLB, multus or
KubeVirt, and that the cluster has no Cilium CRD and no `replicated` or
`local` class.
The lane reuses the e2e sandbox container for tooling and
`collect-report`, attached to the kind network.
`hack/isp-hosted-lane_test.bats` pins the wiring: the Package selects
`isp-hosted`, the install uses `variant=hosted`, the kind DNS domain
matches `networking.clusterDomain`, and kind is pinned by checksum and
digest. Six of its seven tests were red before the implementation
existed.
`hack/sandbox-runner-headroom.bats` gets a named exemption. That check
treats any workflow touching `packages/core/testing` as a lane that
boots the QEMU guests, and this one boots none. The exemption is checked
after the job match, and removing it turns the test red again.
The pins come from the kind v0.32.0 release: the binary sha256
`50030de2…` from its `kind-linux-amd64.sha256sum`, and
`kindest/node:v1.33.12@sha256:3f5c8443…` from its release notes.
v1.33.12 is the Kubernetes version the Talos lanes use, so the variant
is the only thing that differs. All three are plain `env` values in the
workflow, where a Renovate custom manager can match them.
### Real runs
The first real run on this branch,
[36961209755](https://github.com/cozystack/cozystack/actions/runs/36961209755),
went red on `tenant-root`:
```text
tenant-root False Helm install failed for release tenant-root/tenant-root with chart tenant@0.0.0+629b61300902: unable to build kubernetes objects from release manifest: resource mapping not found for name: "allow-external-communication" namespace: "tenant-root" from "": no matches for kind "CiliumNetworkPolicy" in version "cilium.io/v2"
```
This is #3387 reproduced live: `packages/apps/tenant` renders Cilium
policies on a cluster without Cilium. The other releases went Ready,
except `cozystack-basics`, which was still in an upgrade at the last
listing. The fix landed in #4710.
The run after it,
[37089699259](https://github.com/cozystack/cozystack/actions/runs/37089699259),
is green on the first attempt. It installed nightly
`0.0.0-nightly.20261003.37089458387.1`, built from
|
||
|
|
14eab3cfe3 |
fix(backups): restrict the kafka-metadata CLI Pod and correct demo personas (#4695)
## What this PR does
This fixes two problems in the Kafka and NATS backup demos under
`examples/backups/`.
The Kafka metadata demo ran its CLI Pod without any securityContext, so
a namespace that enforces the restricted Pod Security level would reject
it. The Pod now gets the same restricted context as the CLI Pod in the
topic-data demo: `runAsNonRoot`, the `RuntimeDefault` seccomp profile,
no privilege escalation, all capabilities dropped. The new bats case
reads the overrides passed to `kubectl run` and fails if any of those
fields is missing or the patch is not sent as `--override-type=json`. It
is red on main, and it goes red on each of seven single-field mutations
of the patch while the other five cases stay green.
One known limit: `runAsNonRoot` needs an image that runs as a non-root
user. The default Strimzi image runs as UID 1001 and is fine. If
`KAFKA_IMAGE` is overridden with an image that runs as root, the kubelet
refuses to start the container, and the helper fails only when its 5
minute `kubectl wait` times out.
The kafka and nats walkthroughs told a tenant to run scripts that the
tenant roles forbid. Steps 03, 04, 06, 07 and `cleanup.sh` were in the
tenant column, and the scenario docs and several script comments had the
tenant create the backup Secret. Those rows now say admin and name the
part a tenant admin can do on its own, and the scenario docs say the
same. After this change the persona column in both tables matches the
tenant roles. kafka-metadata has no persona table, so I didn't change
anything there.
These are the tenant rules on pods and backup objects, plus one example
of the per-app secret rules. Each app chart's dashboard Role allows
`get`, `list` and `watch` on a few named Secrets and no other verb on
secrets. Nothing aggregated into the tenant roles grants `pods/exec`,
`pods/log`, `helmreleases`, `objectstorage.k8s.io`, `statefulsets` or
`kafkas.kafka.strimzi.io`.
```yaml
# packages/system/cozystack-basics/templates/clusterroles.yaml:100-112 (cozy:tenant:view:base, every tenant level)
- apiGroups:
- ""
resources:
- pods
- services
- persistentvolumes
- endpoints
- events
- resourcequotas
verbs:
- get
- list
- watch
# packages/system/cozystack-basics/templates/clusterroles.yaml:238-246 (cozy:tenant:admin:base)
- apiGroups: [""]
resources:
- pods
- services
- persistentvolumes
- endpoints
- events
verbs:
- delete
# packages/system/backup-controller/templates/tenant-clusterroles.yaml:35-44 (cozy:backups:admin)
- apiGroups: ["backups.cozystack.io"]
resources:
- plans
- backupjobs
- restorejobs
verbs:
- create
- update
- patch
- delete
# packages/apps/bucket/templates/dashboard-resourcemap.yaml:6-14 (bound at the tenant "use" level)
- apiGroups:
- ""
resources:
- secrets
resourceNames:
{{- range $name, $user := .Values.users }}
- {{ $.Release.Name }}-{{ $name }}-credentials
{{- end }}
verbs: ["get", "list", "watch"]
```
How each changed row runs into those rules:
| Row | What the script needs | Why a tenant can't |
| --- | --- | --- |
| 03 (kafka, nats) | waits on the `HelmRelease` and COSI objects, `get`
on the `bucket-<name>-backup` Secret, kafka also reads the CA Secret in
`tenant-root` | no `helmreleases` or `objectstorage.k8s.io` rule; the
bucket Role covers only `<release>-<user>-credentials` |
| 04 (kafka, nats) | waits on the `HelmRelease` and the Strimzi `Kafka`
or the `StatefulSet`, `create` on `secrets`, `create` on `pods`,
`pods/exec` | pods are `get`/`list`/`watch`/`delete` only, and nothing
grants `create` on secrets |
| 06 (kafka, nats) | `create` on `pods` and `pods/exec` for the CLI Pod,
kafka also `get` on `pods/log` for the refused restore | same pod rules,
and nothing grants `pods/log` |
| 07 (kafka, nats) | same as 04 | same as 04 |
| `cleanup.sh` (kafka, nats) | `delete` on `secrets`, on `backups`, and
on the cluster-scoped `BackupClass` and strategy | `cozy:backups:admin`
deletes only plans, backupjobs and restorejobs, and no tenant rule
covers secrets or cluster-scoped objects |
Step 05 stays a tenant step. Its `BackupJob` comes from
`cozy:backups:admin`, so in practice that means a tenant admin. The nats
demo's CLI Pod has the same missing securityContext as the
kafka-metadata one, and I left it for a separate issue.
Closes #4625
Closes #4636
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
Not a UI change.
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
fix(backups): the kafka-metadata backup demo now runs its CLI Pod with the restricted Pod Security context, and the kafka and nats backup demos now list steps 03, 04, 06, 07 and cleanup as admin steps, because they need Pod, exec, Secret and cluster-scoped permissions that tenant roles do not grant.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Security**
* Kafka metadata backup CLI Pods are recreated with restricted security
settings when they are not running or lack the required non-root
configuration.
* **Documentation**
* Kafka and NATS guides clarify which setup, restore, and cleanup steps
require administrator access and which job actions tenants can perform.
* Updated examples clarify that the demo provides required storage
Secrets and certificates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
9239555e6a |
ci(workflows): pin flux, yq and helm-unittest installs (#4645)
## What this PR does Workflows no longer run the flux install script as root. Every step that needed the flux CLI downloaded `https://fluxcd.io/install.sh` and ran it with `sudo`, so whatever that host served at the moment ran on runners that later get registry credentials. PR #4442 pinned the version, but the script was still fetched and executed. All 12 of those steps now use `fluxcd/flux2/action`, pinned to the v2.8.6 commit with `version: "2.8.6"`. The action downloads the release tarball, checks it against that release's checksums file, and adds the binary to `PATH`. Two workflows still took yq from the floating latest release, nightly and the fork e2e lane. They now use the same install the promotion steps already have: yq v4.53.3 checked against a committed SHA-256. I verified that hash against the release's `checksums` file and against the binary itself. The helm-unittest plugin was installed at whatever version was newest. The plugin embeds its own Helm, so a plugin release could change chart test results on a PR that touched no chart. It's pinned to v1.1.2 now, the version CI already runs. A runner that carries a different version gets it replaced. A new contract test scans every workflow for these three tools, because an unpinned install usually comes back by copying an older step. On main it fails on the install script, the missing setup action, the latest-release yq and the unpinned plugin. I ran one mutation per guard, and each one failed only the test meant to catch it. The existing contract tests that checked `FLUX_VERSION` inside the old steps now check for the action step. The e2e sandbox image still runs `install.sh` in its Dockerfile. That's an image build, not a workflow step, so I left it for a separate change. Closes #3619 Closes #3640 Closes #4510 ### Screenshots Not applicable, no UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only workflow tool-install steps and `hack/*.bats` contract tests. Nothing in the trigger map reaches them. ### Release note ```release-note ci(workflows): install flux through the commit-pinned upstream setup action instead of running fluxcd.io/install.sh as root, pin yq with checksum verification in the nightly and fork e2e workflows, and pin the helm-unittest plugin version. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * CI workflows now use a consistent, pinned Flux version. The yq and Helm plugin setup also verifies or enforces specific versions. * **Tests** * Added checks to ensure workflow tool versions are pinned and consistent, and that downloaded yq releases are checksum-verified. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
40638429bd |
fix(release): collect refs stored as registry/image/tag/digest keys (#4697)
## What this PR does The shared image-ref enumeration skipped refs stored the way upstream ingress-nginx stores them: `registry`, `image`, `tag` and `digest` as separate keys, with the repository in `image` and a digest that has no `@`. Two refs in the tree use this layout, the ingress-nginx controller and its default backend, both in `packages/system/ingress-nginx/values.yaml`. Every tool that reads refs through `hack/lib/image-refs.sh` missed both of them: the promote retag, the nightly mirror, the multi-arch gate, the stitch step and the promoted-packages verifier. I added a sixth YAML shape that joins `<registry>/<image>@<digest>`. It fires only on a complete `sha256:` digest, so upstream's unpinned default (`digest: ""`) and image maps without a digest emit nothing. It checks that `digest` is a string before `test()`, same as shape 3 does. The library header and `docs/agents/image-refs.md` now say "the YAML sub-shapes listed" instead of a number, so the next shape doesn't make the count wrong again. The real tree now gives 93 refs instead of 91, and the two new ones are exactly the ingress-nginx pair. The retag and mirror output does not change today, because both refs sit outside the owned registry and the ownership filters drop them. Both digests are multi-arch indexes, so `hack/verify-multiarch.sh --refs-file` with just those two passes and the release gate stays green. To show the consumers really pick the shape up, I ran both dry-runs on a copy of the tree with the controller moved under the build registry, the host being the whole `registry` value. In that copy the controller is the only build-registry ref. Before: ```text $ REGISTRY=iad.ocir.io/idyksih5sir9/cozystack hack/promote-retag.sh 1.99.0 --dry-run No cozystack-owned digest-pinned image refs found under iad.ocir.io/idyksih5sir9/cozystack/ — is this the rc's baked tree? exit 1 $ hack/nightly-mirror.sh 1.99.0 <tree>/packages --dry-run No cozystack-owned digest-pinned image refs found under iad.ocir.io/idyksih5sir9/cozystack/ in '<tree>/packages' — is this the baked main tree? exit 1 ``` After: ```text $ REGISTRY=iad.ocir.io/idyksih5sir9/cozystack hack/promote-retag.sh 1.99.0 --dry-run ▸ iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller sha256:a8ddcee59b63f6cbf93f4d3c9ffa1d1dda3c1d39497748d4484655356c68b3f6 DRY-RUN skopeo copy --multi-arch all docker://iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller@sha256:a8ddcee5… docker://iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller:1.99.0 exit 0 $ hack/nightly-mirror.sh 1.99.0 <tree>/packages --dry-run ▸ iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller -> ghcr.io/cozystack/cozystack/ingress-nginx-controller sha256:a8ddcee5… DRY-RUN skopeo copy --multi-arch all docker://iad.ocir.io/…/ingress-nginx-controller@sha256:a8ddcee5… docker://ghcr.io/cozystack/cozystack/ingress-nginx-controller:1.99.0 DRY-RUN skopeo copy --multi-arch all docker://iad.ocir.io/…/ingress-nginx-controller@sha256:a8ddcee5… docker://ghcr.io/cozystack/cozystack/ingress-nginx-controller:nightly exit 0 ``` The red test is a new case in `collect_refs_from_file understands every YAML shape`. It has a fixture in the ingress-nginx layout, with a non-string digest in front of it, and three near-misses that must emit nothing: no digest, an empty digest, and a digest without `image`. On main the positive case fails with `got: <nothing>`. Removing the rule, the registry join, the full-digest check, or the type guard on `image` or on `digest` turns that test red, and only that one. The bats suites of all library consumers pass with both the old and the new library. Two gaps remain in the tests and I left them out on purpose. No fixture covers the new rule without a `registry` key, and none pins its type guard on `registry`. Shapes 2 and 3 have the same unpinned `registry` guard on main. The overlay script has no such gap. `hack/overlay-main-images.sh` does not use the library, and its line classifier already accepts `image:`, `registry:`, `tag:` and `digest:` lines. Closes #4518 ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map against the diff. None of the changed files are in it. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(release): the image-ref enumeration used by promotion, the nightly mirror and the multi-arch gate now reads refs stored as separate registry/image/tag/digest keys, the layout of the ingress-nginx controller and default backend. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Image references can now be read from configurations that store the image name and SHA-256 digest separately, with an optional registry. * **Bug Fixes** * Incomplete or invalid image and digest values are ignored rather than producing malformed references. * **Documentation** * Documented the additional image storage format. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9196b9798e |
fix(e2e): bound the dataplane capture collector by wall clock (#4640)
## What this PR does The e2e data-plane collector limited itself by counting pods and LoadBalancers, but both of its callers stop it by the clock. `hack/cozytest.sh` gives it 600s and the Chainsaw catch gives it 300s. One affected pod could already run past the smaller limit, and the LoadBalancer capture, which runs last, was the part that got lost. I moved the collector to a wall-clock budget and fixed the other issues filed against it. - Every kubectl call now goes through `dp_run`. It cuts the call's timeout to what is left of `COZY_DATAPLANE_BUDGET` and starts nothing once the budget is spent. The default is 270s, and `cozytest.sh` passes 570s. The pod section stops 90s early, so the LoadBalancer section always gets time to start. A call the deadline cut says so, a call cut by its own bound names that bound, and a refused call says it was not started and names its section; exec blocks carry that line inside the block. The node capture and the LoadBalancer probe used to blame an unanswered lookup or a missing cni-server when the budget stopped a call. Now they name the budget (#3671). - A static test fails on any kubectl call that bypasses `dp_run` and lists the lines. It matches by command position, so a call inside a double-quoted `$(...)` is still checked (#3694). - Per-pod and per-LoadBalancer capture files join namespace and name with `_`, which neither can contain. A second run into the same directory appends, with the run separator `capture-notes.txt` already uses (#3679). - `capture_node` and `capture_lb_node` no longer write their callers' loop variables. A static test fails when a function assigns a name that a `read -r` loop or another scope owns (#3675). - The tests that shrank every timeout to one second now cut only the read under test. A `timeout` shim decides by argv, so a slow machine can't cut a read the test needs answered (#3685). - `cilium-dbg bpf lb list` runs unfiltered in every affected node's capture, not only for a failing LoadBalancer (#3689). The call-site comment in `.chainsaw.yaml` and the collector paragraph in `docs/agents/e2e-testing.md` now match the code. Every behaviour change has a test in `hack/capture-dataplane.bats` that failed on the code before it. Two parts of the issues live elsewhere. The runner stopping a bats file at its first failing test is #4620, and a conntrack read filtered by service VIP is #4623. From #4629 I fixed the items on lines this PR already changes: the node capture now gets the run separator, a failed speaker log read writes a note, the endpoint tcpdump says when it falls back to the geneve interface, and the per-read timeout knobs are validated (a zero there would otherwise look like a spent budget). The rest of #4629 stays open: `ovn-lflows.txt` and `lb-speaker-logs.txt` are still replaced on each run, and two comments still describe older behaviour. - Closes #3694 - Closes #3671 - Closes #3679 - Closes #3675 - Closes #3685 - Closes #3689 - Refs #4629 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): bound the failure-path data-plane collector by a wall-clock budget, keep its per-pod and per-LoadBalancer captures from overwriting each other, and capture the cilium service map for ClusterIP failures. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Data-plane diagnostics now follow a configurable time budget, reserving time for LoadBalancer captures and stopping operations when the budget is spent. * Capture reports distinguish failed, successful, unknown, and budget-limited checks. LoadBalancers with successful probes are skipped for heavy capture. * Repeated captures are preserved together, and limits help control LoadBalancer diagnostic collection. * Diagnostics include additional network details from relevant nodes during probe replay. * **Documentation** * Updated guidance covers capture time limits and how unanswered EndpointSlice reads are reported. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
f209c477e2 |
test(e2e): read the opensearch release history through the pinned name (#4689)
<!-- 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 opensearch suite now reads HelmRelease history through the shared
jsonpath, and a unit test fails if any Chainsaw suite goes back to a
copied literal. Before this, the suite carried its own copy of the
expression, and the upstream contract test in `internal/fluxcontract`
only checks `HELMRELEASE_HISTORY_JSONPATH`. I checked the copy against
helm-controller `api` v1.5.1, the version go.mod pins: the history field
is `history` and each snapshot has `status`. So the read works today, it
was just not pinned.
The suite passes `-o "jsonpath=${HELMRELEASE_HISTORY_JSONPATH}"` now,
which gives kubectl exactly the same argument as before. The new test
reads every suite under `hack/e2e-chainsaw/`. A suite that calls
`helmrelease_has_remediation_cycle` must fill the local it hands the
guard from the shared name, and no suite may assign that name itself. It
reuses the matchers that already pin `run-kubernetes.sh`. The test is
red on main and green with the fix.
One known limit. The second-assignment check counts over the whole file,
which fits `run-kubernetes.sh` because it runs as one shell. A Chainsaw
suite runs each `script` step in its own shell, so an unrelated
`statuses=` in another opensearch step would turn the test red with
nothing broken. That fails toward red, never toward a silent pass, so I
left it.
Closes #4550
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
test(e2e): the opensearch suite reads HelmRelease history through the shared jsonpath, which is now pinned against the upstream Flux type for every Chainsaw suite that runs the remediation guard.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Tests**
* Expanded validation across Chainsaw end-to-end suites to check
consistent HelmRelease history handling in remediation checks, including
the order and isolation of script steps.
* The OpenSearch suite now uses the shared history lookup. Empty-history
checks and remediation-cycle failure handling are unchanged, and there
is no change to end-user behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
|
b4fbaf00a2 |
test(e2e): keep gateway and redis-tls op ceilings above their own waits (#4643)
<!-- 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 Two Chainsaw script ops had a timeout smaller than the waits inside them. When the op ceiling fires first, Chainsaw kills the shell: the script never prints its own error, and a cleanup trap never runs. The gateway listener-set step could poll for 660s under a 6m op. Its cleanup trap also deleted two tenants with no limit on the wait. Each delete now runs under an outer `timeout`, because kubectl's own `--timeout` does not cover discovery or a stalled request. A tenant delete gets 300s, the same 5m that `.chainsaw.yaml` gives a delete op. The parent is deleted only after the child is gone, because deleting a parent while its child is still uninstalling wedges the parent's cleanup. A failed tenant delete fails the step with kubectl's own error, so a leaked tenant shows up here and not in a later suite. The op is now 24m, which covers the polls plus those bounds. The redis-tls CA-projection step has 30s read caps that add up to well over its 5m op. Raising the op would not hold, because several reads run in loops. So all reads now share a 240s step budget. No read starts after the budget runs out, and a read that is still running gets cut off when it does. A read that hits its bound, or finds the budget already spent, says which read it was. Two exec checks in the same step blamed the 15s redis-cli cap for any failure, including a refused exec or a missing container. They now print the exit status and what the exec printed. The step also runs under bash now, for pipefail. The sandbox's `/bin/sh` is dash, which rejects `set -o pipefail`. Chainsaw 0.2.15, the version the sandbox image installs, supports a `shell:` field on script ops. Without pipefail, a failed Secret read took base64's zero exit status and was reported as a wrong source name. The private-key check read the projection a second time, so a failed second read made it pass. It now checks the value the step already fetched. I added `hack/chainsaw-op-budgets.bats` to guard all of this. It holds the gateway op above its polls plus its delete bounds, keeps the child-before-parent order, holds the redis budget under its op with every kubectl call going through the clamped helper, and runs the redis step's script against a stubbed kubectl to check each failure message and the clamp. On main, 11 of its 12 tests fail. The one that passes is the healthy-stub control. Closes #4114 Closes #4250 Closes #4246 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches two Chainsaw suites and adds one unit test file under `hack/`. It moves or renames nothing a downstream repository reads. ### Release note ```release-note test(e2e): the gateway listener-set and redis-tls projection steps now fail on their own errors instead of being killed by the op timeout, and redis-tls reports the real cause of a failed read or exec. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added automated checks to verify gateway operations have sufficient time to complete and cleanup deletions are bounded and correctly ordered. * Expanded gateway scenarios to cover cleanup failures: if child-tenant deletion fails or times out, the parent is left in place; deletion failures are reported. * Updated Redis TLS checks with bounded read times and a shared execution limit. Failure reports include command output and status, and checks cover private-key material in the decoded projection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
1ca603a7b2 |
test(e2e): wait for the S3 gateway to accept a key before using it (#4694)
## What this PR does The bucket e2e suite fails now and then because the S3 gateway rejects a key that COSI already reports as granted. The error is `The access key ID you provided does not exist in our records.` from `mc alias set`. In job 109130131391 the BucketAccess assert finished at 21:23:43.57 and the first `mc alias set` failed at 21:23:44.82. In job 109396174094 cap1 and cap2 were accepted, and cap3 was rejected two seconds into the step. Granted access is not the same as a usable key. In seaweedfs-cosi-driver v0.3.1, `DriverGrantBucketAccess` writes the new key into `/etc/iam/identity.json` on the filer and returns. The S3 gateway in SeaweedFS 4.45 reloads that file later, from a filer metadata subscription (`weed/s3api/auth_credentials_subscribe.go`). In our chart the driver writes through `seaweedfs-filer` and the two gateways follow `seaweedfs-filer-client`, so the change may have to pass through the other filer replica first. The suite now waits until the gateway accepts the key before it writes. Every alias in both write steps goes through `await_key`, which uses the same port-forward as the writes and so asks the same gateway pod. It waits only while the answer is the InvalidAccessKeyId text, for at most 60s. Any other error fails at once, and so does an attempt cut off at its 15s cap. When the window runs out, the step fails with a message that names the BucketAccess. One probe costs at most 77s, so both steps go from a 5m to an 8m op timeout. The inner budgets now sum to 409s and 441s, and the budget comments say so. I checked the probe locally against a fake `mc`: ```text case=ok rc=0 attempts=1 elapsed=0s Added `a` successfully. case=lag rc=0 attempts=3 elapsed=5s Added `a` successfully. case=other rc=1 attempts=1 elapsed=0s FAIL: mc alias set for BucketAccess bucket-test-admin failed (exit 1) with an error other than the key not being loaded yet case=never rc=1 attempts=31 elapsed=60s FAIL: S3 gateway still rejects the key of BucketAccess bucket-test-admin after 60s of probing (the gateway never loaded it, or the driver never wrote it) ``` A hanging `mc` fails on its first attempt after 15s with exit 124. `chainsaw lint test` from v0.2.15 accepts the file. The read-only upload check gets fixed along the way. Before, an unloaded `ro-user` key also made the upload fail, and that failure passed as the expected denial. One thing I left as it is: `await_key` is copied into both steps, because chainsaw steps share no shell, and moving it to `_lib/` would run every suite on each edit. Closes #4580 ### Screenshots Not applicable, no UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note test(e2e): the bucket suite waits for the S3 gateway to accept a new key before writing with it, instead of trusting BucketAccess readiness. ``` |
||
|
|
858ca4d0a3 |
test(e2e): cover monitoring OIDC modes with a Chainsaw suite (#4678)
## What this PR does Monitoring OIDC gets live-cluster e2e coverage again. It had none since the two pre-Chainsaw BATS suites were removed without a port. This adds a Chainsaw suite in `hack/e2e-chainsaw/monitoring/` that ports what they asserted, plus the in-place switch from an inline config to a `secretRef` that nothing covered. The suite takes one Monitoring release in `tenant-test` through three modes in a row: System, CustomConfig with an inline config, and CustomConfig with a `secretRef`. It creates that release itself and never reads the tenant-root monitoring instance, so it keeps working if #4023 or #4479 turns root monitoring off. What each phase checks: - System mode: cozystack-api admits `spec.oidc` and both HelmReleases go Ready. The Grafana CR points `auth.generic_oauth` at the cozy realm, has no `role_attribute_path`, and has `skip_org_role_sync` and `oauth_allow_insecure_email_lookup` forced. The KeycloakClient is confidential and references the generated Secret, which holds 32 alphanumeric characters. The audience scope is bound to this release's clientId. There is no chart-owned `KeycloakRealmGroup`. The users-Job carries the users and the mode, and `grafana-deployment` reads the client secret from the Secret. - Switch to inline: the Grafana CR takes the tenant's map, keeps the forced keys and drops every System-only key. The users-Job reruns with the new list. The KeycloakClient, the audience scope and the client Secret are pruned, and the Deployment stops referencing that Secret. - Switch to `secretRef`: the inline `auth.generic_oauth` block is gone from the Grafana CR. The operator Secret is mounted at `/etc/grafana/oidc` on both the CR and the Deployment. This checks the mount only and does not prove Grafana loads the file, see #4679. - After the install and after each switch, neither release may show a remediation cycle. That means no `failed` or `uninstalled` snapshot, and the latest revision must be exactly 1, then 2, then 3. The revision check is there because helm-controller truncates history after the previous deployed snapshot. An upgrade that failed, rolled back and then passed on retry leaves no failed entry, only extra revisions. I did not assert `GF_PATHS_CUSTOM_INI`. I could not find anything in Grafana that reads it (#4679), and pinning it would make possibly dead wiring part of the contract. The coverage split with the render tests is this. `packages/system/monitoring/tests/oidc_test.yaml` keeps the fresh-install render of each mode, including the CustomConfig negatives (no Keycloak objects, no client Secret). This suite covers what a render cannot see: admission, the Deployment the Grafana operator builds, and what a switch between modes prunes or leaves behind. The Keycloak objects are checked as written, so creating the client in Keycloak is not asserted. Removing it is, because the operator's finalizer holds the KeycloakClient until the client is gone from Keycloak. Before the first apply the suite also waits until the tenant's `cozystack-values` carries `oidc-enabled: "true"`. The outer release copies `_cluster` into the inner release once, so a Monitoring applied right after the tenant is created can render from stale values and fail the System-mode guard for the full 15m. That is what the first CI run on this PR hit. The Keycloak CRDs are required, not probed for. The e2e install always enables platform OIDC, so a missing CRD is a broken install. The removed suites skipped in that case, and their probe could not tell an absent API group from a present one. The selector picks the suite up by name from `cozystack.monitoring-application` and `cozystack.monitoring`. A change to `system/monitoring`, `extra/monitoring`, `monitoring-rd` or `grafana-operator` now runs only this suite instead of the full run. The tenant-root Grafana bring-up stays covered by the install BATS while it exists. One selector unit test relied on `cozystack.monitoring` reaching no suite, so it now renames that source in its copy of the graph. The second commit keeps Keycloak changes on every suite that drives a Keycloak client. Before this PR a change to `system/keycloak` or `system/keycloak-operator` escalated to the full run. With the monitoring suite it would select monitoring alone, because `monitoring-application` is the only source that depends on `keycloak-operator`, and `kubernetes-latest` with its OIDC lifecycle would drop out. `src_to_suites()` now maps both to `kubernetes-latest monitoring`. The mapping has to be explicit because `kubernetes-application` has no `dependsOn` on `keycloak-operator` (#4680). A `postgres-operator` change now also reaches `kubernetes-latest`, through Keycloak's database. The selector comment and the e2e doc no longer use `system/postgres-operator` as the example of a path with a source that reaches no suite. Cost: one monitoring stack bring-up, two in-place upgrades and teardown. I expect 10 to 20 minutes on every full run. The selector change makes the CI run on this PR a full run, so the suite's own timing is in the chainsaw report. A red run can spend up to 16m on each of four waits, since each one sits a minute above the 15m timeout of the release it waits on. The suite cannot run locally, so I checked it this way: - `chainsaw lint test` at v0.2.15, the sandbox version, passes. - I ran the suite's own steps against a local envtest apiserver. Chainsaw applies the real manifests, and a stand-in for helm and the operators renders the chart from the stored Monitoring spec. With the chart intact all nine steps pass. Each of 30 single breakages of the chart or of the simulated controllers turns the expected step red. - `hack/select-e2e_test.bats` and `hack/select-install_test.bats` pass. The new Keycloak selection case fails without the mapping and when `kubernetes-latest` is dropped from it. A browser login through Keycloak is out of scope, same as in the removed suites. Left for later: - KeycloakClient and scope `status` after the operator creates them in Keycloak is not checked. - `hack/e2e-chainsaw/README.md` lists suites by hand. I added `monitoring`, but `backup`, `kafka-metadata` and `kubernetes-proxmox` were already missing. - The `operator diff selects all dependent app suites` selector test could also check `monitoring`. - The comment on the Keycloak mapping says both suites wait for the finalizer to remove the client. The monitoring suite does wait. `kubernetes-latest` only reads once after its upgrade. - The ready checks compare `observedGeneration` with `metadata.generation` through `contains()`. Both are fields of the same object, not a number literal, so the v0.2.15 literal gotcha should not apply, but no live run has passed this suite yet. - The inline fixture carries no `enabled: "true"`, so that phase checks a disabled OAuth block. The asserts do not depend on it. Closes #4051 Closes #3785 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added end-to-end coverage for Monitoring installations using system OIDC, inline custom configuration, and Secret-based configuration. The tests verify readiness, rendered resources, configuration cleanup, and upgrade history. * Updated test selection so Keycloak-related changes also run the Monitoring suite. * **Documentation** * Added Monitoring to the documented Chainsaw suite list and clarified how changed paths determine test coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
6e4c9c4b6a |
fix(ci): harden the PR labeler's title parsing and map recurring scopes (#4666)
## What this PR does Fixes three problems in `.github/workflows/pr-labeler.yaml`. Each one either killed the job or put a PR in the wrong area, and none of them showed up anywhere. - Both maps are now looked up with `Object.hasOwn`. A title typed or scoped `constructor`, `toString`, `hasOwnProperty` or `__proto__` used to resolve to an `Object.prototype` member and crash the step on `l.startsWith`, which left the PR with no labels at all. - Every leading `[Backport ...]` prefix is now stripped, not only the first one. With two prefixes, the second was read as a bracket scope, so the real scope and the `kind/*` label were both lost. The `area/release` label hid the problem by keeping `area/uncategorized` from firing. - Scopes that recur three or more times over the full PR history now map to an area. So does each area's own name (`networking`, `storage`, `database`, `virtualization`, `testing`, `dependencies`). Scope-to-area decisions: - `area/database`: `database`, `etcd-operator`, `opensearch`, `opensearch-operator` - `area/networking`: `networking`, `kubeovn`, `multus`, `kilo`, `hetzner-robotlb`, `ouroboros` (it rewrites the CoreDNS Corefile to fix hairpin NAT, so it is networking, not platform) - `area/virtualization`: `virtualization`, `vm`, `kubevirt-operator`, `kubevirt-cdi`, `kubevirt-instancetypes` - `area/platform`: `operator`, `cozystack-operator`, `controller`, `cozystack-controller`, `cozystack-basics`, `flux-shard-operator`, `migrations`, `scheduler`, `cozystack-scheduler`, `reloader`, `bootbox`, and `lineage`. I put `lineage` here on purpose: no existing label covers it more precisely, and it is too narrow for an area of its own. - `area/storage`: `storage`, `piraeus-operator`, `backupstrategy-controller` - `area/release`: `changelog`, `changelogs` - `area/ci`: `workflows`, `labeler` - `area/dependencies`: `dependencies`, `renovate` - `area/api`: `registry` (the cozystack-api storage layer) - `area/dashboard`: `dashboard-controller` - `area/monitoring`: `monitoring-agents` - `area/keycloak`: `keycloak-configure` - `area/testing`: `testing` - New `area/dx`, added to `.github/labels.yml`: `dx`, `hack`, `cozypkg`. These are developer tooling (hack/ scripts, package Makefiles, cozypkg). `area/build` covers image builds and `area/testing` covers tests, and neither describes this work. Left unmapped on purpose: - `http-cache`: the application was removed from the tree. - `openbao`: already mapped on main. - `apps`, `system`, `docs`: catch-all scopes that don't point to one area. - `fix`, `bugfix`: malformed titles. - `security`, `rbac`, `governance`, `codeowners`, `maintenance`, `images`, `readme`, `telemetry`, `cluster-autoscaler`: no single area fits any of them. I did not add spelling normalisation. `kubeovn` got its own row, since it is the only alternate spelling that recurs. A new contract test, `hack/pr-labeler-contract.bats`, pulls the workflow's own `script:` block out of the file and runs it under node with stubbed `context`, `core` and `github`, so it tests the shipped code, not a copy. On main it is red on the prototype titles (`THROW: l.startsWith is not a function`), on the double backport (only `area/release`), on the newly mapped scopes and on the area-name scopes. It also checks that every map target exists in `labels.yml` and that no key is written twice. The suite fails rather than skips when node is missing. The older node-based suites skip, so this is the first one that needs node on the unit-test runner. Some things stay out of this PR. The scope table in `docs/agents/contributing.md` is a summary. It has no row for `area/dx` and does not mention that an area's own name works as a scope. Separately, the singular `migration` still maps to `area/virtualization` as it did before, while the new `migrations` maps to `area/platform`. A backport PR whose scope is unmapped still never gets `area/uncategorized`, even with a single prefix. The `area/release` added by the strip satisfies the fallback check. This behaviour is the same on main and is not changed here. The test that checks map targets against `labels.yml` passes by itself if the map literal cannot be extracted. The duplicate-key and area-name tests go red in that case, so the suite as a whole still fails. Closes #4125, closes #4124, closes #4122 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(ci): the PR labeler no longer crashes on titles scoped like Object.prototype members, handles nested backport prefixes, and maps more recurring scopes, including a new area/dx. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added the `area/dx` label for issues and pull requests related to developer tooling. * **Improvements** * Pull requests can now be categorized using a broader set of scope aliases. * Repeated leading backport prefixes are recognized and route the pull request to the release area. * Label mapping now avoids treating inherited property names as valid mappings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
02fbe76418 |
fix(ci): close three blind spots in the e2e test selectors (#4658)
<!-- 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 e2e selectors miss Chainsaw suites that have no `chainsaw-test.yaml` or `.yml` directly in their directory. Chainsaw v0.2.15 also runs a directory that holds only numbered step files such as `01-install.yaml`, and its discovery walks every subdirectory of the path the runner hands it. A suite in either layout ran only when nothing was selected. No narrowed selection named it, the full-suite list left it out, and `select-install.sh --validate` never asked for its mapping. Now `hack/select-e2e.sh` and `hack/select-install.sh` list a top-level directory as a suite when Chainsaw would load a test from it or from anywhere below it. No suite in the current tree uses either layout, so today's suite list stays the same. A file placed directly in `examples/backups/` (a shared `common.sh`, for example) selected nothing, because it fell through to the inert `examples/` rule. It now escalates to the full suite, same as `examples/backups/_lib/`. `README.md` there stays inert through the `*.md` rule. The `test-chainsaw` comment in `packages/core/testing/Makefile` said Chainsaw discovers every `chainsaw-test.yaml`, which was too narrow. I fixed it, and the matching sentences in `docs/agents/e2e-testing.md` too. Every behaviour change has a bats test that fails against the scripts on main. Known gaps I left for a follow-up, none of them a wrong answer today: - The `full_suite_list` helper in `hack/select-e2e_test.bats` repeats the selector's `find`/`sed`, so a bug in that shared pattern would pass every full-suite assertion. The step-file and nested tests compare against literal lists, which covers the new layouts. - The selectors are a little wider than Chainsaw: a broken `chainsaw-test.y*ml` symlink, or one with no Test in it, still lists its suite. That errs toward running more. - A few test comments describe what used to happen rather than why the rule exists. Closes #4630 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(ci): the e2e test selectors now see Chainsaw suites made of numbered step files or nested below a suite directory, and run the full suite for an edit to a file placed directly in examples/backups/. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * E2E suite discovery recognizes supported manifest and numbered-step files at any depth and maps them to the top-level suite. * Changes to files directly under `examples/backups/` trigger full-suite selection. * Suite selection reports an error if discovery fails or finds no suites. * **Documentation** * Clarified supported suite layouts, disabled suite-file handling, discovery and escalation rules, and per-path coverage guidance. * **Tests** * Added coverage for nested suites, numbered steps, unmapped suite layouts, and backup-path selection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9485f9118b |
test(tests): make negated BATS assertions able to fail (#3790)
## What this PR does
A hundred and twelve assertions across twelve BATS suites begin with
`!`. A hundred and seven of them pass no matter what they find. POSIX
exempts a command whose return value is inverted with `!` from `set -e`,
so the failing status is dropped where it is produced, and
`hack/cozytest.sh` closes each test function with `return 0`, so it
never resurfaces as the test's own status either. The other five are
followed by `|| { ... exit 1; }`, which reads the inverted status and
does work; they are rewritten too, because telling the two apart needs
the tail of every statement parsed, and a guard that guesses wrong there
is green on a real one.
Switching to the `bats` binary does not fix the hundred and seven. Its
ERR trap carries the identical exemption: `bash -c 'set -eE; trap "echo
TRAP" ERR; ! true; echo REACHED'` prints `REACHED`, runs no trap and
exits zero. The only negated assertion `bats` reports is one standing
last in the test body, where the inverted status happens to be what the
function returns. Two of the hundred and seven stood last; the rest did
not.
What was lost is specific rather than diffuse. `!` is how you spell
"this must not appear", and an absence is what no other assertion in a
test covers. The affected suites assert that a migration ran nothing
destructive, that no image was copied, that a flag was not passed, that
a backup cleanup never flushed the master, that an object does not
exist. On that half of their contract they held for any input.
Each site is now written as `if cmd; then echo "FAIL: ..."; false; fi`,
the form already used in the files where this had been noticed one at a
time, and each carries a message naming what must not have happened.
`hack/bats-no-negated-assert.bats` scans every `hack/**/*.bats` and
refuses the old form, so a new one cannot be added silently. Inverted
conditions (`if !`, `while !`, `until !`) are untouched, because `set
-e` is not what decides a condition. `[ ! -f x ]` is untouched for a
different reason and remains an ordinary failing assertion: the `!`
there is an operator of `test`, not negation of a command, so nothing
exempts its status.
The guard pins its own premise instead of describing it, on both sides.
One test asserts the runner still passes a bare negated assertion; a
second, that neither `set -e` nor the ERR trap sees an inverted status;
a third, that the prescribed `if cmd; then ...; false; fi` does fail,
under both runners, and *at* the form rather than somewhere before it,
since a run that died early would satisfy a bare status check and pin
nothing.
Every rewrite here rests on the third test. If `false` inside a `then`
body ever stopped propagating, the first test would still pass, because
a negated assertion would still be vacuous, while every converted
absence went silently green. A ban that prescribes a replacement has to
test the replacement too.
Every claim the guard makes is pinned by removing the thing it claims.
The recursion is exercised *through* the audit function against a
fixture in a subdirectory, not through a second copy of its `find`
expression: a copy keeps passing while the function loses its recursion,
and an audit that returns empty reads as a clean tree. Recursion matters
because `BATS_UNIT_FILES` is a non-recursive `wildcard`, so `make
unit-tests` never runs a suite placed in a subdirectory.
`hack/e2e-apps/` held such suites until they moved to Chainsaw and is
gone now, which is why the test pins against a fixture and not a real
directory.
The rule's surface was checked in the other direction too. The shape
appears in no `hack/**/*.sh` script as an assertion: the one hit,
`hack/testdata/install-wait/call-shapes.sh`, is fixture text that
`hack/run-kubernetes-install-wait_test.bats` parses, not a command
anything runs. It appears in no Chainsaw `script:` step either, so
`.bats` is the whole surface rather than the part that was convenient to
guard.
The scan is lexical, so its header names the statement boundaries it
owns rather than implying it understands shell: `;` `&` `&&` `||` `{`
`(`, the `)` closing a case pattern, and the words `then` `do` `else`,
each measured against the runner before being added. A keyword may
legally be preceded only by whitespace or by `;`, since every other
separator in front of `then`, `do` or `else` is a shell syntax error,
checked with `sh -n` rather than recalled. The scan therefore admits
exactly those two, and that part of the class is closed rather than
patched one spelling at a time. All three keywords are vacuous after
either spelling, so missing one is a silent miss, the single direction
this scan must not err in, given that its whole design trades noise for
safety. For the same reason it takes `!(cmd)` with no space before the
subshell, which is vacuous on dash and bash alike, and reads through a
`time` or `time -p` prefix, which is vacuous under `bats` and under
cozytest wherever `/bin/sh` is bash.
Five over-counts remain and are deliberate, each named in the header: `(
! cmd )` does propagate its inverted status and is reported anyway; so
would a `!` opening a statement after a redirection `&`; `x=$(true || !
false)` is reported although the `||` reads the inverted status; a
condition continued by a trailing `&&` instead of a backslash is
reported, because the strip only recognises a condition whose end it can
see; and `time ! cmd` is reported although on a dash `/bin/sh`, which
has no `time` keyword, it runs an external `time` that fails. Each costs
one rewritten line, where the opposite error costs a silent pass. They
are enumerated rather than engineered away on purpose, because a removed
error leaves no trace while a listed one tells the next reader what they
are looking at. The scan also errs *quiet* in at least two places, named
here rather than in the header. A file cut off midway through a line
continuation loses its last buffered statement, but that input is not a
runnable test, since `sh -n` rejects it with `unexpected end of file`.
And `time -- ! cmd`, which bash accepts, is vacuous under `bats` and not
peeled by the scan; nothing in the tree uses it, and I stopped adding
spellings there rather than chase every prefix bash allows. The test
naming the shapes claims only what its fixture holds. A lexical scan
over shell cannot promise it has enumerated the language, and a name
that promised it would be false whether or not a gap existed today.
The branch name is narrower than the subject and I am leaving it as it
is rather than churn it. This is not a defect in `hack/cozytest.sh` that
moving to the `bats` binary would fix. It is a POSIX rule both runners
inherit, and moving to `bats` would close two of the hundred and seven
sites.
One suite went red for real once its assertions could fail.
`seaweedfs-guard-parity` forbids four mutable discriminators by grepping
each chart, and both charts carry a template comment block arguing why
those signals were rejected, so the grep matched the argument and would
have reported a chart for documenting the rule it follows. The charts
are correct; the assertion was not. It now strips `{{/* ... */}}` before
matching. Two details there are load-bearing and neither is obvious. The
strip removes the comment *span*, not the line that opens it: dropping
the line is shorter and silently loses a discriminator sharing a line
with a comment, which is a chart classifying on a forbidden signal and a
test reporting that it does not. And the strip reports when the file
ended inside a comment, and the checks refuse to read its output when it
did, because every `{{/*` is read as an opener, so one inside a quoted
string (these charts carry long `fail (printf ...)` messages) would
swallow the rest of the file and leave the checks reading nothing.
One commit widens six comments that explained this trap as applying to a
"negated pipeline". It is not about pipelines, and the narrower wording
invites the conclusion that the other shapes are safe. Comment text
only; each of those files already uses the working form.
The last commit makes the one absence check in
`hack/e2e-install-cozystack.bats` that already used the `if` form tell a
missing object from a failed read. It asserted that the version
configmap was gone on `kubectl get`'s exit status alone, and that status
is 1 both for a NotFound and for an api-server the test cannot reach.
### Verification
Every unit suite this branch touches was run through `hack/cozytest.sh`
on the current base and is green. `migration-seaweedfs-db-adopt` passes
15 of 15 on macOS only when its temp directory sits under `$HOME`. Its
container cannot write into a bind mount under `/var/folders`, and
`/usr/bin/mktemp -d` ignores `TMPDIR`, so the run needs GNU `mktemp`
first on `PATH` as well. Measured on Linux before the rebase, it passed
15 of 15 as is.
The six sites that reached main after this branch was first written, in
`redis-backup-strategy` and `proxmox-e2e-fixture_test`, were each shown
to be able to fail: with the condition inverted, the suite goes red at
that assertion and prints its own `FAIL:` message. The nine sites in
`migration-seaweedfs-db-adopt` carry a two-sided check as well: with the
pattern swapped for one the same test proves is present, the rewritten
form fails the suite and the original `!` form passes it.
The guard itself is green under cozytest with bash and with dash as
`/bin/sh`, under the `bats` binary, and with mawk, gawk and BSD awk.
Each branch of its separator class and of the `time` and `!(cmd)`
handling was removed in turn, and each removal turns the shapes test
red.
`e2e-install-cozystack.bats` is an e2e suite and runs only in the E2E
workflow, so its one changed absence check is not exercised locally.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff file by file. The change touches
eighteen existing BATS files, adds one, and adds a convention to
`docs/agents/e2e-testing.md`. It does not touch
`hack/e2e-prepare-cluster.bats`, which is the node contract
`ansible-cozystack` and `talm` restate; it moves and renames nothing
under `hack/` and changes no make target's behaviour, which is what
`ccp` gates on; and it touches no package, schema, enum, default, CRD,
label, annotation or metric.
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### Release note
```release-note
NONE
```
Closes #3565
|
||
|
|
6248d458ad |
fix(platform): keep the legacy etcd-operator stopped after etcd adoption
Migration 50 scales the legacy etcd-operator to 0, adopts every legacy etcd.aenix.io cluster with etcd-migrate and then scales the operator back to 1. At that point it is still the legacy operator, and it has nothing left to manage. Until a tenant's etcd HelmRelease reconciles the new chart, Flux can upgrade it with the old one, which re-creates the legacy EtcdCluster that etcd-migrate just orphan-deleted. The legacy operator then reconciles it, re-creates the legacy StatefulSet and rewrites the legacy etcd-headless Service, dropping the owner references etcd-migrate pointed at the adopted EtcdMembers. When the new chart prunes the legacy EtcdCluster, the garbage collector deletes the Service with it, and the adopted members' etcd-<i>.etcd-headless names stop resolving. After a successful adoption the operator now stays at 0 until the etcd-operator HelmRelease replaces the Deployment with the v1alpha2 operator later in the same upgrade. A failed or interrupted adoption still restores it through the EXIT trap. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
|
2df578bffd |
test(e2e): read the opensearch release history through the pinned name
The opensearch suite wrote the history jsonpath out as a literal, so the contract test that runs HELMRELEASE_HISTORY_JSONPATH against the upstream Flux type never saw the expression this suite hands kubectl. A field rename upstream would have reached it only as an empty read. The suite now takes the shared expression, and the contract test reads every Chainsaw suite that calls the guard, so a suite copying the literal, or assigning the shared name after sourcing the library, fails in the unit lane instead of going unchecked. The read is matched inside the script step that calls the guard, because every step runs in a shell of its own and a read in another step never reaches the call. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
8cb1023788 |
fix(e2e): fail on a failed release of cilium and csi as on the parent
The history guard covered only the parent Kubernetes HelmRelease. A failed action on one of the addon releases it installs left no mark the suite inspected, so a run surfaced only the downstream symptom, typically the node-join deadline, with nothing naming the release behind it. The tenant CNI and CSI are the sharp cases, since they carry a cluster-wide precondition. The guard now reads every addon release of the parent, selected from the cluster by name prefix so a release added later is covered too. A "failed" or "uninstalled" Snapshot fails the run on a release whose install strategy is RetryOnFailure: the parent, which the aggregated apiserver stamps that way, and cilium and csi. The same footprint on the other addons, which pair the default strategy with retries: -1, is printed as a note and leaves the run green. The footprint is not split into a teardown and a retried failure. In helm-controller v1.5.0, the version the platform installs, and unchanged in v1.5.1, a release with an active retry strategy never remediates: atomic_release.go returns an upgrade for a failed release before it reaches the remediation switch (lines 465-475), and the two other paths that uninstall, a changed release target and deletion, both call ClearHistory afterwards. On a RetryOnFailure release an "uninstalled" Snapshot is therefore unreachable, and the "failed" one is the only signal. A retry that later recovered still fails the run, by design, with one blind spot the statuses cannot close: a failed install that the very next attempt recovers is marked superseded by Helm, because there is no deployed release to supersede instead, and its history reads like an ordinary upgrade. The guard runs after the assertions, while every deadline a failed addon breaks ends the script earlier, so the same classification is also printed without a verdict from the node-join collector and from the exit handler. That handler is now armed at the tenant CR apply, so the management-cluster waits before the tenant answers are covered too. Every read is bounded and never folded into "no history", and the parent must read back RetryOnFailure: an empty value there means the jsonpath stopped resolving, and any other value means the apiserver stamp is gone, either of which would turn its footprint into a note. The history read takes its jsonpath from HELMRELEASE_HISTORY_JSONPATH, and the upstream-type pin accepts the bounded wrapper the read goes through. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
874e1e44b7 |
fix(backups): run the kafka-metadata demo CLI Pod as restricted
The kafka-metadata walkthrough started its long-lived CLI Pod with a bare kubectl run, so the Pod carried no securityContext and a namespace enforcing the restricted Pod Security level would reject it. The kafka topic-data walkthrough already layers the restricted context onto its CLI Pod; the metadata demo now applies the same JSON patch. A running CLI Pod left by an earlier run without that context is replaced rather than reused, since a Pod's securityContext cannot change in place. The new bats cases read the overrides kubectl run receives and fail unless both the Pod-level and the container-level fields are present and the patch is sent as --override-type=json, and check that an owned Pod without the restricted context is recreated. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
cc731e3105 |
test(e2e): keep redis-tls failures legible under the op ceiling
The CA-projection step caps each kubectl at 30s, but more capped reads run than its 5m op can cover, so a degraded apiserver could spend the ceiling and leave nothing but "signal: killed". The reads now share a step budget held under the op, and a read that hits its bound or finds the budget spent says which read it was. Two exec checks reported the 15s redis-cli cap for any failure, sending a reader after a slow redis when the exec was refused or the container was missing. They now report the exit status and what the exec printed. The step also runs under bash with pipefail: under dash a failed Secret read took base64's zero status and surfaced as a wrong source name. The private-key check reads the projection already fetched, since a second read that failed made it pass. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
a057d738d1 |
test(e2e): give the gateway listener-set op room for its own waits
The step's polls can wait 660s under a 6m op, and its cleanup trap deletes two tenants with no bound on the wait. When the op ceiling fires first, Chainsaw kills the shell: the poll that was running never reports, the trap never runs, and both tenants stay behind for every later suite. The deletes keep waiting, child before parent, since a parent deleted while its child is still uninstalling wedges its own cleanup. Each delete runs under an outer timeout with a kill grace, because kubectl's --timeout bounds only the wait after the request and not discovery or a stalled request. The parent is deleted only once the child is gone. A failed tenant delete fails the step with kubectl's error, its exit status and the tenant left behind, rather than passing with the leak. The op is sized above the polls plus those bounds, and a unit guard holds it to that sum and the cleanup to that order. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
b1085782c7 |
test(e2e): fail the isp-hosted no-Cilium check on any cilium.io CRD
The check looked for one CRD by name, so a cluster carrying only part of Cilium's CRDs passed as a cluster without Cilium, and a failed lookup read as an absent CRD and passed too. It now lists the CRD groups once, fails if that list cannot be read, and fails on any CRD in cilium.io. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
bd504e0b4e |
ci(e2e): run the isp-hosted install daily against the nightly
The lane installs the latest GHCR nightly on a kind cluster once a day, after the nightly has published. It takes no pull request trigger: a run would install the nightly's images, not the pull request's, and report on code the pull request did not change. The unit test holds the lane's wiring in place, since a lane that quietly installed another variant would test something other than its name says. The sandbox headroom check reads any workflow touching packages/core/testing as one that boots the QEMU guests; this lane starts only the sandbox container, so it is exempted by name, after the job match, and is sized like the others if it ever starts calling the guest targets. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
3e36b35795 |
test(e2e): add an install suite for the isp-hosted variant on kind
isp-hosted shipped unable to converge and nothing noticed, because no lane installs it: every e2e lane runs isp-full on Talos, where Cozystack provides the CNI and storage itself. This suite installs it the way it is meant to run, through the hosted installer variant on a cluster that brings its own CNI, kube-proxy and default StorageClass, and passes only when every HelmRelease is Ready and the variant really left out the data plane it delegates to the host. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
9873b39a2a |
test(vminstance): cover VMDisk clones from an image and from a disk
Only source.http had e2e coverage, so a regression in either clone path would ship unnoticed. The suite now clones a VMDisk from a golden image in cozy-public and then clones that disk again, checking that each DataVolume points at the expected PVC and becomes Ready, that its PVC is Bound, and that each VMDisk reports WorkloadsReady. CDI derives Ready from the DataVolume phase alone and can reach Succeeded from the claim's annotations before the claim binds, so Ready does not stand in for the binding. vm-default-images is opt-in and the e2e cluster does not enable it, so the suite creates its own small golden image under an e2e name rather than relying on the package. The second clone reuses the first disk, so the Alpine image is the only download. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
3a07885b70 |
fix(platform): resume a retried migration Job from the stamped version
The migration hook Job gets CURRENT_VERSION once, at render time, and a failed pod is retried with that same environment. Each migration that finished has stamped the cozystack-version ConfigMap by then, yet the retry started from the rendered version and ran those migrations again. Migrations are written to run once, so a re-run is not safe in general. The runner now starts from the version stamped in the ConfigMap. A failed read aborts the pod, since falling back would bring the re-run back and the Job retries anyway. The rendered value is kept only when the ConfigMap holds no integer version, which is also the case where the chart could not derive a real one from it. An all-digit stamp past the shell's integer range aborts too: its comparison fails inside an if, out of reach of set -e, and the Job would go green having run nothing, while the chart renders that stamp as 0. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
bf03c84711 |
chore(deps): move to the Kubernetes 1.37 libraries without the apimachinery fork (#4473)
## What this PR does This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, and drops the `k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and #4472; those were a stack of the same commits. The four commits are meant to be read one by one, and each builds and passes the tests on its own: 1. **Plain watches stop getting the initial-events-end bookmark.** The apiserver defaults `sendInitialEvents=true` onto any watch with an empty or `0` resourceVersion and leaves bookmarks off, and our aggregated storages took that flag alone as a WatchList request. From `k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api panics on that bookmark, so `kubectl get -w` on any cozystack-api resource would die after the bump. The marker now needs both flags, the same condition the watch cache uses. There are tests per storage and one through the real handler chain. 2. **The apimachinery fork goes.** Every apps kind shares the `Application` Go type, and stock `Scheme.ConvertToVersion` picks the first kind registered for it. The fork patched that; the apps group now encodes through a serializer that keeps the kind the object carries. A handler-chain test covers create, get, list, update, watch, apply and table for two kinds and fails without it. 3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the plugin is on by default and refuses every write until it has synced `mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and which clusters before 1.36 do not serve. Without this, every create, update and delete would fail after the bump. It is also re-applied in `Complete`, since `--disable-admission-plugins` replaces the list. 4. **The bump itself**, with the regenerated clients, openapi and strategy CRDs. Commits 2 and 4 are where the review effort goes; 1 and 3 are small. Known changes in behaviour: - Without the fork, the row objects of a Table requested with `includeObject=Object` carry the first registered app kind. kubectl asks for that only with `--sort-by`, and it sorts on fields, not on the kind. Nothing at this layer can fix it, because the handler converts through `APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks and ValidatingAdmissionPolicy still see the right kind. - The alpha PodSpec field `workloadRef` became `schedulingGroup` upstream. Once the new strategy CRDs are applied, the apiserver prunes `workloadRef` from stored strategies it serves, and their next write erases it. It only ever worked behind an alpha feature gate. The branch is rebased onto the Renovate rollup (#4447), with `go mod tidy` and codegen rerun on top of it. ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note chore(deps): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application API responses now preserve the requested resource kind across operations, including lists and watches. * Backup strategy resources support additional Kubernetes pod settings, including probe protocols, volume ownership options, and scheduling groups. * Backup strategy schemas include updated Kubernetes pod fields and descriptions. * **Bug Fixes** * Initial-event watch bookmarks are sent only when requested; plain watches continue without an ending bookmark. * Mutating Admission Policy is disabled by default, including when admission plugin options are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
9809b22280 |
fix(vm-disk): report DataVolume readiness through WorkloadsReady (#4380)
## What this PR does Nothing on a vm-disk application tells whether CDI ever finished populating the disk. Its WorkloadMonitor has no pods to count, and `Ready` follows the Helm release. So an import or clone that never completes can leave a green application over a disk nobody can use. cozystack/cozystack#2908 is one way to get there: a clone into a StorageClass that cannot place a replica where the source data sits. The expensive part there was the silence, not the stuck clone. The disk state was in the DataVolume phase all along, and nothing read it. With this change the WorkloadMonitor in cozystack-controller reads it. The monitor lists the DataVolumes its selector matches and reports not operational while any of them is in flight, Failed, Unknown or has no phase. An upload counts as in flight until it completes. That surfaces on the application as `WorkloadsReady=False` with reason `DataVolumeNotReady` and a message that names the DataVolume and its phase, for example `DataVolume vm-disk-x is ImportInProgress`. `Ready` still follows the HelmRelease, as for any other kind (`convertHelmReleaseToApplication` in `pkg/registry/apps/application/rest.go`), and for a vm-disk it no longer waits on the DataVolume at all. So the disk state has to be visible outside `Ready`, and this PR puts it where operators look by default: - `kubectl get` on any application kind gets a `WORKLOADS` column next to `READY`. It shows the `WorkloadsReady` status, or `<none>` when the application has no WorkloadMonitor. - The console badge turns amber and reads `DataVolumeNotReady` while `Ready` is True and `WorkloadsReady` is not True for that reason. A failing `Ready` still wins. The console shows only the disk case: other kinds need a separate decision, because a stopped VM legitimately has no pod and reports `WorkloadsReady=False` too. A Helm readiness gate on the DataVolume would be the obvious alternative, and it is not used on purpose. helm-controller runs that wait inside its reconcile, so an importing disk would hold one of the shard's worker slots for the whole install window, and a disk stuck for good would keep taking the slot back. cozystack/cozystack#4145 has the details. The monitor costs no slot, keeps watching after install, and covers existing disks without anyone writing to them. Without any gate, the install would still wait on the DataVolume. helm-controller's default poller falls back to a resource's `Ready` condition for kinds it has no rule for (`checkReadyCondition` in fluxcd/cli-utils `pkg/kstatus/status/status.go`), so a vm-disk install could hold a worker slot for the whole install timeout. The ApplicationDefinition therefore gets a `healthCheckExprs` entry for DataVolume with `current: "true"`, the same idiom postgres-rd uses for its ScaledObject. With it Helm never waits on the DataVolume. The entry is pinned by a helm-unittest in `packages/system/vm-disk-rd`, which gets a `test` target for it. Release settings reach a HelmRelease only when the application is created or updated, so an existing disk gets the entry on its next update. The reconciler that syncs definitions onto existing releases copies chartRef, valuesFrom and labels only. That affects any kind with release settings, so it is filed as cozystack/cozystack#4409 and not fixed here. `WorkloadsReady` covers every disk right away, since the monitor needs no write to the application. CDI copies the DataVolume labels onto the PVC it creates and makes the DataVolume its controller, so the monitor also publishes a `pvc-<name>` Workload for the disk. That Workload reads not operational while its DataVolume is not ready, even though the PVC is already Bound, so `kubectl get workloads` agrees with `WorkloadsReady`. While the DataVolumes cannot be read it keeps the verdict it has, like the monitor. The WorkloadMonitor CRD now requires at least one selector label. An empty selector would match every Pod, PVC, Service and DataVolume in the namespace. No chart renders one. When an application has several monitors that are not operational and only some of them name a cause, `WorkloadsReady` carries the named messages and reason only. A vm-disk has one monitor, so it does not hit this. ### Which phases count as ready Settled phases pass: `Succeeded`, `PendingPopulation`, `WaitForFirstConsumer`, `Paused`. A disk with `bindImmediately: false` on a `WaitForFirstConsumer` class rests in `PendingPopulation` or `WaitForFirstConsumer` until a VM consumes it. A multi-stage import rests in `Paused` until its next checkpoint, and the vm-disk chart renders no checkpoint source. These report ready while holding no data, because that is where such a disk is supposed to sit. `UploadReady` is not one of them. CDI enters it when the upload server pod is running and ready, and leaves it for `Succeeded` only when that pod succeeds (`updateStatusPhase` in `pkg/controller/datavolume/upload-controller.go` at v1.66.1), so the one phase covers a disk nobody has uploaded to and a half-uploaded one. A `source: upload` disk therefore reports `WorkloadsReady=False` from creation until `virtctl image-upload` completes. A phase the controller does not know also reads as ready, so a CDI release that adds or renames a phase cannot turn working disks red. The cost goes the other way: a renamed in-flight phase silently reads as settled. The list wants a re-read whenever the CDI version moves. It matches the named phases of the CDI v1.66.1 API, the version vendored in `packages/system/kubevirt-cdi-operator`. ### Clusters without CDI CDI comes with the IaaS bundle, while cozystack-controller runs everywhere, and CDI can be installed while the controller is running. A watch on a kind whose CRD is absent never syncs, and then the whole controller does not start. So the watch is not set up at startup. A runnable adds the DataVolume watch once the kind is served. It waits for the first reconcile, because only from then on does a new watch start at once, and then checks discovery right away and every 30s after that. It installs the DataVolume reader only after the DataVolume informer has synced within a timeout, because a reconcile context has no deadline and a List on an informer that never syncs would block every WorkloadMonitor reconcile. The watch source is started once: a controller-runtime `Kind` source adds an event handler to the informer and never removes it, so on a timeout the next check waits on the source it already has instead of starting another one. RBAC needs no change: the controller's ClusterRole already has read on everything. When the DataVolumes cannot be read, because the reader is not installed yet or the List fails, the monitor still publishes its replica counts and keeps its last DataVolume verdict, and a List error is returned so the request is retried and recorded as a `DataVolumesUnavailable` Warning event on the monitor. The status of a monitor without DataVolumes is not affected by a failing List. A disk whose verdict is already recorded does not read as ready in the window after a controller restart. A monitor with no recorded verdict yet, which is every monitor right after the upgrade that brings this change and any disk created right after a restart, reads ready until the DataVolume watch has synced. Because the watch starts on the first reconcile rather than on the next discovery tick, that window is the time to sync the DataVolume informer plus at most one 5s requeue, not a 30s poll interval. The window is not zero, since a monitor reconciled in it is published before its DataVolumes can be read. From then on, a reconcile that runs before the watch has synced is requeued after a few seconds, so the DataVolume events that queue it are not lost. A pass that changes nothing in the monitor status does not write it, so that requeue sends no status writes to the API server. Once the reader is installed, every monitor that holds a DataVolume verdict is queued once more. So a verdict whose DataVolume was deleted while the controller was down is recomputed then, not at the periodic resync. ### Tests The table test in `workloadmonitor_controller_test.go` covers the named phases of CDI v1.66.1, plus Failed, Unknown, an empty phase and a missing status. A second table checks the message and reason for an importing, a failed, a phaseless and a populated disk, and a third test lists two stuck DataVolumes in both orders and expects one message. It also covers a DataVolume outside the selector, the kind not being served, a List error that still publishes the replica counts, and a missing reader that keeps the last verdict. The watch tests check that the watch starts on the first reconcile and not before it. A source that never syncs leaves no reader and is retried on the same source, with the reconciles in between still requeued. A List failure records the Warning event, the sync window records none, and an unchanged pass writes no status. A manager that stops during the wait gets the cancellation back, and a reconcile that runs while the watch syncs is requeued without an error. A table test covers the PVC Workload: importing, populated, a PVC with no DataVolume controller or a controller of another kind or group, and the unread cases. A helm-unittest in `packages/system/cozystack-controller` pins the selector's `minProperties`. `rest_table_test.go` pins the `WORKLOADS` column, `rest_workloads_test.go` the condition message and reason, and `status.test.ts` the console badge, including a halted VM that stays green. In `hack/e2e-chainsaw/vminstance` the existing VMDisk scenarios now wait for the DataVolume to be Ready, then for `Ready` and `WorkloadsReady` on the VMDisk itself. That replaces the wait on the HelmRelease. Those asserts would pass even if the monitor ignored DataVolumes, so a new scenario imports from an `.invalid` URL. It first waits for the importer to have failed, which CDI reports on the DataVolume as `Running=False` with reason `Error`, and then for `Ready=True` with `WorkloadsReady=False` and reason `DataVolumeNotReady` on the VMDisk. Without the first wait the VMDisk assert passed in about a second, on a DataVolume that had no phase yet. The e2e runs on a fresh install only, so nothing here exercises an existing release across an upgrade. ### What it does not do #2908 lists possible fix directions, and none of them is here. The cross-class clone still never completes, it just stops being invisible. Keep the issue open. While the DataVolume watch syncs, every monitor is still requeued. Only the ones whose selector reaches a DataVolume need it, but the controller cannot tell which those are before it can read DataVolumes. Closes #4449 ### Screenshots There is no screenshot: I had no running console to take one from. The badge change is pinned by the vitest cases in `packages/system/dashboard/images/console/apps/console/src/lib/status.test.ts`: a VMDisk whose `WorkloadsReady` has reason `DataVolumeNotReady` goes amber, and a halted VMInstance with the generic reason stays green. ### Downstream repositories I walked the trigger map against the diff. cozystack/terraform-provider-cozystack is affected: its `wait_for_ready` reads only `Ready`, so on a `cozystack_vmdisk` it returns before the disk is populated once this lands. That needs a decision on the provider side, so it is an issue there. The release-time bot regenerates `packages/apps/vm-disk/README.md` into the website's virtualization pages, and `vm-disk` is in the `VMS` list that generator reads, so the website needs no follow-up. The ApplicationDefinition keeps its kind, plural, prefix and path. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/59 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(vm-disk): report whether CDI finished populating a disk in the WorkloadsReady condition, which names the stuck DataVolume and its phase. kubectl get shows WorkloadsReady in a new WORKLOADS column for every application kind, and the console badge turns amber while a disk is not populated, so a disk whose import or clone never completes no longer looks healthy. Ready of a new or updated vm-disk no longer waits for the DataVolume, so wait on WorkloadsReady to know the disk is populated. ``` |
||
|
|
0a3546e126 |
test(e2e): tell a missing object from a failed read
The assertion that the no-delete guardrail can be bypassed decides on `kubectl get`'s exit status alone. That status is 1 for a NotFound and 1 for a connection, auth or discovery failure, so an api-server the test cannot reach reads as proof the configmap is gone. `--ignore-not-found` collapses the NotFound alone into success with empty output, which leaves the status free to mean a failed read. Read the two separately: a non-zero status fails the test naming the read that did not answer, non-empty output fails it naming the object that must not be there. Capture stdout only. kubectl writes deprecation and throttling notices to stderr on a call that succeeded, and folding those into the value under test would report an object that is not there. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
cd448fdc04 |
test(bats): widen the comments describing the errexit exemption
Six suites explain why they avoid `! cmd` by saying errexit is suppressed for a "negated pipeline". The exemption is not about pipelines: a bare `! true`, a negated `[ ... ]`, one inside a `for` body or a brace group, one in a case branch, and one following `;`, `&`, `&&` or `||` are all exempt too. Anyone reading the narrower wording could conclude the other shapes are safe. One of the six carries the sentence twice, and the second copy is split across two physical lines -- `` `!` `` ending one and `pipeline)` opening the next -- so a line-oriented search for the phrase does not see it. Prose wraps; a grep for it has to fold the lines first. Comment text only; every one of these files already uses the working form the comment recommends. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
5137d98e71 |
test(bats): refuse a negated assertion across every suite
A negated assertion is the natural way to spell "this must not
appear", and an absence is what nothing else in a test covers, so the
errexit exemption lands where it is least likely to be noticed and
invisible in a green run. Switching runners does not lift it: the ERR
trap the bats binary reports through carries the same exemption, so
neither runner sees a negated assertion that has any statement below
it.
Add a suite that scans every hack/**/*.bats, subdirectories included,
and fails on a statement whose first token is `!` -- naming the file,
the line the statement opens on, and the form to use instead.
Statement boundaries are the POSIX separators, each measured against
the runner before being admitted: `;` `&` `&&` `||` `{` `(`, the `)`
closing a case pattern, and the words `then` `do` `else`. A keyword may
legally be preceded only by whitespace or by `;` -- every other
separator in front of one is a shell syntax error, checked with `sh -n`
rather than recalled -- so the scan admits exactly those two and that
part of the class is closed rather than patched a spelling at a time.
Both spellings are vacuous under the runner, so missing either is a
silent miss, the one direction this scan must not err in. For the same
reason the scan takes a `!` with no space before its subshell,
`!(cmd)`, which is vacuous on dash and bash alike, and reads through a
`time` or `time -p` prefix, which is vacuous under bats and under
cozytest where /bin/sh is bash. A scan that wants `!` and then
whitespace as the first token misses both. On a dash /bin/sh `time` is
not a keyword and `time ! cmd` fails, so there it is an over-count.
Five over-counts remain, all erring loud, each named in the header:
they are enumerated rather than removed, because a removed error
leaves no trace while a named one tells the next reader what they are
looking at.
Inverted conditions are untouched because errexit is not what decides
a condition. `[ ! -f x ]` is untouched for a different reason and
stays a normal failing assertion: the `!` there is an operator of
`test`, not negation of a command, so nothing exempts its status.
Each claim the suite makes is pinned against removing the thing it
claims, and the separator class is measured branch by branch rather than
only at its newest entries. The recursion is exercised through the audit
function on a fixture in a subdirectory rather than through a second
copy of its find expression, because a copy passes while the function
loses its recursion -- and an audit that returns empty reads as a clean
tree. The condition-strip carries fixtures that put the negation after
an `&&` inside the condition, where only the strip can save it. The
continuation fold is pinned by a fixture whose negation sits on the
continued line, so folded and unfolded name different lines. Both
keyword spellings, the `time` prefix and `!(cmd)` have fixtures of
their own. The subshell over-count carries a fixture for both of its
halves -- that the scan reports the shape, and that the shape really
does fail.
Both halves of the premise are pinned, not just the half about the
banned form. A test asserts the runner still passes a bare negated
assertion; a second asserts the prescribed `if cmd; then ...; false; fi`
does fail, under both runners, and fails AT the form rather than
somewhere before it. Every converted absence rests on that second
half, and its failure mode is asymmetric: were `false` in a `then`
body to stop propagating, the first test would still pass while every
conversion went silently green.
The rule joins the E2E conventions doc as its own section, and the
inline list in AGENTS.md that an agent may read instead of the doc --
it belongs beside the EXIT-trap ban for the same reason, both leaving
a green suite that checks nothing.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
|
e5a1ca853f |
fix(ci): run every Keycloak client suite on a Keycloak change
With a monitoring suite in the tree, a change to system/keycloak or system/keycloak-operator stopped escalating to the full run and selected monitoring alone, because monitoring-application is the only source that declares the keycloak-operator edge. That dropped kubernetes-latest, whose OIDC lifecycle also creates a KeycloakClient and waits on the operator's finalizer to remove it. The mapping is explicit because kubernetes-application has no dependsOn on keycloak-operator. A postgres-operator change now reaches kubernetes-latest too, through Keycloak's database. The selector comment and the e2e doc no longer name system/postgres-operator as a path with a source that reaches no suite, which stopped being true when monitoring got its suite. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
472f962eff |
test(e2e): cover monitoring OIDC modes with a Chainsaw suite
Monitoring OIDC lost its live-cluster coverage when the two pre-Chainsaw BATS suites were removed without a port. Render tests pin each mode on a fresh install, but not admission by cozystack-api, the Deployment the Grafana operator builds from the CR, or what an in-place mode change leaves behind on a running release. The suite drives one tenant-test Monitoring release through System, CustomConfig with an inline config, and CustomConfig with a secretRef, so a single stack bring-up covers all three and the upgrades between them: the cozy-realm client, scope and client Secret are pruned when System mode is left, and the inline generic_oauth block is gone once the secretRef mount arrives. The monitoring package sources now map to this suite, so a change to system/monitoring, extra/monitoring, monitoring-rd or grafana-operator selects it alone instead of escalating to the full run, and the selector test that relied on cozystack.monitoring reaching no suite seeds a source that does not. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
40dac09aaa |
test(bats): spell absences so they can fail
An assertion written `! cmd` cannot fail a test. POSIX exempts a command whose return value is inverted with `!` from errexit, so the status is dropped where it is produced. hack/cozytest.sh then closes each test function with `return 0`, and the bats binary's ERR trap carries the same exemption -- it reports only a negated assertion standing last in the body, where the inverted status happens to be what the function returns. Every absence these suites assert was written that way: a command that must not reach the log, an image that must not be copied, a flag that must not be passed, an object that must not exist. On that half of each suite's contract the tests held for any input. Rewrite each as `if cmd; then echo "FAIL: ..."; false; fi`, the form already used where the trap had been noticed one file at a time, and give every one a message naming what must not have happened. The message states the property, not the pattern: where the forbidden string is a substring of a required value -- an https endpoint the test requires in its http form -- naming the string would describe the opposite of what is asserted. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |