Skip to content

Commit ee3a409

Browse files
amacaskillamacaskill
committed
add storage pool helper functions to common package, and change DiskParameters to use StoragePool type
1 parent 2c674dd commit ee3a409

File tree

5 files changed

+325
-14
lines changed

5 files changed

+325
-14
lines changed

‎pkg/common/parameters.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ type DiskParameters struct {
9797
ForceAttach bool
9898
// Values: {[]string}
9999
// Default: ""
100-
StoragePools []string
100+
StoragePools []StoragePool
101101
}
102102

103103
// SnapshotParameters contains normalized and defaulted parameters for snapshots
@@ -109,6 +109,13 @@ type SnapshotParameters struct {
109109
Labels map[string]string
110110
}
111111

112+
type StoragePool struct {
113+
Project string
114+
Zone string
115+
Name string
116+
ResourceName string
117+
}
118+
112119
// ExtractAndDefaultParameters will take the relevant parameters from a map and
113120
// put them into a well defined struct making sure to default unspecified fields.
114121
// extraVolumeLabels are added as labels; if there are also labels specified in

‎pkg/common/parameters_test.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package common
1919
import (
2020
"reflect"
2121
"testing"
22+
23+
"github.com/google/go-cmp/cmp"
2224
)
2325

2426
func TestExtractAndDefaultParameters(t *testing.T) {
@@ -199,6 +201,37 @@ func TestExtractAndDefaultParameters(t *testing.T) {
199201
Labels: map[string]string{},
200202
},
201203
},
204+
{
205+
name: "storage pool parameters",
206+
parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1,projects/my-project/zones/us-central1-b/storagePools/storagePool-2"},
207+
labels: map[string]string{},
208+
expectParams: DiskParameters{
209+
DiskType: "hyperdisk-balanced",
210+
ReplicationType: "none",
211+
Tags: map[string]string{},
212+
Labels: map[string]string{},
213+
StoragePools: []StoragePool{
214+
{
215+
Project: "my-project",
216+
Zone: "us-central1-a",
217+
Name: "storagePool-1",
218+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
219+
},
220+
{
221+
Project: "my-project",
222+
Zone: "us-central1-b",
223+
Name: "storagePool-2",
224+
ResourceName: "projects/my-project/zones/us-central1-b/storagePools/storagePool-2",
225+
},
226+
},
227+
},
228+
},
229+
{
230+
name: "invalid storage pool parameters",
231+
parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "zones/us-central1-a/storagePools/storagePool-1"},
232+
labels: map[string]string{},
233+
expectErr: true,
234+
},
202235
}
203236

204237
for _, tc := range tests {
@@ -211,8 +244,8 @@ func TestExtractAndDefaultParameters(t *testing.T) {
211244
return
212245
}
213246

214-
if !reflect.DeepEqual(p, tc.expectParams) {
215-
t.Errorf("ExtractAndDefaultParameters(%+v) = %v; expected params: %v", tc.parameters, p, tc.expectParams)
247+
if diff := cmp.Diff(p, tc.expectParams); diff != "" {
248+
t.Errorf("ExtractAndDefaultParameters(%+v): -want, +got \n%s", tc.parameters, diff)
216249
}
217250
})
218251
}

‎pkg/common/utils.go

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ var (
8080
// zones/zone/machineTypes/machine-type
8181
machineTypeRegex = regexp.MustCompile(machineTypePattern)
8282

83+
storagePoolFieldsRegex = regexp.MustCompile(`^projects/([^/]+)/zones/([^/]+)/storagePools/([^/]+)$`)
84+
8385
// userErrorCodeMap tells how API error types are translated to error codes.
8486
userErrorCodeMap = map[int]codes.Code{
8587
http.StatusForbidden: codes.PermissionDenied,
@@ -393,15 +395,72 @@ func isValidDiskEncryptionKmsKey(DiskEncryptionKmsKey string) bool {
393395
return kmsKeyPattern.MatchString(DiskEncryptionKmsKey)
394396
}
395397

396-
// TODO(amacaskill): Implement this function.
397398
// ParseStoragePools returns an error if none of the given storagePools
398399
// (delimited by a comma) are in the format
399400
// projects/project/zones/zone/storagePools/storagePool.
400-
func ParseStoragePools(storagePools string) ([]string, error) {
401-
return nil, status.Errorf(codes.Unimplemented, "")
401+
func ParseStoragePools(storagePools string) ([]StoragePool, error) {
402+
spSlice := strings.Split(storagePools, ",")
403+
parsedStoragePools := []StoragePool{}
404+
for _, sp := range spSlice {
405+
project, location, spName, err := fieldsFromStoragePoolResourceName(sp)
406+
if err != nil {
407+
return nil, err
408+
}
409+
spObj := StoragePool{Project: project, Zone: location, Name: spName, ResourceName: sp}
410+
parsedStoragePools = append(parsedStoragePools, spObj)
411+
412+
}
413+
return parsedStoragePools, nil
402414
}
403415

404-
// TODO(amacaskill): Implement this function.
405-
func StoragePoolInZone(storagePools []string, zone string) string {
406-
return ""
416+
// fieldsFromResourceName returns the project, zone, and Storage Pool name from the given
417+
// Storage Pool resource name. The resource name must be in the format
418+
// projects/project/zones/zone/storagePools/storagePool.
419+
// All other formats are invalid, and an error will be returned.
420+
func fieldsFromStoragePoolResourceName(resourceName string) (project, location, spName string, err error) {
421+
fieldMatches := storagePoolFieldsRegex.FindStringSubmatch(resourceName)
422+
// Field matches should have 4 strings: [resourceName, project, zone, storagePool]. The first
423+
// match is the entire string.
424+
if len(fieldMatches) != 4 {
425+
err := fmt.Errorf("invalid Storage Pool resource name. Got %s, expected projects/project/zones/zone/storagePools/storagePool", resourceName)
426+
return "", "", "", err
427+
}
428+
project = fieldMatches[1]
429+
location = fieldMatches[2]
430+
spName = fieldMatches[3]
431+
return
432+
}
433+
434+
// StoragePoolZones returns the unique zones of the given storage pool resource names.
435+
// Returns an error if multiple storage pools in 1 zone are found.
436+
func StoragePoolZones(storagePools []StoragePool) ([]string, error) {
437+
zonesSet := sets.String{}
438+
var zones []string
439+
for _, sp := range storagePools {
440+
if zonesSet.Has(sp.Zone) {
441+
return nil, fmt.Errorf("found multiple storage pools in zone %s. Only one storage pool per zone is allowed", sp.Zone)
442+
}
443+
zonesSet.Insert(sp.Zone)
444+
zones = append(zones, sp.Zone)
445+
}
446+
return zones, nil
447+
}
448+
449+
func StoragePoolInZone(storagePools []StoragePool, zone string) *StoragePool {
450+
for _, pool := range storagePools {
451+
if zone == pool.Zone {
452+
return &pool
453+
}
454+
}
455+
return nil
456+
}
457+
458+
func UnorderedSlicesEqual(slice1 []string, slice2 []string) bool {
459+
set1 := sets.NewString(slice1...)
460+
set2 := sets.NewString(slice2...)
461+
spZonesNotInReq := set1.Difference(set2)
462+
if spZonesNotInReq.Len() != 0 {
463+
return false
464+
}
465+
return true
407466
}

‎pkg/common/utils_test.go

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"testing"
2626

2727
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
28+
"github.com/google/go-cmp/cmp"
2829
"google.golang.org/api/googleapi"
2930
"google.golang.org/grpc/codes"
3031
"google.golang.org/grpc/status"
@@ -1101,3 +1102,214 @@ func TestIsValidDiskEncryptionKmsKey(t *testing.T) {
11011102
}
11021103
}
11031104
}
1105+
1106+
func TestFieldsFromResourceName(t *testing.T) {
1107+
testcases := []struct {
1108+
name string
1109+
resourceName string
1110+
expectedProject string
1111+
expectedZone string
1112+
expectedName string
1113+
expectedErr bool
1114+
}{
1115+
{
1116+
name: "StoragePool_WithValidResourceName_ReturnsFields",
1117+
resourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
1118+
expectedProject: "my-project",
1119+
expectedZone: "us-central1-a",
1120+
expectedName: "storagePool-1",
1121+
},
1122+
{
1123+
name: "StoragePool_WithFullResourceURL_ReturnsError",
1124+
resourceName: "https://www.googleapis.com/compute/v1/projects/project/zones/zone/storagePools/storagePool",
1125+
expectedErr: true,
1126+
},
1127+
{
1128+
name: "StoragePool_WithMissingProject_ReturnsError",
1129+
resourceName: "zones/us-central1-a/storagePools/storagePool-1",
1130+
expectedErr: true,
1131+
},
1132+
{
1133+
name: "StoragePool_WithMissingZone_ReturnsError",
1134+
resourceName: "projects/my-project/storagePools/storagePool-1",
1135+
expectedErr: true,
1136+
},
1137+
{
1138+
name: "StoragePool_WithMissingStoragePoolName_ReturnsError",
1139+
resourceName: "projects/my-project/zones/us-central1-a/storagePool-1",
1140+
expectedErr: true,
1141+
},
1142+
}
1143+
for _, tc := range testcases {
1144+
t.Run(tc.name, func(t *testing.T) {
1145+
project, zone, name, err := fieldsFromStoragePoolResourceName(tc.resourceName)
1146+
input := fmt.Sprintf("fieldsFromStoragePoolResourceName(%q)", tc.resourceName)
1147+
gotErr := err != nil
1148+
if gotErr != tc.expectedErr {
1149+
t.Errorf("%s error presence = %v, expected error presence = %v", input, gotErr, tc.expectedErr)
1150+
}
1151+
if project != tc.expectedProject || zone != tc.expectedZone || name != tc.expectedName {
1152+
t.Errorf("%s returned {project: %q, zone: %q, name: %q}, expected {project: %q, zone: %q, name: %q}", input, project, zone, name, tc.expectedProject, tc.expectedZone, tc.expectedName)
1153+
}
1154+
})
1155+
}
1156+
}
1157+
1158+
func TestZones(t *testing.T) {
1159+
testcases := []struct {
1160+
name string
1161+
storagePools []StoragePool
1162+
expectedZones []string
1163+
expectedErr bool
1164+
}{
1165+
{
1166+
name: "StoragePools_WithValidResourceNames_ReturnsZones",
1167+
storagePools: []StoragePool{
1168+
{
1169+
Project: "my-project",
1170+
Zone: "us-central1-a",
1171+
Name: "storagePool-1",
1172+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
1173+
},
1174+
{
1175+
Project: "my-project",
1176+
Zone: "us-central1-b",
1177+
Name: "storagePool-2",
1178+
ResourceName: "projects/my-project/zones/us-central1-b/storagePools/storagePool-2",
1179+
},
1180+
},
1181+
expectedZones: []string{"us-central1-a", "us-central1-b"},
1182+
},
1183+
{
1184+
name: "StoragePools_WithDuplicateZone_ReturnsError",
1185+
storagePools: []StoragePool{
1186+
{
1187+
Project: "my-project",
1188+
Zone: "us-central1-a",
1189+
Name: "storagePool-1",
1190+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
1191+
},
1192+
{
1193+
Project: "my-project",
1194+
Zone: "us-central1-a",
1195+
Name: "storagePool-2",
1196+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-2",
1197+
},
1198+
},
1199+
expectedErr: true,
1200+
},
1201+
}
1202+
for _, tc := range testcases {
1203+
t.Run(tc.name, func(t *testing.T) {
1204+
zones, err := StoragePoolZones(tc.storagePools)
1205+
input := fmt.Sprintf("StoragePoolZones(%q)", tc.storagePools)
1206+
gotErr := err != nil
1207+
if gotErr != tc.expectedErr {
1208+
t.Errorf("%s error presence = %v, expected error presence = %v", input, gotErr, tc.expectedErr)
1209+
}
1210+
if diff := cmp.Diff(tc.expectedZones, zones); diff != "" {
1211+
t.Errorf("%s: -want err, +got err\n%s", input, diff)
1212+
}
1213+
})
1214+
}
1215+
}
1216+
1217+
func TestStoragePoolInZone(t *testing.T) {
1218+
testcases := []struct {
1219+
name string
1220+
storagePools []StoragePool
1221+
zone string
1222+
expectedStoragePool *StoragePool
1223+
expectedErr bool
1224+
}{
1225+
{
1226+
name: "ValidStoragePools_ReturnsStoragePoolInZone",
1227+
storagePools: []StoragePool{
1228+
{
1229+
Project: "my-project",
1230+
Zone: "us-central1-a",
1231+
Name: "storagePool-1",
1232+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
1233+
},
1234+
{
1235+
Project: "my-project",
1236+
Zone: "us-central1-b",
1237+
Name: "storagePool-2",
1238+
ResourceName: "projects/my-project/zones/us-central1-b/storagePools/storagePool-2",
1239+
},
1240+
},
1241+
zone: "us-central1-a",
1242+
expectedStoragePool: &StoragePool{
1243+
Project: "my-project",
1244+
Zone: "us-central1-a",
1245+
Name: "storagePool-1",
1246+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
1247+
},
1248+
},
1249+
{
1250+
name: "StoragePoolNotInZone_ReturnsNil",
1251+
storagePools: []StoragePool{
1252+
{
1253+
Project: "my-project",
1254+
Zone: "us-central1-a",
1255+
Name: "storagePool-1",
1256+
ResourceName: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1",
1257+
},
1258+
},
1259+
zone: "us-central1-b",
1260+
expectedStoragePool: nil,
1261+
},
1262+
}
1263+
for _, tc := range testcases {
1264+
t.Run(tc.name, func(t *testing.T) {
1265+
sp := StoragePoolInZone(tc.storagePools, tc.zone)
1266+
input := fmt.Sprintf("StoragePoolInZone(%q)", tc.storagePools)
1267+
if diff := cmp.Diff(tc.expectedStoragePool, sp); diff != "" {
1268+
t.Errorf("%s: -want, +got \n%s", input, diff)
1269+
}
1270+
})
1271+
}
1272+
}
1273+
1274+
func TestUnorderedSlicesEqual(t *testing.T) {
1275+
testcases := []struct {
1276+
name string
1277+
slice1 []string
1278+
slice2 []string
1279+
expectedSlicesEqual bool
1280+
}{
1281+
{
1282+
name: "OrderedSlicesEqual_ReturnsTrue",
1283+
slice1: []string{"us-central1-a", "us-central1-b"},
1284+
slice2: []string{"us-central1-a", "us-central1-b"},
1285+
expectedSlicesEqual: true,
1286+
},
1287+
{
1288+
name: "UnorderedSlicesEqual_ReturnsTrue",
1289+
slice1: []string{"us-central1-a", "us-central1-b"},
1290+
slice2: []string{"us-central1-b", "us-central1-a"},
1291+
expectedSlicesEqual: true,
1292+
},
1293+
{
1294+
name: "SlicesNotEqualSameLength_ReturnsFalse",
1295+
slice1: []string{"us-central1-a", "us-central1-b"},
1296+
slice2: []string{"us-central1-a", "us-central1-a"},
1297+
expectedSlicesEqual: false,
1298+
},
1299+
{
1300+
name: "SlicesNotEqualDifferentLength_ReturnsFalse",
1301+
slice1: []string{"us-central1-a"},
1302+
slice2: []string{},
1303+
expectedSlicesEqual: false,
1304+
},
1305+
}
1306+
for _, tc := range testcases {
1307+
t.Run(tc.name, func(t *testing.T) {
1308+
slicesEqual := UnorderedSlicesEqual(tc.slice1, tc.slice2)
1309+
input := fmt.Sprintf("UnorderedSlicesEqual(%v, %v)", tc.slice1, tc.slice2)
1310+
if diff := cmp.Diff(tc.expectedSlicesEqual, slicesEqual); diff != "" {
1311+
t.Errorf("%s: -want, +got \n%s", input, diff)
1312+
}
1313+
})
1314+
}
1315+
}

0 commit comments

Comments
 (0)