Skip to content

Commit c937afd

Browse files
amacaskillamacaskill
committed
add create regional clone from zonal disk and improve tests
1 parent 46c7c75 commit c937afd

File tree

7 files changed

+186
-65
lines changed

7 files changed

+186
-65
lines changed

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

+27-3
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,34 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
239239
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume unknown get disk error when validating: %v", err))
240240
}
241241
}
242-
// Verify the zone, region, and disk type of the clone must be the same as that of the source disk.
243-
if err := gce.ValidateDiskParameters(diskFromSourceVolume, params); err != nil {
244-
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume parameters do not match CreateVolumeRequest Parameters: %v`, err)
242+
243+
// Verify the disk type and encryption key of the clone are the same as that of the source disk.
244+
if diskFromSourceVolume.GetPDType() != params.DiskType || !kmsKeyEqual(diskFromSourceVolume.GetKMSKeyName(), params.DiskEncryptionKMSKey) {
245+
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume Parameters do not match CreateVolumeRequest Parameters %v`, params)
246+
}
247+
248+
if params.ReplicationType == replicationTypeNone {
249+
// For zonal->zonal disk clones, verify the zone is the same as that of the source disk.
250+
if sourceVolKey.Zone != volKey.Zone {
251+
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume zone %s does not match CreateVolume disk zone %s`, sourceVolKey.Zone, volKey.Zone)
252+
}
253+
// regional->zonal disk clones are not allowed.
254+
if diskFromSourceVolume.LocationType() == meta.Regional {
255+
return nil, status.Errorf(codes.InvalidArgument, `Cannot create a zonal disk clone from a regional disk`)
256+
}
245257
}
258+
259+
if params.ReplicationType == replicationTypeNone {
260+
// For regional->regional disk clones, verify the region is the same as that of the source disk.
261+
if diskFromSourceVolume.LocationType() == meta.Regional && sourceVolKey.Region != volKey.Region {
262+
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume region %s does not match CreateVolume disk region %s`, sourceVolKey.Region, volKey.Region)
263+
}
264+
// For zonal->regional disk clones, verify one of the replica zones matches the source disk zone.
265+
if diskFromSourceVolume.LocationType() == meta.Zonal && !contains(zones, sourceVolKey.Zone) {
266+
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)
267+
}
268+
}
269+
246270
// Verify the source disk is ready.
247271
ready, err := isDiskReady(diskFromSourceVolume)
248272
if err != nil {

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

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

900900
func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
901+
901902
testSourceVolumeName := "test-volume-source-name"
902903
testZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, zone, testSourceVolumeName)
903904
testRegionalVolumeSourceID := fmt.Sprintf("projects/%s/regions/%s/disks/%s", project, region, testSourceVolumeName)
904-
testVolumeSourceIDDifferentZone := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, "different-zone", testSourceVolumeName)
905+
testSecondZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, "different-zone1", testSourceVolumeName)
906+
zonalParams := map[string]string{
907+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeNone,
908+
common.ParameterKeyDiskEncryptionKmsKey: "encryption-key",
909+
}
910+
regionalParams := map[string]string{
911+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeRegionalPD,
912+
common.ParameterKeyDiskEncryptionKmsKey: "encryption-key",
913+
}
905914
topology := &csi.TopologyRequirement{
906915
Requisite: []*csi.Topology{
907916
{
908-
Segments: map[string]string{common.TopologyKeyZone: region + "-b"},
917+
Segments: map[string]string{common.TopologyKeyZone: zone},
909918
},
910919
{
911-
Segments: map[string]string{common.TopologyKeyZone: region + "-c"},
920+
Segments: map[string]string{common.TopologyKeyZone: secondZone},
912921
},
913922
},
914923
}
915-
regionalParams := map[string]string{
916-
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: "regional-pd",
917-
}
924+
918925
// Define test cases
919926
testCases := []struct {
920927
name string
@@ -923,67 +930,128 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
923930
sourceVolumeID string
924931
reqParameters map[string]string
925932
sourceReqParameters map[string]string
926-
topology *csi.TopologyRequirement
933+
sourceTopology *csi.TopologyRequirement
934+
requestTopology *csi.TopologyRequirement
927935
}{
928936
{
929-
name: "success with data source of zonal volume type",
937+
name: "success zonal disk clone of zonal source disk",
930938
volumeOnCloud: true,
931939
sourceVolumeID: testZonalVolumeSourceID,
932-
reqParameters: stdParams,
933-
sourceReqParameters: stdParams,
940+
reqParameters: zonalParams,
941+
sourceReqParameters: zonalParams,
942+
sourceTopology: topology,
943+
requestTopology: topology,
934944
},
935945
{
936-
name: "success with data source of regional volume type",
946+
name: "success regional disk clone of regional source disk",
937947
volumeOnCloud: true,
938948
sourceVolumeID: testRegionalVolumeSourceID,
939949
reqParameters: regionalParams,
940950
sourceReqParameters: regionalParams,
941-
topology: topology,
951+
sourceTopology: topology,
952+
requestTopology: topology,
942953
},
943954
{
944-
name: "fail with with data source of replication-type different from CreateVolumeRequest",
955+
name: "success regional disk clone of zonal data source",
945956
volumeOnCloud: true,
946-
expErrCode: codes.InvalidArgument,
947957
sourceVolumeID: testZonalVolumeSourceID,
948-
reqParameters: stdParams,
949-
sourceReqParameters: regionalParams,
950-
topology: topology,
958+
reqParameters: regionalParams,
959+
sourceReqParameters: zonalParams,
960+
sourceTopology: topology,
961+
requestTopology: topology,
951962
},
952963
{
953-
name: "fail with data source of zonal volume type that doesn't exist",
954-
volumeOnCloud: false,
955-
expErrCode: codes.NotFound,
964+
name: "fail regional disk clone with no matching replica zone of zonal data source",
965+
volumeOnCloud: true,
966+
expErrCode: codes.InvalidArgument,
956967
sourceVolumeID: testZonalVolumeSourceID,
957-
reqParameters: stdParams,
958-
sourceReqParameters: stdParams,
968+
reqParameters: regionalParams,
969+
sourceReqParameters: zonalParams,
970+
sourceTopology: topology,
971+
requestTopology: &csi.TopologyRequirement{
972+
Requisite: []*csi.Topology{
973+
{
974+
Segments: map[string]string{common.TopologyKeyZone: "different-zone1"},
975+
},
976+
{
977+
Segments: map[string]string{common.TopologyKeyZone: "different-zone2"},
978+
},
979+
},
980+
},
959981
},
960982
{
961-
name: "fail with data source of zonal volume type with invalid volume id format",
962-
volumeOnCloud: false,
963-
expErrCode: codes.InvalidArgument,
964-
sourceVolumeID: testZonalVolumeSourceID + "invalid/format",
965-
reqParameters: stdParams,
966-
sourceReqParameters: stdParams,
983+
name: "fail zonal disk clone with different disk type",
984+
volumeOnCloud: true,
985+
expErrCode: codes.InvalidArgument,
986+
sourceVolumeID: testZonalVolumeSourceID,
987+
reqParameters: zonalParams,
988+
sourceReqParameters: map[string]string{
989+
common.ParameterKeyType: "different-type",
990+
},
991+
sourceTopology: topology,
992+
requestTopology: topology,
967993
},
968994
{
969-
name: "fail with data source of zonal volume type with invalid disk parameters",
995+
name: "fail zonal disk clone with different DiskEncryptionKMSKey",
970996
volumeOnCloud: true,
971997
expErrCode: codes.InvalidArgument,
972-
sourceVolumeID: testVolumeSourceIDDifferentZone,
973-
reqParameters: stdParams,
998+
sourceVolumeID: testZonalVolumeSourceID,
999+
reqParameters: zonalParams,
9741000
sourceReqParameters: map[string]string{
975-
common.ParameterKeyType: "different-type",
1001+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: replicationTypeNone,
1002+
common.ParameterKeyDiskEncryptionKmsKey: "different-encryption-key",
9761003
},
1004+
sourceTopology: topology,
1005+
requestTopology: topology,
9771006
},
9781007
{
979-
name: "fail with data source of zonal volume type with invalid replication type",
1008+
name: "fail zonal disk clone with different zone",
9801009
volumeOnCloud: true,
9811010
expErrCode: codes.InvalidArgument,
1011+
sourceVolumeID: testSecondZonalVolumeSourceID,
1012+
reqParameters: zonalParams,
1013+
sourceReqParameters: zonalParams,
1014+
sourceTopology: &csi.TopologyRequirement{
1015+
Requisite: []*csi.Topology{
1016+
{
1017+
Segments: map[string]string{common.TopologyKeyZone: "different-zone1"},
1018+
},
1019+
{
1020+
Segments: map[string]string{common.TopologyKeyZone: "different-zone2"},
1021+
},
1022+
},
1023+
},
1024+
requestTopology: topology,
1025+
},
1026+
{
1027+
name: "fail zonal disk clone of regional data source",
1028+
volumeOnCloud: true,
1029+
expErrCode: codes.InvalidArgument,
1030+
sourceVolumeID: testRegionalVolumeSourceID,
1031+
reqParameters: zonalParams,
1032+
sourceReqParameters: regionalParams,
1033+
sourceTopology: topology,
1034+
requestTopology: topology,
1035+
},
1036+
1037+
{
1038+
name: "fail zonal source disk does not exist",
1039+
volumeOnCloud: false,
1040+
expErrCode: codes.NotFound,
9821041
sourceVolumeID: testZonalVolumeSourceID,
983-
reqParameters: regionalParams,
1042+
reqParameters: stdParams,
9841043
sourceReqParameters: stdParams,
1044+
requestTopology: topology,
9851045
},
986-
}
1046+
{
1047+
name: "fail invalid source disk volume id format",
1048+
volumeOnCloud: false,
1049+
expErrCode: codes.InvalidArgument,
1050+
sourceVolumeID: testZonalVolumeSourceID + "/invalid/format",
1051+
reqParameters: stdParams,
1052+
sourceReqParameters: stdParams,
1053+
requestTopology: topology,
1054+
}}
9871055

9881056
for _, tc := range testCases {
9891057
t.Logf("test case: %s", tc.name)
@@ -1001,18 +1069,15 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
10011069
},
10021070
},
10031071
},
1072+
AccessibilityRequirements: tc.requestTopology,
10041073
}
10051074

10061075
sourceVolumeRequest := &csi.CreateVolumeRequest{
1007-
Name: testSourceVolumeName,
1008-
CapacityRange: stdCapRange,
1009-
VolumeCapabilities: stdVolCaps,
1010-
Parameters: tc.sourceReqParameters,
1011-
}
1012-
1013-
if tc.topology != nil {
1014-
// req.AccessibilityRequirements = tc.topology
1015-
sourceVolumeRequest.AccessibilityRequirements = tc.topology
1076+
Name: testSourceVolumeName,
1077+
CapacityRange: stdCapRange,
1078+
VolumeCapabilities: stdVolCaps,
1079+
Parameters: tc.sourceReqParameters,
1080+
AccessibilityRequirements: tc.sourceTopology,
10161081
}
10171082

10181083
if tc.volumeOnCloud {
@@ -1045,7 +1110,6 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
10451110

10461111
// Make sure the response has the source volume.
10471112
sourceVolume := resp.GetVolume()
1048-
t.Logf("response has source volume: %v ", sourceVolume)
10491113
if sourceVolume.ContentSource == nil || sourceVolume.ContentSource.Type == nil ||
10501114
sourceVolume.ContentSource.GetVolume() == nil || sourceVolume.ContentSource.GetVolume().VolumeId == "" {
10511115
t.Fatalf("Expected volume content source to have volume ID, got none")

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

+31-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package gceGCEDriver
1919
import (
2020
"errors"
2121
"fmt"
22+
"strings"
2223

2324
"context"
2425

@@ -28,7 +29,8 @@ import (
2829
)
2930

3031
const (
31-
fsTypeXFS = "xfs"
32+
fsTypeXFS = "xfs"
33+
cryptoKeyVerDelimiter = "/cryptoKeyVersions"
3234
)
3335

3436
var ProbeCSIFullMethod = "/csi.v1.Identity/Probe"
@@ -199,3 +201,31 @@ func collectMountOptions(fsType string, mntFlags []string) []string {
199201
}
200202
return options
201203
}
204+
205+
// kmsKeyEqual returns true if fetchedKMSKey and storageClassKMSKey refer to the same key.
206+
// fetchedKMSKey - key returned by the server
207+
// example: projects/{0}/locations/{1}/keyRings/{2}/cryptoKeys/{3}/cryptoKeyVersions/{4}
208+
// storageClassKMSKey - key as provided by the client
209+
// example: projects/{0}/locations/{1}/keyRings/{2}/cryptoKeys/{3}
210+
// cryptoKeyVersions should be disregarded if the rest of the key is identical.
211+
func kmsKeyEqual(fetchedKMSKey, storageClassKMSKey string) bool {
212+
return removeCryptoKeyVersion(fetchedKMSKey) == removeCryptoKeyVersion(storageClassKMSKey)
213+
}
214+
215+
func removeCryptoKeyVersion(kmsKey string) string {
216+
i := strings.LastIndex(kmsKey, cryptoKeyVerDelimiter)
217+
if i > 0 {
218+
return kmsKey[:i]
219+
}
220+
return kmsKey
221+
}
222+
223+
func contains(s []string, str string) bool {
224+
for _, v := range s {
225+
if v == str {
226+
return true
227+
}
228+
}
229+
230+
return false
231+
}

‎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)