USHIFT-7407: Wire prometheus receiver scrape.d drop-ins for metrics exporters#7068
USHIFT-7407: Wire prometheus receiver scrape.d drop-ins for metrics exporters#7068copejon wants to merge 6 commits into
Conversation
|
@copejon: This pull request references USHIFT-7407 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. |
openshift-ci-robot
added
the
jira/valid-reference
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughThe observability configuration now loads Prometheus scrape drop-ins for optional workloads. RBAC permits endpoint discovery and metrics access, RPM packaging installs the fragments, collector pipelines export scraped metrics, and Robot Framework tests verify kube-state-metrics, node-exporter, and metrics-server data. ChangesObservability scrape drop-ins
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OptionalWorkloads
participant KubernetesEndpoints
participant PrometheusReceiver
participant OTLPExporter
OptionalWorkloads->>KubernetesEndpoints: Expose HTTPS service endpoints
PrometheusReceiver->>KubernetesEndpoints: Discover endpoint targets
PrometheusReceiver->>OptionalWorkloads: Scrape configured metrics
PrometheusReceiver->>OTLPExporter: Export scraped metrics
Possibly related PRs
Suggested labels: Suggested reviewers: ✅ Pre-merge checks override appliedThe pre-merge checks have been overridden successfully. You can now proceed with the merge. Overridden by ❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
|
This is a follow-up PR to #6809 and should not be merged until that PR is merged. /hold |
openshift-ci
Bot
added
the
do-not-merge/hold
3939bed to
71ccb68
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@assets/optional/kube-state-metrics/02-custom-resource-state-configmap.yaml`:
- Around line 417-490: Update the HELP descriptions for the maxAllowed CPU and
memory metrics in the source bundle from “Minimum” to “Maximum,” while keeping
their extraction paths and names unchanged. Regenerate the rebase-managed custom
resource state ConfigMap so the generated manifest reflects both corrected
descriptions.
🪄 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: fce4f36a-9f76-42ea-9610-573c978d761c
📒 Files selected for processing (43)
assets/optional/kube-state-metrics/00-namespace.yamlassets/optional/kube-state-metrics/01-cluster-role-binding.yamlassets/optional/kube-state-metrics/01-cluster-role.yamlassets/optional/kube-state-metrics/01-service-account.yamlassets/optional/kube-state-metrics/02-custom-resource-state-configmap.yamlassets/optional/kube-state-metrics/02-kube-rbac-proxy-secret.yamlassets/optional/kube-state-metrics/03-deployment.yamlassets/optional/kube-state-metrics/04-service.yamlassets/optional/kube-state-metrics/kustomization.aarch64.yamlassets/optional/kube-state-metrics/kustomization.x86_64.yamlassets/optional/kube-state-metrics/kustomization.yamlassets/optional/kube-state-metrics/release-kube-state-metrics-aarch64.jsonassets/optional/kube-state-metrics/release-kube-state-metrics-x86_64.jsonassets/optional/node-exporter/00-namespace.yamlassets/optional/node-exporter/01-cluster-role-binding.yamlassets/optional/node-exporter/01-cluster-role.yamlassets/optional/node-exporter/01-security-context-constraints.yamlassets/optional/node-exporter/01-service-account.yamlassets/optional/node-exporter/02-accelerators-collector-configmap.yamlassets/optional/node-exporter/02-kube-rbac-proxy-secret.yamlassets/optional/node-exporter/03-daemonset.yamlassets/optional/node-exporter/04-service.yamlassets/optional/node-exporter/kustomization.aarch64.yamlassets/optional/node-exporter/kustomization.x86_64.yamlassets/optional/node-exporter/kustomization.yamlassets/optional/node-exporter/release-node-exporter-aarch64.jsonassets/optional/node-exporter/release-node-exporter-x86_64.jsonassets/optional/observability/02-cluster-role.yamlpackaging/observability/opentelemetry-collector-large.yamlpackaging/observability/opentelemetry-collector-medium.yamlpackaging/observability/opentelemetry-collector-small.yamlpackaging/observability/scrape.d/kube-state-metrics.yamlpackaging/observability/scrape.d/metrics-server.yamlpackaging/observability/scrape.d/node-exporter.yamlpackaging/rpm/microshift.specpkg/cmd/run.gopkg/components/metrics.gopkg/healthcheck/microshift_optional_workloads.goscripts/auto-rebase/assets_cluster_monitoring_operator.yamltest/assets/observability/otel_config.yamltest/bin/common.shtest/suites/optional/metrics.robottest/suites/optional/observability.robot
| processors: [ resourcedetection/system ] | ||
| exporters: [ otlp ] | ||
| metrics/pods: | ||
| metrics/scrape: |
There was a problem hiding this comment.
Can this name change break something in an upgrade from 4.22?
There was a problem hiding this comment.
No it affects internal logs only
71ccb68 to
7aa09ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@assets/optional/node-exporter/03-daemonset.yaml`:
- Line 32: Update the automountServiceAccountToken setting in the node-exporter
DaemonSet to false, since this workload does not require an automatically
mounted service account token.
- Line 33: Update the container definition under containers to add both
livenessProbe and readinessProbe for every container, using the required probe
paths and settings from the repository’s deployment instructions.
- Around line 148-150: Update the DaemonSet container securityContext to remove
privileged execution and root access by eliminating privileged: true and
runAsUser: 0. Retain only the minimum required security settings, and add an
explicit justification only if either setting is genuinely necessary.
- Around line 132-133: Remove hostNetwork and hostPID from the node-exporter
DaemonSet unless the deployment includes explicit, documented justification for
requiring host namespaces; preserve the DaemonSet’s other settings unchanged.
- Line 69: Update the container securityContext currently set to `{}` in the
node-exporter DaemonSet to disable privilege escalation, enforce a read-only
root filesystem, require a non-root user, and drop all Linux capabilities.
🪄 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: b9108772-37fe-4401-a04f-1a94eac4a4b3
📒 Files selected for processing (43)
assets/optional/kube-state-metrics/00-namespace.yamlassets/optional/kube-state-metrics/01-cluster-role-binding.yamlassets/optional/kube-state-metrics/01-cluster-role.yamlassets/optional/kube-state-metrics/01-service-account.yamlassets/optional/kube-state-metrics/02-custom-resource-state-configmap.yamlassets/optional/kube-state-metrics/02-kube-rbac-proxy-secret.yamlassets/optional/kube-state-metrics/03-deployment.yamlassets/optional/kube-state-metrics/04-service.yamlassets/optional/kube-state-metrics/kustomization.aarch64.yamlassets/optional/kube-state-metrics/kustomization.x86_64.yamlassets/optional/kube-state-metrics/kustomization.yamlassets/optional/kube-state-metrics/release-kube-state-metrics-aarch64.jsonassets/optional/kube-state-metrics/release-kube-state-metrics-x86_64.jsonassets/optional/node-exporter/00-namespace.yamlassets/optional/node-exporter/01-cluster-role-binding.yamlassets/optional/node-exporter/01-cluster-role.yamlassets/optional/node-exporter/01-security-context-constraints.yamlassets/optional/node-exporter/01-service-account.yamlassets/optional/node-exporter/02-accelerators-collector-configmap.yamlassets/optional/node-exporter/02-kube-rbac-proxy-secret.yamlassets/optional/node-exporter/03-daemonset.yamlassets/optional/node-exporter/04-service.yamlassets/optional/node-exporter/kustomization.aarch64.yamlassets/optional/node-exporter/kustomization.x86_64.yamlassets/optional/node-exporter/kustomization.yamlassets/optional/node-exporter/release-node-exporter-aarch64.jsonassets/optional/node-exporter/release-node-exporter-x86_64.jsonassets/optional/observability/02-cluster-role.yamlpackaging/observability/opentelemetry-collector-large.yamlpackaging/observability/opentelemetry-collector-medium.yamlpackaging/observability/opentelemetry-collector-small.yamlpackaging/observability/scrape.d/kube-state-metrics.yamlpackaging/observability/scrape.d/metrics-server.yamlpackaging/observability/scrape.d/node-exporter.yamlpackaging/rpm/microshift.specpkg/cmd/run.gopkg/components/metrics.gopkg/healthcheck/microshift_optional_workloads.goscripts/auto-rebase/assets_cluster_monitoring_operator.yamltest/assets/observability/otel_config.yamltest/bin/common.shtest/suites/optional/metrics.robottest/suites/optional/observability.robot
🚧 Files skipped from review as they are similar to previous changes (40)
- assets/optional/kube-state-metrics/kustomization.x86_64.yaml
- assets/optional/kube-state-metrics/kustomization.yaml
- assets/optional/node-exporter/release-node-exporter-aarch64.json
- assets/optional/node-exporter/00-namespace.yaml
- assets/optional/node-exporter/01-security-context-constraints.yaml
- assets/optional/kube-state-metrics/01-cluster-role-binding.yaml
- assets/optional/node-exporter/01-service-account.yaml
- assets/optional/node-exporter/02-kube-rbac-proxy-secret.yaml
- assets/optional/kube-state-metrics/release-kube-state-metrics-x86_64.json
- assets/optional/kube-state-metrics/00-namespace.yaml
- assets/optional/node-exporter/kustomization.yaml
- assets/optional/node-exporter/01-cluster-role.yaml
- assets/optional/node-exporter/04-service.yaml
- assets/optional/kube-state-metrics/01-service-account.yaml
- assets/optional/node-exporter/01-cluster-role-binding.yaml
- assets/optional/kube-state-metrics/04-service.yaml
- packaging/observability/scrape.d/node-exporter.yaml
- test/suites/optional/observability.robot
- assets/optional/kube-state-metrics/02-kube-rbac-proxy-secret.yaml
- assets/optional/node-exporter/release-node-exporter-x86_64.json
- packaging/observability/opentelemetry-collector-small.yaml
- scripts/auto-rebase/assets_cluster_monitoring_operator.yaml
- pkg/healthcheck/microshift_optional_workloads.go
- packaging/observability/scrape.d/kube-state-metrics.yaml
- assets/optional/kube-state-metrics/release-kube-state-metrics-aarch64.json
- assets/optional/node-exporter/kustomization.aarch64.yaml
- packaging/observability/scrape.d/metrics-server.yaml
- assets/optional/node-exporter/kustomization.x86_64.yaml
- test/assets/observability/otel_config.yaml
- assets/optional/kube-state-metrics/01-cluster-role.yaml
- assets/optional/kube-state-metrics/kustomization.aarch64.yaml
- assets/optional/observability/02-cluster-role.yaml
- pkg/cmd/run.go
- packaging/observability/opentelemetry-collector-large.yaml
- packaging/observability/opentelemetry-collector-medium.yaml
- assets/optional/node-exporter/02-accelerators-collector-configmap.yaml
- assets/optional/kube-state-metrics/02-custom-resource-state-configmap.yaml
- assets/optional/kube-state-metrics/03-deployment.yaml
- packaging/rpm/microshift.spec
- test/suites/optional/metrics.robot
|
Addressed CodeRabbit summary pre-merge check findings:
|
|
/lgtm |
|
/test e2e-aws-tests-periodic-arm |
ec70660 to
b42ae8c
Compare
|
/lgtm |
coderabbitai
Bot
added
the
ready-for-human-review
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: copejon, pmtk 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 |
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
openshift-ci
Bot
added
the
needs-rebase
87f768f to
e02b025
Compare
|
New changes are detected. LGTM label has been removed. |
e02b025 to
644a3db
Compare
…ip, update profile comments
…e name The prometheus receiver's kubernetes_sd role:endpoints requires the observability-client service account to list/watch endpoints. Without this permission all scrape.d drop-in configs fail with a 403. Also fixes a misleading comment on the nonResourceURLs /metrics rule, renames the large profile pipeline from metrics/pods to metrics/scrape for consistency with the other profiles, and adds missing blank lines between RPM spec sections. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lone Metrics subpackages use Recommends (not Requires) for microshift-observability, so they can be installed without it. Without owning the parent /etc/microshift/observability directory, rpm -e leaves an empty orphaned directory behind. Add %dir ownership so the full tree is cleaned up on removal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Scrape Metrics From keyword validated the endpoint IP with an IPv4-only regex, which would reject IPv6 addresses and break IPv6-only CI. The jsonpath already returns a valid IP string from the Kubernetes API; the regex was unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Run With Kubeconfig merges stderr into stdout. In Kubernetes 1.33+, oc get endpoints emits a deprecation warning to stderr that gets prepended to the IP address, breaking the curl --resolve argument. Switch to Command Should Work which separates stdout from stderr.
644a3db to
20912ff
Compare
openshift-ci
Bot
removed
the
needs-rebase
|
/verified by ci |
openshift-ci-robot
added
the
verified
|
@copejon: This PR has been marked as verified by 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. |
|
@copejon: This pull request references USHIFT-7407 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. |
|
/test e2e-aws-tests |
|
/test test-rebase |
|
/test e2e-aws-tests |
|
/test e2e-aws-tests-bootc-periodic-el10 to exercise the new RF tests at least once |
|
/test e2e-aws-tests-periodic |
| @@ -0,0 +1,37 @@ | |||
| scrape_configs: | |||
| - job_name: kube-state-metrics | |||
| scrape_interval: 30s | |||
There was a problem hiding this comment.
All scrape_interval values in the opentelemetry-collector-(large|medium|small).yaml are always 10s.
But here is 30s. Should we use 10s for consistency?
Not affecting us or RF tests, but mixing different scrape intervals may be problematic in Prom queries targetting different metrics.
| Wait Until Keyword Succeeds 60s 5s | ||
| ... Check Prometheus Exporter ${USHIFT_HOST} ${PROM_EXPORTER_PORT} kube_node_info |
There was a problem hiding this comment.
I'd suggest to use the same approach we have for other RF Test Cases in this Suite. I find value on checking both the exporter endpoint and running a PromQL query.
| Wait Until Keyword Succeeds 60s 5s | |
| ... Check Prometheus Exporter ${USHIFT_HOST} ${PROM_EXPORTER_PORT} kube_node_info | |
| VAR ${METRIC} kube_node_info scope=TEST | |
| Check Prometheus Query ${PROMETHEUS_HOST} ${PROMETHEUS_PORT} ${METRIC} | |
| Check Prometheus Exporter ${USHIFT_HOST} ${PROM_EXPORTER_PORT} ${METRIC} |
Same idea for the other 2 test cases:
Node Exporter Metrics Are Exported Via Scrape Drop-InMetrics Server Metrics Are Exported Via Scrape Drop-In
|
/retest |
|
@copejon: The following tests 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. |
5 participants
Summary
scrape_config_files: ["/etc/microshift/observability/scrape.d/*.yaml"]to all three otel-col profiles (small/medium/large), enabling prometheus receiver to auto-discover drop-in scrape configskube-state-metrics.yaml,node-exporter.yaml,metrics-server.yaml) that usekubernetes_sd_configswithrole: endpointsand mTLS to scrape each exporter's kube-rbac-proxy sidecarmicroshift-observability-metrics-server,-metrics-kube-state,-metrics-node-exporter) owns its drop-in file with%config(noreplace), so installing the metrics package automatically enables scraping when observability is presentendpointslist/watch to theopenshift-observability-clientClusterRole for kubernetes_sd endpoint discoveryDesign
The scrape.d directory uses a glob-based drop-in pattern: the otel-col prometheus receiver watches
/etc/microshift/observability/scrape.d/*.yamland picks up any file placed there. Each metrics RPM subpackage installs its own scrape config, so the set of active scrape targets is determined entirely by which packages are installed — no manual configuration needed.All three exporters are scraped over HTTPS using the observability-client certificate (mTLS through kube-rbac-proxy) and discovered via kubernetes_sd
role: endpointsin theopenshift-monitoringnamespace.Test plan
test/suites/optional/observability.robot— three new test cases verify characteristic metrics from each exporter (kube_node_info,node_cpu_seconds_total,metrics_server_kubelet_request_total)test/suites/optional/metrics.robot— existing metrics test suite validates exporter deployment and service wiringushift-devel-el98: confirmed all three exporters scraped successfully with live mTLSJIRA
https://issues.redhat.com/browse/USHIFT-7407
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests