From b997e0e4d6ccbead435a47d6ac75b0db3d17252f Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Thu, 20 May 2021 15:18:26 +0200 Subject: [PATCH] Add SupportsMetrics() for Block-mode volumes Volumes that are provisioned with `VolumeMode: Block` often have a MetrucsProvider interface declared in their type. However, the MetricsProvider should implement a GetMetrics() function. In the cases where the storage drivers do not implement GetMetrics(), a panic can occur. Usual type-assertions are not sufficient in this case. All assertions assume the interface is present. There is no straight forward way to verify that a valid GetMetrics() function is provided. By adding SupportsMetrics(), storage driver implementations require careful reviewing for metrics support. --- pkg/kubelet/kubelet_volumes_test.go | 4 ++++ pkg/kubelet/server/stats/volume_stat_calculator.go | 7 ++++++- pkg/kubelet/server/stats/volume_stat_calculator_test.go | 2 ++ pkg/volume/awsebs/aws_ebs_block.go | 6 ++++++ pkg/volume/azuredd/azure_dd_block.go | 7 +++++++ pkg/volume/cinder/cinder_block.go | 7 +++++++ pkg/volume/csi/csi_block.go | 6 ++++++ pkg/volume/gcepd/gce_pd_block.go | 6 ++++++ pkg/volume/iscsi/iscsi.go | 7 +++++++ pkg/volume/local/local.go | 7 +++++++ pkg/volume/metrics_nil.go | 5 +++++ pkg/volume/metrics_nil_test.go | 8 ++++++++ pkg/volume/rbd/rbd.go | 6 ++++++ pkg/volume/volume.go | 4 ++++ pkg/volume/vsphere_volume/vsphere_volume_block.go | 7 +++++++ 15 files changed, 88 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index 9c8f61723f0..0f4117c3a58 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -567,6 +567,10 @@ func (f *stubBlockVolume) UnmapPodDevice() error { return nil } +func (f *stubBlockVolume) SupportsMetrics() bool { + return false +} + func (f *stubBlockVolume) GetMetrics() (*volume.Metrics, error) { return nil, nil } diff --git a/pkg/kubelet/server/stats/volume_stat_calculator.go b/pkg/kubelet/server/stats/volume_stat_calculator.go index 39396d2f5d9..89cf5fd4c2a 100644 --- a/pkg/kubelet/server/stats/volume_stat_calculator.go +++ b/pkg/kubelet/server/stats/volume_stat_calculator.go @@ -112,7 +112,12 @@ func (s *volumeStatCalculator) calcAndStoreStats() { for name, v := range blockVolumes { // Only add the blockVolume if it implements the MetricsProvider interface if _, ok := v.(volume.MetricsProvider); ok { - metricVolumes[name] = v + // Some drivers inherit the MetricsProvider interface from Filesystem + // mode volumes, but do not implement it for Block mode. Checking + // SupportsMetrics() will prevent panics in that case. + if v.SupportsMetrics() { + metricVolumes[name] = v + } } } } diff --git a/pkg/kubelet/server/stats/volume_stat_calculator_test.go b/pkg/kubelet/server/stats/volume_stat_calculator_test.go index 8be5548e666..db3620289f3 100644 --- a/pkg/kubelet/server/stats/volume_stat_calculator_test.go +++ b/pkg/kubelet/server/stats/volume_stat_calculator_test.go @@ -246,6 +246,8 @@ func (v *fakeBlockVolume) GetGlobalMapPath(*volume.Spec) (string, error) { retur func (v *fakeBlockVolume) GetPodDeviceMapPath() (string, string) { return "", "" } +func (v *fakeBlockVolume) SupportsMetrics() bool { return true } + func (v *fakeBlockVolume) GetMetrics() (*volume.Metrics, error) { return expectedBlockMetrics(), nil } diff --git a/pkg/volume/awsebs/aws_ebs_block.go b/pkg/volume/awsebs/aws_ebs_block.go index e10a1fba62a..34c4e286972 100644 --- a/pkg/volume/awsebs/aws_ebs_block.go +++ b/pkg/volume/awsebs/aws_ebs_block.go @@ -165,3 +165,9 @@ func (ebs *awsElasticBlockStore) GetPodDeviceMapPath() (string, string) { name := awsElasticBlockStorePluginName return ebs.plugin.host.GetPodVolumeDeviceDir(ebs.podUID, utilstrings.EscapeQualifiedName(name)), ebs.volName } + +// SupportsMetrics returns true for awsElasticBlockStore as it initializes the +// MetricsProvider. +func (ebs *awsElasticBlockStore) SupportsMetrics() bool { + return true +} diff --git a/pkg/volume/azuredd/azure_dd_block.go b/pkg/volume/azuredd/azure_dd_block.go index dc0652ca99b..1217e4a0196 100644 --- a/pkg/volume/azuredd/azure_dd_block.go +++ b/pkg/volume/azuredd/azure_dd_block.go @@ -129,6 +129,7 @@ func (plugin *azureDataDiskPlugin) newUnmapperInternal(volName string, podUID ty type azureDataDiskUnmapper struct { *dataDisk + volume.MetricsNil } var _ volume.BlockVolumeUnmapper = &azureDataDiskUnmapper{} @@ -157,3 +158,9 @@ func (disk *dataDisk) GetPodDeviceMapPath() (string, string) { name := azureDataDiskPluginName return disk.plugin.host.GetPodVolumeDeviceDir(disk.podUID, utilstrings.EscapeQualifiedName(name)), disk.volumeName } + +// SupportsMetrics returns true for azureDataDiskMapper as it initializes the +// MetricsProvider. +func (addm *azureDataDiskMapper) SupportsMetrics() bool { + return true +} diff --git a/pkg/volume/cinder/cinder_block.go b/pkg/volume/cinder/cinder_block.go index 94295254d75..ae3ab169b8e 100644 --- a/pkg/volume/cinder/cinder_block.go +++ b/pkg/volume/cinder/cinder_block.go @@ -140,6 +140,7 @@ func (plugin *cinderPlugin) newUnmapperInternal(volName string, podUID types.UID type cinderPluginUnmapper struct { *cinderVolume + volume.MetricsNil } var _ volume.BlockVolumeUnmapper = &cinderPluginUnmapper{} @@ -168,3 +169,9 @@ func (cd *cinderVolume) GetPodDeviceMapPath() (string, string) { name := cinderVolumePluginName return cd.plugin.host.GetPodVolumeDeviceDir(cd.podUID, utilstrings.EscapeQualifiedName(name)), cd.volName } + +// SupportsMetrics returns true for cinderVolumeMapper as it initializes the +// MetricsProvider. +func (cvm *cinderVolumeMapper) SupportsMetrics() bool { + return true +} diff --git a/pkg/volume/csi/csi_block.go b/pkg/volume/csi/csi_block.go index 687f2bc4385..7d768768ccc 100644 --- a/pkg/volume/csi/csi_block.go +++ b/pkg/volume/csi/csi_block.go @@ -115,6 +115,12 @@ func (m *csiBlockMapper) GetStagingPath() string { return filepath.Join(m.plugin.host.GetVolumeDevicePluginDir(CSIPluginName), "staging", m.specName) } +// SupportsMetrics returns true for csiBlockMapper as it initializes the +// MetricsProvider. +func (m *csiBlockMapper) SupportsMetrics() bool { + return true +} + // getPublishDir returns path to a directory, where the volume is published to each pod. // Example: plugins/kubernetes.io/csi/volumeDevices/publish/{specName} func (m *csiBlockMapper) getPublishDir() string { diff --git a/pkg/volume/gcepd/gce_pd_block.go b/pkg/volume/gcepd/gce_pd_block.go index 1e24b04bd35..59c9c524653 100644 --- a/pkg/volume/gcepd/gce_pd_block.go +++ b/pkg/volume/gcepd/gce_pd_block.go @@ -174,3 +174,9 @@ func (pd *gcePersistentDisk) GetPodDeviceMapPath() (string, string) { name := gcePersistentDiskPluginName return pd.plugin.host.GetPodVolumeDeviceDir(pd.podUID, utilstrings.EscapeQualifiedName(name)), pd.volName } + +// SupportsMetrics returns true for gcePersistentDisk as it initializes the +// MetricsProvider. +func (pd *gcePersistentDisk) SupportsMetrics() bool { + return true +} diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 169e277de54..5cbf3069369 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -393,6 +393,13 @@ type iscsiDiskUnmapper struct { *iscsiDisk exec utilexec.Interface deviceUtil ioutil.DeviceUtil + volume.MetricsNil +} + +// SupportsMetrics returns true for SupportsMetrics as it initializes the +// MetricsProvider. +func (idm *iscsiDiskMapper) SupportsMetrics() bool { + return true } var _ volume.BlockVolumeUnmapper = &iscsiDiskUnmapper{} diff --git a/pkg/volume/local/local.go b/pkg/volume/local/local.go index 696d78c4237..b2cfa9b28f6 100644 --- a/pkg/volume/local/local.go +++ b/pkg/volume/local/local.go @@ -633,9 +633,16 @@ func (m *localVolumeMapper) GetStagingPath() string { return "" } +// SupportsMetrics returns true for SupportsMetrics as it initializes the +// MetricsProvider. +func (m *localVolumeMapper) SupportsMetrics() bool { + return true +} + // localVolumeUnmapper implements the BlockVolumeUnmapper interface for local volumes. type localVolumeUnmapper struct { *localVolume + volume.MetricsNil } var _ volume.BlockVolumeUnmapper = &localVolumeUnmapper{} diff --git a/pkg/volume/metrics_nil.go b/pkg/volume/metrics_nil.go index 5438dc3de35..11b74e07978 100644 --- a/pkg/volume/metrics_nil.go +++ b/pkg/volume/metrics_nil.go @@ -23,6 +23,11 @@ var _ MetricsProvider = &MetricsNil{} // metrics. type MetricsNil struct{} +// SupportsMetrics returns false for the MetricsNil type. +func (*MetricsNil) SupportsMetrics() bool { + return false +} + // GetMetrics returns an empty Metrics and an error. // See MetricsProvider.GetMetrics func (*MetricsNil) GetMetrics() (*Metrics, error) { diff --git a/pkg/volume/metrics_nil_test.go b/pkg/volume/metrics_nil_test.go index e6a25d1ff69..93ad9b4b693 100644 --- a/pkg/volume/metrics_nil_test.go +++ b/pkg/volume/metrics_nil_test.go @@ -20,6 +20,14 @@ import ( "testing" ) +func TestMetricsNilSupportsMetrics(t *testing.T) { + metrics := &MetricsNil{} + supported := metrics.SupportsMetrics() + if supported { + t.Error("Expected no support for metrics") + } +} + func TestMetricsNilGetCapacity(t *testing.T) { metrics := &MetricsNil{} actual, err := metrics.GetMetrics() diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index adf4ec0854f..cf9e1049f6c 100644 --- a/pkg/volume/rbd/rbd.go +++ b/pkg/volume/rbd/rbd.go @@ -938,6 +938,12 @@ func (rbd *rbd) rbdPodDeviceMapPath() (string, string) { return rbd.plugin.host.GetPodVolumeDeviceDir(rbd.podUID, utilstrings.EscapeQualifiedName(name)), rbd.volName } +// SupportsMetrics returns true for rbdDiskMapper as it initializes the +// MetricsProvider. +func (rdm *rbdDiskMapper) SupportsMetrics() bool { + return true +} + type rbdDiskUnmapper struct { *rbdDiskMapper } diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 642f72645f8..63246a85a5d 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -49,6 +49,10 @@ type BlockVolume interface { // ex. pods/{podUid}/{DefaultKubeletVolumeDevicesDirName}/{escapeQualifiedPluginName}/, {volumeName} GetPodDeviceMapPath() (string, string) + // SupportsMetrics should return true if the MetricsProvider is + // initialized + SupportsMetrics() bool + // MetricsProvider embeds methods for exposing metrics (e.g. // used, available space). MetricsProvider diff --git a/pkg/volume/vsphere_volume/vsphere_volume_block.go b/pkg/volume/vsphere_volume/vsphere_volume_block.go index a3414f5be83..721c252ebc9 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_block.go +++ b/pkg/volume/vsphere_volume/vsphere_volume_block.go @@ -144,6 +144,7 @@ var _ volume.BlockVolumeUnmapper = &vsphereBlockVolumeUnmapper{} type vsphereBlockVolumeUnmapper struct { *vsphereVolume + volume.MetricsNil } // GetGlobalMapPath returns global map path and error @@ -159,3 +160,9 @@ func (v *vsphereVolume) GetGlobalMapPath(spec *volume.Spec) (string, error) { func (v *vsphereVolume) GetPodDeviceMapPath() (string, string) { return v.plugin.host.GetPodVolumeDeviceDir(v.podUID, utilstrings.EscapeQualifiedName(vsphereVolumePluginName)), v.volName } + +// SupportsMetrics returns true for vsphereBlockVolumeMapper as it initializes the +// MetricsProvider. +func (vbvm *vsphereBlockVolumeMapper) SupportsMetrics() bool { + return true +}