Skip to content

helm: add optional CRD-gated ServiceMonitor for the controller manager - #1865

Open
apurvapatkeshwar wants to merge 1 commit into
michelangelo-ai:mainfrom
apurvapatkeshwar:feat/monitoring-servicemonitor
Open

helm: add optional CRD-gated ServiceMonitor for the controller manager#1865
apurvapatkeshwar wants to merge 1 commit into
michelangelo-ai:mainfrom
apurvapatkeshwar:feat/monitoring-servicemonitor

Conversation

@apurvapatkeshwar

Copy link
Copy Markdown
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

What changed?

First slice of the observability starter pack (#1692): a monitoring.enabled toggle (default off) that creates a ServiceMonitor for the controller manager's /metrics endpoint -- the only component that exposes one today.

monitoring:
  enabled: true
  serviceMonitor:
    interval: 30s
    additionalLabels: {}   # e.g. to match a Prometheus serviceMonitorSelector

The resource is gated on .Capabilities.APIVersions.Has "monitoring.coreos.com/v1", so enabling the toggle on a cluster without the Prometheus Operator is a safe no-op -- matching the issue's "skipped when the operators are absent" intent. A validation guard fails fast if monitoring.enabled=true with controllermgr.enabled=false.

Two drive-by fixes in the monitoring guide, since this PR ships the exact resource it describes:

  • the ServiceMonitor example used an app: michelangelo-controllermgr label that doesn't match the chart's real selector labels (app.kubernetes.io/name / instance / component) -- corrected, and the guide now leads with the chart-managed path
  • the health-probe port is 8081 (chart default controllermgr.healthPort), not 8083

Follow-up PRs in this series: PrometheusRule starter alerts, GrafanaDashboard CRs, and a docs pass. The per-job scrape ConfigMap named in the issue (item 3) is deferred -- it needs FederatedClient.CreatePromConfigMap call-site work, which is a code change rather than installable YAML.

Why?

#1692 lays out the gap: the chart deploys the platform with zero scrape configuration, alerts, or dashboards, and the monitoring guide asks operators to hand-write resources (with examples that do not match the chart's labels). This starts closing that gap with the smallest installable piece, wired the same way other optional chart features are.

How did you test it?

  • helm lint clean; all checks re-run on top of current main before posting.
  • Default render is byte-identical to main (verified by diffing full helm template -f values-k3d.yaml output against a clean main worktree) -- strictly additive.
  • Rendered ServiceMonitor's selector.matchLabels verified to exactly match the controllermgr Service's labels, and the metrics endpoint port name matches the Service port.
  • New CI checks in helm-lint.yaml: monitoring-enabled render with --api-versions monitoring.coreos.com/v1 asserts the resource renders; without it, asserts it is skipped; negative test for the controllermgr guard. All three verified locally with the exact CI invocations.

Potential risks

  • None for existing deployments: the toggle defaults to off and the default render is byte-identical to main.
  • The CRD gate uses .Capabilities.APIVersions, which is populated from the cluster on real installs but empty under bare helm template; anyone rendering manifests offline for a cluster that has the Prometheus Operator needs --api-versions monitoring.coreos.com/v1. The values comment says so.

Breaking Changes

  • No breaking changes
  • API changes (Go exported symbols or function signatures, Python public functions or classes)
  • Proto changes (enum value renumbering, field number changes, field removal, service removal)
  • Helm changes (new required values, renamed or removed keys, changed value semantics)
  • Config/deployment changes (new required env vars, renamed container args, changed ports or mount paths)

Release notes

Added an optional monitoring.enabled chart toggle that creates a ServiceMonitor for the controller manager when the Prometheus Operator CRDs are present. Default off; no action needed for existing deployments. Also corrected the label selector and health-probe port in the monitoring guide's examples.

Documentation Changes

docs/operator-guides/operations/monitoring.md now leads with the chart-managed ServiceMonitor path and its examples match the chart's real labels and ports. helm/michelangelo/README.md documents the new values.

First slice of the observability starter pack (michelangelo-ai#1692): a monitoring
toggle (default off) that creates a ServiceMonitor for controllermgr's
/metrics endpoint. The resource is gated on the Prometheus Operator
CRDs being present, so enabling the toggle on a cluster without the
operator is a safe no-op.

Also fixes two drifts in the monitoring guide: the ServiceMonitor
example used labels that do not match the chart's real selector labels,
and the health-probe port is 8081 (chart default), not 8083.

CI gains three template checks: monitoring-enabled render with the CRD
API version present, graceful skip without it, and a negative test for
the new monitoring-requires-controllermgr guard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant