mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
feat/cozystack-controller-gpu-only
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
f527ef17ee |
test(fluxcontract): accept a timeout-bounded history read
The monitoring suite bounds its HelmRelease history read with `timeout 30 kubectl`, which keeps a hung API call from eating the step's whole budget and the diagnostic with it. The shared-read pin admitted kubectl only bare or through the bounded wrapper, so the two landed side by side and left the unit tests red on main. The pin now takes an optional `timeout [-k N] N` in front of kubectl, the forms the suites already use for bounded reads. The bound is spelled out instead of allowing any prefix, since `$(echo kubectl ...)` would name the expansion without running it; that form, bare and behind a timeout, is now a refuse case. So is a timeout in front of _cozy_guard_kubectl: timeout runs a program, the wrapper is a shell function, and the read would fail every time. The command name has to end at a blank, so `kubectlx` or `_cozy_guard_kubectl_v2` is not taken for either one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
415430bcc7 |
fix(e2e): fail on a failed release of cilium and csi as on the parent, and report addon remediation (#3594)
## What this PR does The history guard in the tenant Kubernetes e2e suite checked only the parent `kubernetes-<test>` HelmRelease. A failed action on one of the addon releases it installs left nothing the suite looked at, so a run showed only the downstream symptom, usually the node-join deadline, and never named the release behind it. The guard now reads every addon release of the parent. It selects them from the cluster by name prefix, so a release added to the chart later is covered too. The rule: - A `failed` or `uninstalled` Snapshot fails the run on any release whose install strategy is `RetryOnFailure`. That is the parent (the aggregated apiserver stamps it on every Application HelmRelease) and the tenant `cilium` and `csi`. - The same footprint on the other addons, which run the default strategy with `retries: -1`, is printed as a note and the run stays green. For the CRD addons that configuration is tracked in #3555. A transient `cilium` or `csi` failure that later recovers on a retry still turns the run red, by design, as long as its Snapshot still reads `failed`. One recovery leaves no such Snapshot: when an install fails and the very next attempt succeeds, Helm marks the failed install `superseded`, because there is no deployed release to supersede, and the history reads like an ordinary upgrade. The guard cannot see that case from the statuses alone. The code and the docs now say so, a test pins it, and detection is left to a follow-up. An earlier revision of this PR tried to tell a teardown apart from a retried failure and made only `uninstalled` fatal. That split does not hold. In helm-controller v1.5.0, the version the platform installs, a release with an active retry strategy never remediates. `atomic_release.go` returns an upgrade for a failed release before it reaches the remediation switch, and the two other paths that uninstall (a changed release target, and deletion) both clear history afterwards. So on these releases `uninstalled` never shows up, and `failed` is the only signal. The parent keeps the rule it already had on main, and `cilium` and `csi` now follow it too. The failure paths get the same reading. The guard runs after the assertions, while the deadlines a failed addon breaks end the script earlier, so the same classification is also printed, without a verdict, from the node-join collector and from the exit handler. That handler is now armed at the tenant CR apply, so the management-cluster waits before the tenant answers are covered too. Smaller changes that came with this: - Every guard read is bounded, and a failed read is never folded into "no history". - The parent must read back a non-empty strategy. An empty value there means the jsonpath stopped resolving, and without this check every footprint would quietly turn into a note. - The history read uses `HELMRELEASE_HISTORY_JSONPATH`, and the upstream-type pin in `internal/fluxcontract` now accepts the bounded wrapper the read goes through. `docs/agents/e2e-testing.md` §6 and checklist item 8 describe the new rule. On related issues: this adds the `local` that #3774 reports missing in `helmrelease_has_remediation_cycle`, but not the sweep over the other `_lib/` helpers that the issue also asks for. For #3773 it rewrites the guard comment the issue quotes. The waits without a backstop are not touched. Closes #3553. ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map against the diff. It touches `hack/e2e-chainsaw/_lib/`, `hack/*.bats`, one Go test under `internal/fluxcontract`, two spend-order comments in the kubernetes suites, and `docs/agents/e2e-testing.md`. No package, schema, CRD, make target or user-facing document changes. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): the HelmRelease history guard now also covers the tenant addon releases. A failed or uninstalled Snapshot fails the run on the parent, cilium and csi, which retry in place; other addons get a note. The same classification is printed on the failure paths where the guard itself cannot run. ``` |
||
|
Aleksei Sviridkin
|
2df578bffd |
test(e2e): read the opensearch release history through the pinned name
The opensearch suite wrote the history jsonpath out as a literal, so the contract test that runs HELMRELEASE_HISTORY_JSONPATH against the upstream Flux type never saw the expression this suite hands kubectl. A field rename upstream would have reached it only as an empty read. The suite now takes the shared expression, and the contract test reads every Chainsaw suite that calls the guard, so a suite copying the literal, or assigning the shared name after sourcing the library, fails in the unit lane instead of going unchecked. The read is matched inside the script step that calls the guard, because every step runs in a shell of its own and a read in another step never reaches the call. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
8cb1023788 |
fix(e2e): fail on a failed release of cilium and csi as on the parent
The history guard covered only the parent Kubernetes HelmRelease. A failed action on one of the addon releases it installs left no mark the suite inspected, so a run surfaced only the downstream symptom, typically the node-join deadline, with nothing naming the release behind it. The tenant CNI and CSI are the sharp cases, since they carry a cluster-wide precondition. The guard now reads every addon release of the parent, selected from the cluster by name prefix so a release added later is covered too. A "failed" or "uninstalled" Snapshot fails the run on a release whose install strategy is RetryOnFailure: the parent, which the aggregated apiserver stamps that way, and cilium and csi. The same footprint on the other addons, which pair the default strategy with retries: -1, is printed as a note and leaves the run green. The footprint is not split into a teardown and a retried failure. In helm-controller v1.5.0, the version the platform installs, and unchanged in v1.5.1, a release with an active retry strategy never remediates: atomic_release.go returns an upgrade for a failed release before it reaches the remediation switch (lines 465-475), and the two other paths that uninstall, a changed release target and deletion, both call ClearHistory afterwards. On a RetryOnFailure release an "uninstalled" Snapshot is therefore unreachable, and the "failed" one is the only signal. A retry that later recovered still fails the run, by design, with one blind spot the statuses cannot close: a failed install that the very next attempt recovers is marked superseded by Helm, because there is no deployed release to supersede instead, and its history reads like an ordinary upgrade. The guard runs after the assertions, while every deadline a failed addon breaks ends the script earlier, so the same classification is also printed without a verdict from the node-join collector and from the exit handler. That handler is now armed at the tenant CR apply, so the management-cluster waits before the tenant answers are covered too. Every read is bounded and never folded into "no history", and the parent must read back RetryOnFailure: an empty value there means the jsonpath stopped resolving, and any other value means the apiserver stamp is gone, either of which would turn its footprint into a note. The history read takes its jsonpath from HELMRELEASE_HISTORY_JSONPATH, and the upstream-type pin accepts the bounded wrapper the read goes through. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
97cb2781db |
test(e2e): pin the release-history read against the upstream type
The test that claimed to pin the HelmRelease v2 history shape wrote a HelmRelease document containing the field names it expected, then read that same document back with the same jsonpath. It established that yq can read a fixture the test itself composed. Its own comment described the failure it was meant to catch: a future Flux release renames the field, the jsonpath matches nothing, the guard reports no remediation cycle, and real cycles pass the e2e assertion. A rename moves neither the fixture nor the expression, so the test stays green through exactly that event. The comment also named where the truth lives, github.com/fluxcd/helm-controller/api/v2, and pinned a copy of the author's expectations instead. Replace it with a Go test that supplies neither half. The expression is read out of the shell library that ships it, and the object is built from the upstream type at the version go.mod holds, then serialized and matched the way kubectl does. A renamed or retyped field is a compile error or a failed match; a guard that stops reading history fails the lookup with a message saying so. Both directions were confirmed by mutation. Locating the expression by searching the file is not enough on its own. Inline in a long script, locating it means deciding which occurrences are code and which are prose, and every way of getting that wrong fails green: an example quoted near a broken read satisfies the pin while the guard reads nothing. So the expression moves into a named assignment in remediation-guard.sh, which the guard already sources, and the test sources that library and takes the value the shell is left holding. Deciding which text is the assignment is then not the test's problem: an `export`, an indent, a second assignment later in the file all resolve the way the guard will resolve them, because the thing resolving them is the shell. Sourcing borrows the environment along with the shell, so the snippet unsets the name before it sources. `go test` hands the child its own environment, and a shell that had exported this name would answer in place of a library that stopped assigning it: the value comes back non-empty, the empty check stays quiet, and the pin passes over a guard that reads nothing. That is this file's own subject reaching it through the environment rather than through the source. Moving the expression out of the call site puts a step between what is pinned and what runs, so the call site is checked too, and the check starts from the call rather than from the text: the local the guard is handed must be assigned exactly once, by an anchored assignment whose jsonpath is the expansion of the shared name. That a well-formed read exists somewhere in the file is a weaker claim than it reads as, since a second assignment to the same local, or a read filling another one, satisfies it while the guard runs a literal nothing here pins. The expansion and not the name: kubectl echoes a braceless template back verbatim, so a read that dropped the `$` would return its own text, clear the script's empty-history backstop, and carry no status the cycle check matches. The library is checked by sourcing rather than by reading, so an assignment the shell would run is resolved by the shell instead of by a guess about which text the shell would pick. Each rule is pinned by its own test, so a regression names which one it opened rather than only that extraction broke. How kubectl's printer treats an absent path is its own configuration rather than part of this contract, so it is left unasserted: guessing at it would put the expectation and the fixture back in the same hands. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |