Skip to content

Commit df60f26

Browse files
amacaskillamacaskill
committed
parent 19d670f
author Alexis MacAskill <[email protected]> 1634669949 +0000 committer Alexis MacAskill <[email protected]> 1636674064 +0000 added volume cloning add custom timeout for cloning
1 parent 19d670f commit df60f26

File tree

6 files changed

+111
-52
lines changed

6 files changed

+111
-52
lines changed

‎deploy/kubernetes/images/prow-gke-release-staging-rc-master/image.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,5 @@ metadata:
5353
imageTag:
5454
name: k8s.gcr.io/cloud-provider-gcp/gcp-compute-persistent-disk-csi-driver
5555
newName: gcr.io/k8s-staging-cloud-provider-gcp/gcp-compute-persistent-disk-csi-driver
56-
newTag: "v1.3.2-rc1"
56+
newTag: "v1.3.4-rc1"
5757
---

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (d *CloudDisk) GetSnapshotId() string {
177177
}
178178
}
179179

180-
func (d *CloudDisk) GetDiskId() string {
180+
func (d *CloudDisk) GetSourceDiskId() string {
181181
switch {
182182
case d.disk != nil:
183183
return d.disk.SourceDiskId

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

+9-15
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,8 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
179179
if err != nil {
180180
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk %v had error checking ready status: %v", volKey, err))
181181
}
182-
183182
if !ready {
184-
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk %v is not ready", volKey))
183+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume existing disk %v is not ready.", volKey))
185184
}
186185

187186
// If there is no validation error, immediately return success
@@ -207,39 +206,35 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
207206

208207
if content.GetVolume() != nil {
209208
volumeContentSourceVolumeID = content.GetVolume().GetVolumeId()
210-
211209
// Verify that the source VolumeID is in the correct format.
212-
project, volKey, err := common.VolumeIDToKey(volumeContentSourceVolumeID)
210+
project, sourceVolKey, err := common.VolumeIDToKey(volumeContentSourceVolumeID)
213211
if err != nil {
214-
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("CreateVolume source volume ID is invalid: %v", err))
212+
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("CreateVolume source volume id is invalid: %v", err))
215213
}
214+
216215
// Verify that the volume in VolumeContentSource exists.
217-
diskFromSourceVolume, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gceAPIVersion)
216+
diskFromSourceVolume, err := gceCS.CloudProvider.GetDisk(ctx, project, sourceVolKey, gceAPIVersion)
218217
if err != nil {
219218
if gce.IsGCEError(err, "notFound") {
220219
return nil, status.Errorf(codes.NotFound, "CreateVolume source volume %s does not exist", volumeContentSourceVolumeID)
221220
} else {
222221
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume unknown get disk error when validating: %v", err))
223222
}
224223
}
225-
226-
// Verify the zone, region, and disk type of the clone must be the same as that of the source disk.
224+
// Verify the zone, region, and disk type of the clone must be the same as that of the source disk.
227225
if err := gce.ValidateDiskParameters(diskFromSourceVolume, params); err != nil {
228226
return nil, status.Errorf(codes.InvalidArgument, `CreateVolume source volume parameters do not match CreateVolumeRequest Parameters: %v`, err)
229227
}
230-
231228
// Verify the source disk is ready.
232229
if err == nil {
233230
ready, err := isDiskReady(diskFromSourceVolume)
234231
if err != nil {
235-
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk from source volume %v had error checking ready status: %v", volKey, err))
232+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk from source volume %v had error checking ready status: %v", sourceVolKey, err))
236233
}
237234
if !ready {
238-
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk from source volume %v is not ready", volKey))
235+
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume disk from source volume %v is not ready", sourceVolKey))
239236
}
240237
}
241-
//TODO does this go inside of err == nil?
242-
243238
}
244239
}
245240
// Create the disk
@@ -1089,7 +1084,7 @@ func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string) *csi.Crea
10891084
},
10901085
}
10911086
snapshotID := disk.GetSnapshotId()
1092-
diskID := disk.GetDiskId()
1087+
diskID := disk.GetSourceDiskId()
10931088
if diskID != "" || snapshotID != "" {
10941089
contentSource := &csi.VolumeContentSource{}
10951090
if snapshotID != "" {
@@ -1101,7 +1096,6 @@ func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string) *csi.Crea
11011096
},
11021097
}
11031098
}
1104-
diskID := disk.GetDiskId()
11051099
if diskID != "" {
11061100
contentSource = &csi.VolumeContentSource{
11071101
Type: &csi.VolumeContentSource_Volume{

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

+82-34
Original file line numberDiff line numberDiff line change
@@ -902,52 +902,90 @@ func TestCreateVolumeWithVolumeSourceFromSnapshot(t *testing.T) {
902902

903903
func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
904904
testSourceVolumeName := "test-volume-source-name"
905-
testVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, zone, testSourceVolumeName)
905+
testZonalVolumeSourceID := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, zone, testSourceVolumeName)
906+
testRegionalVolumeSourceID := fmt.Sprintf("projects/%s/regions/%s/disks/%s", project, region, testSourceVolumeName)
906907
testVolumeSourceIDDifferentZone := fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, "different-zone", testSourceVolumeName)
908+
topology := &csi.TopologyRequirement{
909+
Requisite: []*csi.Topology{
910+
{
911+
Segments: map[string]string{common.TopologyKeyZone: region + "-b"},
912+
},
913+
{
914+
Segments: map[string]string{common.TopologyKeyZone: region + "-c"},
915+
},
916+
},
917+
}
918+
regionalParams := map[string]string{
919+
common.ParameterKeyType: "test-type", common.ParameterKeyReplicationType: "regional-pd",
920+
}
907921
// Define test cases
908922
testCases := []struct {
909-
name string
910-
project string
911-
volKey *meta.Key
912-
volumeOnCloud bool
913-
expErrCode codes.Code
914-
sourceVolumeID string
915-
parameters map[string]string
923+
name string
924+
volumeOnCloud bool
925+
expErrCode codes.Code
926+
sourceVolumeID string
927+
reqParameters map[string]string
928+
sourceReqParameters map[string]string
929+
topology *csi.TopologyRequirement
916930
}{
917931
{
918-
name: "success with data source of volume type",
919-
project: "test-project",
920-
volKey: meta.ZonalKey("my-disk", zone),
921-
volumeOnCloud: true,
922-
sourceVolumeID: testVolumeSourceID,
932+
name: "success with data source of zonal volume type",
933+
volumeOnCloud: true,
934+
sourceVolumeID: testZonalVolumeSourceID,
935+
reqParameters: stdParams,
936+
sourceReqParameters: stdParams,
923937
},
924938
{
925-
name: "fail with data source of volume type that doesn't exist",
926-
project: "test-project",
927-
volKey: meta.ZonalKey("my-disk", zone),
928-
volumeOnCloud: false,
929-
expErrCode: codes.NotFound,
930-
sourceVolumeID: testVolumeSourceID,
939+
name: "success with data source of regional volume type",
940+
volumeOnCloud: true,
941+
sourceVolumeID: testRegionalVolumeSourceID,
942+
reqParameters: regionalParams,
943+
sourceReqParameters: regionalParams,
944+
topology: topology,
931945
},
932946
{
933-
name: "fail with data source of volume type with invalid volume id format",
934-
project: "test-project",
935-
volKey: meta.ZonalKey("my-disk", zone),
936-
volumeOnCloud: false,
937-
expErrCode: codes.InvalidArgument,
938-
sourceVolumeID: testVolumeSourceID + "invalid/format",
947+
name: "fail with with data source of replication-type different from CreateVolumeRequest",
948+
volumeOnCloud: true,
949+
expErrCode: codes.InvalidArgument,
950+
sourceVolumeID: testZonalVolumeSourceID,
951+
reqParameters: stdParams,
952+
sourceReqParameters: regionalParams,
953+
topology: topology,
954+
},
955+
{
956+
name: "fail with data source of zonal volume type that doesn't exist",
957+
volumeOnCloud: false,
958+
expErrCode: codes.NotFound,
959+
sourceVolumeID: testZonalVolumeSourceID,
960+
reqParameters: stdParams,
961+
sourceReqParameters: stdParams,
962+
},
963+
{
964+
name: "fail with data source of zonal volume type with invalid volume id format",
965+
volumeOnCloud: false,
966+
expErrCode: codes.InvalidArgument,
967+
sourceVolumeID: testZonalVolumeSourceID + "invalid/format",
968+
reqParameters: stdParams,
969+
sourceReqParameters: stdParams,
939970
},
940971
{
941-
name: "fail with data source of volume type with invalid disk parameters",
942-
project: "test-project",
943-
volKey: meta.ZonalKey("my-disk", zone),
972+
name: "fail with data source of zonal volume type with invalid disk parameters",
944973
volumeOnCloud: true,
945974
expErrCode: codes.InvalidArgument,
946975
sourceVolumeID: testVolumeSourceIDDifferentZone,
947-
parameters: map[string]string{
976+
reqParameters: stdParams,
977+
sourceReqParameters: map[string]string{
948978
common.ParameterKeyType: "different-type",
949979
},
950980
},
981+
{
982+
name: "fail with data source of zonal volume type with invalid replication type",
983+
volumeOnCloud: true,
984+
expErrCode: codes.InvalidArgument,
985+
sourceVolumeID: testZonalVolumeSourceID,
986+
reqParameters: regionalParams,
987+
sourceReqParameters: stdParams,
988+
},
951989
}
952990

953991
for _, tc := range testCases {
@@ -958,7 +996,7 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
958996
Name: name,
959997
CapacityRange: stdCapRange,
960998
VolumeCapabilities: stdVolCaps,
961-
Parameters: stdParams,
999+
Parameters: tc.reqParameters,
9621000
VolumeContentSource: &csi.VolumeContentSource{
9631001
Type: &csi.VolumeContentSource_Volume{
9641002
Volume: &csi.VolumeContentSource_VolumeSource{
@@ -972,16 +1010,26 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
9721010
Name: testSourceVolumeName,
9731011
CapacityRange: stdCapRange,
9741012
VolumeCapabilities: stdVolCaps,
975-
Parameters: stdParams,
1013+
Parameters: tc.sourceReqParameters,
9761014
}
977-
if tc.parameters != nil {
978-
sourceVolumeRequest.Parameters = tc.parameters
1015+
1016+
if tc.topology != nil {
1017+
// req.AccessibilityRequirements = tc.topology
1018+
sourceVolumeRequest.AccessibilityRequirements = tc.topology
9791019
}
9801020

9811021
if tc.volumeOnCloud {
9821022
// Create the source volume.
983-
gceDriver.cs.CreateVolume(context.Background(), sourceVolumeRequest)
1023+
sourceVolume, _ := gceDriver.cs.CreateVolume(context.Background(), sourceVolumeRequest)
1024+
req.VolumeContentSource = &csi.VolumeContentSource{
1025+
Type: &csi.VolumeContentSource_Volume{
1026+
Volume: &csi.VolumeContentSource_VolumeSource{
1027+
VolumeId: sourceVolume.GetVolume().VolumeId,
1028+
},
1029+
},
1030+
}
9841031
}
1032+
9851033
resp, err := gceDriver.cs.CreateVolume(context.Background(), req)
9861034
t.Logf("response: %v err: %v", resp, err)
9871035
if err != nil {

‎test/k8s-integration/config/test-config-template.in

+5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ StorageClass:
44
SnapshotClass:
55
FromFile: {{ .SnapshotClassFile }}
66
{{end}}
7+
{{if .Timeouts}}
8+
Timeouts:
9+
{{ range $key, $value := .Timeouts }}{{ $key }}: {{ $value }}
10+
{{ end }}
11+
{{end}}
712
DriverInfo:
813
Name: csi-gcepd-{{.StorageClass}}
914
SupportedFsType:

‎test/k8s-integration/driver-config.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,18 @@ type driverConfig struct {
1717
SupportedFsType []string
1818
MinimumVolumeSize string
1919
NumAllowedTopologies int
20+
Timeouts map[string]string
2021
}
2122

2223
const (
2324
testConfigDir = "test/k8s-integration/config"
2425
configTemplateFile = "test-config-template.in"
2526
configFile = "test-config.yaml"
27+
// configurable timeouts for the k8s e2e testsuites.
28+
dataSourceProvisionTimeout = "480s"
29+
30+
// These are keys for the configurable timeout map.
31+
dataSourceProvisionTimeoutKey = "DataSourceProvision"
2632
)
2733

2834
// generateDriverConfigFile loads a testdriver config template and creates a file
@@ -76,7 +82,6 @@ func generateDriverConfigFile(testParams *testParameters, storageClassFile strin
7682
}
7783

7884
/* Unsupported Capabilities:
79-
pvcDataSource
8085
RWX
8186
volumeLimits # PD Supports volume limits but test is very slow
8287
singleNodeVolume
@@ -117,12 +122,18 @@ func generateDriverConfigFile(testParams *testParameters, storageClassFile strin
117122
absSnapshotClassFilePath = filepath.Join(testParams.pkgDir, testConfigDir, testParams.snapshotClassFile)
118123
}
119124

125+
caps = append(caps, "pvcDataSource")
120126
minimumVolumeSize := "5Gi"
121127
numAllowedTopologies := 1
122128
if storageClassFile == regionalPDStorageClass {
123129
minimumVolumeSize = "200Gi"
124130
numAllowedTopologies = 2
125131
}
132+
// GCE PD CSI Driver takes in the order of minutes to provision storage with a pvc data source provisioned,
133+
// and with dynamic provisioning (WaitForFirstCustomer policy), some e2e tests need a longer pod start timeout.
134+
timeouts := map[string]string{
135+
dataSourceProvisionTimeoutKey: dataSourceProvisionTimeout,
136+
}
126137
params := driverConfig{
127138
StorageClassFile: filepath.Join(testParams.pkgDir, testConfigDir, storageClassFile),
128139
StorageClass: storageClassFile[:strings.LastIndex(storageClassFile, ".")],
@@ -131,6 +142,7 @@ func generateDriverConfigFile(testParams *testParameters, storageClassFile strin
131142
Capabilities: caps,
132143
MinimumVolumeSize: minimumVolumeSize,
133144
NumAllowedTopologies: numAllowedTopologies,
145+
Timeouts: timeouts,
134146
}
135147

136148
// Write config file

0 commit comments

Comments
 (0)