feat(monitoring): add support for additional labels in PodMonitor configuration
This pull request introduces support for adding custom labels to the generated PodMonitor resource in the cluster Helm chart, making it easier to integrate with various monitoring stacks and workflows. Additionally, the logic for PodMonitor and its relabeling configuration has been refactored to improve maintainability, with the relevant configuration now handled in a dedicated template.
PodMonitor labels support and configuration refactoring:
- Added a new
labelsfield undercluster.monitoring.podMonitorinvalues.yaml,values.schema.json, and documented it inREADME.md, allowing users to specify additional labels for the PodMonitor resource. [1] [2] [3] - Updated example and test manifests to demonstrate usage of the new
labelsfield for PodMonitor, including sample team and environment labels. [1] [2] [3]
Refactoring PodMonitor resource generation:
- Moved PodMonitor resource creation and relabeling configuration to a new dedicated template file
podmonitor.yaml, improving separation of concerns and maintainability. - Removed PodMonitor relabeling and metricRelabeling configuration from
cluster.yaml, centralizing this logic in the new template. - Updated PodMonitor enablement logic in
cluster.yamlto defer resource creation to the new template, settingenablePodMonitorto false in the main spec.
closes: https://github.com/cloudnative-pg/charts/issues/647
Any chance PR will be merged?
@itay-grudev could you please take a look at this PR when you have a moment?
@Dashing-Nelson I'll fix the test failure.
Upgrading with monitoring already enabled would cause:
helm upgrade --install database \
--namespace database \
--create-namespace \
--reuse-values \
~/Projects/oss/cnpg/charts/charts/cluster
Error: UPGRADE FAILED: Unable to continue with update: PodMonitor "database-cluster" in namespace "database" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "database"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "database"
Which makes sense. Even though we set enablePodMonitor: false, the operator doesn't remove it faster than Helm attempts to apply the new helm-managed PodMonitor.
My only solution then would be adding a note in the the release log that you need set --set cluster.monitoring.enabled=false when upgrading, then re-enable it, after the upgrade.
Before I merge this, does anyone have a better idea on how to solve the upgrade problem?
Upgrading with monitoring already enabled would cause:
helm upgrade --install database \ --namespace database \ --create-namespace \ --reuse-values \ ~/Projects/oss/cnpg/charts/charts/cluster Error: UPGRADE FAILED: Unable to continue with update: PodMonitor "database-cluster" in namespace "database" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "database"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "database"Which makes sense. Even though we set
enablePodMonitor: false, the operator doesn't remove it faster than Helm attempts to apply the new helm-managed PodMonitor.My only solution then would be adding a note in the the release log that you need set
--set cluster.monitoring.enabled=falsewhen upgrading, then re-enable it, after the upgrade.Before I merge this, does anyone have a better idea on how to solve the upgrade problem?
@itay-grudev I can think of:
Pre-upgrade hook to delete the PodMonitor: Only downside I see is that there is a need for RBAC for the hook to delete PodMonitors. Added complexity, i am afraid or maybe we could instead of deleting, the hook patches the existing operator-managed PodMonitor with the required Helm labels/annotations so Helm can adopt it. This could arguably be the cleanest solution. WDYT
I was missing the rather obvious solution - to rename it.