Skip to content

🌱 Migrate e2e prometheus from custom chart to kube-prometheus-stack - #2757

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
pedjak:migrate-prometheus-to-kube-prometheus-stack
Jun 11, 2026
Merged

🌱 Migrate e2e prometheus from custom chart to kube-prometheus-stack#2757
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
pedjak:migrate-prometheus-to-kube-prometheus-stack

Conversation

@pedjak

@pedjakpedjak commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

Replace the hand-rolled prometheus-operator install script (hack/test/install-prometheus.sh) and custom Helm chart (helm/prometheus/) with the official kube-prometheus-stack community chart (v86.2.2), installed from OCI registry.

Motivation: The custom chart required maintaining 12 templates, a separate install script, kustomize tooling, and conftest policies. The official chart provides the same functionality with less maintenance burden.

What changed

  • Prometheus deployment: Single helm install from oci://ghcr.io/prometheus-community/charts/kube-prometheus-stack replaces the kustomize-based operator install + custom chart template pipeline
  • Unused components disabled: grafana, alertmanager, node-exporter, kube-state-metrics, default rules, admission webhooks, operator TLS
  • Kubelet ServiceMonitor: Uses chart's built-in kubelet support with kubelet: false (only cAdvisor), honorTimestamps: false, trackTimestampsStaleness: false, and cAdvisorRelabelings: [] to match old behavior
  • Custom ServiceMonitors: operator-controller and catalogd added via additionalServiceMonitors using bearerTokenFile (projected SA token) instead of the legacy prometheus-metrics-token Secret
  • PrometheusRules: Split into controller-panic-alerts and controller-resource-alerts map keys so the experimental override only replaces the resource-usage group (reducing duplication)
  • NetworkPolicies: Managed by the chart via prometheus.networkPolicy and prometheusOperator.networkPolicy settings
  • Install logic: Inlined into the Makefile prometheus target — no separate script
  • Conftest: Removed prometheus-networkpolicies.rego policy (NetworkPolicy now managed by the chart)
  • Kustomize: Removed unused bingo tooling (was only used by the deleted install script)

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

CopilotAI review requested due to automatic review settings June 10, 2026 16:24
@openshift-ciopenshift-ciBot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the e2e Prometheus setup from a bespoke Helm chart + install script to the upstream kube-prometheus-stack chart (installed from the OCI registry), aiming to reduce maintenance overhead while preserving the existing e2e monitoring/alerting behavior.

Changes:

  • Added kube-prometheus-stack values files under testdata/prometheus/ (including an experimental override).
  • Updated the prometheus Makefile target to install kube-prometheus-stack directly via Helm and removed Prometheus-specific helm+conftest linting.
  • Removed the custom Prometheus Helm chart, the install script, and the Prometheus NetworkPolicy conftest policy.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
testdata/prometheus/values.yamlNew baseline kube-prometheus-stack values for e2e (disables unused components, adds ServiceMonitors and rules).
testdata/prometheus/values-experimental.yamlNew experimental override values to adjust alert thresholds.
MakefileReplaces the Prometheus install script with an inline Helm install from OCI; updates linting and experimental values path.
hack/conftest/policy/README.mdUpdates documentation to reflect removal of Prometheus-specific conftest policies and lint flow.
AGENTS.mdUpdates repository layout documentation to reference the new testdata/prometheus/ location.
helm/prometheus/Chart.yaml (deleted)Removes the custom Prometheus Helm chart definition.
helm/prometheus/values.yaml (deleted)Removes custom chart values (thresholds/namespaces).
helm/prometheus/templates/servicemonitor-operator-controller-controller-manager-metrics-monitor.yml (deleted)Removes custom ServiceMonitor for operator-controller metrics.
helm/prometheus/templates/servicemonitor-catalogd-controller-manager-metrics-monitor.yml (deleted)Removes custom ServiceMonitor for catalogd metrics.
helm/prometheus/templates/servicemonitor-kubelet.yml (deleted)Removes custom kubelet ServiceMonitor.
helm/prometheus/templates/prometheusrule-controller-alerts.yml (deleted)Removes custom PrometheusRule definitions (now supplied via upstream chart values).
helm/prometheus/templates/prometheus-prometheus.yml (deleted)Removes custom Prometheus CR manifest.
helm/prometheus/templates/serviceaccount-prometheus.yml (deleted)Removes custom Prometheus ServiceAccount manifest.
helm/prometheus/templates/secret-prometheus-metrics-token.yml (deleted)Removes the legacy service-account-token Secret approach for scraping.
helm/prometheus/templates/service-prometheus-service.yml (deleted)Removes the custom NodePort Service manifest.
helm/prometheus/templates/networkpolicy-prometheus.yml (deleted)Removes custom Prometheus NetworkPolicy manifest.
helm/prometheus/templates/networkpolicy-prometheus-operator.yml (deleted)Removes custom Prometheus Operator NetworkPolicy manifest.
helm/prometheus/templates/clusterrole-prometheus.yml (deleted)Removes custom Prometheus ClusterRole manifest.
helm/prometheus/templates/clusterrolebinding-prometheus.yml (deleted)Removes custom Prometheus ClusterRoleBinding manifest.
helm/prom_experimental.yaml (deleted)Removes the legacy experimental Prometheus values file (replaced by testdata/prometheus/values-experimental.yaml).
hack/test/install-prometheus.sh (deleted)Removes the bespoke Prometheus install script (replaced by Makefile logic).
hack/conftest/policy/prometheus-networkpolicies.rego (deleted)Removes Prometheus NetworkPolicy conftest policy (now chart-managed).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadMakefile
Comment threadMakefile Outdated
@netlify

netlifyBot commented Jun 10, 2026

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commita0e1d24
🔍 Latest deploy loghttps://app.netlify.com/projects/olmv1/deploys/6a2a734392b4ec00072bc10c
😎 Deploy Previewhttps://deploy-preview-2757--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changesRun an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

CopilotAI review requested due to automatic review settings June 10, 2026 18:59

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Comment threadtestdata/prometheus/values.yaml
Comment threadtestdata/prometheus/values.yaml
Comment threadtestdata/prometheus/values.yaml
Comment threadtestdata/prometheus/values.yaml
Comment threadMakefile Outdated
CopilotAI review requested due to automatic review settings June 10, 2026 19:11
@pedjak
pedjakforce-pushed the migrate-prometheus-to-kube-prometheus-stack branch from ba05248 to 3251893CompareJune 10, 2026 19:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.

Comment threadMakefile Outdated
Comment threadtestdata/prometheus/values.yaml Outdated
Comment threadtestdata/prometheus/values.yaml Outdated
Comment threadtestdata/prometheus/values.yaml
@codecov

codecovBot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.82%. Comparing base (23b7e52) to head (a0e1d24).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #2757 +/- ##
==========================================
- Coverage 66.84% 66.82% -0.02% 
==========================================
Files 149 149 Lines 11382 11382 ==========================================
- Hits 7608 7606 -2 - Misses 3218 3219 +1 - Partials 556 557 +1 
FlagCoverage Δ
e2e35.09% <ø> (-0.05%)⬇️
experimental-e2e52.34% <ø> (ø)
unit52.17% <ø> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pedjak
pedjakforce-pushed the migrate-prometheus-to-kube-prometheus-stack branch from 3251893 to dc22383CompareJune 10, 2026 19:37
CopilotAI review requested due to automatic review settings June 10, 2026 19:41
@pedjak
pedjakforce-pushed the migrate-prometheus-to-kube-prometheus-stack branch from dc22383 to 1ae258fCompareJune 10, 2026 19:41

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comment threadMakefile
@pedjak
pedjak marked this pull request as ready for review June 10, 2026 21:19
@openshift-ciopenshift-ciBot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 10, 2026
@pedjak
pedjak requested a review from dtfranzJune 10, 2026 21:20
groups:
- name: resource-usage
rules:
- alert: oom-events

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule can probably move to values.yaml as well since it doesn't need to change.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. However, the oom-events rule is part of the resource-usage group which is a YAML array — Helm replaces arrays entirely when merging -f files, so we can't selectively keep individual rules from the base. Moving it to a third map key (e.g. controller-oom-alerts) would work but adds another PrometheusRule resource. Happy to do it if you prefer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine then, we can leave it like this

Comment threadtestdata/prometheus/values.yaml Outdated
Comment threadtestdata/prometheus/values.yaml Outdated
…-stack
Replace the hand-rolled prometheus-operator install script and custom
Helm chart (helm/prometheus/) with the official kube-prometheus-stack
community chart (v86.2.2), installed from OCI registry.
- Disable all unused components (grafana, alertmanager, exporters,
default rules, admission webhooks, operator TLS)
- Configure Prometheus instance, NetworkPolicies, and kubelet
ServiceMonitor via chart values
- Add operator-controller and catalogd ServiceMonitors as
additionalServiceMonitors using bearerTokenFile (projected SA token)
instead of the legacy prometheus-metrics-token Secret
- Split PrometheusRules into controller-panic-alerts and
controller-resource-alerts so the experimental override only
replaces the resource-usage group
- Inline the install logic into the Makefile prometheus target
- Remove conftest prometheus-networkpolicies.rego policy (NetworkPolicy
now managed by the chart)
- Remove unused kustomize bingo tooling
Co-Authored-By: Claude <noreply@anthropic.com>
@pedjak
pedjakforce-pushed the migrate-prometheus-to-kube-prometheus-stack branch from 1ae258f to a0e1d24CompareJune 11, 2026 08:35
@pedjak
pedjak requested a review from dtfranzJune 11, 2026 08:59
@pedjak
pedjak requested review from rashmigottipati and removed request for ankitathomas and dtfranzJune 11, 2026 09:19
@dtfranz

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026

@rashmigottipatirashmigottipati left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@rashmigottipatirashmigottipati added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2026
@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: rashmigottipati

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-botBot merged commit 9c49467 into operator-framework:mainJun 11, 2026
26 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.lgtmIndicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pedjak@dtfranz@rashmigottipati