Skip to content

Allow cross project snapshots and volumes #782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 1, 2021
Merged

Allow cross project snapshots and volumes #782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
77f5439
Allow cross project snapshots
christian-roggia May 19, 2021
c73c98a
Improve readability of snapshot id splitting
christian-roggia May 19, 2021
1364a18
feat: fixed silent override of project
christian-roggia May 28, 2021
0ece154
fix: repair project for underspecified volume key
christian-roggia May 28, 2021
64b6592
fix: remove default project whenever available
christian-roggia May 28, 2021
d955f28
fix: always repair underspecified volume key
christian-roggia May 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions pkg/common/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ const (
// Snapshot ID
snapshotTotalElements = 5
snapshotTopologyKey = 2
snapshotProjectKey = 1

// Node ID Expected Format
// "projects/{projectName}/zones/{zoneName}/disks/{diskName}"
nodeIDFmt = "projects/%s/zones/%s/instances/%s"
nodeIDProjectValue = 1
nodeIDZoneValue = 3
nodeIDNameValue = 5
nodeIDTotalElements = 6
Expand All @@ -68,17 +70,17 @@ func GbToBytes(Gb int64) int64 {
return Gb * 1024 * 1024 * 1024
}

func VolumeIDToKey(id string) (*meta.Key, error) {
func VolumeIDToKey(id string) (string, *meta.Key, error) {
splitId := strings.Split(id, "/")
if len(splitId) != volIDTotalElements {
return nil, fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id)
return "", nil, fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id)
}
if splitId[volIDToplogyKey] == "zones" {
return meta.ZonalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
return splitId[nodeIDProjectValue], meta.ZonalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
} else if splitId[volIDToplogyKey] == "regions" {
return meta.RegionalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
return splitId[nodeIDProjectValue], meta.RegionalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
} else {
return nil, fmt.Errorf("could not get id components, expected either zones or regions, got: %v", splitId[volIDToplogyKey])
return "", nil, fmt.Errorf("could not get id components, expected either zones or regions, got: %v", splitId[volIDToplogyKey])
}
}

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

func SnapshotIDToKey(id string) (string, error) {
func SnapshotIDToProjectKey(id string) (string, string, error) {
splitId := strings.Split(id, "/")
if len(splitId) != snapshotTotalElements {
return "", fmt.Errorf("failed to get id components. Expected projects/{project}/global/snapshot/{name}. Got: %s", id)
return "", "", fmt.Errorf("failed to get id components. Expected projects/{project}/global/snapshot/{name}. Got: %s", id)
}
if splitId[snapshotTopologyKey] == "global" {
return splitId[snapshotTotalElements-1], nil
return splitId[snapshotProjectKey], splitId[snapshotTotalElements-1], nil
} else {
return "", fmt.Errorf("could not get id components, expected global, got: %v", splitId[snapshotTopologyKey])
return "", "", fmt.Errorf("could not get id components, expected global, got: %v", splitId[snapshotTopologyKey])
}
}

Expand Down
36 changes: 25 additions & 11 deletions pkg/common/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,23 +146,33 @@ func TestVolumeIDToKey(t *testing.T) {
testName := "test-name"
testZone := "test-zone"
testProject := "test-project"
testCrossProject := "test-cross-project"
testRegion := "test-region"

testCases := []struct {
name string
volID string
expKey *meta.Key
expErr bool
name string
volID string
expProject string
expKey *meta.Key
expErr bool
}{
{
name: "normal zonal",
volID: fmt.Sprintf(volIDZoneFmt, testProject, testZone, testName),
expKey: meta.ZonalKey(testName, testZone),
name: "normal zonal",
volID: fmt.Sprintf(volIDZoneFmt, testProject, testZone, testName),
expKey: meta.ZonalKey(testName, testZone),
expProject: testProject,
},
{
name: "cross project",
volID: fmt.Sprintf(volIDZoneFmt, testCrossProject, testZone, testName),
expKey: meta.ZonalKey(testName, testZone),
expProject: testCrossProject,
},
{
name: "normal regional",
volID: fmt.Sprintf(volIDRegionFmt, testProject, testRegion, testName),
expKey: meta.RegionalKey(testName, testRegion),
name: "normal regional",
volID: fmt.Sprintf(volIDRegionFmt, testProject, testRegion, testName),
expKey: meta.RegionalKey(testName, testRegion),
expProject: testProject,
},
{
name: "malformed",
Expand All @@ -177,7 +187,7 @@ func TestVolumeIDToKey(t *testing.T) {
}
for _, tc := range testCases {
t.Logf("test case: %s", tc.name)
gotKey, err := VolumeIDToKey(tc.volID)
project, gotKey, err := VolumeIDToKey(tc.volID)
if err == nil && tc.expErr {
t.Errorf("Expected error but got none")
}
Expand All @@ -191,6 +201,10 @@ func TestVolumeIDToKey(t *testing.T) {
if !reflect.DeepEqual(gotKey, tc.expKey) {
t.Errorf("Got key %v, but expected %v, from volume ID %v", gotKey, tc.expKey, tc.volID)
}

if project != tc.expProject {
t.Errorf("Got project %v, but expected %v, from volume ID %v", project, tc.expProject, tc.volID)
}
}

}
Expand Down
93 changes: 48 additions & 45 deletions pkg/gce-cloud-provider/compute/fake-gce.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,31 +80,34 @@ func (cloud *FakeCloudProvider) GetDefaultZone() string {
return cloud.zone
}

func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, volumeKey *meta.Key) (*meta.Key, error) {
func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) {
if project == common.UnspecifiedValue {
project = cloud.project
}
switch volumeKey.Type() {
case meta.Zonal:
if volumeKey.Zone != common.UnspecifiedValue {
return volumeKey, nil
return project, volumeKey, nil
}
for name, d := range cloud.disks {
if name == volumeKey.Name {
volumeKey.Zone = d.GetZone()
return volumeKey, nil
return project, volumeKey, nil
}
}
return nil, notFoundError()
return "", nil, notFoundError()
case meta.Regional:
if volumeKey.Region != common.UnspecifiedValue {
return volumeKey, nil
return project, volumeKey, nil
}
r, err := common.GetRegionFromZones([]string{cloud.zone})
if err != nil {
return nil, fmt.Errorf("failed to get region from zones: %v", err)
return "", nil, fmt.Errorf("failed to get region from zones: %v", err)
}
volumeKey.Region = r
return volumeKey, nil
return project, volumeKey, nil
default:
return nil, fmt.Errorf("Volume key %v not zonal nor regional", volumeKey.Name)
return "", nil, fmt.Errorf("Volume key %v not zonal nor regional", volumeKey.Name)
}
}

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

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

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 {
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 {
if disk, ok := cloud.disks[volKey.Name]; ok {
err := cloud.ValidateExistingDisk(ctx, disk, params,
int64(capacityRange.GetRequiredBytes()),
Expand All @@ -264,7 +267,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
Name: volKey.Name,
SizeGb: common.BytesToGbRoundUp(capBytes),
Description: "Disk created by GCE-PD CSI Driver",
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
SourceSnapshotId: snapshotID,
Status: cloud.mockDiskStatus,
Labels: params.Labels,
Expand All @@ -277,10 +280,10 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
switch volKey.Type() {
case meta.Zonal:
computeDisk.Zone = volKey.Zone
computeDisk.SelfLink = fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name)
computeDisk.SelfLink = fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, volKey.Zone, volKey.Name)
case meta.Regional:
computeDisk.Region = volKey.Region
computeDisk.SelfLink = fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name)
computeDisk.SelfLink = fmt.Sprintf("projects/%s/regions/%s/disks/%s", project, volKey.Region, volKey.Name)
default:
return fmt.Errorf("could not create disk, key was neither zonal nor regional, instead got: %v", volKey.String())
}
Expand All @@ -289,16 +292,16 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
return nil
}

func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, volKey *meta.Key) error {
func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, project string, volKey *meta.Key) error {
if _, ok := cloud.disks[volKey.Name]; !ok {
return notFoundError()
}
delete(cloud.disks, volKey.Name)
return nil
}

func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
source := cloud.GetDiskSourceURI(volKey)
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
source := cloud.GetDiskSourceURI(project, volKey)

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

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

func (cloud *FakeCloudProvider) GetDiskTypeURI(volKey *meta.Key, diskType string) string {
func (cloud *FakeCloudProvider) GetDiskTypeURI(project string, volKey *meta.Key, diskType string) string {
switch volKey.Type() {
case meta.Zonal:
return cloud.getZonalDiskTypeURI(volKey.Zone, diskType)
return cloud.getZonalDiskTypeURI(project, volKey.Zone, diskType)
case meta.Regional:
return cloud.getRegionalDiskTypeURI(volKey.Region, diskType)
return cloud.getRegionalDiskTypeURI(project, volKey.Region, diskType)
default:
return fmt.Sprintf("could not get disk type uri, key was neither zonal nor regional, instead got: %v", volKey.String())
}
}

func (cloud *FakeCloudProvider) getZonalDiskTypeURI(zone, diskType string) string {
return fmt.Sprintf(diskTypeURITemplateSingleZone, cloud.project, zone, diskType)
func (cloud *FakeCloudProvider) getZonalDiskTypeURI(project, zone, diskType string) string {
return fmt.Sprintf(diskTypeURITemplateSingleZone, project, zone, diskType)
}

func (cloud *FakeCloudProvider) getRegionalDiskTypeURI(region, diskType string) string {
return fmt.Sprintf(diskTypeURITemplateRegional, cloud.project, region, diskType)
func (cloud *FakeCloudProvider) getRegionalDiskTypeURI(project, region, diskType string) string {
return fmt.Sprintf(diskTypeURITemplateRegional, project, region, diskType)
}

func (cloud *FakeCloudProvider) WaitForAttach(ctx context.Context, volKey *meta.Key, instanceZone, instanceName string) error {
func (cloud *FakeCloudProvider) WaitForAttach(ctx context.Context, project string, volKey *meta.Key, instanceZone, instanceName string) error {
return nil
}

// Regional Disk Methods
func (cloud *FakeCloudProvider) GetReplicaZoneURI(zone string) string {
func (cloud *FakeCloudProvider) GetReplicaZoneURI(project, zone string) string {
return ""
}

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

// Snapshot Methods
func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, snapshotName string) (*computev1.Snapshot, error) {
func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, project, snapshotName string) (*computev1.Snapshot, error) {
snapshot, ok := cloud.snapshots[snapshotName]
if !ok {
return nil, notFoundError()
Expand All @@ -384,7 +387,7 @@ func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, snapshotName st
return snapshot, nil
}

func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
if snapshot, ok := cloud.snapshots[snapshotName]; ok {
return snapshot, nil
}
Expand All @@ -394,13 +397,13 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta
DiskSizeGb: int64(DiskSizeGb),
CreationTimestamp: Timestamp,
Status: "UPLOADING",
SelfLink: cloud.getGlobalSnapshotURI(snapshotName),
SelfLink: cloud.getGlobalSnapshotURI(project, snapshotName),
}
switch volKey.Type() {
case meta.Zonal:
snapshotToCreate.SourceDisk = cloud.getZonalDiskSourceURI(volKey.Name, volKey.Zone)
snapshotToCreate.SourceDisk = cloud.getZonalDiskSourceURI(project, volKey.Name, volKey.Zone)
case meta.Regional:
snapshotToCreate.SourceDisk = cloud.getRegionalDiskSourceURI(volKey.Name, volKey.Region)
snapshotToCreate.SourceDisk = cloud.getRegionalDiskSourceURI(project, volKey.Name, volKey.Region)
default:
return nil, fmt.Errorf("could not create snapshot, disk key was neither zonal nor regional, instead got: %v", volKey.String())
}
Expand All @@ -409,7 +412,7 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta
return snapshotToCreate, nil
}

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

// Snapshot Methods
func (cloud *FakeCloudProvider) DeleteSnapshot(ctx context.Context, snapshotName string) error {
func (cloud *FakeCloudProvider) DeleteSnapshot(ctx context.Context, project, snapshotName string) error {
delete(cloud.snapshots, snapshotName)
return nil
}
Expand All @@ -434,7 +437,7 @@ func (cloud *FakeCloudProvider) ValidateExistingSnapshot(resp *computev1.Snapsho
return fmt.Errorf("disk does not exist")
}

diskSource := cloud.GetDiskSourceURI(volKey)
diskSource := cloud.GetDiskSourceURI(cloud.project, volKey)
if resp.SourceDisk != diskSource {
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))
}
Expand All @@ -443,37 +446,37 @@ func (cloud *FakeCloudProvider) ValidateExistingSnapshot(resp *computev1.Snapsho
return nil
}

func (cloud *FakeCloudProvider) GetDiskSourceURI(volKey *meta.Key) string {
func (cloud *FakeCloudProvider) GetDiskSourceURI(project string, volKey *meta.Key) string {
switch volKey.Type() {
case meta.Zonal:
return cloud.getZonalDiskSourceURI(volKey.Name, volKey.Zone)
return cloud.getZonalDiskSourceURI(project, volKey.Name, volKey.Zone)
case meta.Regional:
return cloud.getRegionalDiskSourceURI(volKey.Name, volKey.Region)
return cloud.getRegionalDiskSourceURI(project, volKey.Name, volKey.Region)
default:
return ""
}
}

func (cloud *FakeCloudProvider) getZonalDiskSourceURI(diskName, zone string) string {
func (cloud *FakeCloudProvider) getZonalDiskSourceURI(project, diskName, zone string) string {
return BasePath + fmt.Sprintf(
diskSourceURITemplateSingleZone,
cloud.project,
project,
zone,
diskName)
}

func (cloud *FakeCloudProvider) getRegionalDiskSourceURI(diskName, region string) string {
func (cloud *FakeCloudProvider) getRegionalDiskSourceURI(project, diskName, region string) string {
return BasePath + fmt.Sprintf(
diskSourceURITemplateRegional,
cloud.project,
project,
region,
diskName)
}

func (cloud *FakeCloudProvider) getGlobalSnapshotURI(snapshotName string) string {
func (cloud *FakeCloudProvider) getGlobalSnapshotURI(project, snapshotName string) string {
return BasePath + fmt.Sprintf(
snapshotURITemplateGlobal,
cloud.project,
project,
snapshotName)
}

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

func notFoundError() *googleapi.Error {
Expand Down
Loading