Merge pull request #79851 from jparklab/master

Fix nil pointer dereference error in volume_stat_calculator

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)

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{

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,