Skip to content

Commit 1dfb01c

Browse files
pwschuurmanpwschuurman
committed
Add support for multi-zone provisioning
1 parent 4d6be71 commit 1dfb01c

File tree

14 files changed

+1690
-63
lines changed

14 files changed

+1690
-63
lines changed

‎cmd/gce-pd-csi-driver/main.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ var (
8484
useInstanceAPIForListVolumesPublishedNodesFlag = flag.Bool("use-instance-api-to-list-volumes-published-nodes", false, "Enables using the instances.list API to determine published_node_ids in ListVolumes. When false (default), the disks.list API is used")
8585
instancesListFiltersFlag = flag.String("instances-list-filters", "", "Comma separated list of filters to use when calling the instances.list API. By default instances.list fetches all instances in a region")
8686

87+
setAccessModeGCEAPIVersion gce.GCEAPIVersion = gce.GCEAPIVersionV1
88+
8789
extraTagsStr = flag.String("extra-tags", "", "Extra tags to attach to each Compute Disk, Image, Snapshot created. It is a comma separated list of parent id, key and value like '<parent_id1>/<tag_key1>/<tag_value1>,...,<parent_idN>/<tag_keyN>/<tag_valueN>'. parent_id is the Organization or the Project ID or Project name where the tag key and the tag value resources exist. A maximum of 50 tags bindings is allowed for a resource. See https://cloud.google.com/resource-manager/docs/tags/tags-overview, https://cloud.google.com/resource-manager/docs/tags/tags-creating-and-managing for details")
8890

8991
version string
@@ -101,6 +103,7 @@ func init() {
101103
// Use V(6) for extra repeated/polling information
102104
stringEnumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment")
103105
urlFlag(&computeEndpoint, "compute-endpoint", "Compute endpoint")
106+
stringEnumFlag(&setAccessModeGCEAPIVersion, "set-access-mode-gce-api-version", gce.GCEAPIVersions, "API version that is used when calling disks.updateto set disk access mode")
104107
klog.InitFlags(flag.CommandLine)
105108
flag.Set("logtostderr", "true")
106109
}
@@ -200,10 +203,15 @@ func handle() {
200203
UseInstancesAPIForPublishedNodes: *useInstanceAPIForListVolumesPublishedNodesFlag,
201204
}
202205

206+
// Initialized setAccessModeConfig
207+
setAccessModeConfig := gce.SetAccessModeConfig{
208+
DisksUpdateAPIVersion: setAccessModeGCEAPIVersion,
209+
}
210+
203211
// Initialize requirements for the controller service
204212
var controllerServer *driver.GCEControllerServer
205213
if *runControllerService {
206-
cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, computeEndpoint, computeEnvironment, waitForAttachConfig, listInstancesConfig)
214+
cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, computeEndpoint, computeEnvironment, waitForAttachConfig, listInstancesConfig, setAccessModeConfig)
207215
if err != nil {
208216
klog.Fatalf("Failed to get cloud provider: %v", err.Error())
209217
}

‎pkg/common/parameters.go

+30-3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const (
3333
ParameterKeyEnableConfidentialCompute = "enable-confidential-storage"
3434
ParameterKeyStoragePools = "storage-pools"
3535
ParameterKeyResourceTags = "resource-tags"
36+
ParameterKeyEnableMultiZoneProvisioning = "enable-multi-zone-provisioning"
3637

3738
// Parameters for VolumeSnapshotClass
3839
ParameterKeyStorageLocations = "storage-locations"
@@ -66,6 +67,10 @@ const (
6667
tagKeyCreatedForSnapshotName = "kubernetes.io/created-for/volumesnapshot/name"
6768
tagKeyCreatedForSnapshotNamespace = "kubernetes.io/created-for/volumesnapshot/namespace"
6869
tagKeyCreatedForSnapshotContentName = "kubernetes.io/created-for/volumesnapshotcontent/name"
70+
71+
// Label that is set on a disk when it is used by a 'multi-zone' VolumeHandle
72+
// TODO: Keep this in sync with the utils.go
73+
multiZoneLabel = "goog-gke-multi-zone"
6974
)
7075

7176
// DiskParameters contains normalized and defaulted disk parameters
@@ -102,6 +107,9 @@ type DiskParameters struct {
102107
// Values: {map[string]string}
103108
// Default: ""
104109
ResourceTags map[string]string
110+
// Values: {bool}
111+
// Default: false
112+
MultiZoneProvisioning bool
105113
}
106114

107115
// SnapshotParameters contains normalized and defaulted parameters for snapshots
@@ -121,11 +129,17 @@ type StoragePool struct {
121129
ResourceName string
122130
}
123131

132+
type ParameterProcessor struct {
133+
DriverName string
134+
EnableStoragePools bool
135+
EnableMultiZone bool
136+
}
137+
124138
// ExtractAndDefaultParameters will take the relevant parameters from a map and
125139
// put them into a well defined struct making sure to default unspecified fields.
126140
// extraVolumeLabels are added as labels; if there are also labels specified in
127141
// parameters, any matching extraVolumeLabels will be overridden.
128-
func ExtractAndDefaultParameters(parameters map[string]string, driverName string, extraVolumeLabels map[string]string, enableStoragePools bool, extraTags map[string]string) (DiskParameters, error) {
142+
func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]string, extraVolumeLabels map[string]string, extraTags map[string]string) (DiskParameters, error) {
129143
p := DiskParameters{
130144
DiskType: "pd-standard", // Default
131145
ReplicationType: replicationTypeNone, // Default
@@ -210,7 +224,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
210224

211225
p.EnableConfidentialCompute = paramEnableConfidentialCompute
212226
case ParameterKeyStoragePools:
213-
if !enableStoragePools {
227+
if !pp.EnableStoragePools {
214228
return p, fmt.Errorf("parameters contains invalid option %q", ParameterKeyStoragePools)
215229
}
216230
storagePools, err := ParseStoragePools(v)
@@ -222,12 +236,25 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
222236
if err := extractResourceTagsParameter(v, p.ResourceTags); err != nil {
223237
return p, err
224238
}
239+
case ParameterKeyEnableMultiZoneProvisioning:
240+
if !pp.EnableMultiZone {
241+
return p, fmt.Errorf("parameters contains invalid option %q", ParameterKeyEnableMultiZoneProvisioning)
242+
}
243+
paramEnableMultiZoneProvisioning, err := ConvertStringToBool(v)
244+
if err != nil {
245+
return p, fmt.Errorf("parameters contain invalid value for %s parameter: %w", ParameterKeyEnableMultiZoneProvisioning, err)
246+
}
247+
248+
p.MultiZoneProvisioning = paramEnableMultiZoneProvisioning
249+
if paramEnableMultiZoneProvisioning {
250+
p.Labels[multiZoneLabel] = "true"
251+
}
225252
default:
226253
return p, fmt.Errorf("parameters contains invalid option %q", k)
227254
}
228255
}
229256
if len(p.Tags) > 0 {
230-
p.Tags[tagKeyCreatedBy] = driverName
257+
p.Tags[tagKeyCreatedBy] = pp.DriverName
231258
}
232259
return p, nil
233260
}

‎pkg/common/parameters_test.go

+45-2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
2929
parameters map[string]string
3030
labels map[string]string
3131
enableStoragePools bool
32+
enableMultiZone bool
3233
extraTags map[string]string
3334
expectParams DiskParameters
3435
expectErr bool
@@ -350,19 +351,61 @@ func TestExtractAndDefaultParameters(t *testing.T) {
350351
labels: map[string]string{},
351352
expectErr: true,
352353
},
354+
{
355+
name: "multi-zone-enable parameters, multi-zone label is set, multi-zone feature enabled",
356+
parameters: map[string]string{ParameterKeyType: "hyperdisk-ml", ParameterKeyEnableMultiZoneProvisioning: "true"},
357+
labels: map[string]string{multiZoneLabel: "true"},
358+
enableMultiZone: true,
359+
expectParams: DiskParameters{
360+
DiskType: "hyperdisk-ml",
361+
ReplicationType: "none",
362+
Tags: map[string]string{},
363+
Labels: map[string]string{multiZoneLabel: "true"},
364+
ResourceTags: map[string]string{},
365+
MultiZoneProvisioning: true,
366+
},
367+
},
368+
{
369+
name: "multi-zone-enable parameters, multi-zone label is false, multi-zone feature enabled",
370+
parameters: map[string]string{ParameterKeyType: "hyperdisk-ml", ParameterKeyEnableMultiZoneProvisioning: "false"},
371+
enableMultiZone: true,
372+
expectParams: DiskParameters{
373+
DiskType: "hyperdisk-ml",
374+
ReplicationType: "none",
375+
Tags: map[string]string{},
376+
ResourceTags: map[string]string{},
377+
Labels: map[string]string{},
378+
},
379+
},
380+
{
381+
name: "multi-zone-enable parameters, invalid value, multi-zone feature enabled",
382+
parameters: map[string]string{ParameterKeyType: "hyperdisk-ml", ParameterKeyEnableMultiZoneProvisioning: "unknown"},
383+
enableMultiZone: true,
384+
expectErr: true,
385+
},
386+
{
387+
name: "multi-zone-enable parameters, multi-zone label is set, multi-zone feature disabled",
388+
parameters: map[string]string{ParameterKeyType: "hyperdisk-ml", ParameterKeyEnableMultiZoneProvisioning: "true"},
389+
expectErr: true,
390+
},
353391
}
354392

355393
for _, tc := range tests {
356394
t.Run(tc.name, func(t *testing.T) {
357-
p, err := ExtractAndDefaultParameters(tc.parameters, "testDriver", tc.labels, tc.enableStoragePools, tc.extraTags)
395+
pp := ParameterProcessor{
396+
DriverName: "testDriver",
397+
EnableStoragePools: tc.enableStoragePools,
398+
EnableMultiZone: tc.enableMultiZone,
399+
}
400+
p, err := pp.ExtractAndDefaultParameters(tc.parameters, tc.labels, tc.extraTags)
358401
if gotErr := err != nil; gotErr != tc.expectErr {
359402
t.Fatalf("ExtractAndDefaultParameters(%+v) = %v; expectedErr: %v", tc.parameters, err, tc.expectErr)
360403
}
361404
if err != nil {
362405
return
363406
}
364407

365-
if diff := cmp.Diff(p, tc.expectParams); diff != "" {
408+
if diff := cmp.Diff(tc.expectParams, p); diff != "" {
366409
t.Errorf("ExtractAndDefaultParameters(%+v): -want, +got \n%s", tc.parameters, diff)
367410
}
368411
})

‎pkg/common/utils.go

+14
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ const (
7373
// Full or partial URL of the machine type resource, in the format:
7474
// zones/zone/machineTypes/machine-type
7575
machineTypePattern = "zones/[^/]+/machineTypes/([^/]+)$"
76+
77+
// Full or partial URL of the zone resource, in the format:
78+
// projects/{project}/zones/{zone}
79+
zoneURIPattern = "projects/[^/]+/zones/([^/]+)$"
7680
)
7781

7882
var (
@@ -85,6 +89,8 @@ var (
8589

8690
storagePoolFieldsRegex = regexp.MustCompile(`^projects/([^/]+)/zones/([^/]+)/storagePools/([^/]+)$`)
8791

92+
zoneURIRegex = regexp.MustCompile(zoneURIPattern)
93+
8894
// userErrorCodeMap tells how API error types are translated to error codes.
8995
userErrorCodeMap = map[int]codes.Code{
9096
http.StatusForbidden: codes.PermissionDenied,
@@ -556,6 +562,14 @@ func isValidDiskEncryptionKmsKey(DiskEncryptionKmsKey string) bool {
556562
return kmsKeyPattern.MatchString(DiskEncryptionKmsKey)
557563
}
558564

565+
func ParseZoneFromURI(zoneURI string) (string, error) {
566+
zoneMatch := zoneURIRegex.FindStringSubmatch(zoneURI)
567+
if zoneMatch == nil {
568+
return "", fmt.Errorf("failed to parse zone URI. Expected projects/{project}/zones/{zone}. Got: %s", zoneURI)
569+
}
570+
return zoneMatch[1], nil
571+
}
572+
559573
// ParseStoragePools returns an error if none of the given storagePools
560574
// (delimited by a comma) are in the format
561575
// projects/project/zones/zone/storagePools/storagePool.

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

+11
Original file line numberDiff line numberDiff line change
@@ -256,3 +256,14 @@ func (d *CloudDisk) GetLabels() map[string]string {
256256
return nil
257257
}
258258
}
259+
260+
func (d *CloudDisk) GetAccessMode() string {
261+
switch {
262+
case d.disk != nil:
263+
return d.disk.AccessMode
264+
case d.betaDisk != nil:
265+
return d.betaDisk.AccessMode
266+
default:
267+
return ""
268+
}
269+
}

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

+34-9
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,11 @@ func CreateFakeCloudProvider(project, zone string, cloudDisks []*CloudDisk) (*Fa
7979
mockDiskStatus: "READY",
8080
}
8181
for _, d := range cloudDisks {
82-
fcp.disks[d.GetName()] = d
82+
diskZone := d.GetZone()
83+
if diskZone == "" {
84+
diskZone = zone
85+
}
86+
fcp.disks[meta.ZonalKey(d.GetName(), diskZone).String()] = d
8387
}
8488
return fcp, nil
8589
}
@@ -101,8 +105,8 @@ func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Contex
101105
if volumeKey.Zone != common.UnspecifiedValue {
102106
return project, volumeKey, nil
103107
}
104-
for name, d := range cloud.disks {
105-
if name == volumeKey.Name {
108+
for diskVolKey, d := range cloud.disks {
109+
if diskVolKey == volumeKey.String() {
106110
volumeKey.Zone = d.GetZone()
107111
return project, volumeKey, nil
108112
}
@@ -127,6 +131,11 @@ func (cloud *FakeCloudProvider) ListZones(ctx context.Context, region string) ([
127131
return []string{cloud.zone, "country-region-fakesecondzone"}, nil
128132
}
129133

134+
func (cloud *FakeCloudProvider) ListCompatibleDiskTypeZones(ctx context.Context, project string, zones []string, diskType string) ([]string, error) {
135+
// Assume all zones are compatible
136+
return zones, nil
137+
}
138+
130139
func (cloud *FakeCloudProvider) ListDisks(ctx context.Context, fields []googleapi.Field) ([]*computev1.Disk, string, error) {
131140
d := []*computev1.Disk{}
132141
for _, cd := range cloud.disks {
@@ -167,7 +176,7 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string
167176

168177
// Disk Methods
169178
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) {
170-
disk, ok := cloud.disks[volKey.Name]
179+
disk, ok := cloud.disks[volKey.String()]
171180
if !ok {
172181
return nil, notFoundError()
173182
}
@@ -204,7 +213,7 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *
204213
}
205214

206215
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool) error {
207-
if disk, ok := cloud.disks[volKey.Name]; ok {
216+
if disk, ok := cloud.disks[volKey.String()]; ok {
208217
err := cloud.ValidateExistingDisk(ctx, disk, params,
209218
int64(capacityRange.GetRequiredBytes()),
210219
int64(capacityRange.GetLimitBytes()),
@@ -259,15 +268,15 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, project string,
259268
if containsBetaDiskType(hyperdiskTypes, params.DiskType) {
260269
betaDisk := convertV1DiskToBetaDisk(computeDisk)
261270
betaDisk.EnableConfidentialCompute = params.EnableConfidentialCompute
262-
cloud.disks[volKey.Name] = CloudDiskFromBeta(betaDisk)
271+
cloud.disks[volKey.String()] = CloudDiskFromBeta(betaDisk)
263272
} else {
264-
cloud.disks[volKey.Name] = CloudDiskFromV1(computeDisk)
273+
cloud.disks[volKey.String()] = CloudDiskFromV1(computeDisk)
265274
}
266275
return nil
267276
}
268277

269278
func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, project string, volKey *meta.Key) error {
270-
delete(cloud.disks, volKey.Name)
279+
delete(cloud.disks, volKey.String())
271280
return nil
272281
}
273282

@@ -307,6 +316,22 @@ func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, project, deviceN
307316
return nil
308317
}
309318

319+
func (cloud *FakeCloudProvider) SetDiskAccessMode(ctx context.Context, project string, volKey *meta.Key, accessMode string) error {
320+
disk, ok := cloud.disks[volKey.String()]
321+
if !ok {
322+
return fmt.Errorf("disk %v not found", volKey)
323+
}
324+
325+
if disk.disk != nil {
326+
disk.disk.AccessMode = accessMode
327+
}
328+
if disk.betaDisk != nil {
329+
disk.betaDisk.AccessMode = accessMode
330+
}
331+
332+
return nil
333+
}
334+
310335
func (cloud *FakeCloudProvider) GetDiskTypeURI(project string, volKey *meta.Key, diskType string) string {
311336
switch volKey.Type() {
312337
case meta.Zonal:
@@ -390,7 +415,7 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, project stri
390415
}
391416

392417
func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, project string, volKey *meta.Key, requestBytes int64) (int64, error) {
393-
disk, ok := cloud.disks[volKey.Name]
418+
disk, ok := cloud.disks[volKey.String()]
394419
if !ok {
395420
return -1, notFoundError()
396421
}

0 commit comments

Comments
 (0)