Skip to content

Commit 3180a98

Browse files
amacaskillamacaskill
committed
add enable_storage_pools label value to OperationErrorMetric
1 parent 63146c9 commit 3180a98

File tree

3 files changed

+72
-30
lines changed

3 files changed

+72
-30
lines changed

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

+29-20
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()
@@ -457,8 +458,9 @@ func (gceCS *GCEControllerServer) DeleteVolume(ctx context.Context, req *csi.Del
457458
var err error
458459
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
459460
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
461+
enableStoragePools := metrics.DefaultEnableStoragePools
460462
defer func() {
461-
gceCS.Metrics.RecordOperationErrorMetrics("DeleteVolume", err, diskTypeForMetric, enableConfidentialCompute)
463+
gceCS.Metrics.RecordOperationErrorMetrics("DeleteVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
462464
}()
463465
// Validate arguments
464466
volumeID := req.GetVolumeId()
@@ -488,7 +490,7 @@ func (gceCS *GCEControllerServer) DeleteVolume(ctx context.Context, req *csi.Del
488490
}
489491
defer gceCS.volumeLocks.Release(volumeID)
490492
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
491-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
493+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
492494
err = gceCS.CloudProvider.DeleteDisk(ctx, project, volKey)
493495
if err != nil {
494496
return nil, common.LoggedError("Failed to delete disk: ", err)
@@ -502,8 +504,9 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r
502504
var err error
503505
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
504506
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
507+
enableStoragePools := metrics.DefaultEnableStoragePools
505508
defer func() {
506-
gceCS.Metrics.RecordOperationErrorMetrics("ControllerPublishVolume", err, diskTypeForMetric, enableConfidentialCompute)
509+
gceCS.Metrics.RecordOperationErrorMetrics("ControllerPublishVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
507510
}()
508511
// Only valid requests will be accepted
509512
_, _, _, err = gceCS.validateControllerPublishVolumeRequest(ctx, req)
@@ -517,7 +520,7 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r
517520
}
518521

519522
resp, err, disk := gceCS.executeControllerPublishVolume(ctx, req)
520-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
523+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
521524
if err != nil {
522525
klog.Infof("For node %s adding backoff due to error for volume %s: %v", req.NodeId, req.VolumeId, err)
523526
gceCS.errorBackoff.next(backoffId, common.CodeForError(err))
@@ -603,9 +606,9 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
603606
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
604607
if err != nil {
605608
if gce.IsGCENotFoundError(err) {
606-
return nil, status.Errorf(codes.NotFound, "Could not find disk %v: %v", volKey.String(), err.Error()), disk
609+
return nil, status.Errorf(codes.NotFound, "ControllerPublishVolume could not find disk %v: %v", volKey.String(), err.Error()), disk
607610
}
608-
return nil, common.LoggedError("Failed to getDisk: ", err), disk
611+
return nil, common.LoggedError("ControllerPublishVolume, failed to getDisk: ", err), disk
609612
}
610613
instanceZone, instanceName, err := common.NodeIDToZoneAndName(nodeID)
611614
if err != nil {
@@ -667,8 +670,9 @@ func (gceCS *GCEControllerServer) ControllerUnpublishVolume(ctx context.Context,
667670
var err error
668671
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
669672
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
673+
enableStoragePools := metrics.DefaultEnableStoragePools
670674
defer func() {
671-
gceCS.Metrics.RecordOperationErrorMetrics("ControllerUnpublishVolume", err, diskTypeForMetric, enableConfidentialCompute)
675+
gceCS.Metrics.RecordOperationErrorMetrics("ControllerUnpublishVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
672676
}()
673677
_, _, err = gceCS.validateControllerUnpublishVolumeRequest(ctx, req)
674678
if err != nil {
@@ -681,7 +685,7 @@ func (gceCS *GCEControllerServer) ControllerUnpublishVolume(ctx context.Context,
681685
return nil, status.Errorf(gceCS.errorBackoff.code(backoffId), "ControllerUnpublish not permitted on node %q due to backoff condition", req.NodeId)
682686
}
683687
resp, err, disk := gceCS.executeControllerUnpublishVolume(ctx, req)
684-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
688+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
685689
if err != nil {
686690
klog.Infof("For node %s adding backoff due to error for volume %s: %v", req.NodeId, req.VolumeId, err)
687691
gceCS.errorBackoff.next(backoffId, common.CodeForError(err))
@@ -776,8 +780,9 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
776780
var err error
777781
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
778782
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
783+
enableStoragePools := metrics.DefaultEnableStoragePools
779784
defer func() {
780-
gceCS.Metrics.RecordOperationErrorMetrics("ValidateVolumeCapabilities", err, diskTypeForMetric, enableConfidentialCompute)
785+
gceCS.Metrics.RecordOperationErrorMetrics("ValidateVolumeCapabilities", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
781786
}()
782787
if req.GetVolumeCapabilities() == nil || len(req.GetVolumeCapabilities()) == 0 {
783788
return nil, status.Error(codes.InvalidArgument, "Volume Capabilities must be provided")
@@ -804,12 +809,12 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
804809
defer gceCS.volumeLocks.Release(volumeID)
805810

806811
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
807-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
812+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
808813
if err != nil {
809814
if gce.IsGCENotFoundError(err) {
810-
return nil, status.Errorf(codes.NotFound, "Could not find disk %v: %v", volKey.Name, err.Error())
815+
return nil, status.Errorf(codes.NotFound, "ValidateVolumeCapabilities could not find disk %v: %v", volKey.Name, err.Error())
811816
}
812-
return nil, common.LoggedError("Failed to getDisk: ", err)
817+
return nil, common.LoggedError("ValidateVolumeCapabilities, failed to getDisk: ", err)
813818
}
814819

815820
// Check volume capabilities supported by PD. These are the same for any PD
@@ -939,8 +944,9 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
939944
var err error
940945
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
941946
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
947+
enableStoragePools := metrics.DefaultEnableStoragePools
942948
defer func() {
943-
gceCS.Metrics.RecordOperationErrorMetrics("CreateSnapshot", err, diskTypeForMetric, enableConfidentialCompute)
949+
gceCS.Metrics.RecordOperationErrorMetrics("CreateSnapshot", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
944950
}()
945951
// Validate arguments
946952
volumeID := req.GetSourceVolumeId()
@@ -962,7 +968,7 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
962968

963969
// Check if volume exists
964970
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
965-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(disk)
971+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(disk)
966972
if err != nil {
967973
if gce.IsGCENotFoundError(err) {
968974
return nil, status.Errorf(codes.NotFound, "CreateSnapshot could not find disk %v: %v", volKey.String(), err.Error())
@@ -1191,8 +1197,9 @@ func (gceCS *GCEControllerServer) DeleteSnapshot(ctx context.Context, req *csi.D
11911197
var err error
11921198
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
11931199
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
1200+
enableStoragePools := metrics.DefaultEnableStoragePools
11941201
defer func() {
1195-
gceCS.Metrics.RecordOperationErrorMetrics("DeleteSnapshot", err, diskTypeForMetric, enableConfidentialCompute)
1202+
gceCS.Metrics.RecordOperationErrorMetrics("DeleteSnapshot", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
11961203
}()
11971204
// Validate arguments
11981205
snapshotID := req.GetSnapshotId()
@@ -1281,8 +1288,9 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
12811288
var err error
12821289
diskTypeForMetric := metrics.DefaultDiskTypeForMetric
12831290
enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
1291+
enableStoragePools := metrics.DefaultEnableStoragePools
12841292
defer func() {
1285-
gceCS.Metrics.RecordOperationErrorMetrics("ControllerExpandVolume", err, diskTypeForMetric, enableConfidentialCompute)
1293+
gceCS.Metrics.RecordOperationErrorMetrics("ControllerExpandVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
12861294
}()
12871295
volumeID := req.GetVolumeId()
12881296
if len(volumeID) == 0 {
@@ -1306,8 +1314,9 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
13061314
return nil, common.LoggedError("ControllerExpandVolume error repairing underspecified volume key: ", err)
13071315
}
13081316
sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1309-
diskTypeForMetric, enableConfidentialCompute = metrics.GetMetricParameters(sourceDisk)
1317+
diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(sourceDisk)
13101318
resizedGb, err := gceCS.CloudProvider.ResizeDisk(ctx, project, volKey, reqBytes)
1319+
13111320
if err != nil {
13121321
return nil, common.LoggedError("ControllerExpandVolume failed to resize disk: ", err)
13131322
}
@@ -1627,13 +1636,13 @@ func getZonesFromTopology(topList []*csi.Topology) ([]string, error) {
16271636
zones := []string{}
16281637
for _, top := range topList {
16291638
if top.GetSegments() == nil {
1630-
return nil, fmt.Errorf("topologies specified but no segments")
1639+
return nil, fmt.Errorf("preferred topologies specified but no segments")
16311640
}
16321641

16331642
// GCE PD cloud provider Create has no restrictions so just create in top preferred zone
16341643
zone, err := getZoneFromSegment(top.GetSegments())
16351644
if err != nil {
1636-
return nil, fmt.Errorf("could not get zone from topology: %w", err)
1645+
return nil, fmt.Errorf("could not get zone from preferred topology: %w", err)
16371646
}
16381647
zones = append(zones, zone)
16391648
}

‎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

+34-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,69 @@ 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",
6894
},
6995
}
7096

7197
for _, tc := range testCases {
7298
t.Logf("Running test: %v", tc.name)
73-
diskType, confidentialCompute := GetMetricParameters(tc.disk)
99+
diskType, confidentialCompute, enableStoragePools := GetMetricParameters(tc.disk)
74100
if confidentialCompute != tc.expectedEnableConfidentialCompute {
75-
t.Fatalf("Got confidentialCompute value %v expected %v", confidentialCompute, tc.expectedEnableConfidentialCompute)
101+
t.Fatalf("Got confidentialCompute value %q expected %q", confidentialCompute, tc.expectedEnableConfidentialCompute)
76102
}
77103
if diskType != tc.expectedDiskType {
78-
t.Fatalf("Got confidentialCompute value %v expected %v", diskType, tc.expectedDiskType)
104+
t.Fatalf("Got diskType value %q expected %q", diskType, tc.expectedDiskType)
105+
}
106+
if enableStoragePools != tc.expectedEnableStoragePools {
107+
t.Fatalf("Got enableStoragePools value %q expected %q", enableStoragePools, tc.expectedEnableStoragePools)
79108
}
80109
}
81110
}

0 commit comments

Comments
 (0)