From 528521cfae4db914b930f4fcfb37b6d61256e71a Mon Sep 17 00:00:00 2001
From: Ji-Young Park <jiyoung.park.dev@gmail.com>
Date: Sat, 6 Jul 2019 10:32:40 -0400
Subject: [PATCH] Return MetricsError with ErrCodeNotSupported code

GetMetrics function expects MetricsError error
with ErrCodeNotSupported code when driver for the volume
does not support metrics
Updated csi_metrics to return error when underlying csi
driver does not have GET_VOLUME_STATS capability
---
 pkg/volume/csi/csi_metrics.go      |  4 ++-
 pkg/volume/csi/csi_metrics_test.go | 44 ++++++++++++++++++++++++++++++
 pkg/volume/metrics_errors.go       | 11 +++++++-
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/pkg/volume/csi/csi_metrics.go b/pkg/volume/csi/csi_metrics.go
index bd7ed55305d..2e0c984cad4 100644
--- a/pkg/volume/csi/csi_metrics.go
+++ b/pkg/volume/csi/csi_metrics.go
@@ -61,9 +61,11 @@ func (mc *metricsCsi) GetMetrics() (*volume.Metrics, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	// if plugin doesnot support volume status, return.
 	if !volumeStatsSet {
-		return nil, nil
+		return nil, volume.NewNotSupportedErrorWithDriverName(
+			string(mc.csiClientGetter.driverName))
 	}
 	// Get Volumestatus
 	metrics, err := csiClient.NodeGetVolumeStats(ctx, mc.volumeID, mc.targetPath)
diff --git a/pkg/volume/csi/csi_metrics_test.go b/pkg/volume/csi/csi_metrics_test.go
index bcdb352e90f..9806587bb13 100644
--- a/pkg/volume/csi/csi_metrics_test.go
+++ b/pkg/volume/csi/csi_metrics_test.go
@@ -22,6 +22,7 @@ import (
 
 	csipbv1 "github.com/container-storage-interface/spec/lib/go/csi"
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/csi/fake"
 )
 
@@ -93,6 +94,49 @@ func TestGetMetrics(t *testing.T) {
 	}
 }
 
+// test GetMetrics with a volume that does not support stats
+func TestGetMetricsDriverNotSupportStats(t *testing.T) {
+	tests := []struct {
+		name          string
+		volumeID      string
+		targetPath    string
+		expectSuccess bool
+	}{
+		{
+			name:          "volume created by simple driver",
+			expectSuccess: true,
+			volumeID:      "foobar",
+			targetPath:    "/mnt/foo",
+		},
+	}
+
+	for _, tc := range tests {
+		metricsGetter := &metricsCsi{volumeID: tc.volumeID, targetPath: tc.targetPath}
+		metricsGetter.csiClient = &csiDriverClient{
+			driverName: "com.simple.SimpleDriver",
+			nodeV1ClientCreator: func(addr csiAddr) (csipbv1.NodeClient, io.Closer, error) {
+				nodeClient := fake.NewNodeClientWithVolumeStats(false /* VolumeStatsCapable */)
+				fakeCloser := fake.NewCloser(t)
+				nodeClient.SetNodeVolumeStatsResp(getRawVolumeInfo())
+				return nodeClient, fakeCloser, nil
+			},
+		}
+		metrics, err := metricsGetter.GetMetrics()
+		if err == nil {
+			t.Fatalf("for %s: expected error, but got nil error", tc.name)
+		}
+
+		if !volume.IsNotSupported(err) {
+			t.Fatalf("for %s, expected not supported error but got: %v", tc.name, err)
+		}
+
+		if metrics != nil {
+			t.Fatalf("for %s, expected nil metrics, but got: %v", tc.name, metrics)
+		}
+	}
+
+}
+
 func getRawVolumeInfo() *csipbv1.NodeGetVolumeStatsResponse {
 	return &csipbv1.NodeGetVolumeStatsResponse{
 		Usage: []*csipbv1.VolumeUsage{
diff --git a/pkg/volume/metrics_errors.go b/pkg/volume/metrics_errors.go
index 50e7c2a21b9..80ef96d5e42 100644
--- a/pkg/volume/metrics_errors.go
+++ b/pkg/volume/metrics_errors.go
@@ -35,7 +35,16 @@ func NewNotSupportedError() *MetricsError {
 	}
 }
 
-// NewNoPathDefined creates a new MetricsError with code NoPathDefined.
+// NewNotSupportedErrorWithDriverName creates a new MetricsError with code NotSupported.
+// driver name is added to the error message.
+func NewNotSupportedErrorWithDriverName(name string) *MetricsError {
+	return &MetricsError{
+		Code: ErrCodeNotSupported,
+		Msg:  fmt.Sprintf("metrics are not supported for %s volumes", name),
+	}
+}
+
+// NewNoPathDefinedError creates a new MetricsError with code NoPathDefined.
 func NewNoPathDefinedError() *MetricsError {
 	return &MetricsError{
 		Code: ErrCodeNoPathDefined,