Skip to content

Commit db9f2b9

Browse files
amacaskillamacaskill
committed
add create regional clone from zonal disk and improve tests
1 parent 19f3653 commit db9f2b9

File tree

8 files changed

+341
-96
lines changed

8 files changed

+341
-96
lines changed

‎pkg/gce-cloud-provider/compute/gce-compute.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ func ValidateDiskParameters(disk *CloudDisk, params common.DiskParameters) error
330330
return fmt.Errorf("actual disk replication type %v did not match expected param %s", locationType, params.ReplicationType)
331331
}
332332

333-
if !kmsKeyEqual(
333+
if !KmsKeyEqual(
334334
disk.GetKMSKeyName(), /* fetchedKMSKey */
335335
params.DiskEncryptionKMSKey /* storageClassKMSKey */) {
336336
return fmt.Errorf("actual disk KMS key name %s did not match expected param %s", disk.GetKMSKeyName(), params.DiskEncryptionKMSKey)
@@ -1119,7 +1119,7 @@ func (cloud *CloudProvider) waitForSnapshotCreation(ctx context.Context, project
11191119
// storageClassKMSKey - key as provided by the client
11201120
// example: projects/{0}/locations/{1}/keyRings/{2}/cryptoKeys/{3}
11211121
// cryptoKeyVersions should be disregarded if the rest of the key is identical.
1122-
func kmsKeyEqual(fetchedKMSKey, storageClassKMSKey string) bool {
1122+
func KmsKeyEqual(fetchedKMSKey, storageClassKMSKey string) bool {
11231123
return removeCryptoKeyVersion(fetchedKMSKey) == removeCryptoKeyVersion(storageClassKMSKey)
11241124
}
11251125

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

+31-3
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,38 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
243243
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume unknown get disk error when validating: %v", err))
244244
}
245245
}
246-
// Verify the zone, region, and disk type of the clone must be the same as that of the source disk.
247-
if err := gce.ValidateDiskParameters(diskFromSourceVolume, params); err != nil {
248-
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume parameters do not match CreateVolumeRequest Parameters: %v`, err)
246+
247+
// Verify the disk type and encryption key of the clone are the same as that of the source disk.
248+
if diskFromSourceVolume.GetPDType() != params.DiskType || !gce.KmsKeyEqual(diskFromSourceVolume.GetKMSKeyName(), params.DiskEncryptionKMSKey) {
249+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume Parameters %v do not match source volume Parameters", params)
250+
}
251+
// Verify the disk capacity range are the same or greater as that of the source disk.
252+
if diskFromSourceVolume.GetSizeGb() > common.BytesToGbRoundDown(capBytes) {
253+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume disk CapacityRange %d is less than source volume CapacityRange %d", common.BytesToGbRoundDown(capBytes), diskFromSourceVolume.GetSizeGb())
254+
}
255+
256+
if params.ReplicationType == replicationTypeNone {
257+
// For zonal->zonal disk clones, verify the zone is the same as that of the source disk.
258+
if sourceVolKey.Zone != volKey.Zone {
259+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume disk zone %s does not match source volume zone %s", volKey.Zone, sourceVolKey.Zone)
260+
}
261+
// regional->zonal disk clones are not allowed.
262+
if diskFromSourceVolume.LocationType() == meta.Regional {
263+
return nil, status.Errorf(codes.InvalidArgument, "Cannot create a zonal disk clone from a regional disk")
264+
}
249265
}
266+
267+
if params.ReplicationType == replicationTypeNone {
268+
// For regional->regional disk clones, verify the region is the same as that of the source disk.
269+
if diskFromSourceVolume.LocationType() == meta.Regional && sourceVolKey.Region != volKey.Region {
270+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume disk region %s does not match source volume region %s", volKey.Region, sourceVolKey.Region)
271+
}
272+
// For zonal->regional disk clones, verify one of the replica zones matches the source disk zone.
273+
if diskFromSourceVolume.LocationType() == meta.Zonal && !containsZone(zones, sourceVolKey.Zone) {
274+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume regional disk replica zones %v do not match source volume zone %s", zones, sourceVolKey.Zone)
275+
}
276+
}
277+
250278
// Verify the source disk is ready.
251279
ready, err := isDiskReady(diskFromSourceVolume)
252280
if err != nil {

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

+174-75
Original file line numberDiff line numberDiff line change
@@ -1004,97 +1004,200 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
10041004
testSourceVolumeName := "test-volume-source-name"
10051005
testZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, zone, testSourceVolumeName)
10061006
testRegionalVolumeSourceID := fmt.Sprintf("projects/%s/regions/%s/disks/%s", project, region, testSourceVolumeName)
1007-
testVolumeSourceIDDifferentZone := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, "different-zone", testSourceVolumeName)
1007+
testSecondZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, "different-zone1", testSourceVolumeName)
1008+
zonalParams := map[string]string{
1009+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeNone,
1010+
common.ParameterKeyDiskEncryptionKmsKey: "encryption-key",
1011+
}
1012+
regionalParams := map[string]string{
1013+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeRegionalPD,
1014+
common.ParameterKeyDiskEncryptionKmsKey: "encryption-key",
1015+
}
10081016
topology := &csi.TopologyRequirement{
10091017
Requisite: []*csi.Topology{
10101018
{
1011-
Segments: map[string]string{common.TopologyKeyZone: region + "-b"},
1019+
Segments: map[string]string{common.TopologyKeyZone: zone},
10121020
},
10131021
{
1014-
Segments: map[string]string{common.TopologyKeyZone: region + "-c"},
1022+
Segments: map[string]string{common.TopologyKeyZone: secondZone},
10151023
},
10161024
},
10171025
}
1018-
regionalParams := map[string]string{
1019-
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: "regional-pd",
1020-
}
1026+
10211027
// Define test cases
10221028
testCases := []struct {
1023-
name string
1024-
volumeOnCloud bool
1025-
expErrCode codes.Code
1026-
sourceVolumeID string
1027-
reqParameters map[string]string
1028-
sourceReqParameters map[string]string
1029-
topology *csi.TopologyRequirement
1029+
name string
1030+
volumeOnCloud bool
1031+
expErrCode codes.Code
1032+
sourceVolumeID string
1033+
reqParameters map[string]string
1034+
sourceReqParameters map[string]string
1035+
sourceCapacityRange *csi.CapacityRange
1036+
requestCapacityRange *csi.CapacityRange
1037+
sourceTopology *csi.TopologyRequirement
1038+
requestTopology *csi.TopologyRequirement
10301039
}{
10311040
{
1032-
name: "success with data source of zonal volume type",
1033-
volumeOnCloud: true,
1034-
sourceVolumeID: testZonalVolumeSourceID,
1035-
reqParameters: stdParams,
1036-
sourceReqParameters: stdParams,
1037-
},
1038-
{
1039-
name: "success with data source of regional volume type",
1040-
volumeOnCloud: true,
1041-
sourceVolumeID: testRegionalVolumeSourceID,
1042-
reqParameters: regionalParams,
1043-
sourceReqParameters: regionalParams,
1044-
topology: topology,
1045-
},
1046-
{
1047-
name: "fail with with data source of replication-type different from CreateVolumeRequest",
1048-
volumeOnCloud: true,
1049-
expErrCode: codes.InvalidArgument,
1050-
sourceVolumeID: testZonalVolumeSourceID,
1051-
reqParameters: stdParams,
1052-
sourceReqParameters: regionalParams,
1053-
topology: topology,
1054-
},
1055-
{
1056-
name: "fail with data source of zonal volume type that doesn't exist",
1057-
volumeOnCloud: false,
1058-
expErrCode: codes.NotFound,
1059-
sourceVolumeID: testZonalVolumeSourceID,
1060-
reqParameters: stdParams,
1061-
sourceReqParameters: stdParams,
1062-
},
1063-
{
1064-
name: "fail with data source of zonal volume type with invalid volume id format",
1065-
volumeOnCloud: false,
1066-
expErrCode: codes.InvalidArgument,
1067-
sourceVolumeID: testZonalVolumeSourceID + "invalid/format",
1068-
reqParameters: stdParams,
1069-
sourceReqParameters: stdParams,
1041+
name: "success zonal disk clone of zonal source disk",
1042+
volumeOnCloud: true,
1043+
sourceVolumeID: testZonalVolumeSourceID,
1044+
requestCapacityRange: stdCapRange,
1045+
sourceCapacityRange: stdCapRange,
1046+
reqParameters: zonalParams,
1047+
sourceReqParameters: zonalParams,
1048+
sourceTopology: topology,
1049+
requestTopology: topology,
1050+
},
1051+
{
1052+
name: "success regional disk clone of regional source disk",
1053+
volumeOnCloud: true,
1054+
sourceVolumeID: testRegionalVolumeSourceID,
1055+
requestCapacityRange: stdCapRange,
1056+
sourceCapacityRange: stdCapRange,
1057+
reqParameters: regionalParams,
1058+
sourceReqParameters: regionalParams,
1059+
sourceTopology: topology,
1060+
requestTopology: topology,
1061+
},
1062+
{
1063+
name: "success regional disk clone of zonal data source",
1064+
volumeOnCloud: true,
1065+
sourceVolumeID: testZonalVolumeSourceID,
1066+
requestCapacityRange: stdCapRange,
1067+
sourceCapacityRange: stdCapRange,
1068+
reqParameters: regionalParams,
1069+
sourceReqParameters: zonalParams,
1070+
sourceTopology: topology,
1071+
requestTopology: topology,
1072+
},
1073+
{
1074+
name: "fail regional disk clone with no matching replica zone of zonal data source",
1075+
volumeOnCloud: true,
1076+
expErrCode: codes.InvalidArgument,
1077+
sourceVolumeID: testZonalVolumeSourceID,
1078+
requestCapacityRange: stdCapRange,
1079+
sourceCapacityRange: stdCapRange,
1080+
reqParameters: regionalParams,
1081+
sourceReqParameters: zonalParams,
1082+
sourceTopology: topology,
1083+
requestTopology: &csi.TopologyRequirement{
1084+
Requisite: []*csi.Topology{
1085+
{
1086+
Segments: map[string]string{common.TopologyKeyZone: "different-zone1"},
1087+
},
1088+
{
1089+
Segments: map[string]string{common.TopologyKeyZone: "different-zone2"},
1090+
},
1091+
},
1092+
},
10701093
},
10711094
{
1072-
name: "fail with data source of zonal volume type with invalid disk parameters",
1073-
volumeOnCloud: true,
1074-
expErrCode: codes.InvalidArgument,
1075-
sourceVolumeID: testVolumeSourceIDDifferentZone,
1076-
reqParameters: stdParams,
1095+
name: "fail zonal disk clone with different disk type",
1096+
volumeOnCloud: true,
1097+
expErrCode: codes.InvalidArgument,
1098+
sourceVolumeID: testZonalVolumeSourceID,
1099+
requestCapacityRange: stdCapRange,
1100+
sourceCapacityRange: stdCapRange,
1101+
reqParameters: zonalParams,
10771102
sourceReqParameters: map[string]string{
10781103
common.ParameterKeyType: "different-type",
10791104
},
1105+
sourceTopology: topology,
1106+
requestTopology: topology,
10801107
},
10811108
{
1082-
name: "fail with data source of zonal volume type with invalid replication type",
1083-
volumeOnCloud: true,
1084-
expErrCode: codes.InvalidArgument,
1085-
sourceVolumeID: testZonalVolumeSourceID,
1086-
reqParameters: regionalParams,
1087-
sourceReqParameters: stdParams,
1088-
},
1089-
}
1109+
name: "fail zonal disk clone with different DiskEncryptionKMSKey",
1110+
volumeOnCloud: true,
1111+
expErrCode: codes.InvalidArgument,
1112+
sourceVolumeID: testZonalVolumeSourceID,
1113+
requestCapacityRange: stdCapRange,
1114+
sourceCapacityRange: stdCapRange,
1115+
reqParameters: zonalParams,
1116+
sourceReqParameters: map[string]string{
1117+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeNone,
1118+
common.ParameterKeyDiskEncryptionKmsKey: "different-encryption-key",
1119+
},
1120+
sourceTopology: topology,
1121+
requestTopology: topology,
1122+
},
1123+
{
1124+
name: "fail zonal disk clone with different zone",
1125+
volumeOnCloud: true,
1126+
expErrCode: codes.InvalidArgument,
1127+
sourceVolumeID: testSecondZonalVolumeSourceID,
1128+
requestCapacityRange: stdCapRange,
1129+
sourceCapacityRange: stdCapRange,
1130+
reqParameters: zonalParams,
1131+
sourceReqParameters: zonalParams,
1132+
sourceTopology: &csi.TopologyRequirement{
1133+
Requisite: []*csi.Topology{
1134+
{
1135+
Segments: map[string]string{common.TopologyKeyZone: "different-zone1"},
1136+
},
1137+
{
1138+
Segments: map[string]string{common.TopologyKeyZone: "different-zone2"},
1139+
},
1140+
},
1141+
},
1142+
requestTopology: topology,
1143+
},
1144+
{
1145+
name: "fail zonal disk clone of regional data source",
1146+
volumeOnCloud: true,
1147+
expErrCode: codes.InvalidArgument,
1148+
sourceVolumeID: testRegionalVolumeSourceID,
1149+
requestCapacityRange: stdCapRange,
1150+
sourceCapacityRange: stdCapRange,
1151+
reqParameters: zonalParams,
1152+
sourceReqParameters: regionalParams,
1153+
sourceTopology: topology,
1154+
requestTopology: topology,
1155+
},
1156+
1157+
{
1158+
name: "fail zonal source disk does not exist",
1159+
volumeOnCloud: false,
1160+
expErrCode: codes.NotFound,
1161+
sourceVolumeID: testZonalVolumeSourceID,
1162+
requestCapacityRange: stdCapRange,
1163+
sourceCapacityRange: stdCapRange,
1164+
reqParameters: stdParams,
1165+
sourceReqParameters: stdParams,
1166+
requestTopology: topology,
1167+
},
1168+
{
1169+
name: "fail invalid source disk volume id format",
1170+
volumeOnCloud: false,
1171+
expErrCode: codes.InvalidArgument,
1172+
sourceVolumeID: testZonalVolumeSourceID + "/invalid/format",
1173+
requestCapacityRange: stdCapRange,
1174+
sourceCapacityRange: stdCapRange,
1175+
reqParameters: stdParams,
1176+
sourceReqParameters: stdParams,
1177+
requestTopology: topology,
1178+
},
1179+
{
1180+
name: "fail zonal disk clone with smaller disk capacity",
1181+
volumeOnCloud: true,
1182+
expErrCode: codes.InvalidArgument,
1183+
sourceVolumeID: testZonalVolumeSourceID,
1184+
requestCapacityRange: stdCapRange,
1185+
sourceCapacityRange: &csi.CapacityRange{
1186+
RequiredBytes: common.GbToBytes(40),
1187+
},
1188+
reqParameters: zonalParams,
1189+
sourceReqParameters: zonalParams,
1190+
sourceTopology: topology,
1191+
requestTopology: topology,
1192+
}}
10901193

10911194
for _, tc := range testCases {
10921195
t.Logf("test case: %s", tc.name)
10931196
gceDriver := initGCEDriver(t, nil)
10941197

10951198
req := &csi.CreateVolumeRequest{
10961199
Name: name,
1097-
CapacityRange: stdCapRange,
1200+
CapacityRange: tc.requestCapacityRange,
10981201
VolumeCapabilities: stdVolCaps,
10991202
Parameters: tc.reqParameters,
11001203
VolumeContentSource: &csi.VolumeContentSource{
@@ -1104,18 +1207,15 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
11041207
},
11051208
},
11061209
},
1210+
AccessibilityRequirements: tc.requestTopology,
11071211
}
11081212

11091213
sourceVolumeRequest := &csi.CreateVolumeRequest{
1110-
Name: testSourceVolumeName,
1111-
CapacityRange: stdCapRange,
1112-
VolumeCapabilities: stdVolCaps,
1113-
Parameters: tc.sourceReqParameters,
1114-
}
1115-
1116-
if tc.topology != nil {
1117-
// req.AccessibilityRequirements = tc.topology
1118-
sourceVolumeRequest.AccessibilityRequirements = tc.topology
1214+
Name: testSourceVolumeName,
1215+
CapacityRange: tc.sourceCapacityRange,
1216+
VolumeCapabilities: stdVolCaps,
1217+
Parameters: tc.sourceReqParameters,
1218+
AccessibilityRequirements: tc.sourceTopology,
11191219
}
11201220

11211221
if tc.volumeOnCloud {
@@ -1148,7 +1248,6 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
11481248

11491249
// Make sure the response has the source volume.
11501250
sourceVolume := resp.GetVolume()
1151-
t.Logf("response has source volume: %v ", sourceVolume)
11521251
if sourceVolume.ContentSource == nil || sourceVolume.ContentSource.Type == nil ||
11531252
sourceVolume.ContentSource.GetVolume() == nil || sourceVolume.ContentSource.GetVolume().VolumeId == "" {
11541253
t.Fatalf("Expected volume content source to have volume ID, got none")

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

+10
Original file line numberDiff line numberDiff line change
@@ -199,3 +199,13 @@ func collectMountOptions(fsType string, mntFlags []string) []string {
199199
}
200200
return options
201201
}
202+
203+
func containsZone(zones []string, zone string) bool {
204+
for _, z := range zones {
205+
if z == zone {
206+
return true
207+
}
208+
}
209+
210+
return false
211+
}

‎test/e2e/tests/multi_zone_e2e_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ var _ = Describe("GCE PD CSI Driver Multi-Zone", func() {
101101
Segments: map[string]string{common.TopologyKeyZone: zones[1]},
102102
},
103103
},
104-
})
104+
}, nil)
105105
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
106106

107107
// Validate Disk Created

0 commit comments

Comments
 (0)