mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 02:27:01 +09:00
ci/shellcheck-gate
5280
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
c5f352829c |
ci(tests): gate the shell scripts this repository owns on shellcheck
shellcheck ran nowhere: not from the Makefile, not from a workflow, not from pre-commit. Seven files under hack/ nevertheless carried `# shellcheck disable=` directives, twelve of the twenty-eight with a rationale beside them, so the tree was written as though the check existed. hack/shellcheck.sh lints every shell script this repository owns and compares the result with hack/shellcheck-baseline.txt. Enumeration is a union of shebang and `.sh` suffix, and neither half is redundant. The suffix alone misses fifty-seven files with no extension, fifty-three of them the platform migrations run-migrations.sh runs on a cluster upgrade and which no linter had seen before this; the shebang alone misses thirteen held by the suffix, the sourced libraries that open with a `# shellcheck shell=` directive and four with no marker at all. Narrowing to either one loses files, and what the run then reports never names the enumeration as the cause. Measured, and it differs by half: a script that leaves scope carrying baseline entries goes STALE, one carrying none -- sixty-six of the hundred and ninety -- leaves in silence, and dropping the suffix branch also removes the sourced libraries from the command line, so their sourcers gain SC1091 as NEW. The reader is shown a fixed finding to regenerate away, or a new finding to go fix, and in neither case a narrower gate. The baseline records the findings that predate the gate as `path check count` triples. Among the scripts in scope no file is skipped and nothing is excluded on the command line, though in-file `# shellcheck disable=` directives are still honoured, so a finding suppressed that way is absent from the list rather than recorded in it; two classes sit outside the scan and say so in the baseline header, the vendored chart scripts that make update regenerates and the .bats files, whose bats shebang the enumeration does not match. shellcheck parses those perfectly well, with a dialect and checks of its own for them; they are not baselined yet, and the .bats arm of the filter is a guard for one that acquires a shell shebang rather than an exclusion that does anything today. Dropping line and column numbers keeps an unrelated edit from reddening the ratchet, while the count still catches a new finding in a script already listed. An entry whose findings are gone fails the run too, so fixing a script forces the list to shrink rather than leaving a standing licence to reintroduce what was removed. The regenerate advice is withheld whenever the same run also reports a NEW finding. Printed together, the last line read wins, and regenerating a mixed run records the new findings rather than reporting them -- the cure offered for one half launders the other. Three ways the gate could have passed without checking anything are closed. A missing binary fails rather than skips. The version is pinned on both sides and the script refuses to run when they disagree, because findings differ between shellcheck releases and an unpinned binary reports against a baseline not written for it. And shellcheck's own exit status is kept instead of being piped through xargs, which collapses it: a status above 1 means an input was never analysed, and that now fails rather than reading as a clean file. A path holding a space or a tab is refused instead of linted. A baseline entry is three space-separated fields, so such a path cannot be recorded: the summary splits it and the entry names a file that does not exist. Refusing is also what licenses the word splitting the enumeration relies on, which until now was a comment asserting the tree had no such name rather than a check that it does not. hack/shellcheck.bats drives the gate through a mutation that introduces a violation and its removal, both directions of the ratchet, each of the four refusals above, and the enumeration each half of the union catches, the shrink half in the case where a count drops without reaching zero as well as the one where an entry vanishes, and the whitespace refusal on a tab as well as on a space. Every red path asserts the exit status before it asserts the message: `! cmd` alone proves nothing, because `set -e` never fires on an inverted command, and a gate that printed each ERROR line while exiting 0 satisfied all thirteen output greps when that was measured. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6c82988570 |
fix(tests): freeze the EXIT-trap count for multus-install-cni-plugins.bats (#3584)
<!-- 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 `hack/multus-install-cni-plugins.bats` arrived on main in |
||
|
IvanHunters
|
318be935ca |
fix(e2e): capture Talos logs for tenant worker join failures (#3548)
## What this PR does Addresses #3513. When tenant Kubernetes workers miss the 18-minute Ready deadline, the E2E runner now mints a one-hour `os:reader` Talos client certificate from the tenant's existing issuer and runs authenticated `talosctl` from a hardened same-namespace diagnostics Pod. It captures bounded `dmesg` and kubelet output for every worker VMI in the cozyreport snapshot, records missing IPs and command exit codes, continues across unreachable workers, and preserves the original test failure. The diagnostics Pod has no service-account token or Secret volume; credentials are streamed into an `emptyDir`, and the Certificate, Secret, and Pod are labeled for cleanup. Workers that have not reached Talos `apid` can only yield a bounded connection error, while workers that reached `apid` provide guest logs. This adds the evidence needed to distinguish the remaining guest boot failure but does not claim to resolve its underlying cause. The readiness budget and success path are unchanged. Because this changes a shared E2E helper, test-impact analysis selects the full E2E suite for this PR. ### Screenshots Not applicable; no UI changes. ### Testing - `hack/cozytest.sh hack/run-kubernetes-talos-diagnostics_test.bats` (10 tests) - `hack/cozytest.sh hack/run-kubernetes-drain_test.bats` - `hack/cozytest.sh hack/cozyreport-talos.bats` - `bash -n hack/e2e-chainsaw/_lib/run-kubernetes.sh` - `git diff --check origin/main...HEAD` The full `make bats-unit-tests` run reached the unrelated `hack/migration-seaweedfs-db-adopt.bats` suite and could not continue because Docker is unavailable in the development environment. A live failure-path run was not performed because the available development environment has no Talos tenant fixture; this PR is intended to exercise that path in Cozystack CI. ### 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(e2e): capture authenticated Talos kernel and kubelet diagnostics when tenant Kubernetes workers miss the Ready deadline ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved troubleshooting when worker nodes fail to join by collecting per-node operating system and kubelet diagnostics. * Diagnostics now continue across partial failures and clean up temporary resources afterward. * Extended Kubernetes test operation timeouts to allow failure diagnostics to complete reliably. * **Tests** * Added regression coverage for diagnostic collection, timeout handling, multi-worker scenarios, missing node addresses, and cleanup. * **Chores** * Ensured the required diagnostic image is preloaded on nodes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b4e203164a |
fix(kubernetes): retry the tenant CSI install instead of uninstalling it (#3581)
## What this PR does The tenant CSI HelmRelease carried `remediation.retries: -1` with no strategy, so Flux applied its default, `RemediateOnFailure`. Install remediation is an uninstall in every case the API offers, so an install that missed its budget was torn down between attempts: the tenant lost its CSI node plugin and the StorageClasses this release propagates until a later attempt applied them again, and that attempt restarted the DaemonSet rollout instead of resuming one already under way. That teardown races the rollout it exists to recover, and the run recorded in #3576 shows how narrowly. Both DaemonSet pods were scheduled at the same second on both workers, so nothing was waiting on capacity; the ten minutes went to image pulls. The slower worker started its last container six seconds after the install timeout fired, and the uninstall removed it one second after that. `RetryOnFailure` leaves the manifests applied and retries the failed release. Both actions carry it because the controller picks the active retry from `.status.lastAttemptedReleaseAction` and performs that retry as an upgrade: set on install alone, the retry still happens, but its own failure is then handled by upgrade remediation, whose default is a rollback with no previous release to roll back to. The `remediation` blocks stay, because an absent release consults `install.remediation` and an out-of-sync one consults `upgrade`, both by retry count rather than by strategy. This is the change #3552 makes for the tenant CNI, and the reason for putting the strategy on both actions is the one given there. It is also the shape upstream prescribes: the "Recommended settings" block in the [HelmRelease spec shipped with the helm-controller this installs](https://github.com/fluxcd/helm-controller/blob/v1.5.1/docs/spec/v2/helmreleases.md#recommended-settings) puts `strategy.name: RetryOnFailure` under both `install` and `upgrade` for production deployments. Nothing changes on the test side, and that is deliberate rather than deferred. Because no manifest is removed at the attempt boundary, the rollout continues across it and the kubelet finishes what it started, while the retry re-applies the same manifests every 30s. The release therefore reports Ready once the rollout completes rather than once a whole attempt has fit inside the timeout, so what the existing e2e wait has to outlast is the rollout and no longer an attempt boundary that discards it. That widens what the wait tolerates without making it unbounded: a rollout slower than the wait itself still fails, and this does not claim otherwise. Scope. #3576 records more than this fixes, so this does not close it. The other tenant addon HelmReleases this chart renders keep the remediation-only shape; CSI is the one this changes. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: The trigger map was walked against the diff. The change is a HelmRelease template and a chart test, not a `values.schema.json` field, a `values.yaml` default, a version enum, a `release.prefix`, an `ApplicationDefinition`, a chart source kind or a namespace. ### Release note ```release-note fix(kubernetes): the tenant CSI HelmRelease now retries a failed install instead of being uninstalled between attempts, so a failed install no longer takes the tenant's CSI node plugin and its propagated StorageClasses away while it retries. The same strategy applies to the upgrade action, which it has to: a retried install runs as an upgrade. One consequence is operator-visible — a failed CSI upgrade in a tenant cluster now retries the new manifests every 30s instead of rolling back to the previous release. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability when installing or upgrading the CSI component by automatically retrying failed actions every 30 seconds. * Retains unlimited remediation retries to help recover from transient failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
0dae3ca95a |
fix(e2e): budget tenant scheduling separately from workload readiness (#3579)
## What this PR does The tenant backend Deployment in the kubernetes suites had one 300s budget to reach `condition=Available`, and two unrelated variable costs shared it. The first is scheduling. What the suite establishes before that point is two tenant nodes Ready, which is weaker than schedulable: a Ready node still carries `node.cilium.io/agent-not-ready` until the tenant cilium agent claims it, and a node the bringup has not finished with is `SchedulingDisabled`. In the run recorded in #3577 scheduling took 2m18s and 1m57s in the two suites, which left the image pull to finish inside what remained. It did not, and both suites reported the same `timed out waiting for the condition` for what were two different shortfalls. So this waits for a node that actually accepts a Pod before creating the workload, on its own budget and its own failure message. The gate encodes the scheduler's rule for a Pod that tolerates nothing (Ready, not unschedulable, no `NoSchedule` or `NoExecute` taint) over a `custom-columns` probe, prints the node table on both outcomes so a timeout names the taint that held it, and treats a failed probe as not-schedulable so an API blip cannot release it. On the happy path it adds no wall time: it spends the seconds the Pod would otherwise spend Pending, plus at most one 5s poll interval. On a failing run the two budgets stack, so a run that exhausts both now gives up at ~600s where it used to give up at 300s, inside the 40m Chainsaw script op the suites document as a ~25m bringup. The readiness wait keeps its 300s, now starting from a schedulable node, and dumps deployment, pod and event state when it runs out. The workload image is also pinned by digest. The tenant workers reach no Docker Hub mirror (`hack/e2e-talos-image-cache.yaml` serves the Talos worker OS disk image over HTTP and is not a registry mirror), so nginx is pulled from Docker Hub on every run either way. The digest does not take the pull off the critical path; it fixes what that pull returns instead of leaving a floating tag free to change size and content under a fixed deadline. Preloading the image would need infrastructure this tree does not have, and is not attempted here. Nothing will bump the pin: no renovate manager reads `hack/`, and the comment says so, because for a throwaway test workload the freeze is the point. What this does not claim: it removes two measured consumers from a fixed budget, both taken from the events of a single run. Whether that budget was the only thing making those suites red is not established from one run, so this is not offered as the fix for a red pipeline. `hack/run-kubernetes-schedulable_test.bats` covers the new logic: every branch of the predicate, the multi-node scan, the poll-again path, the deadline, and a probe that fails. Each test was verified by mutating the helper and checking that the intended test, and no other, went red. Note for whoever reviews alongside #3575: that branch mirrors `ghcr.io` for tenant worker pulls, not `docker.io`, so it does not change the Docker Hub pull described here. `git merge-tree` reports no conflict between the two, nor with #3548. Observed in #3577. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: The trigger map was walked against the diff. The change is confined to `hack/e2e-chainsaw/_lib/run-kubernetes.sh` and one new `hack/*.bats` unit test. It moves and renames nothing under `hack/`, changes no make target, and does not touch `hack/e2e-prepare-cluster.bats`, any package, any CRD or any namespace name, so none of the listed repositories are reached. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Kubernetes deployment readiness by waiting for a suitable, schedulable node before timing backend startup. * Added clearer diagnostics when nodes are unavailable or backend readiness fails. * Replaced the floating backend container image tag with a fixed, verified version. * **Tests** * Added coverage for node readiness, cordoning, taints, polling, timeouts, probe failures, and deployment sequencing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
3aef42b993 |
fix(tests): freeze the EXIT-trap count for multus-install-cni-plugins.bats
hack/multus-install-cni-plugins.bats arrived on main in
|
||
|
Aleksei Sviridkin
|
879d0f6db6 |
fix(installer): make operator readiness honest in the chart and the install gate (#3466)
## What this PR does
The operator Deployment ran `replicas: 1` with `maxUnavailable: 1`, so
the Deployment controller took the minimum available replica count as `1
- 1 = 0` and the `Available` condition reported `True` with no running
pod at all. Measured against an operator whose image tag does not exist:
the condition was `True` with `reason: MinimumReplicasAvailable` while
the only pod sat in `ImagePullBackOff`, `unavailableReplicas: 1`, and no
`availableReplicas` key in the status.
That broke two separate things, so this fixes both.
The E2E install gate asked `kubectl wait --for=condition=Available` and
passed in 156 ms against that dead operator. The install then carried on
and died about two minutes later on the `packages.cozystack.io` CRD wait
further down the file, which sends you after CRD installation when the
operator never started. It now uses `kubectl rollout status`, which
reads updated and available replicas directly, and dumps the deployment,
pods, operator logs and namespace events when it fails. This half is
deliberately independent of the chart: `rollout status` is correct under
any rollout strategy, so a later change to that strategy cannot quietly
make the gate vacuous again.
The chart is the other half, and fixing the gate does not cover it.
`Available` is read by more than this test, and until the strategy
changes an alert on that condition, a health dashboard, or a person
reading `kubectl get deploy` all show green while the operator is dead.
The strategy moves to `maxSurge: 1` / `maxUnavailable: 0`, which
restores the ordinary meaning of the condition. Kubernetes refuses
`maxSurge: 0` and `maxUnavailable: 0` together, so the surge has to move
for the unavailability to reach zero.
That pair is worth reading twice, because it is not a configuration I
picked. At `replicas: 1` the Kubernetes default of `25%` / `25%`
resolves to exactly `maxSurge: 1` and `maxUnavailable: 0`, since the
surge rounds up and the unavailability rounds down. What this PR does is
stop overriding the default, not introduce a tuning of its own, and the
risk of the change should be judged on that basis.
Moving the surge is safe here, and I checked rather than assumed,
because the pair came from somewhere. It was carried over from the
earlier `cozystack` Deployment, where a sidecar published host port 8123
and `hostNetwork` left the surge pod unschedulable on a single-node
cluster. It got here in two steps, which is worth knowing if you go
looking for the reason: by the commit before this file existed the
sidecar was already gone and no port was declared, yet the pair stayed.
It had outlived its own reason before it was ever copied, which is why
there is no justification to find in this file's own history. On a
single-node cluster the surge pod is scheduled next to the outgoing one
and the rollout completes; adding a declared container port reproduces
the original `didn't have free ports for the requested pod ports`
failure exactly. Nothing else in the pod holds a port either: metrics
and health probes are bound to `0`, and the configured webhook server
never starts because the operator registers no webhook and never asks
the manager for the server.
The surge makes leader election load-bearing, though it covers less of
the overlap than it first appears. While `maxSurge: 0` capped total pods
at `replicas` a second manager process could not exist, so
`--leader-elect=true` was defence in depth. With a surge pod the lease
is what keeps the incoming manager's reconcile loops off the cluster
while the outgoing one is still leader, and because there is no
readiness probe the incoming pod counts as available as soon as its
container starts, so that overlap covers the whole image pull. It does
not cover everything: the operator installs CRDs, installs Flux and
creates the platform PackageSource before `mgr.Start()`, and the lease
is only taken inside `mgr.Start()`, so those three privileged installs
run outside leader election entirely. They are safe because both pods
write the same objects through server-side apply under the same field
manager, which is a different guarantee from the lease. The
`k8s-await-election` wrapper this operator replaced did hold the whole
process until it won leadership, so if that property sounds familiar it
belongs to the predecessor rather than to this binary.
The sibling `opensearch-operator` chart turns leader election off at one
replica for a real reason: a transient lease-renewal blip makes
controller-runtime exit the process, which at one replica reads as a
crashlooping Deployment. This operator installs first, when the
apiserver is at its shakiest, so that scenario applies here too. I am
accepting the risk knowingly, because after `maxSurge: 1` the
alternative is two managers reconciling the same cluster, which is
worse. The flag is pinned in the unit test so it cannot be turned off
without the test going red.
One consequence is permanent, so I am writing it down now instead of
leaving it to be found in a year. Because any bound port breaks the
surge pod under `hostNetwork`, an `httpGet` readiness probe is off the
table for this Deployment for good and only an `exec` probe could ever
work. So `Available` still answers "a container started", not "the
operator is healthy": an operator that starts and then dies partway
through installing CRDs is caught by the CRD waits below the gate, not
by the gate itself.
The surge also opens one new window. The operator installs CRDs and Flux
before the manager starts, which puts that work outside leader election,
so the incoming pod runs those applies while the outgoing pod is still
leader. Both apply the same objects server-side and the outgoing pod
exits moments later.
`--wait` is dropped from the helm step in the same change. Helm decides
a Deployment is ready with the same `replicas - maxUnavailable`
arithmetic, so on the old strategy it was satisfied at zero ready pods
and gated nothing, which is why the broken install got past it as well.
Once the strategy is honest it would gate for real, but on its own 2m
budget, and it would fail the install before the rollout gate ran and
hide the diagnostics behind a bare Helm timeout. Hooks are waited for
with or without the flag, and the release otherwise carries only the
Deployment and its RBAC objects, so the rollout gate is the better
single place to decide the operator is up.
A helm unittest suite pins the strategy and all four preconditions the
surge makes load-bearing: no container or initContainer declares a port,
both listener arguments stay disabled, leader election stays on, and no
probe uses `httpGet`. That last one follows from the two disabled
listeners leaving no port for a probe to reach, and it is the worst of
the four to get wrong, because the others wedge an upgrade while a
`livenessProbe` against a disabled listener crashloops the pod on every
cluster, fresh install included. Only the `httpGet` field is forbidden
rather than the probes themselves, so an `exec` probe stays available.
All four run against all three chart variants, because the template
branches on variant inside the container spec and a port added under one
branch would render for that variant alone.
I checked the suite fails for the right reasons instead of trusting it.
Reverting the strategy, declaring a `containerPort` on the operator
container, declaring one under a single variant branch, declaring one on
an initContainer, pointing a bind-address at a real address, turning
leader election off, and adding a readiness, liveness or startup probe
with `httpGet` each turns it red on its own assert. Inserting a
container ahead of the operator trips the container-name pin before the
argument asserts can start describing the wrong container. An `exec`
probe correctly stays green.
The rationale is split by audience. What an operator applying the
rendered manifest has to respect stays in plain comments and travels
into `_out/assets/cozystack-operator-*.yaml`: why the strategy is what
it is, that no port may be declared or bound, and that leader election
has to stay on. Everything that is only about this repository moves into
a `{{/* */}}` template comment that stops at the chart boundary. That
covers the pointer to the test and which assert pins what, where the
strategy was copied from, why the webhook server on 9443 is not a third
port binder, and why `opensearch-operator` reaches the opposite
conclusion on leader election. The other templates in this chart already
keep maintainer notes that way.
### Screenshots
Not applicable, no UI changes.
### Downstream repositories
I left the boxes empty on purpose rather than to skip the question,
because the honest answer is not one of them and someone else should
make the call.
Walking the trigger map in `docs/agents/contributing.md`, nothing fires.
The `hack/` triggers name `hack/e2e-prepare-cluster.bats`,
`hack/package.mk`, `hack/update-crd.sh` and moves or renames under
`hack/`, and this PR only edits the body of
`hack/e2e-install-cozystack.bats`. The `ansible-cozystack` trigger is
renaming an object the role waits on, and
`deployment/cozystack-operator` keeps its name.
The behaviour that role sees does change, though. It runs `kubectl wait
deployment/cozystack-operator --for=condition=Available`, the same
vacuous check this PR replaces here, so the chart change turns its wait
from one that returns instantly into one that means something. Its
`cozystack_operator_wait_timeout` defaults to 300 seconds, the same
budget I gave the gate here, so a cold image pull fits and I do not
think anything needs to change over there. No follow-up PR seemed
warranted for a change that only makes an existing check work, but that
is a judgement call rather than a trigger, which is why the boxes are
empty.
- [ ] 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(installer): the cozystack-operator Deployment no longer reports Available with zero running pods. It ran replicas: 1 with maxUnavailable: 1, which made the minimum available replica count 0, so the condition stayed True while the operator was dead and anything reading it, including the E2E install gate, saw green. The rollout strategy is now maxSurge: 1 / maxUnavailable: 0 and the install gate waits for the rollout instead of the condition.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved operator installation checks by monitoring deployment
rollouts and providing deployment, pod, log, and event details when a
rollout fails.
* Updated operator upgrades to maintain availability during rollout,
including in single-node environments.
* Documented rollout requirements and behavior across supported
installation configurations.
* **Tests**
* Added coverage for rollout settings and upgrade compatibility across
supported installation variants.
* Clarified Helm unit-test directory coverage documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Aleksei Sviridkin
|
2527e915bd |
fix(e2e): collect diagnostics for unready pods and record every truncated capture (#3567)
## What this PR does The e2e diagnostic tarball exists so a red run can be explained from the uploaded artifact alone. Several of its collectors answered "nothing is wrong" instead — some because they selected on a column that never carries the value they filtered for, others because they stopped early and left nothing saying they had. Both failure modes look identical to a healthy cluster from the outside, which is the worst possible shape for a diagnostic. Four selectors in `hack/cozyreport.sh` and `hack/cozyreport-summary.sh` were reading the wrong thing: - Per-pod evidence was chosen by the STATUS column, so a pod left at `0/1 Running` by a readiness probe that never passes was skipped. That is a common way for a suite to time out, and the artifact carried no trace of the pod the run died on. Selection now compares the two halves of the READY column; terminal `Succeeded`/`Completed` pods stay excluded. - The pending-LoadBalancer section filtered `$4`, which is CLUSTER-IP. CLUSTER-IP holds an address, `None` or `<none>` and never `<pending>`, so that loop had never executed. `<pending>` only ever appears in EXTERNAL-IP, `$5`. - The cozystack-apps section gated all three of its kinds on `kubectl get crd <kind>.apps.cozystack.io`. ApplicationDefinition is a CRD but in a different group, and Tenant comes from an aggregated APIService with no CRD to ask about, so the probe failed and the section never ran. - The kamaji section was gated on `cozy-linstor`'s `linstor-controller` deployment rather than kamaji's own, so a cluster without LINSTOR got no kamaji logs and no TenantControlPlanes — on exactly the runs that need them. The summary had three of its own: the HelmRelease listing printed `$5` where STATUS is the Ready condition's whole message, reducing every failed release to one word like "Helm"; the Flux sources listing used a jsonpath with a nested filter that kubectl rejects outright, so it printed nothing on every report ever taken; and pod AGE was read as `$6`, which shifts whenever RESTARTS prints the `7 (4m12s ago)` form — that is, for exactly the rows worth reading. The second half is bounding. The pod, VM, VMI and services walks now run under a count cap and a wall-clock budget, and every read is individually time-boxed. The point is not the bounds themselves but that each one names itself when it fires: a walk stopped by the cap or the clock leaves `COLLECTION-TRUNCATED.txt`, a pod list that never returned leaves `COLLECTION-FAILED.txt` instead of the empty tree a healthy cluster produces, a log cut short says so on its own last line, and a `--tail` bound is recorded beside the logs rather than inside them, since the platform's controllers log JSON per line and a trailing prose line costs a parser the whole file. A knob value the collector could not use is reported too, so a truncation note quoting `600` is never mistaken for the `10m` somebody actually set. Where `timeout` is absent the reads run unbounded rather than not at all, and the section says which of the two happened, because a missing local binary is not a statement about the cluster. One behaviour change worth stating plainly, because it is a trade and not a pure gain: the per-pod section used to be uncapped and read container logs whole, and it is now bounded by `COZY_REPORT_PODS_MAX` (40), `COZY_REPORT_PODS_BUDGET` (600s) and `COZY_REPORT_POD_TAIL` (2000). Below those thresholds the new tree is a strict superset of the old one — the old selector missed `Running 0/1` entirely and skipped logs for `Pending` pods. Above them the artifact is smaller than before, every drop is named in the tree, and `COZY_REPORT_POD_TAIL=-1` with a larger `COZY_REPORT_PODS_MAX` restores the old reach. Because widening the selection and capping the walk in one change could otherwise let a newly-in-scope pod evict one the uncapped walk always collected, the ordering splits by evidence class before namespace: pods that are not `Running` — the class the collector took before this — all rank ahead of pods that are `Running` but unready, so the cap can only ever drop a pod the old collector did not keep either. `hack/e2e-capture-previous-logs.sh` gets the same treatment: its notes now go to `capture-notes.txt` in the artifact as well as to the job log, and a pod list that never returned is reported as such rather than as "nothing restarted". That script now creates its own output directory before its first line of output, which makes the comment in `hack/e2e-chainsaw/.chainsaw.yaml` wrong about why the once-per-test guard there holds — the only change to that file is the corrected comment, saying the `-d` check keys on whether the catch block already ran rather than on what the helper found. The rest is test coverage — a new `hack/cozyreport.bats`, an extended `hack/capture-previous-logs.bats` — and the removal of `EXIT`-trap cleanup from the bats files those tests now cover. That trap is worth its own note: inside an `@test` body it replaces the one the `bats` binary installs for its own bookkeeping, so a test that then fails prints no TAP line at all. It does not appear as `not ok`; it disappears, and only the `# bats warning: Executed N instead of expected M tests` line at the end distinguishes the run from a green one. A guard test enforces the ban over a named list of files that grows as the remaining ones are converted. ### Testing Every collector this touches is now covered by unit tests that run without a cluster: a new `hack/cozyreport.bats` (166 tests) and an extended `hack/capture-previous-logs.bats` (34), driven through `hack/cozytest.sh`. Ten `hack/*.bats` files run green locally, 265 tests in total, with each file's `@test` count reconciled against its passing count rather than trusted to the exit status — that reconciliation is the point of the trap change, since an exit code answers "did anything fail" and never "did anything run". The guard test that bans `EXIT`-trap cleanup was checked by committing the violation rather than by reading it: a folded `trap 'rm -rf "$tmp"' \` + `EXIT` added to a file on its list turns the guard red and names that file, and removing it turns the guard green again. `shellcheck --shell=sh` over the three changed scripts reports strictly fewer findings than before the change (`hack/cozyreport.sh` SC2086 170 to 116, SC2162 15 to 13; the three SC2102 in `hack/cozyreport-summary.sh` are gone), and `hack/e2e-capture-previous-logs.sh` is clean. The parts that need a cluster — that the bounds fire, and that each one lands in the artifact — are exercised by the e2e run itself, since these collectors run on every failed suite. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. This PR touches `hack/cozyreport.sh`, `hack/cozyreport-summary.sh`, `hack/e2e-capture-previous-logs.sh`, `hack/e2e-chainsaw/.chainsaw.yaml`, ten `hack/*.bats` files and `docs/agents/e2e-testing.md`. Nothing is moved or renamed under `hack/`, no make target changes what it does, and the files downstream consumers key on (`hack/package.mk`, `hack/common-envs.mk`) are untouched. No package, values key, CRD, variant, namespace or release asset is involved. - [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): the diagnostic report now collects per-pod evidence for pods that are Running but not Ready, which it previously skipped, and several collector sections that silently produced nothing now report correctly. Collection that stops early on a bound now says so inside the artifact, so a truncated report can be told apart from a healthy cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Diagnostic reports now prioritize relevant resources and collect bounded pod, VM, service, storage, certificate, and Flux data. - Reports preserve partial results and clearly identify timeouts, unavailable resources, truncation, warnings, and collection failures. - Previous-container logs now include bounded capture, namespace prioritization, failure details, and partial-read evidence. - Reports target the operator workload and safely handle missing dependencies or temporary storage failures. - **Documentation** - Expanded guidance for reliable end-to-end testing and diagnostic report collection. - **Tests** - Added comprehensive coverage for report generation, log capture, failure handling, limits, and portability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
a20f3620b1 |
fix(e2e): select catch pod logs by the nested release instance label (#3564)
## What this PR does The harbor, openbao and qdrant catch blocks selected pod logs on the app release name. Each of these charts deploys its workload through a nested HelmRelease named `<release>-system` and passes `fullnameOverride: <release>` down to it, so the inner chart stamps `app.kubernetes.io/instance` from its own release name, with the suffix, while object names come from the override, without it. The selector matched no pods, and a failed run collected nothing. - points the three `podLogs` catch collectors at the `-system` instance label - adds a `podLogs` collector for Harbor's CNPG database, which no collector reached before, selected on `cnpg.io/cluster` rather than on the instance label CNPG merely derives from the Cluster name - pins the instance label on a resource in an assert each suite already runs, so a chart renaming that label fails a step instead of quietly emptying a capture - prints the instance label of every pod in the namespace from each test-wide catch, because `podLogs` cannot report that it matched nothing - keeps the unsuffixed value as a second collector in qdrant only, because the parent chart's PVC-cleanup Job labels its pod template with the parent release name and that suite deletes the parent inside a `try:` Harbor renders the same hook and deliberately does not get the same collector: it has no `delete` op, so its parent release is uninstalled only in Chainsaw's cleanup phase, which no catch can observe: catch is a defer inside the step, while the cleaner runs from `t.Cleanup`, after the test function returns. A comment records that asymmetry so the next reader does not restore the collector for symmetry. Each comment states what the pin does and does not cover: it does not catch a wrong value typed into a selector, and it catches nothing when a run fails before reaching the assert. The qdrant comment is equally explicit about how narrow its collector's window is, because the hook always exits 0, so Helm calls it succeeded and `hook-succeeded` takes the pod away; what remains is an uninstall that hangs with Helm still waiting. Telling a lookup's outcomes apart is one helper under `hack/e2e-chainsaw/_lib/` rather than a copy per call site, with the outcome logic split from the kubectl call so it can be pinned without a cluster — the shape `_lib/etcd-probe.sh` already uses. Both the census and the qdrant cleanup listing report through it, differing only in wording. Each lookup keeps stderr apart from the value it captures: kubectl exits 0 while logging that it could not reach an API group, and an aggregated APIService in that state is what a failed run usually looks like, so folding the streams would make the "nothing found" branch unreachable and print that log line as though it were a result. Note what the extraction costs in CI: touching the shared `_lib/` and adding a top-level `hack/*.bats` escalates test-impact analysis from the three suites this change edits to the full Chainsaw suite, so this PR runs everything and is exposed to the whole spread of known flakes. A red suite unrelated to harbor, openbao or qdrant is that exposure, not a defect in this diff. ## Testing - `hack/pod-label-census_test.bats`, 9 cases over the helper's outcomes, run via `hack/cozytest.sh` and picked up by `make unit-tests` through the existing `BATS_UNIT_FILES` glob - mutation-checked rather than assumed: folding stderr back into the captured value, propagating kubectl's exit code out of the helper, and collapsing the failed/empty distinction each turn the suite red, and each is green again on revert - `chainsaw lint test --file` on each of the three suites, and `chainsaw test --no-cluster --config hack/e2e-chainsaw/.chainsaw.yaml --include-test-regex '^$'` over all three, both with v0.2.15, the version the sandbox image pins - `shellcheck --shell=sh` on the helper and on the catch scripts extracted from the YAML - the qdrant step catch driven against a stub kubectl that fails: it reports the failed lookup rather than dying on it - every selector, pin and comment claim checked against the chart that stamps it ### 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: ### Release note ```release-note fix(e2e): collect harbor, openbao and qdrant pod logs on a failed run ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added pod-label census diagnostics to improve visibility into workload labels during test failures. * Enhanced failure diagnostics for Harbor, OpenBao, and Qdrant by collecting relevant workload and database logs. * Added bounded reporting for missing pods, lookup failures, and incomplete cleanup resources. * **Bug Fixes** * Updated diagnostic selectors and readiness checks to match chart-generated instance labels. * Improved PVC cleanup diagnostics with clearer reporting of remaining resources. * **Tests** * Added coverage for successful, empty, failed, and partial diagnostic results. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
a4a8e4143f |
fix(kubernetes): retry the tenant CNI install instead of uninstalling it (#3552)
## What this PR does The tenant Cilium HelmRelease carried no install strategy, so Flux applied its default, `RemediateOnFailure`, which uninstalls the release between attempts. Cilium is the tenant cluster's only CNI, so every cycle takes it away and gives it back when the next install applies the manifests again, and `retries: -1` lets that repeat without end. What sets a cycle off is an install that outruns its budget, which can happen while the tenant still has no registered node, since the operator Deployment cannot become Available until one exists. Remediation neither causes that delay nor is fixed by this change: what it adds is a repeated teardown of the CNI on top of a delay that resolves itself once the node arrives. This removes the amplifier, not the cause. `RetryOnFailure` keeps the applied manifests in place and retries a failed install as an upgrade, so the agent DaemonSet rolls out as soon as a worker registers. Both actions carry the strategy because helm-controller selects the active retry from `.status.lastAttemptedReleaseAction`, and the retry of a failed install runs as an upgrade. With the strategy on `install` alone that retry still happens, but once it is the last attempted action its own failure is handled by upgrade remediation, whose default is a rollback with no previous release to roll back to. The `remediation` blocks stay because two branches consult the retry count rather than the strategy and refuse to act once it is exhausted: an absent release reads the install block, an out-of-sync one reads upgrade. `retries: -1` keeps both open. The cost is that a genuine upgrade failure no longer rolls back either, it retries on the failed manifests instead. That is forced, since the controller cannot tell a retried install from a real upgrade, and what it replaces was an unbounded rollback-and-retry flap rather than a stable safety net: `upgrade.remediation.retries: -1` with the default rollback strategy already retried forever. This is the strategy and retry interval that `pkg/registry/apps/application/rest.go` and `internal/operator/package_reconciler.go` already set on every HelmRelease they generate, except that they leave `remediation` nil where this release keeps it. The other tenant addons this chart renders still carry the remediation-only shape; this PR changes the CNI alone, where the teardown removes the cluster's only CNI rather than churning a single component. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(kubernetes): the tenant Cilium HelmRelease now retries a failed install instead of being uninstalled between attempts, so a failed install no longer takes the tenant cluster's only CNI away while it retries. The same strategy applies to the upgrade action, which it has to: a retried install runs as an upgrade. One consequence is operator-visible, a failed Cilium upgrade in a tenant cluster now retries the new manifests every 30s instead of rolling back to the previous release. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Cilium installation and upgrade reliability by automatically retrying failed operations every 30 seconds. * Preserved deployed resources during retries and continued supporting unlimited remediation attempts. * **Tests** * Added coverage validating installation and upgrade retry behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
b758c3bde2 |
feat(multus)!: stage reference CNI plugins from the multus image (#3195)
## What this PR does Multus-backed NetworkAttachmentDefinitions delegate to the upstream containernetworking reference plugins (`bridge`, `macvlan`, `ipvlan`, ...), and nothing guarantees a node has them where Multus looks. On k3s they live under `/var/lib/rancher/k3s/data/cni` while Multus and Cilium install into `/opt/cni/bin`, which is left without a `bridge` binary. A `bridge`-type NAD then fails with `failed to find plugin "bridge" in path [/opt/cni/bin]`, and today you work around it by copying the binary in by hand. #3196 documents one such pattern (attaching a VM to an external VLAN) that this unblocks. This bundles 14 of the reference plugins into the `multus-cni` image and adds an `install-cni-plugins` init container that installs them into the host `/opt/cni/bin`, mirroring the existing `install-multus-binary` init container. ## Where it applies, and how to turn it off The new `networking.stageCniPlugins` value defaults to on for both bundles. Talos puts six binaries in that directory -- `bridge`, `firewall`, `host-local`, `loopback` and `portmap` stripped from the same upstream release, plus `flannel`, which is a different project -- so the other ten are missing there exactly as on generic Linux and a NAD naming `macvlan`, `ipvlan` or `vlan` fails on either platform. Four of those six are names this change stages; `loopback` is deliberately not staged and `flannel` was never in the reference set. The four names that overlap are rewritten on every Multus pod recreation with the same upstream version -- Talos pins `containernetworking/plugins` `v1.9.1` in `siderolabs/pkgs`, and so does this image -- so it is not a downgrade of a system component. A marker in the image Dockerfile records the Talos release that pin was compared against, and a test holds it to the release the platform ships. `isp-hosted` ships no multus at all and is unaffected. Both defaults can be overridden either way. If you run a generic-Linux cluster and manage `/opt/cni/bin` yourself, this changes behaviour on upgrade. Multus starts installing 14 plugins into that directory every time its pod is recreated, replacing files of the same name, so a hand-placed or locally patched plugin there will not survive. Set `networking.stageCniPlugins: false` to decline, and set it **before** the upgrade that first enables staging: nothing here removes or restores a plugin, so declining afterwards protects what is still yours and does not give back what was already replaced. Changing the value edits the Multus DaemonSet's pod template, so Multus rolls node by node, and while a node's daemon is down pod creation and deletion on that node fail for the length of the restart. The four plugins the package never touches are `loopback`, `dhcp`, `dummy` and `tap`. Where staging runs it is unconditional, not only-when-absent, because nothing here ever revisits a plugin it has written. Installing only when absent would pin each node to whichever version arrived first, and a later `CNI_PLUGINS_VERSION` bump, including one fixing a binary that runs as root on every node, would reach zero existing nodes. The directory is not ours alone and the decision does not assume it is: kube-ovn copies `loopback`, `portmap` and `macvlan` there from its own image, two of which this change also writes, node provisioning may place the set, and `/opt/cni/bin` is also the CNI bin directory on RKE2. Overwriting is what delivering a version requires, and `networking.stageCniPlugins: false` is how an operator who owns those files declines it. ## Notes - The manifest change lives in `patches/customize-deployment.patch` so it survives `make update`, and the release rewrite in that target runs after the patch rather than before it, so the line the patch adds lands on the same reference as the two it already had. The patch seeds that line with the upstream image reference while the committed template carries the pinned cozystack digest, because the package's `image:` target rewrites every `multus-cni` image line to one digest. So it does not reverse-apply against the committed template. That `make update` reproduces the tree is verified forward from pristine upstream instead. - `/opt/cni/bin` stays shared where staging is on. kube-ovn installs `portmap` and `macvlan` there and has been seen to do so with a plain copy onto the live path, which truncates in place. Each plugin is installed under a temporary name and swapped in with a rename, atomic within the mount, so the kubelet never execs a half-written binary. The Dockerfile records which kube-ovn release the version pin was checked against, and `hack/cni-plugins-staging-contract.bats` fails when that marker and the vendored release part ways. That check fires on the next kube-ovn bump and compares the marker with the vendored release; it does not compare the two `CNI_PLUGINS_VERSION` pins. Moving the marker is the acknowledgement that someone looked -- so read kube-ovn's `dist/images/Dockerfile.base` before moving it, because a silent divergence there makes `portmap` and `macvlan` flap between the two DaemonSets on every pod recreation. - The init container runs unprivileged, with capabilities dropped and no mount propagation. Writing into a hostPath needs root inside the container and nothing on the host, which is how cilium's `install-cni-binaries` copies plugin binaries into the same directory on the same nodes. `Bidirectional` propagation governs mounts made inside a container, this script makes none, and asking for it is what would force `privileged`. The tests pin the absence. - `loopback` is excluded because replacing it is what a node cannot survive: the runtime calls it for every sandbox, and Cilium and kube-ovn already install it. `dhcp` needs a host daemon this package does not run. `tap` and `dummy` have no consumer here. `portmap` is included because a NAD conflist may chain it for hostPort. - The plugin version is pinned via `ARG CNI_PLUGINS_VERSION` (v1.9.1) and the release tarball is verified against a per-arch SHA-256 before extraction. These are root-executed node binaries, so a tag alone is not enough. An unknown or empty `TARGETARCH` fails the build. - The init container skips staging when the image has no `/cni-plugins`, rather than crashlooping the DaemonSet node-wide. The manifest and the image digest are re-pinned on different schedules, so a tree taken between a Dockerfile change and the next release bake can carry one without the other. A plugin it cannot copy is reported on stderr and skipped, for the same reason. `multusConfigFile: auto` makes `00-multus.conf` the primary CNI config and there is no `cleanupConfigOnExit`, so on a node where Multus has already run, an init container that never completes leaves the shim's socket gone with that config still primary, and the node stops scheduling anything that goes through CNI; host-network pods still start. Failing on a copy error would therefore leave the node worse off than before this change, which only ever left it without one plugin. The cost of continuing is that the pod still reports Ready, so the summary is also written to `/dev/termination-log`, which the kubelet surfaces in `kubectl describe pod` regardless of the exit code. - The bundled plugins add roughly 67 MiB uncompressed per architecture to an image pulled by every node, including installs where staging is off. Measured by extracting the staged set from the v1.9.1 release tarball. ## Testing `packages/core/platform/tests/bundles_multus_staging_test.yaml` pins the delivery point: both variant defaults, both explicit overrides, and that multus is still shipped in each bundle at all. That last one because multus is now included by hand in two branches rather than by the shared helper. `hack/multus-install-cni-plugins.bats` extracts the init container's script from the rendered DaemonSet and runs it. It pins that every plugin lands executable, that a replacement swaps the inode rather than truncating in place, that temp files never leak, that a missing or empty directory skips instead of failing, and that a plugin which cannot be installed is reported on stderr and skipped rather than failing the container. Both halves of that last one are asserted, because exit 0 alone would still pass with the report deleted. `hack/cni-plugins-staging-contract.bats` covers what the rendered manifest cannot show: that the Dockerfile stages the intended plugins and only those, copies them into the final stage, and that nothing there removes them again, that the tarball is verified before extraction, and that the version marker matches the vendored kube-ovn release. helm-unittest pins both states of the value, including off, which is what most installs run, plus the quoted-string forms a values file can produce. An explicit `null` is covered too: it is how a values file clears the key, and the chart folds it onto its own default rather than refusing it as an unreadable spelling. The feature is inert until the image is re-baked. All three `image:` lines still point at the pre-change digest, which has no `/cni-plugins`, so on `main` after merge a generic install renders the init container, logs `no /cni-plugins in this image; skipping staging`, and installs nothing until release-prep re-pins. That is the intended flow (first-party digests are release-managed, not hand-edited), and the guide documents the log line, but anyone testing this on `main` will meet it first and the pod will look healthy. No CI job stages plugins on a node. CI boots Talos, where the value is off, and `isp-full-generic` appears in no e2e workflow. The mechanics are covered by the bats above, which run the real script from the real manifest and also under `dash`, the shell in the runtime image, but nothing demonstrates it end to end on a node. Defaulting it on for the generic bundle is a deliberate maintainer decision taken with that gap known, not a side effect of the diff. Shipping it off would leave the bug in place on the only platform where the feature does anything, and would not add coverage either, since the code stays unexecuted in CI both ways. What makes the risk acceptable: the init container skips rather than crashlooping when the image carries no plugins, each plugin is swapped in with an atomic rename instead of being written onto the live path, the tarball is checksummed before extraction, and declining is a value rather than disabling the package. And a plugin it cannot install is reported and skipped rather than failing the pod, so the worst case leaves the node exactly as it was before this change rather than unable to schedule. ### Screenshots N/A, no UI changes. ### Release note ```release-note feat(multus): install the reference CNI plugins (bridge, macvlan, ipvlan, ...) into /opt/cni/bin on generic-Linux installs, where nodes do not otherwise provide them. On by default on every platform: Talos ships five of the fourteen, and the four that overlap are rewritten with the same upstream version. On generic clusters this replaces same-named files in /opt/cni/bin every time the Multus pod is recreated, so a locally managed plugin there will not survive. Set networking.stageCniPlugins: false to decline. ``` BREAKING CHANGE: enabling stageCniPlugins replaces reference CNI plugins in /opt/cni/bin on existing nodes, Talos included -- four of the names it writes are ones the Talos node image also carries. Opt out before upgrading: declining afterwards does not restore what was replaced. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Multus can now stage reference CNI plugins into `/opt/cni/bin`, adding missing plugins and safely replacing matching versions. - Added the `stageCniPlugins` setting, with platform-specific defaults and explicit opt-out support. - Invalid configuration values now produce clear rendering errors. - Improved plugin installation diagnostics and failure reporting. - **Documentation** - Expanded guidance for CNI prerequisites, plugin staging behavior, troubleshooting, rollout impact, and configuration. - Clarified Multus image reference handling in platform documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
0c4c11c845 |
fix(kubernetes): retry the tenant CSI install instead of uninstalling it
Flux defaults a HelmRelease to the RemediateOnFailure strategy, whose install remediation is always an uninstall. A tenant CSI install that misses its timeout was therefore torn down between attempts, taking the CSI node plugin and the StorageClasses this release propagates with it, and the next attempt restarted the DaemonSet rollout from nothing instead of resuming one already under way. That teardown races the rollout it is meant to recover. In one observed run both DaemonSet pods were scheduled immediately and spent the whole budget pulling images; the slower node started its last container six seconds after the install timeout fired, and the uninstall removed it one second later. Retrying in place keeps that work. RetryOnFailure leaves the manifests applied and retries the failed release. Both actions carry it because the controller picks the active retry by .status.lastAttemptedReleaseAction and runs that retry as an upgrade: set on install alone, the retry happens but its own failure falls through to upgrade remediation, which rolls back by default with no previous release to roll back to. The cost is that a real upgrade failure now retries rather than rolls back, and that gives up no stable safety net: with retries: -1 and the default rollback strategy, upgrade remediation already rolled back and retried without end. The remediation blocks stay for a separate reason. An absent release consults install.remediation and an out-of-sync one consults upgrade, both by retry count rather than by strategy, so retries: -1 is what keeps those two branches acting. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5542ae52b7 |
fix(e2e): budget tenant scheduling separately from workload readiness
The tenant backend Deployment had a single 300s budget to reach condition=Available, and two unrelated variable costs shared it. The first is scheduling. What the suite establishes before that point is two tenant nodes Ready, which is weaker than schedulable: a Ready node still carries node.cilium.io/agent-not-ready until the tenant cilium agent claims it, and a node the bringup has not finished with is SchedulingDisabled. Scheduling took 2m18s and 1m57s in the two tenant suites of one run, leaving the image pull to finish inside what was left. It did not, and both suites failed with the same message for two different shortfalls. Wait for a node that actually accepts a toleration-free Pod before creating the workload, on its own budget and its own failure message. The gate encodes the scheduler's rule for such a Pod (Ready, not unschedulable, no NoSchedule or NoExecute taint) over a custom-columns probe, prints the node table on both outcomes so a timeout names the taint that held it, and treats a failed probe as not-schedulable so an API blip cannot release it. The readiness wait keeps its 300s, now starting from a schedulable node, and dumps deployment, pod and event state when it runs out. Pin the workload image by digest. The tenant workers reach no registry mirror, so nginx is pulled from Docker Hub on every run either way; the digest fixes what that pull returns instead of leaving a floating tag free to change size and content under a fixed deadline. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4d16f4fdb2 |
ci: re-run e2e in a confirmed green window
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
115d9bbaaf |
fix(installer): let the operator Deployment report Available honestly
At replicas: 1 with maxUnavailable: 1 the Deployment controller takes the minimum available replica count as 1 - 1 = 0, so the Available condition reports True with no running pod at all. That reading is what every consumer of the status sees: an alert on Available, a health dashboard, or a person reading kubectl get deploy all show green while the operator is dead. The replacement is not a tuned pair, it is the platform default written down. At replicas: 1 the default 25% / 25% resolves to maxSurge 1 and maxUnavailable 0, because the surge rounds up and the unavailability rounds down. So this change stops overriding the default rather than introducing a configuration of its own, and Kubernetes rejects maxUnavailable: 0 together with maxSurge: 0, so the surge could not have gone the other way regardless. The overridden pair was carried over from the earlier cozystack Deployment, where a darkhttpd sidecar published a host port and hostNetwork left a surge pod unschedulable on a single-node cluster. It arrived here in two steps: by the commit before this file was created the sidecar was already gone and no port was declared, yet the pair stayed, so it had outlived its own reason before it was ever copied. That is why looking for a justification in this file's history finds none. With no port declared anywhere in the pod there is no hostNetwork port conflict left, so the surge pod schedules beside the outgoing one; declaring one reintroduces exactly the scheduling failure the pair was added for. Not exercised on a live cluster in this rework. Stay on RollingUpdate rather than move to Recreate, which would fix the condition too, and by a shorter route: MaxUnavailable() reports 0 for any strategy that is not RollingUpdate, so the minimum available replica count becomes replicas - 0 = 1 and the condition is honest without any surge pod at all. The transition was not attempted, and the failure this project fixed in ouroboros 0.8.1 does not transfer to it. There the rollingUpdate block was the one the apiserver defaults onto a Deployment created without one, so it belonged to no applier and outlived the switch to Recreate. This Deployment has carried an explicit block since the commit that created it, and nothing applies it server-side, so whether Recreate would upgrade cleanly is an open question about the field-ownership history of the live object rather than a known rejection. What keeps RollingUpdate here is availability rather than apply semantics: maxUnavailable: 0 leaves the outgoing operator running when the replacement never starts, where Recreate deletes it first and leaves none. Editing the numbers inside the block is upgrade-safe from every state the field can already be in. The surge introduces new windows, and leader election covers less of them than it looks. The operator runs four privileged install steps before mgr.Start(), and the lease is only acquired inside mgr.Start(): the CRD install, the Flux install, the platform source resource and the platform PackageSource. All four therefore run in the incoming pod outside leader election entirely. What the lease does cover is the reconcile loops, the part that would otherwise conflict continuously instead of once at startup. That leaves three seams, and only the first two happen on a healthy upgrade. The routine one is the incoming pod's install steps against the outgoing pod's live reconcile loops, since the outgoing pod ran its own installs at its startup and has been inside mgr.Start() ever since. Those do not contend for the same fields: the installs write as field manager cozystack-operator, while the reconcilers write as cozystack-packagesource-controller and cozystack-package-controller and go mostly to status subresources. Different owners, different objects, so this seam is safe by separation rather than by any shared manager. The second is version skew rather than field contention, and maxSurge: 1 widens it rather than opening it. Under maxSurge: 0 the surge budget counts desired replicas rather than live pods, so the incoming pod is created while the outgoing one may still be Terminating - the overlap existed. What it did not have was a live counterparty: scale-down sends SIGTERM before the new pod is created, and that cancels the manager context, so the old loops were already unwinding. Here the outgoing pod is not signalled until the incoming one is available, so the overlap runs against a fully live reconciler for a termination grace period instead of a vestigial one. The incoming pod reports Ready the instant its container starts, because there is no readiness probe, so its CRD install runs while the outgoing binary's reconcile loops are still live and now writing against a schema the incoming pod has already moved. Old code against a new schema, which no field manager separates. It is bounded rather than benign: the outgoing pod exits on SIGTERM within seconds, a rejected write is transient and the next reconcile repairs it, and no object is left corrupt. Widening the skew, by adding a probe that delays Ready or by a migration the old code cannot round-trip, is what would make this seam matter. The third seam is install against install, and it needs the outgoing pod to be restarting, since a crashlooping pod re-runs its four steps on every restart, which is exactly the broken operator this gate exists to catch. Three of the four steps use server-side apply under the same field manager, so they never conflict and resolve last-writer-wins whenever the two pods carry different manifests. That ordering matters in one direction, and it is accepted here rather than guarded against: the CRD install and the Flux install both apply with Force: true, so an outgoing pod that writes last silently overwrites what the incoming pod has just applied, with no conflict and no error to exit on. The incoming pod is inside mgr.Start() by then and will not apply again until it restarts, so that revert of the upgrade's install phase persists rather than healing itself. Reaching it needs a restart landing in the seconds between the incoming pod's installs and the outgoing pod's SIGTERM. installPlatformSourceResource is the remaining exception, a read-modify-write of a Get followed by Create or by Update with the observed resourceVersion, so two pods running it at once can collide on AlreadyExists or on a conflict; that failure is loud and self-healing, since any error there exits the process and the pod restarts, and the object is not left corrupt. Pin the strategy and all four preconditions the surge makes load-bearing in a unit test: no container or initContainer declares a port, both listener arguments stay disabled, leader election stays on, and no probe uses httpGet. They fail differently and none is visible to the others, since a declared port stops the scheduler, a bound one only fails once the incoming process starts, and a second manager without a lease fails nothing at all until it writes. The httpGet precondition follows from the two disabled listeners leaving no port to target, and it is the worst of the four to get wrong: the other three wedge an upgrade, while a livenessProbe against a disabled listener crashloops the pod on every cluster, including a fresh install where no surge happens. Only the httpGet field is forbidden rather than the probes themselves, because an exec probe stays legitimate. All four are checked against all three variants, because the template branches on variant inside the container spec and a port added under one branch renders for that variant alone. The strategy type is pinned alongside its two values, since maxSurge and maxUnavailable say nothing on their own once the type is no longer RollingUpdate. The listener preconditions are pinned less tightly than the other two, and the test says so rather than implying coverage it does not have. A membership assert has no notion of position while Go's flag package keeps the last occurrence, so a duplicate after the disabled one wins and the assert still passes, while a duplicate before it loses and the assert passes just the same - the test cannot tell those two apart. Moving the disabled flag last fixes the process without making the assert catch anything. Rejecting the two addresses this binary defaults to closes the shape a values-gated block to enable metrics would take, and nothing available here closes an arbitrary address, since helm-unittest cannot express that a flag appears at most once. Note in hack/helm-unit-tests.sh that the loop covers packages/core, which its comment omitted. The suite added here lives there, so a later edit reconciling the loop with the stale comment would have silently stopped running it. Split the rationale by audience rather than keeping it all in shipped comments. What an operator applying the rendered manifest has to respect stays in plain comments and travels into the install artifacts: that the pair is the default, why maxUnavailable must stay 0, that no port may be declared or bound, that no probe may use httpGet, and that leader election has to stay on. Most of what stays in the template comment is what the chart's own suite pins, so it cannot drift out of step with the chart without a test going red: the four preconditions and which assert covers each, and why the variant loop is needed. Three things there are deliberate exceptions, kept because anyone editing the strategy has to know them and no assert can carry them - that the webhook server configured on 9443 is not a port binder, since controller-runtime adds it to the runnables only inside GetWebhookServer() and this binary never calls it; that two overlaps between the pods sit outside the lease; and why RollingUpdate is kept rather than Recreate. Those name what exists. The seam analysis above, which is how each one works, deliberately stays here instead. It describes another file's internals, nothing pins it, and a commit message is fixed in time, so it cannot quietly fall out of date the way a comment can. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e80b8e370f |
fix(e2e): collect logs for unready pods and record truncated captures
The failure snapshot selected pods by their STATUS column, so a pod whose container runs while its readiness probe never passes -- STATUS Running, READY 0/1 -- was dropped from the report entirely. That is a common way for a suite to time out: the replica never joins, its owning HelmRelease sits in InProgress, and the artifact carries no trace of the pod the run died on. Select on the READY column instead, which also covers the partially ready multi-container case, and keep every phase that was collected before. On one degraded cluster that is four of the seven broken pods the old filter reported nothing about. The VMI and VM sections had the same blind spot and now share one selector that reads READY as the last field, because a Pending object prints fewer columns than a Running one and the two kinds differ only in how many columns precede READY. READY counts ready containers rather than the pod's Ready condition, so a pod held back by an unsatisfied readiness gate is out of scope; a test pins that rather than leaving it to be found in an empty directory. Logs are requested for every container of a selected pod rather than skipped for Pending ones, and the per-pod collection moved into a helper so it can be driven against a stub kubectl. kubectl's refusal is the finding, and an empty result is ambiguous on its own: a read against an unscheduled pod exits 0 having printed nothing, which is what a container that started and stayed quiet also leaves. Every site that classifies a failed read now orders its branches the same way, naming the timeout first where both a cutoff and an uncapturable stderr apply. The CRD gate in the summary had no such branch at all: it returned in silence, which is byte-identical to the CRD being absent, so every section behind that gate vanished from a report whose only real problem was that the machine writing it had nowhere to put a temp file. Collection used to stop silently. A walk that hit its cap said so in the job log and nowhere in the artifact. Now the pod walk leaves COLLECTION-TRUNCATED.txt when it stops at COZY_REPORT_PODS_MAX or COZY_REPORT_PODS_BUDGET, CONTAINER-LOGS-TRUNCATED.txt on a pod cut off part way through, COLLECTION-FAILED.txt when the pod list never came back rather than the empty tree a healthy cluster produces, and COLLECTION-UNBOUNDED.txt when timeout is missing locally, which is a fact about the machine writing the report and not about the cluster. Tenant namespaces are collected first, so a cap that only bites on a broadly degraded cluster is not spent on platform pods while the pod the suite died on falls off the end. Container logs are read with --tail=2000 where they were previously read whole, and every bounded file states the bound it was written under, because --tail drops the oldest lines and nothing else in the output would say so. COZY_REPORT_POD_TAIL=-1 restores the complete log. The bounded whole-object read no longer merges stderr into pod.yaml, vm.yaml and vmi.yaml, which are read by yq rather than by eye: a warning on an otherwise complete read left an unparseable file. stderr goes to a scratch file and is copied in only when kubectl returned no object, and the truncation markers those files carry are written as YAML comments, since a bare marker line after a complete mapping is a second top-level node. Six latent faults surfaced while making these reads report. The kamaji module was gated on the LINSTOR controller, so a cluster without LINSTOR got no kamaji evidence at all, which is what a tenant-kubernetes bootstrap timeout needs most. The Flux sources section nested one jsonpath filter inside another, which kubectl rejects outright, so it printed nothing on every report ever taken and read as "all sources Ready". The HelmRelease listing took a single field as STATUS where STATUS is the Ready condition's whole message, so every failed release rendered as one word. The services section filtered for a pending LoadBalancer address on the CLUSTER-IP column, which never carries that value. The cozystack module read its image from a Deployment the installer stopped shipping when it moved to the operator, putting kubectl's NotFound in the first file of the report. And the cozystack apps section gated all three of its kinds on a CRD in an aggregated API group, so it never ran once; two of those names were wrong, and both are collected again. The tests avoid EXIT-trap cleanup, and e2e-testing.md says why: a bats test that installs one and then fails prints no TAP line at all, so output grepped for `not ok` reads as green, while hack/cozytest.sh reports the same failure correctly. A guard enforces that over a named list of files rather than a glob with exceptions, because a list of exceptions is the same claim written so that forgetting to shorten it makes it false. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3fc8e524b2 |
fix(e2e): select catch pod logs by the nested release instance label
The harbor, openbao and qdrant app charts deploy their workload through a nested HelmRelease named `<release>-system`, and pass `fullnameOverride: <release>` down to it. The inner chart therefore stamps app.kubernetes.io/instance from its own release name, with the suffix, while object names come from the override, without it. The catch blocks selected on the outer release name, so podLogs matched no pods and every failed run collected an empty result. A pod name says nothing about its instance label; the two come from different sources on purpose. In qdrant the unsuffixed value is therefore not retired but kept as a collector of its own: the parent chart's PVC-cleanup Job labels its pod template with the parent release name, and that suite deletes the parent inside a `try:`, so the Job is in scope while the test still runs. The comment there is explicit about how narrow that window is -- the hook always exits 0, so Helm calls it succeeded and `hook-succeeded` takes the pod away; what is left is an uninstall that hangs with Helm still waiting on it. Harbor renders the same hook and does not get the same collector. It has no `delete` op, so its parent release is uninstalled only in Chainsaw's cleanup phase, which no catch can observe: catch is a defer inside the step, while the cleaner runs from t.Cleanup, after the test function returns. That collector would be the same permanently-empty capture this commit removes. A comment records the asymmetry so the next reader does not restore it for symmetry. openbao renders no such hook at all. Each workload value is now also pinned on a resource in an assert the suite already runs, so the chart renaming a label fails a step instead of quietly emptying a capture. The comments state what that does and does not cover: it does not catch a wrong value typed into a selector, and it catches nothing when the run fails before the assert. Harbor's database was reached by no collector at all. It is selected on `cnpg.io/cluster`, CNPG's own primary label, rather than on the instance label CNPG merely derives from the Cluster name -- the primary label has nothing to drift from, and it is what the postgres suite and the WorkloadMonitors already use. Each lookup keeps stderr apart from the value it captures. kubectl exits 0 while logging a klog line about an API group it could not reach, and an aggregated APIService in that state is what a failed run usually looks like; folded into the value, that line makes the "nothing found" branch unreachable and prints as though it were a result. Telling those outcomes apart is one helper in hack/e2e-chainsaw/_lib/ rather than a copy per call site, separated from the kubectl call so it can be pinned without a cluster, the way etcd-probe.sh already is. Both the census and the qdrant cleanup listing report through it, differing only in wording, so the discrimination is implemented and tested once. The tests cover each outcome for both wordings, including the zero exit with a klog line on stderr -- the case a folded stream turned into a row. A collector that matches nothing cannot say so: podLogs is silent, and `kubectl get -l` exits zero on an empty match. Each test-wide catch prints the instance label of every pod in the namespace, and the qdrant cleanup catch reports its three outcomes apart from a single call -- printing what that call returned rather than running it again, which could put a healthy listing under a failure heading. That census runs on a degraded cluster, where kubectl otherwise retries discovery against an unreachable apiserver until something kills the script, so every lookup carries `--request-timeout`. Measured against an unreachable apiserver, a bounded call costs about five times the flag, since kubectl retries discovery five times before it gives up. Each script then carries an explicit op timeout sized above its own calls -- 2m for the single-call censuses, 5m for the qdrant cleanup listing, which makes three. An inner budget is what should fire: a SIGKILL at the op boundary yields nothing, where a kubectl that gives up leaves an error to print. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ff867b6a4f |
ci: re-run e2e after install-phase ClusterIP unreachability in the sandbox
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e9e56097bc |
ci: re-run e2e after infra-level apiserver unreachability in the sandbox
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d3ae21b7f0 |
fix(e2e): name every way the previous-instance capture falls short
The capture wrote its reasons to the job log only. A reader holding the uploaded artifact saw N logs in a directory with no way to tell N from all of them: the container cap that dropped the rest, a pod list that never returned, a previous instance that produced nothing, and a read the outer backstop cut short all looked alike from outside. Every one of those lines now goes to capture-notes.txt as well, and the directory is created before the first of them, so a capture that found nothing is distinguishable from one that never ran. A pod list that failed is no longer reported as nothing having restarted. That is a claim about the cluster which the script never observed, and it is the claim most likely to talk a triager out of the crash-loop hypothesis on exactly the run where it is right. Exit 137 no longer asserts whose signal ended the read. 124 comes from timeout and from nothing else, but 137 is 128+SIGKILL, which the kill grace produces and so does the OOM killer or a teardown signalling the process group. Both belong in the cut-off branch, since the capture ends early either way, but reported flatly as the script's own timeout a read killed at second two reads as one that waited the full eighteen. COZY_PREVLOG_TAIL rejects a value kubectl cannot use and says so. A zero tail asks for no lines while kubectl still exits 0, which the empty-output branch would otherwise report as the container having logged nothing, discarding the only remaining copy of a log the kubelet has already collected. A leading zero is rejected because `$(( ))` reads it as octal while `[` reads it as decimal, so one string means two numbers. Each kept log names the bound it was written under, since --tail drops the oldest lines and nothing else in the output says so, and -1 suppresses that note because no bound was applied. An output directory that cannot be written is reported as the local condition it is. It arrives as an argument, so it can be a file, or missing, or unwritable; the redirection then fails before kubectl runs and hands the shell's own status to the branch that names kubectl, which reported "no previous instance retrieved (kubectl exit 2)" once per container. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ceccac27b0 |
test(hack): clean up bats temp dirs without EXIT traps
A `trap ... EXIT` inside an `@test` replaces the one the bats binary installs for its own bookkeeping, so a test that then fails prints no TAP line at all. Not `not ok`, nothing: the run ends with a warning that N of M tests executed, and anyone judging it by grepping for `not ok` reads a green suite. Cleanup moves to the last line of each body instead. Both runners set -e, so a failed test leaves its scratch directory behind, which is what you want to look at anyway. hack/promote-retag_test.bats was failing this way already, reporting five passes and no failures while six of its eleven tests printed nothing. Those six invoke the script for real, and it requires sha256sum; they pin PATH to the stub directory plus /usr/bin and /bin, which is where coreutils installs that binary and where macOS does not, so the failure was of the platform rather than of the code and CI never saw it. The stub directory already supplied yq and skopeo for the same reason and now supplies sha256sum too, resolved from absolute candidates because the stub itself is first on PATH. Two file headers described the trap arrangement they no longer use and are corrected rather than left to outlive it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrei Kvapil
|
df157da618 |
ci(workflows): open the E2E breakpoint for maintainer-authored PRs (#3158)
## What this PR does Decouples opening the E2E SSH breakpoint from the `debug` label. Today, the only way to open a breakpoint on a failed feature PR is to add the `debug` label. That same label also flips every job's `runs-on` onto self-hosted runners, so you cannot get a breakpoint on the normal ephemeral runner — debugging always drags the whole pipeline onto self-hosted infrastructure. This adds a maintainer-authorship path to the breakpoint's `if` condition: a failed PR whose author is an `OWNER`, `MEMBER`, or `COLLABORATOR` now opens the breakpoint automatically, with no label required and without moving the jobs onto self-hosted runners. The breakpoint simply holds the default `oracle-vm` ephemeral runner the job already runs on. The existing `debug` and `release` label paths are unchanged, and the `authorized-users` list still controls who may actually SSH into an open breakpoint (second defense layer). Workflow triggers are untouched — this is purely an `if`-condition change. ### Release note ```release-note ci(workflows): open the E2E SSH breakpoint automatically on failed maintainer-authored PRs, without requiring the `debug` label or moving the jobs onto self-hosted runners ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated E2E failure debugging so breakpoints can now open for pull requests authored by repository maintainers, not just when specific labels are applied. * Clarified the conditions and runner behavior for breakpoint access in the workflow documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
0612a538a3 |
fix(e2e): bound the Talos-capture failure-branch forensic kubectl calls
The describe/get-events/get-certificaterequests calls that run when the reader Certificate or the diagnostics Pod misses its readiness wait were unbounded, yet those branches fire precisely when the management apiserver is wedged. A stalled forensic call there can burn minutes of the 50m step budget before the pre-existing (a2)/(c) diagnostics and the EXIT-trap tenant snapshot run. Add --request-timeout=30s to them, matching the happy-path capture calls. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
4ca64a99f4 |
ci: re-trigger checks after label events cancelled the real run
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
cfbfd770b8 |
fix(e2e): bound the tenant Talos capture kubectl calls
The in-guest diagnostics chain runs only when the sandbox is already degraded enough to miss the 18m node-join deadline (the documented management-etcd fsync-storm regime), yet its kubectl legs were unbounded while every talosctl leg is wrapped in timeout. A stalled apiserver call in the (b) capture could consume the remaining step budget before (a2), (c) and the EXIT-trap tenant crust-gather run, so a diagnostics feature could reduce total diagnostic yield under the exact conditions it targets. Add --request-timeout=30s to the raw Certificate/Pod apply, the three reader-Secret reads and the worker VMI list, and route the Certificate Ready wait through kubectl_wait_retry so a single transient etcd-leader flap retries instead of aborting the whole capture. Update the bats assertion for the reworded wait call. Also correct a stale comment: the chainsaw step timeout is 50m, not 40m. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
5e3c286e20 |
fix(e2e): read files with cat in the Talos diagnostics bats helper
assert_file_contains used $(<file), a bash-only construct. cozytest.sh is run under /bin/sh (dash on the Ubuntu CI runner via Makefile), where $(<file) expands to an empty string instead of the file contents, so every assertion after it failed and reddened the unit-tests job, which in turn skips the e2e gate and leaves the diagnostics path unexercised. Read the file with $(cat ...), which is POSIX and behaves identically under bash and dash. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
cd57ee6678 |
test(multus): anchor the master-CNI assertion to the top-level key
The check that multus is pointed at the cilium conflist matched its JSON fragment anywhere in the rendered daemon config. The same text nested inside any other key satisfies it, so a config whose effective multusMasterCNI names a different conflist passes. Multus uses that value as written when it is non-empty -- it delegates to whatever is named, so a wrong-but-existing name silently routes through the other CNI instead of cilium, which is the state this assertion exists to prevent. Auto-discovery happens only when the value is empty, and a name that does not exist fails manager startup rather than falling back. Anchor it to the top level, where the template writes its keys. That pins the position rather than parsing the document; deciding which key is effective needs a JSON parser, which helm-unittest does not have. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7739d1e294 |
feat(multus)!: stage reference CNI plugins from the multus image
Multus-backed NetworkAttachmentDefinitions delegate to the upstream containernetworking reference plugins (bridge, macvlan, ipvlan, ...), and nothing guarantees a node has them in the directory Multus searches. On k3s they live under /var/lib/rancher/k3s/data/cni while Multus and Cilium install into /opt/cni/bin, so a bridge-type NAD fails there with `failed to find plugin "bridge" in path [/opt/cni/bin]`. Bundle 14 of the release's plugins into the multus image and add an install-cni-plugins init container that installs them into the host /opt/cni/bin, mirroring the existing install-multus-binary container. loopback, dhcp, tap and dummy are left out; the Dockerfile records why. Gate it on a new chart value, stageCniPlugins, resolved per bundle and on for both. Talos bundles only bridge, firewall, flannel, host-local, loopback and portmap, so the other ten are missing there exactly as on generic Linux; the four that overlap are rewritten with the same upstream version, v1.9.1 on both sides, rather than an older one. A marker in the image Dockerfile records the Talos release that pin was compared against, and a test holds it to the release the platform ships. multus therefore moves out of system.common-packages into the two variant branches, the way linstor already does for talos.enabled. networking.stageCniPlugins overrides the variant default in either direction. Where staging is on the install is unconditional: a plugin is replaced on every pod recreation, not only when the destination is missing. A destination that is already a directory is reported and left alone, since replacing it is not something this can do safely. Turning the value off afterwards does not undo it; nothing here removes or restores a plugin. The container is not privileged and takes no mount propagation. It runs in the spc_t SELinux domain, without which an enforcing node refuses the write. Each plugin is installed under a temporary name and swapped in with a rename: /opt/cni/bin is shared with cilium and kube-ovn. The plugin tarball is checksummed before extraction. A plugin that cannot be installed is reported on stderr and to /dev/termination-log, and the container still exits 0: Multus stays the node's primary CNI and nothing removes its config, so an init container that never completes would leave the shim with no daemon behind it. The manifest change lives in customize-deployment.patch so it survives make update. BREAKING CHANGE: enabling stageCniPlugins replaces reference CNI plugins in /opt/cni/bin on existing nodes, Talos included -- four of the names it writes are ones the Talos node image also carries. Opt out before upgrading: declining afterwards does not restore what was replaced. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
517dd11d1a |
fix(tests): wait for operator rollout instead of condition=Available
condition=Available answers whether the Deployment sits within the slack its rollout strategy allows, not whether the operator came up. That is the wrong question for an install gate, and it stays wrong whatever the strategy is: any maxUnavailable equal to replicas drives the minimum available replica count to 0 and the condition then reports True with no running pod. Measured against an operator stuck in ImagePullBackOff, the old check passed in 156 ms, and the install went on to fail two minutes later on the CRD wait, which pointed the investigation at the wrong component. rollout status reads updated and available replicas directly and only succeeds once the updated pod is available, which holds under any rollout strategy. Dump the deployment, pods, operator logs and events on failure so the cause is legible where it is detected. Drop --wait from the helm step. Helm 3 decides a Deployment is ready with the same replicas - maxUnavailable arithmetic, so on the current strategy it is satisfied at zero ready pods and gates nothing; were the strategy tightened it would instead fail the install on its own 2m budget before this gate runs, hiding the diagnostics behind a bare Helm timeout. That arithmetic is specific to Helm 3, since Helm 4 reworked --wait into a wait strategy with a different default, so treat the reason as version-bound. The conclusion is not: one gate that reports what it saw is better than two that can disagree about whose budget expired first, whichever major is installed. Hooks are waited for with or without the flag. The 5m budget here covers scheduling and a cold pull of an image that is not in the prepull set. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
80a2090444 |
fix(kubernetes): retry the tenant CNI install instead of uninstalling it
The tenant cilium HelmRelease carried no install strategy, so Flux applied its default, RemediateOnFailure, which uninstalls the release between attempts. Cilium is the tenant cluster's only CNI, so every cycle takes it away and gives it back when the next install applies the manifests again, and retries: -1 lets that repeat without end. What sets a cycle off is an install that outruns its budget, which can happen while the tenant still has no registered node, since the operator Deployment cannot become Available until one exists. Remediation neither causes that delay nor is fixed by this change; what it adds is a repeated teardown of the CNI on top of a delay that resolves itself once the node arrives. This removes the amplifier, not the cause. RetryOnFailure keeps the applied manifests in place and retries a failed install as an upgrade, so the rollout completes on its own once a worker registers. Both actions carry the strategy because the retry of a failed install runs as an upgrade, which would otherwise fall back to upgrade remediation. It is the strategy and retry interval cozystack-api and cozystack-operator already set on every HelmRelease they generate, except that they leave remediation nil where this release keeps it. The other tenant addons the chart renders still carry the remediation-only shape; this one is where the teardown removes the cluster's only CNI rather than churning a single component. The cost is that a genuine upgrade failure no longer rolls back either, it retries on the failed manifests instead. That is forced, since the controller cannot tell a retried install from a real upgrade, and what it replaces was an unbounded rollback-and-retry flap rather than a stable safety net. The remediation blocks stay, since they cover the branches no strategy reaches: an absent release, which consults the install block, and an out-of-sync one, which consults upgrade. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
60e44b24f1 |
fix(e2e): make tenant Talos diagnostics reliable
Fix admission, capture setup, timeout headroom, and portability. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
c8ec60cfe3 |
test(e2e): remove optional search tool dependencies
Use Bash and awk assertions in the Talos diagnostics regression tests so they run in the standard E2E sandbox without optional search utilities. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
80152b1a07 |
fix(e2e): capture Talos logs for tenant worker join failures
Tenant Kubernetes worker join failures currently stop at host-side VMI and CSR state because the worker-only topology does not materialize a tenant talosconfig. Mint a short-lived os:reader client certificate and run talosctl from a hardened same-namespace Pod after the Ready deadline. Preserve bounded dmesg and kubelet captures for every worker without masking the original test failure. Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
ee26a50f0e |
fix(postgres-operator): align CNPG operator and CRDs to 1.28.2 for PVC resize-deadlock fix (#3510)
## What this PR does Raises the CloudNativePG operator image **and its CRDs together** to 1.28.2, which carries the PVC resize-deadlock fix (cloudnative-pg#9980 / cloudnative-pg#9981). Symptom: after a simultaneous `resources` + `size` change on a single-instance Postgres, the operator deletes the sole primary Pod, classifies the PVC as `resizing`, and never recreates the Pod — the cluster wedges with zero instances and the disk FS resize never completes. Upstream publishes a CloudNativePG chart only per minor (`.0`/`.1`), so there is no 1.28.2 chart on the 1.28 line. Following the pattern of #3526 / #3528, `patches/cloudnative-pg-1.28.2.patch` (applied by `make update`) raises the vendored chart's `appVersion` and CRDs from 1.28.1 to 1.28.2 in lockstep. The operator image follows `appVersion`, so there is no `image.tag` pin (keeping operator and CRDs aligned and avoiding #3479). Verified #3479-safe: there are no CRD `status` field changes between 1.28.1 and 1.28.2 — only the `extensions` spec grows. Backportable to release-1.6 as-is (same 0.27.1 base chart). ### Release note ```release-note fix(postgres-operator): align CloudNativePG operator and CRDs to 1.28.2, fixing the PVC resize-deadlock that wedged single-instance PostgreSQL clusters after a simultaneous resources+size change ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for PostgreSQL extension configuration in catalog and cluster schemas. - Made extension image settings optional where appropriate. - **Bug Fixes** - Updated CloudNativePG to version 1.28.2, including the PVC resize deadlock fix. - Kept the operator image and CRDs aligned at the same version. - **Documentation** - Clarified versioning and chart update guidance. - **Tests** - Added checks confirming operator image and CRD version alignment. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
bcb36262ef |
fix(postgres-operator): align CNPG operator and CRDs to 1.28.2 for PVC resize-deadlock fix
Raises the CloudNativePG operator image and its CRDs together to 1.28.2, which carries the PVC resize-deadlock fix (cloudnative-pg#9980 / #9981): after a simultaneous resources+size change the operator deletes the sole primary Pod, leaves the PVC in the resizing class, and never recreates the Pod, wedging the cluster. Upstream publishes a chart only per minor .0/.1, so there is no 1.28.2 chart on the 1.28 line. Following #3526/#3528, patches/cloudnative-pg-1.28.2.patch (applied by make update) raises the vendored chart's appVersion and CRDs from 1.28.1 to 1.28.2 in lockstep; the operator image follows appVersion, so no image.tag pin. Verified #3479-safe: no CRD status field changes between 1.28.1 and 1.28.2 (only the extensions spec grows). Backportable to release-1.6. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
myasnikovdaniil
|
7cef85f988 |
feat(release): cutting an rc freezes the line into release-X.Y (#3399)
## What this changes Cutting the first `vX.Y.0-rc.N` now creates `release-X.Y` at the tagged commit and closes the line. Every later cut for that line must be dispatched from the branch; a dispatch from `main` is refused. Before this, `release-X.Y` was created only when the stable promote PR merged. So `rc.2` and `rc.3` were cut from `main`'s tip and silently absorbed every feature merged since `rc.1` — the shipped release could contain code that no rc had ever validated. `main` now reopens for the next minor immediately, and fixes reach the release by cherry-pick. ## Commits | Commit | What | | --- | --- | | `feat(release)` | `cut-prerelease.yaml`: create `release-X.Y` after the tag push; refuse a frozen line from `main` | | `ci(backport)` | `backport.yaml`: target the newest existing `release-X.Y` branch | | `docs(release)` | Rewrite the RC/Regular/Patch sections, diagrams, and workflow reference | ## Behaviour changes worth a close look **The promote PR base flips from `main` to `release-X.Y` on its own.** No code change causes this — `promote-rc.yaml` already prefers `release-X.Y` whenever it exists, and after the freeze it always does. Consequence: the digest-vendored `Prepare release vX.Y.0` commit stays on the release line and no longer lands on `main`. This matches how patch releases already work, but it is a real change for `.0` releases and is easy to miss in review. **`finalize`'s `Ensure maintenance branch` becomes a no-op.** The merge commit is already the branch tip, so the fast-forward has nothing to do. No change was needed there. **Backport targeting changes during a freeze window.** `backport.yaml` derived its target from `getLatestRelease`, which returns the newest published *stable*. Since `release-X.Y` now exists before `vX.Y.0` ships, that name lagged by a full line for the whole freeze window — a `backport` on a fix for the release being stabilised would have landed on the previous line and missed the release it was written for. It now enumerates real `release-X.Y` branches and takes the newest. Outside a freeze window the result is unchanged. This also drops a latent bug: `previous` was computed as `min - 1`, which names a branch that does not exist whenever a minor is skipped. Sorting is now numeric, so `release-1.10` correctly outranks `release-1.9`. ## A note on image pins An earlier revision of this PR also replaced the committed `ghcr.io/cozystack/cozystack/*` digests on `main` with an unpullable sentinel tag, on the theory that they are a build leftover that nothing restores — and that the freeze makes this worse, since the promote PR no longer merges into `main` to refresh them. That was wrong and has been dropped. Per #3143, PR CI rebuilds only the packages a diff touches (`hack/build-matrix.sh` "emits only the units whose package dir changed") and the installer bundles the whole digest-patched tree, so **every untouched package deploys from its committed pin**. A sentinel would have made those unpullable across E2E. The staleness problem is real, though, and #3143 already tracks it as a systemic gap. Evidence gathered while investigating is posted there rather than acted on here. ## Testing - Full `hack/helm-unit-tests.sh` green - Branch-selection logic tested against the live 424-branch list, including double-digit minors (`1.10 > 1.9`) and skipped minors; outside a freeze window the result is unchanged (`release-1.5` / `release-1.4`) - `zizmor` clean on both workflows - No `values.yaml` or `images/*.tag` file is touched by this PR ## Release note ```release-note Cutting a release candidate now creates the `release-X.Y` branch and freezes the release line. Features merged to `main` after the first rc no longer ship in that release; fixes must be cherry-picked or backported onto `release-X.Y`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Release branches are now created and frozen when the first release candidate is published. * Later pre-releases and patches use the frozen release branch while `main` advances independently. * Release-line package artifacts can be generated manually, including for recovery. * Backport targets are selected from existing release branches, with clearer failures when a prior line is unavailable. * **Documentation** * Updated release, patch-release, workflow, promotion, finalization, and cleanup guidance to reflect the new freeze model. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
a7149d11a3 |
chore(etcd-operator): bump etcd-operator to v0.5.4 (#3529)
## What this PR does Bumps the cozystack etcd-operator packages from **v0.5.3 to v0.5.4**. `v0.5.4` is a controller bug-fix release — no API, RBAC or values changes: - fix(controllers): derive `--initial-cluster-state` from phase, not from the seed ([cozystack/etcd-operator#355](https://github.com/cozystack/etcd-operator/pull/355)) - fix(controllers): stop exempting the bootstrap seed from self-heal ([cozystack/etcd-operator#354](https://github.com/cozystack/etcd-operator/pull/354)) - fix(controllers): extend crash-loop self-heal to memory members ([cozystack/etcd-operator#352](https://github.com/cozystack/etcd-operator/pull/352)) - fix(controllers): switch the PDB from `maxUnavailable` to `minAvailable` ([cozystack/etcd-operator#351](https://github.com/cozystack/etcd-operator/pull/351)) Changes in this repo: - `packages/system/etcd-operator/Chart.yaml` — `appVersion: v0.5.3 → v0.5.4`. The manager image tag defaults to `.Chart.AppVersion` (`values.yaml` keeps `tag: ""`), so this reimages the controller to `ghcr.io/cozystack/etcd-operator:v0.5.4`. - `packages/system/etcd-operator/Makefile`, `packages/system/etcd-operator-crds/Makefile` — `ETCD_OPERATOR_REF: v0.5.3 → v0.5.4`. - `packages/system/etcd-operator-crds/templates/etcdmembers.yaml` — re-vendored at v0.5.4 via `make update`. Description-only change to the `/scale` `replicas`/`selector` field docs, tracking the PDB `minAvailable` fix. `etcdclusters` and `etcdsnapshots` are byte-identical to v0.5.3. - `templates/rbac.yaml` intentionally left as-is: `manager-role-rules.yaml` is byte-identical between v0.5.3 and v0.5.4. **Upgrade path (PDB switch, cozystack/etcd-operator#351).** This is the one change that rewrites live objects: the operator moves each EtcdCluster's PodDisruptionBudget from `maxUnavailable` to `minAvailable`. Setting both fields is invalid, but upstream `reconcilePDB` handles the migration — it treats a surviving pre-migration `maxUnavailable` as divergence and explicitly clears it (`MaxUnavailable = nil`) before writing `MinAvailable`, so existing clusters are reconciled cleanly on upgrade rather than wedging their PDB. Verified locally: `helm lint` and `helm template` pass for both packages; rendered manager image resolves to `ghcr.io/cozystack/etcd-operator:v0.5.4`. These `packages/system/*` packages have no `generate:` target and no `values.schema.json`, so there is nothing for `make generate` to regenerate. ### Screenshots Not applicable — no UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` file-by-file against the diff: - The diff touches only `packages/system/etcd-operator*` — no `packages/apps/**` or `packages/extra/**` add/rename/remove, no `packages/core/platform` or `installer` values, no Talos bump, no asset-name or dev-tooling change → **website / ansible-cozystack** not reached. - The CRD edit is description-only, inside etcd-operator's own `etcd-operator.cozystack.io` CRDs — not the provider's hand-typed `Package`/`Plan`/`RestoreJob` types, and no new managed app → **terraform-provider-cozystack** not reached. - No `hack/` change, no `packages/system/<name>-rd/cozyrds/**`, no `ApplicationDefinition` CRD / `chartRef.kind` enum change, no `cozyhr`/`package.mk` contract change, no telemetry-metric or proxy-label rename → **ccp / talm / cozyhr / cozy-proxy / telemetry-server / examples / external-apps-example** not reached. - [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(etcd-operator): bump etcd-operator to v0.5.4 (controller bug-fixes: PDB switched to minAvailable, crash-loop self-heal extended to memory members, --initial-cluster-state derived from phase, seed no longer exempt from self-heal) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Updates** - Updated the etcd operator to version v0.5.4. - Clarified scale-related resource descriptions, including replica counts, selectors, and disruption budget behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
2f8c953cf5 |
fix(kubernetes): render the talos-reconcile Job for the default md0 group (#3535)
## What this PR does Fixes #3504. The chart has two loops over worker node groups and they read different sources. `templates/cluster.yaml` iterates the effective set through the `kubernetes.nodeGroups` helper, whose else-branch supplies the built-in `md0` group when the user declares none — that is how #2936 made `md0` removable without a Helm values merge re-adding it. `templates/talos/talos-reconcile-job.yaml` iterated the raw `.Values.nodeGroups` map instead, which is empty in exactly that case. The consequence is a dangling reference. The chart cannot render the `TalosConfigTemplate` itself — it needs the apiserver Service ClusterIP, the Talos CA and the Kubernetes CA, none of which exist while Helm is executing the template — so `cluster.yaml` deliberately stopped rendering it and the Job became its only producer, while the `MachineDeployment` still names it in `spec.template.spec.bootstrap.configRef`. On a cluster with the default `nodeGroups`, `MachineDeployment/<release>-md0` renders and zero Jobs render, so `TalosConfigTemplate/<release>-md0` is never created and CAPI blocks every Machine that ever joins the group. The same Job also patches `KamajiControlPlane.spec.network.certSANs` with the live Service ClusterIP, so that is skipped too. Nothing fails at install time, because the built-in group carries `minReplicas: 0`. The failure waits for the first scale-up, which is the documented path rather than a corner: `values.yaml` tells operators that enabling the ingress-nginx addon on a cluster with default `nodeGroups` means waiting for the cluster-autoscaler to bring `md0` up in response to the controller Pods becoming Pending, and the autoscaler is deployed unconditionally whenever the tenant has an etcd DataStore. The fix is to range over the helper so the Job set tracks the MachineDeployment set exactly. A cluster that declares its own groups renders byte-identically — which is why the pinned content-hash fixtures in `tests/talos_templates_test.yaml` are untouched — and `md0` stays removable, because the fix defers to the helper rather than merging `md0` in unconditionally. On upgrade it is additive: the Job appears where there was none, applies the `TalosConfigTemplate` the MachineDeployment already expects, and the content-hash name suffix means Helm creates a fresh Job rather than attempting to patch an immutable one. ### How this survived Every helm-unittest fixture and every e2e suite declares `md0` explicitly. `tests/nodegroups_default_test.yaml` does render the empty-`nodeGroups` case, but lists only `templates/cluster.yaml`, so it never looked at a Job. The two OIDC chainsaw lanes do install with an empty map, but assert that the HelmRelease exists rather than that it becomes Ready, so a worker that never boots is invisible to them. Offline `helm template` cannot reach this path at all: `cluster.yaml`'s instanceType validator needs a live `lookup` for `u1.medium`. `tests/talos_reconcile_nodegroups_test.yaml` closes the gap and pins both halves of the helper contract — an empty map yields exactly one `md0` Job carrying `GROUP_NAME=md0`, and a two-group map yields exactly two Jobs with no `md0` among them, so a future fix that merges `md0` in unconditionally fails here. Reverting the template change fails the first case on the document count, so it guards the behaviour rather than restating it. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff: it is one Helm template loop and one new helm-unittest suite inside `packages/apps/kubernetes`. No package is added, renamed or removed; no `values.yaml`, `values.schema.json`, `ApplicationDefinition`, version enum or default changes; nothing under `hack/`, no namespace, variant, label, annotation or metric renamed. The behaviour now matches what `values.yaml` already documents, so no reference page drifts. - [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): a tenant Kubernetes cluster created without an explicit `nodeGroups` map now gets a talos-reconcile Job for the built-in `md0` node group. Previously that Job rendered only for explicitly declared groups, so the default `md0` MachineDeployment referenced a TalosConfigTemplate nothing ever created: every worker the cluster-autoscaler added to `md0` stayed stuck waiting for its bootstrap config, and the KamajiControlPlane certSANs patch the same Job performs never ran. Clusters that declare their own node groups were unaffected. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured the built-in `md0` node group receives its Talos reconciliation Job when no node groups are configured. * Prevented duplicate reconciliation Jobs when custom node groups are specified. * Preserved correct group-specific settings and control-plane configuration in generated Jobs. * **Tests** * Added regression coverage for default and custom node-group scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
83fce03839 |
test(release): scope the freeze pins to their steps and cover the guard
Three assertions extracted a step block, checked it was non-empty, and then ran their grep over the whole workflow. An unrelated step carrying the same condition would keep them green after the protected step lost its own guard, which is precisely the slip they exist to catch. Each now greps its own block, and `id: freeze` moved into the freeze step's test where it belongs. The force-detection greps looked for `+refs/heads/` and `+refs/tags/`, which is not how git spells a forced refspec: the `+` leads the refspec, so `git push origin "+HEAD:refs/heads/$BRANCH"` force-moves the branch and passed both checks. Match a leading `+` on any push refspec instead. Adds a test for the new build-release dispatch guard: the pattern anchored at both ends, the ref_type comparison and not merely its env wiring, a hard failure rather than a skip, and the guard ordered before the checkout. Every pin mutation-tested — forcing either push with a `+` refspec, parking the freeze condition on another step, dropping `id: freeze`, removing or unanchoring the dispatch guard, demoting it to exit 0, and relocating it after the checkout all turn the suite red. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
dfb1a41f28 |
ci(release): refuse a build-release dispatch from a non-line ref
The workflow_dispatch trigger added for the freeze takes any ref, and the "Run workflow" ref selector preselects the default branch — which is exactly where this workflow's own recovery advice, and cut-prerelease's warning, send an operator. IMAGE_TAG is github.ref_name, so a dispatch from main spends a two-hour `make build` republishing cozystack-packages:main and every main image tag while build-main.yaml may be writing the same ones: the 409 tag-collision class #2711 fixed. Nothing is ever mislabeled, since the content really is that ref, so the cost is a wasted runner rather than a wrong artifact. Guard it anyway: it is five lines, and the mis-click is easy. Anchored at both ends so the per-release staging branches (release-1.6.1, release-1.6.0-rc.4) do not qualify — those are built by tags.yaml — and gated on ref_type so a tag named release-1.6 cannot pass the pattern on its own. Runs before the checkout and the OCIR login, so a refused dispatch never touches the credentials. Fails rather than skips: a skipped job reads in the run list like one that produced the artifact. The push trigger was already filtered to release-X.Y; this closes the dispatch path to the same set. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
e8675dd321 |
docs(release): reword the changelog-backstop comments for the freeze model
Three comments still described the changelog reaching main by default, with a release-X.Y-targeted promote PR as a patch-release corner case. After the freeze that is inverted: promote-rc.yaml bases the promote PR on release-X.Y whenever the branch exists, and cutting vX.Y.0-rc.1 creates it, so every release merges its changelog onto the maintenance line and none reach main on the way. The tags.yaml backstop is now the normal route, and its self-skip on `exists` is the exception. These comments are the first thing anyone debugging a missing changelog reads, so pointing at the wrong model of which path is normal costs real time. Reworded in place: the generate-changelog job header, the three-paths comment above check_changelog, its inline at_tag note, finalize's explanation of why update-releasenotes.yaml is not trusted with the release body, and the contract test that pins the porting behaviour. Prose only, no behaviour change. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
22c1df8b94 |
test(etcd-operator): pin deployment unittest to v0.5.4
The manager image tag derives from .Chart.AppVersion, so bumping appVersion to v0.5.4 without updating the two image literals in the deployment unittest broke the suite (helm-unittest gates the PR via `make unit-tests`). Update both asserts (manager container image and the agent OPERATOR_IMAGE env) to ghcr.io/cozystack/etcd-operator:v0.5.4. Suite is green again (18/18). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
19f969b2f8 |
chore(etcd-operator): bump etcd-operator to v0.5.4
Bump the cozystack etcd-operator packages from v0.5.3 to v0.5.4. The release is a controller bug-fix set with no API, RBAC or values changes: - fix(controllers): derive --initial-cluster-state from phase, not from the seed (cozystack/etcd-operator#355) - fix(controllers): stop exempting the bootstrap seed from self-heal (cozystack/etcd-operator#354) - fix(controllers): extend crash-loop self-heal to memory members (cozystack/etcd-operator#352) - fix(controllers): switch the PDB from maxUnavailable to minAvailable (cozystack/etcd-operator#351) Adaptations: - etcd-operator/Chart.yaml: appVersion v0.5.3 -> v0.5.4 (the manager image tag defaults to .Chart.AppVersion, so this reimages the controller). - etcd-operator/Makefile, etcd-operator-crds/Makefile: ETCD_OPERATOR_REF v0.5.3 -> v0.5.4. - etcd-operator-crds/templates/etcdmembers.yaml: re-vendored at v0.5.4 via `make update`; description-only change to the /scale replicas/selector fields tracking the PDB minAvailable fix. etcdclusters/etcdsnapshots unchanged. - templates/rbac.yaml left as-is: manager-role-rules.yaml is byte-identical between v0.5.3 and v0.5.4. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Myasnikov Daniil
|
4e9c5ce8ca |
fix(kubernetes): render the talos-reconcile Job for the default md0 group
The Job in templates/talos/talos-reconcile-job.yaml is the only producer of the TalosConfigTemplate that each worker MachineDeployment names in spec.template.spec.bootstrap.configRef; cluster.yaml renders the reference but deliberately not the object. Its loop read .Values.nodeGroups directly, while the MachineDeployment loop reads the effective set through the kubernetes.nodeGroups helper. For a cluster that supplies no nodeGroups the two disagree: the helper's else-branch emits the built-in md0 group, so the md0 MachineDeployment renders, but the raw map is empty and no Job renders at all. Every Machine the cluster-autoscaler adds to md0 then blocks indefinitely on a TalosConfigTemplate that nothing will ever create, and the KamajiControlPlane spec.network.certSANs patch the same Job performs is skipped too. Range over the helper so the Job set tracks the MachineDeployment set exactly. No change for a cluster that declares its own groups, and md0 stays removable: the helper's if-branch keeps a user-supplied map authoritative. The gap survived because every helm-unittest fixture and e2e suite declares md0 explicitly. tests/nodegroups_default_test.yaml does render the empty-nodeGroups case but lists only templates/cluster.yaml, so it never looked at the Job. The new suite pins both halves of the helper contract and fails without this fix. Fixes #3504 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
e532844007 |
test(release): pin the freeze and backport-target contracts
Neither cut-prerelease.yaml nor backport.yaml had any test coverage, and
neither can be exercised by a PR lane: the first runs only on
workflow_dispatch and its whole purpose is a one-way write-once side
effect, the second only on a merged main-targeted PR. Every invariant
here is one that would otherwise first report a slip at a real release.
Pinned, following promote-gate-contract.bats so a guard demoted to a
comment cannot satisfy a pin:
- the freeze is gated to kind == 'rc' && patch == '0'. Widening either
half re-opens a frozen line with no error surfacing.
- neither the branch create nor the tag push is forced, in any spelling
including a `+refs/` refspec. A force-push on the branch would drag
in everything merged since the freeze.
- an existing release-X.Y is left untouched rather than reused.
- the refuse gate is gated on a main dispatch and runs BEFORE the tag
push, so a mistaken dispatch cannot burn a write-once tag name; the
freeze runs after it, at the commit that was actually tagged.
- backport targets come from enumerated branches, with no
getLatestRelease and no min-1 arithmetic, and `previous` is the
second-newest existing line.
- the comparator sorts numerically descending, which is the only reason
release-1.10 outranks release-1.9. A slip to the default
lexicographic sort aims every backport at a stale line silently.
- freezing a line dispatches its first build-release run.
backport.yaml's logic is a github-script block whose comments name
getLatestRelease and min-1, so the assertions over it strip `//`
comments; the YAML `#` filter alone would let a comment satisfy a pin.
The Makefile discovers hack/*.bats, so this runs under `make unit-tests`
with no wiring.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
5ed6b2204a |
ci(release): build the packages artifact when a freeze creates a line
build-release.yaml publishes `cozystack-packages:<line>` on push to a release line, and pull-requests.yaml's overlay reads it so a release-line PR tests its own line's binaries instead of main's (#3471, #3437). It cannot fire for the push that creates the line. The freeze points release-X.Y at a commit that is already on main, so the push carries no new commits, and GitHub does not run a workflow whose paths/paths-ignore filter finds no changed files ("If there are no files changed, the workflow will not run"). The line would therefore have no artifact until its first cherry-pick merged, and in that window the overlay finds nothing to pull and leaves every package on its committed ref — at freeze time the previous release's, which is exactly the cross-generation mix #3437 fixed. The old flow had no such window: it branched at the promote merge commit, which carried real commits and its own release's refs. Add workflow_dispatch to build-release.yaml and have the freeze step dispatch it for the branch it just created, so the artifact exists from the moment the line does. The dispatch is non-fatal: without it the overlay no-ops and early cherry-pick PRs test their committed refs, which is where they were before #3471. The tag is pushed and the line is frozen by that point, so failing there would misreport both. The trigger also gives a line build a re-run button, which previously needed an empty commit pushed to the line. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
24e9c5396c |
docs(release): resync the release doc with the freeze model
The freeze changed three things the prose still described the old way. The backport table claimed `backport-previous` resolves to `release-X.(Y-1)` and that resolution runs through `getLatestRelease`. Neither is true any more: the job enumerates the real `release-X.Y` branches, sorts them numerically descending, and takes the newest and second-newest existing lines. The `Y-1` derivation is the min-1 bug this branch dropped. Line 155 already carried the correct description, so the file contradicted itself. The freeze window's effect on backport targets lived only in a code comment. During it `backport` aims at the line being stabilised and `backport-previous` at the last published stable, so the line one step further back has no label for the duration. Document the trade-off where the labels are described. The changelog backstop and Phase 6 both framed a promote PR targeting `release-X.Y` as "a patch release". After the freeze every promote PR targets the release line, minor and patch alike, which makes the backstop the only path a `.0` changelog takes to `main`. Also note that re-cutting a line after an unusable rc.1 means deleting `release-X.Y` by hand — the refuse step blocks every other route, by design. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
66ed32c6f9 |
ci(release): print the recovery command if the freeze push fails
Address review feedback from lexfrei on .github/workflows/cut-prerelease.yaml:293: the tag push and this branch push are independent, so a transient failure here leaves the line tagged but not frozen, and the write-once tag guard refuses a same-tag re-dispatch — recovery is manual. Fail with the exact push command rather than leaving the operator to reconstruct it. Deliberately not an atomic two-ref push: the tag push relies on being anchored to the branch tip for GitHub to populate the push event's base_ref, which tags.yaml requires, and whether a multi-ref atomic push preserves that is only observable on a live release cut. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a59ad88eb5 |
ci(release): distinguish absent from broken in the freeze branch probe
Address review feedback from lexfrei on .github/workflows/cut-prerelease.yaml:286: the release-X.Y existence probe piped ls-remote straight into cut, so a transport or auth failure produced empty output and read as "branch not found", steering into the create path. The write-once tag guard, the stale-tip guard, and the refuse step above all make this distinction; match them here. Behaviour is unchanged on the happy path — pipefail already failed the step on a real error, and the non-forced push below fails closed on a conflict. This makes the intent explicit rather than incidental. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b55e822eaf |
docs(release): align the patch-release diagrams with the freeze model
Address review feedback from coderabbitai on docs/release.md:150: the patch-release gitGraphs still branched release-1.2 off main after the v1.2.0 commit, depicting the branch as created at the stable release. Under the rc freeze the line forks at vX.Y.0-rc.1 and the release commit never lands on main, as the surrounding prose already states. Fork release-1.2 at the rc.1 commit and place 'Release v1.2.0' on the release line in all three patch-section diagrams. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |