Skip to content

Commit e93478d

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1534 from amacaskill/add-storage-pool-metric
add enable_storage_pools label to OperationErrorMetric
2 parents 63146c9 + 19650ca commit e93478d

File tree

3 files changed

+75
-24
lines changed

3 files changed

+75
-24
lines changed

‎pkg/gce-pd-csi-driver/controller.go

+25-14
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,9 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
215215
var err error
216216
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
217217
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
218+
enableStoragePools := metrics.DefaultEnableStoragePools
218219
defer func() {
219-
gceCS.Metrics.RecordOperationErrorMetrics("CreateVolume", err, diskTypeForMetric, enableConfidentialCompute)
220+
gceCS.Metrics.RecordOperationErrorMetrics("CreateVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
220221
}()
221222
// Validate arguments
222223
volumeCapabilities := req.GetVolumeCapabilities()
@@ -244,6 +245,8 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
244245
params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name, gceCS.Driver.extraVolumeLabels)
245246
diskTypeForMetric = params.DiskType
246247
enableConfidentialCompute = strconv.FormatBool(params.EnableConfidentialCompute)
248+
hasStoragePools := len(params.StoragePools) > 0
249+
enableStoragePools = strconv.FormatBool(hasStoragePools)
247250
if err != nil {
248251
return nil, status.Errorf(codes.InvalidArgument, "failed to extract parameters: %v", err.Error())
249252
}
@@ -457,8 +460,9 @@ func (gceCS *GCEControllerServer) DeleteVolume(ctx context.Context, req *csi.Del
457460
var err error
458461
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
459462
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
463+
enableStoragePools := metrics.DefaultEnableStoragePools
460464
defer func() {
461-
gceCS.Metrics.RecordOperationErrorMetrics("DeleteVolume", err, diskTypeForMetric, enableConfidentialCompute)
465+
gceCS.Metrics.RecordOperationErrorMetrics("DeleteVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
462466
}()
463467
// Validate arguments
464468
volumeID := req.GetVolumeId()
@@ -488,7 +492,7 @@ func (gceCS *GCEControllerServer) DeleteVolume(ctx context.Context, req *csi.Del
488492
}
489493
defer gceCS.volumeLocks.Release(volumeID)
490494
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
491-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
495+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
492496
err = gceCS.CloudProvider.DeleteDisk(ctx, project, volKey)
493497
if err != nil {
494498
return nil, common.LoggedError("Failed to delete disk: ", err)
@@ -502,8 +506,9 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r
502506
var err error
503507
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
504508
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
509+
enableStoragePools := metrics.DefaultEnableStoragePools
505510
defer func() {
506-
gceCS.Metrics.RecordOperationErrorMetrics("ControllerPublishVolume", err, diskTypeForMetric, enableConfidentialCompute)
511+
gceCS.Metrics.RecordOperationErrorMetrics("ControllerPublishVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
507512
}()
508513
// Only valid requests will be accepted
509514
_, _, _, err = gceCS.validateControllerPublishVolumeRequest(ctx, req)
@@ -517,7 +522,7 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r
517522
}
518523

519524
resp, err, disk := gceCS.executeControllerPublishVolume(ctx, req)
520-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
525+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
521526
if err != nil {
522527
klog.Infof("For node %s adding backoff due to error for volume %s: %v", req.NodeId, req.VolumeId, err)
523528
gceCS.errorBackoff.next(backoffId, common.CodeForError(err))
@@ -667,8 +672,9 @@ func (gceCS *GCEControllerServer) ControllerUnpublishVolume(ctx context.Context,
667672
var err error
668673
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
669674
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
675+
enableStoragePools := metrics.DefaultEnableStoragePools
670676
defer func() {
671-
gceCS.Metrics.RecordOperationErrorMetrics("ControllerUnpublishVolume", err, diskTypeForMetric, enableConfidentialCompute)
677+
gceCS.Metrics.RecordOperationErrorMetrics("ControllerUnpublishVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
672678
}()
673679
_, _, err = gceCS.validateControllerUnpublishVolumeRequest(ctx, req)
674680
if err != nil {
@@ -681,7 +687,7 @@ func (gceCS *GCEControllerServer) ControllerUnpublishVolume(ctx context.Context,
681687
return nil, status.Errorf(gceCS.errorBackoff.code(backoffId), "ControllerUnpublish not permitted on node %q due to backoff condition", req.NodeId)
682688
}
683689
resp, err, disk := gceCS.executeControllerUnpublishVolume(ctx, req)
684-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
690+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
685691
if err != nil {
686692
klog.Infof("For node %s adding backoff due to error for volume %s: %v", req.NodeId, req.VolumeId, err)
687693
gceCS.errorBackoff.next(backoffId, common.CodeForError(err))
@@ -776,8 +782,9 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
776782
var err error
777783
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
778784
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
785+
enableStoragePools := metrics.DefaultEnableStoragePools
779786
defer func() {
780-
gceCS.Metrics.RecordOperationErrorMetrics("ValidateVolumeCapabilities", err, diskTypeForMetric, enableConfidentialCompute)
787+
gceCS.Metrics.RecordOperationErrorMetrics("ValidateVolumeCapabilities", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
781788
}()
782789
if req.GetVolumeCapabilities() == nil || len(req.GetVolumeCapabilities()) == 0 {
783790
return nil, status.Error(codes.InvalidArgument, "Volume Capabilities must be provided")
@@ -804,7 +811,7 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
804811
defer gceCS.volumeLocks.Release(volumeID)
805812

806813
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
807-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
814+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
808815
if err != nil {
809816
if gce.IsGCENotFoundError(err) {
810817
return nil, status.Errorf(codes.NotFound, "Could not find disk %v: %v", volKey.Name, err.Error())
@@ -939,8 +946,9 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
939946
var err error
940947
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
941948
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
949+
enableStoragePools := metrics.DefaultEnableStoragePools
942950
defer func() {
943-
gceCS.Metrics.RecordOperationErrorMetrics("CreateSnapshot", err, diskTypeForMetric, enableConfidentialCompute)
951+
gceCS.Metrics.RecordOperationErrorMetrics("CreateSnapshot", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
944952
}()
945953
// Validate arguments
946954
volumeID := req.GetSourceVolumeId()
@@ -962,7 +970,7 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
962970

963971
// Check if volume exists
964972
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
965-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
973+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
966974
if err != nil {
967975
if gce.IsGCENotFoundError(err) {
968976
return nil, status.Errorf(codes.NotFound, "CreateSnapshot could not find disk %v: %v", volKey.String(), err.Error())
@@ -1191,8 +1199,9 @@ func (gceCS *GCEControllerServer) DeleteSnapshot(ctx context.Context, req *csi.D
11911199
var err error
11921200
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
11931201
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
1202+
enableStoragePools := metrics.DefaultEnableStoragePools
11941203
defer func() {
1195-
gceCS.Metrics.RecordOperationErrorMetrics("DeleteSnapshot", err, diskTypeForMetric, enableConfidentialCompute)
1204+
gceCS.Metrics.RecordOperationErrorMetrics("DeleteSnapshot", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
11961205
}()
11971206
// Validate arguments
11981207
snapshotID := req.GetSnapshotId()
@@ -1281,8 +1290,9 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
12811290
var err error
12821291
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
12831292
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
1293+
enableStoragePools := metrics.DefaultEnableStoragePools
12841294
defer func() {
1285-
gceCS.Metrics.RecordOperationErrorMetrics("ControllerExpandVolume", err, diskTypeForMetric, enableConfidentialCompute)
1295+
gceCS.Metrics.RecordOperationErrorMetrics("ControllerExpandVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
12861296
}()
12871297
volumeID := req.GetVolumeId()
12881298
if len(volumeID) == 0 {
@@ -1306,8 +1316,9 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
13061316
return nil, common.LoggedError("ControllerExpandVolume error repairing underspecified volume key: ", err)
13071317
}
13081318
sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1309-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(sourceDisk)
1319+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(sourceDisk)
13101320
resizedGb, err := gceCS.CloudProvider.ResizeDisk(ctx, project, volKey, reqBytes)
1321+
13111322
if err != nil {
13121323
return nil, common.LoggedError("ControllerExpandVolume failed to resize disk: ", err)
13131324
}

‎pkg/metrics/metrics.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const (
3636
pdcsiDriverName = "pd.csi.storage.gke.io"
3737
DefaultDiskTypeForMetric = "unknownDiskType"
3838
DefaultEnableConfidentialCompute = "unknownConfidentialMode"
39+
DefaultEnableStoragePools = "unknownStoragePools"
3940
)
4041

4142
var (
@@ -52,7 +53,7 @@ var (
5253
Help: "CSI server side error metrics",
5354
StabilityLevel: metrics.ALPHA,
5455
},
55-
[]string{"driver_name", "method_name", "grpc_status_code", "disk_type", "enable_confidential_storage"})
56+
[]string{"driver_name", "method_name", "grpc_status_code", "disk_type", "enable_confidential_storage", "enable_storage_pools"})
5657
)
5758

5859
type MetricsManager struct {
@@ -94,12 +95,13 @@ func (mm *MetricsManager) RecordOperationErrorMetrics(
9495
operationName string,
9596
operationErr error,
9697
diskType string,
97-
enableConfidentialStorage string) {
98+
enableConfidentialStorage string,
99+
enableStoragePools string) {
98100
err := codes.OK.String()
99101
if operationErr != nil {
100102
err = common.CodeForError(operationErr).String()
101103
}
102-
pdcsiOperationErrorsMetric.WithLabelValues(pdcsiDriverName, "/csi.v1.Controller/"+operationName, err, diskType, enableConfidentialStorage).Inc()
104+
pdcsiOperationErrorsMetric.WithLabelValues(pdcsiDriverName, "/csi.v1.Controller/"+operationName, err, diskType, enableConfidentialStorage, enableStoragePools).Inc()
103105
}
104106

105107
func (mm *MetricsManager) EmitGKEComponentVersion() error {
@@ -156,12 +158,14 @@ func IsGKEComponentVersionAvailable() bool {
156158
return true
157159
}
158160

159-
func GetMetricParameters(disk *gce.CloudDisk) (string, string) {
161+
func GetMetricParameters(disk *gce.CloudDisk) (string, string, string) {
160162
diskType := DefaultDiskTypeForMetric
161163
enableConfidentialStorage := DefaultEnableConfidentialCompute
164+
enableStoragePools := DefaultEnableStoragePools
162165
if disk != nil {
163166
diskType = disk.GetPDType()
164167
enableConfidentialStorage = strconv.FormatBool(disk.GetEnableConfidentialCompute())
168+
enableStoragePools = strconv.FormatBool(disk.GetEnableStoragePools())
165169
}
166-
return diskType, enableConfidentialStorage
170+
return diskType, enableConfidentialStorage, enableStoragePools
167171
}

‎pkg/metrics/metrics_test.go

+41-5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package metrics
2020
import (
2121
"testing"
2222

23+
computealpha "google.golang.org/api/compute/v0.alpha"
2324
computebeta "google.golang.org/api/compute/v0.beta"
2425
"google.golang.org/api/compute/v1"
2526
gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/compute"
@@ -41,41 +42,76 @@ func CreateDiskWithConfidentialCompute(betaVersion bool, confidentialCompute boo
4142
})
4243
}
4344

44-
func TestGetEnableConfidentialCompute(t *testing.T) {
45+
func CreateDiskWithStoragePool(storagePool string, diskType string) *gce.CloudDisk {
46+
return gce.CloudDiskFromAlpha(&computealpha.Disk{
47+
StoragePool: storagePool,
48+
Type: diskType,
49+
})
50+
}
51+
52+
func TestGetMetricParameters(t *testing.T) {
4553
testCases := []struct {
4654
name string
4755
disk *gce.CloudDisk
4856
expectedEnableConfidentialCompute string
4957
expectedDiskType string
58+
expectedEnableStoragePools string
5059
}{
5160
{
5261
name: "test betaDisk with enableConfidentialCompute=false",
5362
disk: CreateDiskWithConfidentialCompute(true, false, hyperdiskBalanced),
5463
expectedEnableConfidentialCompute: "false",
5564
expectedDiskType: hyperdiskBalanced,
65+
expectedEnableStoragePools: "false",
5666
},
5767
{
5868
name: "test betaDisk with enableConfidentialCompute=true",
5969
disk: CreateDiskWithConfidentialCompute(true, true, hyperdiskBalanced),
6070
expectedEnableConfidentialCompute: "true",
6171
expectedDiskType: hyperdiskBalanced,
72+
expectedEnableStoragePools: "false",
6273
},
6374
{
64-
name: "test disk withpit enableConfidentialCompute",
75+
name: "test disk without enableConfidentialCompute",
6576
disk: CreateDiskWithConfidentialCompute(false, false, hyperdiskBalanced),
6677
expectedEnableConfidentialCompute: "false",
6778
expectedDiskType: hyperdiskBalanced,
79+
expectedEnableStoragePools: "false",
80+
},
81+
{
82+
name: "test alphaDisk with storage pool projects/my-project/zone/us-central1-a/storagePools/sp1",
83+
disk: CreateDiskWithStoragePool("projects/my-project/zone/us-central1-a/storagePools/sp1", hyperdiskBalanced),
84+
expectedEnableConfidentialCompute: "false",
85+
expectedDiskType: hyperdiskBalanced,
86+
expectedEnableStoragePools: "true",
87+
},
88+
{
89+
name: "test alphaDisk with no storage pool",
90+
disk: CreateDiskWithStoragePool("", hyperdiskBalanced),
91+
expectedEnableConfidentialCompute: "false",
92+
expectedDiskType: hyperdiskBalanced,
93+
expectedEnableStoragePools: "false",
94+
},
95+
{
96+
name: "test nil disk",
97+
disk: nil,
98+
expectedEnableConfidentialCompute: DefaultEnableConfidentialCompute,
99+
expectedDiskType: DefaultDiskTypeForMetric,
100+
expectedEnableStoragePools: DefaultEnableStoragePools,
68101
},
69102
}
70103

71104
for _, tc := range testCases {
72105
t.Logf("Running test: %v", tc.name)
73-
diskType, confidentialCompute := GetMetricParameters(tc.disk)
106+
diskType, confidentialCompute, enableStoragePools := GetMetricParameters(tc.disk)
74107
if confidentialCompute != tc.expectedEnableConfidentialCompute {
75-
t.Fatalf("Got confidentialCompute value %v expected %v", confidentialCompute, tc.expectedEnableConfidentialCompute)
108+
t.Fatalf("Got confidentialCompute value %q expected %q", confidentialCompute, tc.expectedEnableConfidentialCompute)
76109
}
77110
if diskType != tc.expectedDiskType {
78-
t.Fatalf("Got confidentialCompute value %v expected %v", diskType, tc.expectedDiskType)
111+
t.Fatalf("Got diskType value %q expected %q", diskType, tc.expectedDiskType)
112+
}
113+
if enableStoragePools != tc.expectedEnableStoragePools {
114+
t.Fatalf("Got enableStoragePools value %q expected %q", enableStoragePools, tc.expectedEnableStoragePools)
79115
}
80116
}
81117
}

0 commit comments

Comments
 (0)