mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
77cb19c8db6f35e402fa5778422f56bcedf2a190
5613
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Andrei Kvapil
|
77cb19c8db |
fix(redis): label RedisFailover with the operator group
The freshworks-oss operator filters RedisFailovers by the redis-failover.freshworks.com/operator-group label matching its OPERATOR_GROUP_ID; without the label it ignores the CR and no Redis is provisioned. Stamp the group (cozystack) so the migrated operator adopts the app's clusters. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
0b57261d38 |
fix(redis-operator): set required OPERATOR_GROUP_ID for the freshworks fork
The freshworks-oss fork exits fatal at startup when operator_group_id is unset, unlike the archived spotahome operator. Pin a stable non-empty value via extraEnvVars so the operator boots instead of CrashLooping and blocking the install. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
abb38ab72f |
chore(redis-operator): pin built freshworks v3.3.5 image digest
Build the freshworks-oss v3.3.5 operator image (with our label-preservation patch) and pin its digest in values.yaml, replacing the placeholder tag. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
fe01ee17bc |
feat(redis-operator): migrate from archived spotahome to freshworks-oss fork
spotahome/redis-operator was archived 2026-06-11 (read-only). Switch the operator to the actively-maintained freshworks-oss/redis-operator fork, which keeps the same RedisFailover CRD and API group (databases.spotahome.com) and adds a native Valkey engine (spec.engine: Redis|Valkey) — so managed Redis is unaffected and managed Valkey no longer needs a Redis-compat shim image. - re-vendor the chart from oci://ghcr.io/freshworks-oss/charts/redis-operator - build the operator image from freshworks-oss v3.3.5 source - re-target the label/annotation-preservation patch onto the freshworks module The operator image is built out-of-band (make -C packages/system/redis-operator image) and its digest pinned in values.yaml; a maintainer must rebuild and pin before merge (tag is a placeholder here). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
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> |
||
|
Aleksei Sviridkin
|
fa2cedc50f |
feat(kubernetes): let a node group set the CPU limit of its worker VMs (#3859)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Adds `nodeGroups.<name>.podCpuLimit` to the kubernetes chart. It renders into `domain.resources.limits.cpu` on the worker VM, which is the CPU ceiling of the virt-launcher `compute` container. The field has no default, so a cluster that does not set it renders exactly what it rendered before. The knob exists because there is currently no way to give a worker VM a ceiling different from its vCPU count. `nodeGroups.<name>.resources.cpu` sizes the guest, not the pod. KubeVirt fills the pod side in: when the namespace carries a ResourceQuota that limits CPU, it sets `limits.cpu` to the guest vCPU count, and a tenant namespace with `resourceQuotas` set always does. So a worker sized at N vCPUs runs under a ceiling of N, and the QEMU emulator thread and the IO threads come out of the same N as the vCPU threads. Declaring the limit on the VM is the only clean way out: KubeVirt stops deriving one as soon as the VMI carries it (`doesVMIRequireAutoResourceLimits` returns early), and the value declared on the VM lands on the container. Removing `cpu` from the tenant's `resourceQuotas` instead would be worse, because the tenant LimitRange then applies its 250m default. The field moves the ceiling and nothing else. It renders no CPU request, so the request stays what KubeVirt computes from the vCPU count and the cluster CPU allocation ratio, and the scheduler weight of a worker does not change with it. That is deliberate: a knob that moved both would make its own effect unreadable. It cannot be combined with `instanceType` sizing, and the chart says so at render time rather than letting the failure land later. KubeVirt's instancetype applier reports `domain.resources.limits.cpu` as a conflict instead of resolving it, so a VM that carries both an instancetype and the limit is refused by the admission webhook, which leaves a node group with no Machine and a Kubernetes CR that explains nothing. Sizing a group by `resources` is exactly what drops the instancetype from the VM, so that is the combination the limit is usable in, and a group still sized by its `instanceType` now fails the render with a message naming the group. This sits beside the chart's existing guard for the neighbouring `resources.cpu` without `resources.memory` mistake, which is the same conflict class one field over. Rendering is byte-identical for a node group that sets nothing, down to the whitespace, and that is load-bearing rather than tidy. The `KubevirtMachineTemplate` around this block is named by a hash of its content, so a stray key or a moved space would rename the template for every node group in every cluster and roll every live worker VM on upgrade. The existing hash pins in `cluster_test.yaml` cover it: they stay green, and they go red under a mutation that renders the limit unconditionally. The e2e suite sets the limit to 3 for its two-vCPU workers. This is an experiment aimed at a measured mechanism, not a claimed fix for cozystack/cozystack#3513. What a per-thread capture on a red run showed: the two vCPU threads of a stuck worker burn 1.6 to 1.8 cores between them under a ceiling of exactly 2.0 while the guest console records no progress at all, and the emulator, IO, vhost and vnc threads together stay under half a percent of the sampling interval. That is the shape of a vCPU spinning on a lock whose holder is the other vCPU, preempted out of the quota the two share, and it predicts that a ceiling with room above the vCPU count lets the preempted one back in. If the node-join failure rate does not move, the prediction was wrong and the mechanism needs a different lever; the chart field is useful either way, since a ceiling equal to the vCPU count is not a sensible default for a nested guest that pays emulation overhead out of it. Tests are helm-unittest in the same package: the limit lands in `domain.resources.limits.cpu`, a fractional quantity survives, no CPU request appears beside it, the ephemeral-storage pair the block already carried is untouched on both sides, nothing renders when the field is absent, and a group that keeps its instancetype fails the render on the message that names the reason. The `kubernetes-nodes` chart gets the same field, its guard and its own tests, 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, on the grounds that the `KubevirtMachineTemplate` is content-hash named. A field only one side knows would not fail that gate, it would pass it and then quietly drop the ceiling when a pool is adopted into a split-out release. The gate gains a case that sets the limit and compares the two renders, and that case goes red if either side stops rendering it. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> Not a UI change. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> Walked against the diff. One repository is reached: `terraform-provider-cozystack` models every app field by hand, and a new field in `packages/apps/kubernetes/values.schema.json` needs the schema, the model and the expand/flatten pair over there, the same way `logSerialConsole` did in cozystack/terraform-provider-cozystack#26 and `kernelModules` did in cozystack/terraform-provider-cozystack#20. The follow-up is filed as cozystack/terraform-provider-cozystack#32. 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 remaining repositories restate nothing this diff touches. 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. - [ ] 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 <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(kubernetes): a node group can now set `podCpuLimit`, the CPU ceiling of its worker VMs, independently of the vCPU count that sizes the guest. Without it KubeVirt keeps deriving that ceiling from the vCPU count, which leaves the QEMU emulator and IO threads sharing the guest's own allowance. 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 optional `podCpuLimit` configuration for setting CPU limits on worker VM containers. * Supports integer and fractional Kubernetes CPU quantities. * Preserves CPU requests and ephemeral-storage settings while applying the configured limit. * Worker changes trigger VM template rollouts. * **Bug Fixes** * Added validation for incompatible combinations with instance-type sizing and insufficient resource configuration. * **Documentation** * Updated configuration references and schemas with usage, sizing, scheduling, and rollout guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
78b9bc3e6e |
feat(kubernetes-nodes): mirror the worker pod CPU limit into the pool chart
The split-out pool chart renders the same worker VM as the monolithic one, and tests/render-parity.sh exists to prove it renders it byte-identically, because the KubevirtMachineTemplate is content-hash named and any divergence rolls every live worker VM. A field that only one of the two charts knows breaks that quietly rather than loudly: a pool adopted into a kubernetes-nodes release would render without the CPU limit it was given, the hash would change, and the ceiling would fall back to the vCPU count with nothing reporting it. So the field, its render-time guard against combining it with an instancetype, and its tests arrive here in the shape the parent chart has them, and the parity gate gains a case that compares the two renders with the limit set. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
c2c50b4119 |
test(e2e): give tenant workers CPU headroom above their vCPU count
Workers that fail to join have been measured burning their vCPU threads flat out under a ceiling equal to their vCPU count while the guest kernel recorded no progress at all, with the emulator, IO, vhost and vnc threads flat at zero. That is the shape of a vCPU spinning on a lock whose holder is the other vCPU, preempted out of the quota the two share, and it predicts that a ceiling with room above the vCPU count lets the preempted one back in. What the next runs answer is whether the join failure rate moves. If it does not, the lever is elsewhere and the value goes back. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
a666b9b3bd |
feat(kubernetes): let a node group set the CPU limit of its worker VMs
The chart had no way to give a worker VM a CPU ceiling different from its vCPU count. `resources.cpu` sizes the guest, and the pod side is filled in by KubeVirt: with a ResourceQuota limiting CPU in the namespace, which every tenant carrying resourceQuotas has, it derives limits.cpu from the guest vCPU count. The QEMU emulator thread and the IO threads then come out of the same allowance as the vCPU threads, which is tight for a nested guest that pays emulation overhead. Declaring the limit on the VM is what stops KubeVirt deriving one, and it is the only way out that does not make things worse: dropping cpu from the tenant resourceQuotas would let the LimitRange apply its 250m default instead, four times tighter than what it replaced. The limit is usable only on a group sized by resources, because KubeVirt's instancetype applier reports domain.resources.limits.cpu as a conflict rather than resolving it, and a VM carrying both is refused by the admission webhook with nothing on the CR to say why. Sizing by resources is what drops the instancetype from the VM, so the chart fails the render for a group that kept its instancetype, beside the existing guard for the same conflict class one field over. The field renders no CPU request, so the request stays derived from the vCPU count and the cluster CPU allocation ratio, and the scheduler weight of a worker does not move with its ceiling. It renders nothing at all when unset, byte for byte: the enclosing KubevirtMachineTemplate is named by a hash of its content, so anything else there would roll every live worker VM on upgrade. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
9e3f66f2b2 |
test(e2e): split the tenant worker CPU time across QEMU threads (#3843)
## What this PR does The node-join diagnostics already read the worker's CPU counters twice on failure, but they read the compute container's cgroup, and that cgroup holds every thread QEMU runs: the guest vCPUs, the emulator thread, the IO threads. A container spending most of its quota while the guest makes no progress has two incompatible readings and no container-level counter picks one. This adds the per-thread split. It execs into each Running worker's compute container and copies the QEMU process's `/proc/<pid>/task/<tid>/stat` lines into the artifact verbatim, with a legend. Nothing is computed in the collector. The legend is there because the field positions are a trap that gives a wrong answer instead of an error: a vCPU thread is named `CPU 0/KVM`, that space sits inside the comm parentheses, and awk over whitespace then reads the wrong column for exactly the threads the split is about. It runs in the sampling loop the other two subjects already share, so there is no second wait. Rates come from the two files and the stamps in them. Every way the read can fall short gets its own sentence in the artifact instead of an empty directory, and neither the legend nor the pairing instruction is attached to a read that did not finish. Coverage is `hack/run-kubernetes-thread-cpu_test.bats`, under `make unit-tests`. It drives those outcomes rather than grepping the source for them, and runs the probe against staged `/proc` trees. Context: cozystack/cozystack#3513. This is an instrument, not a fix, and it does not close that issue. ### Screenshots Not a UI change. ### Downstream repositories Walked against the diff. It touches `hack/e2e-chainsaw/_lib/run-kubernetes.sh`, the two kubernetes chainsaw suites, three `hack/*.bats` files and one paragraph in `docs/agents/e2e-testing.md`. Nothing under `packages/` moves, no ApplicationDefinition, schema, enum or default changes, no namespace or platform variant is renamed, no metric changes, and no label or annotation another component matches as a literal is touched. The two repositories that vendor out of `hack/` take `hack/package.mk` and `hack/update-crd.sh`, and neither is in this diff. - [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): on tenant node-join failure, capture how the compute container's CPU time splits across QEMU's vCPU, emulator and IO threads ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added per-thread CPU diagnostics for worker compute processes. - Reports sampling timestamps, outcomes, interpretation notes, and relevant missing or incomplete data. - Improved Pod and node discovery for diagnostic collection. - **Documentation** - Clarified diagnostic sampling intervals, timing, and collection budgets. - **Tests** - Added comprehensive coverage for per-thread CPU diagnostics, bounded sampling, failure handling, fallbacks, and budget enforcement. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
27ccb50376 |
Revert "docs(agents): defer the image-refs routing edit to its own PR"
This reverts commit
|
||
|
Aleksei Sviridkin
|
cf2d9c6d13 |
test(e2e): split the tenant worker CPU time across QEMU threads
The node-join diagnostics phase samples the tenant worker's CPU counters twice on failure, but it reads them from the compute container's cgroup, and that cgroup holds every thread QEMU runs: the guest's vCPUs, QEMU's main thread, and its IO and worker threads. A container burning most of its quota while the guest reports no progress is two incompatible findings wearing one number, and no container-level counter separates them. Add a collector that execs into each Running worker's compute container, finds the QEMU process and copies every /proc/<pid>/task/<tid>/stat line into the artifact verbatim, with a legend beside them. Nothing is computed here. The legend names where utime and stime sit and why counting fields over whitespace reads the wrong column for exactly the threads the split is about: a KVM vCPU thread is named "CPU 0/KVM", that space is inside the comm parentheses, and every field after it shifts by one. It runs inside the sampling loop the two existing subjects already share, so it costs no second wait: both readings ride the interval already being paid, and the rate comes from the two files plus the stamps each one carries. Every way the read can come up short is a named outcome rather than an empty directory, and the legend and the pairing instruction are withheld from each of them: telling a reader to subtract two files asserts that both hold a whole reading. The probe shells out to nothing but getconf, whose absence it names, because a probe that needed cat would report a container missing coreutils as a container whose QEMU had exited, and it counts what it managed to read so that having read no process at all stays separable from having read them all without finding QEMU. Its stderr is suppressed on the walk rather than on either read: redirections are applied left to right, so a 2>/dev/null after an input redirect is not in place yet when opening the input fails, and a thread that exits between the glob and the read would otherwise put a shell complaint in the artifact beside a healthy capture. It also reports a QEMU that was named and then gone before its task files could be opened, because a process heading alone is enough to make the capture non-empty and would otherwise carry a legend describing columns the file does not have. The Pod listing the cAdvisor captures already share now takes its whole query as a parameter, so this collector reuses that read rather than carrying a third copy of it, and asks for the Running Pods only: a launcher left over from an earlier attempt costs the sibling caller nothing and would cost this one a bounded exec and a slot under its cap. The guard that holds the sampling group against the room the tenant snapshot needs now counts every capped walk in the loop rather than the two spelled the same way, so the new subject is inside the arithmetic rather than beside it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
66cd0fbe89 |
test(e2e): time the guest console capture and baseline it on a green run (#3841)
## What this PR does A tenant worker's guest serial console is stamped in seconds since that guest's own kernel started. So a log that ends mid-boot tells you how far the guest got and nothing at all about when that was. Whether it ends there because the guest started late or because the guest stopped printing is the difference between two unrelated failures, and neither instant was written down anywhere: the read's own instant was nowhere in the file, surviving only as the artifact's mtime, which is metadata rather than content and so says nothing to anyone who opens the log, and the guest's start only inside a `describe` dump that the capture triggers exclusively when some console came back silent. On a run where every console returned bytes, that dump does not exist. I went looking for the answer in three collected reports before changing anything. Every one of the twelve captures across them exited zero, so no read was truncated by its 30s bound or by `--limit-bytes`, and the logs are 23 to 79 KB against a 1 MiB cap. The captures stop at 305, 370, 432, 456, 474, 497, 564, 570, 583, 599 and 639 guest seconds. A fixed cutting mechanism does not produce a 334 second spread. The logs end where the guests stopped printing. On the single worker where both ends were recoverable at all, by reading the file's mtime and the describe together, the guest had been alive for 955 seconds and had printed up to second 564. Each console log now carries the instant its own read started and returned, and `CAPTURE-TIMELINE.txt` beside it carries each Pod's phase, its start time and the start of its compute and console containers, from one bounded read taken once for the whole selector rather than per Pod. Those two subtract to how long a guest had been alive when it was read, which is what the log's last kernel stamp gets compared against. Close together means the guest was still printing and the log ends because it had not got further. Far apart means it stopped. A Pod with no compute start never ran a guest at all, whatever its console file says. The legend in the file names the units on both ends, because an epoch integer and an RFC 3339 string subtract to nonsense if you take them as printed. A read that failed and a read that answered with nothing are written down as themselves instead of leaving an absent file. The same capture now also runs on the passing path, after the last assertion and before the tenant is deleted, because the container holding the stream goes with the Pod and no read recovers it afterwards. Every console this tree has collected came from a failure, so nobody has ever measured what a healthy boot looks like, while "slower than usual" is the claim the node-join failure keeps resting on. That caller asks for the pool's minimum rather than its maximum, since two workers booting is the same measurement as ten, and it warns in the job log instead of failing a suite that has already proved everything it exists to prove. Both paths write the same directory, so the capture records which of them produced it. The cost moves by one bounded read. The serial console family goes from roughly 5m50s to 6m25s at worst and from 3m30s to 4m05s at the pool minimum, both against the 7m diagnostics phase budget, and the figure in both suite comments moves with it under the guard that holds the two in step. The passing path adds six bounded reads at 30s and 5s kill grace, so 210s. That 210s is real wall clock at the tail of an operation with a fixed 50m ceiling, which is a way for a run that already passed to be killed while collecting a debugging aid. So the passing-path capture declines itself out loud when the ceiling no longer has room for it, against a reserve derived from its own cost plus the deletes after it. The ceiling it measures against is read from both sides by a guard, so a suite that changes its `timeout:` fails there rather than leaving the reserve protecting a number that moved. What this deliberately does not do is widen a capture window. I could not derive a number for one that would be honest. The reads are already complete, so there is no read side bound to relax, and the only remaining lever is waiting longer before reading. How much longer depends on whether the tail silence is the guest freezing or the stream stopping, and that is precisely what was not measurable. The evidence leans toward the guest: the logs end on complete lines, and one of them ends on an RCU stall backtrace, which is the kernel reporting it was not being scheduled. If that holds, the silence has no ceiling and no fixed window can promise to contain a milestone. The green baseline this adds is what would set the number, and it does not exist yet. Context for cozystack/cozystack#3513, which this does not fix. It restores the instrument that investigation needs. ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the actual diff. It touches `docs/agents/e2e-testing.md`, the shared e2e helper `hack/e2e-chainsaw/_lib/run-kubernetes.sh`, the two `kubernetes-*` suite files and three unit test files under `hack/`. Nothing under `packages/`, no `ApplicationDefinition`, no values schema or enum or default, no namespace, no emitted metric, no label or annotation another component reads, no node prerequisite in `hack/e2e-prepare-cluster.bats`, no make target and no file moved or renamed under `hack/`. No trigger in the map matches. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note test(e2e): stamp each tenant worker guest console capture with the instant it was read and the instant its guest started, and take the same capture on passing runs so a failing boot has a healthy one to be compared against ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Diagnostics** * Improved Kubernetes serial-console diagnostics with capture context, bounded collection, per-read timestamps, and cluster-wide Pod start-time timelines. * Added clearer reporting for incomplete, unavailable, or failed diagnostic data. * Added absolute timestamps and explicit time units to support reliable troubleshooting. * **Reliability** * Passing test runs now collect baseline diagnostics only when sufficient time remains, helping prevent suite timeouts. * **Tests & Documentation** * Expanded diagnostic coverage and updated documented timing expectations for supported Kubernetes versions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
959e90c9ab |
test(e2e): time the guest console capture and baseline it on a green run
A tenant worker's guest console is stamped in seconds since that guest's own kernel started, so a log ending mid-boot says how far the guest got and nothing about when. Whether it ends there because the guest started late or because the guest stopped printing decides which failure is being looked at, and neither instant was recorded: the read's own instant was nowhere in the file, surviving only as the artifact's mtime, which is metadata rather than content and so says nothing to anyone reading the log; and the guest's start only inside a describe dump that a capture with no silent console never triggers. Across three collected runs every capture exited zero and every log began at the first kernel line, so the reads were complete at both ends and the logs end where the guests stopped -- and which of the two happened could not be read off any of them. Each console log now carries the instant its own read started and returned, and CAPTURE-TIMELINE.txt beside it carries each Pod's phase, its start time and the start of its compute and console containers, from one bounded read taken once for the whole selector. Those subtract to how long a guest had been alive when it was read, which is what the log's last kernel stamp is then compared against. The console container is asked for in both status lists, since KubeVirt runs it as an init container and reading only one of them would report a container that started fine as one that never ran. A read that failed, a read cut off part way and a read that answered with nothing are recorded as themselves rather than left as a missing file, and the note names the error log only when there is one. The same capture also runs on the passing path, before the tenant is torn down and the Pod holding the stream goes with it. Every console this tree has collected came from a failure, so what a healthy boot looks like has never been measured, while "slower than usual" is the claim the failure keeps resting on. That caller asks for the pool's minimum rather than its maximum, since two workers booting is the same measurement as ten, and it warns in the job log rather than failing a suite that has already proved everything it exists to prove. Both paths write the same directory, so the capture also records which of them produced it. That capture costs real wall clock at the tail of an operation with a fixed ceiling, which is a way for a run that passed to be killed while collecting a debugging aid. So it is declined out loud when the ceiling no longer has room for it, against a reserve derived from its own cost and held in step with the timeout both suites give the operation. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |