USHIFT-6951: Add kube-state-metrics as optional MicroShift component#6809
Open
copejon wants to merge 3 commits into
Open
USHIFT-6951: Add kube-state-metrics as optional MicroShift component#6809copejon wants to merge 3 commits into
copejon wants to merge 3 commits into
Conversation
|
@copejon: This pull request references USHIFT-6951 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
Contributor
|
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:
WalkthroughAdds optional kube-state-metrics: Kubernetes manifests, kustomize overlays and release JSONs, OTEL Collector scrape config, RPM packaging/install rules, and test/healthcheck wiring. Changeskube-state-metrics metrics collection feature
Sequence Diagram(s)sequenceDiagram
participant OTEL as OTELCollector
participant KubeAPI as KubernetesAPI
participant KSM as kube-state-metrics
participant RBAC as kube-rbac-proxy
KubeAPI->>KSM: apply manifests (Namespace, SA, ClusterRole/Binding, ConfigMap, Secret, Deployment, Service)
OTEL->>KubeAPI: discover endpoints in openshift-monitoring via kubeconfig
OTEL->>RBAC: HTTPS scrape to kube-state-metrics (https-main)
RBAC->>KSM: forward proxied request to localhost upstream (127.0.0.1:8081)
KSM-->>RBAC: metrics response
RBAC-->>OTEL: proxied metrics response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
This was referenced Jun 5, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: copejon 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 |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
assets/optional/kube-state-metrics/kustomization.aarch64.yaml (1)
2-4: 💤 Low valueConsider adding version comments for maintainability.
While digest pinning is secure, adding a comment with the human-readable version would improve maintainability when updating images.
📝 Example with version comment
images: + # kube-state-metrics v2.x.x - name: quay.io/openshift/kube-state-metrics newName: registry.redhat.io/openshift4/ose-kube-state-metrics-rhel9 digest: sha256:c84d67c1427187b67a8eecde93ed964ffd390afbdac58eb9d943489ee7729b1a🤖 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 `@assets/optional/kube-state-metrics/kustomization.aarch64.yaml` around lines 2 - 4, Add a human-readable version comment next to the image entry for quay.io/openshift/kube-state-metrics / newName registry.redhat.io/openshift4/ose-kube-state-metrics-rhel9 so maintainers can see the tagged version alongside the digest; edit the kube-state-metrics image block to include a short comment (e.g., "# vX.Y.Z" or "# openshift X.Y" ) immediately above or beside the newName/digest lines and ensure the comment is kept in sync when updating the digest.assets/optional/kube-state-metrics/kustomization.x86_64.yaml (1)
2-4: 💤 Low valueConsider adding version comments for maintainability.
As with the aarch64 file, adding human-readable version comments would ease future updates and troubleshooting.
📝 Example with version comment
images: + # kube-state-metrics v2.x.x - name: quay.io/openshift/kube-state-metrics newName: registry.redhat.io/openshift4/ose-kube-state-metrics-rhel9 digest: sha256:cc300698b3f0abd36267e740e1669f83ca17aa4b710218c5f5b2ee71994af9ba🤖 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 `@assets/optional/kube-state-metrics/kustomization.x86_64.yaml` around lines 2 - 4, Add a human-readable version comment above the image mapping that documents the component and version for future maintainers; specifically, annotate the block referencing "quay.io/openshift/kube-state-metrics" / "registry.redhat.io/openshift4/ose-kube-state-metrics-rhel9" and its "digest" with a comment like a version or release tag (mirroring the approach used in the aarch64 file) so updates and troubleshooting can quickly identify which upstream release this digest corresponds to.
🤖 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`:
- Line 433: The help text for metrics that read from the maxAllowed field is
incorrect: change the wording from "Minimum" to "Maximum" for the metrics that
reference maxAllowed (e.g., the cpu metric whose help currently reads "Minimum
cpu resources the VerticalPodAutoscaler can set for containers matching the
name."). Update the help strings for both occurrences (the cpu maxAllowed metric
at the line shown and the second occurrence around line 470) to say "Maximum cpu
resources the VerticalPodAutoscaler can set for containers matching the name."
so they accurately describe maxAllowed.
In `@assets/optional/kube-state-metrics/03-deployment.yaml`:
- Around line 64-67: The manifest sets only resource requests; add corresponding
resource limits for cpu and memory on the three containers named
"kube-state-metrics", "kube-rbac-proxy-main", and "kube-rbac-proxy-self" (add a
"limits" block with appropriate cpu and memory values alongside the existing
"requests"). Update each container's spec where requests are defined (there are
additional identical request blocks for the same containers elsewhere in the
file) so that every container includes both resources.requests and
resources.limits (cpu and memory) to satisfy the "Resource limits on every
container" guideline.
- Around line 35-120: Add proper liveness and readiness probes for each
container (kube-state-metrics, kube-rbac-proxy-main, kube-rbac-proxy-self): for
kube-state-metrics add an httpGet readiness and liveness probe against the
metrics/health endpoint on port 8081 (and telemetry port 8082 for self-probe if
applicable) with sensible defaults (initialDelaySeconds, periodSeconds,
timeoutSeconds, failureThreshold); for both kube-rbac-proxy containers add
httpGet probes against their secure ports (8443 for kube-rbac-proxy-main, 9443
for kube-rbac-proxy-self) pointing to a health or /ready endpoint and similar
timeout/period settings; ensure probes are added under each container spec using
the field names readinessProbe and livenessProbe so Kubernetes can correctly
manage restarts and traffic routing.
- Line 68: Replace the empty securityContext maps with explicit hardened
settings at both pod and container scope: for the PodSpec-level securityContext
set runAsNonRoot: true and runAsUser (e.g. 1000), and for each container's
securityContext set allowPrivilegeEscalation: false, readOnlyRootFilesystem:
true, runAsNonRoot: true (and runAsUser if needed), capabilities: drop: ["ALL"]
(and add only specific capabilities if required), and seccompProfile: type:
RuntimeDefault; apply these changes for the securityContext entries referenced
(pod-level and container-level occurrences around the empty maps) so containers
run non-root, cannot escalate privileges, have a read-only rootfs, dropped
capabilities, and use the default seccomp profile.
- Around line 42-50: The deployment args contain duplicate --metric-denylist
flags so the later one overrides the earlier denylist; merge all denylist
regexes into a single --metric-denylist=... value (comma-separated) that
includes ^kube_secret_labels$, ^kube_.+_annotations$,
^kube_customresource_.+_annotations_info$, ^kube_customresource_.+_labels_info$
and remove the second --metric-denylist entry; ensure the existing
--metric-labels-allowlist entry remains unchanged and that only one
--metric-denylist flag is present in the container args.
In
`@packaging/observability/otelcol.d/microshift-metrics-kube-state-metrics.yaml`:
- Around line 10-19: For the scrape block using kubernetes_sd_configs with role:
endpoints that keeps kube-state-metrics;https-main (the relabel_configs regex),
add a tls_config.server_name set to the kube-state-metrics Service DNS name
(e.g., kube-state-metrics.openshift-monitoring.svc) so TLS hostname verification
succeeds against the serving cert SANs; update the scrape_config that contains
kubernetes_sd_configs/role: endpoints and the relabel_configs regex
"kube-state-metrics;https-main" to include tls_config.server_name with the
appropriate service DNS (or make it configurable if needed).
---
Nitpick comments:
In `@assets/optional/kube-state-metrics/kustomization.aarch64.yaml`:
- Around line 2-4: Add a human-readable version comment next to the image entry
for quay.io/openshift/kube-state-metrics / newName
registry.redhat.io/openshift4/ose-kube-state-metrics-rhel9 so maintainers can
see the tagged version alongside the digest; edit the kube-state-metrics image
block to include a short comment (e.g., "# vX.Y.Z" or "# openshift X.Y" )
immediately above or beside the newName/digest lines and ensure the comment is
kept in sync when updating the digest.
In `@assets/optional/kube-state-metrics/kustomization.x86_64.yaml`:
- Around line 2-4: Add a human-readable version comment above the image mapping
that documents the component and version for future maintainers; specifically,
annotate the block referencing "quay.io/openshift/kube-state-metrics" /
"registry.redhat.io/openshift4/ose-kube-state-metrics-rhel9" and its "digest"
with a comment like a version or release tag (mirroring the approach used in the
aarch64 file) so updates and troubleshooting can quickly identify which upstream
release this digest corresponds to.
🪄 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: 060e9c6c-3fbf-4e9a-8297-183b2c97e1bc
📒 Files selected for processing (15)
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.yamlpackaging/observability/otelcol.d/microshift-metrics-kube-state-metrics.yamlpackaging/rpm/microshift.specpkg/healthcheck/microshift_optional_workloads.gotest/bin/common.sh
copejon
force-pushed
the
ushift-6951/kube-state-metrics
branch
from
ee20850 to
f20f28d
Compare
coderabbitai
Bot
added
the
ready-for-human-review
copejon
force-pushed
the
ushift-6951/kube-state-metrics
branch
from
f20f28d to
93e8e18
Compare
coderabbitai
Bot
removed
the
ready-for-human-review
Contributor
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/03-deployment.yaml`:
- Around line 96-98: Change the TLS secret volume mounts for the kube-rbac-proxy
containers to be read-only: locate the Deployment definition that contains the
containers named kube-rbac-proxy-main and kube-rbac-proxy-self and update their
volumeMount entry with mountPath /etc/tls/private and name
kube-state-metrics-tls to set readOnly: true (replace the current readOnly:
false) for both containers.
🪄 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: dea0740c-1242-46ec-88b5-a58fdbe5dbe4
📒 Files selected for processing (16)
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.yamlpackaging/observability/otelcol.d/microshift-metrics-kube-state-metrics.yamlpackaging/rpm/microshift.specpkg/healthcheck/microshift_optional_workloads.goscripts/auto-rebase/assets.yamltest/bin/common.sh
✅ Files skipped from review due to trivial changes (3)
- assets/optional/kube-state-metrics/kustomization.aarch64.yaml
- scripts/auto-rebase/assets.yaml
- assets/optional/kube-state-metrics/01-service-account.yaml
🚧 Files skipped from review as they are similar to previous changes (11)
- assets/optional/kube-state-metrics/kustomization.x86_64.yaml
- assets/optional/kube-state-metrics/00-namespace.yaml
- packaging/observability/otelcol.d/microshift-metrics-kube-state-metrics.yaml
- assets/optional/kube-state-metrics/kustomization.yaml
- assets/optional/kube-state-metrics/04-service.yaml
- pkg/healthcheck/microshift_optional_workloads.go
- assets/optional/kube-state-metrics/01-cluster-role-binding.yaml
- assets/optional/kube-state-metrics/02-kube-rbac-proxy-secret.yaml
- test/bin/common.sh
- packaging/rpm/microshift.spec
- assets/optional/kube-state-metrics/01-cluster-role.yaml
copejon
force-pushed
the
ushift-6951/kube-state-metrics
branch
4 times, most recently
from
7d046a2 to
3631f72
Compare
coderabbitai
Bot
added
ready-for-human-review
Contributor
Author
|
@CodeRabbit re-evaluate past comments against current code and resolve those that have been satisfied |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch all review comments by coderabbitai on PR `#6809`
gh pr view 6809 --repo openshift/microshift --json reviews,reviewDecision
echo "---COMMENTS---"
gh api repos/openshift/microshift/pulls/6809/comments \
--jq '.[] | select(.user.login == "coderabbitai") | {id: .id, body: .body, path: .path, line: .line, resolved: .position}'
echo "---REVIEW_THREADS---"
gh api graphql -f query='
{
repository(owner: "openshift", name: "microshift") {
pullRequest(number: 6809) {
reviewThreads(first: 50) {
nodes {
id
isResolved
comments(first: 5) {
nodes {
author { login }
body
path
line
}
}
}
}
}
}
}'Length of output: 32315
Already resolved (5 threads — no action needed):
Still open (2 threads — unresolved in current code):
These two are still flagged by the coding guidelines. Let me know if you'd like to defer them or address them in this PR. 🧠 Learnings used |
coderabbitai
Bot
added
the
ready-for-human-review
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
copejon
force-pushed
the
ushift-6951/kube-state-metrics
branch
from
d0ba1fd to
c1bc1a5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add kube-state-metrics as an optional MicroShift component deployed via kustomize manifests and packaged as
microshift-metrics-kube-state-metricsRPM sub-package.This is PR 2/3 splitting #6763 into independently-mergeable patches. The three PRs can merge in any order.
Sibling: #6808 (metrics-server)
What's included
assets/optional/kube-state-metrics/) — Deployment with kube-rbac-proxy sidecars, ClusterRole with extensive RBAC, custom-resource-state ConfigMap (VPA, Gateway metrics), service-ca TLS, and own00-namespace.yamlfor independent deployabilitymergeWorkloads()function (identical to sibling PRs; git auto-merges)%package metrics-kube-state-metricswith per-file installs to081-microshift-kube-state-metrics/microshift-metrics-kube-state-metrics.yamlscrape config using Kubernetes endpoints SDtest/bin/common.shSummary by CodeRabbit
New Features
Chores
Tests