Skip to content

Commit 7f5fa35

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #782 from christian-roggia/feature/cross-project-snapshot
Allow cross project snapshots and volumes
2 parents 2d8ee4b + d955f28 commit 7f5fa35

File tree

10 files changed

+273
-231
lines changed

10 files changed

+273
-231
lines changed

‎pkg/common/utils.go‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,12 @@ const (
3939
// Snapshot ID
4040
snapshotTotalElements = 5
4141
snapshotTopologyKey = 2
42+
snapshotProjectKey = 1
4243

4344
// Node ID Expected Format
4445
// "projects/{projectName}/zones/{zoneName}/disks/{diskName}"
4546
nodeIDFmt = "projects/%s/zones/%s/instances/%s"
47+
nodeIDProjectValue = 1
4648
nodeIDZoneValue = 3
4749
nodeIDNameValue = 5
4850
nodeIDTotalElements = 6
@@ -68,17 +70,17 @@ func GbToBytes(Gb int64) int64 {
6870
return Gb * 1024 * 1024 * 1024
6971
}
7072

71-
func VolumeIDToKey(id string) (*meta.Key, error) {
73+
func VolumeIDToKey(id string) (string, *meta.Key, error) {
7274
splitId := strings.Split(id, "/")
7375
if len(splitId) != volIDTotalElements {
74-
return nil, fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id)
76+
return "", nil, fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id)
7577
}
7678
if splitId[volIDToplogyKey] == "zones" {
77-
return meta.ZonalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
79+
return splitId[nodeIDProjectValue], meta.ZonalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
7880
} else if splitId[volIDToplogyKey] == "regions" {
79-
return meta.RegionalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
81+
return splitId[nodeIDProjectValue], meta.RegionalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
8082
} else {
81-
return nil, fmt.Errorf("could not get id components, expected either zones or regions, got: %v", splitId[volIDToplogyKey])
83+
return "", nil, fmt.Errorf("could not get id components, expected either zones or regions, got: %v", splitId[volIDToplogyKey])
8284
}
8385
}
8486

@@ -100,15 +102,15 @@ func GenerateUnderspecifiedVolumeID(diskName string, isZonal bool) string {
100102
return fmt.Sprintf(volIDRegionalFmt, UnspecifiedValue, UnspecifiedValue, diskName)
101103
}
102104

103-
func SnapshotIDToKey(id string) (string, error) {
105+
func SnapshotIDToProjectKey(id string) (string, string, error) {
104106
splitId := strings.Split(id, "/")
105107
if len(splitId) != snapshotTotalElements {
106-
return "", fmt.Errorf("failed to get id components. Expected projects/{project}/global/snapshot/{name}. Got: %s", id)
108+
return "", "", fmt.Errorf("failed to get id components. Expected projects/{project}/global/snapshot/{name}. Got: %s", id)
107109
}
108110
if splitId[snapshotTopologyKey] == "global" {
109-
return splitId[snapshotTotalElements-1], nil
111+
return splitId[snapshotProjectKey], splitId[snapshotTotalElements-1], nil
110112
} else {
111-
return "", fmt.Errorf("could not get id components, expected global, got: %v", splitId[snapshotTopologyKey])
113+
return "", "", fmt.Errorf("could not get id components, expected global, got: %v", splitId[snapshotTopologyKey])
112114
}
113115
}
114116

‎pkg/common/utils_test.go‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,33 @@ func TestVolumeIDToKey(t *testing.T) {
146146
testName := "test-name"
147147
testZone := "test-zone"
148148
testProject := "test-project"
149+
testCrossProject := "test-cross-project"
149150
testRegion := "test-region"
150151

151152
testCases := []struct {
152-
name string
153-
volID string
154-
expKey *meta.Key
155-
expErr bool
153+
name string
154+
volID string
155+
expProject string
156+
expKey *meta.Key
157+
expErr bool
156158
}{
157159
{
158-
name: "normal zonal",
159-
volID: fmt.Sprintf(volIDZoneFmt, testProject, testZone, testName),
160-
expKey: meta.ZonalKey(testName, testZone),
160+
name: "normal zonal",
161+
volID: fmt.Sprintf(volIDZoneFmt, testProject, testZone, testName),
162+
expKey: meta.ZonalKey(testName, testZone),
163+
expProject: testProject,
164+
},
165+
{
166+
name: "cross project",
167+
volID: fmt.Sprintf(volIDZoneFmt, testCrossProject, testZone, testName),
168+
expKey: meta.ZonalKey(testName, testZone),
169+
expProject: testCrossProject,
161170
},
162171
{
163-
name: "normal regional",
164-
volID: fmt.Sprintf(volIDRegionFmt, testProject, testRegion, testName),
165-
expKey: meta.RegionalKey(testName, testRegion),
172+
name: "normal regional",
173+
volID: fmt.Sprintf(volIDRegionFmt, testProject, testRegion, testName),
174+
expKey: meta.RegionalKey(testName, testRegion),
175+
expProject: testProject,
166176
},
167177
{
168178
name: "malformed",
@@ -177,7 +187,7 @@ func TestVolumeIDToKey(t *testing.T) {
177187
}
178188
for _, tc := range testCases {
179189
t.Logf("test case: %s", tc.name)
180-
gotKey, err := VolumeIDToKey(tc.volID)
190+
project, gotKey, err := VolumeIDToKey(tc.volID)
181191
if err == nil && tc.expErr {
182192
t.Errorf("Expected error but got none")
183193
}
@@ -191,6 +201,10 @@ func TestVolumeIDToKey(t *testing.T) {
191201
if !reflect.DeepEqual(gotKey, tc.expKey) {
192202
t.Errorf("Got key %v, but expected %v, from volume ID %v", gotKey, tc.expKey, tc.volID)
193203
}
204+
205+
if project != tc.expProject {
206+
t.Errorf("Got project %v, but expected %v, from volume ID %v", project, tc.expProject, tc.volID)
207+
}
194208
}
195209

196210
}

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

Lines changed: 48 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -80,31 +80,34 @@ func (cloud *FakeCloudProvider) GetDefaultZone() string {
8080
return cloud.zone
8181
}
8282

83-
func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, volumeKey *meta.Key) (*meta.Key, error) {
83+
func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) {
84+
if project == common.UnspecifiedValue {
85+
project = cloud.project
86+
}
8487
switch volumeKey.Type() {
8588
case meta.Zonal:
8689
if volumeKey.Zone != common.UnspecifiedValue {
87-
return volumeKey, nil
90+
return project, volumeKey, nil
8891
}
8992
for name, d := range cloud.disks {
9093
if name == volumeKey.Name {
9194
volumeKey.Zone = d.GetZone()
92-
return volumeKey, nil
95+
return project, volumeKey, nil
9396
}
9497
}
95-
return nil, notFoundError()
98+
return "", nil, notFoundError()
9699
case meta.Regional:
97100
if volumeKey.Region != common.UnspecifiedValue {
98-
return volumeKey, nil
101+
return project, volumeKey, nil
99102
}
100103
r, err := common.GetRegionFromZones([]string{cloud.zone})
101104
if err != nil {
102-
return nil, fmt.Errorf("failed to get region from zones: %v", err)
105+
return "", nil, fmt.Errorf("failed to get region from zones: %v", err)
103106
}
104107
volumeKey.Region = r
105-
return volumeKey, nil
108+
return project, volumeKey, nil
106109
default:
107-
return nil, fmt.Errorf("Volume key %v not zonal nor regional", volumeKey.Name)
110+
return "", nil, fmt.Errorf("Volume key %v not zonal nor regional", volumeKey.Name)
108111
}
109112
}
110113

@@ -212,7 +215,7 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string
212215
}
213216

214217
// Disk Methods
215-
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) {
218+
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) {
216219
disk, ok := cloud.disks[volKey.Name]
217220
if !ok {
218221
return nil, notFoundError()
@@ -249,7 +252,7 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *
249252
return ValidateDiskParameters(resp, params)
250253
}
251254

252-
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, multiWriter bool) error {
255+
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, multiWriter bool) error {
253256
if disk, ok := cloud.disks[volKey.Name]; ok {
254257
err := cloud.ValidateExistingDisk(ctx, disk, params,
255258
int64(capacityRange.GetRequiredBytes()),
@@ -264,7 +267,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
264267
Name: volKey.Name,
265268
SizeGb: common.BytesToGbRoundUp(capBytes),
266269
Description: "Disk created by GCE-PD CSI Driver",
267-
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
270+
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
268271
SourceSnapshotId: snapshotID,
269272
Status: cloud.mockDiskStatus,
270273
Labels: params.Labels,
@@ -277,10 +280,10 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
277280
switch volKey.Type() {
278281
case meta.Zonal:
279282
computeDisk.Zone = volKey.Zone
280-
computeDisk.SelfLink = fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name)
283+
computeDisk.SelfLink = fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, volKey.Zone, volKey.Name)
281284
case meta.Regional:
282285
computeDisk.Region = volKey.Region
283-
computeDisk.SelfLink = fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name)
286+
computeDisk.SelfLink = fmt.Sprintf("projects/%s/regions/%s/disks/%s", project, volKey.Region, volKey.Name)
284287
default:
285288
return fmt.Errorf("could not create disk, key was neither zonal nor regional, instead got: %v", volKey.String())
286289
}
@@ -289,16 +292,16 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
289292
return nil
290293
}
291294

292-
func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, volKey *meta.Key) error {
295+
func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, project string, volKey *meta.Key) error {
293296
if _, ok := cloud.disks[volKey.Name]; !ok {
294297
return notFoundError()
295298
}
296299
delete(cloud.disks, volKey.Name)
297300
return nil
298301
}
299302

300-
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
301-
source := cloud.GetDiskSourceURI(volKey)
303+
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
304+
source := cloud.GetDiskSourceURI(project, volKey)
302305

303306
attachedDiskV1 := &computev1.AttachedDisk{
304307
DeviceName: volKey.Name,
@@ -315,7 +318,7 @@ func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, volKey *meta.Key
315318
return nil
316319
}
317320

318-
func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, deviceName, instanceZone, instanceName string) error {
321+
func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, project, deviceName, instanceZone, instanceName string) error {
319322
instance, ok := cloud.instances[instanceName]
320323
if !ok {
321324
return fmt.Errorf("Failed to get instance %v", instanceName)
@@ -332,31 +335,31 @@ func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, deviceName, inst
332335
return nil
333336
}
334337

335-
func (cloud *FakeCloudProvider) GetDiskTypeURI(volKey *meta.Key, diskType string) string {
338+
func (cloud *FakeCloudProvider) GetDiskTypeURI(project string, volKey *meta.Key, diskType string) string {
336339
switch volKey.Type() {
337340
case meta.Zonal:
338-
return cloud.getZonalDiskTypeURI(volKey.Zone, diskType)
341+
return cloud.getZonalDiskTypeURI(project, volKey.Zone, diskType)
339342
case meta.Regional:
340-
return cloud.getRegionalDiskTypeURI(volKey.Region, diskType)
343+
return cloud.getRegionalDiskTypeURI(project, volKey.Region, diskType)
341344
default:
342345
return fmt.Sprintf("could not get disk type uri, key was neither zonal nor regional, instead got: %v", volKey.String())
343346
}
344347
}
345348

346-
func (cloud *FakeCloudProvider) getZonalDiskTypeURI(zone, diskType string) string {
347-
return fmt.Sprintf(diskTypeURITemplateSingleZone, cloud.project, zone, diskType)
349+
func (cloud *FakeCloudProvider) getZonalDiskTypeURI(project, zone, diskType string) string {
350+
return fmt.Sprintf(diskTypeURITemplateSingleZone, project, zone, diskType)
348351
}
349352

350-
func (cloud *FakeCloudProvider) getRegionalDiskTypeURI(region, diskType string) string {
351-
return fmt.Sprintf(diskTypeURITemplateRegional, cloud.project, region, diskType)
353+
func (cloud *FakeCloudProvider) getRegionalDiskTypeURI(project, region, diskType string) string {
354+
return fmt.Sprintf(diskTypeURITemplateRegional, project, region, diskType)
352355
}
353356

354-
func (cloud *FakeCloudProvider) WaitForAttach(ctx context.Context, volKey *meta.Key, instanceZone, instanceName string) error {
357+
func (cloud *FakeCloudProvider) WaitForAttach(ctx context.Context, project string, volKey *meta.Key, instanceZone, instanceName string) error {
355358
return nil
356359
}
357360

358361
// Regional Disk Methods
359-
func (cloud *FakeCloudProvider) GetReplicaZoneURI(zone string) string {
362+
func (cloud *FakeCloudProvider) GetReplicaZoneURI(project, zone string) string {
360363
return ""
361364
}
362365

@@ -375,7 +378,7 @@ func (cloud *FakeCloudProvider) GetInstanceOrError(ctx context.Context, instance
375378
}
376379

377380
// Snapshot Methods
378-
func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, snapshotName string) (*computev1.Snapshot, error) {
381+
func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, project, snapshotName string) (*computev1.Snapshot, error) {
379382
snapshot, ok := cloud.snapshots[snapshotName]
380383
if !ok {
381384
return nil, notFoundError()
@@ -384,7 +387,7 @@ func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, snapshotName st
384387
return snapshot, nil
385388
}
386389

387-
func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
390+
func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
388391
if snapshot, ok := cloud.snapshots[snapshotName]; ok {
389392
return snapshot, nil
390393
}
@@ -394,13 +397,13 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta
394397
DiskSizeGb: int64(DiskSizeGb),
395398
CreationTimestamp: Timestamp,
396399
Status: "UPLOADING",
397-
SelfLink: cloud.getGlobalSnapshotURI(snapshotName),
400+
SelfLink: cloud.getGlobalSnapshotURI(project, snapshotName),
398401
}
399402
switch volKey.Type() {
400403
case meta.Zonal:
401-
snapshotToCreate.SourceDisk = cloud.getZonalDiskSourceURI(volKey.Name, volKey.Zone)
404+
snapshotToCreate.SourceDisk = cloud.getZonalDiskSourceURI(project, volKey.Name, volKey.Zone)
402405
case meta.Regional:
403-
snapshotToCreate.SourceDisk = cloud.getRegionalDiskSourceURI(volKey.Name, volKey.Region)
406+
snapshotToCreate.SourceDisk = cloud.getRegionalDiskSourceURI(project, volKey.Name, volKey.Region)
404407
default:
405408
return nil, fmt.Errorf("could not create snapshot, disk key was neither zonal nor regional, instead got: %v", volKey.String())
406409
}
@@ -409,7 +412,7 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta
409412
return snapshotToCreate, nil
410413
}
411414

412-
func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, volKey *meta.Key, requestBytes int64) (int64, error) {
415+
func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, project string, volKey *meta.Key, requestBytes int64) (int64, error) {
413416
disk, ok := cloud.disks[volKey.Name]
414417
if !ok {
415418
return -1, notFoundError()
@@ -424,7 +427,7 @@ func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, volKey *meta.Key
424427
}
425428

426429
// Snapshot Methods
427-
func (cloud *FakeCloudProvider) DeleteSnapshot(ctx context.Context, snapshotName string) error {
430+
func (cloud *FakeCloudProvider) DeleteSnapshot(ctx context.Context, project, snapshotName string) error {
428431
delete(cloud.snapshots, snapshotName)
429432
return nil
430433
}
@@ -434,7 +437,7 @@ func (cloud *FakeCloudProvider) ValidateExistingSnapshot(resp *computev1.Snapsho
434437
return fmt.Errorf("disk does not exist")
435438
}
436439

437-
diskSource := cloud.GetDiskSourceURI(volKey)
440+
diskSource := cloud.GetDiskSourceURI(cloud.project, volKey)
438441
if resp.SourceDisk != diskSource {
439442
return status.Error(codes.AlreadyExists, fmt.Sprintf("snapshot already exists with same name but with a different disk source %s, expected disk source %s", diskSource, resp.SourceDisk))
440443
}
@@ -443,37 +446,37 @@ func (cloud *FakeCloudProvider) ValidateExistingSnapshot(resp *computev1.Snapsho
443446
return nil
444447
}
445448

446-
func (cloud *FakeCloudProvider) GetDiskSourceURI(volKey *meta.Key) string {
449+
func (cloud *FakeCloudProvider) GetDiskSourceURI(project string, volKey *meta.Key) string {
447450
switch volKey.Type() {
448451
case meta.Zonal:
449-
return cloud.getZonalDiskSourceURI(volKey.Name, volKey.Zone)
452+
return cloud.getZonalDiskSourceURI(project, volKey.Name, volKey.Zone)
450453
case meta.Regional:
451-
return cloud.getRegionalDiskSourceURI(volKey.Name, volKey.Region)
454+
return cloud.getRegionalDiskSourceURI(project, volKey.Name, volKey.Region)
452455
default:
453456
return ""
454457
}
455458
}
456459

457-
func (cloud *FakeCloudProvider) getZonalDiskSourceURI(diskName, zone string) string {
460+
func (cloud *FakeCloudProvider) getZonalDiskSourceURI(project, diskName, zone string) string {
458461
return BasePath + fmt.Sprintf(
459462
diskSourceURITemplateSingleZone,
460-
cloud.project,
463+
project,
461464
zone,
462465
diskName)
463466
}
464467

465-
func (cloud *FakeCloudProvider) getRegionalDiskSourceURI(diskName, region string) string {
468+
func (cloud *FakeCloudProvider) getRegionalDiskSourceURI(project, diskName, region string) string {
466469
return BasePath + fmt.Sprintf(
467470
diskSourceURITemplateRegional,
468-
cloud.project,
471+
project,
469472
region,
470473
diskName)
471474
}
472475

473-
func (cloud *FakeCloudProvider) getGlobalSnapshotURI(snapshotName string) string {
476+
func (cloud *FakeCloudProvider) getGlobalSnapshotURI(project, snapshotName string) string {
474477
return BasePath + fmt.Sprintf(
475478
snapshotURITemplateGlobal,
476-
cloud.project,
479+
project,
477480
snapshotName)
478481
}
479482

@@ -490,11 +493,11 @@ type FakeBlockingCloudProvider struct {
490493
// Upon starting a CreateSnapshot, it passes a chan 'executeCreateSnapshot' into readyToExecute, then blocks on executeCreateSnapshot.
491494
// The test calling this function can block on readyToExecute to ensure that the operation has started and
492495
// allowed the CreateSnapshot to continue by passing a struct into executeCreateSnapshot.
493-
func (cloud *FakeBlockingCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
496+
func (cloud *FakeBlockingCloudProvider) CreateSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
494497
executeCreateSnapshot := make(chan struct{})
495498
cloud.ReadyToExecute <- executeCreateSnapshot
496499
<-executeCreateSnapshot
497-
return cloud.FakeCloudProvider.CreateSnapshot(ctx, volKey, snapshotName)
500+
return cloud.FakeCloudProvider.CreateSnapshot(ctx, project, volKey, snapshotName)
498501
}
499502

500503
func notFoundError() *googleapi.Error {

0 commit comments

Comments
 (0)