MON-4432: Add EndpointSlice support to DaemonSet ServiceMonitors #2839

slashpai · 2025-11-17T06:52:17Z

Configure all ServiceMonitor objects for DaemonSet workloads to use EndpointSlice for service discovery instead of the default Endpoints API, improving scalability and performance for monitoring DaemonSet pods.

Changes:

Add serviceDiscoveryRole: EndpointSlice to ServiceMonitor specs
Add discovery.k8s.io/endpointslices RBAC permissions to prometheus-k8s Roles

Related Epic: https://issues.redhat.com/browse/MON-4216

cc @simonpasquier

/hold to test changes

Configure all ServiceMonitor objects for DaemonSet workloads to use EndpointSlice for service discovery instead of the default Endpoints API, improving scalability and performance for monitoring DaemonSet pods. Changes: - Add serviceDiscoveryRole: EndpointSlice to ServiceMonitor specs - Add discovery.k8s.io/endpointslices RBAC permissions to prometheus-k8s Roles Signed-off-by: Jayapriya Pai <janantha@redhat.com>

openshift-ci-robot · 2025-11-17T06:52:20Z

@slashpai: This pull request references MON-4432 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 sub-task to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Configure all ServiceMonitor objects for DaemonSet workloads to use EndpointSlice for service discovery instead of the default Endpoints API, improving scalability and performance for monitoring DaemonSet pods.

Changes:

Add serviceDiscoveryRole: EndpointSlice to ServiceMonitor specs

Add discovery.k8s.io/endpointslices RBAC permissions to prometheus-k8s Roles

Related Epic: https://issues.redhat.com/browse/MON-4216

cc @simonpasquier

/hold to test 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.

coderabbitai · 2025-11-17T06:52:27Z

Walkthrough

This change adds EndpointSlice-based service discovery support across multiple Prometheus monitoring configurations. ServiceMonitor resources are updated with serviceDiscoveryRole: EndpointSlice, and corresponding RBAC Role permissions are extended to grant Prometheus access to endpointslices resources in the discovery.k8s.io API group.

Changes

Cohort / File(s)	Summary
EndpointSlice Discovery Configuration `bindata/kube-proxy/monitor.yaml`, `bindata/network/frr-k8s/monitor.yaml`, `bindata/network/network-metrics/002-prometheus.yaml`, `bindata/network/openshift-sdn/monitor.yaml`, `bindata/network/ovn-kubernetes/common/monitor-node.yaml`	Adds `serviceDiscoveryRole: EndpointSlice` field to ServiceMonitor specs. Extends prometheus-k8s Role with new RBAC rule granting get, list, and watch permissions on endpointslices resource in discovery.k8s.io API group.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Homogeneous changes applied consistently across 5 manifests (low complexity pattern)
All modifications are YAML configuration additions with no code logic changes
Each file requires validation that RBAC permissions are correctly specified and aligned
Verify that serviceDiscoveryRole: EndpointSlice field is syntactically valid for ServiceMonitor resources

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between fda7a9f and 57acbaf.

📒 Files selected for processing (5)

bindata/kube-proxy/monitor.yaml (2 hunks)
bindata/network/frr-k8s/monitor.yaml (2 hunks)
bindata/network/network-metrics/002-prometheus.yaml (2 hunks)
bindata/network/openshift-sdn/monitor.yaml (3 hunks)
bindata/network/ovn-kubernetes/common/monitor-node.yaml (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

bindata/kube-proxy/monitor.yaml
bindata/network/frr-k8s/monitor.yaml
bindata/network/network-metrics/002-prometheus.yaml
bindata/network/openshift-sdn/monitor.yaml
bindata/network/ovn-kubernetes/common/monitor-node.yaml

🔇 Additional comments (5)

bindata/network/network-metrics/002-prometheus.yaml (1)

27-27: Consistent implementation across ServiceMonitor and RBAC.

Changes align with the pattern seen in other monitored workloads. RBAC rule is correctly structured with all required verbs.

Also applies to: 65-72

bindata/network/frr-k8s/monitor.yaml (1)

60-60: Consistent pattern applied across frr-k8s ServiceMonitor and RBAC.

Changes follow the established pattern. Note that serviceDiscoveryRole applies cluster-wide to the ServiceMonitor's scope, regardless of the number of endpoints defined.

Also applies to: 78-85

bindata/network/openshift-sdn/monitor.yaml (1)

27-27: Correctly applied to multiple ServiceMonitors with consolidated RBAC.

Both ServiceMonitor resources (monitor-sdn and monitor-sdn-controller) correctly receive serviceDiscoveryRole: EndpointSlice. The single RBAC rule properly covers both, avoiding duplication—this is the correct approach.

Also applies to: 76-76, 116-123

bindata/kube-proxy/monitor.yaml (1)

27-27: No issues found — serviceDiscoveryRole is valid and RBAC is correctly configured.

ServiceMonitor supports the serviceDiscoveryRole field with valid values "EndpointSlice" or "Endpoints" and is documented in the Prometheus Operator API reference. The RBAC rule at lines 67-74 correctly grants the required permissions for reading EndpointSlices.

bindata/network/ovn-kubernetes/common/monitor-node.yaml (1)

34-34: Verified: All DaemonSet ServiceMonitors have serviceDiscoveryRole field; all non-DaemonSet ServiceMonitors correctly lack it.

Verification confirms the review comment is accurate. All five DaemonSet workload ServiceMonitors (kube-proxy, frr-k8s, network-metrics-daemon, sdn, ovnkube-node) have received the serviceDiscoveryRole: EndpointSlice field. The five ServiceMonitors without this field correctly target Deployments, not DaemonSets. No DaemonSet ServiceMonitors were missed.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

simonpasquier · 2025-11-19T14:20:43Z

/skip
/retest-required

simonpasquier

/lgtm

Let's see if the e2e failures are caused by the PR but it looks ok to me.

slashpai · 2025-11-20T00:27:00Z

/retest-required

slashpai · 2025-11-20T04:57:53Z

/retest-required

slashpai · 2025-11-20T07:28:26Z

/test e2e-aws-ovn-windows

simonpasquier · 2025-11-20T13:15:06Z

/test e2e-aws-ovn-windows

simonpasquier · 2025-11-21T08:19:47Z

/retest-required

openshift-ci · 2025-11-21T10:56:50Z

@slashpai: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/4.21-upgrade-from-stable-4.20-e2e-gcp-ovn-upgrade	`57acbaf`	link	false	`/test 4.21-upgrade-from-stable-4.20-e2e-gcp-ovn-upgrade`
ci/prow/4.21-upgrade-from-stable-4.20-e2e-azure-ovn-upgrade	`57acbaf`	link	false	`/test 4.21-upgrade-from-stable-4.20-e2e-azure-ovn-upgrade`
ci/prow/security	`57acbaf`	link	false	`/test security`
ci/prow/frrk8s-e2e	`57acbaf`	link	false	`/test frrk8s-e2e`
ci/prow/4.21-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade	`57acbaf`	link	false	`/test 4.21-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade`

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.

slashpai · 2025-11-24T07:30:32Z

windows test failure looks legit

=== RUN   TestWMCO/destroy/Deletion/Prometheus_configuration
2025/11/21 10:40:24 test failed, attempting to gather Node logs
--- FAIL: TestWMCO (791.85s)
    --- FAIL: TestWMCO/destroy (791.12s)
        --- PASS: TestWMCO/destroy/Mirror_settings_cleared_from_nodes (0.20s)
        --- FAIL: TestWMCO/destroy/Deletion (790.78s)
            --- FAIL: TestWMCO/destroy/Deletion/BYOH_node_removal (564.17s)
                --- FAIL: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal (138.78s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/windows_exporter (0.00s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/kube-proxy (0.00s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/hybrid-overlay-node (0.00s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/kubelet (0.00s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/windows-instance-config-daemon (0.00s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/containerd (0.00s)
                    --- PASS: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/Proxy_certificate_removal (0.00s)
                    --- FAIL: TestWMCO/destroy/Deletion/BYOH_node_removal/ip-10-0-27-91.us-west-2.compute.internal/AWS_metadata_endpoint (10.41s)
            --- PASS: TestWMCO/destroy/Deletion/Prometheus_configuration (0.14s)
FAIL
FAIL	github.com/openshift/windows-machine-config-operator/test/e2e	792.076s

we might need to update in https://github.com/openshift/windows-machine-config-operator as well IIUC

simonpasquier · 2025-11-24T08:16:52Z

@slashpai correct!

slashpai · 2025-12-16T16:45:54Z

openshift/windows-machine-config-operator#3583 is merged lets retest

/retest-required

slashpai · 2025-12-16T16:46:35Z

/hold cancel

ricky-rav

/LGTM

slashpai · 2025-12-17T10:58:18Z

@ricky-rav Thank you for the review, I think it needs approved label

ricky-rav · 2025-12-17T11:01:15Z

@kyrtapz PTAL

kyrtapz · 2025-12-17T15:10:34Z

/lgtm
@slashpai where you able to verify this change behaves as expected? I am not sure it's covered by CI

openshift-ci · 2025-12-17T15:11:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyrtapz, ricky-rav, simonpasquier, slashpai

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:

~~OWNERS~~ [kyrtapz]

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

slashpai · 2025-12-19T06:13:40Z

I am trying to add a e2e test openshift/origin#30627

Add a test to verify that network DaemonSet ServiceMonitor targets are correctly discovered. This validates the changes from openshift/cluster-network-operator#2839 which migrates DaemonSet ServiceMonitors to use EndpointSlice instead of the Endpoints API. Signed-off-by: Jayapriya Pai <janantha@redhat.com> Assisted-By: Cursor AI

slashpai · 2025-12-19T11:58:37Z

/testwith openshift/origin#30627

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 17, 2025

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2025

openshift-ci bot requested review from arghosh93 and miheer November 17, 2025 06:53

simonpasquier reviewed Nov 19, 2025

View reviewed changes

openshift-ci bot assigned simonpasquier Nov 19, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 19, 2025

slashpai mentioned this pull request Nov 24, 2025

MON-4437: Migrate windows-exporter ServiceMonitor to EndpointSlice openshift/windows-machine-config-operator#3583

Merged

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 16, 2025

ricky-rav approved these changes Dec 17, 2025

View reviewed changes

openshift-ci bot assigned ricky-rav Dec 17, 2025

openshift-ci bot assigned kyrtapz Dec 17, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2025

slashpai mentioned this pull request Dec 19, 2025

WIP MON-4432: Add e2e test for network DaemonSet target discovery openshift/origin#30627

Open

MON-4432: Add EndpointSlice support to DaemonSet ServiceMonitors #2839

Are you sure you want to change the base?

MON-4432: Add EndpointSlice support to DaemonSet ServiceMonitors #2839

Uh oh!

Conversation

slashpai commented Nov 17, 2025

Uh oh!

openshift-ci-robot commented Nov 17, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

simonpasquier commented Nov 19, 2025

Uh oh!

simonpasquier left a comment

Choose a reason for hiding this comment

Uh oh!

slashpai commented Nov 20, 2025

Uh oh!

slashpai commented Nov 20, 2025

Uh oh!

slashpai commented Nov 20, 2025

Uh oh!

simonpasquier commented Nov 20, 2025

Uh oh!

simonpasquier commented Nov 21, 2025

Uh oh!

openshift-ci bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slashpai commented Nov 24, 2025

Uh oh!

simonpasquier commented Nov 24, 2025

Uh oh!

slashpai commented Dec 16, 2025

Uh oh!

slashpai commented Dec 16, 2025

Uh oh!

ricky-rav left a comment

Choose a reason for hiding this comment

Uh oh!

slashpai commented Dec 17, 2025

Uh oh!

ricky-rav commented Dec 17, 2025

Uh oh!

kyrtapz commented Dec 17, 2025

Uh oh!

openshift-ci bot commented Dec 17, 2025

Uh oh!

slashpai commented Dec 19, 2025

Uh oh!

slashpai commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

openshift-ci-robot commented Nov 17, 2025 •

edited by openshift-ci bot

Loading

coderabbitai bot commented Nov 17, 2025 •

edited

Loading

openshift-ci bot commented Nov 21, 2025 •

edited

Loading