From d3b4b305b036e56550f183032ee462ff8dc708ea Mon Sep 17 00:00:00 2001 From: Zeke Date: Fri, 24 Jul 2026 11:29:05 -0700 Subject: [PATCH] feat(chart): ship the alert rules as a PrometheusRule + ServiceMonitor scrape knobs (P2-6) Closes the monitoring asymmetry #758 introduced (the dashboard was auto-provisioned but the alert rules stayed a bare file) and lands plan item P2-6. - templates/prometheusrule.yaml: a gated PrometheusRule embedding the rule bodies via .Files.Get. The alerts file MOVED into the chart (deploy/prometheus/ -> deploy/helm/ironcache/alerts/) because .Files.Get is chart-relative -- same single-source move the dashboard got in #758. The file is still exactly a `groups:` document, so a plain Prometheus can load it via `rule_files:` and `promtool check rules` it unchanged; the template just indents it under `spec:`. Verified the render parses with all 6 groups / 8 active rules intact (the 2 remaining `alert:` hits in the file are its documented commented-out INFO-only block, not lost rules). Includes the operator gotcha: the Operator only adopts rules its `ruleSelector` matches, so metrics.prometheusRule.additionalLabels exists for the usual `release: ` selector. - servicemonitor.yaml (P2-6): sampleLimit, scrapeTimeout, relabelings, metricRelabelings and additionalLabels passthrough -- the levers for staggering/undersampling scrapes and dropping high-cardinality series on a large cluster. All optional: at defaults NOTHING extra renders (no empty keys), verified. - values + values.schema.json typed for both. - deploy-lint: a `mon_set` value-set with guards asserting the PrometheusRule renders AND carries real rule groups (a moved/misnamed file would otherwise yield a valid-but-EMPTY spec that kubeconform SKIPS, since CRDs have no core schema) plus the sampleLimit passthrough. Also refreshes docs the moves/earlier PRs left stale: the 4 references to the old alerts path (README, DEPLOY.md, chart README, docs/METRICS.md), and three chart-README rows that still documented pre-#755/#747/#660 behaviour (image.tag "latest"; "a bare helm upgrade ROTATES the cluster secret"; clusterTls.ca "REQUIRED"). Plan updated: the alerts row is now DONE. Verified with helm 3.15.4 + kubeconform 0.6.7: lint clean; the full 8-way matrix validates; defaults render byte-identically to main (both new templates are gated off). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FfFZ8gkkNhDBASuntB72HR --- .github/workflows/deploy-lint.yml | 17 ++++++++- DEPLOY.md | 3 +- README.md | 2 +- deploy/K8S_READINESS_PLAN.md | 15 +++----- deploy/helm/ironcache/README.md | 15 +++++--- .../ironcache/alerts}/ironcache-alerts.yml | 0 .../ironcache/templates/prometheusrule.yaml | 26 +++++++++++++ .../ironcache/templates/servicemonitor.yaml | 22 +++++++++++ deploy/helm/ironcache/values.schema.json | 37 +++++++++++++++++++ deploy/helm/ironcache/values.yaml | 21 +++++++++++ docs/METRICS.md | 3 +- 11 files changed, 142 insertions(+), 19 deletions(-) rename deploy/{prometheus => helm/ironcache/alerts}/ironcache-alerts.yml (100%) create mode 100644 deploy/helm/ironcache/templates/prometheusrule.yaml diff --git a/.github/workflows/deploy-lint.yml b/.github/workflows/deploy-lint.yml index 4b5f8707..aae3acbc 100644 --- a/.github/workflows/deploy-lint.yml +++ b/.github/workflows/deploy-lint.yml @@ -75,7 +75,11 @@ jobs: # a bad key in values-k3s.yaml fails CI (helm auto-validates -f files against the schema). k3s_set="-f deploy/helm/ironcache/values-k3s.yaml" gd_set="--set metrics.grafanaDashboard.enabled=true" - for extra in "" "--set console.enabled=true --set console.replicas=2" "$cluster_tls_set" "--set topologySpread.enabled=true" "$np_set" "$k3s_set" "$gd_set"; do + # The monitoring CRD surface: the PrometheusRule (rules embedded from the in-chart + # alerts file) plus the ServiceMonitor scrape knobs. -ignore-missing-schemas skips the + # CRDs themselves, so the guards below assert the CONTENT actually rendered. + mon_set="--set metrics.prometheusRule.enabled=true --set metrics.serviceMonitor.enabled=true --set metrics.serviceMonitor.sampleLimit=50000" + for extra in "" "--set console.enabled=true --set console.replicas=2" "$cluster_tls_set" "--set topologySpread.enabled=true" "$np_set" "$k3s_set" "$gd_set" "$mon_set"; do echo "::group::helm template ${extra:-(defaults)}" # $extra must WORD-SPLIT into multiple `--set` flags, so it is # intentionally unquoted (SC2086 does not apply here). @@ -124,6 +128,17 @@ jobs: grep -qE "^ replicas: 1$" <<< "$rendered" \ || { echo "ERROR: k3s overlay did not render replicas: 1"; exit 1; } ;; + *prometheusRule.enabled=true*) + # The rules are embedded from alerts/ironcache-alerts.yml via .Files.Get; assert the + # PrometheusRule rendered AND actually carries rule bodies (a bad path or a moved + # file would otherwise yield a valid-but-EMPTY spec that kubeconform happily skips). + grep -q "kind: PrometheusRule" <<< "$rendered" \ + || { echo "ERROR: prometheusRule.enabled rendered no PrometheusRule"; exit 1; } + grep -q "ironcache.availability" <<< "$rendered" \ + || { echo "ERROR: PrometheusRule has no rule groups (alerts file not embedded?)"; exit 1; } + grep -q "sampleLimit: 50000" <<< "$rendered" \ + || { echo "ERROR: ServiceMonitor sampleLimit passthrough did not render"; exit 1; } + ;; *grafanaDashboard.enabled=true*) # Assert the dashboard ConfigMap rendered AND carries the sidecar discovery label # (a silently-empty gate would ship no dashboard while kubeconform stays happy). diff --git a/DEPLOY.md b/DEPLOY.md index f25f93ae..b5908f9e 100644 --- a/DEPLOY.md +++ b/DEPLOY.md @@ -563,7 +563,8 @@ Full catalog of every `ironcache_*` series and the key `INFO` fields is in chart at [`deploy/helm/ironcache/dashboards/`](deploy/helm/ironcache/dashboards/) (set `metrics.grafanaDashboard.enabled=true` to auto-provision it) and the Prometheus -alert rules in [`deploy/prometheus/`](deploy/prometheus/). +alert rules in [`deploy/helm/ironcache/alerts/`](deploy/helm/ironcache/alerts/) (set +`metrics.prometheusRule.enabled=true` to ship them as a PrometheusRule). When something is wrong at 3am, [`docs/RUNBOOK.md`](docs/RUNBOOK.md) is the symptom-to-action index: every operator-visible error string, log line, and probe diff --git a/README.md b/README.md index 0badc7bd..4018585e 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ full contract. sits on the data path. The HA deployment runbook is [`deploy/CONSOLE_DEPLOY.md`](deploy/CONSOLE_DEPLOY.md). - A shipped **Grafana dashboard** (`deploy/helm/ironcache/dashboards/ironcache-dashboard.json`) and - **Prometheus alert rules** (`deploy/prometheus/ironcache-alerts.yml`) over the + **Prometheus alert rules** (`deploy/helm/ironcache/alerts/ironcache-alerts.yml`) over the `/metrics` series cataloged in [`docs/METRICS.md`](docs/METRICS.md). - **CalVer rolling releases** on every push to `main` plus formal `v*` releases: reproducible `musl` + `glibc` tarballs for **amd64 and arm64**, a consolidated diff --git a/deploy/K8S_READINESS_PLAN.md b/deploy/K8S_READINESS_PLAN.md index 70a6da1b..1f040d95 100644 --- a/deploy/K8S_READINESS_PLAN.md +++ b/deploy/K8S_READINESS_PLAN.md @@ -61,14 +61,11 @@ not stale text). The chart is production-safe for **fixed-size** clusters. beta/default-on only from **1.30**, not 1.29), but `ct` and an **upgrade**-path test are still missing. The chart is now 0.3.0. - **Genuinely remaining** (each also flagged in the Section 2 table): - 1. **P2-3 finish** -- an upgrade-path e2e (install -> `helm upgrade` -> re-test), the case that - catches charts which lint clean but fail to *upgrade*. Highest value of the leftovers. - 2. **Alert rules are not templated** -- the dashboard is auto-provisioned but - `deploy/prometheus/ironcache-alerts.yml` has no `PrometheusRule` template (an asymmetry - introduced by #758). - 3. **P2-6** ServiceMonitor `relabelings`/`sampleLimit` passthrough; **P2-7** console-UI Ingress - template; **P2-8** doc-only clarifications (incl. pinning the busybox init image by digest). - 4. **The day-2 scaling Operator** (Section 5) -- a deliberate non-goal for now; the chart + the + 1. **P2-3 finish** -- the upgrade-path e2e (install -> `helm upgrade` -> re-test), the case that + catches charts which lint clean but fail to *upgrade*. In flight (#763). + 2. **P2-7** a console-UI Ingress template, and **P2-8** the doc-only clarifications (incl. + pinning the busybox init image by digest). + 3. **The day-2 scaling Operator** (Section 5) -- a deliberate non-goal for now; the chart + the #392 upgrade driver cover fixed-size clusters. --- @@ -113,7 +110,7 @@ not stale text). The chart is production-safe for **fixed-size** clusters. | No HPA / VPA-caution documented | **DONE** | `deploy/SCALING.md` forbids HPA outright and restricts VPA to Off/Initial (#749). | | Backup / restore / DR (snapshot, VolumeSnapshot, PITR) | **DONE (docs-first, by decision)** | `deploy/BACKUP.md`: the artifact (whole `data_dir`), RPO/RTO, CSI VolumeSnapshot/Velero, the app-level sidecar copy for local-path, per-key DUMP, and a restore runbook (#752). No always-on CronJob is shipped on purpose (RWO + shell-free image + no native object-store = env-specific). | | ServiceMonitor (CRD-gated) | **DONE** | Rendered only when enabled; correct for CRD-less k3s. | -| Prometheus alerts + Grafana dashboard | **PARTIAL** | Dashboard is now auto-provisioned as a sidecar-discoverable ConfigMap (#758); the **alert rules are still a bare file** (`deploy/prometheus/ironcache-alerts.yml`) with no `PrometheusRule` template -- the remaining asymmetry. | +| Prometheus alerts + Grafana dashboard | **DONE** | Dashboard auto-provisioned as a sidecar-discoverable ConfigMap (#758); alert rules shipped as a `PrometheusRule` from the same in-chart file a plain Prometheus can load (`metrics.prometheusRule.enabled`). | | Cache-pods NetworkPolicy | **DONE** | Opt-in policy locking cluster-bus/repl to peers; client + metrics ports stay open by design (the metrics port serves the kubelet probes) (#748). Inert on flannel/k3s-default. | | `values.schema.json` | **DONE** | draft-07: types, enums, port bounds, conditional-requires, and top-level `additionalProperties:false`; doc-only for the non-encodable rules. Negative tests in CI (#756). | | `helm test` + `ct` (kind/k3d install) | **PARTIAL** | `helm test` PING hook + a kind **install** e2e (#759) -- which caught a real shipped preStop bug on its first run. Still missing: `ct` and, more importantly, an **upgrade** path test (the plan's P2-3 asks for install AND upgrade). | diff --git a/deploy/helm/ironcache/README.md b/deploy/helm/ironcache/README.md index 81f47282..a578b3f7 100644 --- a/deploy/helm/ironcache/README.md +++ b/deploy/helm/ironcache/README.md @@ -59,14 +59,14 @@ See `values.yaml` for the full commented surface. The ones that matter first: | Value | Default | Notes | | --- | --- | --- | | `replicas` | `3` | Keep ODD (3, 5, 7) for an unambiguous Raft majority. `1` = standalone. | -| `image.tag` | `"latest"` | Pin an immutable tag in production, e.g. `"0.1.0"`. The registry tag has NO leading `v` (the image CI strips it); `"v0.1.0"` does not exist and fails the pull. | +| `image.tag` | `""` | Empty = the chart `appVersion` (an immutable pinned release) -- the recommended default. Set only to override. The registry tag has NO leading `v` (the image CI strips it); `"v0.1.0"` does not exist and fails the pull. | | `cluster.enabled` | `true` | `false` deploys `replicas` INDEPENDENT standalone nodes (only sensible with `replicas=1`). | | `persistence.enabled` | `true` | `data_dir` on a PVC (`persistence.size`, `persistence.storageClassName`); the Raft log must survive a pod restart. `false` = emptyDir, data lost on reschedule. | | `persistence.saveIntervalSecs` / `saveMinChanges` | `900` / `1` | The background-save cadence that bounds the RPO. | | `auth.enabled` | `false` | Client AUTH (`requirepass`) via a chart Secret (`auth.password`) or `auth.existingSecret` (key `requirepass`). STRONGLY recommended. | -| `clusterSecret.value` / `existingSecret` | `""` | The shared bus/replication handshake secret. Auto-generated per install when both are empty; NOTES.txt warns that a bare `helm upgrade` then ROTATES it. | +| `clusterSecret.value` / `existingSecret` | `""` | The shared bus/replication handshake secret. Auto-generated on first install and PRESERVED across `helm upgrade` (#747). Under GitOps / `helm template` / `--dry-run` the lookup is blind and it WOULD regenerate, so set an explicit value there. | | `clusterTls.enabled` | `false` | Encrypts the node-to-node bus + replication links. Without it the cluster secret travels in cleartext on the pod network. | -| `clusterTls.ca` | `""` | REQUIRED with a chart-managed Secret: the StatefulSet always mounts the `cluster.ca` key and sets `IRONCACHE_CLUSTER_CA_PATH`, but `templates/secret.yaml` only writes that key when `clusterTls.ca` is set, so leaving it empty fails the pod at volume mount. For a self-signed cluster cert, set `ca` to the cert PEM itself. Tracked as #660. | +| `clusterTls.ca` | `""` | Optional since #660: when empty the chart writes `cluster.ca` = the cert, so a self-signed cluster cert acts as its own CA. Set it only for a distinct external-PKI trust chain. | | `tls.enabled` | `false` | TLS on the PUBLIC client RESP listener (separate from `clusterTls`). | | `metrics.port` | `9121` | The chart passes `--metrics-addr 0.0.0.0:`; probes and Prometheus scrape here. `metrics.serviceMonitor.enabled` renders a ServiceMonitor (needs the Prometheus Operator CRD). | | `console.enabled` | `false` | The stateless monitoring/management console; see `deploy/CONSOLE_DEPLOY.md` for the HA + security walkthrough. | @@ -77,8 +77,11 @@ Every node serves `/metrics` + `/livez` + `/readyz` on `metrics.port`; the starter Grafana dashboard ships in the chart at `dashboards/ironcache-dashboard.json` (set `metrics.grafanaDashboard.enabled=true` to auto-provision it as a sidecar-discoverable ConfigMap, or import it manually), -and the Prometheus alert rules in `deploy/prometheus/ironcache-alerts.yml` (see +and the Prometheus alert rules in `alerts/ironcache-alerts.yml` (set +`metrics.prometheusRule.enabled=true` to ship them as a PrometheusRule; see `docs/METRICS.md`). -The chart is CI-gated by `helm lint` + `helm template | kubeconform` for the -default and console-enabled value sets (`.github/workflows/deploy-lint.yml`). +The chart is CI-gated by `helm lint` + `helm template | kubeconform` across a matrix of +value sets plus `values.schema.json` negative tests (`.github/workflows/deploy-lint.yml`), +and by a live `helm install` + `helm upgrade` + `helm test` on a kind cluster +(`.github/workflows/chart-e2e.yml`). diff --git a/deploy/prometheus/ironcache-alerts.yml b/deploy/helm/ironcache/alerts/ironcache-alerts.yml similarity index 100% rename from deploy/prometheus/ironcache-alerts.yml rename to deploy/helm/ironcache/alerts/ironcache-alerts.yml diff --git a/deploy/helm/ironcache/templates/prometheusrule.yaml b/deploy/helm/ironcache/templates/prometheusrule.yaml new file mode 100644 index 00000000..cb5af22f --- /dev/null +++ b/deploy/helm/ironcache/templates/prometheusrule.yaml @@ -0,0 +1,26 @@ +{{- /* SPDX-License-Identifier: MIT OR Apache-2.0 */ -}} +{{- if .Values.metrics.prometheusRule.enabled }} +# The IronCache alerting rules as a Prometheus Operator PrometheusRule (requires the +# monitoring.coreos.com CRD). The rule bodies live in alerts/ironcache-alerts.yml in the chart -- +# the SAME file you can hand to a plain Prometheus via `rule_files:` and validate with +# `promtool check rules` -- so there is one source of truth for both consumption paths. +# +# GOTCHA: the Operator only picks up rules its `ruleSelector` matches. kube-prometheus-stack +# commonly selects on `release: `, so if your alerts never show up, set +# metrics.prometheusRule.additionalLabels to satisfy that selector, e.g. +# --set metrics.prometheusRule.additionalLabels.release=kube-prometheus-stack +# Off by default (the CRD is usually absent on k3s / a bare cluster). +apiVersion: monitoring.coreos.com/v1 +kind: PrometheusRule +metadata: + name: {{ include "ironcache.fullname" . }} + namespace: {{ .Release.Namespace }} + labels: + {{- include "ironcache.labels" . | nindent 4 }} + {{- with .Values.metrics.prometheusRule.additionalLabels }} + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + # The file is exactly a `groups:` document, i.e. already the PrometheusRule spec shape. +{{ .Files.Get "alerts/ironcache-alerts.yml" | indent 2 }} +{{- end }} diff --git a/deploy/helm/ironcache/templates/servicemonitor.yaml b/deploy/helm/ironcache/templates/servicemonitor.yaml index ed4e860f..485a5091 100644 --- a/deploy/helm/ironcache/templates/servicemonitor.yaml +++ b/deploy/helm/ironcache/templates/servicemonitor.yaml @@ -9,12 +9,34 @@ metadata: namespace: {{ .Release.Namespace }} labels: {{- include "ironcache.labels" . | nindent 4 }} + {{- with .Values.metrics.serviceMonitor.additionalLabels }} + {{- toYaml . | nindent 4 }} + {{- end }} spec: selector: matchLabels: {{- include "ironcache.selectorLabels" . | nindent 6 }} + {{- with .Values.metrics.serviceMonitor.sampleLimit }} + # Drop a scrape that would ingest more than this many samples (0 = unlimited). A blast + # radius cap for a large cluster / a cardinality regression. + sampleLimit: {{ int . }} + {{- end }} endpoints: - port: metrics path: /metrics interval: {{ .Values.metrics.serviceMonitor.interval }} + {{- with .Values.metrics.serviceMonitor.scrapeTimeout }} + scrapeTimeout: {{ . }} + {{- end }} + {{- with .Values.metrics.serviceMonitor.relabelings }} + # Applied to the TARGET before scraping (relabel_configs). + relabelings: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.metrics.serviceMonitor.metricRelabelings }} + # Applied to each scraped SAMPLE (metric_relabel_configs) -- the lever for dropping + # high-cardinality series before they hit storage. + metricRelabelings: + {{- toYaml . | nindent 8 }} + {{- end }} {{- end }} diff --git a/deploy/helm/ironcache/values.schema.json b/deploy/helm/ironcache/values.schema.json index d30e5d62..633ab32e 100644 --- a/deploy/helm/ironcache/values.schema.json +++ b/deploy/helm/ironcache/values.schema.json @@ -234,6 +234,43 @@ "interval": { "type": "string", "description": "Scrape interval as a Prometheus duration, e.g. 30s." + }, + "additionalLabels": { + "type": "object", + "description": "Extra labels, e.g. to satisfy the Operator's serviceMonitorSelector (kube-prometheus-stack often selects on release=)." + }, + "scrapeTimeout": { + "type": "string", + "description": "Per-scrape timeout as a Prometheus duration (must be <= interval). Empty = Prometheus default." + }, + "sampleLimit": { + "type": "integer", + "minimum": 0, + "description": "Max samples accepted per scrape; 0 = unlimited. Guards against a cardinality regression." + }, + "relabelings": { + "type": "array", + "description": "relabel_configs applied to the target, passed through verbatim.", + "items": {} + }, + "metricRelabelings": { + "type": "array", + "description": "metric_relabel_configs applied to each scraped sample, passed through verbatim.", + "items": {} + } + } + }, + "prometheusRule": { + "type": "object", + "description": "Ship the alerting rules as a Prometheus Operator PrometheusRule (requires the CRD).", + "properties": { + "enabled": { + "type": "boolean", + "description": "Render the PrometheusRule (off by default)." + }, + "additionalLabels": { + "type": "object", + "description": "Extra labels so the Operator's ruleSelector adopts the rules (commonly release=)." } } }, diff --git a/deploy/helm/ironcache/values.yaml b/deploy/helm/ironcache/values.yaml index cf5bc0a9..507a73a2 100644 --- a/deploy/helm/ironcache/values.yaml +++ b/deploy/helm/ironcache/values.yaml @@ -141,6 +141,27 @@ metrics: serviceMonitor: enabled: false interval: 30s + # Extra labels, in case the Operator's serviceMonitorSelector requires one + # (kube-prometheus-stack commonly selects on `release: `). + additionalLabels: {} + # Per-scrape timeout (must be <= interval). Empty = the Prometheus default. + scrapeTimeout: "" + # Cap the samples ingested per scrape; 0/unset = unlimited. A blast-radius guard + # against a cardinality regression on a large cluster. + sampleLimit: 0 + # relabel_configs (applied to the TARGET) and metric_relabel_configs (applied to each + # SAMPLE) -- passed through verbatim. metricRelabelings is the lever for dropping + # high-cardinality series before they reach storage, e.g. undersampling per-shard series. + relabelings: [] + metricRelabelings: [] + # Ship the IronCache alerting rules as a Prometheus Operator PrometheusRule (requires the + # CRD). The rule bodies come from alerts/ironcache-alerts.yml in the chart -- the same file + # a plain Prometheus can load via `rule_files:` -- so there is one source of truth. + prometheusRule: + enabled: false + # The Operator only adopts rules its `ruleSelector` matches; if your alerts never appear, + # set the selector label here, e.g. additionalLabels: {release: kube-prometheus-stack}. + additionalLabels: {} # Auto-provision the IronCache Grafana dashboard as a sidecar-discoverable ConfigMap. # The Grafana dashboard sidecar (kube-prometheus-stack / the grafana chart) loads any # ConfigMap carrying `label: labelValue` into Grafana. Requires that sidecar enabled + diff --git a/docs/METRICS.md b/docs/METRICS.md index 389f3314..47f534d8 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -16,7 +16,8 @@ Companion artifacts: connections, memory, per-shard hot-shard detection, replication, persistence). Set `metrics.grafanaDashboard.enabled=true` to auto-provision it as a sidecar-discoverable ConfigMap. -- `deploy/prometheus/ironcache-alerts.yml` -- starter Prometheus alerting rules. +- `deploy/helm/ironcache/alerts/ironcache-alerts.yml` -- starter Prometheus alerting rules + (also shipped as a PrometheusRule via `metrics.prometheusRule.enabled=true`). ## The ops endpoint