## Why
The `cozystack-operator` Package reconciler builds every `HelmRelease`
it generates with a handful of hardcoded values. Three of those have
caused recurring pain across the project:
- **`Spec.Interval = 5m`** (line 220 before this PR) — too long for E2E
install where dependent HRs reconcile out of order; previously addressed
by a manual `flux reconcile --force` workaround in
`hack/e2e-install-cozystack.bats`.
- **`Install.Remediation{Retries: -1}` + `Upgrade.Remediation{Retries:
-1}`** — semantically "retry forever, never remediate", but the
retry-on-failure timing was tied to `Spec.Interval` (5m). Audit of the
last 30 successful PR runs found that **every** run hit `kubectl wait
hr/seaweedfs-system` timeouts at the 2-min mark, recovering only after
the inline `flux reconcile --force` workaround.
- **`Install.Timeout = 10m` / `Upgrade.Timeout = 10m`** — directly
contradicts the per-Application timeout work in `pkg/config/config.go` +
`pkg/registry/apps/application/rest.go` (commit `7b146cbe`).
`packages/apps/tenant/templates/etcd.yaml` hand-rolls its HR YAML to
bypass this for cert rotation; harbor has had multiple commits chasing
the same problem.
- **`Spec.MaxHistory`** unset → Helm default 5. Test clusters bounce HRs
frequently and accumulate release-history Secrets.
This PR exposes all four as operator flags + helm chart values, with
defaults that preserve current behavior.
## What
### New operator flags
| Flag | Default | HR field |
|---|---|---|
| `--helmrelease-interval` | `5m` | `Spec.Interval` |
| `--helmrelease-retry-interval` | `30s` |
`Spec.{Install,Upgrade}.Strategy.RetryInterval` |
| `--helmrelease-install-timeout` | `10m` | `Spec.Install.Timeout` |
| `--helmrelease-upgrade-timeout` | `10m` | `Spec.Upgrade.Timeout` |
| `--helmrelease-max-history` | `5` | `Spec.MaxHistory` |
### Strategy switch
`Install.Strategy.Name` and `Upgrade.Strategy.Name` are now set to
`RetryOnFailure` (instead of leaving the default `RemediateOnFailure`),
with `RetryInterval` set from the new flag. Functionally equivalent to
the previous `Remediation{Retries:-1}` ("retry forever") since
remediation never fired anyway, but **decouples retry-on-failure timing
from `Spec.Interval`** so failed releases recover at 30s without polling
healthy releases at the same fast cadence.
### Chart exposure
`packages/core/installer/values.yaml` gets five new optional values
(`cozystackOperator.helmRelease{Interval,RetryInterval,InstallTimeout,UpgradeTimeout,MaxHistory}`),
each defaulting to empty so the operator's own default applies. The
chart conditionally renders the corresponding flag only when the value
is set.
## Production safety
- All static defaults match the previous hardcoded values (5m / 10m /
10m / 5). No new code path runs until an admin explicitly sets a flag.
- **One intentional behavior change: failed-release retry cadence**
drops from ~5m (previously coupled to `Spec.Interval`) to 30s
(`RetryInterval` default). This is the whole motivation for the strategy
switch — the recurring `kubectl wait hr/seaweedfs-system` flakes
documented above were caused by the previous coupling. Healthy-release
reconcile cadence is unchanged at 5m in production; CI overrides this to
30s via `--set cozystackOperator.helmReleaseInterval=30s` in
`hack/e2e-install-cozystack.bats` for faster convergence on E2E
clusters.
- `RetryOnFailure` is functionally equivalent to `RemediateOnFailure +
Retries:-1` since neither ever triggers remediation.
- Empty value in the chart → flag not rendered → operator falls back to
its built-in default. (`helmReleaseMaxHistory: 0` — unlimited history
per Helm semantics — is also honored; the chart guards on `ne (toString
…) ""` rather than truthiness so int `0` is not silently dropped.)
## Verification
- `go build ./...` clean
- `go test ./internal/operator/...` passes (covers `Interval`,
`MaxHistory`, `Install/Upgrade.Timeout`, `Strategy.Name`,
`RetryInterval`, `ChartRef` propagation, plus a regression test for
`MaxHistory=0`)
- `helm template installer packages/core/installer` renders cleanly with
default and overridden values; `--set
cozystackOperator.helmReleaseMaxHistory=0` correctly emits
`--helmrelease-max-history=0`
## Followups (deliberately not bundled)
- **Per-component timeout overrides** on `ComponentInstall` (mirroring
the `UpgradeCRDs` pattern from #2427). Charts like etcd that need a 30m
timeout would set it on their `Component` rather than tuning a global
flag. Touches the Package CR API + codegen — bigger lift, separate PR.
- **`Remediation.Retries` as a flag.** No current pain; skip.
- **`createOrUpdateHelmRelease` field preservation.** Currently
preserves `Suspend` but not `Test`/`Rollback`/`Uninstall`. Would
silently overwrite an SRE's manual edit. Minor cleanliness, separate PR.
## Test plan
- [ ] CI passes E2E with `helmReleaseInterval=30s` set in install bats
(existing T1 test path)
- [ ] No regression on production install path (default values)
- [ ] HR objects in a healthy cluster render with
`Strategy.Name=RetryOnFailure` and `RetryInterval=30s`
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Operator and Helm chart now accept CLI flags and Helm values to
configure HelmRelease reconciliation intervals, retry behavior,
install/upgrade timeouts, and history retention.
* **Bug Fixes**
* Operator now validates HelmRelease timing/history inputs on startup
and will exit if values are invalid or non-positive.
* **Tests**
* Added unit and e2e coverage to verify HelmRelease configuration and a
chart install path using a custom reconciliation interval.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Why
helm v3 has a chicken-and-egg with charts that ship their own Namespace
resource:
- **With `--create-namespace`** on the install: helm pre-creates the
namespace via plain kubectl-create (no helm meta annotations); the
chart's own Namespace apply then fails with `already exists`.
- **Without `--create-namespace`**: helm fails immediately because it
can't write its release-secret to a non-existent namespace.
The cozy-installer chart shipped a `Namespace cozy-system` resource
since the chart's introduction (December 2025). Every cold install since
has hit this conflict. It was masked by the 3× retry on `Install
Cozystack` in `.github/workflows/pull-requests.yaml`: attempt 1 fails
with the conflict, attempt 2 sees the existing failed release and takes
the upgrade code path which patch-merges instead of strict-create.
Surfaced when retries on the install step were dropped.
## What
- Remove `Namespace cozy-system` from
`packages/core/installer/templates/cozystack-operator.yaml`.
- Add `packages/core/installer/templates/cozy-system-labels.yaml`: a
pre-install/pre-upgrade hook (ServiceAccount + ClusterRole +
ClusterRoleBinding + Job) that patches `cozystack.io/system=true` and
`pod-security.kubernetes.io/enforce=privileged` onto the namespace.
- Hook RBAC is scoped to `verbs: [get, patch]` on `resourceNames:
[cozy-system]` only.
- `hook-delete-policy: before-hook-creation,hook-succeeded` so the hook
surface only exists during install/upgrade.
Install commands now use `helm upgrade --install --namespace cozy-system
--create-namespace …` — standard pattern, matches kube-prometheus-stack
/ argo-cd / cert-manager.
## Verification
End-to-end on a kind cluster:
- Cold install: 3.3s clean, no retries needed
- Upgrade (re-run): 3.0s, idempotent, namespace stays labeled
- Helm uninstall + re-install: clean
## Test plan
- [ ] CI on this PR passes E2E
- [ ] Existing clusters: namespace already labeled correctly, hook
re-asserts on next upgrade — no migration needed
```release-note
fix(installer): cold installs no longer fail on the chart's Namespace resource colliding with `helm --create-namespace`; cozy-system is now bootstrapped via a pre-install hook.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added an automated pre-install/pre-upgrade step that applies system
identity and pod-security labels to the system namespace during
deployment, run in a hardened one-shot task before normal workloads
start.
* **Chores**
* Separated namespace initialization from the operator install flow so
the operator no longer renders or retains the system namespace directly,
improving installation sequencing.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
With Strategy=RetryOnFailure + RetryInterval=30s plus the bats
--set cozystackOperator.helmReleaseInterval=30s override, the existing
2m wait converges without the manual flux reconcile --force band-aid.
Removing the workaround turns CI itself into the test of the fix.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
- Bump alpine/k8s to 1.33.4 (was 1.32.0, EOL); matches in-tree pin used
by cozystack-api, seaweedfs, vm-disk, and vm-instance hooks.
- Skip hostNetwork=true and not-ready/CNI tolerations on the hosted
variant: managed clusters provide CNI before install, and kube-system
may carry strict PSA there that would reject hostNetwork pods.
- Add packages/core/installer/templates/cozy-system-namespace.yaml gated
on .Values.bareNamespace (default false). The top-level Makefile's
manifests target now passes --set bareNamespace=true and renders
Namespace + operator into _out/assets/cozystack-operator-*.yaml so
the kubectl-apply install path stays self-contained. Helm
install/upgrade callers keep the default and bootstrap via
--create-namespace + the pre-install labeler hook.
- Add a bats assertion in hack/e2e-install-cozystack.bats checking
cozy-system carries pod-security.kubernetes.io/enforce=privileged and
cozystack.io/system=true after helm install.
- Inline comment near hostNetwork=true documenting why dnsPolicy left
at Default (cluster DNS not up at install time;
ClusterFirstWithHostNet would not help).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
## What this PR does
Makes `cozyreport.tgz` actually useful when triaging an E2E failure
deeper than a single pod log. Adds:
- Flux controller logs (helm-controller, source-controller,
notification-controller, kustomize-controller, last 2000 lines each)
- Flux source resources (`HelmRepository`, `OCIRepository`,
`GitRepository`, `ExternalArtifact`)
- Decoded Helm storage secrets for non-Ready HRs (`base64 -d | base64 -d
| gzip -d`)
- Cluster events (all + warning-only filtered file)
- cert-manager `Certificate` / `CertificateRequest` / `Order` /
`Challenge` resources + cert-manager logs
- `cozystack-operator` deployment logs (current + previous)
- `Application` / `ApplicationDefinition` / `Tenant` resources
- Sandbox host context per node: `df`, `free`, `ps`, `dmesg`, `talosctl
logs/dmesg/kubelet/containerd`
New executable `hack/cozyreport-summary.sh` writes a `summary.txt` at
the archive root listing what is broken right now — the first thing to
read when downloading the artifact from a CI failure.
Surfaced from #2500.
### Release note
```
NONE
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a timestamped diagnostic summary report that highlights failing
resources and key cluster issues.
* Expanded diagnostic collection: controller logs (current+previous),
Flux source status, cert-manager certificate details, Helm release
payloads, and Cozystack resource status.
* Added host-level diagnostics (disk/memory/process/dmesg) and improved
event/warning listings to aid troubleshooting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Some workloads (kube-ovn raft, LINSTOR controller / piraeus-server)
fail when replicas start at materially different times due to
image-pull stagger across nodes. Add a DaemonSet-based pre-pull step
that runs before `helm install`, ensuring every node has the images
cached so replicas start within milliseconds of each other.
The script accepts image refs on stdin and creates one container per
image so pulls run in parallel — total time = max(any single image)
rather than sum. The bats test sources image refs directly from the
rendered platform charts via `yq`, walking only PodSpec-shaped
objects, so version bumps stay in sync without a hardcoded list.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
- Resolve hack/cozyreport-summary.sh via SCRIPT_DIR so cozyreport.sh
can be invoked from any working directory, not only the repo root.
- Discover Talos node IPs via `kubectl get nodes` (with the existing
192.168.123.11-13 list as fallback) instead of hardcoding the E2E
sandbox topology.
- Replace broken `awk '$NF != "True"'` filter on the cozystack-apps
loop with a jsonpath check against the Ready condition; the previous
filter compared AGE ("10m" etc.) against "True" and let every
resource through, defeating the non-Ready filter.
- Switch the Flux source CRD summary loop from column parsing to
jsonpath (column positions differ between HelmRepository and
OCIRepository printer columns) and add ExternalArtifact, matching
what cozyreport.sh already collects.
- Add `kubectl get crd` guards before HR/source CRD queries so the
summary script does not abort under `set -eu` when Flux v2 is not
installed.
- Relabel "OOMKilled in last 30 min" to "Recent OOMKilled events
(last 20)" — the kubectl call only limits count, not time.
Surfaced from coderabbit and gemini review on #2553.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
`hack/cozyreport.sh` now collects materially more signal so a triage
artifact can show what is broken rather than just dumping pod logs:
- Flux controller logs (helm-controller, source-controller,
notification-controller, kustomize-controller, last 2000 lines)
- Flux source resources (HelmRepository, OCIRepository, GitRepository,
ExternalArtifact)
- Decoded Helm storage secrets for non-Ready HRs
(`base64 -d | base64 -d | gzip -d`)
- Cluster events (all + warning-only filter)
- cert-manager Certificate / CertificateRequest / Order / Challenge
resources + cert-manager logs
- `cozystack-operator` deployment logs (current + previous)
- `Application` / `ApplicationDefinition` / `Tenant` resources
- Sandbox host context per node: df, free, ps, dmesg,
talosctl logs/dmesg/kubelet/containerd
New `hack/cozyreport-summary.sh` writes a `summary.txt` at the archive
root listing what is broken right now — first thing to read when
triaging an artifact.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
## What this PR does
Bumps the vendored `etcd-operator` chart from `v0.4.2` to `v0.4.3` and
wires the new backup machinery through the cozystack `Etcd` app API.
Upstream v0.4.3 ships two new CRDs — `EtcdBackup` (one-shot snapshot)
and `EtcdBackupSchedule` (periodic snapshots) — along with the
`EtcdCluster.spec.bootstrap` restore field, a new `OPERATOR_IMAGE` env
on the operator deployment, and a replacement of the deprecated
`gcr.io/kubebuilder/kube-rbac-proxy` image. The chart portion of this PR
is a clean re-vendor via `make update` — no local patches.
On top of the bump, `packages/extra/etcd` exposes a new optional
`backup.*` values block. When `backup.enabled=true`, the chart renders
an `EtcdBackupSchedule` referencing the platform `EtcdCluster/etcd` and
a `Secret/etcd-s3-creds` with the S3 credentials expected by the
backup-agent. The values field names (`destinationPath`, `endpointURL`,
`s3AccessKey`, `s3SecretKey`, `schedule`) mirror the existing `postgres`
backup pattern for consistency. The chart `fail`s at template render if
any of the four required S3 fields are empty or if `destinationPath` is
malformed (missing `s3://` prefix, `//` segments, no bucket).
Test coverage: 15 `helm-unittest` cases across
`tests/backup-secret_test.yaml` and
`tests/etcd-backup-schedule_test.yaml` (wired into `make test`, picked
up by `hack/helm-unit-tests.sh`), plus three bats cases in
`hack/e2e-apps/etcd.bats` covering the baseline `Etcd` app, the `backup:
{}` default-off path, and the full `backup.enabled=true` case that
verifies the `EtcdBackupSchedule` materializes and the operator
generates the downstream `CronJob`.
`packages/core/platform/sources/etcd-operator.yaml` now sets
`upgradeCRDs: CreateReplace` on the etcd-operator component, so Flux
applies the new CRDs on upgrade of the chart instead of silently
skipping them.
Depends on #2427.
### Scope deferred to follow-ups
- The one-shot `EtcdBackup` CRD is vendored but not exposed through this
chart — users can still hand-apply the CRD directly.
- `EtcdCluster.spec.bootstrap` (restore from an existing snapshot) is
likewise vendored but not exposed. Both gaps are called out in
`packages/extra/etcd/README.md`.
### Release note
```release-note
feat(etcd): update etcd-operator to v0.4.3 and expose S3 backup schedules on the Etcd app. Set `backup.enabled=true` along with `backup.s3AccessKey`, `backup.s3SecretKey`, `backup.destinationPath`, and `backup.endpointURL` to schedule periodic snapshots.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Scheduled S3 backups for etcd clusters (configurable cron, generated
S3 credentials) and restore/init from S3 or PVC via new backup
resources.
* **Tests**
* New end-to-end tests for etcd lifecycle and backup scenarios; added
Helm unit tests for backup templates and validations.
* **Documentation**
* Backup configuration guide and parameter reference added.
* **Chores**
* Operator chart version bump; RBAC, webhooks, and deployment wiring
updated; added chart test target.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
The `Bootstrap Talos cluster` step in `hack/e2e-prepare-cluster.bats`
wraps `talosctl bootstrap` in `timeout 10 sh -ec 'until talosctl
bootstrap ...; do sleep 1; done'`. On a freshly booted Talos node NTP
has not yet converged, so `talosctl bootstrap` returns
`FailedPrecondition: time is not in sync yet` after ~5-6s. With the 10s
outer timeout the embedded `until/sleep 1` loop only completes a single
attempt before it is killed, and the E2E job fails during `Prepare
environment`.
On slower self-hosted runners NTP typically converges before the first
attempt. On fast ephemeral runners (Oracle VMs) it does not, producing a
consistent flake that only the outer workflow-level 3-attempt retry can
paper over.
Bump the outer timeout to 60s and the inter-attempt sleep to 2s. This
aligns with the other bootstrap gates in the same test (etcd members
check at 180s, drain RPC errors at 60s, node registration at 60s) and
gives NTP plenty of time to converge.
### Release note
```release-note
fix(tests): increase `Bootstrap Talos cluster` timeout in E2E from 10s to 60s to accommodate NTP convergence on fast ephemeral runners.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Enhanced cluster bootstrap process resilience by extending retry
mechanisms and adjusting timing intervals to better tolerate transient
failures during system initialization.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
Replace the ephemeral `emptyDisk` volume on tenant Kubernetes worker VMs
with persistent PVC-backed storage via KubeVirt `dataVolumeTemplates`.
Previously, kubelet certificates, kubeconfig, and containerd state were
lost on VM reboot, causing the node to fall out of the tenant cluster.
**Changes:**
- Rename `ephemeralStorage` field to `diskSize` in NodeGroup
configuration
- Replace `emptyDisk` volume with `dataVolumeTemplates` using CDI blank
source
- Add `cdi.kubevirt.io/storage.usePopulator: "false"` annotation to work
around CDI volume population issues with LINSTOR CSI
- Add optional per-nodeGroup `storageClass` field (defaults to cluster
default)
- Set fixed 2Gi ephemeral-storage resource for virt-launcher pod
- Rename internal mount paths from `/ephemeral` to `/persistent`
- Add helm unit tests (`cluster_test.yaml`) covering dataVolumeTemplates
defaults, custom disk size, storageClass presence/absence, fixed
ephemeral-storage, disk name consistency, and CDI populator annotation
- Quote storage values in templates (`diskSize`, `storageClassName`) for
Helm correctness
- Guard bind-mount commands with `mountpoint -q` checks and create
target directories to handle fresh images and cloud-init retries
- Set `blockSize.matchVolume` on worker node disks for automatic block
size detection, fixing DRBD 4Kn compatibility with QEMU
**Known gotchas:**
- CDI volume population (prime PVC rebind) causes `ClaimMisbound` errors
with LINSTOR CSI driver, regardless of StorageClass (`replicated` or
`local`). The `usePopulator: false` annotation switches to the legacy
import path which works correctly. This is a known CDI issue documented
in
[kubevirt/containerized-data-importer#3146](https://github.com/kubevirt/containerized-data-importer/issues/3146).
- The `replicated` StorageClass with DRBD `suspend-io` quorum policy can
cause transient VM IO errors during LINSTOR satellite restarts (quorum
loss). The `local` StorageClass is recommended for worker node disks.
When `storageClass` is not specified in a nodeGroup, the cluster default
StorageClass is used.
- Existing clusters will trigger a rolling replacement of all worker
nodes on upgrade due to changed KubevirtMachineTemplate hash.
- DRBD volumes use 4096-byte logical sectors (`block-size 4096`).
Without explicit block size configuration, QEMU defaults to 512-byte
sectors, causing immediate IO errors and VM pause. The
`blockSize.matchVolume` setting in the disk spec ensures KubeVirt
auto-detects the correct sector size from the volume.
**Tested on a live cluster:**
- Verified DataVolume provisioning with `local` StorageClass
- Verified nodes join the tenant cluster and reach Ready state
- Verified node survives VM reboot (pod deletion) and returns to Ready
with the same Node object, reusing kubelet certificates from persistent
disk
### Screenshots
N/A — no UI changes.
### Release note
```release-note
feat(kubernetes)!: replace ephemeral emptyDisk with persistent storage for worker nodes
Worker node VMs now use persistent PVC-backed disks instead of ephemeral emptyDisk volumes. Kubelet certificates, kubeconfig, and containerd state survive VM reboots, preventing nodes from falling out of the tenant cluster.
BREAKING CHANGE: The `ephemeralStorage` field in nodeGroups has been renamed to `diskSize`. A new optional `storageClass` field allows per-nodeGroup storage class selection. Users must update their values overrides accordingly.
Worker node disks now use `blockSize.matchVolume` to automatically detect the underlying volume's sector size, ensuring compatibility with DRBD-backed storage classes that use 4096-byte logical sectors.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Optional storageClass parameter to select storage class for worker
node persistent disks (defaults to cluster default).
* Makefile test target added to run Helm unit tests for the chart.
* **Configuration Changes**
* Worker node storage parameter renamed from ephemeralStorage to
diskSize (default: 20Gi).
* Kubelet/containerd state moved to persistent disk paths and node
ephemeral-storage accounting fixed to 2Gi; disk provisioning aligns
block sizing with backing volumes.
* **Documentation**
* Updated docs and chart/schema to reflect the persistent disk model and
new fields.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
- Regenerate deepcopy artifacts to remove orphaned PostgreSQLParameters methods
- Update e2e test to use string value for max_connections parameter
- Fix template to avoid .Values mutation and use quote function for safe YAML escaping
Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Integrate the HAMi GPU virtualization addon from main while
keeping the persistent-storage diskSize and storageClass fields
introduced by this branch.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
## What this PR does
Adds GPU observability to Cozystack: five Grafana dashboards, a set of
VictoriaMetrics recording rules with a throttle-regression alert, and
reference manifests for wiring the NVIDIA GPU Operator and DCGM Exporter
on Talos when running GPU workloads directly in pods.
### Components
1. **Dashboards** — `dashboards/gpu/*.json`, registered in
`packages/system/monitoring/dashboards-infra.list`. All follow the
project convention: the Prometheus data source is selected through the
`$ds_prometheus` template variable.
- `gpu-performance` — per-GPU utilization (NVML, tensor pipe, graphics
engine, memory copy), VRAM, power, temperature, and health (XID errors,
power/thermal throttling). All metrics are shown per-GPU, filterable by
the `$Hostname` selector.
- `gpu-fleet` — cluster-wide inventory, capacity, utilization, and
per-node power draw as percentage of TDP.
- `gpu-tenants` — per-namespace allocation and 24-hour GPU-hour/kWh
accounting; utilization and power are shown per-node (DCGM cannot
attribute hardware metrics to namespaces).
- `gpu-quotas` — allocated vs. requested GPUs, pending pods, and a
per-namespace requests/limits table. All counts derive from the
`namespace:gpu_count:allocated` recording rule, which excludes
`Failed`/`Succeeded` pod carryover.
- `gpu-efficiency` — cluster and per-GPU efficiency: tensor saturation,
util-per-watt, and per-GPU power/thermal throttle fractions.
2. **Recording rules and alert** —
`packages/system/monitoring-agents/alerts/gpu-recording.rules.yaml`.
Four rule groups:
- `gpu.recording.cluster.1m` — cluster-wide totals consumed by overview
panels.
- `gpu.recording.node.1m` — per-node hardware aggregates (grouped by
`Hostname`) plus per-namespace allocation counts from kube resource
requests; filters `cozy-*`/`kube-*`.
- `gpu.recording.efficiency.1m` — per-GPU tensor saturation and
util-per-watt, plus per-GPU power/thermal throttle fractions bounded via
`clamp_max(..., 1)`.
- `gpu.recording.throttle.validation.5m` — houses the
`GPUThrottleFractionOverOne` warning alert that fires if the pre-clamp
throttle fraction exceeds 1.0. This guards against DCGM counter-unit
drift between GPU families; the `/1e9` divisor is verified on NVIDIA A10
with DCGM 3.x.
Rules are safe on clusters without DCGM — they evaluate to empty series
when no matching metrics are scraped.
3. **Reference manifests** under
`packages/system/gpu-operator/examples/`. These are **not** templates;
they document one working configuration:
- `values-native-talos.yaml` — Cozystack Package values for the
native-pod scenario.
- `dcgm-custom-metrics.yaml` — ConfigMap extending the DCGM CSV with
profiling, ECC, throttling, and energy counters used by the dashboards
and recording rules.
- `nvidia-driver-compat.yaml` — DaemonSet that stages
`libnvidia-ml.so.1` and `nvidia-smi` from the Talos glibc tree into a
location the GPU Operator validator inspects. Workaround for
[NVIDIA/gpu-operator#1687](https://github.com/NVIDIA/gpu-operator/issues/1687).
Requires `pod-security.kubernetes.io/enforce: privileged` on the target
namespace when the cluster enforces baseline/restricted
PodSecurityStandards.
- `README.md` — explains the two deployment paths (sandbox vs. native),
which DCGM metrics each dashboard depends on, and how the pieces fit
together.
4. **Cross-validation test** — `hack/check-gpu-recording-rules.bats`
enforces that every recording-rule reference in tracked dashboards
resolves, and that every DCGM metric referenced by the rules is declared
in either the upstream default CSV snapshot
(`hack/dcgm-default-counters.csv`) or the custom CSV shipped under
`examples/`.
### Why reference manifests, not templates
The out-of-the-box `packages/system/gpu-operator/values-talos.yaml`
targets **sandbox workloads** — GPUs passed through to KubeVirt VMs via
VFIO, driver disabled on the host, device plugin disabled. That is the
sensible default for Cozystack today.
Running GPU workloads directly in pods on Talos is also possible but
requires a different configuration that makes assumptions not every user
shares:
- the NVIDIA Talos system extension must be installed on GPU nodes,
- the operator's own driver and toolkit components must be disabled
because the extension provides them,
- the validator's hardcoded paths need a workaround until
NVIDIA/gpu-operator#1687 lands upstream.
Shipping those as active templates would silently impose those
assumptions. Shipping them as reference files next to the package lets
operators opt in with eyes open and understand the moving parts before
applying them.
### Scope note
The dashboards and recording rules are useful independently of the
reference manifests — any cluster that scrapes DCGM Exporter (however it
is installed) will populate them. The reference manifests are
self-contained and do not change any default behavior of the
`gpu-operator` package.
### Screenshots
<img width="2824" height="1055" alt="2026-04-27_12-36-52"
src="https://github.com/user-attachments/assets/71f85de7-1c80-4817-b36c-aeb1c2bdd39a"
/>
<img width="2913" height="1255" alt="2026-04-27_12-36-39"
src="https://github.com/user-attachments/assets/7a54daca-0727-4510-938c-080d31bdc229"
/>
<img width="2932" height="1251" alt="2026-04-27_12-36-30"
src="https://github.com/user-attachments/assets/f6c6c215-3d05-400c-b21c-29f267c93f23"
/>
<img width="2864" height="1222" alt="2026-04-27_12-36-00"
src="https://github.com/user-attachments/assets/e9d34bab-ade5-434b-92c3-f36712cf68d9"
/>
<img width="2958" height="1226" alt="image"
src="https://github.com/user-attachments/assets/0047a5f8-12a5-4cda-8676-041de139cb76"
/>
### Release note
```release-note
feat(monitoring): add GPU observability — five Grafana dashboards (performance, fleet, tenants, quotas, efficiency), VictoriaMetrics recording rules with a throttle-regression alert, and reference manifests under packages/system/gpu-operator/examples/ for running GPU workloads natively in pods on Talos.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added five GPU dashboards for performance, efficiency, fleet overview,
quotas/allocation, and tenant usage (utilization, power, throttling,
allocation, billing).
* Added cluster recording rules and a throttling validation alert; new
Prometheus-derived metrics and per-GPU rankings/timeseries.
* **Documentation**
* Added GPU Operator examples and README documenting native Talos setup
and required DCGM counters.
* **Tests**
* Added validation tests ensuring dashboard ↔ recording-rule and DCGM
metric consistency.
* **Chores**
* Registered new GPU dashboards in infra listings.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Adds --helmrelease-interval (default 5m, matching today's behaviour) to the
operator and a corresponding cozystackOperator.helmReleaseInterval value
on the cozy-installer chart (default empty -> no flag rendered ->
operator default 5m applies).
E2E install sets it to 30s. Rationale: with the 5m default, dependency-
blocked HRs (waiting on cert-manager webhooks, CRDs) are requeued only
every 5 min, producing an 8-10 min dead zone in the E2E install where
the fast pack of HRs is Ready but the slow tail hasn't been retried yet.
30s collapses the gap.
Production defaults are unchanged. The override is opt-in per install.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Incorporate images.waitForKubeconfig support from main while
keeping the persistent-storage diskSize and storageClass fields
introduced by this branch.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Address review feedback from coderabbitai on
hack/e2e-apps/etcd.bats:89: the test verifies that the chart renders
EtcdBackupSchedule and that the etcd-operator materializes a CronJob,
not that backups fire. Use "0 0 1 1 *" so no Jobs run against the
intentionally invalid endpoint during the ~6-minute test window.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback from coderabbitai on
hack/e2e-apps/etcd.bats:39-40: replace the sleep-then-wait pattern with
a wait_etcd_hr_ready helper that polls hr/etcd until the Cozystack API
reconciles the Etcd CR and only then waits for condition=ready.
kubectl wait exits immediately when the resource does not yet exist,
which caused spurious failures when HelmRelease creation took longer
than 5s.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback from coderabbitai on
hack/e2e-apps/etcd.bats:9-14: drop the '|| true' guards in setup() so
real teardown failures (RBAC errors, stuck finalizers, wait timeouts)
fail the test setup instead of letting the next test run against stale
HelmRelease, Secret, or EtcdBackupSchedule state. --ignore-not-found
still covers the empty-namespace case on a clean run.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback from coderabbitai on
hack/e2e-apps/etcd.bats:19-21: redirect stdout to fd 3 first, then dup
stderr onto the new fd 1 so failures from kubectl (NotFound, RBAC,
timeouts) reach the test diagnostics stream instead of being swallowed
by the bats-captured default stdout.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The e2e test still used the old ephemeralStorage field name,
which triggers the migration guard fail() in the Helm template.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
cozytest.sh does not implement the bats 'run <cmd>' helper and '$status'
variable, so the 'Create Etcd with empty backup block (disabled by
default)' test died with 'run: not found' on every attempt. Replace the
bats idiom with plain shell: use '!' prefix and redirect stderr to
assert the resources absent-when-backup-disabled invariant directly.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
On fast ephemeral runners a freshly booted Talos node has not yet
converged NTP when the previous 10s outer timeout fired, so the
embedded 'until talosctl bootstrap; do sleep 1; done' retry loop only
got one 5-6s attempt before the loop was killed. Every RPC returned
'FailedPrecondition: time is not in sync yet' and E2E failed in the
Prepare environment step.
Bump the outer timeout to 60s and the inter-attempt sleep to 2s. This
aligns with the surrounding bootstrap gates (etcd members check at
180s, drain RPC errors at 60s, node registration at 60s) and gives
NTP plenty of time to converge on ephemeral runners.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Fourth review round fixes.
Simplify backup validation by dropping placeholder-regex guards and switching default values of s3AccessKey, s3SecretKey, destinationPath, and endpointURL from angle-bracketed hints to empty strings. A single 'required' check on each field is now the whole validation for presence; the regex guard was brittle (silently stopped matching if the hint ever changed) and dropping it makes the values.yaml defaults match the behavior contract the README describes. s3:// prefix and '//' rejection still live in etcd-backup-schedule.yaml as template-time fail branches.
Add helm-unittest suites in packages/extra/etcd/tests/ covering both templates end-to-end: rendered output assertions for the happy path (bucket/key split, region passthrough, conditional rendering of optional fields) and failedTemplate assertions for every fail branch. A 'test' target is wired into the package Makefile so hack/helm-unit-tests.sh picks it up on 'make unit-tests'.
Add type: Opaque to backup-secret.yaml for consistency with the mongodb template.
E2E: wait for HelmRelease, Secret, and EtcdBackupSchedule deletion in setup() so the empty-backup case does not flake against lingering state from the previous test. Replace the fictitious minio-e2e hostname with no-such-endpoint.invalid and explicitly document that the e2e verifies schedule/CronJob materialization, not backup execution.
Restore via EtcdCluster.spec.bootstrap and the one-shot EtcdBackup CRD are still not exposed through this chart; both are explicit follow-ups called out in the README.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Use name='etcd' in all three e2e cases. The chart pins the Helm
release name to 'etcd' via check-release-name.yaml, so any other
name causes template rendering to fail before the HelmRelease
can reconcile. This is why the tenant chart hardcodes HR name
'etcd' in packages/apps/tenant/templates/etcd.yaml.
- Consolidate all backup validation guards into
etcd-backup-schedule.yaml so Helm's alphabetical render order no
longer determines which failure a user sees first. Previously, a
bad destinationPath would be masked by the earlier placeholder
check in backup-secret.yaml.
- Use regex match '^<.*>$' for placeholder detection instead of
exact string equality. The previous check silently stopped
firing if anyone updated the default placeholder string in
values.yaml; the regex form catches any angle-bracketed default.
- Reject destinationPath strings containing '//' after the s3://
prefix. Previously 's3://bucket//subdir' silently produced a key
beginning with '/', confusing S3 clients and retention policies.
- Document the backup contract in README.md: what enabling it
produces, which fields are required, why placeholders are
rejected, and that restore is a known follow-up gap.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Mark backup.enabled as optional in the values typedef so
generated JSON schema no longer lists it in 'required'.
Previously, 'backup: {}' (relying on the enabled:false default)
failed schema validation at CRD admission.
- Validate backup.destinationPath has s3:// prefix. Without this
check, passing a path without the scheme silently used the
first path segment as the bucket name.
- Add tail newline to values.schema.json to match repo convention
(cozyvalues-gen output lacks it — worth fixing in that project
separately).
- Add bats setup() that clears tenant-test etcd resources before
each test so a failure mid-test does not leak into subsequent
tests. Hardcoded chart-level resource names ('etcd',
'etcd-s3-creds') make two Etcd apps in the same namespace
collide; setup() enforces a single owner per test.
- Add dump_diagnostics() helper that dumps operator logs + the
relevant CRD state on wait-for-ready timeouts, so CI failures
surface actionable context instead of bare exit code 124.
- Add a third e2e case 'Create Etcd with empty backup block' that
asserts backup:{} admits and produces no backup resources — a
regression test for the required:['enabled'] bug fixed here.
Known scope: the upstream EtcdCluster.spec.bootstrap restore field
(also new in v0.4.3) is not exposed through this values.yaml yet.
Restoring a cluster from an existing snapshot is a follow-up PR.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Review found three issues:
1. The guard in backup-secret.yaml only triggered on empty strings,
not on the placeholder defaults ('<your-access-key>' etc). A user
enabling backup.enabled=true without overriding the values would
produce a Secret containing the literal placeholder, and the
backup CronJob would fail at runtime with an opaque 403 far from
the source. Add explicit template guards that fail rendering when
any of the four required fields still hold their placeholder
default.
2. The backup e2e test waited on .spec.schedule of the newly-created
EtcdBackupSchedule, which is set at apply time — the wait was
tautological and did not prove the etcd-operator reconciled
anything. Replace it with a wait on the EtcdCluster's Ready
condition plus a poll for the CronJob the operator generates
from the schedule (selected via the
etcd.aenix.io/etcdbackupschedule-name label).
3. The backup e2e test also did not wait for EtcdCluster readiness
before tearing down. Add the same Ready wait as the baseline
test, and verify the Secret contains the real access key value
rather than just existing.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Adds two bats cases: a baseline Etcd managed-app deployment and a
deployment with backup.enabled=true that verifies the
EtcdBackupSchedule resource and the S3 credentials Secret are
materialized. The backup case uses fake S3 credentials — the
operator will not successfully push backups, but the schedule
resource itself is reconciled, which is what this test validates.
Also hardcodes the EtcdBackupSchedule and backup Secret names to
'etcd' / 'etcd-s3-creds' to match the existing hardcoded
EtcdCluster name in the chart (one etcd per tenant namespace).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
gpu-fleet.json references DCGM_FI_DEV_POWER_MGMT_LIMIT for its
"TDP vs draw" panel, but the custom DCGM Exporter CSV did not declare
it, so the panel silently rendered "No data" on clusters using that
config. Declare the counter, fix the dashboards table in the
gpu-operator examples README, and add a bats test that cross-checks
every DCGM_FI_* reference in tracked dashboards and recording rules
against the union of the upstream default set (snapshotted under
hack/) and the project's custom CSV.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Catch dangling references at PR time: every recording-rule name used
inside a tracked GPU dashboard must exist in
packages/system/monitoring-agents/alerts/gpu-recording.rules.yaml. The
first iteration of gpu-efficiency.json shipped panels keyed on
pod:tensor_saturation:avg5m without the rule defined; the test fails
on exactly that class of bug.
Scoped to dashboards listed under gpu/* in dashboards-infra.list, so
untracked drafts stay out of scope until they are registered. Reverse
direction (rule defined but unused) is intentionally NOT enforced —
some rules exist for ad-hoc PromQL or upcoming dashboards.
Auto-discovered by make bats-unit-tests via hack/cozytest.sh.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
The test body asserts .status.history[].status extraction, but the
test name still referenced the old installFailures counter (leftover
from when the guard used that field before switching to status.history
to avoid ClearFailures zeroing the counters on successful reconcile).
Address review feedback from coderabbitai on
hack/remediation-guard.bats:84: rename so grep for what the test
actually pins matches the test name.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Log .status.history regardless of content so a silently empty result
(Flux field rename) shows up in CI logs, and treat empty history on
a Ready HelmRelease as a distinct failure. A Ready HR by definition
has at least one snapshot; anything else is a shape-drift signal.
- Replace the unquoted heredoc in remediation-guard.sh with a printf |
grep pipeline. printf %s treats statuses as literal payload (no $
expansion surprises for future callers), grep --quiet --extended-regexp
returns exit status the caller can forward directly.
- Share the etcd-absent values file between both invariant tests
(packages/apps/kubernetes/tests/values-ci-no-etcd.yaml) instead of
duplicating the --set block.
- Fix typo "override applied" -> "override is applied" in the
Kubernetes ApplicationDefinition.
- Add a coupling comment in the ApplicationDefinition annotation that
points at the wait-for-kubeconfig init deadline in _helpers.tpl, so
a future operator raising the HR timeout updates the init deadline
too.
- Clarify the per-annotation timeout comment in rest.go so it stops
implying the feature is Kubernetes-only (it is not - only today's
one user is).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
17 child HelmReleases (cilium, coredns, csi, cert-manager,
metrics-server, ...) referenced *-admin-kubeconfig via
kubeConfig.secretRef and rendered even when _namespace.etcd was empty.
On an etcd-less tenant each one sat in NotReady forever because the
admin-kubeconfig Secret only exists after a KamajiControlPlane
reconciles, and KamajiControlPlane now only renders when etcd is set.
The outcome contradicted the "beacon only" contract claimed in the
soft-skip commit.
Extend the existing addon guards to also require _namespace.etcd, and
wrap the four unconditional HelmReleases (csi, metrics-server,
prometheus-operator-crds, volumesnapshot-crd) plus the always-on
cilium/coredns HR resources in the same gate. Add an invariant bats
test that renders the whole chart with etcd empty and asserts zero
HelmReleases reference *-admin-kubeconfig.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Flux helm-controller's ClearFailures() zeroes installFailures and
upgradeFailures on every successful reconciliation (see the upstream
HelmReleaseStatus method). The previous guard ran after the HelmRelease
was Ready, at which point the counters were always 0 - the assertion
was vacuous and would have passed against a reverted fix.
Switch to .status.history, which retains per-revision release
Snapshots that survive a subsequent successful reconciliation. A
remediation cycle leaves behind a Snapshot with status=uninstalled
(the install-remediation code path) or status=failed (Helm release
failure that remediation then uninstalled). Either one signals the
race actually fired.
Rewrite the bats unit tests to cover: empty history, deployed-only,
deployed+superseded (happy path - not detected), single failed,
single uninstalled, uninstalled-then-deployed, and deployed-then-failed
(all detected). The pinned-shape test feeds a realistic HR status
snippet through yq the same way run-kubernetes.sh does via kubectl
-o jsonpath.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The soft-skip wrap in cluster.yaml only silenced Cluster and
KamajiControlPlane rendering when _namespace.etcd is empty. The three
CP-side Deployments (cluster-autoscaler, kccm, kcsi-controller) still
rendered, their wait-for-kubeconfig init containers CrashLoopBackOff'd
forever (no KamajiControlPlane = no admin-kubeconfig Secret),
HelmRelease hit its 15m wait timeout and triggered the very install
remediation cycle the rest of this PR prevents. Self-contradiction.
Wrap each of the three Deployment templates in
{{- if .Values._namespace.etcd }}...{{- end }} so they render only
when there is a DataStore to back them. Add an invariant bats test
that renders the whole chart with etcd empty and asserts zero
Deployments reference *-admin-kubeconfig.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
run-kubernetes.sh extracts .status.installFailures and
.status.upgradeFailures via kubectl -o jsonpath. If a future flux
release renames the counters, kubectl returns empty, the guard reports
no cycle, and e2e silently misses real remediation loops.
Add a bats unit test that feeds a pinned HelmRelease v2 status snippet
through the same jsonpath and asserts the extraction still yields the
expected values. Also leave a pointer comment in run-kubernetes.sh so a
future flux bump surfaces the version coupling in review.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The per-template unittests in packages/apps/kubernetes/tests/ assert
that cluster-autoscaler, kccm, and the csi controller each mount the
admin-kubeconfig Secret optional and carry the wait-for-kubeconfig
init. That locks in today's three Deployments by name - a fourth
Deployment that mounts the same Secret but forgets the guard would
slip past them.
Add a bats-unit test that renders the entire chart, enumerates every
Deployment whose spec mounts a Secret ending in -admin-kubeconfig, and
asserts optional:true plus wait-for-kubeconfig init on all of them.
Verified by temporarily removing optional:true from csi/deploy.yaml:
the test correctly flagged invariant-kcsi-controller as an offender.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Shell && and || have equal precedence and left-to-right associativity,
so the previous guard parsed as (((A && B) || C) && D) and silently
passed on the canonical failure mode: install_failures=1 with an empty
upgrade_failures.
Extract the check into helmrelease_has_remediation_cycle() in a
dedicated helper sourced from run-kubernetes.sh, and add unit tests
under hack/remediation-guard.bats that pin the expected behavior for
every combination of empty, zero, and positive counter values.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Before cleanup, inspect the parent HelmRelease installFailures and
upgradeFailures counters. A non-zero value means flux helm-controller
hit its wait timeout, ran install/upgrade remediation (uninstall),
and re-installed - the exact race condition this PR closes. Fail the
bats test in that case so the signal surfaces in CI instead of being
masked by a green retry.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
The aggregated API previously accepted tenant names containing hyphens
(e.g. `foo-bar`) because `ValidateApplicationName` delegated entirely to
`IsDNS1035Label`, which permits them. The tenant Helm chart's
`tenant.name` helper then rejected the release at template time — so
users saw a successful `kubectl apply` followed by a confusing
Flux/HelmRelease reconciliation error.
This PR extends `ValidateApplicationName` with a `kindName` parameter
and enforces an alphanumeric-only rule (`^[a-z][a-z0-9]*$`) for the
Tenant kind. A `TenantKind` constant centralizes the kind string, and
`REST.validateNameFormat` wraps the check symmetrically with the
existing `validateNameLength`. The tenant chart README is updated to
match the already-correct website documentation.
Test coverage includes:
- Unit tests for the validation package (format + error message
contract)
- REST wrapper and Update→Create fall-through path tests with a fake
client
- E2E BATS regression test with explicit exit-code checks and cleanup
Fixes#2375
### Release note
```release-note
[api] Tenant names containing dashes are now rejected at API level during creation, with a specific error message, instead of being silently accepted and failing later during Helm reconciliation.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Tenant names now require lowercase letters and digits only, must start
with a letter; dashes are rejected with a tenant-specific error.
* **Documentation**
* Tenant naming guidelines updated to reflect the stricter
alphanumeric-only requirement.
* **Tests**
* New unit and end-to-end tests added to verify tenant naming
enforcement and tenant-specific error messaging.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The api/apps/v1alpha1/* subtags share the same commit as the release
tags. git describe --exact-match picks the first match alphabetically,
returning api/apps/v1alpha1/vX.Y.Z instead of vX.Y.Z, which produces
invalid Docker image tags.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Four follow-ups from review round 4:
1. BLOCKER: the Update(forceAllowCreate=true) path delegates to Create() when the object does not yet exist (rest.go:452) — the typical kubectl apply upsert flow. Add TestUpdate_ForceAllowCreate_RejectsTenantDashName using a fake client so a future refactor of that delegation cannot silently bypass the tenant name check that r.validateNameFormat alone cannot catch.
2. BLOCKER: the e2e BATS test used || true inside the command substitution, which swallowed the kubectl exit code. Rework the test to capture exit code and stdout+stderr explicitly, then assert the exit code is non-zero before asserting on the error message. This distinguishes validation-success (kubectl exit 0 — regression) from environmental failures (exit non-zero but wrong message) from the happy path.
3. Extract TenantKind = "Tenant" as a named constant in the validation package with a comment pointing at the upstream ApplicationDefinition source of truth, and switch the kindName check to use it.
4. Add a clarifying comment on TestValidateApplicationName_TenantLengthFallthrough that it pins an architectural layering decision and is not a user-facing requirement, so a future promotion of tenant length into tenant-specific wording is a legitimate change rather than a test regression.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Three follow-ups from review:
1. Add defensive cleanup of the foo-bar tenant before and after the e2e regression test. If a prior run left the object in the cluster (e.g. a transient validation regression), the test no longer inherits that state; and if the test itself trips a regression in the future, the cluster is left clean for subsequent tests.
2. Switch kubectl --validate=false to --validate=ignore. The bool form was deprecated in kubectl 1.25 and only kept as an alias; --validate=ignore is the modern, stable spelling.
3. Drop the brittle Contains("63") assertion in TestValidateApplicationName_TenantLengthFallthrough. It pinned the test to the exact DNS-1035 error text from k8s.io/apimachinery, which could break on unrelated upstream wording changes. The surviving assertions still cover the intent: an error exists and it is not the tenant-specific message.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Follow-up from review round 2:
The BATS regression test now pins a namespace precondition, disables kubectl client-side validation with --validate=false so the request is guaranteed to reach the server-side name check, and documents the intent of each assertion. Previously a client-side schema rejection could have produced a false negative on the tenant-specific error grep, and a missing tenant-root namespace could have produced an unrelated failure that was hard to diagnose.
Also pin the edge case where a tenant name consists of valid characters but exceeds the DNS-1035 63-char label limit. The resulting error is intentionally the generic DNS-1035 message because length is not a tenant-specific constraint — the package-level function is not responsible for the stricter Helm-release-prefix length budget that REST.validateNameLength enforces.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two follow-ups from review:
1. The original tenantNameRegex ^[a-z0-9]+$ let leading-digit names like 123foo slip past the tenant check and receive a generic DNS-1035 error, which defeated the goal of surfacing a tenant-specific message. Require a leading lowercase letter so every tenant-invalid name returns the tenant-contract error.
2. Add a BATS regression test in hack/e2e-install-cozystack.bats that exercises the aggregated API end-to-end. Unit tests construct REST{kindName: "Tenant"} by hand, so a future change to ApplicationDefinition kind registration could break real behavior without breaking the unit tests.
Also pin the error-message contract with a new TestValidateApplicationName_TenantErrorMessage that asserts every tenant-invalid input returns a message containing 'tenant names must' (not the generic DNS-1035 text).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The disk_usage helper shells out to 'du -sh $path' for reporting in
the warning message. On the exact directories this script is meant
to warn about (a /var/lib/containerd accumulating millions of files
from months of unpruned builds), traversing the tree with du can
take minutes and stall the preflight indefinitely.
Wrap the call in 'timeout 5s' so the helper returns quickly even on
a pathologically slow filesystem. If the timeout binary is absent
(e.g. minimal busybox userland), the pipeline still exits 0 via the
existing '|| true' guard and usage stays empty — the warning still
prints, just without the size detail — so the change is backward
compatible.
Addresses an inline review comment from gemini-code-assist on the
open PR.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address three review findings on the bats suite:
- The 'standalone containerd service active' and 'standalone docker
service active' tests previously asserted 'systemctl disable --now
<service>' but did not require the 'sudo ' prefix, so the two-
service test was the only guard against a silent drop of the
prefix. Extend both single-service tests to require the prefix
exactly as the 'both services active' test does.
- The 'both services active' test now captures the exit code
explicitly with 'bash ... || status=$?' and asserts '$status -eq
0'. The script contract ('always exits 0') was previously enforced
implicitly via 'set -e', which produces a generic test failure on
a regression instead of a contract-specific error. The explicit
check locks in the contract and makes regressions readable.
- New test: 'docker socket paths with glob chars do not expand' —
sets COZYSTACK_DOCKER_SOCKET_PATHS to a literal glob against real
directories and asserts no docker warning fires. Locks in the
array-based parsing of the path list.
- Extend the file header with a 'title syntax constraints' section
documenting how cozytest.sh's awk parser treats double quotes and
non-alphanumeric characters in @test titles, so future
contributors do not stumble on the parser's quirks.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
'set -euo pipefail' does not include 'set -f', so the previous
'for sock in $DOCKER_SOCKET_PATHS' loop relied on both word splitting
AND unintended glob expansion. If the variable ever contained a
literal '*' or '?' — for example a test setting
COZYSTACK_DOCKER_SOCKET_PATHS='/run/docker-*.sock' against a tree
that happens to have real files matching the pattern — the loop
would iterate over directory entries instead of the intended socket
paths, producing false-positive docker warnings.
Replace the implicit split+glob loop with 'read -ra' into a bash
array and iterate 'for sock in "${_socks[@]}"'. This keeps the
space-separated input format, disables glob expansion, and removes
the fragile word splitting that shellcheck otherwise has to be
talked out of.
Covered by a new regression test ('docker socket paths with glob
chars do not expand') that sets the env var to a literal '*' pattern
pointing at real directories; without this fix the test fails with
an unexpected docker.service warning.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
myasnikovdaniil