Skip to content

Commit 632cfd3

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

File tree

8 files changed

+225
-66
lines changed

8 files changed

+225
-66
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

+27-3
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,34 @@ 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 source volume Parameters do not match CreateVolumeRequest Parameters %v`, params)
250+
}
251+
252+
if params.ReplicationType == replicationTypeNone {
253+
// For zonal->zonal disk clones, verify the zone is the same as that of the source disk.
254+
if sourceVolKey.Zone != volKey.Zone {
255+
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume zone %s does not match CreateVolume disk zone %s`, sourceVolKey.Zone, volKey.Zone)
256+
}
257+
// regional->zonal disk clones are not allowed.
258+
if diskFromSourceVolume.LocationType() == meta.Regional {
259+
return nil, status.Errorf(codes.InvalidArgument, `Cannot create a zonal disk clone from a regional disk`)
260+
}
249261
}
262+
263+
if params.ReplicationType == replicationTypeNone {
264+
// For regional->regional disk clones, verify the region is the same as that of the source disk.
265+
if diskFromSourceVolume.LocationType() == meta.Regional && sourceVolKey.Region != volKey.Region {
266+
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume region %s does not match CreateVolume disk region %s`, sourceVolKey.Region, volKey.Region)
267+
}
268+
// For zonal->regional disk clones, verify one of the replica zones matches the source disk zone.
269+
if diskFromSourceVolume.LocationType() == meta.Zonal && !containsZone(zones, sourceVolKey.Zone) {
270+
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume zone %s does not match one of the replica zones %v of the CreateVolume regional disk`, sourceVolKey.Zone, zones)
271+
}
272+
}
273+
250274
// Verify the source disk is ready.
251275
ready, err := isDiskReady(diskFromSourceVolume)
252276
if err != nil {

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

+109-45
Original file line numberDiff line numberDiff line change
@@ -1001,23 +1001,30 @@ func TestCreateVolumeWithVolumeSourceFromSnapshot(t *testing.T) {
10011001
}
10021002

10031003
func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
1004+
10041005
testSourceVolumeName := "test-volume-source-name"
10051006
testZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, zone, testSourceVolumeName)
10061007
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)
1008+
testSecondZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, "different-zone1", testSourceVolumeName)
1009+
zonalParams := map[string]string{
1010+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeNone,
1011+
common.ParameterKeyDiskEncryptionKmsKey: "encryption-key",
1012+
}
1013+
regionalParams := map[string]string{
1014+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeRegionalPD,
1015+
common.ParameterKeyDiskEncryptionKmsKey: "encryption-key",
1016+
}
10081017
topology := &csi.TopologyRequirement{
10091018
Requisite: []*csi.Topology{
10101019
{
1011-
Segments: map[string]string{common.TopologyKeyZone: region + "-b"},
1020+
Segments: map[string]string{common.TopologyKeyZone: zone},
10121021
},
10131022
{
1014-
Segments: map[string]string{common.TopologyKeyZone: region + "-c"},
1023+
Segments: map[string]string{common.TopologyKeyZone: secondZone},
10151024
},
10161025
},
10171026
}
1018-
regionalParams := map[string]string{
1019-
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: "regional-pd",
1020-
}
1027+
10211028
// Define test cases
10221029
testCases := []struct {
10231030
name string
@@ -1026,67 +1033,128 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
10261033
sourceVolumeID string
10271034
reqParameters map[string]string
10281035
sourceReqParameters map[string]string
1029-
topology *csi.TopologyRequirement
1036+
sourceTopology *csi.TopologyRequirement
1037+
requestTopology *csi.TopologyRequirement
10301038
}{
10311039
{
1032-
name: "success with data source of zonal volume type",
1040+
name: "success zonal disk clone of zonal source disk",
10331041
volumeOnCloud: true,
10341042
sourceVolumeID: testZonalVolumeSourceID,
1035-
reqParameters: stdParams,
1036-
sourceReqParameters: stdParams,
1043+
reqParameters: zonalParams,
1044+
sourceReqParameters: zonalParams,
1045+
sourceTopology: topology,
1046+
requestTopology: topology,
10371047
},
10381048
{
1039-
name: "success with data source of regional volume type",
1049+
name: "success regional disk clone of regional source disk",
10401050
volumeOnCloud: true,
10411051
sourceVolumeID: testRegionalVolumeSourceID,
10421052
reqParameters: regionalParams,
10431053
sourceReqParameters: regionalParams,
1044-
topology: topology,
1054+
sourceTopology: topology,
1055+
requestTopology: topology,
10451056
},
10461057
{
1047-
name: "fail with with data source of replication-type different from CreateVolumeRequest",
1058+
name: "success regional disk clone of zonal data source",
10481059
volumeOnCloud: true,
1049-
expErrCode: codes.InvalidArgument,
10501060
sourceVolumeID: testZonalVolumeSourceID,
1051-
reqParameters: stdParams,
1052-
sourceReqParameters: regionalParams,
1053-
topology: topology,
1061+
reqParameters: regionalParams,
1062+
sourceReqParameters: zonalParams,
1063+
sourceTopology: topology,
1064+
requestTopology: topology,
10541065
},
10551066
{
1056-
name: "fail with data source of zonal volume type that doesn't exist",
1057-
volumeOnCloud: false,
1058-
expErrCode: codes.NotFound,
1067+
name: "fail regional disk clone with no matching replica zone of zonal data source",
1068+
volumeOnCloud: true,
1069+
expErrCode: codes.InvalidArgument,
10591070
sourceVolumeID: testZonalVolumeSourceID,
1060-
reqParameters: stdParams,
1061-
sourceReqParameters: stdParams,
1071+
reqParameters: regionalParams,
1072+
sourceReqParameters: zonalParams,
1073+
sourceTopology: topology,
1074+
requestTopology: &csi.TopologyRequirement{
1075+
Requisite: []*csi.Topology{
1076+
{
1077+
Segments: map[string]string{common.TopologyKeyZone: "different-zone1"},
1078+
},
1079+
{
1080+
Segments: map[string]string{common.TopologyKeyZone: "different-zone2"},
1081+
},
1082+
},
1083+
},
10621084
},
10631085
{
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,
1086+
name: "fail zonal disk clone with different disk type",
1087+
volumeOnCloud: true,
1088+
expErrCode: codes.InvalidArgument,
1089+
sourceVolumeID: testZonalVolumeSourceID,
1090+
reqParameters: zonalParams,
1091+
sourceReqParameters: map[string]string{
1092+
common.ParameterKeyType: "different-type",
1093+
},
1094+
sourceTopology: topology,
1095+
requestTopology: topology,
10701096
},
10711097
{
1072-
name: "fail with data source of zonal volume type with invalid disk parameters",
1098+
name: "fail zonal disk clone with different DiskEncryptionKMSKey",
10731099
volumeOnCloud: true,
10741100
expErrCode: codes.InvalidArgument,
1075-
sourceVolumeID: testVolumeSourceIDDifferentZone,
1076-
reqParameters: stdParams,
1101+
sourceVolumeID: testZonalVolumeSourceID,
1102+
reqParameters: zonalParams,
10771103
sourceReqParameters: map[string]string{
1078-
common.ParameterKeyType: "different-type",
1104+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeNone,
1105+
common.ParameterKeyDiskEncryptionKmsKey: "different-encryption-key",
10791106
},
1107+
sourceTopology: topology,
1108+
requestTopology: topology,
10801109
},
10811110
{
1082-
name: "fail with data source of zonal volume type with invalid replication type",
1111+
name: "fail zonal disk clone with different zone",
10831112
volumeOnCloud: true,
10841113
expErrCode: codes.InvalidArgument,
1114+
sourceVolumeID: testSecondZonalVolumeSourceID,
1115+
reqParameters: zonalParams,
1116+
sourceReqParameters: zonalParams,
1117+
sourceTopology: &csi.TopologyRequirement{
1118+
Requisite: []*csi.Topology{
1119+
{
1120+
Segments: map[string]string{common.TopologyKeyZone: "different-zone1"},
1121+
},
1122+
{
1123+
Segments: map[string]string{common.TopologyKeyZone: "different-zone2"},
1124+
},
1125+
},
1126+
},
1127+
requestTopology: topology,
1128+
},
1129+
{
1130+
name: "fail zonal disk clone of regional data source",
1131+
volumeOnCloud: true,
1132+
expErrCode: codes.InvalidArgument,
1133+
sourceVolumeID: testRegionalVolumeSourceID,
1134+
reqParameters: zonalParams,
1135+
sourceReqParameters: regionalParams,
1136+
sourceTopology: topology,
1137+
requestTopology: topology,
1138+
},
1139+
1140+
{
1141+
name: "fail zonal source disk does not exist",
1142+
volumeOnCloud: false,
1143+
expErrCode: codes.NotFound,
10851144
sourceVolumeID: testZonalVolumeSourceID,
1086-
reqParameters: regionalParams,
1145+
reqParameters: stdParams,
10871146
sourceReqParameters: stdParams,
1147+
requestTopology: topology,
10881148
},
1089-
}
1149+
{
1150+
name: "fail invalid source disk volume id format",
1151+
volumeOnCloud: false,
1152+
expErrCode: codes.InvalidArgument,
1153+
sourceVolumeID: testZonalVolumeSourceID + "/invalid/format",
1154+
reqParameters: stdParams,
1155+
sourceReqParameters: stdParams,
1156+
requestTopology: topology,
1157+
}}
10901158

10911159
for _, tc := range testCases {
10921160
t.Logf("test case: %s", tc.name)
@@ -1104,18 +1172,15 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
11041172
},
11051173
},
11061174
},
1175+
AccessibilityRequirements: tc.requestTopology,
11071176
}
11081177

11091178
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
1179+
Name: testSourceVolumeName,
1180+
CapacityRange: stdCapRange,
1181+
VolumeCapabilities: stdVolCaps,
1182+
Parameters: tc.sourceReqParameters,
1183+
AccessibilityRequirements: tc.sourceTopology,
11191184
}
11201185

11211186
if tc.volumeOnCloud {
@@ -1148,7 +1213,6 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
11481213

11491214
// Make sure the response has the source volume.
11501215
sourceVolume := resp.GetVolume()
1151-
t.Logf("response has source volume: %v ", sourceVolume)
11521216
if sourceVolume.ContentSource == nil || sourceVolume.ContentSource.Type == nil ||
11531217
sourceVolume.ContentSource.GetVolume() == nil || sourceVolume.ContentSource.GetVolume().VolumeId == "" {
11541218
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(s []string, str string) bool {
204+
for _, v := range s {
205+
if v == str {
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

‎test/e2e/tests/resize_e2e_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
4545
Segments: map[string]string{common.TopologyKeyZone: z},
4646
},
4747
},
48-
})
48+
}, nil)
4949
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
5050

5151
// Validate Disk Created
@@ -152,7 +152,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
152152
Segments: map[string]string{common.TopologyKeyZone: z},
153153
},
154154
},
155-
})
155+
}, nil)
156156
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
157157

158158
// Validate Disk Created & size
@@ -264,7 +264,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
264264
Segments: map[string]string{common.TopologyKeyZone: z},
265265
},
266266
},
267-
})
267+
}, nil)
268268
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
269269

270270
// Validate Disk Created

0 commit comments

Comments
 (0)