mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-08 21:14:56 +09:00
499034e960f5327c8d806581830bc0ccd6871fd2
377
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
a2cb47c961 |
feat(gateway): update the CRD bundle to Gateway API v1.6.1 and gate GRPCRoute hostnames (#3872)
## What this PR does Bumps the vendored Gateway API CRD bundle in `packages/system/gateway-api-crds` from v1.5.1 to v1.6.1. The package Makefile pins the upstream ref, so the change is that pin plus the re-rendered `templates/crds-experimental.yaml`. The bundle version is not only bookkeeping: it is a compatibility signal consumers read at runtime. Every CRD in the bundle carries a `gateway.networking.k8s.io/bundle-version` annotation, and a Gateway API controller can read it off the `gatewayclasses` CRD to decide whether the installed bundle matches the one it was built against. Controllers that do this typically accept any patch release but require an exact major and minor match, and report `SupportedVersion=False` with reason `UnsupportedVersion` on their GatewayClass when the minor differs, a status condition that surfaces the skew rather than a functional break. The Cloudflare Tunnel Gateway API controller proposed for the platform in #3858 is built against Gateway API v1.6.1 and performs exactly that comparison, so against the v1.5.1 bundle shipped today its GatewayClass reports the bundle unsupported, and with this bump it reports supported. Neither change depends on the other landing first: this PR only moves the bundle, and the check lives entirely on the consumer side. One consequence worth stating plainly, since it will outlive this PR: an exact-minor check makes the shipped bundle version a coupled pin for any such controller. A future bump to v1.7.x flips the same condition back to `UnsupportedVersion` until that controller is rebuilt against the newer bundle, so the two want to move together. The bundle still emits the same twelve CustomResourceDefinitions and the same two `safe-upgrades.gateway.networking.k8s.io` admission objects, and every CRD still carries the `helm.sh/resource-policy: keep` stamp the Makefile's post-processing step adds. One of the policy's two CEL rules is rewritten, though, and it changes what a cluster accepts. That has its own section below. Two kinds change their version surface: TCPRoute and UDPRoute gain a served `v1` that takes over as the storage version, and their `v1alpha2` stays served with a deprecation marker. Every other kind keeps exactly the versions it served at v1.5.1. That matters because the Cilium operator resolves GatewayClass, Gateway, HTTPRoute and GRPCRoute in `v1`, ReferenceGrant in `v1beta1` and TLSRoute in `v1alpha2` before it enables its Gateway API controller; all six are still served by the v1.6.1 bundle. TLSRoute in particular keeps `v1alpha2`, which the tenant gateway controller, the platform TLSRoute templates and the tenant hostname admission policy all address. Schema changes are small and land on fields nothing in this repository sets. `sessionPersistence.idleTimeout` is gone from HTTPRoute, GRPCRoute and XBackendTrafficPolicy, removed upstream under GEP-1619 because no implementation ever implemented it and HTTP cookies have no native idle-timeout mechanism, so no working behaviour is lost even though the field disappears from the schema; HTTPRoute gains a lower bound on `retry.attempts` and now rejects duplicate `retry.codes`; ReferenceGrant requires `spec` at the root. In the other direction, Gateway infrastructure annotations and frontend CA certificate references go from a limit of 8 to 16, and TLSRoute hostnames from 16 to 1024. No CRD schema loses or changes a CEL validation rule, and the six added belong exclusively to the new TCPRoute and UDPRoute `v1` schemas. ### The safe-upgrades admission policy accepts a different set of bundles The experimental kustomization ships a `ValidatingAdmissionPolicy` and its binding, both named `safe-upgrades.gateway.networking.k8s.io`, next to the twelve CRDs. The binding is `validationActions: [Deny]` with `failurePolicy: Fail`, matching CREATE and UPDATE on `apiextensions.k8s.io/v1 customresourcedefinitions` cluster-wide, and Cozystack installs it in two places: on the host through `cozystack.gateway-api-crds`, and inside every guest cluster that sets `addons.gatewayAPI.enabled`. Its second rule reads the `gateway.networking.k8s.io/bundle-version` annotation off an incoming CRD. At v1.5.1 that rule was a blacklist: reject anything matching `v1.[0-4].\d+` or containing `v0`, allow the rest. At v1.6.1 it is a whitelist: allow `v0.0.0-dev` outright, otherwise require the annotation to start with `v1.` and not match `^v1\.[0-4](\.|$)`. Evaluating both expressions over a spread of annotation values: | bundle-version | v1.5.1 policy | v1.6.1 policy | | --- | --- | --- | | `v1.6.1`, `v1.5.1`, `v1.5.0-dev`, `v1.10.0` | allow | allow | | `v1.4.0`, `v1.4.10`, `v1.0.0` | reject | reject | | `v0.0.0-dev` | reject | **allow** | | `v2.0.0` | allow | **reject** | | `1.6.1` (no leading `v`) | allow | **reject** | | `v1.4` (no patch) | allow | **reject** | | `main`, empty string | allow | **reject** | Nothing in this repository trips the new rejections: every CRD the bundle ships is stamped `v1.6.1`. What is exposed is an operator applying a Gateway API CRD set from somewhere else on a cluster that carries the policy. A fork, a vendor build or a nightly whose annotation reads `1.6.1` or `v2.0.0` used to go in and is now denied with `reason: Invalid`, naming a policy the operator may not know is installed, and whatever HelmRelease owns those CRDs then sits un-Ready. Uninstalling the policy is the escape hatch, and the denial message says so. The same hunk fixes an upstream inconsistency: at v1.5.1 the policy pair was stamped `v1.5.0-dev` while its twelve CRDs said `v1.5.1`. At v1.6.1 all fourteen documents agree. The Gateway and HTTPRoute objects the tenant gateway controller renders were serialized across all three certificate modes and checked against both the v1.5.1 and the v1.6.1 schemas; the two runs are identical, so the bump does not change what the controller can create. `go.mod` still pins `sigs.k8s.io/gateway-api` v1.4.1 and this PR deliberately leaves it there. The CRD bundle and the Go client types have been on separate versions since before this change, and nothing the controller emits depends on the gap. One thing to know before the next bump: from v1.6 on, the two things upstream calls the experimental bundle disagree. This package follows `config/crd/experimental`, the kustomization, which emits 12 CRDs at v1.6.1; the published `experimental-install.yaml` for the same tag emits 13, the extra one being `xbackends.gateway.networking.x-k8s.io`, new in v1.6 and not listed in the kustomization. At v1.5.1 the two agreed, so there was nothing to choose between. Nothing in this repository references XBackend and no vendored CRD depends on it, so following the kustomization costs nothing today, but whoever bumps next inherits the choice, and it should be made deliberately rather than by whichever command gets typed. The second commit corrects a comment this bump invalidated. It claimed TLSRoute stays at `v1alpha2` through Gateway API v1.5 and that the promotion to `v1` was still upstream-only; TLSRoute has been served at `v1` since v1.5.0, so the claim was already wrong before this bump and the version range it named is now wrong too. The third commit closes a gap that correction exposed. `cozystack-route-hostname-policy-tls` named `v1alpha2` alone while the bundle serves `tlsroutes` at `v1`, `v1alpha2` and `v1alpha3`: three served versions against one named. The gap is not new and this bump neither creates nor widens it: the v1.5.1 bundle and the v1.6.1 one serve the same three versions, with the same two marked deprecated. Coverage of the two unnamed versions rested entirely on request conversion, because `matchPolicy` is unset on both rules in that file and so defaults to `Equivalent`, which rewrites a request submitted under any served version into the one the rule names before the policy sees it. That is why there is no bypass to demonstrate, and equally why the arrangement is brittle: the conversion has nothing left to convert into once no named version is served, and a rule that selects nothing is not caught by `failurePolicy: Fail`, because nothing failed. The rule now names all three, which is the shape the HTTPRoute rule above it already used, and `hack/route-hostname-policy-version-coverage.bats` reads the served versions out of the vendored bundle and fails when a policy does not cover every one of them, so a future bundle move cannot outrun the list silently, in either direction. That guard carries negative fixtures rather than only watching a correct tree pass. Three of them drive `assert_covers` itself, with the two readers redefined in a subshell, against an omitted served version, an empty served set and an empty named set. Each requires the specific complaint text rather than merely a non-zero exit, so a helper renamed out from under it fails for the right reason instead of reading as a successful rejection. A fourth drives the comparison helper directly. The fourth commit pins the vendored bundle version in the CRD package's unit suite. Nothing outside the package Makefile named a Gateway API version, so re-running `make update` against a different ref landed green: twelve CRDs, every one stamped, document count unchanged, suite passing. The new case reads the `gateway.networking.k8s.io/bundle-version` annotation off every rendered CRD and fails when it is not the version the Makefile pins. The fifth commit extends the version-coverage guard to `cozystack-gateway-hostname-policy`, which names the `gateways` versions literally the same way the two route policies do, and whose version set lives in the vendored bundle and moves on `make update` there. It matched the served set already; the guard now says so on every run instead of leaving one of the three hostname policies unpinned. Two notes for whoever reads the release: the TCPRoute and UDPRoute storage version moves from `v1alpha2` to `v1`, and while nothing in this repository ships either kind and the CRDs declare no conversion block, an upgraded cluster accumulates both entries in `status.storedVersions` — inert today, load-bearing only if a later bundle stops serving `v1alpha2`. The sixth commit gives GRPCRoute the same hostname rule the other two route kinds have. GRPCRoute carries `spec.hostnames` and it can reach a tenant Gateway: the port-443 listeners pin `allowedRoutes.kinds` to HTTPRoute and TLSRoute, but the port-80 listener leaves `kinds` unset, and an unset list resolves to the kinds the listener protocol supports, which for HTTP includes GRPCRoute. A GRPCRoute in a tenant namespace could claim any hostname on the plaintext listener with nothing checking it. Tenants hold no `gateway.networking.k8s.io` RBAC in Cozystack, so this is the same defence-in-depth against a chart bug that the other two rules are, not a tenant-user control. The rule is the HTTPRoute rule with `grpcroutes` in place of `httproutes` and the one version the bundle serves for it, and the coverage guard pins that list to the bundle like the others. Several places in the tree enumerate which route kinds the hostname policies cover, and all of them named two: the port-443 kinds invariant in the tenant gateway controller and the test that pins it, `packages/extra/gateway/README.md`, and the threat model together with the self-assessment. They name three now. The port-443 kinds set also carries an accurate reason now. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. The README had also described HTTPS and TLS-passthrough listeners as carrying different kinds, where the controller gives both the same one. One more commit makes the suite's existing assertions bite. A ValidatingAdmissionPolicyBinding whose `policyName` points at another policy renders without complaint and leaves its own policy inert, and the suite pinned `validationActions` on the bindings while never pinning `policyName` at all, so pointing the GRPCRoute binding at the HTTPRoute policy kept every case green with the GRPCRoute control doing nothing. All three bindings now pin the policy they name. In the same file, `uncovered()` assigned to the variable names its caller reads back when printing a mismatch, so a call site with the arguments swapped would have printed a diagnostic naming the wrong side. Two things are named rather than made. The version-coverage guard collects `apiVersions` from every `resourceRules` entry mentioning the resource without also matching on `apiGroups` or `operations`, so a rule in another group, or one scoped to a single verb, would donate its versions to the set coverage is measured against; the chart's own suite pins the group and the resource on the rule it checks, which closes that a layer below. And `packages/system/cozystack-api/templates/api-tlsroute.yaml` renders `gateway.networking.k8s.io/v1alpha2` while the bundle serves `v1` as storage and marks `v1alpha2` deprecated, but the identical pin sits in the kubevirt and kubevirt-cdi TLSRoute templates, and the pinned `sigs.k8s.io/gateway-api` carries no `apis/v1` TLSRoute type for the controller side to follow, so moving one of the three here would leave the tree less consistent than leaving all three. Closes #3789. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file: neither `packages/system/gateway-api-crds/Makefile` nor its rendered CRD template matches any entry. Leaving every box empty rather than ticking the "no downstream repository" line, because the docs site carries a Gateway API version statement that is already inaccurate and needs a decision that is not mine: `content/en/docs/next/networking/gateway-api.md` and its v1.5 and v1.6 copies say Gateway API v1.5 ships TLSRoute at `v1alpha2` and that the graduation to `v1` is still upstream-only, when v1.5.0 already shipped TLSRoute `v1` as the storage version. ### Release note ```release-note chore(gateway-api-crds): update the vendored Gateway API CRD bundle to v1.6.1. TCPRoute and UDPRoute gain a served v1 that becomes their storage version, with v1alpha2 still served and now deprecated; every other kind keeps the versions it served at v1.5.1. Upstream removed `sessionPersistence.idleTimeout` from HTTPRoute, GRPCRoute and XBackendTrafficPolicy; the field is not set by anything Cozystack renders, but a hand-written route that carries it loses it silently on its next write, with no admission error. The bundle's `safe-upgrades.gateway.networking.k8s.io` ValidatingAdmissionPolicy changes which CRDs it accepts. Cozystack installs it cluster-wide on the host and in every guest cluster with `addons.gatewayAPI.enabled`. Its `gateway.networking.k8s.io/bundle-version` rule was a blacklist on two patterns and is now a whitelist on the `v1.` prefix with a `v0.0.0-dev` escape hatch. A bundle annotated `v0.0.0-dev` is now accepted where it was denied. A bundle annotated `v2.0.0`, `1.6.1` without the leading `v`, `main`, `v1.4` without a patch, or carrying an empty annotation is now denied where it was accepted. Nothing Cozystack ships is affected, since every CRD in the bundle is stamped `v1.6.1`, but applying a Gateway API CRD set from a fork, a vendor build or a nightly on such a cluster can now fail admission with `reason: Invalid`. Uninstalling the policy remains the documented escape hatch. fix(cozystack-basics): the tenant TLSRoute hostname admission policy now names every served TLSRoute version (`v1`, `v1alpha2`, `v1alpha3`) instead of `v1alpha2` alone. Coverage of the other two rested on request conversion, which holds only while a named version stays served, and a rule left selecting nothing is not caught by `failurePolicy: Fail`. feat(cozystack-basics): GRPCRoute hostnames in `tenant-*` namespaces are now gated by the same admission policy that gates HTTPRoute and TLSRoute. A tenant Gateway's port-80 listener leaves `allowedRoutes.kinds` unset, which admits GRPCRoute by protocol default, so a GRPCRoute could claim any hostname on the plaintext listener unchecked. Tenants hold no `gateway.networking.k8s.io` RBAC, so this is defence-in-depth against a chart bug rather than a tenant-user control. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added hostname admission validation for `GRPCRoute` resources. - Expanded `TLSRoute` hostname validation to support all served Gateway API versions. - Ensured route hostname policies consistently cover HTTP, TLS, and gRPC routes. - **Security** - Updated listener policies to restrict port-443 routes to supported HTTP and TLS route types. - Clarified fail-closed hostname validation behavior across Gateway, Route, and Ingress resources. - **Documentation** - Updated security and threat-model documentation to include GRPCRoute handling and validation coverage. - **Maintenance** - Updated the bundled Gateway API CRDs to version 1.6.1. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
fb740db0b0 |
docs(security): separate the fail-open Gateway-listener policy from the fail-closed route policies
The self-assessment summarised the whole hostname policy set as fail-closed, while its own table row states that the Gateway-listener policy passes a Gateway whose namespace carries no host label. Say which policies are which. The port-443 kinds comment in the reconciler test also tied the GRPCRoute admission rule to the port-80 listener; the rule matches the kind wherever it attaches, and the port-443 exclusion is a least-privilege choice, not the reason the rule exists. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
af19aa933c |
docs(security): record GRPCRoute in the route hostname policy set
The port-443 kinds set still excludes GRPCRoute, TCPRoute and UDPRoute. cilium#45559 governs only that every port-443 listener carries the same set, not what is in it, so the contents are a least-privilege choice: nothing the platform ships needs gRPC, TCP or UDP routing on port 443. TCPRoute and UDPRoute additionally carry no hostname and no admission rule gates them. The gateway README described HTTPS and TLS-passthrough listeners as carrying different kinds. They carry the same one, which is what cilium#45559 requires. The threat model and the self-assessment enumerate which route kinds the hostname policies cover, and both listed two. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrey Kolkov
|
e94fcf8219 |
feat(backups): RabbitMQ definitions backup via a dedicated strategy driver (#3990)
## What this PR does Adds backup/restore support for the managed `RabbitMQ` application. RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no consistent online export of message payloads, so this backs up broker **definitions** (vhosts, users, permissions, queues, exchanges, bindings, policies, parameters) over the management HTTP API (`GET`/`POST /api/definitions`) and stores them in the system bucket. Message payloads and point-in-time recovery are intentionally out of scope; a full-data backup would be a Velero volume snapshot, which is a separate strategy. It is delivered as a **first-class `strategy.backups.cozystack.io/Rabbitmq` driver**, following the Altinity pattern (a dedicated driver that renders a one-shot Job for an HTTP-API engine): - `reconcileRabbitmq` / `reconcileRabbitmqRestore` render the strategy's `PodTemplateSpec` — `curl` `GET`/`POST /api/definitions`, pushing/pulling the JSON to S3 with `curl --aws-sigv4` — as a `batch/v1.Job`, covering both restore flows: in-place (into the source broker) and to-copy (into a separate, differently-named RabbitMQ in the same namespace, authenticating with the target's own default-user credentials and reading the source's object key). - A **Go-read precondition** gates each run on the `RabbitmqCluster` reporting `AllReplicasReady` (read through a small `rabbitmqtypes` partial type), so a not-yet-ready broker surfaces a precise `Ready=False` wait on the BackupJob/RestoreJob rather than a Job that starts and fails against an unreachable management API. - The produced `Backup` records the stored object's location on `status.artifact.uri`, rendered from the strategy's `spec.artifactURITemplate`. The `Backup` CRD has no status subresource, so this persists on Create, mirroring the Etcd driver. Dispatch is wired in both controllers (plus `supportedBackupStrategyKinds`), `rabbitmqtypes` is registered in the scheme, and RBAC grants read on `rabbitmqclusters.rabbitmq.com` (the Job itself is already covered by the existing `batch/jobs` grant). It ships `cozy-default-rabbitmq` (a `$bucketName`-gated `Rabbitmq` strategy) and routes `apps.cozystack.io/RabbitMQ` to it in the `cozy-default` BackupClass, which also enrols it in the `DefaultObjectsGate`. The RabbitMQ application chart is left untouched — it had no prior backup wiring, and the strategy carries the S3 coordinates itself. Unit tests cover the driver's pure helpers (applicationRef validation, cluster-name derivation, the readiness-message builder, artifact-URI rendering, parameter round-trip) and the reconcile precondition gate (defers with `Ready=False` and no Job until the cluster is ready, then proceeds). A runnable example under `examples/backups/rabbitmq/` and an honest-roundtrip Chainsaw e2e under `hack/e2e-chainsaw/rabbitmq/` seed a sentinel vhost, back it up, drop it, and assert it round-trips through object storage on both restore flows. The Job pod reuses the existing `platform-migrations` image (curl + ca-certificates) under a new `backupStrategyController.rabbitmqBackupClientImage` key, stamped in lockstep with `chBackupClientImage` in `packages/core/platform/Makefile` so the repository stays pinned at a single digest (`hack/image-pin-consistency.bats`). A dedicated slim image built by this package is a follow-up. **Validation:** `go build`/`go vet`/`gofmt`/`go test ./internal/backupcontroller/... ./internal/template/...`, deepcopy + Rabbitmq CRD generation, `helm template` proving the CRD-ordering gate both on and off, `helm lint`, and `helm unittest` (18/18). The backup + in-place + to-copy roundtrip was verified end-to-end on a dev cluster (a real management-API export and a real S3 `--aws-sigv4` upload/download against SeaweedFS), so the driver's core contract is exercised on a real cluster; the in-CI Chainsaw run is the remaining automated evidence. The object-cleanup, terminating-namespace teardown and readiness-deadline paths added during review are unit-tested (including `Reconcile`-level finalizer retention and the terminating-namespace release) but not yet cluster-exercised. (Note found while testing: RabbitMQ 4.2 is OOMKilled on the `nano` preset, so the example/e2e use `c1.small`.) ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map against the diff. This is an additive backup strategy plus a new strategy CRD; no downstream repository is mechanically forced by the change. A documentation page for RabbitMQ backup on [cozystack/website](https://github.com/cozystack/website) would be a reasonable follow-up, but it is not required by this diff — leaving the boxes unticked for a human to decide rather than ticking "no downstream" to clear the checklist. ### Release note ```release-note feat(backups): add RabbitMQ definitions backup/restore via a dedicated strategy.backups.cozystack.io/Rabbitmq driver; it gates each run on the RabbitmqCluster being ready and records the stored object on Backup.status.artifact.uri. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added RabbitMQ backup and restore support for definitions, including vhosts, users, queues, exchanges, bindings, policies, and related settings. * Added in-place and restore-to-copy workflows with readiness checks. * Added scheduled and on-demand backup examples. * Integrated RabbitMQ backups with the default backup configuration. * Backups store definitions only; message payloads and point-in-time recovery are not included. * **Documentation** * Added a complete RabbitMQ backup and restore demonstration guide. * **Tests** * Added end-to-end coverage for readiness and backup/restore round trips. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
61026b732c |
docs(agents): require the neutral Assisted-by: LLM trailer
The attribution trailer exists for disclosure, not to advertise a model. A model name in the trailer tells a reviewer nothing they can act on, and it goes stale with every tool change, so the accepted form is a single `Assisted-by: LLM` line however many tools took part. A link to an assistant session is worse still: it rots, and it carries nothing about the change itself, so no session trailer and no transcript URL belong in a commit, a PR description, or a comment. The commit body explains why a change is made; the diff already shows what changed. The hygiene rules for messages, trailers and in-code comments are written down as review blockers for the same reason: a NOT LGTM on one of them is then a stated rule, not a reviewer's taste. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
scooby87
|
3a70f91423 |
fix(vm-disk): fail on storageClass/source edit of existing disk (#3969)
## What this PR does Fixes #2985. Editing a `VMDisk`'s `storageClass` or `source` after its DataVolume already exists was silently ignored: `packages/apps/vm-disk/templates/dv.yaml` looks up the existing DataVolume and, when found, reused its `spec` verbatim (because `DataVolume.spec` is immutable in CDI). The edit therefore rendered as a no-op — the typed CR accepted it and the HelmRelease reported success, while the DataVolume/PVC kept the original spec and the source of truth silently diverged from the cluster. The template now builds the desired `source` into a shared `$desiredSource` variable (used both for rendering and comparison) and, when a DataVolume already exists, calls `fail()` with an explicit message if the requested `storageClass` or `source` differs from the immutable existing spec — mirroring the explicit size handling already done in `pvc-resize-hook.yaml`. Storage growth is unaffected and still flows through the resize hook. `source` is also marked `@immutable` in `values.yaml`, so the generated schema now carries the same `x-kubernetes-validations` transition rule (`self == oldSelf`) that `storageClass` already had; `types.go`, `values.schema.json` and the generated `vm-disk-rd` CRD are regenerated to match. ### Verification - `helm unittest` covers both the immutability guard (via a mocked existing DataVolume through `kubernetesProvider`, since `lookup` needs a cluster) and initial-creation source rendering for every source type — 12/12 pass. - Verified live on a dev cluster with `helm install --dry-run=server` against a real DataVolume: an unchanged edit renders, a `storageClass` change fails, a `source` change fails, and a storage grow still renders the resize hook without tripping the guard. ### Scope note This PR only makes the drift visible and marks `source` immutable. Two deeper platform findings sit outside its scope: (1) #2657 (pre-existing) — the `apps.cozystack.io` aggregated apiserver does not evaluate the `x-kubernetes-validations` CEL rules on update, so `@immutable` is a no-op at admission for every Application kind, which is exactly why the chart-level `fail()` — not the CEL rule — is the substantive guard here; (2) #3971 — that same apiserver does not honour `--dry-run=server` and persists the change (surfaced while testing this fix). ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff file by file. The diff touches only `packages/apps/vm-disk/` (template, `values.yaml`, regenerated `values.schema.json`, tests), the regenerated `packages/system/vm-disk-rd/cozyrds/vm-disk.yaml`, and `api/apps/v1alpha1/vmdisk/types.go`. No package was added, renamed or removed; no `values.schema.json` field was added, renamed or removed (only a validation rule on the existing `source` field); no default changed; `README.md` is unchanged; the `cozyrds` file was regenerated, not renamed. The terraform provider models fields, defaults and enums but not CEL immutability, so it is unaffected. No downstream repository is reached by this change. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(vm-disk): editing storageClass or source on an existing VMDisk now fails with a clear message instead of being silently ignored; delete and recreate the disk to change these immutable fields. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for blank, HTTP, image, disk, and upload VM disk sources. * VM disk sources and storage classes are now protected from unintended changes, while storage expansion remains supported. * Existing disks preserve their configured source and storage settings. * **Bug Fixes** * Invalid or conflicting source configurations are rejected. * Prevented silent no-op updates and incorrect immutability errors for existing disks. * **Documentation** * Documented source and storage class immutability, including the need to recreate disks when changing either setting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
ba32df69c0 |
feat(backups): add RabbitMQ definitions backup via a dedicated strategy driver
RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no
consistent online export of message payloads, so back up broker
DEFINITIONS (vhosts, users, permissions, queues, exchanges, bindings,
policies, parameters) over the management HTTP API and store them in the
system bucket. Message payloads and PITR are out of scope - a full-data
backup is a Velero volume snapshot, a separate strategy.
Add a first-class strategy.backups.cozystack.io/Rabbitmq driver, following
the Altinity pattern (a dedicated driver that renders a one-shot Job for an
HTTP-API engine):
- reconcileRabbitmq / reconcileRabbitmqRestore render the strategy's
PodTemplateSpec (curl GET/POST /api/definitions, push/pull to S3 with
--aws-sigv4) as a batch/v1.Job, for both in-place and to-copy restore.
- A Go-read precondition gates each run on the RabbitmqCluster reporting
AllReplicasReady (via the rabbitmqtypes partial type), surfacing a
precise Ready=False wait instead of a Job that starts and fails.
- The produced Backup records the object location on status.artifact.uri
(rendered from spec.artifactURITemplate; the Backup CRD has no status
subresource, so it persists on Create, as in the Etcd driver).
Wire dispatch in both controllers + supportedBackupStrategyKinds, register
rabbitmqtypes in the scheme, and grant read on rabbitmqclusters. Ship
cozy-default-rabbitmq (a $bucketName-gated Rabbitmq strategy) and route
apps.cozystack.io/RabbitMQ to it in the cozy-default BackupClass. The
application chart is untouched.
Add unit tests for the driver's pure helpers and the precondition gate, a
runnable example, and an honest-roundtrip Chainsaw e2e that seeds a
sentinel vhost, backs it up, drops it, and asserts it round-trips through
object storage on both restore flows.
The Job pod reuses the platform-migrations image (curl + ca-certificates)
under a new rabbitmqBackupClientImage key, stamped in lockstep with
chBackupClientImage so the repository stays pinned at one digest; a
dedicated slim image is a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrei Kvapil
|
440c3b58f3 |
test(valkey): add e2e test and wire build, labeler and docs
Add the Valkey e2e bats test (mirrors redis), build the compat image in the top-level build target, map the valkey scope to area/database in the PR labeler, and list valkey among charts with an immutable storageClass. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Alexey Artamonov
|
06624bacd0 |
fix(vm-disk): fail on storageClass/source edit of existing disk
Editing a VMDisk's storageClass or source after its DataVolume already
exists was silently ignored: dv.yaml reused the existing DataVolume.spec
verbatim (DataVolume.spec is immutable in CDI), so the edit rendered as a
no-op while the HelmRelease reported success and the CR diverged from the
cluster.
When a DataVolume already exists, fail with a clear message if the
requested storageClass or source differs from what the disk was created
with. Both compare the operator's input recorded in an annotation at
creation (vm-disk.cozystack.io/storage-class and .../source), written only
at creation and preserved verbatim afterwards — never overwritten with an
unapplied edit. Recording the input rather than reading the live object
matters on both axes: the image-clone PVC name carries a chart-version
prefix (vm-image- was renamed to vm-default-images-, commit
|
||
|
IvanHunters
|
0fa6cf72a5 |
chore(ci): complete the backport label rename across all consumers
The initial rename missed several literal consumers of the old label names beyond labels.yml and backport.yaml: - stale.yaml exempt-pr-labels still listed backport,backport-previous. actions/stale matches label names exactly and does not resolve label-sync aliases, so after the rename a kind/backport PR would lose its stale exemption and get auto-closed after inactivity. This is the one behavioural regression; the rest are documentation drift. - PULL_REQUEST_TEMPLATE.md told contributors to apply the backport label. - release.md label-target table and the gh pr list --search commands referenced the old names (the searches would silently return nothing). - contributing.md claimed a backport label is auto-applied on the [Backport] prefix; pr-labeler only adds area/release, so the line was already stale and is corrected while renaming. Historical narrative in release.md and the backport scope-to-area doc rows (a Conventional-Commit scope, not the label) are left as-is. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Andrei Kvapil
|
eef95f456d |
docs: add CNCF General Technical Review (GTR) questionnaire (#3572)
## What this PR does Adds the CNCF General Technical Review (GTR) questionnaire for Cozystack, completed for the Sandbox → Incubation Due Diligence (tracks cncf/toc#1916). It answers the Day 0 / Day 1 / Day 2 questions from the CNCF TOC project-reviews template, with versioned links to the v1.6 documentation and to the repo's governance and security files. **Status: DRAFT / work in progress.** The questionnaire itself is a living document per the template's guidance. Everything repository evidence settles has been answered and verified against the tree; the banner at the top of the file names the seven substantive gaps that need measurement, publication or a maintainer decision — load/scale results and tested maximums, published SLOs/SLIs, measured platform baseline overhead, the upgrade→downgrade CI matrix, whether upgrades trigger Talos node reboots, the security-team rotation and diversity policy, and publication of the CVE-scanning pipeline's policy documents. Version-specific links should be pinned to the submitted tag before final submission. ## Changes beyond the new file — please read this part Review feedback found two factual errors that originate **outside** this questionnaire, so fixing them only in the GTR would have left a reviewer clicking through to the contradiction. Three existing files are therefore touched: - **`SECURITY.md` — supported-versions table. This is a policy change and needs a policy review, not a wording check.** The table was dated "As of March 17, 2026" and named `v1.1.x` as the current stable line, five minors behind `v1.6.1`. It is rewritten to be keyed to **observable patch activity on a line** — supported means a patch release within the last 6 months, limited support 6 to 18 months, end of life beyond that — so an adopter can determine their own status from the Releases page and the table cannot drift again. Two intermediate versions of this PR got the policy wrong in opposite directions and both are reverted: one narrowed support to the latest two minors, which contradicts practice since several `1.x` lines receive patch releases concurrently; the other removed the end-of-life category altogether and extended discretionary backports across all of `0.x`. What lands here restores an end-of-life category, puts the `v0.41` cutoff back with its last-patch date, and states explicitly that the six- and eighteen-month marks describe current behaviour rather than committing the project to patch any line for that long. `@kvaps`, as a CODEOWNER for this file: please review the **policy**. If the project would rather state a guaranteed backport window, or keep support entirely at maintainer discretion with no stated marks, that is a governance decision and this PR deliberately does not make it. - **`SECURITY.md` — automated security analysis.** It said "two scanners run continuously" and listed CodeQL and Scorecard. It now also describes `zizmor` (GitHub Actions SAST, gating on PRs), the organization-wide Trivy CVE-scanning pipeline, and Renovate's actual coverage. It also records that CodeQL is **not** currently among the required status checks on `main` — those are `pre-commit` and `E2E Tests` — so the CodeQL gate is review convention rather than branch protection today. - **`CONTRIBUTOR_LADDER.md`** — Reviewer nomination described adding a username to an `OWNERS` file. No `OWNERS` file exists in this repository; line 158 of the same document already refers to `CODEOWNERS`. Corrected to name the files that exist. This does not change the process. - **`docs/security/self-assessment.md`** — brought in step with the above on the same three points (CodeQL's gate status, the Trivy pipeline and its limitations, Renovate's real coverage), so the GTR, `SECURITY.md` and the self-assessment agree rather than contradict each other in front of the TOC. ## Two things for `@kvaps` to decide, not to approve as wording 1. **Which release lines are supported — and the evidence points two ways.** The six-month activity test in the rewritten table classifies every `1.x` line from `1.0` to `1.6` as supported, because each has had a patch release inside six months (`v1.1.7` as recently as 2026-06-03, `v1.0.7` on 2026-03-30). But *recent* activity looks like three lines, not seven: of the sixteen most recent backport branches, fifteen target `release-1.4`, `release-1.5` or `release-1.6`, and only `1.4`, `1.5` and `1.6` have shipped a release since 2026-07-03. Older backport branches for `release-1.2` and `release-1.3` exist but are months old — and since branches are never deleted, their existence proves nothing about the present, which is the same trap the table's own lead-in now warns about. So the concrete question is: **is the intended set `1.4`/`1.5`/`1.6` supported with `1.0`–`1.3` on limited support, or is it genuinely all seven?** If a critical fix would today be backported to `v1.1`, the wide table is right. If it would not, the six-month window is the wrong instrument and should be shortened. I have deliberately not chosen — either answer is defensible and it is a maintainer call, not a documentation one. **A third file turns on the same answer.** [`docs/security/incident-response.md`](https://github.com/cozystack/cozystack/blob/main/docs/security/incident-response.md) step 5 says a fix ships "across all affected supported release lines", pointing at the `SECURITY.md` table — so widening that table to seven lines widens this commitment with it. This PR does not touch that file and does not assert the claim, but the two should move together: if the answer is "all seven", incident response is right and `SECURITY.md` should stop hedging; if it is "1.4–1.6", the table, the window and that sentence all change at once. 2. **Whether there is a backport rule at all.** No project document, public or private, describes how a fix is chosen for backporting to a given supported line. An intermediate version of this PR asserted one (by severity, default-config reachability and adopter count); that was invented and has been removed. The file now says the decision is taken per fix and the rule is not documented. If a rule exists informally, writing it down would answer a question the seven-supported-lines table otherwise raises. ## Gaps this documents rather than fixes Recorded here because they surfaced while verifying the questionnaire's claims and each is a real finding: - CodeQL is not a required status check, though two documents described it as gating merges. - There is no top-level `NOTICE` file, for a distribution bundling on the order of fifty upstream components. - No default Kubernetes audit policy is shipped; audit logging is opt-in with the policy left to the adopter. - Release artifacts are not signed and no issue tracks adding signing; build provenance is explicitly disabled (`--provenance=false`); SBOM generation is implemented but defaulted off. - The CVE-scanning pipeline lives in a repository that returns 404 to non-members. Its aggregate reporting is now published (below); its policy documents and scanning scripts, which hold no finding data, are not, so the method behind the numbers is still not independently auditable. - `nightly.yaml` has been failing since 2026-08-02 and `e2e-tag.yaml` failed around the v1.6.1 release; current CI health is disclosed next to the workflow links. ## New: published security reporting, and a backlog audit that changes the SCA answer `docs/security/reports/` now holds monthly vulnerability-handling reports plus a [backlog reconciliation](https://github.com/cozystack/cozystack/blob/main/docs/security/reports/reconciliation-2026-08.md). The reconciliation exists because the CVE-scanning pipeline has no write-back from this repository, so it has never marked a finding as fixed and its counts were misleading in both directions. Audited against what actually shipped: - Remediation did happen — a clear majority of confirmed findings relate to a component whose in-tree version now carries the fix — but as **one wave in June 2026**, not a steady stream. The changes that did most of the work name no CVE at all: re-vendoring the dashboard console removed the vulnerable npm dependencies rather than upgrading them; refreshing the Alpine bases cleared fourteen; bringing the Go toolchain current satisfies all 62 `stdlib` findings. - **57% of the backlog was never applicable or cannot be assessed.** The largest block is packages containing no executable vulnerable code — chiefly kernel headers, since a container never provides a running kernel, which is the same reasoning the maintainers already applied 80 times to `kernel-headers` and never to `linux-libc-dev`. Reachable exposure in a stock install is an estimated 2–5% of the tracker. - **Four component bumps are available upstream and not taken**: `nats-server`, the CoreDNS image, OpenBao, `fast-uri`. - Seven defects in the pipeline itself, with a write-back specification to fix the root cause. Two issues counted as findings are actually quarterly-review tracking tasks. The disclosure rule moved in the open direction: findings with no upstream fix will be named with mitigation guidance, because under the previous rule that class — a fifth of the backlog — stayed secret permanently, and it is exactly where an adopter needs to know a mitigation is the only control. Conversely, remediation is no longer attributed to individual months, because crossing a count of still-open findings with one month's publicly enumerable component bumps narrows the search for unpatched exposure more than a bare count does. Several findings warrant maintainer attention ahead of the backlog and are being raised privately rather than named here. ### Downstream repositories - [x] No downstream repository is affected by this change (docs-only; no code, API, manifest or packaging changes). ### Release note ```release-note docs: add CNCF General Technical Review (GTR) questionnaire for the Sandbox → Incubation review ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added a comprehensive technical review covering planning, installation, operations, architecture, security, and troubleshooting. - Added April–August 2026 security reports and a complete CVE backlog reconciliation. - Expanded security self-assessment, scanning coverage, reporting guidance, disclosure rules, and audit limitations. - Clarified supported-version policies, release backport decisions, and private vulnerability-reporting procedures. - **Process Updates** - Updated reviewer nomination requirements to use current ownership and maintainer records. - Clarified severity- and risk-based backport decisions for coordinated security releases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
f5b957ee27 |
feat(kubernetes)!: manage worker pools as KubernetesNodes, adopt on upgrade (Phase 2, breaking) (#3315)
## What this PR does Phase 2 of the Kubernetes-app split ([cozystack/community#8](https://github.com/cozystack/community/pull/8)) — the **breaking** half. Stacked on #3314 (review that first). Removes worker node pools from the `kubernetes` chart (they moved to `kubernetes-nodes` in #3314) and adopts existing pools on upgrade: - `kubernetes` chart is now **control-plane only**: drops the pool `range` block, the `talos-reconcile` Job + RBAC (deleted; moved to `kubernetes-nodes`), the preserve-old-KMT block, the `kubernetes.nodeGroups` helper, and the per-group dashboard RBAC. - **`spec.nodeGroups` is removed from the `Kubernetes` CR** (BREAKING). Worker pools are now separate `KubernetesNodes` resources. - **Migration 54** (bumps `targetVersion` to 55): pre-upgrade hook that, for every `kubernetes-<cluster>` release, creates one `kubernetes-nodes-<cluster>-<pool>` HelmRelease and re-annotates the existing worker objects onto it with `helm.sh/resource-policy=keep`, so the now control-plane-only parent upgrade does not prune running workers. The default `md0` (implicit when `nodeGroups` was empty) is materialised explicitly. Idempotent. Genuine infrastructure/read failures fail closed (`exit 1`, retried on the next upgrade); hostile stored shapes (a pool name over Helm's 53-char release-name cap, an RFC-1123-invalid or whitespace/slash map key, a non-object `nodeGroups`) are warned, pinned, and skipped rather than deadlocking the entire fleet's upgrade. - The `ingressNginx` addon no longer auto-provisions a node (the implicit `md0` default is gone). If enabled without a pool carrying `roles: [ingress-nginx]`, the controller DaemonSet simply schedules nothing (degraded, not failed) — the requirement is documented in the chart NOTES.txt rather than enforced at render time. Because the child chart renders the pool objects **byte-identically** to the old parent render (golden parity in #3314; the KubevirtMachineTemplate content-hash name is preserved), adoption does not churn live worker VMs. ### Migration number `54` is contiguous with main (latest is `53`) and `targetVersion` is `55`. The `52`/#3201 collision noted in earlier revisions has been resolved: this branch was rebased and merged past it, so the numbering is contiguous and `run-migrations.sh` has no gap to hard-fail on. ### Not yet done (blocking draft → ready) - [ ] **dev e2e upgrade validation** (`cozystack-pr-test`): upgrade a custom-`nodeGroups` cluster and a default-`md0` cluster; assert zero worker-VM churn (KMT hash, Machine uid/creationTimestamp stable) and that the adopted child HRs reconcile. - [ ] downstream trigger-map walk (website docs for the `nodeGroups` → `KubernetesNodes` change; terraform-provider for the removed API field). ### Downstream repositories Deliberately unticked (draft) — the breaking API change affects docs and the terraform provider; follow-ups to be filed before ready. - [ ] 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: ```release-note Worker node pools of a managed Kubernetes cluster are now managed as separate `KubernetesNodes` resources. `spec.nodeGroups` on the `Kubernetes` CR is removed; existing pools are adopted automatically on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Worker pools are now separate `KubernetesNodes` resources with their own lifecycle. * Added a render-time compatibility guard to prevent worker pools from running ahead of the parent control-plane minor version. * Improved upgrade/uninstall behavior to better preserve already-adopted worker objects. * **Breaking Changes** * Removed per-pool `nodeGroups` and worker node health settings from the `Kubernetes` chart/CR; pool configuration now lives in `KubernetesNodes`. * Ingress requires at least one `KubernetesNodes` pool with `roles: [ingress-nginx]`. * `cluster` and `storageClass` are immutable after creation; pool `version` may lag but must not be ahead of the parent minor. * **Documentation** * Updated READMEs, notes, and schemas to reflect the `KubernetesNodes` split and the immutability/version rules. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ad62a8d62b |
test(e2e): stop blocking merges on the tenant node-join deadline
The tenant node-join wait no longer discriminates. Nested virtualisation on the shared runners degrades far enough that a guest which registers in minutes on real hardware can miss any deadline this test can afford, so a red on that one wait says as much about the machine the run landed on as about the product, and it teaches a reader to re-run the job. The suite still fails on it. That is what keeps Chainsaw's catch running: the previous-instance container logs, the host snapshot, the data-plane capture and the event dump are all keyed on the test failing, and the report stays honest. What changes is one layer up. The wait writes a marker beside its diagnostics, and a new lane-side gate reads that marker to decide whether the job blocks. It opens only when every failed test in the run carries one, and blocks on anything it cannot attribute: a report it could not read, a failed test the report does not name, a failure with no marker. Only the lanes that block a merge ask it. The two pull-request lanes do; nightly and the tag lane run the same suites and stay hard. A soft pass on the tag lane would ship a release candidate whose tenant clusters brought up no worker nodes, and nightly is the only place an e2e failure reaches anyone who did not open a pull request, since no lane in this tree notifies anywhere. A softened red finishes the job green, so the job's result stops answering whether the suite passed, and both kinds of consumer are handed the decision instead of inferring it. A step gated on the job failing would silently stop running: the SSH breakpoint is that case, and it still opens for a pull request marked for debugging. A job that reports the result would silently start reporting a pass: the required commit status now says the deadline was missed and that the merge is not blocked on it, rather than that the suite passed. The deadline goes to 29m. That is the test's own budget and nothing more: its clock starts after the machinedeployment wait, the LoadBalancer assignment and the healthz probe, while the chart's autoscaler and health-check timers have been running since the Machine was created, so no ordering against them is claimed. The chart's 20m and 30m give a scale to pick against, and a budget over the first leaves room for a remediation cycle without promising that one fits. The operation ceiling both suites give the script rises with the deadline, and the diagnostics phase budget rises inside it, because the collectors gated ahead of the serial console cost more between them than the old budget covered. Both budget guards read their terms from the sources that set them. The e2e job cap rises with them, and its guard holds what a guard can hold: the pair's operation and catch ceilings, and one figure across every lane that runs the suite. It also records that the suites' full summed ceilings have never fit any cap in that file, so the margin above the bound is taken from a measured run instead. The prose that restates the cap elsewhere moves with it, and the sweep that holds the deadline's restatements now reads the figure spelled in words as well as in minutes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9ccb133b66 |
fix(ci): de-escalate select-e2e and name every escalation (#3817)
## Why
I replayed the 150 most recently merged pull requests through
`hack/select-e2e.sh` with every escalation branch instrumented. 118 of
them (78.7%) ran the full 21-suite Chainsaw run, and the causes, counted
once per escalating PR with the count where that cause was the only one
in brackets, were `FULL_PATTERN` 88 [68], `NO_SUITE_FOR_GROUP` 24 [24],
`CHAINSAW_SHARED` 18 [1] and `UNCLASSIFIED` 7 [4]. `YQ_BROKEN`,
`NO_GRAPH_OWNER` and `BACKSTOP_EMPTY` never fired.
Two problems come out of that. Most of the escalation is real but
unnecessary. Whole classes of path that provably cannot affect a
Chainsaw suite were escalating because no rule claimed them. And the
commonest cause of a full run was absent from the log, so "why did this
pull request run everything" could only be answered by re-deriving the
selection by hand.
## What changes
**Every escalation now names its cause on stderr.** Seven branches could
reach the full suite and four did it in silence, `full_suite_pattern`
among them, the commonest cause by a wide margin, so the usual answer
was the one the log never carried. The lines go to stderr and must stay
there: stdout is the suite list and both e2e lanes parse it, so a reason
line on stdout would be read as a suite name. A unit test asserts every
message, verified by muting each of the eleven in turn and confirming
the file goes red for all of them. A reason that regresses to silence
changes no selection and is otherwise invisible.
**`cozystack.etcd-application` gains its missing `dependsOn:
cozystack.etcd-operator`.** `packages/extra/etcd` renders `kind:
EtcdCluster` from `etcd-operator.cozystack.io/v1alpha2`, and this was
the only operator-backed application source in the tree with no edge to
its operator. It is a latent install bug as much as a selection gap:
without the edge `etcd-rd` registers the ApplicationDefinition as soon
as the engine is up, so a tenant can create an `Etcd` before the
operator exists and its HelmRelease fails on `no matches for kind
"EtcdCluster"`, and `hack/select-install.sh`'s forward closure for the
`etcd` suite omitted the operator for the same reason. On the selection
side `cozystack.etcd-operator` reached no runnable suite, so every
change to the operator ran all 21.
**`vm-disk-application` maps to the `vminstance` suite.** It resolved to
a `vm-disk` suite that does not exist, `intersect_suites()` dropped the
name, and the source reached nothing runnable. Neither did
`cozystack.kubevirt-cdi` above it, whose only other dependents are
`vm-default-images` and `vm-disk-application` itself, so every CDI
change ran all 21. The coverage was there and the table did not know it:
the `vminstance` suite creates a `VMDisk` and asserts the `DataVolume`
behind it (`hack/e2e-chainsaw/vminstance/vmdisk.yaml`,
`vmdisk-vmi.yaml`). Same defect class as the etcd edge above, on the
mapping side rather than the graph side, and with no production blast
radius of its own, since `src_to_suites` is read by the selector and by
nothing else.
**`cozystack.cozystack-basics` joins `cozystack.cozystack-engine` as a
propagation hub excluded from the reverse-dependency walk.** Every edge
into it exists so a namespace or a platform-wide policy is in place
before the dependent installs, which `kubevirt-cdi` states in its own
source ("Depend on cozystack-basics so the target namespace exists
first"), and that is install ordering rather than behaviour. The damage
runs opposite to the engine's: the engine fans one change out to every
app, basics narrows instead. It reaches no suite today so it escalates,
but it sits upstream of `kubevirt-cdi`, so the first suite to land under
CDI silently converts the platform's namespace-and-policy package from
the full run to that one suite. Not hypothetical — on #3426 enabling the
site-router suite takes `cozystack-basics` from 22 suites to
`site-router` alone, and nothing in the output says coverage was lost.
Dropping the reverse edges keeps the package reachable, stops it
propagating, and leaves a change to it running everything through the
per-path escalation.
**`hack/e2e-apps/<name>.bats` maps to the `<name>` suite**, off the
basename, exactly as the per-suite rule takes the name off a
`hack/e2e-chainsaw/<app>/` directory. Deliberately mapped rather than
marked inert: what remains there is wired to nothing after the Chainsaw
migration, and inert would bake that orphan status into the rule and go
quietly wrong the day a lane runs those files again. The basename is
membership-tested against the suite list on the spot, so an unmatched
one escalates there and then rather than having the verdict deferred to
the final intersection, where the rest of the diff would decide it.
**`packages/tests/` and three `.gitattributes` join
`inert_config_pattern`,** which is what the script's own header says to
do with a genuinely inert path instead of widening the fall-through.
`packages/tests/` is a helm-unittest fixture chart, and changing a test
*of* `cozy-lib` does not change `cozy-lib`, no PackageSource lists those
paths as a component, and nothing installs them, while a change to the
library itself still escalates through `packages/library/`. The
`.gitattributes` entries are enumerated rather than matched by filename,
because the justification is what those files contain (only
`linguist-generated` markers) and the name does not carry it:
`.gitattributes` can also set `filter`, `eol`, `working-tree-encoding`
and `export-subst`, each of which changes what lands in the working tree
and therefore what gets built.
**`full_suite_pattern` escalates only `hack/e2e-*.bats`, not every
`hack/*.bats`.** The root `Makefile` is the authority on the split and
draws it at exactly that prefix, `BATS_UNIT_FILES := $(filter-out
hack/e2e-%.bats,$(wildcard hack/*.bats))`, so the 60 files it keeps are
the unit lane and the e2e sandbox runs none of them, while the three it
filters out are what `packages/core/testing`'s recipes execute and those
still escalate. Being inert here does not leave them untested: `make
unit-tests` is gated on the `plan` job's `code` output, which
`pull-requests.yaml` computes as "any changed path outside `docs/`" and
never from `select-e2e.sh`, so a bats-only pull request still runs the
whole unit lane, plus install and the OpenAPI tests, since the `e2e` job
reads the same output rather than the selection.
## Measured effect
The before-and-after comparison below is a second measurement over a
slightly different population, stated separately because the two are not
interchangeable: it replays the last 150 first-parent merge commits on
`main` (`git diff --name-only $sha^1 $sha`) through the base selector
with base sources versus this branch with its own, where the histogram
above walks the 150 merged pull requests as GitHub lists them. The
populations overlap heavily and the direction is the same, and the
totals differ by two commits.
| | full suite | scoped | nothing |
|---|---|---|---|
| before | 120 | 20 | 10 |
| after | 106 | 25 | 19 |
Full-suite rate 80.0% → 70.7%, verdict changed on 15 commits. Fourteen
de-escalated and I read every one of their file lists: they are
unit-lane bats files, non-e2e workflows, `docs/`, `CODEOWNERS`,
helm-unittest fixtures and linguist markers. The fifteenth gets one
suite *wider*: a `cert-manager` change now also selects `etcd`, which is
correct, since `cert-manager` is a dependency of `etcd-operator` and the
etcd app now genuinely reaches it through the new edge.
That replay predates the two reachability rules, and replaying the same
150 first-parent merges across those two on their own — the selector at
`73bc4c0ae` against the selector at `adf231513`, same sources on both
sides — changes no verdict at all. Two of the 150 touch
`cozystack-basics` and both ran the full suite before and after, which
is the hub exclusion keeping something true rather than failing to do
anything, and none of the 150 touch `kubevirt-cdi` or `vm-disk`, so the
mapping shows up only in a direct selection.
A few individual selections, before → after:
| changed files | before | after |
|---|---|---|
| `hack/select-e2e_test.bats` | 21 suites | *nothing* |
| `hack/select-e2e_test.bats` +
`packages/apps/redis/templates/redis.yaml` | 21 suites | `redis` |
| `hack/e2e-install-cozystack.bats` | 21 suites | 21 suites |
| `packages/tests/cozy-lib-tests/**` | 21 suites | *nothing* |
| `packages/system/.gitattributes` | 21 suites | *nothing* |
| `hack/e2e-apps/postgres.bats` | 21 suites | `postgres` |
| `packages/system/etcd-operator/values.yaml` | 21 suites | `etcd` |
| `packages/system/kubevirt-cdi/values.yaml` | 21 suites | `vminstance`
|
| `packages/apps/vm-disk/values.yaml` | 21 suites | `vminstance` |
| `packages/system/cozystack-basics/values.yaml` | 21 suites | 21 suites
|
## Blast radius of the `dependsOn` edge
A `dependsOn` on a PackageSource is an install-ordering edge in
production, not only a test-selection hint, so this is the part worth
reviewing hardest. Both Packages are emitted by
`packages/core/platform/templates/bundles/system.yaml` under the single
`bundles.system.enabled` guard with no intervening conditional between
them, in every variant, so no supported configuration has the app
without the operator. `hack/select-install.sh --validate` reports the
graph still has no cycle and nothing dangling. The one new exposure is
an admin who lists `cozystack.etcd-operator` in
`bundles.disabledPackages` while keeping the app, which now leaves it
`DependenciesNotReady`. That is the same exposure postgres, mariadb,
kafka and redis already carry, with `Package.spec.ignoreDependencies` as
the escape hatch. On upgrade `etcd-rd` waits for the operator's two
HelmReleases, and an unhealthy `etcd-operator` already means broken etcd
apps.
## Testing
Every rule added or changed has a test in `hack/select-e2e_test.bats`,
the install-side change has one in `hack/select-install_test.bats`, and
the new graph edge has a helm-unittest guard at
`packages/core/platform/tests/sources_etcd_application_dependson_test.yaml`.
The round-trip test that walks every suite through both mapping tables
stays green, `vminstance` included now that two sources map onto it.
The two reachability rules were each checked by removing the rule and
confirming its own test reddens: without the mapping the CDI test falls
back to the full-suite escalation, and without the hub exclusion
`cozystack-basics` selects `redis vminstance` instead of everything. The
hub test has to seed its own hazard, since no suite sits downstream of
basics in this tree yet, so it gives `redis-application` an edge onto
basics in a copied sources directory and pins that a redis change still
selects redis, or the assertion would pass against a graph that resolved
nothing.
`make bats-unit-tests` aborts on the first failing file (`for f in …; do
… || exit 1; done`), and `hack/ghcr-mirror_test.bats` fails on `main`
today and sorts before `select-e2e_test.bats`, so the aggregate target
never reaches these tests. I ran the 60 unit files individually instead:
59 pass, and the one failure is that pre-existing one, byte-identical to
the base commit and referencing nothing this branch touches. Also green:
`make helm-unit-tests`, `make rd-presets-check migrations-target-check
test-check-readiness`, `hack/select-install.sh --validate`, and
`pre-commit run --all-files` leaving a clean tree.
## Two things a reviewer should know, neither addressed here
`assert_selection` in `hack/select-e2e_test.bats` aborts the whole file
with `exit 1` rather than failing a single test. That is the only thing
that works under `hack/cozytest.sh`, which provides no real `run`,
`$status` or `skip`, and it is the idiom the existing tests in that file
already use. It becomes wrong when #3497 and #3498 flatten these onto
real `bats(1)`, where a failing assert should end one test and let the
rest report. Worth converting with that work rather than ahead of it.
`cozystack.mongodb-application` carries the identical missing operator
edge: `packages/apps/mongodb` renders `psmdb.percona.com/v1` while the
source depends only on `cozystack.networking` and
`cozystack.cozystack-engine`, so `cozystack.mongodb-operator` still
reaches no runnable suite and every change to it escalates to all 21.
Same defect class and same fix shape as the etcd edge, but it is a
second production install-ordering change and belongs in its own pull
request.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- The default etcd application now includes its operator and required
platform dependencies, ensuring the EtcdCluster provider is available.
- **Bug Fixes**
- Improved end-to-end test selection for application-specific changes,
shared components, inert paths, and unresolved or unclassified files.
- Full-suite escalations now provide clear reasons through standard
error.
- **Tests**
- Expanded coverage for test selection, dependency reachability, and
etcd application dependency declarations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
IvanHunters
|
f035da8a0f |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Resolve conflicts introduced by main: - Drop the FluxCD tenant addon (API type, deepcopy, values schema, RD) in favour of the GPU type main added; main removed the fluxcd addon wholesale. - Renumber the Phase 2 worker-pool adoption migration 54 -> 56, behind main's redis-operator (54) and fluxcd-removal (55) migrations; targetVersion 55 -> 57. - Accept the branch's removal of the in-chart talos-reconcile Job; it moved to the kubernetes-nodes chart, main only touched its comment. - Regenerate kubernetes values.schema.json / README and the kubernetes-rd ApplicationDefinition schema. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
467e431765 |
chore(ci): add area/cilium label and map the cilium PR scope to it
Cilium has enough open issues of its own (version pins and bumps, the Gateway API implementation, network policy, IPAM next to kube-ovn, LB-IPAM) that area/networking no longer narrows a search. Declare area/cilium in labels.yml, route the cilium scope of PR titles to it instead of area/networking, and update the scope table in the contributing guide. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
IvanHunters
|
cd72d81bb4 |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Reconcile the phase-2b split with main's configurable worker-health feature (MHC condition timeouts, maxNodeProvisionTime, podCpuLimit/ podCpuRequest), which landed on both charts after this branch forked: - kubernetes (parent): pool rendering stays removed; main's parent worker-pool additions do not apply post-split. - kubernetes-nodes (pool): absorb the feature; keep the directional parent-version skew guard and the immutable cluster/storageClass rules. KubevirtMachineTemplate content hashes are unchanged (render snapshot). - migration 54: carry podCpuLimit/podCpuRequest (KMT-hash inputs) and the cluster-wide readyUnknownTimeout/readyFalseTimeout/maxNodeProvisionTime into adopted pools, so adoption neither rolls nor resets a tuned pool. Covered by a new adopt-values bats case. - drop the parent chart's obsolete MHC-timeout suites; the kubernetes-nodes twins cover the feature. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
12e892325a |
docs(release): warn that v1.6.0's naming audit fails open
v1.6.0's own upgrade notes tell operators to run hack/seaweedfs-naming-audit.sh, and the copy that shipped in v1.6.0 silences every kubectl call with 2>/dev/null. A timeout or an RBAC denial therefore prints an empty "all clean" table indistinguishable from a genuinely clean fleet, and that output gates a runbook step that deletes PVCs. Anyone following those notes today can still get a false clean. Verified per tag: v1.6.0 has 11 blanket redirections and no run_kubectl wrapper; v1.6.1, v1.6.2 and v1.5.4 have 4 and 18 respectively, the fail-closed shape from #3436 and its backport #3474. Published changelogs are historical records and are not normally edited. This one is an exception on purpose, because the instruction it carries is still live and still wrong. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
eb8c19d4a1 |
docs(release): enrich the v1.5.4 changelog
The generated changelog is a good inventory but carries no operator-facing layer: no required-actions section, no runnable pre-upgrade checks, and the SeaweedFS 4.31 rename left as three isolated fix bullets that never say what to do about it. Every generated bullet is preserved; this adds to them. Widen the base to v1.5.2. v1.5.3 was tagged but its release was never published and its changelog PR was closed unmerged, so docs/changelogs/ v1.5.3.md exists on no ref and a v1.5.3 base leaves #3212 (persistent EFI/TPM state) and #3194 (filer postgres2 connection pool) documented nowhere at all. Coverage is now 82/82 commits in range. Correct three claims. The slot 45 divergence is a skip rather than an ordering problem: v1.5.4 is the first 1.5.x stamped targetVersion 46, and run-migrations.sh loops seq CURRENT (TARGET - 1), so a cluster at 46 runs 46..53 on the way to 1.6 and never executes 1.6's own slot 45. Its chart-side half was missing too, and it is not redundant -- a fresh v1.5.4 install is stamped 46 having never run any slot, so only the chart can reach it. The S3 checksum bullet named a barman-cloud plugin path that does not exist on this branch. The audit re-run warning names v1.6.0, not earlier releases. The script does not exist at v1.5.2; v1.5.4 is the first 1.5.x to carry it and carries the fail-closed version. v1.6.0 shipped the copy that silenced every kubectl failure, so a clean table from it is not evidence. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
cozystack-ci[bot]
|
c6238e1544 |
docs: add changelog for v1.5.4
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
cozystack-ci[bot]
|
be6e66786f |
docs: add changelog for v1.6.2
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
b76cb2091c |
test(e2e): record the runner VM shape and region in the e2e report
An e2e lane asks for a runner class, not for a machine. A flex shape is filled from whatever capacity the pool has at the time, so one lane can be served by two processor generations, and the region is a preference with a fallback. None of that reached the report, so a red run and a green one could not be compared by the hardware they ran on. The read is taken on the runner VM, before the sandbox container is created, because the metadata service answers on a link-local address the container has no guaranteed route to: an "unreachable" answered from in there would look exactly like a capture that had tried. It is wired into the sandbox apply target rather than into a workflow step, so every lane picks it up through `make prepare-env` on both the passing and the failing path, and cozyreport folds the file into the artifact from inside without going near the endpoint. The keys are fetched one at a time and the root instance document never is. That document carries the instance and compartment OCIDs, the image OCID, the display name, the hostname and the user metadata bag, and the report is a public artifact, so asking per key makes that property something the wire settles rather than something a filter has to be trusted for. It also matches where the shape sizing is published: it has its own subpath and is not in the root document at all. From the sizing object only the numbers are kept, so the rule holds for keys nobody has seen yet, and an answer that is not one scalar is named rather than printed. Every read carries a wall-clock ceiling, since a read that never returns costs the step its whole budget without reaching any exit status, and the first key doubles as the probe so a machine that is not an OCI instance pays one ceiling rather than one per key. Nothing here is fatal: this runs inside the one retried step in the job, so a runner with no metadata service leaves an "imds unreachable" line and an exit status of 0. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
IvanHunters
|
5297df5cf3 |
Merge remote-tracking branch 'origin/main' into pr-3315-lgtm
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> # Conflicts: # api/apps/v1alpha1/kubernetes/types.go # api/apps/v1alpha1/kubernetes/zz_generated.deepcopy.go # hack/e2e-chainsaw/_lib/run-kubernetes.sh # packages/apps/kubernetes-nodes/tests/render-parity.sh # packages/apps/kubernetes/README.md # packages/apps/kubernetes/templates/cluster.yaml # packages/apps/kubernetes/values.schema.json # packages/apps/kubernetes/values.yaml # packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml # packages/system/kubernetes-rd/cozyrds/kubernetes.yaml |
||
|
myasnikovdaniil
|
9f5550abdd |
fix(release): publish rewritten packages during RC promotion (#3795)
## What this PR does Fixes #3477. - Publishes a new packages OCI candidate after rewriting RC image tags to stable tags. - Pins the candidate digest in the stable installer without rebuilding container images. - Verifies the candidate against the prospective merge and original RC artifact before creating stable names. - Refuses promotion when the target maintenance branch lacks the candidate-aware pipeline. - Protects candidates referenced by open release PRs from retention cleanup. - Leaves already published v1.6.0 and v1.6.1 unchanged. ### Screenshots Not applicable. ### 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: ### Testing - `bats --no-tempdir-cleanup hack/promote-packages-artifact_test.bats hack/verify-promoted-packages_test.bats hack/promote-packages-contract.bats hack/promotion-retention-contract.bats` - `actionlint .github/workflows/promote-rc.yaml .github/workflows/pull-requests.yaml .github/workflows/pull-requests-release.yaml .github/workflows/retention.yaml` - `sh -n hack/promote-packages-artifact.sh hack/verify-promoted-packages.sh` - `git diff --check origin/main...HEAD` ### Release note ```release-note fix(release): publish the rewritten stable packages artifact when promoting a release candidate ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Release candidates now use temporary, immutable package artifacts verified before stable publication. - Stable promotions validate package contents, installer references, container digests, and release metadata before tagging. - Temporary promotion artifacts are retained while needed and cleaned up after expiration. - **Bug Fixes** - Improved handling of release labels, fork pull requests, and promotion pull requests. - Added end-to-end verification before release promotion. - Prevented invalid, ambiguous, or untrusted package references from being promoted. - **Documentation** - Updated release and promotion guidance for candidate verification, digest pinning, and transactional promotion. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
adf231513f |
fix(ci): correct two reachability defects in the selection graph
Both are cases of the walk reaching the wrong set of suites for a package, and both were found by enabling the site-router suite on #3426, which is the first suite in the tree to land downstream of kubevirt-cdi. `vm-disk-application` now maps to the `vminstance` suite. It mapped to a `vm-disk` suite that does not exist, which intersect_suites() drops, so the source reached nothing runnable and neither did kubevirt-cdi above it: every CDI change ran all 21 suites. The vminstance suite creates a VMDisk and asserts the DataVolume behind it (hack/e2e-chainsaw/vminstance/vmdisk.yaml, vmdisk-vmi.yaml), so it is the suite that covers both. A CDI change now selects `vminstance` alone. `cozystack.cozystack-basics` joins `cozystack.cozystack-engine` as a propagation hub excluded from the reverse-dependency walk. Every edge into it exists so a namespace or a platform-wide policy is in place before the dependent installs -- kubevirt-cdi says exactly that in its own source -- which is install ordering, not behaviour. The damage runs opposite to the engine's: the engine fans one change out to every app, while basics narrows down instead. It reaches no suite today so it escalates, but it sits upstream of kubevirt-cdi, so the first suite to land under CDI silently converts the platform's namespace-and-policy package from the full run to that one suite. On #3426 that is 22 suites down to `site-router` alone, with nothing in the output to say coverage was lost. Dropping the reverse edges keeps the package reachable, stops it propagating, and leaves a change to it running everything through the per-path escalation. Both rules have a test, each verified to redden with its rule removed: the mapping test falls back to the full-suite escalation, and the hub test -- which seeds the hazard by giving redis-application an edge onto basics, since no suite sits downstream of it in this tree yet -- selects `redis vminstance` instead of everything. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
cf2d9c6d13 |
test(e2e): split the tenant worker CPU time across QEMU threads
The node-join diagnostics phase samples the tenant worker's CPU counters twice on failure, but it reads them from the compute container's cgroup, and that cgroup holds every thread QEMU runs: the guest's vCPUs, QEMU's main thread, and its IO and worker threads. A container burning most of its quota while the guest reports no progress is two incompatible findings wearing one number, and no container-level counter separates them. Add a collector that execs into each Running worker's compute container, finds the QEMU process and copies every /proc/<pid>/task/<tid>/stat line into the artifact verbatim, with a legend beside them. Nothing is computed here. The legend names where utime and stime sit and why counting fields over whitespace reads the wrong column for exactly the threads the split is about: a KVM vCPU thread is named "CPU 0/KVM", that space is inside the comm parentheses, and every field after it shifts by one. It runs inside the sampling loop the two existing subjects already share, so it costs no second wait: both readings ride the interval already being paid, and the rate comes from the two files plus the stamps each one carries. Every way the read can come up short is a named outcome rather than an empty directory, and the legend and the pairing instruction are withheld from each of them: telling a reader to subtract two files asserts that both hold a whole reading. The probe shells out to nothing but getconf, whose absence it names, because a probe that needed cat would report a container missing coreutils as a container whose QEMU had exited, and it counts what it managed to read so that having read no process at all stays separable from having read them all without finding QEMU. Its stderr is suppressed on the walk rather than on either read: redirections are applied left to right, so a 2>/dev/null after an input redirect is not in place yet when opening the input fails, and a thread that exits between the glob and the read would otherwise put a shell complaint in the artifact beside a healthy capture. It also reports a QEMU that was named and then gone before its task files could be opened, because a process heading alone is enough to make the capture non-empty and would otherwise carry a legend describing columns the file does not have. The Pod listing the cAdvisor captures already share now takes its whole query as a parameter, so this collector reuses that read rather than carrying a third copy of it, and asks for the Running Pods only: a launcher left over from an earlier attempt costs the sibling caller nothing and would cost this one a bounded exec and a slot under its cap. The guard that holds the sampling group against the room the tenant snapshot needs now counts every capped walk in the loop rather than the two spelled the same way, so the new subject is inside the arithmetic rather than beside it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
959e90c9ab |
test(e2e): time the guest console capture and baseline it on a green run
A tenant worker's guest console is stamped in seconds since that guest's own kernel started, so a log ending mid-boot says how far the guest got and nothing about when. Whether it ends there because the guest started late or because the guest stopped printing decides which failure is being looked at, and neither instant was recorded: the read's own instant was nowhere in the file, surviving only as the artifact's mtime, which is metadata rather than content and so says nothing to anyone reading the log; and the guest's start only inside a describe dump that a capture with no silent console never triggers. Across three collected runs every capture exited zero and every log began at the first kernel line, so the reads were complete at both ends and the logs end where the guests stopped -- and which of the two happened could not be read off any of them. Each console log now carries the instant its own read started and returned, and CAPTURE-TIMELINE.txt beside it carries each Pod's phase, its start time and the start of its compute and console containers, from one bounded read taken once for the whole selector. Those subtract to how long a guest had been alive when it was read, which is what the log's last kernel stamp is then compared against. The console container is asked for in both status lists, since KubeVirt runs it as an init container and reading only one of them would report a container that started fine as one that never ran. A read that failed, a read cut off part way and a read that answered with nothing are recorded as themselves rather than left as a missing file, and the note names the error log only when there is one. The same capture also runs on the passing path, before the tenant is torn down and the Pod holding the stream goes with it. Every console this tree has collected came from a failure, so what a healthy boot looks like has never been measured, while "slower than usual" is the claim the failure keeps resting on. That caller asks for the pool's minimum rather than its maximum, since two workers booting is the same measurement as ten, and it warns in the job log rather than failing a suite that has already proved everything it exists to prove. Both paths write the same directory, so the capture also records which of them produced it. That capture costs real wall clock at the tail of an operation with a fixed ceiling, which is a way for a run that passed to be killed while collecting a debugging aid. So it is declined out loud when the ceiling no longer has room for it, against a reserve derived from its own cost and held in step with the timeout both suites give the operation. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Myasnikov Daniil
|
c28d7804af |
fix(ci): enumerate the inert .gitattributes instead of matching by name
The rule was broader than its justification. It read `(^|/)\.gitattributes$`, so it claimed every present and future .gitattributes anywhere, while the argument for it was about content: every one in this tree holds nothing but `linguist-generated` markers, which reach no build, chart or test. The filename does not carry that argument. .gitattributes can also set `filter`, `eol`, `working-tree-encoding` and `export-subst`, each of which changes what lands in the working tree and therefore what gets built, so a by-name rule would eventually make a live file inert with nothing to notice it. Nothing is lost by narrowing, because only three files ever reached the rule: api/.gitattributes and internal/crdinstall/manifests/.gitattributes are escalated by full_suite_pattern first, so entries for them would be dead. Measured against the base selector, those three are exactly the paths whose classification this branch changes, and they still select nothing. Same trade as the workflow enumeration a few lines up, and the same upkeep, which the comment now states: after adding or moving one, re-check with `git ls-files '*.gitattributes' '.gitattributes'` and confirm the contents are still only linguist markers. A .gitattributes not on the list is classified by whatever rule its path falls under -- the graph inside a package, the unclassified fall-through at the root -- and both fail safe. A test pins that root case, so a future widening back to a by-name match turns the file red. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b2702621d3 |
fix(ci): decide the per-app bats rule per path, not after the merge
Two defects in the hack/e2e-apps/ arm added by the previous commit. The arm set trigger_any=1 for whatever basename it derived and left the verdict to the final intersection. That makes the escalation depend on the REST of the diff: hack/e2e-apps/monitoring-oidc-system.bats alone emptied the selection and the backstop escalated, but the same file beside packages/apps/redis/templates/redis.yaml selected `redis` with nothing on stderr, where the pre-change selector ran all 21 suites. That is the merge-before-escalate shape #3330 removed from the graph walk, reintroduced one rule over. The basename is now membership-tested against the suite list on the spot and an unmatched one sets trigger_full=1 and names the file. POSIX case matches `/` with `*`, so the arm also sees nested paths, and the unanchored capture turned hack/e2e-apps/fixtures/postgres.bats into the "suite" fixtures/postgres -- right outcome by accident, reason line printing a path where a suite name belongs. The capture is anchored to one segment, and what the arm does with anything that is not a top-level <name>.bats is now stated rather than left to fall out of a regex: it is shared material for those suites the way hack/e2e-chainsaw/_lib/ is for the Chainsaw ones, so it escalates for the same reason. Both claims that said otherwise are corrected -- the arm's own comment and docs/agents/e2e-testing.md both asserted the deferred escalation was equivalent to the old fall-through, which held only for a single-file diff. The backstop comment drops the e2e-apps clause it gained last commit, since that route no longer reaches it. The header's "four of the seven" becomes count-free: this commit adds two escalating branches, so a hardcoded total was already stale. Tests: the mixed-diff case is the regression pin (the isolated case passes with the escalation deferred and cannot see the bug), plus a nested-path case and a non-.bats case. Both fixes are mutation-verified -- restoring either defect turns the file red. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
4d4f5edf92 |
fix(ci): escalate only the e2e bats files, not the whole unit lane
full_suite_pattern took every hack/*.bats, so all 63 of them ran the whole Chainsaw suite -- including the 60 that are the unit lane and that the e2e sandbox never executes. The root Makefile is the authority on the split and draws it at exactly the prefix this narrows to: BATS_UNIT_FILES := $(filter-out hack/e2e-%.bats,$(wildcard hack/*.bats)) The 60 files it keeps run under `make unit-tests`; the three it filters out -- e2e-prepare-cluster, e2e-install-cozystack, e2e-test-openapi -- are the ones packages/core/testing's recipes execute inside the sandbox, and those still escalate. So no Chainsaw suite can regress from an edit to one of the 60. Inert here is not a green gate with nothing behind it. `make unit-tests` is gated on pull-requests.yaml's `code` output, computed in the workflow as "any changed path outside docs/" and never from select-e2e.sh, so a bats-only pull request still runs the whole unit lane. The `e2e` job reads the same output, so it still installs the platform and runs the OpenAPI tests before skipping Chainsaw. Two tests: one pins the rule on both sides of the prefix and asserts an inert bats file beside an app path does not mask the selection, and one walks every hack/*.bats in the tree and requires the selector's verdict to agree with the Makefile's split, so a file named to fit neither lane surfaces here rather than on the next PR that touches it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
33428671fa |
fix(ci): classify hack/e2e-apps/ and packages/tests/ in select-e2e
Three path classes matched no rule in hack/select-e2e.sh and escalated to all 21 Chainsaw suites through the unclassified fall-through -- 7 of the last 150 merged pull requests between them. hack/e2e-apps/<name>.bats now maps to the <name> suite, off the basename exactly as the per-suite rule takes the name off a hack/e2e-chainsaw/<app>/ directory. Not marked inert: what is left in that directory after the Chainsaw migration is wired to nothing, and inert would bake that orphan status into the rule and go quietly wrong the day a lane runs those files again. A basename no suite carries drops out of the intersection at the bottom of the script and escalates through the backstop, which is the outcome those paths already had, so mapping optimistically loses nothing and gains the correct answer for every name that is a suite. packages/tests/ and every .gitattributes join inert_config_pattern, which is what the script's own header says to do with a genuinely inert path rather than widening the fall-through. packages/tests/ is a helm-unittest fixture chart: changing a test OF cozy-lib does not change cozy-lib, no PackageSource lists those paths as a component, and nothing installs them -- a change to the library itself still escalates through packages/library/. .gitattributes is matched by name wherever it lives because every one in this tree is a linguist-generated marker, which reaches no build, chart or test; one under a tree full_suite_pattern already covers still escalates, since that pattern is checked first. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
3c3f9450ae |
fix(ci): name the cause on every select-e2e escalation
Seven branches in hack/select-e2e.sh escalate to the full Chainsaw suite and four of them did it in silence: the shared hack/e2e-chainsaw/_lib/ and .chainsaw.yaml case, the full_suite_pattern match, a packages/ path no PackageSource claims, and the empty-selection backstop. "Run everything" is the same 21 suite names whichever rule produced it, so a reason line is the only thing that tells the seven apart. full_suite_pattern is the commonest cause of a full run by a wide margin -- 88 of the last 150 merged pull requests, 68 of them with no other cause -- so the usual answer to "why did this pull request run the whole suite" was the one the log never carried, recoverable only by re-deriving the selection by hand. Each new line names the path that caused it rather than only the rule, and the backstop names the directly-selected names it could not resolve. They go to stderr, never stdout: stdout is the suite list and both e2e lanes parse it, so a reason line there would be read as a suite name. A unit test asserts each of the four messages. A reason that regresses to silence changes no selection, so it is invisible to every other test in the file. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
ef96292b64 |
fix(ci): give the nightly e2e runner headroom over its guest vCPUs (#3833)
## What this PR does The e2e sandbox boots three QEMU nodes at `-smp 8`, so 24 guest vCPUs are committed before any tenant VM exists. Four jobs boot that sandbox. Three run on the 32-vCPU class; nightly ran on the 24-vCPU one, where 24 guests against 24 host vCPUs leaves nothing for QEMU's own emulation and IO threads, containerd, the kernel and the runner agent. The other lanes moved to the larger class at some point and nightly was not moved with them. The sandbox keeps its 24 vCPUs. That guest count is part of what e2e claims to test, and a smaller nightly cluster stops matching the one PRs run against. This is hygiene, not a fix. Measured boot-phase data across 18 runs says the load a workflow puts on the host dominates guest starvation, not the runner class: the lanes already on 32 vCPU boot guests slower than nightly did on 24, and class and workflow kind are fully confounded there, so neither separates out. Do not read this as making anything faster or as addressing a particular red run. What is left is the arithmetic. A 1:1 commit has no headroom at all, and two lanes running the identical sandbox on different classes is drift nothing was watching. The rest is a check, because the guest demand and the runner classes live in different files and nothing compared them. `hack/sandbox-runner-headroom.bats` reads `-smp` and `-m` times the loop count out of `hack/e2e-prepare-cluster.bats`, reads the runner class of every job that calls the sandbox make targets, and wants the runner strictly larger on both axes. Neither side is a hardcoded list, so bumping `-smp`, adding a management node, or adding a fifth lane on a smaller runner turns it red on the same commit. Memory is in there because the same argument applies to it and `-m 24576` sits on the same line as `-smp 8`, not because a class that violates it exists today. It stops instead of guessing when it cannot read something, since a guess comes out smaller than the real demand and passes a runner that cannot hold the sandbox. Refused: a missing or non-literal `-smp` / `-m`, a launch written outside any `@test` block or inside one left open at end of file, a `for` opener the strict pattern does not match, a `for` list holding anything but literal words, a `while` sharing the block, two launches or two loops in one block, and a `runs-on` with no `oracle-vm-<n>cpu-<m>gb` in it. Unparsed loop openers are counted separately from parsed ones, so moving `do` to the next line is refused rather than silently multiplied by one. There is also a cross-check that every workflow calling the sandbox make targets yields at least one matched job, so a renamed target turns the parser red instead of quietly shrinking what it covers. The derivation runs against the live tree, which passes whether or not those refusals still work, so each one is pinned by a fixture that feeds a synthetic file to the same program. That matters more than it sounds: two of the refusals above exist because the first version of this file did not have them, and a launch moved into a file-level helper would have halved the measured demand and stayed green. The numbers come from: ```bash sed -n '98,107p' hack/e2e-prepare-cluster.bats # three nodes, -smp 8, -m 24576 grep -rn 'prepare-env' .github/workflows/ # the four jobs that boot it grep -rn 'runs-on: oracle-vm-' .github/workflows/ # their runner classes ``` The rule also goes into `docs/agents/e2e-testing.md` next to the other environment invariants, so whoever adds the fifth lane picks its runner class from the docs instead of from a red unit suite. Checked that it bites, twice over. Fourteen mutations of the inputs: reverting the runner class, `-smp 11`, a fourth and fifth management node, `-smp cpus=8`, `-m size=24576`, `do` on the next line, `$(seq 1 3)` as the list, a `while` loop, the launch moved into a file-level helper, renaming the make target, `ubuntu-latest`, a matrix expression in `runs-on`, a hypothetical `32cpu-64gb` class, and a new lane on a 4-vCPU runner. Then eleven mutations of the parser itself, one per refusal, each of which kills exactly the fixture that names it and nothing else. Green on this tree, and the same numbers out of gawk, mawk and BSD awk. Closes #3831. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map against the diff. It changes one `runs-on` line plus a comment in `.github/workflows/nightly.yaml` and adds one file under `hack/`. The `ccp` trigger is moving or renaming something under `hack/`, or changing what a make target does; `hack/package.mk` and `hack/common-envs.mk` are untouched, and the new test is picked up by the existing `hack/*.bats` wildcard without the target changing. The `ansible-cozystack` and `talm` triggers are node prerequisites in `hack/e2e-prepare-cluster.bats`, which this reads and does not modify. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(ci): run the nightly e2e suite on a runner class larger than the sandbox's guest vCPU demand, and add a unit check that fails when an e2e lane loses that headroom ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated nightly end-to-end testing to run on a higher-capacity runner, improving support for multiple QEMU guests and required resource headroom. * Added validation to prevent sandbox runners from being undersized or oversubscribed. * **Documentation** * Documented the resource requirements and deterministic conventions for end-to-end sandbox testing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
527d4ea00f |
test(e2e): measure tenant worker CPU as a rate, and the sandbox nodes under it
Two instruments for the node-join failure the kubernetes-* suites keep hitting. Neither changes what the suites assert. The worker CPU capture now reads container_cpu_usage_seconds_total beside the CFS counters, and reads them twice a fixed interval apart instead of once. The throttled counters say a container met its ceiling and cannot say how much CPU it got, which are the two candidate explanations for a stalled worker: a guest held at a one-core ceiling, and a guest whose vCPU thread is rarely scheduled at all. Both leave throttled periods behind. Every counter involved accumulates from container start, so a single reading is an average over the whole uptime and no rate at all, and the profile under investigation is a guest that freezes for tens of seconds and then runs flat out: the average comes out low, the instantaneous figure comes out at the cap, and only the second separates it from a guest that is simply capped. The sandbox nodes are a new subject. The worker captures describe a cgroup, so a guest that is runnable and never scheduled looks from there like a guest that asked for nothing. Whether the sandbox node itself was given its turn is recorded only in the steal column of its own /proc/stat: no kubelet endpoint carries it, and crust-gather cannot reach it because PodSecurity refuses its node-shell Pod here, so the read goes over the Talos API, which is already how the report reaches those nodes for dmesg. The rows are captured verbatim with their column legend beside them rather than reduced to a percentage, because /proc/stat has no header, steal and guest sit next to each other, and guest is large on a node running VMs and already counted inside user. Both subjects are read on either side of one shared wait, so the pair pays it once. The wait is not the interval either subject's counters span, though: each pass walks its own nodes and the other subject's pass falls between a subject's two readings, and on the run this exists for those passes are the slow part. Each capture therefore stamps both ends of its read, so a rate comes from subtracting stamps rather than from assuming the knob, and the width of those brackets is the uncertainty. That wait is a measurement interval rather than a stand-in for a condition nobody wrote a wait for, so the e2e conventions carve it out by name instead of marking it as a fixed timeout to be replaced. The pair is gated below the serial console rather than above it. Its ceiling is sixteen bounded reads plus the interval, which is most of the phase budget, and admission gates only when a collector may start, so above the console a run that hit those bounds would leave the console never started rather than merely cut short. The console is the only capture that survives a worker which never reached apid, which is the shape this failure usually takes. That overrun is inside the term the budget derivation already allows for one late admission, and a guard holds the pair's ceiling against it. No collector here may leave an empty directory, since an empty directory reads as a healthy node: a missing binary, a missing talosconfig, a listing that never answered, a node with no address and a read that was cut off each land as a named note or a COLLECTION-FAILED.txt. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
db2ddfab2b |
fix(ci): give the nightly e2e runner headroom over its guest vCPUs
The e2e sandbox boots three QEMU management nodes at `-smp 8`, so 24 guest vCPUs are committed before any tenant VM starts. The nightly lane ran that sandbox on a 24-vCPU runner, a 1:1 commit that leaves the host nothing for QEMU's emulation and IO threads, containerd, the kernel and the runner agent. The tag, fork and pull-request lanes already run the same sandbox on the 32-vCPU class, so move nightly there too. This aligns headroom and claims no performance effect. Measured boot data shows the load a workflow puts on the host, not the runner class, dominates guest starvation, and the two are confounded in that data; no red run is attributed to the class here. The case is the arithmetic: a 1:1 commit leaves no headroom, and two lanes running the identical sandbox on different classes is drift nothing was watching. Add hack/sandbox-runner-headroom.bats to keep the two sides paired. It derives the guest vCPU and memory demand from the sandbox bats file, and the runner class of every job that drives the sandbox make targets, then requires the runner to be strictly larger on both axes. Neither side is a literal list, so raising `-smp`, adding a management node or adding a lane on a smaller runner turns it red instead of drifting apart silently. Input the derivation cannot read in full is refused rather than guessed: a guess produces a demand smaller than the real one, which passes a runner that cannot hold the sandbox, while a refusal names the line. That covers a launch written outside any `@test` block, a block left open at end of file, a `for` opener the strict pattern misses, a non-literal loop list, and a `while` loop sharing the block. The derivation runs against the live tree, which passes whether or not those refusals still work, so each one is pinned by a fixture feeding a synthetic file to the same program. Loosening the parser turns a fixture red rather than quietly widening what the check accepts. Record the rule alongside the other environment invariants in docs/agents/e2e-testing.md, so a new lane picks its runner class from the documentation rather than from a failing unit suite. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
a37a3178e5 |
docs(release): name all four sourcers of the image-ref enumeration
The library header still claimed three call sites. There are four: this PR made hack/verify-promoted-packages.sh source it to compare the candidate's refs against the rc artifact's. The historical drift story stays with the original three, since the verifier has only ever read the shared enumeration. The consumer table gained hack/promote-packages-artifact.sh under a bullet requiring every consumer of references to read all three shapes through the library, while the exception below it still named only hack/overlay-main-images.sh. The publisher sources nothing and needs nothing: it hands the whole tree to `flux push artifact` and touches one reference, the installer's own self-pin, by key with yq. Say that, rather than implying a gap it does not have -- the real exception remains overlay-main-images.sh, which does read references and does walk the tree itself. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
04fa8471fe |
docs(release): say what the gates actually guarantee
Three claims that were stronger than the code, plus one legibility fix. The PR gate's comment said a `packages/` edit "on the PR or its base" fails the status before merge. The base half is not true: GitHub fires no `pull_request` event when the base branch advances, it only recomputes refs/pull/N/merge, so a change landing on the base after this went green is never re-verified and the PR merges on the stale result. Checking the merge tree does make base drift visible WHEN the job runs, which is worth keeping — but finalize is what actually catches the drift case, after the merge and before any stable name. Say both, in the workflow and in the reviewer checklist, so nobody reads the green as covering more than it does. "Transactional at every consumer-visible name" is a judgement call dressed as an absolute, inherited from the "this dispatch performs NO registry mutation" it replaced. The dispatch now does write to the registry: the `promotion-*` candidate is a real, publicly listable tag. What it creates is no *stable* name — no vX.Y.Z, no :latest, no stable installer. Stated that way the sentence stops needing the reader to already know which writes were meant. The retention paragraph listed two of the conditions the workflow enforces. It now lists all of them, including the ones the workflow gained here: same-repository, bot-authored, `release-X.Y.Z` branch, the `release` label, exactly one valid digest pin, and protection from a base-branch tip as well as an open PR. Also: `flux pull artifact` reads its credentials from ~/.docker/config.json, which is why finalize now runs `docker login` — but its runner class is not one of the docker-using build ones and the setup step installs everything except docker. Check for it there, where the message can name what is missing, instead of failing with "command not found" halfway down the release path. And the verifier's exclusion list is a restatement of flux's `excludeOCI`, not a derivation — the CLI does not expose it — so the coupling and the pinned version are now written down next to the list. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
f21d277eba |
fix(release): retain live promotion candidates
Protect package candidates referenced by open release PRs while pruning abandoned run-specific promotion artifacts. Document the stable-candidate lifecycle and maintenance-branch rollout requirement. Assisted-By: Claude <noreply@anthropic.com> Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
b7b56530b9 |
docs(platform): explain system component memory limits (#3496)
## What this PR does Adds `docs/operations/system-memory-limits.md`, recording why system components carry memory limits — which is not recoverable from the values alone, and is the kind of thing that gets "cleaned up" by a well-meaning future change. The Talos userspace OOM handler (v1.12+) selects victims by ranking cgroups that lack `memory.max`. Three consequences are each easy to get wrong independently: only a limit grants immunity (a request just moves BestEffort to Burstable, which under `strictCgroupClassOrdering` means "killed second"); every container in the pod needs one, init containers included, because kubelet only sets pod-level `memory.max` when they all have it; and `PriorityClass` — `system-node-critical` included — has no effect whatsoever, because the handler walks cgroupfs and never talks to the API server. That last one is the trap, since setting it looks like a fix. The page also covers the two installer knobs and the constraint that makes them dangerous to lower (a defaulted limit below a container's own request fails admission), how to verify a pod cgroup actually carries `memory.max` rather than trusting the QoS class, what is deliberately left uncovered (Kubernetes static pods are Talos-managed; a handful of vendored containers expose no upstream resources knob), and how to relax the global-PSI backstop clause of the trigger for a cluster that trips it persistently. Ordering note: this documents behaviour introduced by the LimitRange and DaemonSet PRs, so it should not merge ahead of them. ### Downstream repositories Walked the trigger map against the diff. This adds one file under `docs/operations/`, alongside the existing pages there; nothing in the map covers that path, and the website keeps its own copy of user-facing documentation rather than generating it from this directory. A maintainer may still want a version of this on the website, which is a judgement call rather than a forced follow-up. - [x] No downstream repository is affected by this change ### Release note ```release-note docs(platform): document why system components carry memory limits, how the Talos userspace OOM handler selects victims, how to tune or disable the system-namespace LimitRange, and why PriorityClass does not influence any of it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added guidance on Talos userspace out-of-memory behavior and system component memory limits. * Documented namespace boundaries, installer tuning, validation constraints, rollout behavior, and verification steps. * Added instructions for inspecting OOM actions, identifying uncovered workloads, evaluating trigger expressions, and configuring optional overrides. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
340483fdf0 |
fix(hack): declare the shared package targets and the root test target phony (#3353)
## What this PR does `hack/package.mk` set `.PHONY` with `=` instead of `:`: ```make .PHONY=help show diff apply delete update image ``` That assigns a variable which happens to be named `.PHONY`; it declares nothing. `make -p` prints it back among the variables (`.PHONY = help show diff apply delete update image`) rather than as a special target, so none of the targets `package.mk` defines was phony in any package that includes it. Nothing in the tree collides with those names today, so this is preventive rather than a fix for present breakage. The failure it forecloses is quiet: with a directory named `image` beside a package Makefile, `make image` prints `make: 'image' is up to date.` and exits 0 without building. A file of that name is enough on its own where the rule has no prerequisites; where it has one, the recipe keeps running until that prerequisite is shadowed too. The list changed as well. `suspend`, `resume`, `check` and `clean` are defined in this file and were missing, and `check` is the one that matters most, since it is the prerequisite that keeps `show`, `diff`, `apply` and `delete` rebuilding. `update` and `image` are gone, because `package.mk` does not define them; the including package Makefile does. Naming an undefined target in `.PHONY` creates it as an empty target, turning `No rule to make target` (exit 2) into `Nothing to be done` (exit 0) wherever the rule is missing. That reaches CI too: `hack/build-matrix.sh` parses the root Makefile `build:` target and runs `make -C <pkg> image` per unit, so a package arriving in that list without an `image:` rule would go from a red job to a green one that builds nothing. `.DEFAULT_GOAL` on the line above keeps its `=`. That one is a real variable, so the assignment form is right there. A comment now says which of the two lines is which, since they read alike and one of them just changed shape. `hack/package-mk-phony.bats` covers the declaration and is picked up automatically by the `hack/*.bats` glob in `bats-unit-tests`. Five of its six tests cover `package.mk`: that the declared names are really phony with a file of each name sitting beside the Makefile; a control that mutates a copy back to the assignment form and requires the probe to report up-to-date, so the first cannot pass vacuously; that the declared list and the defined targets are the same set, checked both directions; and two pinning the `update`/`image` exclusion, one requiring the loud failure to survive and one extending the list in a copy and requiring it to disappear. The loud-failure test pins both the exit status 2 and the diagnostic. A wrapper that kept make's wording while returning something else would pass a check on the message alone. One point decides where the guarantee actually lives. The first test reads its names from the `.PHONY:` line, so it cannot see a name dropped from that line at all. The set-equality test is what guards every name's presence. One known gap is left in place. The first test decides by matching make's `is up to date`, which make prints only for a target that has a recipe, and a recipe-less alias never draws it: with no prerequisite both forms print `Nothing to be done`, and with a prerequisite that is itself out of date, which every target in this file is for being phony, the working form prints that prerequisite's recipe while the broken form prints `Nothing to be done`. The probe separates neither pair, so such a name would pass the first test either way. Measured on both shapes, not assumed. Every target `package.mk` defines has a recipe, so nothing is uncovered today; the control test does not close the gap either, since an alias is a defined target and the control requires `is up to date` from it, which turns it red against the probe rather than against the alias. Worth closing if an alias is ever added. One more for whoever edits the suite next: `hack/cozytest.sh` rewrites every bare `}` at column 0 into `return 0` followed by `}`. That is aimed at `@test` blocks, but it does not distinguish them from a plain shell function, so a helper factored out of these tests would silently become unable to return a non-zero status. Tests 1 and 2 repeat a loop rather than share one for reasons of their own, but this is the trap waiting for anyone who merges them. Two properties of the suite are worth stating, because both are easy to get wrong in the other direction. The set-equality test extracts the defined targets with a pattern that accepts a rule naming several targets at once, so `suspend resume: check` keeps both names in the set; anchoring the colon to a single name drops every name on such a line, and for a target nothing else depends on that goes unnoticed. And the control test probes the targets `package.mk` defines rather than the names on its `.PHONY:` line, because `is up to date` is only a meaningful answer for a name that has a rule: a declared-but-undefined name prints `Nothing to be done` in both the working and the broken form, so probing it in the control fails against the probe, while the set-equality test holds the message that names the defect. Last one, about how the tests find the repo. All six read it as `repo=$(pwd)`, which assumes the runner starts at the repo root. That holds for `make bats-unit-tests` and for the invocation the header documents, so nothing is broken, and the assumption is the ordinary shape in `hack/` rather than an outlier: about half the bats files there resolve repo-relative paths against the cwd the same way, several stating it in their own headers, and this is not even the only one spelling it with `$(pwd)`. What is worth naming is the failure mode: started elsewhere, these tests go red with `no '.PHONY:' declaration found in hack/package.mk`, which reads like a real finding rather than a wrong directory. The root `Makefile` had the same defect, and there it had already fired. `test` was not on its `.PHONY:` line and a `test/` directory sits beside it, so `make test` printed `'test' is up to date` and exited 0 without running anything, while `docs/agents/overview.md` advertises that command as the way to run the full suite. CI never used it: the workflow calls `test-controllers` and drives e2e out of `packages/core/testing`, so the cost fell on anyone following the docs by hand. One name on the declaration closes it, and the suite gained a sixth test that goes red without it. Every other undeclared target in that file is unshadowed today, so none of them is reachable the way `test` was, and they are left alone here. #3709 covers the same class in the four `packages/core/` Makefiles, which include only `hack/common-envs.mk` and so are out of reach of this change entirely. ### Downstream repositories `scripts/package.mk` in cozystack/external-apps-example was a byte-for-byte copy of `hack/package.mk` (identical sha256 before this change), carrying the same inert `.PHONY=` line. This PR splits the two files, and the copy was fixed separately. It landed the same nine names, and `update` and `image` are absent there for the reason they are absent here: `scripts/package.mk` does not define them either, and neither does any Makefile that includes it. What separates the two files now is the two-line comment added here, and nothing else. - [ ] 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: - [x] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: https://github.com/cozystack/external-apps-example/pull/4 - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed task declaration handling so supported commands are correctly recognized and executed. * Updated available commands by replacing `update` and `image` with `suspend`, `resume`, `check`, and `clean`. * **Tests** * Added coverage to verify command declarations, prevent regressions, and ensure invalid or missing commands report errors correctly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
ca7d2b0182 |
docs(platform): explain system component memory limits
Add docs/operations/system-memory-limits.md, recording why system components carry memory limits. That reasoning is not recoverable from the values themselves, and is exactly the kind of thing a well-meaning later change removes as redundant. The Talos userspace OOM handler (v1.12+, enabled by default) selects its victim by ranking cgroups that lack memory.max. Three consequences are each easy to get wrong on their own: only a limit grants immunity, since a request merely moves BestEffort to Burstable and under strictCgroupClassOrdering that means "killed second"; every container in the pod needs one, init containers included, because kubelet sets pod-level memory.max only when all of them have it; and PriorityClass, system-node-critical included, has no bearing on it whatsoever, because the handler walks cgroupfs and holds no Kubernetes client. That last one is the trap, since setting a priority looks like a fix. The page also covers the two installer knobs and the constraint that makes them dangerous to lower (a defaulted limit below a container's own request fails admission), how to confirm a pod cgroup really carries memory.max rather than trusting the QoS class, what is deliberately left uncovered, and how to relax the global-PSI backstop clause of the trigger on a cluster that trips it persistently. Every Talos claim is checked against the siderolabs/talos v1.13.6 tag, which is the version packages/core/talos currently ships, and every Cozystack claim against the tree. Ordering: this documents behaviour introduced by the system-namespace LimitRange and the node-DaemonSet resource changes, so it should not merge ahead of them. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
IvanHunters
|
09dc7f47fb |
feat(backups): add a native MongoDB (psmdb) backup-strategy driver (#3562)
## What this PR does Adds a native MongoDB (Percona Server for MongoDB / psmdb) backup-strategy driver so `apps.cozystack.io/MongoDB` reaches feature parity with the CNPG, MariaDB and ClickHouse native drivers — logical backups, restore into a differently-named instance, and point-in-time recovery — all through the unified `BackupJob` / `RestoreJob` / `BackupClass` / `Plan` interface (closes #3545). The driver orchestrates the operator's `PerconaServerMongoDBBackup` / `PerconaServerMongoDBRestore` CRDs: - **BackupJob**: creates a one-shot `PerconaServerMongoDBBackup`, polls `status.state`, and persists the S3 destination + storage descriptor (by reference — Secret name only, never a raw credential) onto the Cozystack `Backup` artifact. - **RestoreJob**: drives `PerconaServerMongoDBRestore` via `backupSource` (read back from the source backup status, with a snapshot fallback for a reaped operator CR) so the same artifact restores in-place **or** into a differently-named instance. PITR is exposed as `spec.options.recoveryTime` (RFC3339), matching the CNPG driver's option exactly; unrecognised `spec.options` keys are surfaced as a `UnknownRestoreOption` Warning event rather than silently dropped. Because psmdb resolves the backup storage by name against the cluster's `spec.backup.storages`, the driver names the chart's storage (`s3-storage`) via the `cozy-default-mongodb` Strategy rather than injecting it, and surfaces a clear `Ready=False` precondition when a target has backups disabled instead of hanging until the deadline. **Wiring** - `api/backups/strategy/v1alpha1`: `MongoDB` strategy type + generated CRD/deepcopy. - `internal/backupcontroller`: `psmdbstrategy_controller` + `psmdbtypes` / `mongodbapp` partial-schema mirrors; BackupJob/RestoreJob dispatch, supported-kinds, cleanup-on-delete branch, and scheme registration. - `backupstrategy-controller`: `cozy-default` MongoDB route, `cozy-default-mongodb` Strategy CR, and `psmdb.percona.com` RBAC (get/list/watch, +create on backups/restores, no delete — the driver does not own the S3 archive). - `apps/mongodb`: a `backup.insecureSkipTLSVerify` value (psmdb's s3 storage has no CA-bundle field) so backups can target Cozystack's self-signed in-cluster seaweedfs. Default `false`, fully backward-compatible; regenerated schema/README/RD. **Tests + e2e** - Unit tests for the driver: applicationRef validation, ensure-idempotency (backup + restore), the backups-enabled/storage precondition gate, `backupSource` resolution (live-preferred / snapshot fallback / no-destination), snapshot round-trip, the producer-seam (`createMongoDBBackupArtifact`) driver-metadata, the cleanup-on-delete no-Velero-fall-through guard, and `recoveryTime` → psmdb-UTC mapping + unknown-option reporting. - `examples/backups/mongodb/` + a `mongodb-2-backup-roundtrip` Chainsaw suite that drives `run-all.sh` end-to-end (a sentinel document round-trips through S3 into a restored copy while the source stays untouched), per `docs/agents/e2e-testing.md`. - `docs/operations/backup-classes.md` updated (new route, the app-owns-the-storage model, and the `recoveryTime` PITR usage). Verified end-to-end on a live dev cluster against the platform-default `cozy-backups` bucket: backup via `cozy-default` reached the bucket and the Cozystack `Backup` went `Ready`, and a to-copy `RestoreJob` restored the sentinel into a differently-named instance with the source left intact. ### Screenshots N/A — no UI changes (controller, chart values, CRD, docs and tests only). ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` file by file against the diff: - **cozystack/website** — the only app-facing change is `packages/apps/mongodb` (a new `backup.insecureSkipTLSVerify` value + regenerated `README.md`). The managed-app reference page is regenerated from the package `README.md` by the release bot for apps already in the website `Makefile` (mongodb is one), so it refreshes automatically on the next tag — no manual follow-up. `docs/operations/backup-classes.md` is not a website-mirrored doc in the trigger map. - **cozystack/terraform-provider-cozystack** — the one candidate trigger ("a field added to an app's `values.schema.json`") lands inside the mongodb `backup` block, which the provider's `internal/provider/mongodb_schema.go` explicitly states it does **not** manage ("The deprecated backup block, the bootstrap restore block, and detailed shardingConfig are not managed"). No enum, default, `release.prefix`, Secret/Service name, or hand-typed CRD (`Plan`, `RestoreJob`) changed; the new `strategy.backups.cozystack.io/MongoDB` CRD is opaque to the provider. Not affected. - All other repositories — no matching trigger (no `hack/` layout or make-target change, no namespace rename, no `ApplicationDefinition` semantics change, no `packages/core/platform` values, no telemetry metric or `cozy-proxy` label rename). - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(backups): add a native MongoDB (psmdb) backup-strategy driver — logical backup to the cozy-default BackupClass, restore into a differently-named instance, and point-in-time recovery via spec.options.recoveryTime ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added MongoDB backup and restore support, including scheduled and on-demand backups, restores to new or existing clusters, and point-in-time recovery. * Added configurable S3 TLS certificate verification for MongoDB backups, defaulting to secure verification. * Added MongoDB support to the default backup configuration. * **Documentation** * Added MongoDB backup and restore examples, workflows, and operational guidance. * **Tests** * Added automated coverage for MongoDB backup, restore, cleanup, and end-to-end scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
59878850f9 |
test(e2e): capture worker CPU throttling counters on node-join failure (#3723)
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## What this PR does
When a tenant worker misses the node-Ready deadline, the failure bundle
currently cannot tell two different causes apart: a worker held at its
own CFS ceiling, and a worker losing host CPU it was entitled to. Both
look identical from outside the Pod, and node-level utilisation answers
neither, because it shows what the node used rather than what the
container was allowed.
This adds a collector that reads the CFS counters and the ceiling itself
from the kubelet's cAdvisor endpoint, for every tenant worker on every
node in the sandbox. The counters are the only place that difference is
recorded, and nothing else in the tree collects them for these workers.
The motivating measurement is on nightly run 31290999592: the `compute`
container reports 0.968 and 0.920 core against a `limits.cpu` of 1,
which is 92-97% of a one-core ceiling. Whether the container ever
actually met that ceiling is exactly what the artifact cannot say, and
what these counters answer.
Most of the change is the failure taxonomy rather than the read. An
empty capture is the dangerous outcome here, because zero bytes reads as
a container that never hit its ceiling -- the conclusion the collector
exists to stop a reader reaching by default. So every way the capture
can come up short gets its own sentence: the kubelet was not read, the
read was cut short part way, the metric stream could not be read back on
this runner, the kubelet answered and carried no series for this
namespace, the namespace was there and the worker Pods were not. The one
non-failure outcome is written down too: a container with no CPU limit
puts a single line on the wire, which is also what a truncated read
leaves behind, so it says which one it is.
### Ordering
The collector runs first among the gated collectors, ahead of the guest
serial console. Its ceiling is four bounded reads, about 100s of a 480s
phase budget, so on a slow run the console capture starts that much
later. That is a deliberate trade and not a side effect of putting the
cheapest first: cost is not what settles the order, survivability of the
answer is. The console evidence cannot be re-taken, and this collector's
question has no other answer in the tree, so both go ahead of the
captures whose state is partly recoverable from reads above them.
### Known limits, named rather than left to be found
- The outcome ladder enumerates combinations of read status, filter
status and result emptiness. Every new source of partial output needs a
new rung, and that has already happened twice while this branch was in
review. Deriving the label from the three axes instead would make a new
source land in an existing axis.
- The uncapped note is decided over the whole per-node file, so one
quota line anywhere in it suppresses the note. That is the conservative
direction -- it never claims uncapped over a file that shows a ceiling
-- but it makes the note rare.
- The messages say `see read-error.log` while the file on disk is
`<node>.read-error.log`. That is the convention the neighbouring
serial-console collector already uses, and changing one side alone would
make the two inconsistent.
- The `mkdir -p` for the report directory is unchecked, as it is in four
neighbouring collectors in the same file. Worth fixing across the file
rather than in one collector.
- The section letters in the failure block no longer track emission
order. They already did not before this change; this adds one more entry
to the sequence.
- The count of bats files sourcing the library, in
`docs/agents/e2e-testing.md`, is corrected here from three to seven.
Nothing pins it, so it will drift again.
- The node walk iterates `for node in ${nodes}` unquoted, so pathname
expansion applies alongside word splitting. Node names are RFC1123 so it
cannot bite today; `while IFS= read -r node` would make that structural
instead of incidental.
### 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:
### 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): capture tenant worker CPU throttling counters and their CFS ceiling when a node-join fails, so a worker held at its own limit can be told apart from one losing host CPU
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added worker CPU-throttling diagnostics to Kubernetes troubleshooting
reports.
* Reports now identify CPU quota gaps, partial reads, filtering issues,
and incomplete results.
* Added safeguards for bounded node traversal and configurable
collection budgets.
* **Bug Fixes**
* Improved failure handling when diagnostic commands, timeouts, or
metric reads are unavailable.
* **Tests**
* Expanded coverage for CPU-throttling capture, node-join diagnostics,
timeout behavior, configuration validation, and partial failures.
* **Documentation**
* Updated diagnostic collector counts and collection-order
documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
94c41d100f |
fix(hack): fail when a chart runs no Helm unit test suites
helm-unittest is fail-closed on a suite it cannot parse and on one declaring `tests: []`, and fail-open on suites that are not there at all: it prints "Test Suites: 0 passed, 0 total" and exits 0. Measured on plugin 1.1.1 against a throwaway chart, all three forms. hack/helm-unit-tests.sh judges each package by that exit code alone, so a chart whose suites were deleted, moved, or renamed past the tests/*_test.yaml glob reports success having asserted nothing. Moving packages/system/velero/tests aside reproduces it: the script prints "All Helm unit tests passed" and exits 0. Check the captured output for the zero-suite line and fail the package when it appears. Neither `--strict` nor an explicit non-matching `-f` glob does this on its own; both exit 0. Matching that line names the cause where it applies, but the set of ways to run nothing is not closed: a recipe that never invokes helm-unittest, and a `test:` rule make finds nothing to do for, both print no marker at all, so no further match reaches them. So also require the line a real run always emits, a `Test Suites:` count of at least one. Every way of asserting nothing exits 0, and asking what the run asserted is the question that has an answer. All 77 packages the script visits emit that line today. The tradeoff is that a `test` target which legitimately runs no helm-unittest would need renaming or an opt-out, which is the bargain the zero-suite check already struck. That is a contract contributors have to know about, so docs/agents/overview.md states it where it describes the sweep. Because the check matches helm-unittest's own summary wording, a plugin upgrade that reworded it would fail every package at once while blaming each package's Makefile. The error text says so, so the reader of a tree-wide red has the real cause in front of them rather than 77 copies of the wrong one. The discovery gate has the same shape from the other side. `make -C dir -n test` succeeds against a file-backed target, so a path named `test` beside a package Makefile whose rule is not phony would make both the gate and the run exit 0 without the recipe firing. Of the packages the script visits that define a `test` rule, 18 declare it phony, and no such path exists today. Match what make reports rather than looking for the path, because a package that declares the target phony runs its recipe whatever sits next to it, and refusing that would be a false failure. The quoting around the target name differs between make 3.x and 4.x, so the match accepts either opening quote; the trailing quote alone would also match a sub-make reporting another target whose name ends in "test". Running the suite now captures output rather than streaming it, because a pipeline's exit status in POSIX sh reports the last command rather than make. The capture file is allocated once and removed by the EXIT trap that already owns the failure list, and the run is pinned to LC_ALL=C: all three checks match English wording, one on make's and two on helm-unittest's, so a localized make would disarm the first of them silently, which is the failure mode this guard exists to remove. The locale reaches every package's recipe, not only make. Extend hack/helm-unit-tests.bats with eight tests. Three pin the new refusals; two pin what they must not refuse, a phony target with a colliding path and a sub-make reporting an unrelated target up to date; one pins that a chart which does run suites still passes; two pin behaviour that was already there, namely that a failing package is still reported by name and a package with no `test` rule is skipped rather than failed. That last one carries a second package that does run a suite, because with only the target-less package present the script takes its early exit above the failure summary and the assertion holds whatever the script did. The fixtures stub make output instead of invoking helm, so they need no plugin installed. Both tests that assert a package is named in the failure summary match the summary's " - " prefix rather than the bare path. The script announces "Running tests in $dir" before it runs anything, so the bare path is present in the output whatever the summary does; matching it asserted nothing, and the summary could stop naming packages entirely with the suite still green. The failing-package fixture reports a passing suite count alongside its non-zero exit, so that the exit status is the only thing that can decide the outcome. A fixture that fails without printing the marker is refused by the positive-evidence check first, which leaves the exit-status branch unpinned. That shape occurs in the tree: a `test` target that runs helm-unittest and then a second step, a render-parity script or a vendored-version guard or go tests, emits the marker and still fails. The two tests already in that file cover a package reached under packages/tests, and their fixture printed a marker and nothing else. That is the shape the positive-evidence check refuses, so the fixture now also emits the line a real run emits; without it both would fail for a reason unrelated to what they assert. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b8a5b9ba1d |
test(e2e): capture worker CPU throttling counters on node-join failure
When a tenant worker misses the node-Ready deadline, the sandbox node it runs on is routinely at half its capacity with a clean kernel log. In that state "the host starved the VM" and "the VM was held at its own CFS ceiling" produce identical evidence: node-level utilisation reports what the node used, never what the container was allowed. The CFS counters are the only place the difference is recorded, and nothing in the tree collected them for the tenant workers, so every diagnosis of this failure so far had to be inferred from outside. Read them from the kubelet's cAdvisor endpoint on the node hosting each worker. Five series carry the whole answer and cAdvisor publishes all of them per container, so nothing is computed here: the scheduled and throttled period counts, the time spent throttled, and the quota and period that form the ceiling. The throttled counters alone say a container hit some ceiling, not which one, and a VM capped at one core and a VM capped at eight are the same number without the quota. Absence of the quota series is itself a reading rather than a gap. cAdvisor emits it only when the quota is non-zero, so a container reporting the period and no quota is one running uncapped - which is one of the two answers this collector exists to distinguish, arriving without being derived. Reading the kubelet rather than the container matters beyond convenience. A reader running inside the container shares the cgroup it measures, so it slows down in proportion to how much the answer matters and would need a budget sized for the pathological case; the kubelet is outside that cgroup. It also drops any dependency on what the container image ships and on how the host lays cgroups out. The namespace is not a worker filter and is not used as one: it also holds the tenant control plane, whose apiserver and etcd carry CPU limits of their own and can be genuinely throttled. The Pod name carries the second half of the filter, so a throttled apiserver cannot answer under a heading that says worker. The walk is over nodes rather than Pods, because the endpoint is per node and one read covers every worker it hosts. It is gated on the diagnostics phase budget, and placed first among the five gated collectors: the budget declines whatever has not started when it runs out, its own ceiling is four bounded reads rather than the minutes the four after it can spend, and it is the only one whose question has no other answer in the tree. That order is a trade, stated here because it is a decision rather than a detail - it puts the guest serial console one place further from the budget, and that capture is the only one describing a worker which never reached apid. Both reads take the block's own read-budget knob rather than a literal equal to it, so lowering that knob lowers these too. Each keeps its own exit status rather than routing through the shared helper, which always returns zero by design: this collector has to tell "the kubelet was not read" from "the kubelet answered and carried no worker", and only the status separates them. The stream is captured whole and filtered after, since a pipeline reports its last command's status and would otherwise hand that decision to grep. Every way a read can come up short is named rather than left as zero bytes: a kubelet that was not read, one that answered with nothing for the namespace, a stream cut short, and exit 124 - which cannot distinguish this collector timing out from the read exiting 124 on its own - each get their own line, and each says whether an error log exists to explain it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
91b9f58057 |
fix(hack): declare the shared package targets phony
package.mk set .PHONY with `=` rather than `:`, which assigns a variable named .PHONY instead of declaring anything phony, so none of the targets package.mk defines was phony in any package that includes it. Declare exactly the concrete targets package.mk defines: help, show, diff, apply, delete, suspend, resume, check and clean. The earlier list named update and image, which package.mk does not define — naming an undefined target in .PHONY creates it as an empty target, so `make image` and `make update` would report success without running anything in the many packages that lack those recipes. It also omitted check and clean, the targets a same-named file or directory could actually shadow. The neighbouring .DEFAULT_GOAL is a variable and correctly keeps its equals sign. A comment now records which of the two adjacent lines is which, so the corrected one does not pull the other into its shape by analogy. The trigger map in docs/agents/contributing.md called the two files a byte-for-byte copy, which was exact rather than loose: they were the same object. They stop being one here, so the entry now records that they were identical and no longer are, and asks for a diff-and-port, the wording the map already uses for the other copy that has diverged. It deliberately does not enumerate which lines differ: that set moves as the copy catches up, and an entry that names it would be wrong again by the next port. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
e944619e98 |
docs(release): add changelog for v1.6.1 (#3551)
This PR adds the changelog for release `v1.6.1`.
✅ Changelog has been automatically generated in
`docs/changelogs/v1.6.1.md`.
|
||
|
IvanHunters
|
64815cf12b | Merge remote-tracking branch 'origin/main' into pr-3315-lgtm | ||
|
Andrey Kolkov
|
b496fb1ae8 |
docs(backups): note to-copy MongoDB restore needs target creds that can read the source bucket
A to-copy restore reads the dump from the source backup's bucket/endpoint but authenticates with the target cluster's own S3 credentials (the source release's Secret dies with it in a DR scenario). Document that the target's credentials must therefore be able to read the source bucket - the common same-bucket cozy-backups case - and that a cross-account/bucket target fails loudly with AccessDenied rather than silently restoring nothing. Addresses the MINOR credential-scoping note from PR review. Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
0d4a3e3134 |
refactor(backups): make MongoDB PITR option uniform with CNPG + warn on unknown restore options
PITR: replace the bespoke spec.options.pitr{type,date} blob with a single
spec.options.recoveryTime RFC3339 string, matching the CNPG strategy's option
exactly (CNPGRestoreOptions.RecoveryTime). The driver converts recoveryTime to
the psmdb Restore's pitr{type:date,date} in the operator's "YYYY-MM-DD HH:MM:SS"
UTC format. An empty recoveryTime restores the snapshot as-taken (documented
difference from CNPG, whose empty value means latest WAL — a psmdb logical
backup is already consistent). A non-RFC3339 recoveryTime fails terminally.
Unknown options: parseMongoDBRestoreOptions now also returns the set of
spec.options keys the driver does not recognise; reconcileMongoDBRestore emits a
UnknownRestoreOption Warning event per unknown key (and logs it) instead of
silently ignoring it — so a "recoverytime" typo no longer silently degrades a
PITR restore to a snapshot restore. (BackupJob carries no spec.options blob, so
there is no backup-path equivalent; the only backup-time input is the free-form
BackupClass parameters used as template vars.)
Docs + examples updated to recoveryTime; tests cover the RFC3339→UTC mapping,
empty/invalid recoveryTime, and unknown-key reporting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|