MON-4439: monitoring: ensure platform prometheus targets are discovered via EndpointSlices - #31431
MON-4439: monitoring: ensure platform prometheus targets are discovered via EndpointSlices#31431machine424 wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughPrometheus target retrieval is centralized in a shared helper, and target data now exposes discovered labels. A new test detects targets discovered through Kubernetes Endpoints instead of EndpointSlices. ChangesPrometheus target validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/pipeline required |
|
Scheduling required tests: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: machine424 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retitle MON-4439: monitoring: ensure platform prometheus targets are discovered via EndpointSlices |
|
@machine424: This pull request references MON-4439 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 story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/prometheus/prometheus.go`:
- Around line 1003-1004: Update fetchPrometheusTargets to pass its ctx to
helper.GetURLWithToken, then thread that context through the helper’s
retry/polling loop instead of using context.Background(). Ensure cancellation or
timeout terminates the request and retry loop promptly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2b6c0f47-c94a-40c5-9868-30af2f91d558
📒 Files selected for processing (1)
test/extended/prometheus/prometheus.go
| func fetchPrometheusTargets(ctx context.Context, prometheusURL, bearerToken string) (*prometheusTargets, error) { | ||
| contents, err := helper.GetURLWithToken(helper.MustJoinUrlPath(prometheusURL, "api/v1/targets"), bearerToken) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant function and nearby call sites.
sed -n '980,1045p' test/extended/prometheus/prometheus.go
printf '\n---- call sites ----\n'
rg -n "fetchPrometheusTargets\(" test/extended/prometheus/prometheus.go
printf '\n---- helper definition ----\n'
rg -n "func GetURLWithToken|GetURLWithToken\(" -S .Repository: openshift/origin
Length of output: 3505
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the helper implementation once its location is known.
helper_file=$(rg -l "func GetURLWithToken" -S . | head -n 1 || true)
if [ -n "${helper_file}" ]; then
echo "HELPER_FILE=$helper_file"
sed -n '1,220p' "$helper_file"
fiRepository: openshift/origin
Length of output: 8259
Thread ctx through the Prometheus target fetch
fetchPrometheusTargets drops its ctx, and helper.GetURLWithToken retries on context.Background() for up to a minute. A canceled or timed-out spec can still block in this request path; pass the spec context into the helper and its polling loop.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/prometheus/prometheus.go` around lines 1003 - 1004, Update
fetchPrometheusTargets to pass its ctx to helper.GetURLWithToken, then thread
that context through the helper’s retry/polling loop instead of using
context.Background(). Ensure cancellation or timeout terminates the request and
retry loop promptly.
Source: Path instructions
|
@machine424: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: cb49ebe
New tests seen in this PR at sha: cb49ebe
|
|
/payload 5.0 ci blocking |
|
@machine424: trigger 5 job(s) of type blocking for the ci release of OCP 5.0
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0ca52800-89cc-11f1-9a5e-0e2cccf21c79-0 |
|
/payload 5.0 nightly blocking |
|
@machine424: trigger 14 job(s) of type blocking for the nightly release of OCP 5.0
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0fe6e580-89cc-11f1-985e-9140e3791094-0 |
from #30611
Summary by CodeRabbit