## What this PR does
Addresses #3181, where platform log ingestion could fail **silently**:
fluent-bit shipped to `vlinsert-generic` while the monitoring
`HelmRelease` reported `Ready`, and a `vlinsert-generic` `ExternalName`
in `cozy-monitoring` dead-ended at a `tenant-root` VictoriaLogs that did
not exist when the root tenant had monitoring disabled. Logs were
dropped with no failing signal.
Three complementary layers make the failure loud instead of silent:
1. **Readiness gate** —
`packages/extra/monitoring/templates/helmrelease.yaml` now sets a
`poller` `waitStrategy` and a `healthCheckExprs` entry on the
`VLCluster` `status.updateStatus`. A missing/unhealthy log store makes
the `monitoring-system` release **not-ready** instead of flipping
`Ready` the moment the CR is applied. Mirrors the existing seaweedfs
CNPG pattern.
2. **No empty-log-store hole** —
`packages/system/monitoring/templates/vlogs/vlogs.yaml` fails the render
on an empty `logsStorages`. With no entry no `VLCluster` exists, the
readiness gate has nothing to match, and fluent-bit would silently drop
logs — so this is now a loud misconfiguration.
3. **No silent fallback** — the `cozy-monitoring → tenant-root`
`vlinsert`/`vminsert` `ExternalName` redirects in `cozystack-basics` are
created only when the root tenant hosts monitoring. The template reads
the live `Tenant/root` `spec.monitoring` via `lookup` (source of truth,
so it cannot drift from the tenant's real state) and falls back to a new
platform value `monitoring.rootEnabled` (via
`_cluster.monitoring-enabled`) when `lookup` is empty (dry-run / first
render). The default preserves the historical always-create behaviour.
Covered by helm-unittest across all four affected charts (141 tests):
readiness-gate fields, empty-vs-populated `logsStorages` contract,
`ExternalName` present/absent fallback contract, and the platform wiring
into the `cozystack-values` Secret. Also adds the missing `test:` target
to `packages/extra/monitoring/Makefile` so its suite runs in CI.
**Known limitation / follow-up:** the legacy platform monitoring release
installed directly into `cozy-monitoring` via the `cozystack.monitoring`
PackageSource cannot carry the readiness gate, because the engine's
`ComponentInstall`/HelmRelease builder and the vendored
`helm-controller` API v1.4.3 lack `waitStrategy`/`healthCheckExprs`.
That source is not referenced by any active bundle; full coverage there
needs a helm-controller bump plus engine support.
### Release note
```release-note
fix(monitoring): surface missing platform VictoriaLogs as not-ready and stop the dangling vlinsert-generic ExternalName from silently dropping logs (#3181)
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a platform setting (`monitoring.rootEnabled`) to control whether
root-level monitoring is enabled.
* Monitoring redirect ExternalName services are now rendered only when
root monitoring is enabled (defaulting to enabled).
* **Bug Fixes**
* Ensures an explicitly set `monitoring.rootEnabled=false` is preserved
when generating platform configuration.
* Prevents incomplete monitoring deployments by failing rendering when
`logsStorages` is empty.
* Improves VictoriaMetrics readiness by delaying “ready” until the
operator reports an operational status.
* **Tests**
* Added/expanded Helm-unittest suites covering monitoring wiring,
redirects, readiness gating, and `logsStorages` requirements.
* Added a `make test` target to run chart unittests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Re-review on #3094 (IvanHunters, MAJOR): the volumeClaimTemplate label lands on
PVCs only at creation — the StatefulSet controller never re-labels existing PVCs
and vm-operator's PVC reconcile is resize-only. So on an already-deployed cluster
(exactly the #3091 population) the pre-upgrade VM/VL storage PVCs stay unlabeled
and the cleanup hook's selector misses them.
Add migration 49 (mirrors the ClickHouse-keeper backfill in migration 48 for the
sibling #3072). It relabels the pre-existing vmstorage/vmselect/vlstorage PVCs of
every cozystack-managed VMCluster/VLCluster with apps.cozystack.io/application.name
read from the CR's own spec.managedMetadata, so only monitoring's clusters are
touched (a user's own VMCluster carries no such label) and the value always
matches what the cleanup hook selects. Best-effort like #48: a skipped relabel is
no worse than the pre-existing leak, so transient apiserver errors never abort the
platform upgrade. Bump platform targetVersion 49 -> 50.
Also correct the misleading comments (IvanHunters MINOR): spec.managedMetadata is
never applied to PVCs by vm-operator at any current version, so the claim-template
label is the only mechanism — do not imply a v0.71.0 crossover.
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
Re-review on #3094 (IvanHunters): the AND selector kept managed-by=vm-operator,
which on the shipped vm-operator v0.68.4 is not reliably present on the storage
PVCs (it lives on the StatefulSet selector/pod labels, not the claim template),
making the delete fragile. Match by apps.cozystack.io/application.name=<release>-system
alone: that label is stamped on the VM/VL claim templates (67b087ab) and copied
onto the PVCs by IntoSTSVolume even on v0.68.4, is already release-scoped, and is
absent from the CNPG db PVCs (grafana-db/alerta-db carry only policy.cozystack.io
+ cnpg.io/*), so those stay untouched. Regression guard now forbids re-adding
managed-by to the selector.
Also apply the review's discipline items: the cleanup Job keeps only
before-hook-creation (a Failed run survives for post-mortem; hook-succeeded stays
on SA/Role/RoleBinding, cf. #3072) and sets backoffLimit: 0 (best-effort, no retry).
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
Re-review on #3094 (IvanHunters, lexfrei): the release-scoped selector added
in ba5797f4 was a silent no-op on the shipped vm-operator v0.68.4. The
apps.cozystack.io/application.name label lived only under the VMCluster/VLCluster
spec.managedMetadata.labels, and v0.68.4 does not propagate managedMetadata onto
PVCs (that lands only in v0.71.0). So the delete matched nothing and the #3091
storage leak stayed open.
Stamp the label where it actually reaches the PVCs: on
spec.{vmselect,vmstorage,vlstorage}.storage.volumeClaimTemplate.metadata.labels
of the VMCluster/VLCluster CRs (packages/system/monitoring). vm-operator copies
claim-template labels onto the PVCs (IntoSTSVolume) even on v0.68.4, so the
existing cleanup selector (managed-by=vm-operator,apps.cozystack.io/application.name=
<release>-system) now matches and stays scoped to this release. Verified by
rendering the CRs (label resolves to monitoring-system, matching the hook).
Also add the explicit subjects[].namespace on the cleanup RoleBinding (lexfrei
non-blocking) and a helm-unittest suite asserting the label lands on all three
storage claim templates. Corrected the misleading test comment about how the
label reaches the PVCs.
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
Address IvanHunters review on #3094: the PVC delete selector was
release-agnostic (-l managed-by=vm-operator), matching the VM storage
PVCs of every VMCluster/VLCluster in the namespace. If a namespace ever
hosted more than one monitoring release, tearing one down would delete
the other's VictoriaMetrics/VictoriaLogs storage.
Scope the selector to this release via apps.cozystack.io/application.name=
<release>-system, the ownership label the monitoring chart stamps on the
VM CRs' managedMetadata and vm-operator propagates onto the PVCs — mirroring
the merged seaweedfs hook's release scoping. deletecollection is still not
needed: kubectl delete -l lists then deletes individually.
Tests: assert the release-scoped selector and add a regression guard that
the delete never falls back to the bare managed-by=vm-operator selector.
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
Address review on #3094:
- Add --wait=false to both kubectl delete calls so PVC finalizers
(pvc-protection on pods still terminating) can never block the
post-delete hook and stall the HelmRelease teardown.
- Add activeDeadlineSeconds: 120 on the cleanup Job as a backstop.
- Tests: assert the non-blocking flag, plus negative assertions that the
PVC delete stays label-scoped (no --all) and the secret delete stays
name-scoped (no -l/--all).
deletecollection RBAC verb intentionally not added: kubectl delete -l
lists then deletes individually, so the existing delete verb suffices.
BREAKING CHANGE: disabling monitoring now permanently removes its
VictoriaMetrics/VictoriaLogs storage PVCs and the metrics/logs they hold.
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Assisted-By: Claude <noreply@anthropic.com>
When the monitoring tenant module is disabled, the vm-operator storage PVCs
(VictoriaMetrics/VictoriaLogs) and the alerta/grafana TLS secrets were left
orphaned. Add a post-delete cleanup hook that deletes the PVCs by the
managed-by=vm-operator label and the two TLS secrets by name, plus a test
target and helm-unittest suite (the package had neither). CNPG PVCs are not
targeted (CNPG cleans them). The cleanup Job is hardened (non-root, read-only
root fs, dropped caps, resource limits); failures are logged, never blocking
teardown.
NOTE: disabling monitoring now permanently removes its storage PVCs and the
data they hold.
Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io>
Platform log ingestion could fail silently: fluent-bit shipped to
vlinsert-generic while the monitoring HelmRelease reported Ready, and a
vlinsert-generic ExternalName in cozy-monitoring dead-ended at a
tenant-root VictoriaLogs that did not exist when the root tenant had
monitoring disabled. Logs were dropped with no failing signal.
Three complementary changes:
- Readiness (do not stay silently Ready): the monitoring-system
HelmRelease now uses a `poller` waitStrategy and a healthCheckExpr on
its VLCluster (status.updateStatus), so a missing/unhealthy log store
makes the release not-ready instead of flipping Ready as soon as the
CR is applied. Mirrors the seaweedfs CNPG pattern.
- No empty-log-store hole: system/monitoring rejects an empty
logsStorages at render time (helm fail). With no entry no VLCluster is
created, the readiness gate has nothing to match, and fluent-bit would
silently drop logs — so this is made a loud misconfiguration rather
than a documented footgun.
- No silent fallback: the cozy-monitoring -> tenant-root
vlinsert/vminsert ExternalName redirects in cozystack-basics are now
created only when the root tenant hosts monitoring, gated on
_cluster.monitoring-enabled — fed from a new platform value
monitoring.rootEnabled (default true, preserving the historical
always-create behaviour). monitoring.rootEnabled is the single
render-time source of truth: changing it re-renders cozystack-basics
(helm-controller watches the cozystack-values Secret it flows into). A
live `lookup` of Tenant/root spec.monitoring is deliberately NOT used:
cozystack-basics is an operator-generated HelmRelease with drift
detection off, so it re-renders only on install/upgrade/values-change
— never on a reconcile tick or when a lookup result changes — and a
lookup would go stale in both directions. Operators must keep
monitoring.rootEnabled in sync with Tenant/root spec.monitoring.
Covered by helm-unittest across all four charts: readiness gate fields,
empty-vs-populated logsStorages contract, ExternalName present/absent
and null-guard contract, and the platform wiring into the
cozystack-values Secret. The CEL health predicates are exercised live,
not in helm-unittest (which cannot evaluate CEL). Adds the missing
`test:` target to packages/extra/monitoring/Makefile so its suite runs
in CI.
Known limitation (follow-up): the legacy cozystack.monitoring
PackageSource that installs system/monitoring directly into
cozy-monitoring cannot carry the readiness gate, because the engine's
ComponentInstall/HelmRelease builder and the vendored helm-controller
API v1.4.3 lack waitStrategy/healthCheckExprs. That source is not
referenced by any active bundle; full coverage needs a helm-controller
bump plus engine support.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
scooby87