Skip to content

Commit d9cbaf1

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2235 from hajiler/gv-disk-type-selection-branch
Enable dynamic disk type selection
2 parents 3adbb82 + 49d3444 commit d9cbaf1

File tree

12 files changed

+815
-68
lines changed

12 files changed

+815
-68
lines changed

‎pkg/common/utils.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,10 @@ func HasDiskTypeLabelKeyPrefix(labelKey string) bool {
497497
return strings.HasPrefix(labelKey, constants.DiskTypeKeyPrefix)
498498
}
499499

500+
func DiskTypeFromLabel(labelKey string) string {
501+
return strings.TrimPrefix(labelKey, constants.DiskTypeKeyPrefix+"/")
502+
}
503+
500504
func DiskTypeLabelKey(diskType string) string {
501505
return fmt.Sprintf("%s/%s", constants.DiskTypeKeyPrefix, diskType)
502506
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,8 @@ func validAccessMode(want, got string) bool {
475475
// ValidateDiskParameters takes a CloudDisk and returns true if the parameters
476476
// specified validly describe the disk provided, and false otherwise.
477477
func ValidateDiskParameters(disk *CloudDisk, params parameters.DiskParameters) error {
478-
if disk.GetPDType() != params.DiskType {
478+
// Skip this check for dynamic volumes because dynamic is not a valid disk type.
479+
if disk.GetPDType() != params.DiskType && !params.IsDiskDynamic() {
479480
return fmt.Errorf("actual pd type %s did not match the expected param %s", disk.GetPDType(), params.DiskType)
480481
}
481482

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

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ type GCEControllerServer struct {
115115
// If set to true, the CSI Driver will allow volumes to be provisioned with data cache configuration
116116
enableDataCache bool
117117

118+
// If set to true, the CSI Driver will allow volumes to be provisioned with dynamic disk type selection.
119+
enableDynamicVolumes bool
120+
118121
multiZoneVolumeHandleConfig MultiZoneVolumeHandleConfig
119122

120123
listVolumesConfig ListVolumesConfig
@@ -322,6 +325,13 @@ func (gceCS *GCEControllerServer) createVolumeInternal(ctx context.Context, req
322325
// Apply Parameters (case-insensitive). We leave validation of
323326
// the values to the cloud provider.
324327
params, dataCacheParams, err := gceCS.parameterProcessor().ExtractAndDefaultParameters(req.GetParameters())
328+
if gceCS.enableDynamicVolumes && params.IsDiskDynamic() {
329+
params.DiskType, err = SelectDisk(ctx, req, gceCS.CloudProvider)
330+
if err != nil {
331+
return nil, status.Errorf(codes.InvalidArgument, "failed to select disk type: %v", err.Error())
332+
}
333+
parameters.SanitizeDiskParameters(&params)
334+
}
325335
metrics.UpdateRequestMetadataFromParams(ctx, params)
326336
if err != nil {
327337
return nil, status.Errorf(codes.InvalidArgument, "failed to extract parameters: %v", err.Error())
@@ -735,8 +745,11 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi
735745
}
736746
}
737747
// Verify the disk type and encryption key of the clone are the same as that of the source disk.
738-
if diskFromSourceVolume.GetPDType() != params.DiskType || !gce.KmsKeyEqual(diskFromSourceVolume.GetKMSKeyName(), params.DiskEncryptionKMSKey) {
739-
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume Parameters %v do not match source volume Parameters", params)
748+
if diskFromSourceVolume.GetPDType() != params.DiskType {
749+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume source volume disk type %q must match new volume %q", diskFromSourceVolume.GetPDType(), params.DiskType)
750+
}
751+
if !gce.KmsKeyEqual(diskFromSourceVolume.GetKMSKeyName(), params.DiskEncryptionKMSKey) {
752+
return nil, status.Errorf(codes.InvalidArgument, "CreateVolume source volume KMS key %q must match new volume %q", diskFromSourceVolume.GetKMSKeyName(), params.DiskEncryptionKMSKey)
740753
}
741754
// Verify the disk capacity range are the same or greater as that of the source disk.
742755
if diskFromSourceVolume.GetSizeGb() > common.BytesToGbRoundDown(capBytes) {
@@ -1345,14 +1358,15 @@ func (gceCS *GCEControllerServer) executeControllerUnpublishVolume(ctx context.C
13451358

13461359
func (gceCS *GCEControllerServer) parameterProcessor() *parameters.ParameterProcessor {
13471360
return &parameters.ParameterProcessor{
1348-
DriverName: gceCS.Driver.name,
1349-
EnableStoragePools: gceCS.enableStoragePools,
1350-
EnableMultiZone: gceCS.multiZoneVolumeHandleConfig.Enable,
1351-
EnableHdHA: gceCS.enableHdHA,
1352-
EnableDiskTopology: gceCS.EnableDiskTopology,
1353-
ExtraVolumeLabels: gceCS.Driver.extraVolumeLabels,
1354-
EnableDataCache: gceCS.enableDataCache,
1355-
ExtraTags: gceCS.Driver.extraTags,
1361+
DriverName: gceCS.Driver.name,
1362+
EnableStoragePools: gceCS.enableStoragePools,
1363+
EnableMultiZone: gceCS.multiZoneVolumeHandleConfig.Enable,
1364+
EnableHdHA: gceCS.enableHdHA,
1365+
EnableDiskTopology: gceCS.EnableDiskTopology,
1366+
ExtraVolumeLabels: gceCS.Driver.extraVolumeLabels,
1367+
EnableDataCache: gceCS.enableDataCache,
1368+
ExtraTags: gceCS.Driver.extraTags,
1369+
EnableDynamicVolumes: gceCS.enableDynamicVolumes,
13561370
}
13571371
}
13581372

@@ -2326,19 +2340,12 @@ func getZonesFromTopology(topList []*csi.Topology) ([]string, error) {
23262340
}
23272341

23282342
func getZoneFromSegment(seg map[string]string) (string, error) {
2329-
var zone string
23302343
for k, v := range seg {
2331-
switch k {
2332-
case constants.TopologyKeyZone:
2333-
zone = v
2334-
default:
2335-
return "", fmt.Errorf("topology segment has unknown key %v", k)
2344+
if k == constants.TopologyKeyZone {
2345+
return v, nil
23362346
}
23372347
}
2338-
if len(zone) == 0 {
2339-
return "", fmt.Errorf("topology specified but could not find zone in segment: %v", seg)
2340-
}
2341-
return zone, nil
2348+
return "", fmt.Errorf("topology specified but could not find zone in segment: %v", seg)
23422349
}
23432350

23442351
func (gceCS *GCEControllerServer) pickZones(ctx context.Context, top *csi.TopologyRequirement, numZones int, locationTopReq *locationRequirements) ([]string, error) {

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

Lines changed: 149 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -595,12 +595,13 @@ func TestListSnapshotsArguments(t *testing.T) {
595595

596596
func TestCreateVolumeArguments(t *testing.T) {
597597
testCases := []struct {
598-
name string
599-
req *csi.CreateVolumeRequest
600-
enableStoragePools bool
601-
expVol *csi.Volume
602-
expErrCode codes.Code
603-
enableDiskTopology bool
598+
name string
599+
req *csi.CreateVolumeRequest
600+
enableStoragePools bool
601+
expVol *csi.Volume
602+
expErrCode codes.Code
603+
enableDiskTopology bool
604+
enableDynamicVolumes bool
604605
}{
605606
{
606607
name: "success default",
@@ -784,23 +785,6 @@ func TestCreateVolumeArguments(t *testing.T) {
784785
},
785786
},
786787
},
787-
{
788-
name: "fail with extra topology",
789-
req: &csi.CreateVolumeRequest{
790-
Name: "test-name",
791-
CapacityRange: stdCapRange,
792-
VolumeCapabilities: stdVolCaps,
793-
Parameters: stdParams,
794-
AccessibilityRequirements: &csi.TopologyRequirement{
795-
Requisite: []*csi.Topology{
796-
{
797-
Segments: map[string]string{"ooblezoners": "topology-zone", constants.TopologyKeyZone: "top-zone"},
798-
},
799-
},
800-
},
801-
},
802-
expErrCode: codes.InvalidArgument,
803-
},
804788
{
805789
name: "fail with missing topology zone",
806790
req: &csi.CreateVolumeRequest{
@@ -1385,13 +1369,154 @@ func TestCreateVolumeArguments(t *testing.T) {
13851369
},
13861370
},
13871371
},
1372+
{
1373+
name: "success with dynamic volumes enabled but standard type",
1374+
req: &csi.CreateVolumeRequest{
1375+
Name: "test-name",
1376+
CapacityRange: stdCapRange,
1377+
VolumeCapabilities: stdVolCaps,
1378+
Parameters: stdParams,
1379+
},
1380+
expVol: &csi.Volume{
1381+
CapacityBytes: common.GbToBytes(20),
1382+
VolumeId: testVolumeID,
1383+
VolumeContext: nil,
1384+
AccessibleTopology: []*csi.Topology{
1385+
{
1386+
Segments: map[string]string{
1387+
constants.TopologyKeyZone: zone,
1388+
},
1389+
},
1390+
},
1391+
},
1392+
},
1393+
{
1394+
name: "success with dynamic volumes selecting disk",
1395+
req: &csi.CreateVolumeRequest{
1396+
Name: "test-name",
1397+
CapacityRange: stdCapRange,
1398+
VolumeCapabilities: stdVolCaps,
1399+
Parameters: map[string]string{
1400+
parameters.ParameterKeyType: parameters.DynamicVolumeType,
1401+
parameters.ParameterPDType: "pd-balanced",
1402+
parameters.ParameterHDType: "hyperdisk-balanced",
1403+
},
1404+
},
1405+
enableDynamicVolumes: true,
1406+
expVol: &csi.Volume{
1407+
CapacityBytes: common.GbToBytes(20),
1408+
VolumeId: testVolumeID,
1409+
VolumeContext: nil,
1410+
AccessibleTopology: []*csi.Topology{
1411+
{
1412+
Segments: map[string]string{
1413+
constants.TopologyKeyZone: zone,
1414+
},
1415+
},
1416+
},
1417+
},
1418+
},
1419+
{
1420+
name: "success defaulting pd-type",
1421+
req: &csi.CreateVolumeRequest{
1422+
Name: "test-name",
1423+
CapacityRange: stdCapRange,
1424+
VolumeCapabilities: stdVolCaps,
1425+
Parameters: map[string]string{
1426+
parameters.ParameterKeyType: parameters.DynamicVolumeType,
1427+
parameters.ParameterHDType: "hyperdisk-balanced",
1428+
},
1429+
},
1430+
enableDynamicVolumes: true,
1431+
expVol: &csi.Volume{
1432+
CapacityBytes: common.GbToBytes(20),
1433+
VolumeId: testVolumeID,
1434+
VolumeContext: nil,
1435+
AccessibleTopology: []*csi.Topology{
1436+
{
1437+
Segments: map[string]string{
1438+
constants.TopologyKeyZone: zone,
1439+
},
1440+
},
1441+
},
1442+
},
1443+
},
1444+
{
1445+
name: "success defaulting hd-type",
1446+
req: &csi.CreateVolumeRequest{
1447+
Name: "test-name",
1448+
CapacityRange: stdCapRange,
1449+
VolumeCapabilities: stdVolCaps,
1450+
Parameters: map[string]string{
1451+
parameters.ParameterKeyType: parameters.DynamicVolumeType,
1452+
parameters.ParameterPDType: "pd-balanced",
1453+
},
1454+
},
1455+
enableDynamicVolumes: true,
1456+
expVol: &csi.Volume{
1457+
CapacityBytes: common.GbToBytes(20),
1458+
VolumeId: testVolumeID,
1459+
VolumeContext: nil,
1460+
AccessibleTopology: []*csi.Topology{
1461+
{
1462+
Segments: map[string]string{
1463+
constants.TopologyKeyZone: zone,
1464+
},
1465+
},
1466+
},
1467+
},
1468+
},
1469+
{
1470+
name: "fail with dynamic volume missing dynamic type",
1471+
req: &csi.CreateVolumeRequest{
1472+
Name: "test-name",
1473+
CapacityRange: stdCapRange,
1474+
VolumeCapabilities: stdVolCaps,
1475+
Parameters: map[string]string{
1476+
parameters.ParameterHDType: "hyperdisk-balanced",
1477+
parameters.ParameterPDType: "pd-balanced",
1478+
},
1479+
},
1480+
enableDynamicVolumes: true,
1481+
expErrCode: codes.InvalidArgument,
1482+
},
1483+
{
1484+
name: "success with dynamic volumes selecting disk override",
1485+
req: &csi.CreateVolumeRequest{
1486+
Name: "test-name",
1487+
CapacityRange: stdCapRange,
1488+
VolumeCapabilities: stdVolCaps,
1489+
Parameters: map[string]string{
1490+
parameters.ParameterKeyType: parameters.DynamicVolumeType,
1491+
parameters.ParameterPDType: "pd-balanced",
1492+
parameters.ParameterHDType: "hyperdisk-balanced",
1493+
parameters.ParameterDiskPreference: parameters.ParameterPDType,
1494+
},
1495+
},
1496+
enableDynamicVolumes: true,
1497+
expVol: &csi.Volume{
1498+
CapacityBytes: common.GbToBytes(20),
1499+
VolumeId: testVolumeID,
1500+
VolumeContext: nil,
1501+
AccessibleTopology: []*csi.Topology{
1502+
{
1503+
Segments: map[string]string{
1504+
constants.TopologyKeyZone: zone,
1505+
},
1506+
},
1507+
},
1508+
},
1509+
},
13881510
}
13891511

13901512
// Run test cases
13911513
for _, tc := range testCases {
13921514
t.Run(tc.name, func(t *testing.T) {
13931515
// Setup new driver each time so no interference
1394-
args := &GCEControllerServerArgs{EnableDiskTopology: tc.enableDiskTopology}
1516+
args := &GCEControllerServerArgs{
1517+
EnableDiskTopology: tc.enableDiskTopology,
1518+
EnableDynamicVolumes: tc.enableDynamicVolumes,
1519+
}
13951520
gceDriver := initGCEDriver(t, nil, args)
13961521
gceDriver.cs.enableStoragePools = tc.enableStoragePools
13971522

@@ -4392,18 +4517,6 @@ func TestGetZonesFromTopology(t *testing.T) {
43924517
topology: []*csi.Topology{},
43934518
expZones: sets.NewString(),
43944519
},
4395-
{
4396-
name: "fail: wrong key inside",
4397-
topology: []*csi.Topology{
4398-
{
4399-
Segments: map[string]string{constants.TopologyKeyZone: "test-zone", "fake-key": "fake-value"},
4400-
},
4401-
{
4402-
Segments: map[string]string{constants.TopologyKeyZone: "test-zone2"},
4403-
},
4404-
},
4405-
expErr: true,
4406-
},
44074520
{
44084521
name: "success: no topology",
44094522
expZones: sets.NewString(),

0 commit comments

Comments
 (0)