mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
feat/harbor-multiarch
1208
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
11991b9d05 |
feat(harbor): build the Harbor images for amd64 and arm64
Upstream publishes the Harbor v2.15 component images for amd64 only, so every Harbor pod crash-loops with "exec format error" on an arm64 node. The multi-arch build upstream exists on main but is in no release yet, so the package now rebuilds the pinned v2.15.1 release itself: the Go components cross-compile on the build platform, the registry and the Trivy adapter build from the tags upstream's Makefile pins, the Trivy binary is fetched per architecture against its release checksum, and the runtime images keep upstream's photon base, users and entrypoints. The build fails when those registry and Trivy pins, the swagger generator or the golang and node build images no longer match upstream's Makefile at the pinned release, so a version bump cannot pair a new core with an old registry, scanner or toolchain. The portal compiles to static JS, so it lives in its own Dockerfile listed as architecture-neutral in the cross-compile check, and takes the release pin from the main one so both files build the same commit. Because values.yaml now pins the images, `make update` pins the chart release that ships the same Harbor version and fails when its appVersion differs, instead of pulling the newest chart onto old images. Renovate stops proposing tag bumps for the rebuild's build bases, which follow upstream's toolchain for that release. The committed refs stay on the upstream digests until CI stamps the rebuilt ones. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
084ac47237 |
feat(build): build published images for amd64 and arm64 by default
PLATFORM defaulted to empty, so buildx built for the host alone. On an arm64 workstation `make image` published an arm64-only digest, which amd64 nodes then fail to pull; the cilium pin shipped exactly that until it was rebuilt by hand. A build now targets linux/amd64 and linux/arm64 unless PLATFORM is set, and the digest a package stamps is the index digest. LOAD=1 keeps building for the host only. The classic docker image store cannot load a multi-platform index, and loading is the documented local workflow. This changes local pushes: a `make image` on the default docker driver without the containerd image store now fails with "Multi-platform build is not supported for the docker driver", where it used to build for the host. Such a build needs a docker-container builder (BUILDER), or LOAD=1 or an explicit PLATFORM for a single architecture. talos and testing pin linux/amd64 with `override`, placed before the include because common-envs.mk expands BUILDX_ARGS immediately. The Talos assets are amd64: matchbox bakes the kernel and initramfs into its image, and the e2e sandbox boots the disk under qemu-system-x86_64. An arm64 variant of either would still serve or boot amd64 files. CI pins linux/amd64 on every image build. Nothing there sets up QEMU, fork builds export a single-platform archive, and tags.yaml builds on the default docker driver, so the default would break or emulate those jobs. Native arm64 builds belong in a leg of their own. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d9cbbec5ff |
fix(build): cross-compile image builders on the build platform
The remaining Go builder stages, plus the kube-ovn, console and piraeus-server builders, ran on the target platform. A multi-platform build therefore ran the whole compiler under QEMU: slow, and for Go unreliable, as the migration-controller Dockerfile already records a "found bad pointer in Go heap" crash from exactly that. The builders now run on $BUILDPLATFORM and produce the target architecture from TARGETARCH. Two builders only produced the right architecture because they were emulated. token-proxy ran `go build` with no GOOS/GOARCH, and kube-ovn calls upstream `make build-go`, which hardcodes GOARCH=amd64 even on an arm64 build. On a native builder both would have shipped host binaries inside the other architecture's image, and nothing fails until a node of that architecture starts the container. token-proxy now passes the target explicitly; kube-ovn picks build-go or its arm64 twin build-go-arm from TARGETARCH. The console builder emits static JS and linstor-server's debian/control declares every package Architecture: all, so both run natively without a target. A bats test requires every stage that compiles to sit on a BUILDPLATFORM FROM and to read TARGETARCH or TARGETPLATFORM on a RUN or ENV line. A bare ARG declaration does not count, which is what let token-proxy through. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
623113f920 |
fix(build): pass the shared buildx flags from every image target
Three package Makefiles spelled out their docker buildx flags instead of using $(BUILDX_ARGS) from hack/common-envs.mk: flux-plunger, kubeovn-plunger and both dashboard images. kubeovn-plunger landed just before the change that moved the other Makefiles onto BUILDX_ARGS, flux-plunger was modelled on it, and dashboard kept its own flags when that change touched it. Those targets ignored PLATFORM, SBOM and BUILDX_EXTRA_ARGS. On an arm64 workstation the plungers came out as arm64 images, which fail on amd64 nodes with "exec format error"; CI builds on amd64 and never saw it. Dashboard and metallb hardcoded --platform=linux/amd64 instead; for metallb that sat next to $(BUILDX_ARGS), so setting PLATFORM gave buildx two platforms and a multi-platform build. All of them now build through $(BUILDX_ARGS) alone. CI sets only BUILDER, which these targets already honoured, and runs on amd64, so its builds are unchanged. Local builds of dashboard and metallb now follow the host architecture like every other package: on an arm64 machine build them for an amd64 cluster with PLATFORM=linux/amd64. A bats test dry-runs every package image target with a probe platform and a marker in BUILDX_EXTRA_ARGS, and requires each buildx call to carry the marker, so $(BUILDX_ARGS) is really passed, and that platform and no other. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4a2dea2d08 |
feat(cluster-api): add the Proxmox infrastructure provider and in-cluster IPAM as optional packages (#4087)
## What this PR does **Series behind #3481** — three separable pull requests: 1. **#4087 — Cluster API providers** (capmox + in-cluster IPAM) 2. #4088 — the `substrate` switch in `kubernetes` and `kubernetes-nodes` 3. #4089 — the Proxmox CSI driver and cloud-controller-manager #4087 and #4088 share no files and can be reviewed in parallel. #4089 builds on #4088. --- This is the provisioning layer #3481 asks to specify first: the part that creates the VMs. Neither of the other two is needed to review it. ### What it adds Two Cluster API providers, packaged the way the existing KubeVirt provider is — vendored components, `gzip -9 -n` for a reproducible artifact, a `make components` target that regenerates it from the upstream release and checks the downloads against the SHA-256 sums the Makefile pins: | Package | Version | Contract | |---|---|---| | `capi-providers-infraprovider-proxmox` | capmox v0.7.7 | v1beta1 | | `capi-providers-ipam-in-cluster` | CAIP v1.0.3 | v1beta1 | capmox v0.7.7 rather than the current v0.9.x on purpose: from v0.8 it declares contract `v1beta2`, which the CAPI core shipped here (v1.10.1) does not serve. Both join the `iaas` bundle as **opt-in** packages, where the KubeVirt infrastructure provider is unconditional. The asymmetry is deliberate: capmox drives a hypervisor outside the cluster and needs a Secret holding a Proxmox API token, which capi-operator reads before it installs anything — with no Secret the provider is never deployed at all. Enabling it by default would ask every Cozystack install that has no Proxmox to supply hypervisor credentials. ```yaml bundles: enabledPackages: - cozystack.capi-provider-infra-proxmox # brings IPAM with it - cozystack.capi-provider-ipam-in-cluster # also enableable on its own ``` Enabling capmox emits the IPAM provider automatically — capmox allocates worker addresses from an `InClusterIPPool` and has no DHCP mode, so on its own it never gets a `ProxmoxCluster` to `Ready`. IPAM stays independently enableable, since assigning addresses is a generic Cluster API concern. Naming capmox in `enabledPackages` while disabling IPAM stops the render. The cluster would catch that combination on its own — the capmox `PackageSource` depends on the IPAM `Package`, so with that `Package` absent capmox goes `Ready=False` with `DependenciesNotReady` and no `HelmRelease` is created — but it would not say why. Failing the render names the two conflicting list entries at the moment someone writes them, instead of leaving a condition to decode. ### Why the earlier attempt did not produce a VM #1927's blocker — "ProxmoxMachine does not produce a VM" — was not a capmox incompatibility. It was four independent causes, each of which only became visible once the previous one was cleared: 1. **`PROXMOX_URL` included `/api2/json`.** capmox appends that itself, so every call went to `/api2/json/api2/json/…` and returned 501. 2. **No IPAM provider.** `no matches for kind "InClusterIPPool"`; the `ProxmoxCluster` never reached `Ready` and every `ProxmoxMachine` sat in `WaitingForClusterInfrastructure`. 3. **`templateSelector.matchTags` is set equality, not containment** (`slices.Equal` after sort). A template carrying four tags is not selected by naming one of them: `found 0 VM templates with tags`. 4. **capmox waits for the qemu-guest-agent, which Talos does not ship.** `error waiting for agent: timed out` — cleared with `checks.skipQemuGuestAgent` and `skipCloudInitStatus`. None of these are in the provider. They are packaging and configuration, which is why the earlier work could look complete and still never boot a machine. ### Evidence Run on Proxmox VE 9.2.11 with Talos v1.13.6 and Cozystack's CAPI stack (core v1.10.1, CABPT v0.6.12, Kamaji control-plane provider v0.19.0): ``` $ kubectl get proxmoxmachine,machine -n tenant-proxmox proxmoxmachine/kubernetes-px-md0-… Ready=True VMProvisioned=True machine/kubernetes-px-md0-… Running proxmox://0ef88cc1-6aea-4563-af64-344d8060d5c0 $ qm list 106 kubernetes-px-md0-jmzds-h9npx running ``` The machine boots Talos from the nocloud ISO, reads the machineconfig capmox writes, and joins. That is the layer #69 called a hard blocker. ### What this PR does not claim - One Proxmox node, one tenant cluster, manual runs — there is no e2e coverage for Proxmox in CI, and this PR does not add any. - It does not make Proxmox a supported platform. It makes the provisioning layer reviewable, which #3481 names as the precondition for that conversation. - It does not touch `capi-providers-core`. The same drift check, pointed at that package, shows that the `startupProbe` from #2946 never reached the shipped blob; that fix changes what every new install runs and is #4483, with its own release note. - Nothing here exposes `ProxmoxCluster.spec.credentialsRef` to a tenant, and nothing should: `capmox-manager-role` grants get/list/watch/patch on Secrets cluster-wide and that field takes a namespace. Whatever renders a `ProxmoxCluster` for a tenant in the rest of the series has to keep the field out of tenant hands. ### Testing `make unit-tests` — exit 0. New suites: `packages/core/platform/tests/sources_capi_provider_proxmox_test.yaml` and `bundles_proxmox_wiring_test.yaml` (both bundle paths, the enable/disable precedence, and the refusal above). Every negated `containsDocument` in the wiring suite carries `any: true`; without it the negation passes whether or not the document is in the render. Three mutations, each reverted afterwards — emitting the capmox `Package` unconditionally, emitting the IPAM `Package` unconditionally, and dropping the `disabledPackages` check from `$proxmoxEnabled` — each turn red the tests named after the property they break, and the unmutated chart stays green (44 suites, 198 tests). `hack/capi-provider-components_test.bats` covers both new provider packages: the blob and the YAML are the same bytes; the Makefile version, the ConfigMap name and `spec.version` are one value — `applied-spec-hash` covers the provider spec and the config Secret but not the ConfigMap, so a bump that regenerates the files while `spec.version` stays behind installs the old provider silently; and the vendored files match the SHA-256 sums the Makefile pins, so the pin holds even when nobody runs `make components`. The render was compared against `main` for five configurations (defaults, `isp-full`, `isp-full-generic` with paas, an unrelated `enabledPackages`, and `disabledPackages` naming IPAM). No line is removed in any of them and the `Package` count is unchanged; the only additions are the two `PackageSource` objects, which the chart renders for every package, optional or not. ### Screenshots Not applicable — no UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against this diff, file by file. What it matches is below. **No box is ticked**, because every follow-up it points at depends on a decision that is not mine to make: #3481 asks whether Proxmox becomes a supported platform at all, and writing its documentation or its Terraform schema before that is answered would be work built on an assumption. I have not filed speculative PRs or issues in those repositories for the same reason. If the direction is accepted, I open them; a human should decide which. **cozystack/website — matches, no follow-up yet.** - `packages/core/platform/values.yaml` changed. `content/en/docs/next/operations/configuration/platform-package.md` is a hand-written table of the `spec.components.platform.values.*` keys; this PR does not add a key, but it documents two new values that `bundles.enabledPackages` now accepts, and the table is where a reader would look for them. - Two platform components are added (`capi-providers-infraprovider-proxmox`, `capi-providers-ipam-in-cluster`) → `guides/platform-stack/_index.md`, `operations/configuration/licenses.md`. - The `iaas` bundle gains two optional packages → `operations/configuration/variants.md`. **Everything else: no match.** Nothing under `packages/apps/` or `packages/extra/` (terraform-provider, website app lists), no `packages/core/installer/values.yaml` or `networking.*`/`publishing.*` key the Ansible role sets, no `hack/` file or package-layout change (ccp, external-apps-example), no `ApplicationDefinition` semantics, no telemetry metric, no node prerequisites (talm, ansible), no commit-convention change (community). - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(cluster-api): add optional Cluster API packages for Proxmox — the capmox infrastructure provider and the in-cluster IPAM provider it requires. Neither is deployed unless named in `bundles.enabledPackages`, since capmox drives a hypervisor outside the cluster and does nothing without Proxmox API credentials. Enabling the Proxmox provider brings the IPAM provider with it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added optional Proxmox infrastructure provider support for IaaS clusters. - Added in-cluster IP address management, usable independently or alongside Proxmox. - Proxmox deployments require a pre-created credentials Secret and automatically include in-cluster IPAM. - Added validation for incompatible Proxmox and IPAM settings. - **Tests** - Added coverage for provider dependencies, package enablement, disabling behavior, invalid configurations, and packaged provider components. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
e883f6f7fc |
fix(capi): ship the core provider manifests the repository actually holds (#4483)
## What this PR does `capi-providers-core` ships the Cluster API core manifests twice: as `files/core-components.yaml`, which a reviewer reads, and as `files/components.gz`, which the chart mounts into the cluster. #2946 added a `startupProbe` to `capi-controller-manager` and edited only the YAML; the blob was never regenerated, so the probe never reached a cluster. This regenerates the blob from the YAML (`gzip -9 -n -c files/core-components.yaml > files/components.gz`), so the decompressed blob matches the YAML byte for byte, and the only change it carries is the six lines of #2946. ### What this reaches, and what it does not New installs only. capi-operator re-applies a provider when the hash of its spec and its config Secret changes (`applied-spec-hash`); the ConfigMap holding the components is in neither, so a cluster that already runs `capi-providers-core` keeps its manager without the probe until the provider's version moves. Whether existing clusters should get the probe, and how, is left open on purpose: moving `spec.version` and the ConfigMap name together would force the re-apply, but that is a version decision for the package, not a side effect of regenerating a blob. ### Also in this diff - `.helmignore`: `files/.*-components.yaml` is a glob, so it matched only names beginning with a dot, and `helm package` shipped the readable YAML alongside the blob that duplicates it. Now `files/*-components.yaml`. - `hack/capi-core-components_test.bats` holds the two files to the same bytes from now on; it goes red on a YAML edited without regenerating the blob. Found by the drift check written for the Proxmox provider packages in #4087, pointed at the package that was already here; split out because it changes what every new install runs and deserves its own release note. ### Testing `bats hack/capi-core-components_test.bats`: 1/1; red with a line appended to the YAML, green again after `gzip -9 -n -c`. `helm package` of the chart now contains `files/components.gz` and `files/metadata.yaml` only. ### Screenshots Not applicable, no UI change. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note fix(cluster-api): ship the `startupProbe` from #2946 in the packaged core provider components. New installs get it; a cluster that already runs the core provider keeps its manager without the probe until the provider's version moves, because capi-operator re-applies components only when the provider spec or its config Secret changes. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added a check that the compressed core components match the YAML source, with a diff provided when they differ. * **Chores** * Updated chart packaging exclusions to cover component YAML files regardless of whether their names begin with a dot. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
a285a5ef36 |
feat(kubernetes): add a proxmox substrate for managed clusters and worker pools (#4088)
## What this PR does **Series behind #3481** — three separable pull requests: 1. #4087 — the Cluster API providers (capmox + in-cluster IPAM) 2. **#4088 — the `substrate` switch** in `kubernetes` and `kubernetes-nodes` 3. #4089 — the Proxmox CSI driver and cloud-controller-manager #4087 and #4088 share no files and can be reviewed in parallel. #4089 builds on this one. --- #4087 adds the Cluster API providers; this PR teaches the two managed-cluster charts to point at them. ### The shape A single value, `substrate: kubevirt | proxmox`, on both charts — not a parallel set of charts. The control plane is Kamaji either way; what changes is the infrastructure half: | | `kubevirt` (default) | `proxmox` | |---|---|---| | `Cluster.infrastructureRef` | `KubevirtCluster` | `ProxmoxCluster` | | worker template | `KubevirtMachineTemplate` | `ProxmoxMachineTemplate` | | apiserver Service | `ClusterIP` | `LoadBalancer` | | tenant CCM/CSI | in-cluster (`kccm`, kubevirt-csi) | rendered off; the Proxmox pair is #4089 | The KubeVirt path renders byte-for-byte what it rendered before. Checked by rendering `kubernetes-nodes` with default values at `main` and at this head and diffing the output, not only by the snapshots — the snapshots cover `nodegroup.yaml` alone, and an earlier revision of this branch changed the reconcile Job while they stayed green. The parent chart's default render differs only in the random fields of `talos-secrets`, which differ between two renders of `main` as well. `osImage` (#4171) is refused on the proxmox substrate. Only half of it could be honoured there — `factory.version` reaches the machineconfig's installer image while the worker disk keeps coming from the Proxmox template — and a field that half-works is worse than one that says no. `talos.version` and `talos.schematicID` still set the installer target. ### Three things that only show up once workers are off-cluster **The apiserver endpoint.** A KubeVirt worker runs inside this cluster and reaches the tenant apiserver by ClusterIP. A Proxmox worker does not: it lives on the hypervisor's L2 and cannot route to `10.96.0.0/16`. The machine provisions fine, `VMProvisioned=True`, `providerID` set — and then never joins, with no CSR to show for it. On the proxmox substrate the Kamaji Service is therefore a `LoadBalancer`, and the reconcile Job reads `.status.loadBalancer.ingress[0].ip` instead of `.spec.clusterIP`. **And DNS, for the same reason.** The worker machineconfig carried `nameservers: ${COREDNS_IP}` — the management cluster's CoreDNS ClusterIP, equally unreachable. The proxmox path takes its own resolvers (`proxmox.dnsServers`) and the chart refuses to render without them, rather than writing an address that cannot answer. **`externalManagedControlPlane` and the managed-by annotation.** capmox's webhook nil-derefs on a `ProxmoxCluster` whose control-plane endpoint is empty unless `spec.externalManagedControlPlane` is set, which is correct here — Kamaji owns the endpoint. Separately, the `cluster.x-k8s.io/managed-by: kamaji` annotation that the KubeVirt path carries must **not** be set on the Proxmox object: with it, CAPI never sets an ownerRef and capmox stops at `Waiting for Cluster Controller to set OwnerRef`, leaving the address pool uncreated and every worker waiting on infrastructure. ### Linked clones by default `proxmox.full: false` — worker disks are linked clones of the template rather than full copies. Measured on a ZFS-backed store: **8 KiB per worker instead of 10.2 GiB**. Operators who want independent disks set `full: true`; the value is plumbed straight through to `ProxmoxMachineTemplate`. `proxmox.storage` is a full-clone parameter and is refused with a linked clone: a linked clone always lives on the template's storage, and both Proxmox (`parameter 'storage' not allowed for linked clones`) and capmox's own CRD (`Must set full=true when specifying storage`) reject the pair. The chart says so at render time, naming both values, instead of failing at apply time with an error about a `ProxmoxMachineTemplate` nobody wrote. ### Sizing `ProxmoxMachineTemplate` takes `numCores` and `memoryMiB`, not Kubernetes quantities, so the pool's `resources.cpu`/`memory` are converted (millicores → cores, bytes → MiB, both rounded up). The rounding is pinned in `tests/substrate_branches_test.yaml` with `1500m` and `8G`, where rounding up and down disagree; `tests/substrate_proxmox_sizing_test.yaml` covers the refusal to size a pool that gives neither. ### Known upstream behaviour this surfaces `TalosConfigTemplate.spec` is immutable by schema, so the reconcile Job cannot rewrite an endpoint it has already published — `kubectl apply` fails with `TalosConfigTemplate.Spec is immutable`, and the template has to be deleted for the Job to recreate it. A fresh install never meets it; it appears when the apiserver address changes on an existing cluster. Handling it changes the KubeVirt path too, so it is not in this PR: it is a separate change on `proxmox/tct-replace`, with its own upgrade note. ### Evidence ``` $ KUBECONFIG=<tenant> kubectl get nodes -o wide kubernetes-px-md0-jmzds-ndnl6 Ready <none> 75s v1.35.6 10.0.0.160 Talos (v1.13.6) $ KUBECONFIG=<tenant> kubectl get pods -A cozy-cilium cilium-… Running kube-system konnectivity-agent Running kube-system coredns-… Running ``` No management-cluster address remains in the rendered `TalosConfigTemplate`. ### Testing `make unit-tests` — exit 0. `apps/kubernetes` 26 suites / 314 tests; `apps/kubernetes-nodes` 29 suites / 160 tests / 12 snapshots, all unchanged on the default path. Each of the ten commits passes both suites on its own. Every substrate branch is asserted on both sides, and every assert goes red on the mutation it exists for: inverting the `SVC_IP` condition, inverting the `nameservers` condition, `ceil`→`floor` in the sizing, removing the proxmox gate from each of the kccm and csi templates (the two RBAC bindings included), pinning the retention lookup back to `KubevirtMachineTemplate`, widening the IPv6 class of the `dnsServers` validation, dropping the parent chart's `dnsServers` validation, the storage-with-linked-clone refusal, and the `osImage` refusal. Negated `containsDocument` asserts carry `any: true`. `hack/talos-reconcile-heredoc_test.bats` additionally runs the rendered heredoc through a real shell: a `dnsServers` entry with a substitution is refused and named, an IPv6-shaped one with a substitution is refused too, and a real address survives the shell unchanged. ### Screenshots Not applicable — no UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against this diff, file by file. What it matches is below. **No box is ticked**, because every follow-up it points at depends on a decision that is not mine to make: #3481 asks whether Proxmox becomes a supported platform at all, and writing its documentation or its Terraform schema before that is answered would be work built on an assumption. I have not filed speculative PRs or issues in those repositories for the same reason. If the direction is accepted, I open them; a human should decide which. **cozystack/terraform-provider-cozystack — matches, no follow-up yet.** - `packages/apps/kubernetes/values.schema.json` and `packages/apps/kubernetes-nodes/values.schema.json` both gain fields: `substrate`, and a `proxmox` object (`allowedNodes`, `dnsServers`, `ipv4Config`, and on the pool side `templateTags`, `network`, `storage`, `full`). The provider hand-maintains a schema, a model and an expand/flatten pair per Kind, with no codegen from these files. - `values.yaml` gains defaults for those fields (`substrate: kubevirt`, `proxmox.full: false`, `prefix: 24`). The provider restates defaults it models and sends its own, so a stale one wins silently rather than showing as a diff. **Everything else: no match.** No app added, renamed or removed (website app lists); no version enum, `kind`, `plural`, `release.prefix` or output Secret/Service name changed; `packages/core/platform/values.yaml` untouched; the `*-rd` files are regenerated, not renamed (ccp); no `hack/` change; no node prerequisites (talm, ansible); no telemetry metric; no commit-convention change (community). - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(kubernetes): add a `substrate` value to the `kubernetes` and `kubernetes-nodes` applications, selecting whether worker VMs run on KubeVirt in this cluster (`kubevirt`, the default and unchanged) or on an external Proxmox VE cluster through capmox (`proxmox`). On the Proxmox substrate the tenant apiserver is published through a LoadBalancer and worker disks default to linked clones. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for choosing KubeVirt or Proxmox for Kubernetes clusters and node pools. * Added Proxmox configuration for nodes, networking, storage, DNS, templates, cloning, and IPv4 allocation. * Proxmox deployments now use the appropriate cluster, machine, networking, and control-plane resources. * **Validation** * Added checks for supported substrates, required Proxmox settings, sizing, and incompatible options. * **Documentation & Tests** * Documented the new settings and expanded coverage for rendering, validation, cloning, sizing, and template replacement. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Marian Koreniuk
|
4bc6621a5c |
test(kubernetes,kubernetes-nodes): assert both sides of every substrate branch
Every `if substrate` in both charts is asserted on both sides, so an inverted condition cannot pass: - the reconcile Job reads SVC_IP from the LoadBalancer status on proxmox and from the ClusterIP on kubevirt; - the machineconfig carries the pool's nameservers on proxmox and none on kubevirt; - the proxmox gate on each kubevirt-only template in the parent chart, the two RBAC bindings included, so a proxmox cluster renders no binding to a Role, ClusterRole or ServiceAccount it does not render; - the sizing conversion at 1500m and 8G, where ceil and floor disagree (2 cores and 7630 MiB against 1 and 7629); - the old-template retention loop keeps a ProxmoxMachineTemplate a MachineSet still references; - the proxmox worker monitor carries no Pod threshold while the kubevirt one does; - a live cluster and a live pool refuse a substrate switch while a first install still renders; - both charts refuse a dnsServers entry that is not an address, the parent chart included, where the value reaches ProxmoxCluster.spec.dnsServers. The negations carry `any: true`: without it a negated containsDocument passes whether or not the document is in the render. hack/talos-reconcile-heredoc_test.bats gains the dnsServers cases: the render refuses a command substitution and names the entry; an entry with a colon in it is refused too, which pins the IPv6 branch of the validation and not only the IPv4 one; and a real address survives the Job's shell unchanged. The heredoc is extracted by shape — `cat <<EOF` at the start of a line or right after `=$(` / `="$(` — so the same case keeps working whichever way the Job writes it. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
9934014908 |
fix(capi): ship the core provider manifests the repository actually holds
#2946 added a startupProbe to capi-controller-manager so it survives the certificate provisioning delay. It edited capi-providers-core/files/core-components.yaml, which is the copy a reviewer reads. The chart mounts files/components.gz, and that blob was never regenerated, so the probe never reached a cluster. The blob is regenerated from the YAML it is supposed to carry: gzip -9 -n -c files/core-components.yaml > files/components.gz The decompressed blob now matches the YAML byte for byte, and the only change it carries is the six lines of #2946. This reaches new installs only. capi-operator re-applies a provider when the hash of its spec and its config Secret changes; the ConfigMap holding the components is in neither, so a cluster that already runs capi-providers-core keeps its manager without the probe until the provider's version moves. Whether existing clusters should get the probe, and how, is left open here on purpose: moving spec.version and the ConfigMap name together would force the re-apply, but that is a version decision for the package, not a side effect of regenerating a blob. .helmignore is corrected in the same change: `files/.*-components.yaml` is a glob, so it matched only names beginning with a dot, and `helm package` shipped the readable YAML alongside the blob that duplicates it. hack/capi-core-components_test.bats holds the two files to the same bytes from now on, so the next edit of one without the other fails in CI. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
7843ec98b5 |
test(capi): catch a vendored provider whose blob and YAML have drifted
Each provider package ships the manifests twice: the YAML a reviewer reads and the gzip blob the chart mounts. `make components` writes both from one download and nothing keeps them together afterwards, so editing the YAML alone produces a diff that reviews cleanly while the cluster runs the version nobody read. Both new provider packages are covered. A second pair of tests holds the version together: capi-operator hashes the provider spec and the config Secret into applied-spec-hash, and the ConfigMap is in neither, so a bump that regenerates the vendored files while the ConfigMap name and spec.version stay behind installs the old provider and reports the old version, with nothing reporting an error. The Makefile variable, the ConfigMap name and spec.version have to be one value. A third pair checks the vendored files against the SHA-256 sums the Makefile pins, so the pin holds even when nobody runs `make components`. Written for dash rather than bash, since the repo's runner executes these under sh — process substitution is not available there. Each pair goes red on the edit it exists for: a YAML edited without regenerating the blob, a Makefile version bumped on its own, a ConfigMap renamed without spec.version, and a vendored file that no longer matches its pinned sum. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
368d00b5d4 |
test(e2e): remove unused monitoring suites and audit BATS runners (#4019)
## What this PR does I removed the two monitoring OIDC BATS suites that had no runner after the Chainsaw migration, and brought the runner audit and wrapper OIDC test from [#3778](https://github.com/cozystack/cozystack/pull/3778). Also added checks for audience mapper and `OIDC_MODE=System`. The named `KeycloakRealmGroup` test documents coverage the document-count and kind assertions already provided. Runner audit reports unused BATS suites, missing files in runner paths, and unresolved Make variables (including `$(E2E_SUITE)`). Live Monitoring API admission, Grafana operator reconciliation and mode transitions are not tested here. ### Validation - `make -j4 unit-tests SHELL=/usr/bin/dash` passed. - Monitoring Helm tests: 53 system-chart tests and 10 wrapper-chart tests passed. - Runner audit: 12 cases passed under dash and Bats 1.14.0. The full shell syntax and BATS sweep passed under dash too. - Three chart mutations and seven runner mutations failed as expected. ### Downstream repositories - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` |
||
|
scooby87
|
03e2da5805 |
feat(postgres,mariadb)!: drop plaintext user passwords (#4078)
## What this PR does Removes the plaintext `users[].password` field from the `apps/postgres` and `apps/mariadb` charts (and the mariadb `root` account), so managed-database passwords can no longer be pinned in tenant values or committed to git. Passwords are always chart-generated into the `<release>-credentials` Secret and preserved across reconciles via `lookup`, matching the model `redis`/`valkey` already use. This closes the "User passwords" item of the portal upstream epic (aenix-org/cozyportal#1060). The generated password never lands in the init-script Secret or the SQL it runs: the postgres init-job reads each user's password from the mounted `<release>-credentials` Secret at run time and applies it through a psql variable (`ALTER ROLE … WITH PASSWORD :'pw'`), so it does not linger in the init-script manifest a tenant can read. It does live in cleartext `stringData` in the `<release>-credentials` Secret itself, which is a chart manifest and therefore kept in Helm release history for `MaxHistory` revisions like any chart-generated secret — that residue is exactly why rotation cannot be built on the chart (see below). A `password` or a `passwordRotation` key left over in a tenant's values is accepted-but-inert and draws an admission warning, so an operator editing it is told it has no effect rather than getting a silent 200. Backup/restore into a copy keeps the target's chart-managed credentials working: the CNPG driver clears `bootstrap.enabled` once recovery is healthy so the init-job re-applies the generated passwords onto the recovered roles, and the MariaDB driver excludes `mysql.global_priv` from the dump (`ignoreGlobalPriv`) for backups captured from this release onward, so restoring one of those into a copy does not clobber the target's chart-managed users or `root`. Excluding the grant table has a flip side, and it applies to every MariaDB restore, not only a to-copy one: only accounts declared through `User` CRs (the chart's `users` map) are captured, so an account created out of band directly in MySQL is in no backup and a restore brings back its data and `mysql.db` grants but not the account itself. This is also a backup-side fix only: a restore from a backup taken **before** this change still replays the source's `mysql.global_priv` and overwrites the target's users and `root`, and `root` then needs an out-of-band reset (the operator applies `rootPasswordSecretKeyRef` only at datadir bootstrap, so there is no in-band path). See `docs/operations/backup-classes.md` for the convergence window, the out-of-band-account limitation, and this old-backup caveat. Password **rotation** is deliberately **not** in this PR. A chart-rendered credentials Secret keeps its cleartext in Helm release history for `MaxHistory` revisions, so a rotation built on it cannot revoke a leaked password; rotation is being reworked as a controller that owns the Secret — design proposal in cozystack/community#72. Migration is seamless: existing releases keep their current passwords on upgrade (the value already lives in `<release>-credentials`, which `lookup` re-reads and preserves); a `password` still present in a tenant's values is ignored, so no existing HelmRelease fails validation. Verification: `helm unittest` passes for both charts (postgres 87, mariadb 126, including render-time username guards that reject any name k8s would later reject as an invalid Secret key or User resource name); `go build ./...`, `go vet`, the backup-controller tests and the apps-apiserver tests are green; `make generate` was run with the CI-pinned `cozyvalues-gen` v1.6.0 so `values.schema.json`, the READMEs, the API types, and the `cozyrds` ApplicationDefinitions are all in sync (the branch was rebased onto `main`, which reconciled cleanly with the read-replica autoscaling feature that landed there). ### Screenshots N/A — no UI change. ### Downstream repositories - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: affected — `content/en/docs/next/cozystack-api/go-types.md` is hand-written and still shows `Password:` in the Go examples, and the managed-app reference pages regenerate from these chart READMEs, so both document `users[name].password` until refreshed (not yet filed). - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: affected — the provider hand-maintains the postgres/mariadb schemas and per-field defaults; this PR removes `users[].password`. A follow-up issue/PR is needed there (not yet filed). `terraform-provider-cozystack` and `website` are reached by this diff; both boxes are left unticked because the follow-ups are not yet filed. Same-repo future work this change suggests: the controller-owned password rotation (design in cozystack/community#72), and extending the "passwords only in the Secret" model to the other engines that still accept plaintext (`clickhouse`, `mongodb`, `nats`, `opensearch`, `rabbitmq`, `vpn`). ### Release note ```release-note feat(postgres,mariadb)!: passwords for managed PostgreSQL and MariaDB users can no longer be set via `users[].password` — they are always auto-generated and stored only in the `<release>-credentials` Secret, never rendered into the init-script or the SQL it runs. Existing releases keep their current passwords on upgrade; a `password` (or a `passwordRotation`) left in values is ignored with an admission warning. Password rotation is deferred to a follow-up controller design (cozystack/community#72). MariaDB backups now exclude the `mysql.global_priv` grant table (`ignoreGlobalPriv`), so a restore recreates only accounts declared through the chart's `users` map — an account created out of band directly in MySQL is not captured and must be recreated after a restore. ``` |
||
|
Aleksei Sviridkin
|
6748014165 |
fix(ci): overlay the right line's refs for a PR on a feature branch (#4460)
<!-- 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 `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does A PR stacked on a feature branch now overlays the image refs of the line it grew from, so its E2E no longer falls back to the refs committed in the tree. The Finalize job pulls `cozystack-packages:<base branch>` and overlays its image refs onto the packages the PR did not rebuild. For a stacked PR the base is something like `fix/foo`. The slash is not valid in an OCI tag, and no workflow publishes an artifact for that branch anyway. So the pull failed, the step only printed a warning, and every unbuilt package ran its committed release ref. That is how stacked PRs got the Grafana plugin 404 described in #4391. Now `main` and `release-X.Y` bases read their own artifact. Any other base reads the artifact of whichever of main and the release lines it has the fewest commits on top of, and a notice says which one. A stack on a release line keeps that line's images, for example #3895, which sits on `release-1.5`. On a tie main wins, so a stack on a line cut from main reads main's artifact until the stack contains a commit of the line's own. Always taking main there would put main's images under the line's charts, and that is the install failure from #3437. The new bats case builds a small git fixture and checks the tag pulled for six bases, one of them a deleted branch. The other half of #4391, the stale `grafana-dashboards.tag`, is a release-managed digest pin and is not touched here. It gets re-stamped at release prep. Part of #4391 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(ci): E2E for a PR stacked on a feature branch overlays the image refs of the line the stack grew from, instead of falling back to the committed release refs. ``` |
||
|
Aleksei Sviridkin
|
b981ea7f26 |
fix(backups): stop the nats, kafka-metadata and rabbitmq backup demos from reading empty values (#4458)
## What this PR does The nats, kafka-metadata and rabbitmq backup demos carried the two empty-read defects the redis demo already fixed. **Empty read from an attach race (nats, kafka-metadata).** Both seed/verify helpers ran each CLI call in a throwaway `kubectl run -i` pod. That pod's stdout returns over an attach that only carries what the container writes after the attach registers, so a CLI that finishes first comes back empty with exit 0. Neither NATS nor Kafka ships a long-lived pod that carries its CLI (unlike rabbitmq, which already execs `rabbitmqctl` in its `-server-0` pod), so each demo now runs one long-lived CLI pod and `kubectl exec` into it: an exec'd process owns its pipes, so its output cannot be lost that way, and stderr stays attached so a failed read says why. `cleanup.sh` removes the pod. **Single read of the platform strategy (rabbitmq, kafka-metadata).** Both runners read the platform's `cozy-default-<engine>` strategy once, right after install, where the Helm upgrade that renders it is gated on `dependsOn` and can be absent for minutes. Both now `kubectl wait --for=create` for it, so it polls through NotFound and fails only on a real error. Unit tests for the two rewritten helpers are added under `hack/`, in the `cozytest.sh` dialect, and assert the reply comes back through exec (a throwaway run pod makes the stub fail). Validation: kafka-metadata and rabbitmq are exercised by their in-tree chainsaw E2E suites. The nats demo has no chainsaw suite in this repository, so its change is covered by the unit test only. ### Screenshots No UI changes. ### Downstream repositories <!-- Walked the trigger map in docs/agents/contributing.md against the diff. The change is confined to examples/backups/* demo scripts, one demo README, and two hack/*.bats files. No repository's trigger matches: it touches no packages/apps, no values.schema.json, no ApplicationDefinition, no platform values, no hack/package.mk or make target, and no commit convention. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(backups): stop the nats, kafka-metadata and rabbitmq backup demos from reading empty CLI output ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Kafka and NATS backup demos reuse their CLI pods and limit cleanup to pods belonging to the respective demo. * NATS commands no longer include the password in the server URL, and command errors remain visible. * The RabbitMQ demo waits for its backup strategy to appear and reports missing backup images more clearly. * **Tests** * Added coverage for reusing or creating demo pods, ownership checks, password handling, and command failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
8b6537f062 |
fix(e2e): collect the CDI controller log and CDI pods in cozyreport (#4414)
<!-- 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 `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The e2e report now keeps the `cdi-deployment` log and the YAML of every pod with CDI's `app=containerized-data-importer` label, in any namespace and any phase. In #4374 a Completed importer pod held the worker disk PVC in `Terminating`, and the report had nothing to show why CDI did not delete it. CDI deletes the importer pod once after a successful import and does not retry. The only record of a failed delete is the controller log, which was never collected. The pod itself was out of reach too, because the broken-pod walk skips Completed pods, so there was no way to see if it ever got a `deletionTimestamp`. The controller log gets a 20000-line tail instead of the usual 2000. CDI runs at `-v=1` and logs every reconcile, and the delete can happen an hour before the report is taken. It does not solve the underlying problem from #4374. It makes the next occurrence diagnosable. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(e2e): the e2e diagnostics report now includes the CDI controller log and CDI pods in every phase. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Diagnostic reports now include up to 20,000 lines each from the current and previous Containerized Data Importer deployment logs. * Reports include YAML snapshots of all importer-associated pods across namespaces, regardless of pod phase or deployment probe results. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
c355de63ed |
fix(backups): exec into a CLI Pod in the nats demo
nats_cli seeded and verified JetStream streams through a throwaway `kubectl run -i` Pod per call, with the same attach-race empty-read the redis demo fixed. NATS ships no long-lived Pod carrying the `nats` CLI, so run one nats-box Pod and `kubectl exec` into it; stream_message_count keeps the exec's stderr so a failed read says why. The password travels in NATS_PASSWORD on stdin rather than the --server URL the exec request records in the apiserver audit log. The Pod carries cozystack.io/backup-demo=nats: nats_cli_pod reuses only a Ready Pod the demo owns, replaces a leftover in a terminal phase, and refuses a same-named Pod it does not own; cleanup.sh reaps by that label with --grace-period=1. Unit tests under hack/ pin the exec path, the preserved stderr, and the password staying out of argv. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
9cea7dcd90 |
fix(backups): exec into a CLI Pod in the kafka-metadata demo
kafka_run seeded and verified topics through a throwaway `kubectl run -i` Pod per call. That Pod's stdout returns over an attach that only carries what the container writes after the attach registers, so a CLI that finishes first comes back empty with exit 0 -- the empty-read defect fixed for the redis demo. Run one long-lived CLI Pod and `kubectl exec` into it: an exec'd process owns its pipes, so its output cannot be lost that way, and stderr stays attached (the topic_meta / topic_partitions reads keep it) so a failed read says why. The Pod carries cozystack.io/backup-demo=kafka-metadata: kafka_cli_pod reuses only a Ready Pod the demo owns, replaces a leftover in a terminal phase, and refuses a same-named Pod it does not own; cleanup.sh reaps by that label with --grace-period=1. Unit tests under hack/ pin the exec path -- a throwaway run Pod or a re-added stderr redirect reddens them. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
9c5c5f9d4e |
fix(ci): overlay the right line's refs for a PR on a feature branch
The base-branch overlay pulled cozystack-packages:<base_ref>. A stacked PR's base is a feature branch such as fix/foo: the slash is not valid in an OCI tag and no artifact is published for it, so the pull failed, the step only warned, and every package the PR did not rebuild ran its committed release ref. With a committed ref older than its chart, that broke E2E (Grafana plugin 404). main and release-X.Y still read their own artifact. Any other base reads the artifact of whichever of main and the release lines it has the fewest commits on top of. Always picking main would put main's images under a release line's charts for a stack rooted on that line, the cross-generation mix that fails install. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ab0c9f6e29 |
docs(e2e): document jq requirement for bucket suite (#4457)
<!-- 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 `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does Documents `jq` as a host prerequisite for the `bucket` Chainsaw suite, whose command steps use it to parse S3 responses. Fixes #4295 ### Screenshots N/A — documentation-only change. <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note docs(e2e): document jq as a host prerequisite for the bucket Chainsaw suite ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated the local-run prerequisites for the `bucket` suite to include `jq`, alongside `mc` and `nc`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
564f540f46 |
fix(kubernetes): render the OIDC UID header only from v1.33 (#4407)
## What this PR does Stacked on #4398; review the diff against `fix/kamaji-pin-hash-reset`. The tenant Kubernetes chart now renders `--requestheader-uid-headers=X-Remote-Uid` only from an effective Kubernetes version of v1.33, where `RemoteRequestHeaderUID` is beta and on by default. It no longer renders a `--feature-gates` term of its own on any version. On v1.32 it used to add a term enabling that gate, which is alpha there, whenever it could read the operator's entries and they left the gate unset; no release tag contains that path, so dropping it now changes nothing a user has run. The chart still reads `--emulated-version`, because emulation moves the effective version: `--emulated-version=1.32` on a v1.35 tenant puts the gate back to alpha-and-off, and the header flag is not rendered there. It still reads the operator's `--feature-gates` entries for an opt-out (`RemoteRequestHeaderUID=false`, or `AllBeta=false` with no explicit `=true`), so the header flag never lands next to a disabled gate. `AllAlpha` is no longer read, since it says nothing about a beta gate. An unquoted `--emulated-version=value` entry whose kube version has a non-zero patch component now fails the render and names the entry. The rule mirrors upstream rather than the regex in #4396: `toVersionMap` parses each value with `version.Parse` and refuses only `ver.Patch() != 0` (`staging/src/k8s.io/component-base/compatibility/registry.go` lines 320-326 at v1.35.0, the same code in `staging/src/k8s.io/apiserver/pkg/util/version/registry.go` at v1.31.0). `version.Parse` takes `^\s*v?([0-9]+(?:\.[0-9]+)*)(.*)*$` (`versionMatchRE` in `k8s.io/apimachinery/pkg/util/version/version.go`, identical at v0.31.1 and v0.35.0). So `1.32`, `v1.32`, `1.32-rc`, `1.32.0` and `1.32.0-rc` all start an apiserver emulating 1.32, and `1.32.4` and `1.32.4-rc` do not. A `^[0-9]+\.[0-9]+$` check would have refused the first four. The chart extracts the numeric run with the same pattern, reads the minor from it, and fails only on a non-zero third component. A quoted value is still left unread. Two deliberate narrowings: - The chart no longer refuses a mix of bare and `kube:`-prefixed `--feature-gates` terms. Once the chart writes no gate term, the spelling of the operator's terms no longer concerns anything it renders. The apiserver still refuses the mix (`component-base/compatibility/registry.go:405` at v1.35.0), and the chart now passes it through like the other fatal gate values it already leaves to the apiserver: unknown names, unparseable values, quoted terms. - On v1.32 the chart now neither renders nor judges the UID flags, as it already did on v1.31. An operator's own `--requestheader-uid-headers` entry without the gate reaches an apiserver that refuses it, which is how every released chart version behaved. A cluster built from `main` that already runs a v1.32 tenant with OIDC on loses the chart's `--requestheader-uid-headers` and gate term on the next render; no release carried them. That holds only while no release is cut from `main` before this merges; a release that carries the v1.32 gate term first would turn this into a breaking change for v1.32 tenants, whose migration is to write both flags into `extraArgs` themselves. An element such as `kube=1.32=x` is now split at its first `=` only, the way `toVersionMap` splits it with `SplitN(compVer, "=", 2)`. The chart used to skip that element, which the apiserver reads as 1.32, and so rendered the header flag against an emulated 1.32. That gap predates this branch. Rebased onto #4398 at `fb2e57c3b`, which reads a two-element `--feature-gates` and `--emulated-version` separately. That behaviour carries over unchanged under the v1.33 floor: behind a hidden `--emulated-version` only an explicit `RemoteRequestHeaderUID=false` is refused beside a header entry, while a blanket `AllBeta=false` waits for a reading, because a lower emulated version can make the gate alpha again; nothing is refused as an opt-out behind a hidden `--feature-gates` value; the empty-element refusal applies behind both; and behind either one `X-Remote-Uid` is added only beside a uid-headers list of the operator's. The README breaking-changes note from #4398 now says the chart refuses neither the mixed gate spelling nor two kube emulated versions. In `docs/oidc-tenant.md` the paragraphs that list refusals now name the managed-flag move and refusal from #4398 and no longer carry a count, and the two-element paragraph now says what the chart renders behind a hidden value. Tests: `helm unittest` goes from 308 on #4398 to 298. 26 cases are deleted. 25 of them pinned the v1.32 gate term, `AllAlpha` handling or the mixed-spelling refusal. One emulated a version below 1.32, which the below-floor emulation case now covers. v1.32 cases that tested parsing rather than the gate are retargeted to v1.34. Sixteen cases are added: the patch-level refusal, the version parsing below the floor and above it (on v1.35 emulating `1.33-rc`, `v1.33`, `1.33.0` and `kube=1.33=x`, where only a correct minor renders the header), the first-`=` split, a pin that a mixed gate spelling now passes through, emulation landing exactly on 1.33, and a v1.32 opt-out beside a header entry passing through untouched. Coverage notes, not applied: no case pins that the patch-level refusal stays off with `spec.oidc.mode: None`, where the block that holds it does not run. The case "splits a component element at its first equals sign only" pins that the kube element of `kube=1.32=x` is read. It does not pin the split itself, because the version pattern ignores what follows the numbers either way. The `extraArgs` field description still calls the field the escape hatch for apiserver flags unrelated to OIDC. It is unchanged here, and it stays accurate for the v1.32 advice in the guide, because the UID flags belong to the aggregation layer rather than to OIDC. Optional, not applied: on v1.32 the chart could refuse an operator's `--requestheader-uid-headers` entry that comes without the gate, since the gate is off by default there and the apiserver will not start. The same skip also drops two refusals the branch history had at 1.32, a header entry beside `RemoteRequestHeaderUID=false` and a header list ending in a comma; both now reach an apiserver that refuses them. It stays out so that v1.32 matches v1.31 and the released charts, where the chart judges none of these flags. Not applied: for a value with a zero-prefixed major such as `01.32.4`, the patch refusal suggests `01.32`. The apiserver refuses both values over the leading zero, so the configuration fails either way and only the suggested fix is off. Why the patch refusal stays while the mixed-spelling refusal goes: the patch refusal is about `--emulated-version`, which the chart reads itself to decide what to render. The mixed spelling concerns `--feature-gates`, which the chart no longer writes to. Not split out: the `--emulated-version` parsing and the patch-level refusal share the fix commit with the floor move, because both change how the same value is read. The change also removes a comment from `hack/e2e-chainsaw/_lib/run-kubernetes.sh` that became false, so select-e2e runs the full suite for this PR. Default path: renders of the parent and this branch through `tests/values/common.yaml` and `tests/values/oidc-system.yaml` at v1.31 to v1.35 are byte-identical, once the randomly generated Kamaji token fields are masked, except for OIDC System at v1.32, which loses exactly the two UID lines. ### 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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The only field that changes is the description of `version`, which returns to its earlier wording. The website's kubernetes reference page is regenerated from the README on the next tag, and the Terraform provider keeps its own copy of that description, which already reads "Kubernetes major.minor version to deploy.". Closes #4373 Closes #4396 ### Release note ```release-note fix(kubernetes): the tenant chart renders the aggregation-layer UID header flag with OIDC on only from an effective Kubernetes v1.33, and never renders a --feature-gates term of its own. v1.32 tenants render no UID flags, which matches every released version, so nothing regresses. This supersedes the v1.32 clause of the #3539 release note. An unquoted --emulated-version entry with a patch-level kube version such as 1.32.4, which the kube-apiserver refuses, now fails the render and names the entry. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OIDC-enabled clusters on Kubernetes 1.33 and later now receive the UID request-header setting without an added feature-gate flag. * Chart rendering now checks Kubernetes and Talos version compatibility and prevents rendering when worker deployments do not meet ownership or retention requirements. * **Bug Fixes** * Emulated Kubernetes versions with non-zero patch components now fail with a clear error; supported version formats are recognized more consistently. * **Documentation** * Updated version and OIDC guidance to reflect current chart behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
9e3ed1f2cb |
fix(backups): stop the redis backup demo from reading empty values (#4399)
<!-- 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 `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The redis backup round-trip suite went red on PRs that do not touch redis, each time on an empty read (#4395). The report snapshots of those runs point to two separate causes, and I fix both here. The empty marker reads came from the demo helper that ran redis-cli in a throwaway `kubectl run -i --rm` Pod. kubectl gets that Pod's stdout over an attach, and containerd's attach only carries output written after the attach is registered. kubectl falls back to the Pod log only when the attach fails. A redis-cli that finishes in that gap comes back as an empty string with exit 0. In the failed runs, the master's log shows the key present, and sentinel logged no failover, yet the GET read ''. `redis_cmd` now runs redis-cli by `kubectl exec` in a sentinel Pod, which already has it. An exec'd process gets its own pipes, so its output can't be lost that way, and its exit code comes back. The password now goes over stdin rather than a Pod env var. The missing-strategy failure was real: `cozy-default-redis` did not exist yet. It gets rendered behind a lookup of the platform bucket, so it appears only after a forced Helm upgrade. helm-controller holds that upgrade while `cozystack-basics` is not Ready, and the e2e install's late platform patches keep it not Ready for several minutes. The demo read the object once. It now waits for the object with `kubectl wait --for=create` and a 15 minute bound. That wait fails right away on anything other than NotFound. The demo's reads also stop sending stderr to `/dev/null`, so the next failure names its cause. `hack/redis-backup-demo-helpers.bats` covers `redis_cmd` with stubbed kubectl and redis-cli: it reads through exec, keeps the password out of argv, sends no AUTH when there is no Secret, and fails loudly when the exec fails or sentinel has no master. Closes #4395 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> No UI changes. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `examples/backups/redis/` and a new unit test under `hack/`. No entry in the trigger map covers either. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(backups): the Redis backup demo reads its keys through `kubectl exec`, so a short-lived Pod can no longer drop its output, and it waits for the platform's `cozy-default-redis` strategy to appear instead of failing on its first read. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Redis backup and restore commands now report Kubernetes and Secret read errors instead of hiding them, while still treating a missing password Secret as empty. * Redis commands pass authentication securely and fail clearly when Sentinel cannot identify a master. * Backup demos wait for the Redis backup strategy to appear and provide clearer error messages. * **Tests** * Added coverage for Redis authentication handling, command failures, and missing Sentinel masters. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
nightcityblade
|
fdcbc5c83c |
docs(e2e): document jq requirement for bucket suite
Assisted-by: LLM Signed-off-by: nightcityblade <nightcityblade@gmail.com> |
||
|
Alexey Artamonov
|
f02ca8cbc2 |
test(backups): log in as an application user after each restore
The backup examples and e2e suites no longer set passwords. They read the generated ones from <release>-credentials with jq, since a username may contain a dot that jsonpath treats as a path step, and log in as an application user after each restore, so a restore that leaves the credentials unusable fails the run. The MariaDB run also checks that an account created out of band on the source is absent from the copy. The unreachable-target restore step asserts a terminal Failed attributed to the recovery window, since the reason itself is best effort. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Myasnikov Daniil
|
9faa18a239 |
test(hack): detect BATS suites without a runner
Nested monitoring suites remained unexecuted after their runner was removed. Audit both runner inventories so missing invocations and stale suite paths fail in the unit lane. Make-variable arguments must be reported even when their values hide the file suffix. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
a92e89e969 |
docs(kubernetes): describe the OIDC UID header from v1.33
The tenant OIDC guide, the version field description and the e2e helper comment still described a --feature-gates term rendered on v1.32. Left as they were, they would tell an operator that the chart switches that alpha gate on, which it no longer does. The guide now says what a v1.32 tenant has to write itself to get the UID, and which --emulated-version values fail the render, so an operator who meets that refusal finds it explained. The breaking-changes note in the README said the chart refuses the mixed gate spelling while OIDC is on; with that refusal gone it would promise a render error that no longer comes. The version description returns to its earlier wording in values.yaml and in the update-versions.sh template, because that template rewrites the line on every version bump and would otherwise bring the old text back. make generate carries it into the schema, README, API types and ApplicationDefinition. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
43cbcf9f46 |
fix(kamaji): anchor the ignore patterns to the chart root
Helm skips .helmignore when it reads a chart out of an archive and honours it when it reads one out of a directory. A pattern with no slash is matched against the base name at any depth, so `hack` reached charts/kamaji-crds/hack, where the CRD templates keep the schema they pull in with .Files.Get. The cluster, which gets the archive, has always had the full CRDs; every local render, lint and package produced three CustomResourceDefinitions with an empty spec, and no check that runs before a cluster sees the chart could have caught a schema defect. There is no hack directory at the top level of this package, so the pattern excluded that subchart and nothing else. `images` is anchored alongside it against the same trap; it matches only the intended directory today and the packaged file list is unchanged by it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
c55ab268e2 |
test(monitoring): remove orphaned OIDC BATS suites
The two pre-Chainsaw monitoring OIDC suites have had no invocation site since the app suite migrated to Chainsaw. Remove them instead of carrying dead tests that force the selector to run every live suite. Keep the existing selector and capture guards for the historical path shape, and update their comments now that the directory is empty. Retain a named chart assertion for the no-KeycloakRealmGroup ownership invariant; the existing count and kind assertions already enforce the same behavior. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
95ece888ba |
feat(kafka): add topic-data backup/restore example and e2e roundtrip (#3580)
## What this PR does
Adds a worked backup/restore example for a Cozystack-managed `Kafka`
application's **topic data**, built on the generic `Job` backup strategy
— the same pattern as `examples/backups/nats`, with no purpose-built
backup image. A stock Strimzi Kafka image (the `kafka-*.sh` CLI plus
`bash`, `curl` and `tar`) runs one shell script that branches on `{{
.Mode }}`: on backup it freezes each partition's end offset and drains
`[begin, end)` to a tarball it `PUT`s to S3; on restore it `GET`s the
tarball, recreates the topics with their original partition count and
replication factor, and replays every partition file. S3 access is `curl
--aws-sigv4`, verified against the endpoint's CA projected from the
`<app>-backup-s3` Secret, so no extra client image is needed.
The consistency model is a frozen-end-offset cut: `kafka-get-offsets
--time -1` is read once per partition at backup start and the consumer
drains only up to that offset, so anything produced during the run is
excluded and every partition of a topic is captured as of the same
instant — a cut by log position, independent of record timestamps.
This is deliberately a DATA-only backup. Topic configs, ACLs, SCRAM
users and **consumer-group offsets** are not captured, and re-produced
records receive fresh offsets — the README spells out the scope and
points at a volume-snapshot strategy for a faithful, offset-preserving
backup. Keyed records reproduce into their original partition because
the topic is recreated with the same partition count and the default
(murmur2) partitioner is deterministic.
The script fails closed rather than reporting a partial backup or a
lossy restore as success: it refuses an offset listing short of the
topic's partition count, a drain short of `end - begin`, a `topics`
value with a duplicate or a name outside Kafka's charset, a manifest
that disagrees with the data files, a target topic that already holds
records (checked before the replay, so a retried Pod cannot append a
second copy), a pre-existing topic with a different partition count or
replication factor, a replication factor below the target's effective
`min.insync.replicas`, and a post-replay count that does not match the
manifest. Each topic's replication factor is recorded in the manifest
and restored as captured; the `replicationFactor` BackupClass parameter
overrides it for restoring into a smaller cluster.
Also adds a `kafka-2-backup-roundtrip` Chainsaw test to
`hack/e2e-chainsaw/kafka`, which drives the example `run-all.sh` as its
harness (Bucket → two-broker source Kafka with a seeded 3-partition,
2-replica topic plus a decoy topic that the demo topic matches as a
regex but not as a literal → BackupJob → in-place restore with count,
content and replication-factor assertions → a second RestoreJob into the
populated topic that must be refused with the topic untouched →
two-broker target Kafka + to-copy restore + verify), mirroring
`mariadb-2-backup-roundtrip`. Two brokers, not one, because that is the
shape on which the chart sets `min.insync.replicas: 2` and a restore
with the wrong replication factor cannot land a record. It resolves the
Kafka image from the running operator so the backup Pod reuses the image
already cached on the nodes.
The strategy script itself is pinned by
`hack/kafka-topic-backup-strategy.bats`, on the shape of
`hack/kafka-backup-script.bats`: it renders the strategy through a stub
`kubectl` exactly as step 01 applies it, lifts the container body out,
and executes it against stubbed `kafka-*.sh`, a stubbed `curl` and a
directory standing in for S3, with fault injection for the broker
behaviours the guards exist for. Fourteen cases, one per guard; removing
the partition-listing guard reddens one of them.
Verified: `chainsaw/kafka-2-backup-roundtrip` passed in this PR's CI on
the two-broker shape (record count, byte-identical content dump and
replication factor 2 asserted after both restores, the refused
RestoreJob settled Failed with the topic untouched);
`hack/kafka-topic-backup-strategy.bats` passes under both
`hack/cozytest.sh` and bats.
Files:
- `examples/backups/kafka/` — 14 files (`00-helpers.sh`, `01`..`07`,
`run-all.sh`, `cleanup.sh`, `README.md`, `90`/`91`/`92` scenario
walk-throughs).
- `hack/kafka-topic-backup-strategy.bats` — hermetic test of the
strategy script.
- `hack/e2e-chainsaw/kafka/chainsaw-test.yaml` — new
`kafka-2-backup-roundtrip` Test appended.
- `hack/backup-access-preflight.bats` — the kafka example and Test join
the S3-preflight roster.
- `hack/select-e2e.sh`, `hack/e2e-chainsaw/.chainsaw.yaml` —
comment-only: their enumerations of preflight-gated suites are replaced
by a pointer.
- `.gitignore` — the example's `.source-dump.txt` snapshot.
No package sources (`values.yaml`, `values.schema.json`, `Chart.yaml`,
package `README.md`) are touched, so `make generate` produces no diff.
### Screenshots
N/A — no UI changes.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` file-by-file
against the diff. The change adds example scripts under
`examples/backups/kafka`, one unit test under `hack/`, and appends one
Chainsaw Test to an existing file under `hack/e2e-chainsaw/kafka` — it
adds no package under `packages/apps` or `packages/extra`, changes no
CRD, no `ApplicationDefinition`, no `values.schema.json`, no
platform/installer values, no `hack/*.mk` anchor or make-target
behaviour, and no node prerequisites in `hack/e2e-prepare-cluster.bats`.
None of the downstream triggers match.
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
feat(kafka): add a topic-data backup/restore example and e2e round-trip on the generic Job backup strategy (data-only: topic records with a frozen-end-offset consistency cut and partition-preserving replay; consumer offsets, topic configs, ACLs and users are out of scope)
```
|
||
|
myasnikovdaniil
|
9bec21461d |
feat(kubernetes): boot tenant worker disks from a shared golden Talos image (CDI clone) (#4171)
## What this PR does Tenant worker boots from talos OS disk image that CDI streams over HTTP from image factory, and it does that once per worker. Same bytes N times, and N chances for a fetch to fail. In closed or rate-limited environment that is wrong shape. This PR adds opt-in catalog and per-pool selector that reads from it. `packages/system/kubernetes-worker-image` imports one golden DataVolume into `cozy-public` per `(schematicID, version)`, named `talos-worker-<schematicID>-<version>`. Pool opts in with `osImage.builtin` on its own `KubernetesNodes`, and its `disk-system` DataVolume then carries `source.pvc` pointing at that golden, so CDI does storage layer CSI clone instead of HTTP import. One import per flavor, shared by every worker of every tenant. Catalog is disabled by default, enable by adding `cozystack.kubernetes-worker-image` to `bundles.enabledPackages`. Its knobs (`imageFactoryURL`, `storageClass`, `storage`, `images`) are set through `spec.components.platform.values.kubernetesWorkerImage` on cozystack-platform Package. That needed optional package helper to learn components argument, which `cozystack.platform.package.optional.default` did not have, so every optional package registered through that wrapper could name variant and nothing else. Thirteen other packages get same path for free. `osImage` has two mutually exclusive forms: - `osImage.builtin` boots pool workers by CDI-cloning golden from catalog. Clone is storage layer CSI copy, so OS image never crosses pod network and factory answers once for whole fleet instead of once per worker. - `osImage.factory` keeps HTTP import and lets pool pin own `imageFactoryURL`, `schematicID` and `version`. This is bring-your-own path for operator running own factory or caching mirror, and it is what pool gets when `osImage` is omitted. In-guest talos installer written by reconcile job follows same resolution, so pool that pins release boots it and upgrades into it instead of being flipped back to cluster default halfway. This is pool half of what #3950 asks for. That issue wants platform level defaults for two channels, worker OS image and in-guest registry mirrors. Here image channel gets per-pool source and shared cache, platform default plumbing is left to the issue. ## Existing workers do not roll With `osImage` omitted rendered `KubevirtMachineTemplate` is byte identical to what chart produced before. KMT is named by hash of its own content, so any drift would re-image every worker VM in fleet on next upgrade. Stored render snapshot is untouched by this PR, that is the evidence and not the claim. Editing `osImage` on existing pool re-images that pool, like changing any other image reference. ## What fails at render Clone that would silently degrade into something slower or wrong is refused instead of rendered. Golden absent, or its import in terminal `Failed` phase. Pool `storageClass` different from golden one, because CDI cant CSI-clone across storage classes and falls back to host assisted copy over pod network, which is exactly the transfer `osImage.builtin` removes. Empty class on either side resolves against cluster default class before comparing, so pool at `storageClass: ""` is checked too. `diskSize` below golden storage, because clone target cannot be smaller than source. Talos release the matrix lists as unsupported, whether pool reached it through `talos.version` or through `osImage.*` override. Matrix passes talos minor it does not list, so what it promises is that known-bad pairing is refused, not that every pairing is known. `schematicID` that is not plain lowercase alphanumeric, or version that is not `vMAJOR.MINOR.PATCH`, both are interpolated into DataVolume name and image URL while `values.schema.json` types them as bare strings. Golden that is only mid-import is not an error. CDI holds worker disk until source populates, and failing there would abort whole pool HelmRelease for length of a download. Same reasoning covers pool that already cloned: absent or `Failed` golden fails render only for pool still waiting for its first disk, otherwise reconcile of healthy pool would abort over source it no longer reads. Golden that declares no StorageClass is read from its PVC, not from whatever default is in force today, because those differ once cluster default moves. Cluster with two default classes resolves same way Kubernetes does, newest wins. Pool that already cloned is identified by annotation naming exactly that pool, not by disk name prefix, because pool `md0-gpu` produces disks carrying `md0` prefix, and it reads its own class from its own bound PVC, so admin moving `storageclass.kubernetes.io/is-default-class` does not fail release of pool whose disks are all on right class. That resolved class is then written onto worker disk. Reading one class and binding another is how pool at empty `storageClass` passed the comparison and still took host assisted copy on its next worker, so guard and disk now come from one value. HTTP path leaves empty class to cluster as before, which is what keeps its render byte identical. Catalog side refuses duplicate `(schematicID, version)` entries, entries whose schematic or version cannot produce name pool can reconstruct, and edit to `storage`, `storageClass` or source URL of entry whose golden already exists. Last one is not patchable in place, so without guard it lands as rejected patch on next upgrade. Goldens carry `helm.sh/resource-policy: keep`, so dropping entry stops managing golden instead of deleting it. Missing golden aborts whole Kubernetes HelmRelease of every tenant pinned to it, not just the disk, and each cloned worker disk annotates golden it came from, so "does anything still clone this" is answerable before manual delete. Three drift guards live in `hack/check-worker-image-catalog-defaults.bats`, because values that have to agree sit in files nobody edits together. One-sided talos bump trips all three. ## Upgrade Three pool values that never produced working image rendered before and hard-fail now, so pool carrying any of them flips its HelmRelease to `InstallFailed` on upgrade and not on next edit: `talos.version` in `vMAJOR.MINOR` form, `schematicID` that is not plain lowercase alphanumeric, and empty `talos.imageFactoryURL`, which used to render schemeless `/image/...` source URL. None of the three could boot worker, so this is failing earlier and not a regression, but it surfaces on pool nobody touched and fix in each case is to correct the value. CR carrying such value is admitted by API with value intact, and later unrelated edit reaches `.spec.values` of HelmRelease while it stays `InstallFailed`, so edit is taken everywhere and applied nowhere. ## Why e2e does not exercise the clone path Clone is storage layer copy, so it needs StorageClass whose volumes are reachable from any node. Merge-gating e2e has none: those lanes run on talos containers, which share runner kernel and cannot load DRBD, so node-local `local` is only class there. Nothing would arrange the co-location such a class requires either, golden in `cozy-public` has no consumer and binds where CDI importer landed, while worker disk binds where its VM is scheduled. So the suites keep importing over HTTP with `osImage` omitted, which is also the render the no-roll guarantee rests on. This PR touches no e2e harness file: `hack/` differs from main only by the two guards this feature owns, one tying catalog default golden to pool chart talos and StorageClass defaults, one on in-guest installer heredoc. Switching suites over does not work on that substrate. Golden asks for `replicated`, lane does not have it, CDI resolves no StorageProfile, and catalog HelmRelease times out on DataVolume stuck in `ImportInProgress`. Passing lane own class would fix the class and leave the co-location. Nightly and tag build run same suites on QEMU with DRBD and `replicated`, which is the substrate clone path was verified on. That is where e2e coverage belongs, as its own change with that lane storage headroom measured rather than assumed. Worth being straight about what this leaves, because it bears on whether the feature earns its place. The e2e argument was never the strong one: golden fetches from image factory exactly once, and #3513 established that node-join stall tracks host `kvm_amd` `vgif` flag rather than registry egress. What clone genuinely removes is the fanout, N workers streaming over pod network becomes N storage layer copies. That is property of storage layer, so it is worth having where DRBD is and worth nothing where it is not. This ships as production capability with the constraint stated on the field, not as CI improvement. ## Testing On this tree: - full `make unit-tests` plus `make test-controllers`: 95 charts, 2520 helm-unittest cases, 1710 bats cases, `go test ./pkg/...` and `./internal/...`, exit 0 - `pre-commit run --all-files`: exit 0, root `make generate` and per-app generate both clean, so no generated-file drift - render snapshot unchanged, which is what pins byte identical render for pool that does not adopt `osImage` - mutation pass over render guards, each term deleted or inverted in turn and suite required to notice it by name. Restoring presence gate on catalog StorageClass comparison reddens exactly one case, inverting its `ne` to `eq` reddens five. Reverting worker disk to omit its resolved class reddens three cases that pin where builtin disk binds, and forcing that class non-empty on every path reddens HTTP-path case and all three render snapshots, with snapshot diff naming the changed KMT hash `E2E Tests` passed on run 35831830568. That run was on the previous head, 4fd8e7ba, and this head differs from it only by one test comment and the commit message. ## Verified on a live cluster Throwaway three-node talos stand (OCI, talos v1.13.6, LINSTOR/ZFS, `replicated` StorageClass) installed from this branch `packages/` tree. This covers what neither `helm template` nor unit suites can answer. It predates the change that writes resolved StorageClass onto worker disk, and that stand ran with explicit `replicated` on both sides, which is the value that change now renders. - The catalog wires up from `bundles.enabledPackages` alone: `PackageSource cozystack.kubernetes-worker-image` reconciled, its HelmRelease installed, and the golden `talos-worker-<schematic>-v1.13.6` imported to `Succeeded` in 108s (6Gi, RWX, `replicated`). - A pool on `osImage.builtin` clones it on the storage layer. Both worker DataVolumes went `CSICloneInProgress` then `Succeeded` in 43s with `Running=False reason=Completed message="Clone Complete"`, carrying `source.pvc` and no `source.http`, and no importer or upload Pod appeared in the tenant namespace. - The clone yields a bootable talos root disk. Both worker VMs reached `Running`/`Ready`, joined the tenant cluster and went `Ready` about 66s after creation, and the tenant cilium, coredns, csi, ingress-nginx and metrics-server addons all installed on them. - Access modes match across the hop: golden `ReadWriteMany` 6Gi to worker `ReadWriteMany` 20Gi on the same `replicated` class, grown by CDI. - The written annotations are correct in practice: each worker disk carries `kubernetes-worker-image.cozystack.io/golden` naming the golden and `.../pool` naming its pool. - helm-controller converges on a builtin pool: forcing a reconcile left the content-hashed KMT name unchanged and the VMI and DataVolume UIDs identical, so a reconcile does not roll the pool. - Cross-namespace clone RBAC is in place as shipped: `create datavolumes --subresource=source -n cozy-public` resolves to `yes` for a tenant ServiceAccount through the `cdi-clone-dv` RoleBinding, and CDI runs with `cloneStrategyOverride: csi-clone`. This PR adds no RBAC of its own. ### Screenshots Not applicable, no UI change. ### Downstream repositories Trigger map points at two repositories. Provider models every pool field by hand, so `osImage` needs schema, model and expand/flatten pair there. Already tracked as cozystack/terraform-provider-cozystack#39, filed when the field was still called `image`. `content/en/docs/next/operations/configuration/platform-package.md` is a hand-written table of `spec.components.platform.values.*` keys and nothing generates it, so `kubernetesWorkerImage` needs a row there. Filed as cozystack/website#707. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/707 - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/39 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(kubernetes): tenant worker pools can boot their system disk from a shared golden talos image instead of importing it from the Image Factory once per worker. Enable the opt-in `cozystack.kubernetes-worker-image` package via `bundles.enabledPackages` to publish one golden DataVolume per `(schematicID, version)` in `cozy-public`, then set `osImage.builtin` on a `KubernetesNodes` pool to have CDI clone it on the storage layer. A pool that omits `osImage` keeps importing over HTTP and renders exactly as before. The clone path needs the pool and the golden on the same StorageClass, and one whose volumes are reachable from any node, in practice a replicated/DRBD class. ``` |
||
|
Aleksei Sviridkin
|
d918887aac |
fix(e2e): collect the CDI controller log and CDI pods in cozyreport
CDI deletes an importer pod once, right after it marks the PVC import Succeeded, and never retries that delete. When that one delete does not land, the Completed pod keeps pvc-protection on the PVC and a later DataVolume delete hangs. The report had no evidence for why: the broken-pod walk skips Completed pods and the cdi-deployment log was never read. Read the cdi-deployment log (current and previous) and every pod carrying CDI's app label as YAML, across namespaces and phases. Only the log reads wait on the cdi-deployment probe: an importer pod is owned by its PVC, so it can outlive the Deployment, and a missing controller must not also drop the pod it left behind. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
1f30ac4598 |
fix(tests): let the mariadb waits cover a first boot (#3769)
## What this PR does Closes #3741 Several waits for a database HelmRelease to become Ready gave up before the release could: the mariadb, mongodb, rabbitmq and clickhouse backup walkthroughs waited 300 seconds, and both mariadb Chainsaw suites gave their HelmRelease-ready assert 6 minutes. A generated release carries a 10-minute install timeout (`pkg/cmd/server/start.go:91`), so a failed install was reported as a bare timeout, because the release had not yet recorded why it failed, and for mariadb a first boot the startup probe was still willing to wait for failed the harness instead. A wait on such a release has two floors and has to beat the larger. The first is the app's own first boot: cozystack-api leaves `spec.waitStrategy` unset and helm-controller defaults it to `poller`, so the install waits through kstatus, and kstatus maps the operator CR's `Ready=False` onto `InProgress`. For mariadb that boot may take the chart's 310-second startup budget (`startupProbe.failureThreshold: 30` in `packages/apps/mariadb/templates/mariadb.yaml` with the operator's probe defaults of 20 and 10); mongodb, rabbitmq and clickhouse set no startup probe. The second is the install timeout. At or below it the waiter is the one that expires, because its clock starts when the step runs and the install's only once the Application has been reconciled and the chart artifact fetched. The install timeout binds in every case here, so the walkthrough waits go to 660 seconds and the mariadb asserts to 11 minutes. Raising a wait inside a walkthrough spends the Chainsaw op that runs the script, and an op that runs out SIGKILLs the process group and loses the script's error. Each op therefore grows by exactly what the waits it reaches gained: mariadb's round-trip from 30 to 42 minutes, rabbitmq's from 30 to 42, mongodb's from 30 to 36 (its round-trip runs with `SKIP_RESTORE`, so only the source wait counts), and clickhouse's from 60 to 72, which still sits above the sum of every inner ceiling. That bounds a failure at a single wait, not a run that is slow at every stage and then fails late; bounding that needs a phase budget, not a larger literal. postgres keeps its 360-second waits and every walkthrough keeps its 300-second bucket wait: whether a suite pays the extra minutes on every red run is a per-suite call. The mariadb endpoint asserts keep their job. A freshly applied CR carries no conditions and kstatus reads it as current, so the install wait can end before any pod is up; an endpoint address closes that window. That is also why the `wait_for_field` on the CR stays in the walkthrough. `wait_hr_ready` now dumps every condition and the release history on timeout and on `Stalled=True`, instead of the single Ready message a scheduled retry can overwrite before anyone reads it. It also stops printing one `(none recorded)` line for different answers: a release that never appeared, a release deleted during the wait, a lookup kubectl could not answer, a failed history read and an empty history each say so, with kubectl's own stderr kept, and a release seen earlier still gets its dump when only the last lookup failed. The change goes to every byte-identical copy of the helper, in `examples/backups/{clickhouse,mariadb,mongodb,postgres,rabbitmq}/00-helpers.sh`, and they stay identical. `hack/mariadb-first-boot-waits.bats` pins the mariadb budgets against the tree. It derives both floors from their sources instead of pinning literals, so raising `failureThreshold` past the install timeout, or changing the install-timeout default, reds it; each source has a test that refuses to proceed when it cannot be read, including the ApplicationDefinition, whose two override annotations would each invalidate the derivation. It holds the round-trip op at a fixed remainder over the ceilings up to and including the source wait, bucket stage and S3 preflight included, and at a fixed slack once both mariadb waits are paid, so raising a wait without the op reds. The extractors are also tested against fixtures, because the tree cannot tell a correct extractor from a broken one. Beyond mariadb, the file runs every exit branch of `wait_hr_ready` in each copy against a stubbed kubectl, and pins the copies byte-identical. The population is every definition in the tree found by grep, minus two named exceptions held to still differ, so a new copy joins without editing the test and an exception whose copy converges goes red: - `examples/backups/redis/00-helpers.sh` is written in the compact one-line style of the rest of that file, so converging it rewrites its shape rather than applying the shared body. - `examples/backups/kafka-metadata/00-helpers.sh` is a helper of its own with the same diagnostic gap on timeout (it prints no reason at all), and closing that is a change to kafka-metadata. `docs/agents/e2e-testing.md` replaces the reviewer checklist's 5-6m standard and its list of exceptions, which the tree had outgrown, with the floor a wait has to beat and the rule that a longer wait carries its reason in-line. The derivation gets a section of its own, opening with the rule in one line. For a release an app chart renders itself the floor is `spec.install.timeout`, then `spec.timeout`, then helm-controller's 5m default, and the nested `<name>-system` releases never take cozystack-api's 10m. The harbor comments name the floor each of their asserts sits against, and the opensearch comment stops citing the removed standard. Adding `hack/mariadb-first-boot-waits.bats` does not escalate the E2E selection: `hack/[^/]+\.bats$` sits in `inert_config_pattern` in `hack/select-e2e.sh`, and `full_suite_pattern` escalates only `hack/e2e-*.bats`. On this changeset `hack/select-e2e.sh` selects `clickhouse harbor mariadb mongodb opensearch postgres rabbitmq`. What this costs is paid only on a failing run; the raised numbers are ceilings. Summing the timeouts stated on each suite's own steps, mariadb goes from 92m to 114m, clickhouse from 116m to 128m, rabbitmq from 67m to 79m and mongodb from 95m to 101m, before the untimed steps that fall back to the defaults in `.chainsaw.yaml`. clickhouse is the most expensive suite by that sum. All of them compete for the 215m `timeout-minutes` of the whole `e2e` job, which also covers image download, sandbox preparation and the Cozystack install, and a job cancelled at the cap loses both its log and `cozyreport.tgz`. This does not fix the failure that led here. In the run that prompted the investigation the mariadb replica never opened port 3306 at all, spending its whole 310-second budget inside `mariadb-install-db` before being killed, and no wait budget would have saved it. That defect is #3744. What this fixes is a second, independent defect found while reading the same code. One thing here is argued rather than measured: the mechanism is checked against helm-controller and kstatus, and every number is derived from the tree, but no run ID pins that it is what any particular red run hit. Both floors justify 660s whatever the cause was, so being wrong about the cause leaves slack rather than breakage. ### Not in this PR - This PR retires the mariadb, mongodb, rabbitmq and clickhouse walkthrough waits and the two mariadb asserts. The rest is tracked in #4404: the other Chainsaw HR-Ready asserts under their floor, the harbor asserts, the 300-second bucket waits, the postgres waits, and the redis and kafka-metadata diagnostic gap. - The clickhouse round-trip comment says every wait fails fast, `wait_hr_ready` on `Stalled=True`, "so a genuinely broken backup reports in seconds". A release whose install is still running is neither Ready nor Stalled, so that holds for a stalled release and not for an install that has not finished. The sentence predates this change. - `hack/mariadb-first-boot-waits.bats` is large for what it guards (two floors, two ratchets on the op, and the helper identity pin), with many fixture tests on its own extractors. Trimming it to that core is a separate decision. - Two comments in that file state facts about the mariadb suite that are true today and that nothing checks: three script ops carry a minute timeout, and the only one ahead of the round-trip is the 8m pre-clean. - The mariadb op comment says a bound on the whole flow "needs a phase budget (docs/agents/e2e-testing.md)"; the doc describes it as bounding the collection phase as a phase (`COZY_DIAG_PHASE_BUDGET`), so the pointer would resolve faster if it named that bullet. - Cosmetic: the behavioural test's loop body is not indented under its `for`, one line of the bats header runs past the width of the rest, and the checklist's sentence under the example assert ("what this block demonstrates least") reads awkwardly. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file, for all eighteen files. The five `examples/backups/*/00-helpers.sh` files change the diagnostics of `wait_hr_ready`, and the mariadb, mongodb, rabbitmq and clickhouse walkthrough scripts change wait budgets; the manifests the walkthroughs apply and the flows they document are unchanged, so no website page restates anything that moved. Four Chainsaw suites (mariadb, clickhouse, mongodb, rabbitmq) change timeouts and comments, and the harbor and opensearch suites change comments only. `docs/agents/e2e-testing.md` changes the reviewer checklist entry and adds the sizing section. `hack/mariadb-first-boot-waits.bats` is a new file picked up by the existing `BATS_UNIT_FILES` wildcard. `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` are untouched, and no `values.schema.json`, `values.yaml` default, version enum, `ApplicationDefinition`, CRD, namespace or release asset is touched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(tests): the MariaDB, MongoDB, RabbitMQ and ClickHouse backup examples and the MariaDB Chainsaw suites now wait for a HelmRelease past the release's own install timeout, so a failed install reports its own reason instead of a bare timeout, and the examples' HelmRelease wait now tells a release that never appeared, one deleted mid-wait and a kubectl that could not answer apart. ``` |
||
|
Andrey Kolkov
|
7ce85a2f7f |
test(kafka): pin the strategy script's guards with a hermetic bats suite
The in-Pod script had grown eight fail-closed guards across as many rounds, and the only thing exercising any of them was a 40-minute cluster test that drives the happy path plus one refusal. The partition listing guard in particular - the whole of the previous round's second fix - could be deleted with nothing in the tree going red. Add hack/kafka-topic-backup-strategy.bats on the shape of hack/kafka-backup-script.bats: render the strategy through a stub kubectl exactly as step 01 applies it, lift the container body out of it, and execute it against stubbed kafka-*.sh, a stubbed curl and a directory standing in for S3, with fault injection for the broker behaviours the guards exist for. Deleting that listing guard now fails one case; the script gains BIN/SCRATCH/CA_FILE overrides so it can run outside the Pod, each defaulting to the Pod's value. The same rounds' smaller findings: the manifest's topic name is held to Kafka's charset like the topics parameter, since it reaches unquoted expansions with globbing on; the shape check after --create compares the replication factor as well as the partition count, and refuses a replication factor below the topic's effective min.insync.replicas before replaying, because kafka-console-producer only logs a rejected acks=all write and the run would otherwise end at the count check with the cause unnamed; the manifest's replication factor must agree across a topic's lines; topic_records carries its own || return 1 on each command, as a function called as the operand of || runs with errexit off; the CA lookup in step 03 distinguishes a denied read from an absent Secret, and the README's persona column says the copy needs the admin; the e2e comment no longer claims the 2-label endpoint fails the serving cert's SANs, which include '*.tenant-root'. Rebased onto main, which moved the chart to KRaft: the demo's Kafka CRs drop the zookeeper block the chart no longer has. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-By: LLM |
||
|
Aleksei Sviridkin
|
99de79ff52 |
fix(cdi): pin the CDI release kubevirt-cdi is reconciled against and stop update overwriting it (#4107)
## What this PR does `packages/system/kubevirt-cdi` resolved its CDI version at run time from GitHub's `releases/latest` redirect and wrote the result straight over `templates/cdi-cr.yaml`. That file is not vendored. Upstream ships a bare CDI object; ours adds Helm templating for `cloneStrategyOverride`, `uploadProxyURL` and the worker-pod `podResourceRequirements`, the `ExpandDisks` feature gate, a `CriticalAddonsOnly` toleration, and the two RBAC documents that let a tenant clone from the golden-image namespace. I diffed the two at v1.66.1 to write that list, so it is complete as of that release rather than only hedged: upstream ships 283 bytes with one feature gate, two nodeSelectors and an `imagePullPolicy`, and we are a strict superset of it. Every entry arrived by hand, so `wget -O templates/cdi-cr.yaml` was not refreshing a vendored file, it was overwriting a first-party one. Same loss as the `rm -rf templates` that #4097 removes, arriving through the fetch instead of the wipe. `update` now pins the release the object is reconciled against, and fetches it only to confirm the bytes still hash to a digest I measured. Nothing is written under `templates/`. At the next CDI bump whose asset differs, that check fails, which is the point: upstream moving its own defaults is when the local copy needs a human to reconcile it, not an overwrite. The download is kept on a mismatch, since reconciling one starts by reading it. `RELEASE` is `v1.66.1`, which is the release `kubevirt-cdi-operator` vendors, so the pin and the deployed version agree. What the platform deploys is decided by that package, and it can move to another release without this file changing, so this line is pointed at it by hand. `update` then checks the two agree: it compares `RELEASE` with the `cdi-operator` image tag that package vendors and fails naming both versions when they differ, because otherwise its closing line would report a release the platform may no longer run. That is a check and not a derivation, so `RELEASE` stays a pin a human moves. Upstream publishes no checksum for the asset, so the pin comes from a download rather than from a published file: `a497f90de608c1df26f9ee4095289373f17132d74a2042ca03e00c17c964f8a7`. It catches an asset that moved under a tag that should be immutable. It is not authenticity verification, and the comment beside it says so. The digest command is chosen when `update` expands it, `sha256sum` when it is on PATH and `shasum -a 256` otherwise. Which one ran here is worth stating, because the usual claim about macOS is out of date: this machine picked `sha256sum`, and that binary is `/sbin/sha256sum`, a native Darwin one rather than GNU coreutils. It reports `sha256sum (Darwin) 1.0` and prints the same `<digest> <path>` format. The package already had a `test:` target, from `0418577f5`, so what it gains here is the `.PHONY` line that commit did not add. `hack/package.mk` declares only the nine targets it defines itself, so without it a file or directory named `test` or `update` beside the Makefile makes make consider that target up to date and skip the recipe in silence. The new suite pins the `ExpandDisks` gate, the toleration, `podResourceRequirements`, the `uploadProxyURL` override and the clone RBAC, and renders the upload-proxy Ingress and TLSRoute, each under the `gateway-enabled` value that selects it. The clone-strategy override is pinned by `tests/clone_strategy_test.yaml`, which is already on `main`. One consequence of the pin move is a claim rather than a number, so it gets its own line. Upstream carried `WebhookPvcRendering` from v1.63.0 through v1.65.0 and dropped it by v1.66.0; the local object never carried it, so the divergence ran both ways across those three releases and one way since. The comment says to re-derive the delta at each bump instead of trusting it, because that was a fact about a span of releases and not about CDI. `packages/system/kubevirt-cdi/Makefile` carried two identical `test:` targets, one from `0418577f5` and one appended by a later merge, so make printed `overriding commands for target 'test'` on every invocation in the package. The second sat inside the span this recipe rewrites, so it goes with the rewrite rather than being re-authored into it. The comment kept above it is the reason a reader should not add another. Closes #4157. ### Scope An earlier revision also pinned and tested `kubevirt-cdi-operator`. #3920 has since landed the two real bugs in that recipe, the patch strip level and `patches/tolerations.diff`, so that half is dropped here rather than merged twice. Pinning and testing the operator package is #4190. Named improvements left alone, so they read as choices. - The two upload-proxy tests each render their route under the `gateway-enabled` value that selects it and neither asserts the other is absent; the templates already settle that with `ne "true"` and `eq "true"` on the same variable, and `tests/standalone_render_test.yaml` already asserts each route absent under the values that select the other. So the package does cover it; these two cases just do not, and a third copy would not add anything. - The CDI object carries no version string for a rendered-output assertion, so `RELEASE` is checked by `make update` rather than by `make test`. - Neither route case pins the backend service port, 443 in both templates; that is outside what the suite claims. - The template-loss guard still counts `cdi-cr.yaml` as named, because the download line in `update` mentions the stem, so re-adding a wipe here would have it report two files instead of three; the guard's header already says a mention is not proof of a write, and the case needs a wipe this recipe no longer has. - The suite pins the local additions by design, and most of the upstream-derived half is pinned by nothing: `imagePullPolicy` and the two `nodeSelector` blocks have no assertion anywhere under `tests/`. `HonorWaitForFirstConsumer` is the exception and only as a side effect: `featureGates` is pinned whole, so that gate rides along with `ExpandDisks`. The rest are maintained by the same hand reconciliation the pin forces whenever the upstream asset changes, so a carry-over that drops one of them is green. Three asserts would close what is left. - One more, remote enough to name rather than code around: if the digest tool is missing entirely, `$$actual` comes out empty, the comparison against `CR_SHA256` fails, and the error text diagnoses a moved asset when the real cause is a machine with neither `sha256sum` nor `shasum`. Busybox ships the first and macOS ships the second, so nobody has hit it. Closing it means an executable line, which would then owe mutation coverage of its own, and that is disproportionate to a case with no known instance. The mechanism is written here so the next reader does not have to re-derive it from an error message that points the wrong way. - Separately, the per-package `.PHONY` gap this Makefile's own line argues against is not specific to this package: most package Makefiles do not declare their own `test:` or `update:` phony, and #4214 tracks that. - A key that does not exist yet is not held either: every item in the delta is pinned whole, but a NEW key appearing at `spec` or `spec.config` level passes unnoticed, and such a key can change behaviour rather than sit inert. I confirmed it on a copy extracted from this commit, appending `spec.config.filesystemOverhead.global: "0.99"` and getting a green suite. That is the shape to recognise at the next bump: not a changed value, which is caught, but a new sibling key beside the ones we pin. - And `spec.config.podResourceRequirements` is now pinned whole here and by four leaves in `tests/importer-resources_test.yaml`: the whole-map form is the stronger one and the leaves are a subset of it, so changing the default takes two edits and the second failure reads as a surprise until you know why both exist. - If the operator's image reference ever gains a digest (`:v1.66.1@sha256:...`), `${deployed##*:}` takes the digest's hex instead of the tag, and `update` fails with a loud false mismatch rather than passing silently. - Deriving `RELEASE` with `$(shell ...)` from the sibling manifest was suggested and declined on merits: a derived value is not a pin, the point is that a human chooses which CDI release this object is reconciled against, and computing it from whatever the neighbour last vendored hides that choice from the diff. The operator comparison lives in `update` and not in `test:`, even though `kubevirt-operator` ties its own manifest to the tree from `test:`. A mismatch calls for a hand reconciliation, and `update` is the recipe that asks for one; it is also where the success line that would otherwise name the wrong release is printed. I checked it on a copy extracted from this commit. With the operator's image tag moved to v1.67.0, `make update` exits 2 and names both versions. With the tags equal it passes. With the check's `exit 1` removed, the moved tag passes again, so the red comes from the check. Worth knowing when reading the mismatch path: `kubevirt-operator` deletes its download on a hash mismatch and this package keeps it. That is deliberate here, since reconciling a moved asset starts by reading it, but the two packages do read oppositely and neither file says so. ### Base and ordering This stacks on #4097 and should land after it. Both edit `packages/system/kubevirt-cdi/Makefile`, and this change builds directly on that one's edit to the same recipe, so it is based on the head of `fix/cdi-update-keeps-local-templates` rather than on `main`. GitHub computes mergeability against the PR's base branch, which here is that stack branch and not `main`. Both branches are rebased onto current `main`, which has not touched these paths since the previous merge-base. ### Screenshots No UI changes. ### Downstream repositories Walked the trigger map against the diff. Two entries come close and neither fires. The website triggers on developer tooling and package Makefiles, but `content/en/docs/next/development.md` documents `make update` only as "Update Helm chart and versions from the upstream source", which still holds, and its worked example is cilium, which this does not touch. Its `make test` line refers to the e2e sandbox in `packages/core/testing`, not per-package unit tests. `ccp` triggers on `hack/` layout and on what a make target does, and nothing under `hack/` is touched. Its `package-bump` skill already assumes a package's `update` recipe hardcodes the upstream version instead of resolving it at run time, and names cilium, kafka-operator, opensearch-operator and postgres-operator as examples. `kubevirt-cdi` was one of those counterexamples and now is not. Pinning `kubevirt-cdi-operator` the same way is #4190 and out of scope here. The skill handles vendored Helm charts, in-repo image builds and version enums, and a raw manifest download is none of those. Everything else in the map needs an app, a schema, an installer value, a variant or a release asset. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(cdi): `make update` in kubevirt-cdi no longer overwrites the hand-maintained CDI object. It pins the CDI release that object is reconciled against instead of resolving GitHub's releases/latest at run time, and verifies the fetched asset against a pinned sha256 without writing under templates/. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * Updates now use verified, pinned CDI releases to prevent unintended manifest changes. * Failed or checksum-mismatched downloads no longer overwrite existing configuration. * **Bug Fixes** * Preserved local CDI customizations during release updates. * Added validation for CDI resources, access controls, upload-proxy routing, and TLS configuration. * **Tests** * Added automated checks for incomplete template refreshes and generated Kubernetes resources. * Added chart tests covering CDI behavior, clone permissions, and upload-proxy networking. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
5bd029c46a |
fix(kafka): restore with the captured replication factor and a complete listing
Record each topic's ReplicationFactor in the backup manifest, off the same --describe line the partition count already comes from, and recreate the topic with it on restore. The value used to be a BackupClass parameter defaulting to 1, and the Cozystack Kafka chart sets the broker-level min.insync.replicas to 2 as soon as kafka.replicas reaches 2: kafka-console-producer writes with acks=all, the broker rejects every write into a topic with fewer replicas than that, and the producer logs the rejection and exits 0. A restore onto any multi-broker cluster therefore landed zero records and failed the post-replay count with a message naming the wrong cause; in place, the documented delete had already taken the live topic with it. replicationFactor stays as an override for restoring into a smaller cluster, and a value the target cannot host now fails at --create in Kafka's own words. Hold the pre-replay emptiness check to the topic's partition count, as the backup side already does. kafka-get-offsets skips a partition it could not read and still exits 0, so a leader election in flight dropped the populated partitions from the listing, the surviving empty ones summed to zero, and the replay appended a full copy into a live topic - the exact outcome the check was moved ahead of the replay to prevent. Both restore-side counts now go through one helper that refuses a short listing. The data-file-versus-manifest check runs both ways before anything is created, and the replay walks the manifest topic by topic, announcing each one, so a retried Pod's log says which topics landed. The demo now runs two brokers with two-replica topics, which is the shape the chart puts min.insync.replicas=2 on; the restore steps assert the restored topic's replication factor, and step 06 submits a second RestoreJob against the populated topic and requires it to be refused with the topic untouched. Smaller findings from the same round: the topics parameter is held to Kafka's name charset (a literal "*" no longer path-expands) and refused on a repeated name, which produced a Succeeded backup no restore could balance; the strategy verifies S3 TLS against the seaweedfs CA projected into the <app>-backup-s3 Secret instead of curl -k, following the rabbitmq example, so the e2e endpoint moves to the .svc name the serving cert covers; the image lookup keeps digest-pinned references; the default namespace is tenant-root, the only one the demo ever ran in; step 05 caches the resolved Backup name for the restore steps; the decoy count subtracts begin offsets like the topic count; the bats test name carries its count again; and the scenario walk-throughs the sibling examples ship are added. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-By: LLM |
||
|
Andrey Kolkov
|
2c9b3dfeea |
fix(kafka): validate the restore manifest and pin topic matching in the suite
The manifest arrives inside the downloaded tarball but was consumed as trusted input. A non-numeric partition field slipped through the numeric test that computes a topic's partition count: an if condition is exempt from errexit, so test exits 2, the term reads false, the count comes out short, the topic is created smaller, and the shape check then compares that short value against itself and passes. The per-topic total still added up, so the restore reported Succeeded with every keyed record re-placed. Validate each manifest field on read, refuse an empty manifest whose verification loops would otherwise run zero times and report a successful restore of nothing, and guard the restore-side PartitionCount read the way the backup side already guards its own. The delete-wait still matched the topic name as a regex; use grep -qxF. The suite now pins that class of bug instead of relying on review: the demo topic is orders.v1 and a decoy ordersXv1 sits beside it, matching the first as a regex but not as a literal. The decoy is never named in the BackupClass, so dropping any \Q...\E pin fails the run on its own - the partition-set guard counts the decoy's partitions, or the in-place delete takes the decoy with it. Also: the host-side record count skipped a partition missing from the begin-offset listing instead of defaulting it to 0, undercounting the topic; the two preflight invariants now cover the kafka flow they already describe; and the README names the disabled TLS verification. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
d405a36e3c |
fix(kafka): validate the backup's partition set against the topic
The drain guard only covers a short read within a partition it was told about. kafka-get-offsets lists just the partitions whose lookup succeeded - GetOffsetShell logs a per-partition KafkaException to stderr, skips it and still exits 0 - so a partition lost to a leader election got no manifest line, no data file and no drain check, and the tarball was uploaded with the Backup reporting Succeeded. The restore-side count check could not catch it either, since it sums the same short manifest, and a missing highest-numbered partition made restore recreate the topic with fewer partitions, re-placing every keyed record. Read the topic's own PartitionCount before the drain and fail closed when the offset listing is short. A short "begins" needs no separate check: its per-partition default of 0 can only overstate n, which the existing drain check then catches. Also run the shared BucketAccess preflight in the e2e - accessGranted does not prove the S3 server reloaded the new identity, and a fresh grant answering the SigV4 PUT with 403 reds the run as a backup defect - and state the .chainsaw.yaml namespace override as the rule it is rather than as a claim about which Tests match a name pattern. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
7d8dee66a5 |
fix(kafka): verify restore landed and namespace-scope the S3 key
- Restore verification: kafka-console-producer, like the consumer on backup, exits 0 without every record being accepted, so a replay that dropped records still reported Succeeded. After the replay, compare each restored topic's record count (sum of end-begin over partitions) against the manifest and fail on any shortfall or overshoot - the latter being a restore into a non-empty topic, which this appending DATA restore does not support. - Namespace-scope the S3 object key (<namespace>/<app>/kafka-topics.tar) so two same-named apps in different namespaces sharing one bucket do not collide. The namespace is .Release.Namespace in both modes and always names the source, since a to-copy target shares the source's namespace (a TypedLocalObjectReference). - Warn instead of failing open when the Kafka image cannot be resolved from the operator: the pinned public literal is unreachable on an air-gapped cluster, so tell the caller to set KAFKA_IMAGE. - Word the .chainsaw.yaml namespace note as "most" (etcd-2-backup- roundtrip sets no namespace) and document the empty all-topics backup. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
9673af6aca |
fix(kafka): guard short backup drains and settle example teardown
- Short-drain guard: kafka-console-consumer exits 0 after a timed-out short read, so a truncated drain was tarred and reported Succeeded. Compare the drained line count against end-begin per partition and fail the backup on any shortfall, making the README's "fails closed" promise real for compacted/transactional topics. - Settling teardown: cleanup.sh returned before the Kafka apps and Bucket had uninstalled and could not report failure, letting the neighbouring chainsaw/kafka Test contend on PVC reclaim. Add a wait_deleted helper and a teardown_rc gate that waits for the heavy objects and exits non-zero when the teardown does not settle. - Fully qualify the ambiguous delete kinds (backup/bucket/backupclass/ restorejob/backupjob) so a bare shortcut cannot target a foreign group and silently leave the Cozystack artifact behind. - Default KAFKA_IMAGE from the running operator (newest STRIMZI_KAFKA_ IMAGES entry), falling back to the literal only when the operator cannot be read; drop the duplicate resolution note. - Make the strategy and throwaway CLI Pods restricted-clean (runAsNonRoot + JSON-patch securityContext on the CLI Pods). - gitignore .source-dump.txt; replace the deprecated "small" preset with c1.small; capture the to-copy cluster in the e2e catch; document the all-topics risk; de-enumerate the decaying suite-list comments. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
34a04e6b37 |
chore(kafka): document copy-safety caveats and harden all-topics loop
Document the caveats that make the example safe to copy - a null key round-trips as the literal string "null" (changing key and compaction identity), the drain stages the topic twice under /tmp with no ephemeral-storage bound, every backup of an app overwrites the same S3 key, and the Pods target warn-only Pod Security. Harden the all-topics partition-count loop: the final `[ ... ] && parts=...` returned non-zero under `set -e` when the last test was false, so use `if ... then ... fi`. Align the default KAFKA_IMAGE with the vendored operator (0.45.1-rc1) and list kafka-2- in the .chainsaw.yaml tenant-root header. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
d38b538d98 |
test(kafka): fail fast on terminal backup phases and capture strategy Job logs
Two CI-quality fixes, both diagnostics/wall-clock only (neither can turn a run falsely green): - The chainsaw comment claimed run-all.sh fails fast on terminal BackupJob/RestoreJob phases and Stalled HelmReleases, but the kafka helpers never implemented it (carried over from mariadb). A BackupJob that settled phase=Failed was polled the full 600s and then mislabelled 'Timeout', burning ~10 of the 40 chainsaw minutes per already-red run. Port the mariadb variants: wait_for_field gains a terminal fail_value arg (call sites pass Failed), and a new wait_hr_ready fails fast on Stalled=True with an existence backstop. HR waits in steps 03/04/07 now use it; the Strimzi Kafka CR waits keep their genuine Ready condition. run-all.sh runs under set -e, so a non-zero step aborts the flow immediately, matching the comment. - The catch block only captured the Strimzi broker Pods (app.kubernetes.io/instance=kafka-kafka-test); the generic Job strategy runs the drain/replay in controller-created Pods labelled job.strategy.backups.cozystack.io/mode=backup|restore (never that instance label) and not TTL-reaped, so the actual S3/SigV4 error left no log in the report. Add a second podLogs selector keyed on that label. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
00943392cc |
fix(kafka): resolve a single Kafka image for backup e2e harness
STRIMZI_KAFKA_IMAGES is a newline-separated version=image map with one entry per supported Kafka version. The old 'sed | tr -d [:space:]' collapsed every line into one string, producing an invalid multi-image reference (quay.io/...3.8.0quay.io/...3.8.1...) that failed 'kubectl run' with 'invalid reference format' and broke kafka-2-backup-roundtrip. Split on whitespace, keep image lines, strip the version prefix and take the last (newest) image so a single valid reference is exported. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
7842b88676 |
feat(kafka): add topic-data backup/restore example and e2e roundtrip
Add examples/backups/kafka, a worked backup/restore of a Cozystack-managed Kafka application's topic data via the generic Job backup strategy - the same pattern as examples/backups/nats, with no purpose-built backup image: a stock Strimzi Kafka image (kafka-*.sh CLI + bash/curl/tar) runs one shell script that branches on .Mode. Consistency is a frozen-end-offset cut: kafka-get-offsets --time -1 is read once per partition at backup start and the consumer drains only up to that offset, so anything produced during the run is excluded and every partition is captured as of the same instant. Restore recreates each topic with its original partition count and replays every partition file; keyed records reproduce into their original partition via the default partitioner. This is deliberately a DATA-only backup: topic configs, ACLs, SCRAM users and consumer-group offsets are not captured, and re-produced records get fresh offsets. The README spells out the scope and points at a volume-snapshot strategy for faithful, offset-preserving backups. Add kafka-2-backup-roundtrip to hack/e2e-chainsaw/kafka, driving the example run-all.sh as the harness (Bucket -> source Kafka + seeded topic -> BackupJob -> in-place restore -> to-copy restore + verify) so the test and the documented flow cannot drift, mirroring mariadb-2-backup-roundtrip. The image is resolved from the running operator so the backup Pod reuses the cached Kafka image. Verified end to end on a live cluster: freeze cut orders:0 [0,8), orders:1 [0,11), orders:2 [0,11) = 30 records; in-place and to-copy restores both return 30 with the original 8/11/11 partition distribution. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
b1519acfe7 |
fix(e2e): cap the bucket S3 reads so the verify step honors its timeout (#4281)
## What this PR does Refs #4268 — a plain reference, not `Fixes`: the issue asks for a `script` op to be cut off at its declared ceiling, and that is Chainsaw behaviour this repo cannot change. Chainsaw runs the op's shell under Go's `exec.CommandContext`, whose default cancel SIGKILLs the shell PID alone and then blocks on the inherited stdout/stderr pipe until an orphaned `mc` closes it, so the op returns only when `mc` exits on its own rather than at the ceiling. This PR mitigates that where it bit — per-command caps so no descendant outlives its own budget — but the op-timeout gap itself lives in Chainsaw, so the link stays a reference rather than a `Fixes`. The sibling step that produced the 606s, `verify-every-bucket-takes-a-write` in the `bucket-1-capacity` Test, is on `main` and already carries its own per-call caps; this PR applies the same shape to `verify-s3-access`. On run 34948279363 the bucket suite's `verify-every-bucket-takes-a-write` step declared `timeout: 5m` but ran 606s and ended on `mc`'s own non-zero status rather than being cut at the ceiling. `verify-s3-access` in `hack/e2e-chainsaw/bucket/chainsaw-test.yaml` is the same-shaped step in this suite. This PR wraps every S3 (`mc`) and apiserver (`kubectl`) read in `verify-s3-access` in its own `timeout`, so a hung read dies at a known budget instead of holding the step open, and corrects the two places (`hack/e2e-chainsaw/.chainsaw.yaml` and `docs/agents/e2e-testing.md`) that explained the op ceiling with a mechanism that does not hold. It caps `verify-s3-access` for sharing the shape, not for producing the 606s number. ### Which of the issue's two candidate mechanisms holds The issue named two candidates and stated the log could not separate them. The Chainsaw v0.2.15 source separates them, and it is the second one, with the exact refinement the issue proposed. **Candidate 1 — "an operation `timeout` does not bound a `script` op" — is refuted.** The op timeout is applied and does reach the process: `pkg/runner/operations/script.go:24` copies the step's `timeout` into `Timeouts.Exec`, and `:38` wraps the op context with `ctx, cancel := context.WithTimeout(ctx, tc.Timeouts().Exec)` before calling `op.Exec(ctx, …)`. So a 5m step produces a real 300s deadline on the context the command runs under. **Candidate 2 — "it bounds it and kills the shell, but Chainsaw waits on a pipe a grandchild (`mc`) still holds open" — is confirmed.** In `pkg/engine/operations/script/operation.go` the command is built with `exec.CommandContext(ctx, shell, …)` (`:105`), and in `execute` its stdout/stderr are set to `*bytes.Buffer` (`:128`–`:129`, the buffers are `internal.CommandOutput` in `pkg/engine/operations/internal/command_output.go:12`–`13`) and run via `cmd.Run()` (`:130`). Two Go `os/exec` properties then produce the overrun: `exec.CommandContext`'s default cancel is `Process.Kill()` on the shell PID alone — not a process-group kill, and the operation sets no `SysProcAttr.Setpgid` — and because stdout/stderr are not `*os.File`, `os/exec` allocates OS pipes and copier goroutines, so `cmd.Wait()` blocks until those pipes reach EOF. EOF needs every inherited write-end closed. When the shell forks `mc`, `mc` inherits those fds; at 300s the deadline SIGKILLs the shell but `mc` is orphaned, keeps retrying the S3 PUT, and holds the pipe, so the op does not return until `mc` exits on its own at 606s. `cmd.Run()` then returns `signal: killed`, which is stored in the `$error` binding and never printed — only the `($error == null): true` check failure is logged — which is why grepping the run for `signal: killed` found nothing. **Empirical confirmation.** A standalone Go program reproduces it exactly. `exec.CommandContext` with a 1s timeout, `*bytes.Buffer` stdout/stderr, running `sh -c 'sleep 2 & sleep 4'` (a descendant that inherits the pipe): `cmd.Run()` returns after ~4s, not 1s, with `err=signal: killed` and `ctx.Err()=context deadline exceeded` — the ceiling fired at 1s, control came back only when the descendant closed the pipe. Redirecting the descendants' stdout/stderr away from the pipe (`… >/dev/null 2>&1`) returns at 1.00s. That is the whole mechanism: the ceiling cuts the shell but not a descendant that still holds the pipe. ### The fix Per-command caps, the remedy the issue itself points to ("every read carries its own cap") and the shape already used elsewhere in the suite. The bucket step now defines a `k()` wrapper (`timeout 30 kubectl`) and an `mc()` wrapper (`timeout 15 mc`) for the reads, and calls `timeout 45 mc cp` directly for the two PUTs; a hung read is killed at its own budget, releasing the inherited pipe promptly, so even when the op ceiling fires the residual is one inner cap rather than `mc`'s full retry exhaustion. The terminal precedents for these budgets are the redis suite's terminal `k()` (30s, `redis/chainsaw-test.yaml:298`) and the sibling `verify-every-bucket-takes-a-write` step's terminal `mc cp` (45s, `:390`, sized for a volume grow); the 15s cap on `mc` in `hack/e2e-chainsaw/_lib/backup-access-preflight.sh` is not a precedent, because it sits inside a `while :;` retry loop where a 124 costs one attempt, whereas here under `set -e` a 124 ends the step. The inner budgets — 2×30 for the two `k` reads + 2×45 for the two `mc cp` PUTs + at most 7×15 for the other `mc` calls (eight other call sites, but the two cleanup `mc rm` are mutually exclusive) + the 30s port-forward readiness wait = at most 285s — sit under the 5m op ceiling, which stays as a backstop, per the "op above the sum of inner budgets" rule in `docs/agents/e2e-testing.md`. The long-lived `kubectl port-forward` keeps no cap by design (it must persist for the whole step, and its stdout/stderr are already redirected to `/dev/null`, so it cannot hold the step's pipe). The `mc()` wrapper is non-recursive because `timeout` execs its argument through a PATH lookup that bypasses shell functions — verified in `dash`, `bash` and `sh`. The `catch` block's diagnostic read gets the same treatment — `timeout 30 kubectl` under a 1m op `timeout` — for the same pipe reason. ### The readonly deny probe fails on a timeout The readonly-upload probe used to branch on `mc` directly (`if mc cp …; then`), which suspends `set -e`, so a `timeout` there (exit 124) was indistinguishable from the access-denied non-zero the probe expects: a 45s hang on the deny path would exit 124, take the false branch, reach cleanup and exit 0 — a long hang turned green, the exact window this PR exists to bound. This is the first `ro-user` write in the step, so no earlier call proved that path healthy. The probe now captures the status: exit 0 is the readonly regression, 124 is a timed-out probe, both fail; any other non-zero is the expected denial. ### No unit surface; why the evidence is the proof This is an e2e-config and doc-correctness change. The embedded Chainsaw script is not extracted into a unit harness anywhere in the tree, so there is no red-to-green unit test to add without inventing a bespoke script-linter (which would be a new checker standing in for the fix). The regression is instead pinned by the mechanism proof above: the source citations show the ceiling cannot cut a descendant that holds the pipe, and the caps remove the only way a descendant outlives its budget. The deny-probe status handling and the pipe-vs-file secret reads were exercised against stub exit codes (0 → regression-fail, 124 → timeout-fail, other non-zero → expected denial) and the whole script passes `sh -n` and `bash -n`. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The change touches only `hack/e2e-chainsaw/` suite content and `docs/agents/e2e-testing.md`; it moves or renames nothing under `hack/`, and touches no node-prerequisite or `package.mk` path. No downstream repository is reached. - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved end-to-end S3 access verification so stalled Kubernetes API and S3 operations fail promptly. - Correctly distinguishes timed-out access checks from expected permission denials. - Preserves partial command output and allows subsequent test steps to continue after individual timeouts. - Applies separate time limits to bucket reads, uploads, and access checks for more predictable test behavior. - **Documentation** - Clarified operation timeout behavior and recommended per-command limits within the overall operation limit. - Documented timeout handling for orphaned processes and updated worst-case timing guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ec706c49b1 |
docs(e2e): correct the Chainsaw Test-ordering claim in etcd and clickhouse suites (#4277)
Fixes #4269 ## What `hack/e2e-chainsaw/etcd/chainsaw-test.yaml` explained its layout with a comment claiming Chainsaw executes Tests in **name order, not file order**, and used that claim to justify why teardown lives inside each scenario. The clickhouse suite had picked up the same claim. This removes the ordering claim from both — it is false at the pinned version — and re-grounds the etcd teardown justification in a reason that does not depend on order. ## Why the claim was wrong (evidence, Chainsaw v0.2.15 — the pinned version) At v0.2.15 Chainsaw guarantees **no order between Tests at all**: not file, not document, not name. Restating "name order" as "document order" is wrong for the same reason. Verified against the pinned source: - `pkg/runner/runner.go` — the runner registers each Test through `testing.T.Run` and the per-Test body calls `t.Parallel()` whenever `spec.concurrent` is nil or true. Neither suite sets `concurrent`, so both take that path. Go pauses those subtests and releases them together once the parent returns; they then race for the `-test.parallel` semaphore. - `pkg/runner/flags/flags.go` — `execution.parallel` maps only to `-test.parallel`, a cap on how many Tests run at a time, not an order. - `hack/e2e-chainsaw/.chainsaw.yaml` sets `execution.parallel: 1`, so Tests are serialised — one at a time — but not ordered. The E2E run on this PR's own head demonstrates it: discovery loaded the five selected Tests in document order, then ran `clickhouse-2` before `clickhouse-1` with both etcd Tests interleaved between the clickhouse pair. Serial, non-overlapping, and in an order that is neither file, document, nor name. ## The fix The teardown decision the false claim justified is still correct, for a reason that does not touch order. Each etcd scenario shares the singleton `etcd` release and its cluster-scoped Kamaji `DataStore`, runs with `skipDelete: true` (opting out of Chainsaw auto-cleanup), and v0.2.15 has **no test-level `finally`** — so a scenario is reclaimed only if it reclaims itself, and that is true of whichever Test runs last regardless of order. The etcd comment now says exactly this, with the ordering clause dropped rather than restated. The clickhouse header's ordering sentence is removed outright: the three Tests use distinct releases and share no state, so nothing there needed an ordering claim. This is a comment-correctness change with no runtime surface, so there is no unit test to add — the load-bearing claim was verified empirically against the pinned Chainsaw source, as above. A repo-wide grep confirms no other ordering claim survives in `hack/e2e-chainsaw/` or `docs/agents/e2e-testing.md`. Per `docs/agents/e2e-testing.md`: comment-only, no new retries, no traps, no assertion or teardown-timing changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Corrected end-to-end test comments to drop an incorrect test-ordering claim; Chainsaw v0.2.15 guarantees no order between Tests (not file, not document, not name). * Clarified that teardown behavior is independent of test execution order. * Documented the relationship between ClickHouse backup tests and the corresponding backup test suite. * No test behavior or user-facing functionality changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
bc8c54d6e6 |
feat(kubernetes): boot tenant worker disks from a shared golden Talos image
Every tenant worker VM imports the same Talos raw image from the Image Factory over HTTP, once per worker: a 20-worker cluster fetches the identical ~4.15 GiB artifact 20 times, and each fetch is a live dependency on the Factory at exactly the moment a node joins. packages/system/kubernetes-worker-image is a new opt-in catalog that holds one golden DataVolume per (schematicID, version) in cozy-public, named talos-worker-<schematicID>-<version>. A worker pool opts in with osImage.builtin on its own KubernetesNodes resource, and its disk-system DataVolume then carries source.pvc pointing at that golden, so CDI performs a storage-layer CSI clone instead of an HTTP import -- one import per flavor, shared by every worker of every tenant. Confirmed on a LINSTOR-backed cluster: the worker disk is a CSI clone plus a resize, with no host-assisted upload pod. Both halves are off unless asked for. The catalog is registered through the platform's optional-package helper and disabled by default, so a cluster that does not enable it creates no golden and keeps importing over HTTP. The disk source is derived entirely from osImage, which is absent by default, so a pool that does not set it renders byte-identical to the pre-feature chart and adopting the feature never rolls existing workers. Editing osImage on an existing pool re-images that pool, as changing any other image reference does. What the render refuses, rather than degrading into a silent fallback: - A pool whose golden has no catalog entry, or whose golden terminally failed to import. A golden that is merely mid-import is not an error; CDI holds the worker disk until the source populates. - A pool whose StorageClass differs from its golden's. CDI cannot CSI-clone across classes and falls back to a host-assisted copy over the pod network, which is the transfer this feature exists to remove. An empty class on either side means the cluster default and is resolved before the comparison, and a pool that has already cloned reads its class from its own bound PVC rather than from today's default -- the golden's side is frozen the same way, and an admin moving storageclass.kubernetes.io/is-default-class must not fail the release of a pool whose disks are all on the right class. That resolved class is then written onto the worker disk, so the disk binds where the guard checked instead of on whatever the default has become by then -- reading one class and binding another is how a pool at an empty storageClass passed the comparison and still took the host-assisted copy. The HTTP path leaves an empty class to the cluster as before, which is what keeps its render, and the KubevirtMachineTemplate hash named after it, byte-identical. - A diskSize below the golden's storage size, which a clone target cannot be. - A schematicID that is not a plain lowercase alphanumeric identifier, or a version that is not a vMAJOR.MINOR.PATCH release. Both are interpolated into a DataVolume name and an image URL, and values.schema.json types them as bare strings, so a malformed one otherwise reaches the render. The catalog applies the same patterns, since both sides build the same name from the same fields. - An images[] entry edited after its golden exists. A DataVolume spec cannot be patched in place, so the message names the entry and the supported move -- a new (schematicID, version) -- instead of leaving the API server to reject the patch on the next upgrade. Goldens carry helm.sh/resource-policy: keep, so dropping an entry stops managing a golden rather than deleting it: a missing golden aborts the whole Kubernetes HelmRelease of every tenant pinned to it, not just the disk. Each cloned worker disk annotates the golden it came from and the pool that owns it, so "does anything still clone this?" is answerable before a manual delete instead of after. A builtin pool needs a StorageClass whose volumes are reachable from any node, in practice a replicated/DRBD one. The golden in cozy-public has no consumer and binds as soon as it imports, wherever CDI's importer landed, while a worker disk binds where its VM is scheduled, and nothing here arranges for those to be the same node. The platform ships cloneStrategyOverride: csi-clone, which rules out the host-assisted strategy that would cross nodes. What a node-pinned class does in that situation is not measured, so values.yaml states the requirement and not a symptom. ComputePlane pools forward only the per-pool shape fields, and osImage is excluded there by name: the worker OS image, its Talos installer and its Image Factory are operator decisions about the platform's own images. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b96c822bde |
docs(agents): give the HR-Ready budget derivation its own section
The checklist entry had grown into a derivation and buried the rule it states. The rule stays in the checklist and the derivation moves to a section of its own. The derivation under-budgeted in two ways. It stated the 10m install timeout as universal, while releases an app chart renders itself take their template's spec.install.timeout, then its spec.timeout, or helm-controller's 5m where neither is set. And it restated which kinds kstatus gates on, which is easy to get wrong, so it now points at the upstream table instead. The harbor comments now name the floor each of their asserts actually sits against, and the opensearch comment stops citing a 5-6m standard the checklist no longer states. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
4373c8c0f2 |
test(e2e): pin the mariadb first-boot wait budgets against the tree
The waits are right only relative to numbers that live elsewhere: the chart's failureThreshold, the install timeout cozystack-api stamps, and the Chainsaw op that runs the walkthrough. Deriving the floors from the tree rather than pinning literals means a change to any of them reds the contract instead of silently invalidating the waits. The extractors are tested against fixtures as well, because the tree cannot tell a correct extractor from a broken one: the chart carries a single failureThreshold and every wait clears every floor, so a reader of the wrong operand stays green. The wait_hr_ready exit branches are tested by behaviour against a stubbed kubectl, one stub per answer they have to tell apart, because a static check cannot see what errexit does to a failing read. That test and a byte-identity check cover every definition of the helper in the tree, found by grep rather than listed, so a new copy is picked up without editing the test. The redis and kafka-metadata definitions are named exceptions with their reasons in the header, and each is held to still differ, so an exception whose copy converges goes red instead of lingering. The suite runs under /bin/sh, which is dash on the runner, so the behavioural test runs its body under an explicit bash. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
957ddd7f25 |
fix(tests): raise the mariadb suite HR asserts past the install timeout
At 6m the HelmRelease-ready asserts expired before the release's own 600s install timeout, so a failed install was reported as a bare Chainsaw timeout with no reason. 11m lets the release record why it failed first. The round-trip op grows by exactly what the two walkthrough waits gained. Otherwise a failure at either wait could run past the op, which SIGKILLs the process group and loses the script's error. That bounds one failure, not the whole flow: a run slow at every stage that fails late still reaches the op, and bounding that needs a phase budget rather than a larger literal. The endpoint asserts stay. A freshly applied CR has no conditions and kstatus reads it as current, so the HelmRelease wait can end before any pod is up; an endpoint address closes that window. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
f629280944 |
fix(tests): raise the mongodb, rabbitmq and clickhouse HR waits
Those walkthroughs gave their database HelmReleases 300s, under the 600s install timeout the generated releases carry, so a failed install surfaced as a bare timeout rather than its own reason. None of these charts sets a startup probe, so 660s clears both floors, as it does for mariadb. Each Chainsaw op that runs one of the scripts grows by exactly what the waits it reaches gained, so it keeps the margin it had. The clickhouse op still sits above the sum of every inner ceiling. The rabbitmq one covers a failed install at the source wait, while the ceilings up to its target wait add up to more than the op, as they did before. The mongodb round-trip runs with SKIP_RESTORE, so only its source wait counts there. postgres keeps its 360s: whether a suite pays the extra minutes on every red run is a per-suite call, and this one is not made here. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
89da4a121c |
fix(cdi): stop make update from deleting first-party templates (#4097)
## What this PR does `make update` in `packages/system/kubevirt-cdi` began with `rm -rf templates` and then refetched one file, `cdi-cr.yaml`. Nothing in that directory is purely vendored. Two of the three manifests, the upload-proxy Ingress and TLSRoute from the Gateway API work, are ours and no fetch recreates them, so the next CDI bump would have dropped upload-proxy exposure from the chart and still exited 0. The third is `cdi-cr.yaml` itself, which despite its name carries Helm templating, the `ExpandDisks` gate, a toleration and the two RBAC documents that preauthorize cloning from `cozy-public`; the fetch overwrites it, and removing the wipe does not change that. That recipe now opens with `mkdir -p templates` and deletes nothing, which is where `packages/system/kubevirt-operator` landed at the last KubeVirt bump. The wipe bought nothing there anyway: `wget -O` truncates the file it writes. `kubevirt-cdi-operator` has the same recipe shape, and the same edit there would be a regression rather than a fix. It applies two patches, and since #3920 moved them to `-p4` they resolve, so a hunk upstream has moved out from under leaves `templates/cdi-operator.yaml.rej`; its `awk -i inplace` step leaves `templates/cdi-operator.yaml.gawk.XXXXXX` if gawk dies abnormally. Helm renders every file under `templates/`, and I reproduced both: the reject fails the package with a YAML parse error, and the gawk temp renders a duplicate object at exit 0. The wipe is what takes both, so removing it for symmetry would trade a latent loss for a live one, and the quieter of the two is the one that survives. Deleting the wipe there wants the recipe restructured to transform outside `templates/` and move the result in once, which is what `kubevirt-operator` does today and what its own comment gives this exact reason for. That is a larger change than this one and belongs on its own; #4383 makes it. Meanwhile the scan below covers the package: I added a first-party file to its `templates/` and the guard reported it, so the day that package grows one, this fails rather than the file being lost. `hack/package-update-templates.bats` guards this across the tree rather than in these two packages. It scans every package Makefile and fails when a recipe that clears its own `templates/` leaves behind a tracked file it never names. Put the removal back into `kubevirt-cdi` and it goes red naming both upload-proxy files. A file counts as named only outside the Makefile's own `NAME` and `NAMESPACE`, however the assignment is written. The match is on the variable name rather than on a list of spellings: the tree already carries three of them, `export NAME=`, the bare `NAME=`, and `NAME := ` in `packages/core/testing`, and the hole is identical in each. Without that, a package called `foo-bar` satisfies the check for its own `templates/bar.yaml` through its identity alone. Nothing the scan reaches is exempted that way as the tree stands, so the exclusion is there for the first package that would be: rename the asset in `kubevirt-cdi-operator`'s recipe and put the removal back, and the guard reports the orphaned template only with the exclusion in place. The two variables are matched by name rather than by a `[A-Z_]+=` shape, because a shape that broad is one unspaced assignment away from swallowing a variable that does name templates, which is how `etcd-operator-crds` names all five of its CRDs. A control test runs the same helpers over a fixture broken the same way, over one whose name swallows a template stem, then over that fixture rewritten to spell the removal with a trailing slash, to take one file instead of the directory, and to drop the removal entirely. Both cases pass under `hack/cozytest.sh hack/package-update-templates.bats`. Two limits, both written into the file's header. The check fires on the consequence rather than the shape, so `rm -rf templates` in a package that does refetch everything stays green, which is what keeps `kubevirt-cdi-operator` green for as long as its recipe wipes. And it only looks at a recipe clearing its own directory: `cert-manager` clears `../cert-manager-crds/templates/*` across the package boundary, which needs the removal's target resolved rather than the wiping package's own, so that directory is unguarded. The other packages that clear their own `templates/` refetch all of it and are untouched here. Two more limits the header names. The scan reads the text of each package's own `update:` recipe, so a removal made elsewhere is out of range: in a prerequisite target, behind an include, or behind a recursive make. `capi-operator`, `clustersecret-operator`, `openbao` and `reloader` write `update: clean <name>-update` and take both recipes from `hack/package.mk`, which removes nothing under `templates/` today. And the glob stops at `packages/*/*/Makefile`. The package Makefiles off that depth are the group aggregators such as `packages/system/Makefile` and the vendored kamaji subcharts, and none of them declares `update`. One behaviour of the matcher worth naming, and deliberately not changed here. `wipe_command` evaluates `/^update:/` ahead of its terminator, so a second `update:` in the same Makefile re-arms the scan instead of ending it, and what that means depends on what sits between the two: adjacent or separated by a blank line, both recipes are read; separated by another target, only the first is. The direction that matters is a false positive rather than a miss. A Makefile whose first `update:` clears `templates/` and whose second does not would be reported by this scan, while make runs only the second, so the package fails over a recipe that never executes. No package in the tree defines `update:` twice today. Three more notes, all non-blocking and none acted on. The header says what the scan leaves uncovered is a recipe clearing someone else's `templates/`; that is the uncovered set for *deletion*, and `kubevirt-cdi` is green under the scan while still losing `cdi-cr.yaml`'s content to the in-place `wget -O` on every run, which the Makefile comment records but the header does not. The identity strip has a false-positive direction the header does not argue: a wiping package writing `templates/$(NAME).yaml` would be reported even though the recipe writes it, which no package does today. And the fixture stages `.gitkeep` with `git add --all`, so a developer whose global excludes list that name would get a red for environment rather than defect; `--force` would remove the dependency. The stem match is generous, and the helper's comment now names the two live shapes. A sibling name: `templates/crds.yaml` next to the existing `templates/crds-experimental.yaml` on `gateway-api-crds`. A mention from an unrelated context: `templates/sidecar.yaml` on `objectstorage-controller`, whose Makefile says `sidecar` across its image targets while its recipe clears `templates/` and writes only `templates/controller.yaml`. I fed both to the helper and both come back exempted; a stem the Makefile never mentions is still reported. The duplicate `test:` target in this Makefile is #4157 and stays there. It sits outside the hunks this diff touches, so removing it here would be reaching past what this change is about. One more note, not a defect. Where the comment points at `kubevirt-operator` as the shape that closes the transient-file class, `packages/system/kubevirt/Makefile` is the closer analogue for the `cdi-cr.yaml` half specifically: its `update` downloads nothing and re-applies the parameterization onto a hand-maintained CR with a per-marker self-check. And the failure message offers a third option, staging into a tmpdir the way `etcd-operator-crds` does, in a parenthetical that reads as a gloss on the option before it. What this PR does not fix, and what does. `make update` here still overwrites `templates/cdi-cr.yaml`, which the Makefile comment says plainly. A wholesale overwrite is loud: upstream's object carries neither `podResourceRequirements` nor `cloneStrategyOverride`, and `tests/importer-resources_test.yaml` and `tests/clone_strategy_test.yaml` pin both, so `helm unittest` goes red. The silent case is a partial hand reconciliation that keeps those two and drops something else, because nothing here pins the `ExpandDisks` gate, the `CriticalAddonsOnly` toleration, the `uploadProxyURL` override or the two RBAC documents that preauthorize tenant cloning from `cozy-public`. #4107, stacked on this branch, stops `update` writing the file at all and adds `tests/local_templates_test.yaml`, which pins those. One limit the header does not name, left alone here. The comment strip is anchored to the start of a line, so a trailing comment on a recipe line survives into the searched text and counts as naming the file it mentions. I confirmed it by feeding `wget ... -O templates/cdi-cr.yaml # local-extra.yaml is written by hand` through the filter. Nothing in the tree is written that way, and the header only claims comment lines are dropped, which is what it does. `sed 's/#.*//'` in place of the `grep -v` closes it if it ever matters. Closes #4028. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two entries come close: the website's "developer tooling (the package `Makefile`s)" line, and ccp's "move or rename anything under `hack/`". Neither fires. `make update` still behaves as the website's development page describes, nothing under `hack/` moved or was renamed, and the two files ccp's skills key on (`hack/package.mk`, `hack/common-envs.mk`) are untouched. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(cdi): `make update` in kubevirt-cdi no longer deletes its first-party templates ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Updated Kubernetes CDI package updates to preserve existing template files while refreshing downloaded manifests. - Prevented update operations from clearing first-party and vendored template content. - **Tests** - Added safeguards to detect package update recipes that could remove tracked templates without restoring them. - Added Helm unit test support for the Kubernetes CDI package. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |