feat(pvc): add metrics for VolumeAttributesClass support - #3036
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: FRosner 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 |
|
This issue is currently awaiting triage. If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the The 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. |
|
|
|
Welcome @FRosner! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughPersistentVolumeClaim metric collection now exposes requested and current ChangesPersistentVolumeClaim metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR adds localized experimental PVC metrics without any supplied evidence of an actionable merge-blocking risk. Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Expose the desired vs. currently-applied VolumeAttributesClass on a PersistentVolumeClaim, and the state of any in-progress ModifyVolume operation, via three new EXPERIMENTAL metrics: kube_persistentvolumeclaim_volume_attributes_class, kube_persistentvolumeclaim_status_current_volume_attributes_class, and kube_persistentvolumeclaim_status_modify_volume_status.
Yeah I'm doing it. I am trying to sign as an employee and I'm in contact with our CLA manager but it's a bit difficult for me. I went through this at least 4 times. Now a 5th time and it finally went through :) |
|
/pony Twilight Sparkle |
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 kubernetes-sigs/prow repository. |
|
@CatherineF-dev would you be the one to review the PR? What's the expected timeline to get a reviewer assigned? :) |
|
|
Thanks for picking this up @CatherineF-dev!
The motivating case: I have StatefulSets whose PVCs I want to migrate to a different
None of this is derivable from any existing KSM metric today —
I did consider collapsing these, but I think 3 is the right number given KSM's existing conventions:
I could technically fold Happy to adjust if you see a cleaner shape, though. |
Do you think it's a common use case for other users? |
If they are using VolumeAttributeClasses, yes. I don't know how widely they are adopted, but they are stable since Kubernetes v1.36 so I think it makes sense to provide observability around them. |
|
Don't see the point. These metrics are quite useful |
|
I see. VolumeAttributesClass is a new resource type. Option2: Option2 will increase query friction on joining pvc and vac metric How do you think @dgrisonnet which option is preferred? |
Yes. Please note that I am going to submit another PR to add metrics about the VACs. This PR here is only limited to the VAC related fields in PVC, not the VAC resources themselves. So that means Option 2 is anyway going to happen and is not related to this PR.
You asked that earlier. We could do it, it already carries the storage class and it would require one less join. Why I chose to add a new metric:
I'm open to suggestions, just outlining my reasoning. |
dgrisonnet
left a comment
There was a problem hiding this comment.
VolumeAttributesClass is a new resource type.
Option1:
kube_persistentvolumeclaim_volumeattributesclass_info
kube_persistentvolumeclaim_volumeattributesclass_status
Option2:
kube_volumeattributesclass_info
kube_volumeattributesclass_status
Option2 will increase query friction on joining pvc and vac metric
How do you think @dgrisonnet which option is preferred?
For VAC metric, they should be named kube_volumeattributesclass_ like your option 2, but for PVC metrics about VAC, it should be kube_persistentvolumeclaim_volume_attributes_class like it is in this PR
| "", | ||
| wrapPersistentVolumeClaimFunc(func(p *v1.PersistentVolumeClaim) *metric.Family { | ||
| volumeAttributesClassName := "" | ||
| if p.Spec.VolumeAttributesClassName != nil { |
There was a problem hiding this comment.
if it is nil, it would be better to skip the metric
Emitting a row with an empty volume_attributes_class label for every PVC not using VolumeAttributesClass added noise; follow the existing convention (e.g. modify_volume_status) of omitting the metric instead.
Why
Kubernetes'
VolumeAttributesClassAPI (GA, default-enabled since v1.34) lets aPersistentVolumeClaimreference a mutable class of storage attributes, and lets the CSI driver report progress as it applies changes. kube-state-metrics currently exposes no information about this on PVCs.What
Adds three new EXPERIMENTAL metrics to the PVC store:
kube_persistentvolumeclaim_volume_attributes_class— the VolumeAttributesClass requested in the PVC spec.kube_persistentvolumeclaim_status_current_volume_attributes_class— the VolumeAttributesClass currently applied, as reported by the CSI driver. Kept separate from the spec metric so drift between desired and applied class can be detected.kube_persistentvolumeclaim_status_modify_volume_status— one-hot status (Pending/InProgress/Infeasible) of an in-progressModifyVolumeoperation, with atarget_volume_attributes_classlabel. Emits zero rows when no modification is in progress.The two new
PersistentVolumeClaimConditionTypevalues (ModifyVolumeError,ModifyingVolume) introduced alongside this API are already covered for free by the existing generickube_persistentvolumeclaim_status_conditionmetric.This is scoped to PVC metrics only — no new standalone
VolumeAttributesClassresource/store. I will add these metrics in a follow-up PR.Summary by CodeRabbit
New Features
Bug Fixes
Documentation