Skip to content

Commit

Permalink
Fix restoring GCP regional disks
Browse files Browse the repository at this point in the history 
To create a regional disk, the URLs for the zones in which the disk is
replicated must be provided to the GCP API.

Fixes vmware-tanzu#1183

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>
  • Loading branch information
@nrb
nrb committed Feb 7, 2019
1 parent 3054a38 commit b9357bd
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 10 deletions.
36 changes: 27 additions & 9 deletions pkg/cloudprovider/gcp/block_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package gcp

import (
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"os"
Expand All @@ -36,7 +37,10 @@ import (
"github.com/heptio/velero/pkg/util/collections"
)

const projectKey = "project"
const (
projectKey = "project"
zoneURLTemplate = "https://www.googleapis.com/compute/v1/projects/%s/zones/%s"
)

type blockStore struct {
gce *compute.Service
Expand Down Expand Up @@ -100,19 +104,19 @@ func isMultiZone(volumeAZ string) bool {
return strings.Contains(volumeAZ, "__")
}

// parseRegion parses a failure-domain tag with multiple regions
// and returns a single region. Regions are sperated by double underscores (__).
// parseZone parses a failure-domain tag with multiple zones
// and returns a single zone. Zones are sperated by double underscores (__).
// For example
// input: us-central1-a__us-central1-b
// return: us-central1
// When a custom storage class spans multiple geographical regions,
// such as us-central1 and us-west1 only the region matching the cluster is used
// When a custom storage class spans multiple geographical zones,
// such as us-central1 and us-west1 only the zone matching the cluster is used
// in the failure-domain tag.
// For example
// Cluster nodes in us-central1-c, us-central1-f
// Storage class zones us-central1-a, us-central1-f, us-east1-a, us-east1-d
// The failure-domain tag would be: us-central1-a__us-central1-f
func parseRegion(volumeAZ string) (string, error) {
func parseZone(volumeAZ string) (string, error) {
zones := strings.Split(volumeAZ, "__")
zone := zones[0]
parts := strings.SplitAfterN(zone, "-", 3)
Expand All @@ -122,6 +126,19 @@ func parseRegion(volumeAZ string) (string, error) {
return parts[0] + strings.TrimSuffix(parts[1], "-"), nil
}

// createZoneURLs returns a slice of URLs from zones within a failure-domain tag.
// This is done in order to populate the ReplicaZones field of a regional disk correctly.
// Zones provided are *not* validated by this function.
func createZoneURLs(project, volumeAZ string) []string {
zones := strings.Split(volumeAZ, "__")
for i, z := range zones {
url := fmt.Sprintf(zoneURLTemplate, project, z)
zones[i] = url
}

return zones
}

func (b *blockStore) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (volumeID string, err error) {
// get the snapshot so we can apply its tags to the volume
res, err := b.gce.Snapshots.Get(b.project, snapshotID).Do()
Expand All @@ -142,10 +159,11 @@ func (b *blockStore) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ s
}

if isMultiZone(volumeAZ) {
volumeRegion, err := parseRegion(volumeAZ)
volumeRegion, err := parseZone(volumeAZ)
if err != nil {
return "", err
}
disk.ReplicaZones = createZoneURLs(b.project, volumeAZ)
if _, err = b.gce.RegionDisks.Insert(b.project, volumeRegion, disk).Do(); err != nil {
return "", errors.WithStack(err)
}
Expand All @@ -165,7 +183,7 @@ func (b *blockStore) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, e
)

if isMultiZone(volumeAZ) {
volumeRegion, err := parseRegion(volumeAZ)
volumeRegion, err := parseZone(volumeAZ)
if err != nil {
return "", nil, errors.WithStack(err)
}
Expand Down Expand Up @@ -195,7 +213,7 @@ func (b *blockStore) CreateSnapshot(volumeID, volumeAZ string, tags map[string]s
}

if isMultiZone(volumeAZ) {
volumeRegion, err := parseRegion(volumeAZ)
volumeRegion, err := parseZone(volumeAZ)
if err != nil {
return "", errors.WithStack(err)
}
Expand Down
30 changes: 29 additions & 1 deletion pkg/cloudprovider/gcp/block_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package gcp

import (
"encoding/json"
"fmt"
"testing"

"github.com/pkg/errors"
Expand Down Expand Up @@ -161,54 +162,81 @@ func TestRegionHelpers(t *testing.T) {
expectedRegion string
expectedIsMultiZone bool
expectedError error
expectedZoneURLs []string
}{
{
name: "valid multizone(2) tag",
volumeAZ: "us-central1-a__us-central1-b",
expectedRegion: "us-central1",
expectedIsMultiZone: true,
expectedError: nil,
expectedZoneURLs: []string{
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-a"),
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-b"),
},
},
{
name: "valid multizone(4) tag",
volumeAZ: "us-central1-a__us-central1-b__us-central1-f__us-central1-e",
expectedRegion: "us-central1",
expectedIsMultiZone: true,
expectedError: nil,
expectedZoneURLs: []string{
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-a"),
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-b"),
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-f"),
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-e"),
},
},
{
name: "valid single zone tag",
volumeAZ: "us-central1-a",
expectedRegion: "us-central1",
expectedIsMultiZone: false,
expectedError: nil,
expectedZoneURLs: []string{
fmt.Sprintf(zoneURLTemplate, "project1", "us-central1-a"),
},
},
{
name: "invalid single zone tag",
volumeAZ: "us^central1^a",
expectedRegion: "",
expectedIsMultiZone: false,
expectedError: errors.Errorf("failed to parse region from zone: %q", "us^central1^a"),
expectedZoneURLs: []string{
fmt.Sprintf(zoneURLTemplate, "project1", "us^central1^a"),
},
},
{
name: "invalid multizone tag",
volumeAZ: "us^central1^a__us^central1^b",
expectedRegion: "",
expectedIsMultiZone: true,
expectedError: errors.Errorf("failed to parse region from zone: %q", "us^central1^a__us^central1^b"),
expectedZoneURLs: []string{
fmt.Sprintf(zoneURLTemplate, "project1", "us^central1^a"),
fmt.Sprintf(zoneURLTemplate, "project1", "us^central1^b"),
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.expectedIsMultiZone, isMultiZone(test.volumeAZ))
region, err := parseRegion(test.volumeAZ)
region, err := parseZone(test.volumeAZ)
if test.expectedError == nil {
assert.NoError(t, err)
} else {
assert.Equal(t, test.expectedError.Error(), err.Error())
}
assert.Equal(t, test.expectedRegion, region)

zoneURLs := createZoneURLs("project1", test.volumeAZ)

for i, u := range zoneURLs {
assert.Equal(t, test.expectedZoneURLs[i], u)
}
})
}
}

0 comments on commit b9357bd

Please sign in to comment.