add persistent volume capacity metric #674
Conversation
k8s-ci-robot
added
cncf-cla: yes
| { | ||
| Name: "kube_persistentvolume_capacity", | ||
| Type: metric.MetricTypeGauge, | ||
| Help: "persistentvolume capacity in Gigabytes.", |
There was a problem hiding this comment.
prometheus metrics best practice is to use base units, so this should be bytes (we're not loosing precision because we're using float64)
| Type: metric.MetricTypeGauge, | ||
| Help: "persistentvolume capacity in Gigabytes.", | ||
| GenerateFunc: wrapPersistentVolumeFunc(func(p *v1.PersistentVolume) *metric.Family { | ||
| storage := p.Spec.Capacity[v1.ResourceStorage] |
There was a problem hiding this comment.
could there ever be an additoinal capacity? if so we should make the v1.ResourceStorage a label instead, similar to what we do with resource_requests on pod/containers
There was a problem hiding this comment.
I do not believe there can ever be an additional capacity, because p.Spec.Capacity is a v1.ResourceList, which in turn is a map[ResourceName]resource.Quantity. So, there will only be one resource.Quantity for each ResourceName in the map (in this case the resource name is v1.ResourceStorage).
The name v1.ResourceList is a little misleading in that regard, but since it's a map to resource.Quantity (which is never a list) I think we're good.
| { | ||
| Name: "kube_persistentvolume_capacity", | ||
| Type: metric.MetricTypeGauge, | ||
| Help: "persistentvolume capacity in Gigabytes.", |
There was a problem hiding this comment.
| Help: "persistentvolume capacity in Gigabytes.", | |
| Help: "Persistentvolume capacity in gigabytes.", |
Plus the suggestion by @brancz.
|
Looks like the build probably needs restarting:
|
| @@ -108,6 +108,21 @@ var ( | |||
| } | |||
| }), | |||
| }, | |||
| { | |||
| Name: "kube_persistentvolume_capacity", | |||
There was a problem hiding this comment.
if resource storage is the only capacity possible then this should be kube_persistentvolume_capacity_bytes
|
/lgtm |
k8s-ci-robot
added
lgtm
|
Sorry about that, looks like a recent merge created a conflict. Could you rebase this? Thanks! |
r0fls
force-pushed
the
add-pv-capacity-metric
branch
from
37f5206
to
58fd352
Compare
k8s-ci-robot
removed
the
lgtm
|
@brancz no problem, rebased ✔️ |
|
By the way it was just that the Documentation directory was renamed docs |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brancz, r0fls, tariq1890 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it: Add
kube_persistentvolume_capacityto track PV capacity.Which issue(s) this PR fixes:
Fixes #555