From 7196a2537ecc2bdedca7a4df8cf878296e5bb5b5 Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Wed, 16 Aug 2017 23:52:33 -0700 Subject: Proposal for adding PVC info to VolumeStats Flushes out details for part 1 of the changes described in https://github.com/kubernetes/community/pull/855 Feature: https://github.com/kubernetes/features/issues/363 --- .../design-proposals/volume_stats_pvc_ref.md | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 contributors/design-proposals/volume_stats_pvc_ref.md diff --git a/contributors/design-proposals/volume_stats_pvc_ref.md b/contributors/design-proposals/volume_stats_pvc_ref.md new file mode 100644 index 00000000..917421f5 --- /dev/null +++ b/contributors/design-proposals/volume_stats_pvc_ref.md @@ -0,0 +1,54 @@ +# Add PVC reference in Volume Stats + +## Background +Pod volume stats tracked by kubelet do not currently include any information about the PVC (if the pod volume was referenced via a PVC) + +This prevents exposing (and querying) volume metrics labeled by PVC name which is preferable for users, given that PVC is a top-level API object. + +## Proposal + +Modify ```VolumeStats``` tracked in Kubelet and populate with PVC info: + +``` +// VolumeStats contains data about Volume filesystem usage. +type VolumeStats struct { + // Embedded FsStats + FsStats + // Name is the name given to the Volume + // +optional + Name string `json:"name,omitempty"` ++ // PVCRef is a reference to the measured PVC. ++ // +optional ++ PVCRef PVCReference `json:"pvcRef"` +} + ++// PVCReference contains enough information to describe the referenced PVC. ++type PVCReference struct { ++ Name string `json:"name"` ++ Namespace string `json:"namespace"` ++} +``` + +## Implementation +2 options are described below. Option 1 supports current requirements/requested use cases. Option 2 supports an additional use case that was being discussed and is called out for completeness/discussion/feedback. + +### Option 1 +- Modify ```kubelet::server::stats::calcAndStoreStats()``` + - If the pod volume is referenced via a PVC, populate ```PVCRef``` in VolumeStats using the Pod spec + + - The Pod spec is already available in this method, so the changes are contained to this function. + +- The limitation of this approach is that we're limited to reporting only what is available in the pod spec (Pod namespace and PVC claimname) + +### Option 2 +- Modify the ```volumemanager::GetMountedVolumesForPod()``` (or add a new function) to return additional volume information from the ASOW/DSOW caches + - Use this to populate PVCRef in VolumeStats + +- This allows us to get information not available in the Pod spec such as the PV name/UID which can be used to label metrics - enables exposing/querying volume metrics by PV name +- It's unclear whether this is a use case we need to/should support: + * Volume metrics are only refreshed for mounted volumes which implies a bound/available PVC + * We expect most user-storage interactions to be via the PVC + + + + -- cgit v1.2.3 From 8f90478df125f17748b02ae64eba3aeb7475dfc6 Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Sun, 27 Aug 2017 23:40:56 -0700 Subject: Address review feedback --- contributors/design-proposals/volume_stats_pvc_ref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributors/design-proposals/volume_stats_pvc_ref.md b/contributors/design-proposals/volume_stats_pvc_ref.md index 917421f5..0720a19f 100644 --- a/contributors/design-proposals/volume_stats_pvc_ref.md +++ b/contributors/design-proposals/volume_stats_pvc_ref.md @@ -41,7 +41,7 @@ type VolumeStats struct { - The limitation of this approach is that we're limited to reporting only what is available in the pod spec (Pod namespace and PVC claimname) ### Option 2 -- Modify the ```volumemanager::GetMountedVolumesForPod()``` (or add a new function) to return additional volume information from the ASOW/DSOW caches +- Modify the ```volumemanager::GetMountedVolumesForPod()``` (or add a new function) to return additional volume information from the actual/desired state-of-world caches - Use this to populate PVCRef in VolumeStats - This allows us to get information not available in the Pod spec such as the PV name/UID which can be used to label metrics - enables exposing/querying volume metrics by PV name -- cgit v1.2.3 From 94be197b504960a802a9b54ad9e4b582bc0de5ce Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Wed, 30 Aug 2017 10:34:02 -0700 Subject: Add PV use case --- contributors/design-proposals/volume_stats_pvc_ref.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contributors/design-proposals/volume_stats_pvc_ref.md b/contributors/design-proposals/volume_stats_pvc_ref.md index 0720a19f..1b2f599b 100644 --- a/contributors/design-proposals/volume_stats_pvc_ref.md +++ b/contributors/design-proposals/volume_stats_pvc_ref.md @@ -48,6 +48,9 @@ type VolumeStats struct { - It's unclear whether this is a use case we need to/should support: * Volume metrics are only refreshed for mounted volumes which implies a bound/available PVC * We expect most user-storage interactions to be via the PVC +- Admins monitoring PVs (and not PVC's) so that they know when their users are running out of space or are over-provisioning would be a use case supporting adding PV information to + metrics + -- cgit v1.2.3