mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 14:02:56 +09:00
fd8d378b735cbc3a6786fadece3dea84f49a62e2
5624
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
fd8d378b73 |
fix(kubernetes): rotate bootstrap-token Job on either token half; review nits
Fold bootstrapTokenSecret into the Job content hash alongside bootstrapTokenId. talos-secrets.yaml regenerates the two token halves independently, so hashing only the id left a rotated secret no-oping a completed Job and the tenant stuck on the stale bootstrap-token. Also address review feedback: drop a review-round reference from the tracked test, correct a stale hash in a test comment, pin the alternate-image hash case exactly instead of by regex, and fix a comment indent in talos-reconcile-job.yaml. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
035bde1920 |
fix(kubernetes): correct bootstrap-token Job rationale and make it churn-free
Address review of the main-phase bootstrap-token Job. The deadlock the header claimed cannot occur on this release: the kubernetes cozyrd carries helm-install-disable-wait, which cozystack-api maps to Install.DisableWait and Upgrade.DisableWait, so there is no main-resource wait phase in front of the hooks. Rewrite the rationale around the real reason to leave the hook shape: a post-upgrade hook never runs on a release whose upgrade keeps failing, so the token Secret never lands. Fix the stale 'stays a post-install hook' note in talos-reconcile-job.yaml and the 'fails the chart upgrade loudly' comment, which is untrue once Helm no longer waits on the Job. Give the immutable Job a content-hash name suffix over its whole rendered spec, mirroring talos-reconcile: a static name would fail an upgrade that lands while the Job is live (or inside its TTL) with 'spec.template: field is immutable'. Fold the looked-up bootstrapTokenId into the hash so a rotated token also rolls the Job instead of Helm no-oping a completed one. Make the admin-kubeconfig volume optional and gate the Pod on the shared wait-for-kubeconfig init container, so it waits in init (uncounted by backoffLimit) instead of FailedMount at t0. Extend the template tests for the hashed name, its rotation on an image change, and the optional volume + init container. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
542ab899c2 |
fix(kubernetes): break bootstrap-token Job out of Helm hook to unblock fresh installs
The bootstrap-token-tenant Job was wired as a Helm post-install/post-upgrade hook. post-* hooks fire only after Helm's wait phase resolves, and the tenant kubernetes release wait blocks on MachineDeployment Ready, which blocks on nodes registering, which needs the kubelet CSR path — for which the very bootstrap-token this Job produces has to already exist in the tenant kube-system. Fresh installs and any HR reconcile that finds MD in a non-Ready state deadlock; HR.upgradeFailures climbs while post-upgrade hooks never get their turn to run. pre-install/pre-upgrade would not help either: they fire before non-hook templates apply, so <release>-admin-kubeconfig (materialised by Kamaji after KamajiControlPlane is applied) is not yet available. Move the ServiceAccount and Job to the main install phase, matching what talos-reconcile already does in this chart. Helm applies them together with the KamajiControlPlane and talos-secrets; the Job Pod stays pending until Kamaji publishes the admin kubeconfig Secret (backoffLimit covers the retry window) and then completes before MachineDeployment can converge. Helm's wait treats the Job as another resource to reach terminal state and does not gate it on MD. Extend the existing template test to pin the absence of helm.sh/hook on both the ServiceAccount and the Job, so this cannot regress silently. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
68d83cb686 |
test(e2e): add a runner-layer fixed-work canary around the node-join wait (#3919)
## What this PR does The diagnostics around the node-join wait already carry counter pairs that report time in the CPU rows, events in the KVM exits, and instantaneous values and running maxima in the files beside those. None of them is a fixed quantity of work, so none can separate a machine that was given fewer turns from a machine that took its turns and got less done with them. This adds a collector that runs two fixed amounts of work on the runner VM and times them: an integer loop and a streaming write, each under a ceiling of its own. It takes one sample before the node-join wait and one after it, on the failing and the passing path alike. Each sample is an absolute reading rather than half a difference, so one alone is still readable against the expected ranges the capture prints, and the pair says whether a slowdown began inside the interval it brackets or was already there going into it. The collector sits outside the three counter pairs that bracket the same wait rather than among them, because it occupies a core while it runs, and a burn placed inside one of those intervals would be charged to the join window it is supposed to measure. A new bats suite of 57 tests covers the collector. The existing node-join guards were extended to price it against the phase budget and to keep the spend order documented in both kubernetes suites in step with the order the block actually runs. On the budget: the sample taken before the wait is bounded at two arms of the canary ceiling plus grace, so it can add up to fifty seconds ahead of the wait, and the guard prices it at exactly that, while the guard's total stays below an honest worst case because collectors it already priced can spend more than the figures it prices them at. This branch was reviewed extensively before it was opened, and what is known to be incomplete is written down rather than left implied: the words describing how each floor sits against its healthy band are marked in the code as written rather than derived, so they are the part that can go stale when a constant moves, and the phase budget residuals are tracked in the issue the comments beside them cite. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. The diff is five files: the e2e diagnostics collector library, its two new and extended bats suites, and the two chainsaw suite definitions that document the spend order. The closest call is `cozystack/ccp`, whose skills gate on `hack/package.mk` and `hack/common-envs.mk` and drive `make generate`. Neither file is touched, nothing under `hack/` is moved or renamed, and no make target changes what it does: the new suite is picked up because `BATS_UNIT_FILES` already globs `hack/*.bats`, so the file list grows and the target's contract does not. Nothing else matches. No package is added, renamed or removed, so the website app lists are unaffected. No `values.schema.json`, version enum, default or `ApplicationDefinition` changes, so the Terraform provider is unaffected. `hack/e2e-prepare-cluster.bats` is untouched, so the node prerequisites that `ansible-cozystack` and `talm` restate are unchanged. `hack/update-crd.sh`, the `release.labels` convention and the chart source kinds are untouched, so `external-apps-example` and `cozyhr` are unaffected. No label, annotation or metric name changes, so `cozy-proxy` and `cozystack-telemetry-server` are unaffected. Minimum node and network requirements are unchanged, so `examples` is unaffected. - [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 test(e2e): the node-join diagnostics now time two fixed amounts of work on the runner before and after the wait, so a run that spent its wall clock without completing work is visible in the report. No user-facing change. ``` |
||
|
Aleksei Sviridkin
|
7ec5dff93a |
test(e2e): add a runner-layer fixed-work canary around the node-join wait
The counters the node-join failure path collects are read in whatever unit their source publishes: time in the CPU rows, events in the KVM exits, instantaneous values and running maxima in the files beside those. Not one of them carries a unit of work of its own, so a machine that spent every tick and got a fraction of the work done reads healthy in all of them. Add a collector that carries its own unit of work. It runs two fixed amounts of work at the runner layer and times each: a loop over a handful of integer-valued scalars, which pressure on the shared cache or the memory controller moves little, and a store stream through blocks larger than the last-level cache one core can allocate into, which is dominated by it. Fixed work gives each reading a scale of its own, so a red run can be read with no green one beside it, and the capture states the expected range and where the estimate comes from. Sampled where the three counter pairs already are, before the wait on every run and after it on both paths, but outside their brackets: it occupies a core rather than reading a file, and a burn inside one of those intervals would land in the interval each pair divides by: runner-kernel CPU time charged to the join window, and for the two counting the guests, a core taken away from what they count. Two arms, each under a twenty-second ceiling with the five-second kill grace behind it, so fifty seconds at worst against the 420s diagnostics budget. The budget guard derives that from the ceiling the source declares, and the phase-warning guard names the canary among the collectors that keep running when timeout is missing. An exit shaped like the ceiling is attributed to it only when the whole bound was on the clock: an outside kill produces the same status, and read by status alone it would publish exactly the finding this collector exists to detect. A reading below the range the legend states, and an arm the ceiling stopped before it could produce one, reach the job log from all three reporting sites, each naming where it was taken, the way a missing reading does from the two outside the diagnostics block. On the passing path the report that holds the figure is the artifact nobody downloads. Each floor is printed in the legend it comes from and sits where a bounded arm crosses it before the ceiling stops the arm, because a floor the ceiling reaches first never fires: the arm is cut off before its rate can fall that far, and every alert then arrives as a bound rather than as the slowdown the floor was meant to name. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
899529b386 |
chore(ci): add area/cilium label and map the cilium PR scope to it (#3914)
## What this PR does Adds an `area/cilium` label and routes the `cilium` PR scope to it. Cilium has more than 20 open issues of its own right now (the pinned version and its bumps, the Gateway API implementation, network policy, IPAM next to kube-ovn, LB-IPAM), and `area/networking` no longer narrows a search. #3913 is the tracking issue that uses the label. The label already exists in the repository, created by hand so the open issues could be labelled before this lands. The sync workflow runs with `delete-other-labels: false`, so nothing is at risk in between; this PR makes `labels.yml` the owner of it and updates the scope table in `docs/agents/contributing.md`. The `cilium` scope moves from `area/networking` to `area/cilium`, following the `kamaji` and `keycloak` precedent where the dedicated area replaces the broad one rather than stacking on it. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Added a dedicated `area/cilium` label for Cilium-related issues and pull requests. * Updated automated labeling and contribution guidance so Cilium changes are categorized separately from general networking work. * Refined the networking label description to exclude Cilium-related items. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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> |
||
|
myasnikovdaniil
|
79287cd9b1 |
docs(release): add the v1.5.4 changelog, and fix v1.6.0's fail-open audit note (#3910)
Adds the changelog for `v1.5.4`, and corrects one still-live instruction in v1.6.0's. The generated changelog is a good inventory and every one of its bullets is kept. What it had no layer for is the operator: 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. That is what is added on top, in the shape v1.6.0's changelog uses. Base widened 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 82/82 commits in range. Three claims corrected. The slot 45 divergence is a skip rather than an ordering problem: v1.5.4 is the first 1.5.x release stamped `targetVersion: 46`, and `run-migrations.sh` loops `seq CURRENT (TARGET - 1)`, so a cluster that reaches 46 runs slots 46 through 53 on the way to 1.6 and never executes 1.6's own slot 45. Its chart-side half was missing too and is not redundant, because a fresh v1.5.4 install is stamped 46 having never run any slot, so only the chart can reach that population. And the S3 checksum bullet named a barman-cloud plugin path that does not exist on `release-1.5`. The audit re-run warning names v1.6.0 rather than an earlier release, because the script does not exist at v1.5.2 at all. v1.5.4 is the first 1.5.x release to carry it, and it carries the fail-closed version. The second commit edits `docs/changelogs/v1.6.0.md`, which is deliberate rather than an accident of scope. Published changelogs are historical records and are normally left alone, but v1.6.0's upgrade notes still 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. Verified per tag: v1.6.0 carries 11 blanket redirections and no `run_kubectl` wrapper, while v1.6.1, v1.6.2 and v1.5.4 carry 4 and 18, the fail-closed shape from #3436 and its backport #3474. |
||
|
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> |
||
|
Aleksei Sviridkin
|
0b8837e552 |
fix(telemetry): make the documented opt-out cover both collectors (#3791)
## What this PR does Telemetry is reported by two components — `cozystack-operator` (cluster facts) and `cozystack-controller` (application counts) — but only the operator had a reachable switch. The documented opt-out sets `cozystackOperator.disableTelemetry` on the `cozy-installer` chart. That chart deploys the operator; it cannot reach a package the platform deploys. `packages/system/cozystack-controller` has its own independent `disableTelemetry` key that nothing propagated to, so after an operator had followed the published procedure the controller kept posting `cozy_application_count` every 15 minutes. Nothing failed, and nothing indicated the opt-out was partial. Two changes: - `packages/core/platform` gains a `telemetry.disabled` key and threads it into the `cozystack-controller` component of the `cozystack.cozystack-engine` Package — the same mechanism the bundle already uses for cilium, multus and linstor values. - `packages/core/installer/values.yaml` declares `cozystackOperator.disableTelemetry`. It was only ever referenced from the template, so the `--set` target the documentation names did not appear in the chart's own values. Both keys default to `false`. Nobody who has not opted out sees a behaviour change. The opt-out remains two steps, because the two collectors are installed by two different charts. Collapsing it into one knob would mean the operator writing the value into the platform Package it does not own, which is a design change rather than a fix. `helm unittest` on the platform chart: 137 passed (133 before, plus the four added here). `helm lint` clean on both charts. ### Screenshots Not a UI change. ### Downstream repositories Trigger map walked against the diff. `packages/core/platform/values.yaml` changed, which the map routes to the website's hand-written platform-package table; the same change also makes the published opt-out procedure wrong, so the docs follow-up covers both. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/651 - [ ] [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(telemetry): the documented telemetry opt-out now covers both collectors. Setting `cozystackOperator.disableTelemetry=true` alone left `cozystack-controller` reporting application counts; the platform gains a `telemetry.disabled` key that silences it. Both default to false. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a platform telemetry opt-out setting, disabled by default. * Added a separate operator telemetry opt-out setting, also disabled by default. * Applying the platform setting now disables telemetry reporting for the relevant controller. * **Bug Fixes** * Ensured telemetry settings are correctly applied across supported platform variants while preserving existing configuration overrides. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
4b50a3ceb4 |
fix(ingress-nginx): pull the protobuf-exporter from the project namespace (#3783)
## What this PR does
Points the `protobuf-exporter` sidecar at the project's registry.
The sidecar was still pulled from `ghcr.io/kvaps/...` while the
controller beside it — built from the same repository, pinned to the
same `v1.11.2` — already came from `ghcr.io/cozystack/...`. The
repository moved to the organisation some time ago and the image
reference did not follow. It is the last thing in the tree that ships a
Cozystack-owned component out of an individual's namespace, which is a
Required item on the Incubation checklist ("All project metadata and
resources are vendor-neutral").
The image is the same bytes: `v1.11.2` was copied into the organisation
namespace with the manifest digest preserved, so `sha256:6d9235a9…` is
unchanged here. Nothing is rebuilt and nothing is upgraded.
The version is deliberately not bumped. `v1.11.5` exists in the
organisation namespace, but the tag tracks the upstream ingress-nginx
version and the controller sits on `v1.11.2`; bumping only the sidecar
would split the pair.
The comment in `hack/promote-retag.sh` loses its `ghcr.io/kvaps/...`
entry, because that path now falls under the bullet below it — a
cozystack-org repository outside `$REGISTRY`
(`ghcr.io/cozystack/cozystack`). Selector behaviour is unchanged;
`hack/promote-retag_test.bats` passes, 11/11.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` against the
diff. The two files touched are a package `values.yaml` image reference
and a comment in `hack/promote-retag.sh` — no trigger matches. In
particular the `ccp` trigger is a move, a rename, or a change to what a
make target does; this is neither, and its anchors (`hack/package.mk`,
`hack/common-envs.mk`) are untouched.
- [x] No downstream repository is affected by this change
### Release note
```release-note
fix(ingress-nginx): the protobuf-exporter sidecar is now pulled from ghcr.io/cozystack instead of a personal namespace. Same image and digest; no version change.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated the protobuf exporter image source to the new container
registry location.
* Refreshed image promotion guidance to remove an outdated third-party
registry example.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
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> |
||
|
myasnikovdaniil
|
f4ab9ec8da |
docs(release): add changelog for v1.6.2 (#3907)
This PR adds the changelog for release `v1.6.2`.
✅ Changelog has been automatically generated in
`docs/changelogs/v1.6.2.md`.
|
||
|
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
|
7194edf625 |
fix(kubernetes): validate MachineHealthCheck condition timeouts at render time (#3901)
## What this PR does Follow-up to #3898, closing two gaps in the render-time duration guard it introduced. Context: #3513. First, the guard now also covers the MachineHealthCheck condition timeouts (`readyUnknownTimeout`, `readyFalseTimeout`) in both the kubernetes and kubernetes-nodes charts. They need it more than the autoscaler annotation does: the CAPI webhook validates `nodeStartupTimeout` with a 30s floor but does not validate `unhealthyConditions` timeouts at all, so a zero or negative value used to apply silently and remediate a Machine the moment its Ready condition flipped, which for a KubeVirt worker means deleting the VM. Second, the guard accepts the canonical form a Go duration serializes back to ("20m0s", "1h0m0s"). `metav1.Duration` prints that shape, so a value copied from `kubectl get -o yaml` output previously failed the render. The first segment must still be positive and later segments may be zero, so every accepted string still parses to a positive duration and the overflow and fraction-to-zero rejections stay. Mechanically, the guard moves into a `kubernetes.positiveDuration` named template in each chart's `_helpers.tpl`, and the error message format is unchanged. Tests cover the canonical form on all three fields and the rejection of unitless, zero, and negative condition timeouts in both charts. The previous test case that passed unitless values through verbatim is reworked into rejections: a unitless value used to fail only at CAPI decode on apply, and a zero or negative one did not fail anywhere. ### 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: ### Release note ```release-note fix(kubernetes): reject zero and negative MachineHealthCheck condition timeouts at render time, and accept the canonical Go duration form ("20m0s", "1h0m0s") for the worker join timer values ``` |
||
|
Aleksei Sviridkin
|
3710541278 |
fix(kubernetes): validate MachineHealthCheck condition timeouts at render time
The CAPI webhook validates nodeStartupTimeout with a 30s floor but does
not validate unhealthyConditions timeouts at all, so a zero or negative
readyUnknownTimeout or readyFalseTimeout applied and remediated a
Machine the moment its Ready condition flipped. Route both fields
through the same render-time duration guard as maxNodeProvisionTime,
extracted into a kubernetes.positiveDuration helper in each chart.
The guard now also accepts the canonical form a duration serializes
back to ("20m0s", "1h0m0s"): metav1.Duration prints that shape, so a
value copied from kubectl output previously failed the render. The
first segment must still be positive and later segments may be zero,
which keeps every accepted string a positive duration.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>
|
||
|
Aleksei Sviridkin
|
b32ca5cbb3 |
chore(kubernetes): drop the force-delete flag that blocks partial cleanup (#3899)
## What this PR does Removes `--force-delete-unregistered-nodes=true` from the tenant cluster-autoscaler Deployment. On the clusterapi provider the flag cannot do what its name promises: the provider's `ForceDeleteNodes` returns `cloudprovider.ErrNotImplemented`, the autoscaler falls back to the ordinary `DeleteNodes` path, and that path enforces the node group's minimum size. The one behavior the flag does change is harmful here: it skips the step that trims a long-unregistered delete batch down to the group's headroom above min size, so a batch larger than the headroom is refused whole where the default would trim it and delete what fits. Context: #3513. A regression test renders the Deployment, proves the args list is live with a positive anchor on `--cloud-provider=clusterapi`, and asserts the flag is absent, so reintroducing it fails the suite. ### 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: ### Release note ```release-note chore(kubernetes): drop the cluster-autoscaler force-delete-unregistered-nodes flag; the clusterapi provider does not implement force deletion, and the flag only made batch removal of long-unregistered machines all-or-nothing ``` |
||
|
Aleksei Sviridkin
|
245bca3366 |
chore(kubernetes): drop the force-delete flag that blocks partial cleanup
`--force-delete-unregistered-nodes` reads as an override that lets the autoscaler remove a long-unregistered machine below its node group's min size. On the clusterapi provider it grants nothing of the sort: force deletion is not implemented there, the call falls back to the ordinary delete path, and that path refuses to take a group below its min size. The flag never bought the safety valve its name suggests. What it does buy is a behaviour nobody asked for. With the flag set the autoscaler skips trimming the delete batch down to the group's headroom above min size, and the ordinary delete path then refuses a batch larger than that headroom outright instead of removing what it can. A group of three machines above a min size of one loses two long-unregistered machines with the flag unset, and none with it set. Removing it restores the documented default and takes away a signal that advertised a protection the provider does not offer. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
5fe9450145 |
fix(kubernetes): widen and expose the worker node join timer budgets (#3898)
## What this PR does Worker node join on nested tenants regularly outruns the timer budgets the charts shipped, and the timers then destroy capacity that was about to arrive: the cluster-autoscaler treats a machine as long-unregistered after its built-in 15m default and orders a replacement, and the MachineHealthCheck deletes a Machine that is still legitimately booting. The timer audit in #3513 (https://github.com/cozystack/cozystack/issues/3513#issuecomment-5329896688) walked every timer on the join path against the cozyreport artifact of a red run; this PR sizes the budgets from those measurements and makes the hardcoded ones configurable. Context: #3513. Three changes, one commit each: - Every worker MachineDeployment in both the kubernetes and kubernetes-nodes charts now carries the `cluster.x-k8s.io/autoscaling-options-maxnodeprovisiontime` annotation, rendered from a new `maxNodeProvisionTime` value defaulting to "30m". The autoscaler starts this clock when the machine enters its node-instances cache with a real providerID, not at Machine creation, and on a value it cannot parse it silently falls back to its 15m default rather than failing, so the template validates the value at render time and rejects anything that does not parse to a positive duration. - The MachineHealthCheck `unhealthyConditions` timeouts become values (`readyUnknownTimeout` default "5m", `readyFalseTimeout` default "10m"; nested under `nodeHealthCheck` in the kubernetes chart, flat in kubernetes-nodes) instead of the hardcoded 30s and 300s. 30 seconds of `Ready=Unknown` deletes a worker whose guest was merely starved of CPU, and 300 seconds of `Ready=False` is less than a CNI rollout takes on a loaded host. - The `nodeStartupTimeout` default rises from "10m" to "20m". The MachineHealthCheck measures it from the later of the Machine's creation and its InfrastructureReady transition, so the clock restarts when the infrastructure reports ready: a worker gets up to 20m for the disk image import and the VM to come up, and then up to 20m again for the guest boot and kubelet registration. The `maxUnhealthy` docs now also spell out the round-down trap in small groups: at "50%" a group of one never remediates. The budgets are coherent with the measured phases: in the audited runs a healthy disk import took 2m22s and import plus VM start 2m53s, while the stuck runs spent over 17 minutes holding a running VM with no Node, so 20m per MachineHealthCheck leg and 30m on the autoscaler clock absorb that shape of slowness without letting a genuinely dead machine linger much longer. New helm-unittest suites cover the annotation rendering, the render-time duration guard (including the values the guard exists for: unitless numbers, unknown units, zero, negatives, parser overflow), and the MachineHealthCheck timeout rendering in both charts. The render-parity test no longer pins the MachineHealthCheck values on either side and instead compares the two charts' defaults, so a future drift between parent and child defaults fails the suite. ### Downstream repositories - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: TODO-LEAD-FILE-ISSUE (suggested title: "Kubernetes/KubernetesNodes: model the new readyUnknownTimeout, readyFalseTimeout and maxNodeProvisionTime fields (upstream cozystack#NNNN)"; the provider models node_health_check for both resources and gains no typed access to the three new fields until it learns them) - [ ] [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(kubernetes): widen the worker node join timer budgets and make them configurable: the cluster-autoscaler maxNodeProvisionTime is now set per MachineDeployment (default 30m), the MachineHealthCheck Ready=Unknown and Ready=False timeouts are values (defaults 5m and 10m), and nodeStartupTimeout defaults to 20m ``` |
||
|
Aleksei Sviridkin
|
ddb15ebc6a |
fix(kubernetes): widen the worker node startup budget to 20m
nodeStartupTimeout is the window a Machine has to produce a Node at all, measured from the later of the Machine's creation and its InfrastructureReady transition. What has to fit inside it is the guest boot and the kubelet registering with the tenant apiserver, and on a host under load neither is quick. At 10m a worker that was still booting was deleted before it ever joined, and its replacement started the same race from the beginning. Also record what a percentage maxUnhealthy does at small group sizes, because the interaction is not what the field name suggests: the value is scaled against the machines the check covers and rounded down, and remediation proceeds only while the unhealthy count is at or below the result. At the default "50%" a group of one never remediates, and a group of two remediates one unhealthy machine but not two. The golden-parity gate pinned the child chart's MachineHealthCheck values to the parent's defaults as inputs, which made a default moved on both sides fail there for the wrong reason. Drop the pins so the comparison is between the two charts' defaults, which is the contract it exists to hold, and pin the defaults themselves in each chart's own unit tests instead. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
33701800d6 |
fix(kubernetes): make the worker MachineHealthCheck timeouts configurable
The two unhealthyConditions timeouts were fixed in the template at 30s for `Ready=Unknown` and 300s for `Ready=False`, and both are too tight for a worker joining on a loaded host. `Ready=Unknown` is what the node lifecycle controller writes when a kubelet stops reporting, on a grace period of its own, so 30s on top of that deleted the VM of a guest that had merely stopped being scheduled for a stretch, before it had a chance to answer again. And `Ready=False` is the condition a registered Node holds until its CNI is up, so 300s was the entire budget a first boot got for the CNI rollout to reach it -- a worker deleted mid-rollout takes the rollout with it and its replacement starts over. Move both to values, defaulting to 5m and 10m. The remediation of a worker that is genuinely dead is correspondingly later, which costs less than it appears to: Pods leave an unreachable Node on the node lifecycle controller's taint-based eviction and the toleration that carries, not on either of these timers. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
83f0eef645 |
fix(kubernetes): give the autoscaler the real worker join time
The cluster-autoscaler treats a machine as long-unregistered once its own max-node-provision-time elapses, and a worker that has not registered by then stops counting towards its node group's upcoming capacity. On a host slow enough that the Talos image import, the guest boot and the CNI rollout do not fit inside that window, the timer fires on machines that were still joining: the group reads as short of capacity, so the autoscaler asks for replacements, and the extra provisioning load makes the join it was waiting for slower still. Render the per-node-group override the autoscaler reads off the MachineDeployment, sourced from a new maxNodeProvisionTime value that defaults to 30m. The duration is checked at render time because the autoscaler discards one it cannot parse and falls back to its own default without failing, and because the values schema types the field as a string and so accepts every malformed duration. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
59cc7d7114 |
test(e2e): anchor the phase-warning guard on its own unique phrase (#3897)
The guard in the CPU throttle suite finds the missing-timeout phase warning by the first match of a phrase that stopped being unique when the bracket-pair captures started writing their own [bounds] notes with the same words. The first match is now a note, the guard reads it as the warning, and the unit gate fails on every run, blocking all PRs. Same fix the node-join copy of this guard already carries: anchor on the phrase unique to the warning itself. All fifteen suites that read the shared library as text pass locally with the change. |
||
|
Aleksei Sviridkin
|
ca058ad964 |
test(e2e): anchor the phase-warning guard on its own unique phrase
The guard located the missing-timeout warning by the first match of "timeout is not on PATH" in the shared library. The bracket-pair captures now write [bounds] notes carrying that same phrase earlier in the file, so the guard read a note as the warning and failed on every run, holding the unit gate red for all pull requests. The node-join copy of this guard was moved to the warning's own phrase for the same reason; this aligns the remaining copy. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
ca9a9300bb |
chore(e2e): ignore the artifacts the sandbox bringup writes to the repo root (#3891)
## What this PR does Adds a .gitignore block for the files `hack/e2e-prepare-cluster.bats` writes to the repository root when the e2e sandbox is brought up outside CI: the Talos secrets bundle, the rendered machine configs, the kubeconfigs, the boot image and the per-VM `srv1..3/` directories. None of these was matched by anything in .gitignore (`git check-ignore` answers "not ignored" for every one), so in a tree that once ran the bringup a broad `git add` stages a credentials bundle -- `secrets.yaml` holds the cluster's private keys. This came one pre-push check away from landing in a PR once, which is what moved it from a hypothetical to a standing trap worth closing. All patterns are anchored to the repository root, so files with the same names elsewhere in the tree stay visible to git. |
||
|
Aleksei Sviridkin
|
6ac97dba75 |
chore(e2e): ignore the artifacts the sandbox bringup writes to the repo root
hack/e2e-prepare-cluster.bats leaves the Talos secrets bundle, the rendered machine configs, the kubeconfigs, the boot image and the per-VM directories in the repository root, and nothing in .gitignore matched any of them. In a tree that once ran the bringup, a broad git add stages a credentials bundle; this was caught in review once and is a standing trap rather than a one-off. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
974c280699 |
test(e2e): price the layers above the sandbox on the node-join window (#3890)
## What this PR does Adds two readings to the tenant node-join path, taken on either side of the same wait the KVM counters already bracket: the runner VM kernel's own `/proc/stat`, and the per-thread CPU time of the QEMU processes the sandbox VMs run as. Every CPU reading on this failure path has a subject inside the sandbox: a cgroup, a tenant worker's threads, a sandbox node's own `/proc/stat`. Each of them can say a guest was given its ticks while getting no work done, and none can say what the layers above spent. That is where this suite's own tax is paid. The sandbox nodes' guest time is charged to the runner kernel as user time, and whatever the runner VM loses to the machine hosting it is charged to nobody the sandbox can see, so a node reporting `steal 0` and a node whose hypervisor was itself waiting look identical from inside. `/proc/stat` is not namespaced, so reading it from the sandbox container reads the runner VM's kernel. That is the only place steal on that layer appears, and it is read in one direction only: a steal that climbs proves this runner VM was preempted, while a zero proves nothing, because the column is filled only where the hypervisor exposes a paravirt steal clock and nothing in the capture observes whether this one does. The sandbox nodes a layer down can make the stronger claim, since this repository starts them with `accel=kvm`; nobody here starts the runner VM, so for this lane the answer is unknown until somebody reads a non-zero. The sandbox VMs are QEMU processes in the same PID namespace as the shell driving the suite, so their threads are a local read too. What that adds is not "which node is busy" on its own, since `cozy_capture_sandbox_node_cpu_time` already reads each node's own `/proc/stat` by name; it is that split *on the join window*, since the existing collector samples twice across a twelve-second interval inside the diagnostics block, after the join has already failed. Beyond the window it adds the host-side accounting of each vCPU thread, what the runner actually granted it, which no in-guest counter can report, and QEMU's non-vCPU IO and worker threads, which no in-guest counter sees either. It also names which node each process is, read from the `srv<N>/qemu.pid` files the bringup already writes: a container pid identifies nothing, and whether the starving guest is the control-plane node or a worker-carrying one is the discriminating question. With no usable pid file the capture stays anonymous and says so, as its KVM sibling does; the pid reads themselves are bounded like every other read here, since a pid file that never yields a byte must not hold the capture. Both are taken at the two points the KVM counters already use, for the same reason: the values are cumulative, so a rate needs a pair, and the interval worth measuring is the window the guest is failing to boot in rather than a few seconds after the deadline has expired. The thread reading runs the probe the tenant worker capture already uses rather than a copy of it, because that probe already answers the two outcomes that matter here and already names the three ways it can come up short, and a second copy would drift from it exactly at those failure paths. Each capture carries the layer it describes in its own legend. Three kernels in this report publish a `/proc/stat` and two captures now read QEMU threads, one layer apart in each direction, so a reader who takes one for the other is comparing a hypervisor against its own guest. The runner-kernel legend also carries the trap specific to that layer: nearly all of its user time IS guest time, because running the sandbox VMs is what that kernel does, so adding the two columns double-counts the whole run. Cost and blast radius: two bounded local reads per sample, no cluster access, nothing added to any diagnostics-phase budget window. The failure-path calls sit behind the same reasoning as the KVM reading, which the phase-budget guard in `hack/run-kubernetes-node-join_test.bats` now prices with a cost arm for each; the guards that enumerate collectors (the missing-`timeout` warning and the documented spend order) carry both new names. Tests are in `hack/run-kubernetes-runner-cpu_test.bats`, 44 cases, green under both runner forms. They cover the readings themselves, every legend claim a reader depends on, the bounded read and its knobs, the sample-number split, each way a capture can come up empty (a file with no `cpu` row, a container with no QEMU, a proc mount that yields nothing, a QEMU named and then gone), the routing of a message from a read that succeeded against one that failed, the per-node attribution and each way it can be incomplete, and placement guards of their own: both pairs must bracket the wait on both paths, nothing may be inserted between the three second readings, and each collector must report a shortfall to the job log in its own words rather than only to the report, since one call site is the passing path where the artifact is the one nobody downloads. Two guards exist because a legend can fail without any number being wrong: one asserts that the parse-from-the-last-parenthesis rule the legend states actually works on the lines the capture holds, and one that every bracketed legend line lands as a single line, since an apostrophe inside one of these single-quoted strings shreds it into one word per line while still passing any short grep. No documentation change: the KVM pair this mirrors is not described in `docs/agents/e2e-testing.md` either, and its documentation lives in the legends the captures carry. Context: #3513 ### Screenshots No UI changes. ### Downstream repositories One line of the bringup changes too: `hack/e2e-prepare-cluster.bats` now starts each sandbox QEMU with `-name guest=srv<N>,debug-threads=on`. QEMU names vCPU threads (`CPU N/KVM`) only when asked to, so without the flag every thread reports the bare process name and the vCPU/IO split this capture documents would not exist to read on this lane; a test pins the flag to the legend that relies on it. Walked the trigger map in `docs/agents/contributing.md` against the diff. Of the `hack/` triggers it names, `hack/e2e-prepare-cluster.bats` (ansible-cozystack, talm) is touched: the touched lines are the CI sandbox QEMU flags, which neither downstream repository restates -- both provision real machines, not the e2e sandbox. `hack/package.mk`, `hack/update-crd.sh` and `hack/upload-assets.sh` are untouched. The rest of the change is confined to the Chainsaw kubernetes suites' shared library and its unit tests, which no downstream repository restates. - [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 test(e2e): a tenant node-join failure now records the runner kernel's own CPU time and the sandbox VMs' per-thread QEMU CPU time on either side of the join wait, so steal on the layer above the sandbox and an uneven split across the three nodes are both visible in cozyreport.tgz ``` |
||
|
Aleksei Sviridkin
|
b1b8cbecc2 |
test(e2e): price the layers above the sandbox on the node-join window
Every CPU reading on this failure path has a subject inside the sandbox: a cgroup, a tenant worker's threads, a sandbox node's own /proc/stat. Each of them can say a guest was given its ticks while getting no work done, and none can say what the layers above spent. That is where this suite's own tax is paid: the sandbox nodes' guest time is charged to the runner kernel as user time, and whatever the runner VM loses to the machine hosting it is charged to nobody the sandbox can see. Two readings close that. /proc/stat is not namespaced, so reading it from the sandbox container reads the runner VM's kernel -- the only place steal on that layer appears, which is what separates a slow suite from a machine sharing a core with somebody else. The sandbox VMs are QEMU processes in this container's own PID namespace, so their threads are a local read too, and they say which node spent the kernel's user time and whether the three spent it evenly. Both are taken at the same two points as the KVM counters and for the same reason: the values are cumulative, so a rate needs a pair, and the interval worth measuring is the window the guest is failing to boot in. The thread reading runs the probe the worker capture already uses rather than a copy of it, since a second copy would drift exactly at the failure paths. Each carries the layer it describes in its own legend, because three kernels in this report publish a /proc/stat and two captures read QEMU threads, one layer apart in each direction -- and a reader who takes one for the other compares a hypervisor against its own guest. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
c5e175a88c |
test(e2e): record the runner VM shape and region in the e2e report (#3889)
## What this PR does Records which machine each e2e run landed on, in the job log and in `cozyreport.tgz` under `sandbox-host/runner-identity.txt`: the OCI shape, both region keys, the availability and fault domain, the numbers of the `shapeConfig` sizing object, and the runner VM's own kernel release, online CPU count and processor model. 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 split by the hardware they ran on. That is the first thing worth knowing about a flake that some runs have and others do not, and today the answer is nowhere in the artifact. `hack/e2e-runner-identity.sh` takes the read on the runner VM, before the sandbox container is created, because the instance metadata service answers on a link-local address the container has no guaranteed route to. An "unreachable" answered from inside the container would look exactly like a capture that had tried. The call sits in `packages/core/testing/Makefile`'s `apply` target rather than in a workflow step: every e2e lane reaches the sandbox through `make prepare-env`, so one call covers all of them, on the passing path and the failing one, with no per-workflow step to keep in step. `hack/cozyreport.sh` folds the file into the report from inside the container, without going near the endpoint itself. What it holds to: - **The root instance document is never requested.** The keys are fetched one at a time. That document carries the instance and compartment OCIDs, the image OCID, the display name, the hostname and the user-supplied metadata bag, and the report is a public artifact, so asking per key makes the privacy property something the wire settles rather than something a filter has to be trusted for. It also matches where the shape sizing is actually published: `shapeConfig` has its own subpath and is not in the root document at all, so a reader hunting it inside that document would have reported the size of every machine as unknown and blamed the service for it. From the sizing object only the numbers are kept, and only under keys shaped and sized like field names, which holds for keys nobody has seen yet since a size is a number, an identifier is a string, and a key is printed into the record exactly as a value is; that object is refused whole unless it arrives as one complete JSON object, so a truncated answer cannot be read in part and published as a sizing record; and a per-key answer that is not one scalar is named as such rather than printed, because a service answering a whole document to a key request is the one way this design could still publish what it never asked for. - **Never fatal, and never slow.** This runs inside `Prepare environment`, the one retried step in the job, so a diagnostic that failed it would cost a whole run. Every read is taken through a wall-clock ceiling and every failure is written into the record instead: a runner that is not an OCI instance leaves an `imds unreachable` line and an exit status of 0, and a wedged `lscpu` is cut off and named rather than holding the step. Where `timeout` is not on PATH the local reads run unbounded rather than not at all, and the record says so, which is the same trade `hack/cozyreport.sh` makes for its own. Exiting zero is not enough on its own, since a read that never returns never reaches that exit. The first key doubles as the probe and is held to answering the way this service does, so a machine that is not an OCI instance pays one ceiling rather than one per key whether the address is silent or something else is answering on it, and either way the run leaves an `imds unreachable` line to grep for. - **One filename either way.** When there is no record, the report writes that same file holding the reason there is none, because an unidentified runner and an unremarkable one otherwise leave the same absence. The record also goes to the job log, and that copy is what covers the one path where the artifact cannot: a run whose sandbox never came up produces no `cozyreport.tgz` at all, so the log is where the record lives. Tests are in `hack/runner-identity.bats`, 52 cases, green under both runner forms. They cover the sizing-object number reader (a renamed key, non-numeric fields counted rather than printed, a nested value, an escaped quote inside a string value, input that is not JSON), the scalar guard (including a whole document answered to a key request), the ceiling knob including the zero that would remove the bound outright, and the running script: which paths are requested and that the root document is not among them, the probe short-circuit, a single key failing without the service being called unreachable, a partial transfer of the sizing object, the bounded local reads and the cut-off arm, a write that would never return, the record reaching both sinks, and both arms of the report side. Verified against mawk and dash as well as the local defaults, since the runner image is neither gawk nor bash. Context: #3513 ### Screenshots No UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. The `hack/` triggers it names are `hack/e2e-prepare-cluster.bats` (ansible-cozystack, talm), `hack/package.mk` and `hack/update-crd.sh` (ccp, external-apps-example) and `hack/upload-assets.sh` (website); none of those is touched. The new script and its suite are new files with no counterpart anywhere, `hack/cozyreport.sh` is not restated downstream, and the `packages/core/testing/Makefile` edit adds a diagnostic to the e2e sandbox bring-up without changing what `make prepare-env` does for a caller. The docs change is a bullet in `docs/agents/`, which no downstream repository mirrors. - [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 test(e2e): every e2e run now records the runner VM it landed on (OCI shape, both region keys, availability and fault domain, shape sizing numbers, kernel release, CPU count and processor model) in the job log and in cozyreport.tgz, so red and green runs can be compared by hardware ``` |
||
|
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> |
||
|
Aleksei Sviridkin
|
a1f3b54de6 |
test(e2e): capture worker block IO and volume state on node-join failure (#3888)
## What this PR does The node-join failure path already records what a stuck tenant worker was allowed and what it got. It records nothing about disk, and that gap now decides an open question: on one-vCPU workers the vCPU thread sits at its physical maximum with the quota untouched, throttling near zero, steal near zero and the sandbox node half idle, while unpacking the same initrd takes six to ten times longer than on a run that passes. Cycles are being handed out and no work comes back. A guest spinning in a wait for blocks on nested virtio looks exactly like a guest computing without result, and every counter on this path is blind to the difference: the CPU families measure the cgroup ceiling and what met it, the byte counters measure what crossed the network, and neither counts a read. Two reads close that. The first is the Pod's block IO, taken from the same kubelet cAdvisor endpoint the CPU counters come from, so it needs no new transport and no access into the guest. It is read once rather than twice, unlike its neighbours in the sampling loop: these counters accumulate from the container starting, a worker container starts with the guest it carries, and KubeVirt gives that Pod restartPolicy Never, so one reading spans exactly the window the deadline covers. Totals are not a rate, though, so the filter also keeps `container_start_time_seconds`: it is the divisor, it costs no extra read, and with it in the same file a reader can turn a total into an average without going to a describe dump for the container's age. What one reading still cannot give is a rate inside the window, and the capture says so where the rows are rather than leaving a reader to assume otherwise. The second is the state of the replicated volumes behind the worker disks. A replica that is SyncTarget, Inconsistent or Diskless serves the guest's system disk from a peer over the network rather than locally, which is slow in a way no counter here attributes to storage. That is a state rather than a rate, so one read settles it, and it has to be read now: the report collects the same tables after the suite, by which time cleanup has usually deleted the volumes a red run would need them for. Both sit with the cheap reads ahead of the collectors that cost minutes, for the reason the phase budget makes structural: admission gates when a collector may start, so what runs last is what gets declined, and a collector below the guest captures is one that disappears on exactly the slow runs it exists for. Neither changes the budget, the read bound, the sampling interval or any cap, so the two inequalities the guards hold the phase against are untouched. ### What this does not do **The guest's own `/proc/diskstats` is out of reach, and stays out of reach.** The diagnostics credential minted for the guest carries the `os:reader` role, which Talos documents as granting the safe methods, "for example, that includes the ability to list files, but does not include the ability to read files content"; only the admin and operator roles read file contents, and `features.rbac: true` is set in the worker machine config. No Talos resource carries disk statistics either, so `read` is the only door. Getting through it means minting a credential that also carries reboot and reset rights on the tenant workers, and that is not a trade a diagnostic makes: the point of the existing `os:reader` credential is precisely that it cannot do those things, and a collector is the worst place to reverse that, because the next one gets written from it. **The one guest source that is readable carries no substitute, and that was measured rather than assumed.** A guest blocked on IO would normally leave a hung-task warning in its own dmesg, which this path already captures. The Talos kernel is built with `CONFIG_DETECT_HUNG_TASK` unset (`kernel/build/config-amd64` in the kernel package tree, on trunk and on the pinned series alike), so that line is never printed however long a task waits, and `blk_update_request` covers IO *errors* rather than slowness. So there is no cheap grep over existing artifacts to run first. The same file settles a related question: the kernel ships `CONFIG_PSI=y` with PSI enabled by default, so when the io-pressure families are missing from the kubelet's output, the kubelet feature gate is the reason and not the kernel. **The per-device host view is parked, with its arithmetic, rather than pursued here.** Sandbox-node `/proc/diskstats` would add the service time and queue depth that cgroup v2 does not publish, and its transport already exists in this file. It costs one more read per node in each of the two passes the existing walk makes: six reads at the read bound, 150s, against a sampling pair that already stands at 662s of the 680s the budget derivation allows for one late admission, over a derivation sitting 10s under the window the op leaves after the bringup and the snapshot. Paying for it means raising the op ceiling, and that is worse than it looks: the E2E job is capped at 180 minutes, a run that hits that cap is cancelled with the report and the job log both lost, and two suites at five more minutes each spend exactly that headroom on the runs where every instrument is already at risk. So the ceiling stays, and this is the step to take if the two reads above do not answer, with the result in hand rather than guessed. The families this capture keeps are the ones cAdvisor declares for the disk IO group, verified against the upstream exposition rather than written from the metric names. Under cgroup v2 the kernel's `io.stat` carries transferred bytes and completed operations only, so the service time and queue depth families are either absent or filled from the node filesystem's own counters. That is stated beside the rows, because a missing service time read as an idle disk is the opposite of what was measured. ### Validity, stated in the artifact Every outcome that is not a reading gets a sentence in the file rather than an empty directory: a kubelet that never answered, a kubelet that answered and carried nothing for a worker on that node, a namespace whose containers are none of them workers, a filter that could not read its own input back, and a read cut short part way through. Empty bytes read as a healthy worker, which is the conclusion this capture was added to test, so no path is allowed to leave them. Keeping the divisor in the filter costs one more of those, and it is paid rather than left implicit. The start-time family is published for any container cAdvisor knows, so a file can hold it with no counter beside it, and then "nothing survived the filter" no longer means "no disk counter arrived". The note therefore decides that on the subject families themselves and says so in the file: a start time with no counter under it reads as the kubelet reporting no block IO for this worker, never as a worker that touched no disk. On a capture already marked incomplete, every sentence that treats the rows as a whole reading is withheld, since a total over a prefix understates it and a family missing from a prefix says nothing about the kernel interface. The device legend stays there, because it explains a label on the rows that did arrive. Context: cozystack/cozystack#3513. ### 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. --> No UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. The diff touches `hack/e2e-chainsaw/_lib/run-kubernetes.sh`, the two kubernetes suites' comment blocks, one new unit suite under `hack/`, and the node-join guard suite. It adds no package, changes no `values.schema.json`, no `values.yaml`, no ApplicationDefinition, no CRD, no namespace, no make target, and no node prerequisite in `hack/e2e-prepare-cluster.bats`. Nothing under `hack/` is moved or renamed. That leaves no trigger matched. - [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 NONE ``` |
||
|
Aleksei Sviridkin
|
0a1d2673d0 |
test(e2e): capture worker block IO and volume state on node-join failure
A worker that burns its whole vCPU while making no progress is either computing without result or waiting on blocks, and every counter this path already collects is blind to the second: the CPU families say what the group was allowed and what it got, the byte counters say what crossed the network, and neither counts a read. Read the Pod's block IO counters from the same kubelet endpoint the CPU ones come from, once rather than twice, because they accumulate from the container starting and a worker container starts with the guest it carries. Read the state of the replicated volumes behind the worker disks as well. A replica that is SyncTarget, Inconsistent or Diskless serves the guest's system disk from a peer over the network rather than locally, and the report collects the same tables only after the suite, by which time cleanup has deleted the volumes a red run would need them for. The guest's own /proc/diskstats would answer this from inside and cannot be reached: the credential minted for the guest carries the os:reader role, which is not allowed to read file contents, and Talos publishes no resource carrying disk statistics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
c3247c0ca0 |
test(e2e): capture the sandbox kernel KVM exit counters across the node-join wait (#3886)
## What this PR does Every CPU reading this suite takes on a failing node-join describes a subject inside the sandbox: a cgroup, a QEMU thread, a sandbox node's own `/proc/stat`. All of them can report a guest that was handed its ticks while the guest gets no work done, and none of them can say what a tick cost. That is the state the node-join failures are in now, with quota untouched, throttling near zero, steal at zero and the sandbox host between a third and two thirds idle, while the guest does several times less work per core than the same image does on a run that passes. The price is paid one layer down. A tenant worker is a guest of a sandbox node, which is itself a guest of the runner, and the runner's kernel absorbs the exits, nested page faults and TLB flushes that the arrangement provokes. That kernel publishes the totals in debugfs, and the suite runs in a container sharing it, so this is a local read rather than a walk over nodes. Being local is what makes it affordable. The budget guard in `hack/run-kubernetes-node-join_test.bats` computes both sides of its own inequality, and on this tree the sampling pair can overshoot by 662s against the 680s the derivation allows, which leaves 18 seconds of room. A per-node reading costs two samples times three nodes times the read bound and its kill grace, or 150 seconds at today's knobs, and no transport changes that: the cost sits in the number of per-node calls rather than in the number of counters. Reading the layer the sandbox shares with the runner costs one bounded read instead, and takes nothing away from the collectors already there. debugfs is mounted in a mount namespace belonging to the call. The suite runs for another twenty minutes after this returns, and a mount left behind would be a diagnostic editing the environment the rest of the run measures. The pair is taken across the node-join wait rather than inside the diagnostics block. A counter here accumulates over the life of the VM it belongs to, so one reading is an average over that life rather than a rate over anything; taking the first before the wait and the second after it makes the interval a difference divides by the window the guest was failing to boot in, instead of a few seconds after the deadline had already expired. The kernel-wide files add a wrinkle the artifact states beside the numbers: the kernel answers them by summing over the VMs alive at the moment of the read, so a VM that went away between the samples takes its whole contribution with it while the survivors keep accumulating. The difference is then understated by a whole VM life and its sign proves nothing when positive; when it does come out negative, that is a finding about the sandbox rather than a rate. The second reading is taken on the passing path too, so a green run leaves a baseline to read the failing ones against, and it is taken immediately after the wait rather than at the end of the run. That placement is the measurement: the happy-path tail is another ten to fifteen minutes of a tenant cluster doing storage and LoadBalancer work, so a reading taken after it would price a different window over a different workload while the capture's legend says it priced the join, and the green-versus-red comparison the collector exists for would be gone. A guard in `hack/run-kubernetes-node-join_test.bats` pins both calls to either side of the wait and fails if either drifts. What the capture cannot answer is written into the artifact rather than left for a reader to assume. The counters price the sandbox VMs' own execution and not what runs inside them. A per-VM directory is named after a pid in the kernel's initial namespace, which the container cannot map to its own processes, so the split says how evenly the sandbox VMs paid and not which node paid what. Several counters sitting in that same directory are gauges or running maxima rather than cumulative, so a difference of those is a number with no meaning. And there is no `ept_violation` counter to look for: the nested-paging faults are counted as `pf_taken` and `pf_fixed`, under those names whichever vendor the runner turns out to be. Every branch that can come up short is named where a reader will look for it: no `unshare` or no `mount` on PATH, a mount the sandbox refused, a kernel whose debugfs holds no `kvm` directory at all (which would be a finding about how the sandbox VMs are being run, not a collector that failed), a `kvm` directory holding nothing readable, a walk the wrapper cut short, and a report directory that could not be created. The pairing instruction rides only on the arm that holds a whole reading, so the artifact never invites a difference against a file that has no counters in it. A capture holding no counter answers its caller non-zero, the two findings included, so the passing path writes the shortfall into the job log instead of leaving it in a report nobody downloads. A walk cut short after it wrote counters does not: it left a usable partial reading, marked as partial, and telling the caller nothing was collected would contradict the file sitting beside it. That distinction is decided by whether a counter reached the file rather than by whether the file has bytes in it, because the walk writes a heading before it reads anything. Covered by `hack/run-kubernetes-kvm-exits_test.bats`, which runs under `make unit-tests`. The guards in `hack/run-kubernetes-node-join_test.bats` that enumerate the collectors were updated rather than worked around: the new capture gets a cost arm in the budget guard, an entry in the missing-timeout warning, and an explicit exemption from the spend-order list, because it is not behind the phase gate. It is not behind that gate deliberately: every other collector the phase declines loses only itself, while declining this one would orphan a reading already taken eighteen minutes earlier. Two guards there are new: one pins where the two readings sit rather than what they contain, the other pins that both call sites consume the collector's status, since this is the only collector in that file whose return value is read at all and it is read so a passing run's shortfall reaches the job log. The collector is also stubbed in that suite, at the shared helper and at each of the five inline stub lists inside its `bash -c` bodies. Left out of them the node-join unit tests call it for real, which on Linux means the unit suite runs `unshare --mount` and `mount -t debugfs` with whatever privileges the host gives it, while on macOS the collector stops at its own `command -v unshare` precondition and the path is never exercised at all: a unit test whose result is set by the machine rather than by the code. Measured with a recording fake on `PATH`, the suite makes 21 real invocations with no stub, 9 with the shared helper alone, and none with every site covered. Stubbing costs the block-level read audit nothing, because that audit works by letting real collector bodies reach the kubectl mock and this collector issues no kubectl read. Context for the measurement is cozystack/cozystack#3513. ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff. The diff adds one collector to `hack/e2e-chainsaw/_lib/run-kubernetes.sh`, adds one BATS suite under `hack/`, and edits guards in an existing BATS suite. Nothing under `hack/` is moved or renamed, no make target changes what it does, and `hack/e2e-prepare-cluster.bats` is not touched, so neither the `ccp` trigger on `hack/` layout nor the `ansible-cozystack` and `talm` triggers on node prerequisites fire. - [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 test(e2e): capture the runner kernel's KVM exit counters on either side of the tenant node-join wait, so a node-join failure can be told apart from a nested-virtualisation slowdown that no counter inside the sandbox can see ``` |
||
|
Aleksei Sviridkin
|
827f4e7e10 |
test(e2e): measure the sandbox kernel KVM exit counters across the node-join wait
Every CPU reading on this failure path describes a subject inside the sandbox -- a cgroup, a QEMU thread, a node's own /proc/stat -- and all of them can report a guest that was handed its ticks while the guest gets no work done. None of them can say what a tick cost. That price is paid one layer down: a tenant worker is a guest of a sandbox node, which is itself a guest of the runner, and the runner's kernel absorbs the exits, nested page faults and TLB flushes the arrangement provokes. Read those counters from the debugfs that kernel publishes them in. The suite runs in a container sharing it, so this is a local read rather than a walk over nodes, which is what makes it affordable at all: the phase budget has no room left for a further per-node read. debugfs is mounted in a mount namespace belonging to the call, so nothing is left behind for the twenty minutes of suite that follow it. The pair is taken across the node-join wait rather than inside the diagnostics block: once before the wait on every run, once after it on the failing path and on the passing one. The counters are running totals -- per-VM since that VM was created, kernel-wide summed over the VMs alive at the read -- so the interval a difference divides by becomes the window the guest was failing to boot in, and a passing run leaves a baseline to read the failing ones against. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
myasnikovdaniil
|
420c316e5c |
refactor(dashboard): validate the schema form before page-level checks (#3116)
## What this PR does Re-applies cozystack/cozystack-ui#26 (by @lexfrei) onto the in-tree console, after the UI was vendored into the monorepo (#2963) and the standalone repo is being archived. Part of the cozystack-ui retirement tracked in #3097. In the four backup create pages (Backup, BackupJob, BackupPlan, BackupRestoreJob), the RJSF `SchemaForm.validate()` call now runs right after the name check and before the page-level required-field alerts. The submit button lives outside RJSF and bypasses its validation, so each page triggers `validate()` explicitly; it previously ran only after the manual alert checks, so a schema-required field left empty exited via an alert without RJSF ever rendering its inline errors. Validating first makes inline errors show consistently; the page-level alerts then cover only page-only requirements (application/backup reference, strategy, timestamp). Scope is the validation-ordering change only — the original PR's VMDisk/StorageClass parts were obsoleted by the `DynamicOptionsWidget` refactor (#30) already present in the vendored snapshot. ## Verification - Includes the original per-page regression tests. - `pnpm typecheck` clean; full console suite green (317 passed). Original PR: cozystack/cozystack-ui#26 — credited via `Co-authored-by`. ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved submission validation across backup creation, backup job, backup plan, and backup restore job pages. * Inline required-field errors now appear before page-level alerts. * Invalid submissions are prevented from sending requests or triggering unnecessary alerts. * **Tests** * Expanded coverage for validation order and submission blocking. * Improved form test scenarios for invalid and missing data. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
4bb141f654 |
docs(postgres): name the trust anchor as the tenant actually sees it (#3463)
## What this PR does Two documentation defects that both landed with the CA trust-anchor work in #3407. The Postgres README told a tenant to fetch `<release>.tenant-ca`. The release name carries the `postgres-` prefix the application definition applies, so the object is `postgres-<name>.tenant-ca`, and the postgres chainsaw test shows exactly that for a resource named `test`. A tenant following the README got `NotFound`. I confirmed the real name on a live cluster while converting another engine onto the same contract. The second is a comment in the controller and one in its RBAC chart, both promising a companion `ValidatingAdmissionPolicy` that pins sentinel writes to helm-controller. No such policy ships. It was proposed in #3410 and rejected: authorizing by `request.userInfo.username` duplicates RBAC in a more brittle form, and the RBAC boundary already holds because no tenant role has any verb on `internal.cozystack.io`. The comments now say that instead of pointing at a guard that does not exist. ### Downstream repositories Documentation only, no API or chart behaviour changes. `cozystack/website` regenerates the Postgres reference page from this README on the next stable tag, so no manual follow-up is needed there. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: ### Release note ```release-note docs(postgres): the tenant CA trust anchor is `postgres-<name>.tenant-ca`, not `<release>.tenant-ca`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated PostgreSQL TLS setup instructions with the correct tenant CA Secret name and retrieval command. * Clarified that tenant-accessible Secrets contain only the CA certificate, while the original Secret contains the private key. * Clarified CA sentinel access controls, including tenant isolation and helm-controller-managed behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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 --> |
||
|
Aleksei Sviridkin
|
9513227fc0 |
chore(ci): re-run with diagnostics enabled
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3d076099a2 |
docs(postgres): name the trust anchor as the tenant actually sees it
The README told a tenant to fetch <release>.tenant-ca, but the release name carries the postgres- prefix the application definition applies, so the object is postgres-<name>.tenant-ca. Following the old text returned NotFound. Also drop two comments promising a companion ValidatingAdmissionPolicy that pins sentinel writes to helm-controller. No such policy ships: the proposal for it was rejected because authorization by username duplicates RBAC in a more brittle form. The RBAC boundary is the whole control. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
eebe5862bf |
test(e2e): run tenant workers on one vCPU with CPU headroom above it (#3863)
## What this PR does Moves the e2e tenant workers from two vCPUs to one, keeping a CPU limit above the vCPU count: `resources.cpu: 1` with `podCpuLimit: 2` and `podCpuRequest: 100m`. This is the next step of the cozystack/cozystack#3513 measurement series. With two vCPUs the failing worker's two vCPU threads burn 1.4 to 1.8 cores between them while the guest kernel never gets past SMP bringup, and raising the CFS ceiling from 2 to 3 cores (#3862) changed neither the burn nor the progress, so the spin is between the two vCPUs rather than against the quota. A single-vCPU guest has no sibling vCPU to spin on, which is exactly what this shape tests. The ceiling of 2 keeps the QEMU emulator and IO threads from eating into the one core the guest computes with, which was the failure shape of the original single-vCPU workers whose quota equalled their vCPU count. The request of 100m is what KubeVirt derived for a one-vCPU worker before, so scheduling does not move. Like its predecessors this changes only what the e2e suites run, not what any chart ships by default. ### Screenshots Not applicable, no UI change. ### Downstream repositories The diff is one values block inside `hack/e2e-chainsaw/_lib/run-kubernetes.sh`. Nothing in the trigger map is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note test(e2e): tenant workers in the kubernetes e2e suites run on one vCPU with a CPU limit of two, testing whether the node-join freeze is a spin between sibling vCPUs ``` |
||
|
Aleksei Sviridkin
|
73b8afc256 |
test(e2e): run tenant workers on one vCPU with CPU headroom above it
Under a two-vCPU worker both vCPU threads burn 1.4 to 1.8 cores between them while the guest kernel never gets past SMP bringup, and raising the CFS ceiling from two cores to three changed neither the burn nor the progress, so the spin is between the two vCPUs, not against the quota. A single-vCPU guest has no sibling to spin on. The ceiling of two keeps the QEMU emulator and IO threads from eating into the one core the guest computes with, and the request keeps scheduling where the workers always were. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
6735277377 |
feat(kubernetes): let a node group set the CPU request of its worker VMs (#3862)
## What this PR does Adds `nodeGroups.<name>.podCpuRequest` to the kubernetes chart, rendering into `domain.resources.requests.cpu` on the worker VM, and the same field to `kubernetes-nodes`. It has no default, so a cluster that does not set it renders exactly what it rendered before. It exists because `podCpuLimit` does not do what #3859 said it does. That PR shipped the limit on the claim that it moves the ceiling alone and leaves the CPU request at the value KubeVirt derives from the guest vCPU count and the cluster allocation ratio. KubeVirt does the opposite: `setDefaultResourceRequests` in `pkg/defaults` copies a declared CPU limit into the CPU request whenever the VM declares no request of its own, and it runs in the VMI mutating webhook, before the template service gets there. The request the template service computes from the vCPU count is then overwritten by the copy. So a node group that set only the limit asked the scheduler for its whole ceiling, which is what took every e2e worker out in #3860: Pending on `Insufficient cpu` before a guest booted. The request is the half that has to hold still, and the chart had no way to declare it. Worth correcting one detail of #3860 while I am here: it attributes the defaulting to Kubernetes copying limits into requests. Kubernetes does that too, but here the copy happens a layer earlier, on the VMI, which is why KubeVirt's own vCPU-derived request never appeared on the container. The first commit fixes that documentation, in both charts and in the generated schema, README and CRD descriptions that carry it. It also promised a floor that does not exist, a limit below the derived request was said to leave the container requesting more than its limit, when the copy makes the two equal whatever the limit is. The field itself is the scheduler reservation, not guest sizing, and it is usable on its own. An explicit request does not stop KubeVirt deriving the ceiling: `doesVMIRequireAutoResourceLimits` returns early only on a declared *limit*, so a lone request changes the weighting a worker gets on a contended host and leaves the ceiling where KubeVirt puts it. That asymmetry is why the docs tell you to keep a lone request at or below the guest vCPU count. Two render-time guards, both in the style of the one #3859 added. The instancetype guard is the same conflict one field over, and it is the same upstream check: `validateCPU` in `pkg/instancetype/apply` reports `domain.resources.requests.cpu` and `domain.resources.limits.cpu` in consecutive branches, so a VM carrying an instancetype and either key is refused by admission, leaving a node group with no Machine and a Kubernetes CR that explains nothing. The second guard refuses a request above its own limit, because that rejection otherwise lands on API server validation of the worker's Pod, two objects away from the CR that named the values. The two quantities are compared as numbers through the chart library's `toFloat`, not as the strings that spell them: `500m` under a ceiling of `1` is the ordinary case for this field, and a lexical comparison refuses it. One thing I deliberately did not guard: a lone request above the ceiling KubeVirt derives is also invalid, but the chart cannot see whether it will be. The derivation only happens in a namespace carrying a ResourceQuota that limits CPU, and the derived value counts IO threads on top of the vCPU count. Failing the render on a value that is perfectly fine in a quota-free namespace would take away a working configuration, so that case is documented rather than enforced. Byte identity for a group that sets neither field is measured rather than asserted: rendering both charts at this branch and at `main` with the knobs unset gives byte-identical `KubevirtMachineTemplate` and `MachineDeployment`, content-hash suffix included. That is load-bearing rather than tidy, a stray key or a moved space renames the template for every node group in every cluster and rolls every live worker VM on upgrade. The existing hash pins in `cluster_test.yaml` cover it too, and stay green. `kubernetes-nodes` gets the field, both guards and its own suite, because that chart is the other half of a byte-identity contract rather than a separate feature surface. Its `tests/render-parity.sh` renders a pool through both charts and compares the four pool objects byte for byte; a field only one side knows would pass that gate and then quietly drop the request when a pool is adopted into a split-out release. The gate gains a case that sets the pair. Two defects in the above turned up while I was checking it, and each is its own commit rather than a quiet amendment. The first is in the chart library: `cozy-lib.resources.toFloat` knew every suffix of the quantity grammar except `u` and `n`, and a suffix it does not recognise is not an error, the unit letter stays on the string, `float64` yields zero silently, and the comparison read zero against zero. A request of `100u` above a limit of `50u` rendered. The library gains the two missing decades and a suite that pins the whole suffix grammar, one value per form, since that failure has no error to notice it by. The second is the value domain, and the third commit settles all of it at once rather than patching whichever edge turned up. Both fields were rendered under `with`, which skips a zero as readily as an absent field, and their guards were keyed the same way; the generated schema behind them admits a leading minus just as readily. So: presence, not truth, is the predicate everywhere either field is read. A negative value is refused, it never reaches a running Pod. A zero limit is allowed and means no ceiling at all rather than the tightest one, because the kubelet turns a zero CPU limit into no CFS quota (`MilliCPUToQuota` returns zero for zero) while the declared key still stops KubeVirt deriving one. A zero request on its own is allowed and reserves nothing. A zero request beside a non-zero limit is refused, and that one is worth stating: KubeVirt's defaulter tests the request with `IsZero`, which an explicit zero satisfies exactly as an absent key does, so it overwrites the zero with the limit and the worker asks the scheduler for its whole ceiling, the outcome `podCpuRequest` exists to prevent, reached silently and reported nowhere. Refusing beats documenting when nothing downstream will ever mention it. A zero limit is the one case that needed a second look, because two reviewers independently read it as "no ceiling, so a positive request beside it is fine". It is not: API server validation refuses a request above a limit that *exists*, and zero is a limit that exists, while KubeVirt keeps deriving a request from the vCPU count whenever the VM declares none, the defaulter that would have copied the limit across does not fire, precisely because the limit is zero. So a lone `podCpuLimit: 0` renders a Pod that is rejected on creation, and it is refused here instead. Paired with `podCpuRequest: 0` it is coherent and stays allowed: no ceiling, no reservation. This settles `podCpuLimit` along with its new pair, deliberately: the two are documented as a pair and cannot sanely disagree about what zero means. Worth stating plainly that `podCpuLimit: 0` changes meaning as a result, at `main` it renders nothing, because `with` drops a zero as readily as an absent field, and here it either renders or fails the render depending on what sits beside it. `podCpuLimit` landed on `main` earlier the same day and has never been in a release, so no shipped configuration is affected, but anyone who set it on `main` between then and now should read this paragraph rather than discover it. Tests are helm-unittest in both packages, mirrored case for case, with one per class of the domain above: the request lands, a fractional quantity survives, the pair renders both keys with the ephemeral-storage pair untouched on both sides, a request alone renders no limit beside it, nothing renders when the field is absent, every guard fails on the message that names its reason, an equal request is accepted, a milli request under an integer limit is accepted, a lone zero request renders, a zero request beside a limit is refused, a zero pair is accepted, and a negative value of either field is refused. I ran a mutation ladder over the result: dropping the request render in either chart, flipping the comparison operator, comparing the quantities as strings, dropping either instancetype guard, dropping the field from the child's group dict, reverting the two suffixes, reverting either predicate to truthiness, dropping either negative guard, and letting the zero-request guard stop excluding a zero limit. Every one is caught, each by the case that exists for it, the string comparison by `500m` under a ceiling of `1`, the suffixes by the micro-range pair, the predicate and the domain rules by their own zero and negative cases. The e2e suite gets the headroom experiment back, `podCpuLimit: 3` with `podCpuRequest: 200m`. The request is what KubeVirt derived for these workers before either field existed, so scheduling is unchanged against every historical run and the ceiling is the only variable that moves. It is still an experiment against a measured mechanism, not a claimed fix for #3513: a per-thread capture on a red run showed both vCPU threads of a stuck worker burning flat out at a ceiling equal to their vCPU count while the guest kernel made no progress, which is what a vCPU spinning on a lock looks like when the vCPU holding it is the one preempted out of the quota they share. If the join failure rate does not move, the lever is elsewhere. ### Screenshots Not a UI change. ### Downstream repositories Walked against the diff. One repository is reached, and it is the one #3859 already reached: `terraform-provider-cozystack` models every app field by hand, so `podCpuRequest` in `packages/apps/kubernetes/values.schema.json` and `packages/apps/kubernetes-nodes/values.schema.json` needs the schema, the model and the expand/flatten pair over there. That work is one edit, not two, both fields land in the same model and the same docs, so the pair is added to the open one, cozystack/terraform-provider-cozystack#32, with a comment that also corrects the description of what `podCpuLimit` does, which that issue took from the claim this PR retracts. The website is not reached: its trigger is adding, renaming or removing a package, and the reference page for an existing package is regenerated from its `README.md` by the release bot. The change to `hack/e2e-chainsaw/_lib/run-kubernetes.sh` is a value inside the suite, not a move, a rename or a make target, so `ccp` is untouched. The remaining repositories restate nothing this diff touches. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/32 - [ ] [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(kubernetes): a node group can now set `podCpuRequest`, the CPU request of its worker VMs, next to `podCpuLimit`. Without it KubeVirt copies a declared CPU limit into the request, so raising a worker's CPU ceiling also raised what it asked the scheduler for and could leave the group unschedulable. Unset by default, so existing clusters render and behave exactly as before. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `podCpuRequest` configuration for independently controlling Kubernetes scheduler CPU reservations. * CPU requests and limits now support explicit zero and fractional quantities. * Added validation for negative values, incompatible instance-type settings, invalid zero combinations, and requests exceeding limits. * Improved resource quantity conversion, including micro- and nano-unit support. * **Documentation** * Updated configuration references and schemas with CPU request, limit, validation, and rollout behavior. * **Tests** * Added comprehensive coverage for CPU resource rendering, validation, quantity conversion, and parity. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3cb131785c |
fix(kubernetes): settle what a pod CPU quantity of zero or less means
Both podCpuLimit and podCpuRequest were rendered under `with`, which skips a zero as readily as an absent field, and their guards were keyed the same way. The generated schema behind them admits a leading minus and a zero as readily as a sane value, so rather than patch whichever edge turned up, the whole value domain is settled in one place. Presence, not truth, is the predicate everywhere either field is read. A negative value is refused: it never reaches a running Pod. A zero request on its own is allowed and reserves nothing. A zero request beside a non-zero limit is refused: KubeVirt's defaulter tests the request with IsZero, which an explicit zero satisfies exactly as an absent key does, so it overwrites the zero with the limit and the worker asks the scheduler for its whole ceiling. That is the outcome podCpuRequest exists to prevent, reached silently and reported nowhere, which is why it is refused rather than written down. A zero limit removes the ceiling rather than being the tightest one -- the kubelet turns a zero CPU limit into no CFS quota -- but it reaches a Pod only paired with a zero request. KubeVirt keeps deriving a request from the vCPU count when the VM declares none, the defaulter that would have copied the limit across does not fire because the limit is itself zero, and API server validation refuses a request above a limit that exists. Zero is a limit that exists. So a lone zero limit is refused too. Both fields document the same rules and each class has a test on both charts. A group that sets neither field renders the bytes it rendered before, so the content-hash name of the worker VM template does not move. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
a6770d0f1b |
fix(cozy-lib): convert micro and nano quantities instead of zero
cozy-lib.resources.toFloat knew every suffix of the Kubernetes quantity grammar except `u` and `n`. A suffix it does not recognise is not an error: the unit letter stays on the string, `float64` cannot parse the result and yields zero silently, and the caller divides or compares that zero as if it were the value. The node group CPU request guard is one such caller. A request of 100u above a limit of 50u compared zero against zero and rendered, so the pair reached API server validation on the worker's Pod instead of failing where the values were written. Pinned by a suite over the whole suffix grammar, one value per form, because the failure has no error to notice it by. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
b8964c8233 |
test(e2e): give tenant workers CPU headroom with a request to match
Restores the ceiling the kubernetes suites briefly carried, now that a node group can declare the CPU request that goes with it. The limit on its own was defaulted into the request by KubeVirt, so every worker asked the scheduler for the whole ceiling and sat Pending on Insufficient cpu before a guest existed. The request is set to what KubeVirt derived for these workers before, which leaves scheduling where it was and leaves the ceiling as the only variable the run changes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
1b8340298e |
feat(kubernetes): let a node group set the CPU request of its worker VMs
Adds nodeGroups.<name>.podCpuRequest, rendering into domain.resources.requests.cpu on the worker VM, and the same field on the kubernetes-nodes chart, which is the other half of a byte-identity contract rather than a separate feature surface. podCpuLimit on its own cannot raise the ceiling of a worker without raising what that worker asks the scheduler for, because KubeVirt copies a declared CPU limit into the request when the VM declares none. The request is the half that has to hold still, so the pair can now be moved apart. Both fields conflict with instancetype sizing for the same reason and in the same upstream check, so the request gets the guard the limit already had. A request above its own limit fails the render too, rather than reaching API server validation on a Pod two objects away from the CR that named the values; the quantities are compared as numbers, so 500m under a ceiling of 1 is accepted where a lexical guard would refuse it. A node group that sets neither field renders the bytes it rendered before, which the content-hash name of the KubevirtMachineTemplate depends on, and the render-parity gate gains a case that compares the pair across the two charts. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
669c3e60ab |
docs(kubernetes): correct what podCpuLimit does to the CPU request
The field was documented as moving the ceiling alone and leaving the CPU request at the value KubeVirt derives from the guest vCPU count and the cluster CPU allocation ratio. KubeVirt does the opposite: its VMI defaulter copies a declared CPU limit into the CPU request whenever the VM declares no request of its own, so a node group that sets only the limit asks the scheduler for its whole ceiling, and a ceiling the nodes cannot spare leaves the workers Pending on Insufficient cpu. The same paragraph promised a floor that does not exist either. A limit below the derived request was said to leave the container with a request above its limit; the copy makes the two equal instead, whatever the limit is. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
7103fbe1e4 |
fix(e2e): drop the worker CPU limit until it can carry its own request (#3860)
## What this PR does Removes the `podCpuLimit: 3` the e2e kubernetes suites set on their tenant workers, because as shipped it makes the workers unschedulable: a container with a CPU limit and no CPU request is given a request equal to the limit, so each worker asked the scheduler for its whole three-CPU ceiling, and on the first runs after it landed every kubernetes suite worker sat Pending with `0/3 nodes are available: 3 Insufficient cpu` before any guest booted. The failure diagnostics from those runs carry it directly: the virt-launcher Pods are `phase=Pending` with no start times, the console capture returns empty with the Pod state beside it, and the thread capture declines with a marker naming the absent compute container. The limit itself was an experiment against the measured mechanism of cozystack/cozystack#3513 (both vCPU threads of a stuck worker burn flat out at a ceiling equal to the vCPU count while the guest kernel makes no progress). The experiment was never run: the request side spoiled it before the first guest started. It returns once the chart can pair the limit with an explicit request, which is a follow-up to #3859. ### Screenshots Not applicable, no UI change. ### Downstream repositories The diff is one values block inside `hack/e2e-chainsaw/_lib/run-kubernetes.sh`. Nothing in the trigger map is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(e2e): tenant workers in the kubernetes e2e suites no longer set a CPU limit, since without a paired request it defaulted the scheduler request to the full ceiling and left the workers unschedulable ``` |
||
|
Aleksei Sviridkin
|
80e2753bf4 |
fix(e2e): drop the worker CPU limit until it can carry its own request
A container that sets a CPU limit and no request is given a request equal to the limit, so each tenant worker asked the scheduler for its whole three-CPU ceiling and every kubernetes suite worker sat Pending on Insufficient cpu across all three sandbox nodes, before any guest booted. The headroom experiment comes back when the chart can pair the limit with an explicit request. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |