Skip to content

MON-4500: Migrate Prometheus targets discovering from Endpoints to EndpointSlices - #460

Open
machine424 wants to merge 3 commits into
openshift:masterfrom
machine424:endpoinmig
Open

MON-4500: Migrate Prometheus targets discovering from Endpoints to EndpointSlices#460
machine424 wants to merge 3 commits into
openshift:masterfrom
machine424:endpoinmig

Conversation

@machine424

@machine424 machine424 commented Feb 9, 2026

Copy link
Copy Markdown

This PR migrates Prometheus service discovery from the deprecated Endpoints API to the EndpointSlices API, by:

  • Setting serviceDiscoveryRole: EndpointSlice on ServiceMonitors.
  • Granting Prometheus endpointslices permissions.

We're taking a conservative approach by keeping the existing endpoints permissions alongside the new endpointslices ones. This provides a safety net in case any ServiceMonitors, whether deployed from this repo or from another source, still rely on the same Role and were missed during the migration.

That said, since both resources provide essentially the same data, keeping both isn't meaningfully more permissive from a security standpoint.

These changes target OpenShift 4.22+ and should not be backported to earlier releases.

Due to the scope of changes across multiple repositories, these modifications were generated with Claude assistance.

Summary by CodeRabbit

  • New Features

    • Added EndpointSlice-based service discovery for DNS metrics monitoring.
    • Extended monitoring permissions to read EndpointSlice resources.
    • Added automatic reconciliation to create or update the metrics access role when configuration changes.
  • Bug Fixes

    • Improved compatibility with clusters that expose service endpoints through EndpointSlice resources.
  • Tests

    • Added coverage for metrics access-role updates and ServiceMonitor discovery configuration changes.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2026
@openshift-ci

openshift-ci Bot commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@machine424
machine424 marked this pull request as ready for review February 9, 2026 12:18
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2026
@openshift-ci
openshift-ci Bot requested review from bentito and knobunc February 9, 2026 12:28
@machine424 machine424 changed the title Migrate Prometheus targets discovering from Endpoints to EndpointSlices MON-4500: Migrate Prometheus targets discovering from Endpoints to EndpointSlices Feb 9, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 9, 2026
@openshift-ci-robot

openshift-ci-robot commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

@machine424: This pull request references MON-4500 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

This PR migrates Prometheus service discovery from the deprecated Endpoints API to the EndpointSlices API, by:

  • Setting serviceDiscoveryRole: EndpointSlice on ServiceMonitors.
  • Granting Prometheus endpointslices permissions.

We're taking a conservative approach by keeping the existing endpoints permissions alongside the new endpointslices ones. This provides a safety net in case any ServiceMonitors, whether deployed from this repo or from another source, still rely on the same Role and were missed during the migration.

That said, since both resources provide essentially the same data, keeping both isn't meaningfully more permissive from a security standpoint.

These changes target OpenShift 4.22+ and should not be backported to earlier releases.

Due to the scope of changes across multiple repositories, these modifications were generated with Claude assistance.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@machine424

Copy link
Copy Markdown
Author

/verified by existing tests
/jira refresh

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Mar 9, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@machine424: This PR has been marked as verified by existing tests.

Details

In response to this:

/verified by existing tests
/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot

openshift-ci-robot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

@machine424: This pull request references MON-4500 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

/verified by existing tests
/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@machine424

Copy link
Copy Markdown
Author

/verified by existing tests
/jira refresh

@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@machine424: This PR has been marked as verified by existing tests.

Details

In response to this:

/verified by existing tests
/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot

openshift-ci-robot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

@machine424: This pull request references MON-4500 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

/verified by existing tests
/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@alebedev87

Copy link
Copy Markdown
Contributor

/assign @rikatz

@rikatz

rikatz commented Mar 11, 2026

Copy link
Copy Markdown
Member

@machine424 you need to apply some extra changes:

Please add unit tests for these changes as well.

Thanks!

@openshift-ci-robot openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Mar 18, 2026
@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5a1870a0-99e9-40ae-a15a-56a193cdc851

📥 Commits

Reviewing files that changed from the base of the PR and between 454182e and 5ad1159.

📒 Files selected for processing (2)
  • manifests/0000_70_dns-operator_00-cluster-role.yaml
  • pkg/operator/controller/controller_metrics_role_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/operator/controller/controller_metrics_role_test.go
  • manifests/0000_70_dns-operator_00-cluster-role.yaml

Walkthrough

The pull request adds EndpointSlice monitoring support to the DNS operator by extending RBAC permissions, reconciling the metrics Role through dedicated helpers, and configuring the ServiceMonitor to use EndpointSlice service discovery.

Changes

EndpointSlice monitoring and RBAC

Layer / File(s) Summary
RBAC permissions and metrics assets
manifests/0000_70_dns-operator_00-cluster-role.yaml, manifests/0000_90_dns-operator_00_prometheusrole.yaml, pkg/manifests/assets/dns/metrics/role.yaml
Adds EndpointSlice get, list, and watch permissions, and allows updates to RBAC roles alongside clusterroles.
Metrics Role reconciliation
pkg/operator/controller/controller.go, pkg/operator/controller/controller_metrics_role.go, pkg/operator/controller/controller_metrics_role_test.go
Delegates metrics Role provisioning to reconciliation helpers that create or update the Role based on manifest rules, with tests for rule changes and idempotence.
ServiceMonitor EndpointSlice discovery
manifests/0000_90_dns-operator_02_servicemonitor.yaml, pkg/operator/controller/controller_service_monitor.go, pkg/operator/controller/controller_service_monitor_test.go
Sets serviceDiscoveryRole to EndpointSlice and tests detection of changes to that field.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: bentito, knobunc

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: migrating Prometheus discovery from Endpoints to EndpointSlices.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed No Ginkgo titles were added; the changed tests use static Go test/subtest descriptions and contain no dynamic name content.
Test Structure And Quality ✅ Passed PASS: These are table-driven unit tests, not Ginkgo; they test one behavior per function, need no cluster cleanup/timeouts, and include clear failure messages.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the only new test is a plain Go unit test (TestDNSMetricsRoleChanged) with no MicroShift-only APIs.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Only plain Go unit tests were added; no Ginkgo e2e specs or multi-node/SNO assumptions were introduced.
Topology-Aware Scheduling Compatibility ✅ Passed No scheduling constraints or topology assumptions were added; changes only adjust RBAC and ServiceMonitor EndpointSlice discovery.
Ote Binary Stdout Contract ✅ Passed Touched files add controller/helper logic and tests only; no main/init/TestMain/BeforeSuite stdout writes were introduced.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR adds only unit tests/manifests; no new Ginkgo e2e tests, IPv4-only IP handling, or external connectivity requirements were introduced.
No-Weak-Crypto ✅ Passed Touched files only add EndpointSlice RBAC/ServiceMonitor updates; no weak crypto, custom crypto, or secret/token comparisons were found.
Container-Privileges ✅ Passed Changed manifests only add EndpointSlice RBAC and ServiceMonitor discovery settings; no privileged, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation fields appear.
No-Sensitive-Data-In-Logs ✅ Passed No new log statements expose secrets; the added logs only report RBAC role names, and the ServiceMonitor diff log was pre-existing and unchanged.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from rikatz. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

…rviceDiscoveryRole: EndpointSlice in ServiceMonitors

This PR migrates Prometheus service discovery from the deprecated Endpoints API to the EndpointSlices API, by:

    Setting serviceDiscoveryRole: EndpointSlice on ServiceMonitors.
    Granting Prometheus endpointslices permissions.

We're taking a conservative approach by keeping the existing endpoints permissions alongside the new endpointslices ones. This provides a safety net in case any ServiceMonitors, whether deployed from this repo or from another source, still rely on the same Role and were missed during the migration.

That said, since both resources provide essentially the same data, keeping both isn't meaningfully more permissive from a security standpoint.

These changes target OpenShift 4.22+ and should not be backported to earlier releases.

Due to the scope of changes across multiple repositories, these modifications were generated with Claude assistance.
- rbac.authorization.k8s.io
resources:
- clusterroles
- roles

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

allows the operator to update pkg/manifests/assets/dns/metrics/role.yaml on the cluster

@machine424

Copy link
Copy Markdown
Author

Pushed some changes, keeping an eye on the CI.

rbacv1 "k8s.io/api/rbac/v1"
)

func TestDNSggMetricsRoleChanged(t *testing.T) {

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.

nit: DNSgg?

@@ -0,0 +1,26 @@
package controller

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.

- get
- list
- watch
- apiGroups:

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.

you need to add these same permissions to the ClusterRole, otherwise the user running openshift-dns-operator won´t be able to give this permission.

See:

	ERROR	Reconciler error	{"controller": "dns_controller", "object": {"name":"default"}, "namespace": "", "name": "default", "reconcileID": "edd6c3b1-ee05-40eb-a566-6a83471fb643", "error": "failed to ensure dns default: failed to integrate metrics with openshift-monitoring for dns default: failed to ensure dns metrics role for default: failed to update dns metrics role openshift-dns/prometheus-k8s: roles.rbac.authorization.k8s.io \"prometheus-k8s\" is forbidden: user \"system:serviceaccount:openshift-dns-operator:dns-operator\" (groups=[\"system:serviceaccounts\" \"system:serviceaccounts:openshift-dns-operator\" \"system:authenticated\"]) is attempting to grant RBAC permissions not currently held:\n{APIGroups:[\"discovery.k8s.io\"], Resources:[\"endpointslices\"], Verbs:[\"get\"]}", "errorCauses": [{"error": "failed to ensure dns default: failed to integrate metrics with openshift-monitoring for dns default: failed to ensure dns metrics role for default: failed to update dns metrics role openshift-dns/prometheus-k8s: roles.rbac.authorization.k8s.io \"prometheus-k8s\" is forbidden: user \"system:serviceaccount:openshift-dns-operator:dns-operator\" (groups=[\"system:serviceaccounts\" \"system:serviceaccounts:openshift-dns-operator\" \"system:authenticated\"]) is attempting to grant RBAC permissions not currently held:\n{APIGroups:[\"discovery.k8s.io\"], Resources:[\"endpointslices\"], Verbs:[\"get\"]}"}]}

@rikatz

rikatz commented May 26, 2026

Copy link
Copy Markdown
Member

@machine424 are you still working on this? lmk :)

@rikatz

rikatz commented Jul 16, 2026

Copy link
Copy Markdown
Member

@machine424 friendly ping

@machine424
machine424 requested a review from rikatz July 25, 2026 18:10
@openshift-ci-robot

openshift-ci-robot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

@machine424: This pull request references MON-4500 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

This PR migrates Prometheus service discovery from the deprecated Endpoints API to the EndpointSlices API, by:

  • Setting serviceDiscoveryRole: EndpointSlice on ServiceMonitors.
  • Granting Prometheus endpointslices permissions.

We're taking a conservative approach by keeping the existing endpoints permissions alongside the new endpointslices ones. This provides a safety net in case any ServiceMonitors, whether deployed from this repo or from another source, still rely on the same Role and were missed during the migration.

That said, since both resources provide essentially the same data, keeping both isn't meaningfully more permissive from a security standpoint.

These changes target OpenShift 4.22+ and should not be backported to earlier releases.

Due to the scope of changes across multiple repositories, these modifications were generated with Claude assistance.

Summary by CodeRabbit

  • New Features

  • Added EndpointSlice-based service discovery for DNS metrics monitoring.

  • Extended monitoring permissions to read EndpointSlice resources.

  • Added automatic reconciliation to create or update the metrics access role when configuration changes.

  • Bug Fixes

  • Improved compatibility with clusters that expose service endpoints through EndpointSlice resources.

  • Tests

  • Added coverage for metrics access-role updates and ServiceMonitor discovery configuration changes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci

openshift-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

@machine424: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants