[DO NOT MERGE] UPSTREAM: <carry>: fix kube_persistentvolume_plugin_type_counts#2673
[DO NOT MERGE] UPSTREAM: <carry>: fix kube_persistentvolume_plugin_type_counts#2673dfajmon wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
|
@dfajmon: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dfajmon The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughPersistent volume controller adds a separate metrics plugin manager, initialized from metrics-specific volume plugins probed during setup. Local volume plugin is registered. Metrics plugins are probed and wired into controller initialization, allowing metrics collection to use different plugins than volume provisioning. ChangesMetrics Plugin Manager Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@cmd/kube-controller-manager/app/core.go`:
- Around line 334-336: The error return in newPersistentVolumeBinderController
incorrectly returns three values; change the error path in the metrics volume
plugin probe to return only (nil, error) by removing the extraneous boolean and
wrap the original probe error using fmt.Errorf("failed to probe metrics volume
plugins when starting persistentvolume controller: %w", err) so the function
signature (Controller, error) is respected and the underlying error is
preserved.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 55e2cc95-d7d6-404c-bb43-74e4598e3128
📒 Files selected for processing (5)
cmd/kube-controller-manager/app/core.gocmd/kube-controller-manager/app/plugins.gopkg/controller/volume/persistentvolume/metrics/metrics_test.gopkg/controller/volume/persistentvolume/pv_controller.gopkg/controller/volume/persistentvolume/pv_controller_base.go
…ng plugin_name Commit 9e29f95 refactored KCM volume plugins, replacing ProbeControllerVolumePlugins (which included CSI) with ProbeProvisionableRecyclableVolumePlugins (filtered to Provisionable/Deletable/Recyclable). The CSI plugin implements none of those interfaces, so it was excluded from volumePluginMgr. metrics.Register was still using &ctrl.volumePluginMgr, so getPVPluginName() -> FindPluginBySpec() failed for CSI PVs, falling back to "N/A" instead of e.g. "kubernetes.io/csi:ebs.csi.aws.com". Fix: initialize a separate metricsPluginMgr with all plugins (ProbePersistentVolumePlugins, no filter) and pass it to metrics.Register. Fixes: OCPBUGS-44815 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@dfajmon: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
|
@dfajmon: 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. |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR is related to:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Summary by CodeRabbit
New Features
Tests