diff --git a/CHANGELOG.md b/CHANGELOG.md index e89a356f5..ac08567b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,13 @@ * _Example: Add XYZ support (#issue, @user)_ - ### Bug Fixes / Other Changes - * Delete spec.priority in pod restore action (#879, @mwieczorek) - * Added brew reference (#1051, @omerlh) - * Update to go 1.11 (#1069, @gliptak) - * Initialize empty schedule metrics on server init (#1054, @cbeneke) - * Update CHANGELOGs (#1063, @wwitzel3) +### Bug Fixes / Other Changes + * add multizone/regional support to gcp (#765, @wwitzel3) + * Delete spec.priority in pod restore action (#879, @mwieczorek) + * Added brew reference (#1051, @omerlh) + * Update to go 1.11 (#1069, @gliptak) + * Initialize empty schedule metrics on server init (#1054, @cbeneke) + * Update CHANGELOGs (#1063, @wwitzel3) ## Current release: * [CHANGELOG-0.10.md][8] diff --git a/pkg/cloudprovider/gcp/block_store.go b/pkg/cloudprovider/gcp/block_store.go index 2320d5ec4..6b738d7ec 100644 --- a/pkg/cloudprovider/gcp/block_store.go +++ b/pkg/cloudprovider/gcp/block_store.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "net/http" "os" + "strings" "github.com/pkg/errors" "github.com/satori/uuid" @@ -91,6 +92,36 @@ func extractProjectFromCreds() (string, error) { return creds.ProjectID, nil } +// isMultiZone returns true if the failure-domain tag contains +// double underscore, which is the separator used +// by GKE when a storage class spans multiple availablity +// zones. +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 (__). +// 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 +// 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) { + zones := strings.Split(volumeAZ, "__") + zone := zones[0] + parts := strings.SplitAfterN(zone, "-", 3) + if len(parts) < 2 { + return "", errors.Errorf("failed to parse region from zone: %q", volumeAZ) + } + return parts[0] + strings.TrimSuffix(parts[1], "-"), nil +} + 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() @@ -110,19 +141,44 @@ func (b *blockStore) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ s Description: res.Description, } - if _, err = b.gce.Disks.Insert(b.project, volumeAZ, disk).Do(); err != nil { - return "", errors.WithStack(err) + if isMultiZone(volumeAZ) { + volumeRegion, err := parseRegion(volumeAZ) + if err != nil { + return "", err + } + if _, err = b.gce.RegionDisks.Insert(b.project, volumeRegion, disk).Do(); err != nil { + return "", errors.WithStack(err) + } + } else { + if _, err = b.gce.Disks.Insert(b.project, volumeAZ, disk).Do(); err != nil { + return "", errors.WithStack(err) + } } return disk.Name, nil } func (b *blockStore) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { - res, err := b.gce.Disks.Get(b.project, volumeAZ, volumeID).Do() - if err != nil { - return "", nil, errors.WithStack(err) - } + var ( + res *compute.Disk + err error + ) + if isMultiZone(volumeAZ) { + volumeRegion, err := parseRegion(volumeAZ) + if err != nil { + return "", nil, errors.WithStack(err) + } + res, err = b.gce.RegionDisks.Get(b.project, volumeRegion, volumeID).Do() + if err != nil { + return "", nil, errors.WithStack(err) + } + } else { + res, err = b.gce.Disks.Get(b.project, volumeAZ, volumeID).Do() + if err != nil { + return "", nil, errors.WithStack(err) + } + } return res.Type, nil, nil } @@ -138,6 +194,18 @@ func (b *blockStore) CreateSnapshot(volumeID, volumeAZ string, tags map[string]s snapshotName = volumeID[0:63-len(suffix)] + suffix } + if isMultiZone(volumeAZ) { + volumeRegion, err := parseRegion(volumeAZ) + if err != nil { + return "", errors.WithStack(err) + } + return b.createRegionSnapshot(snapshotName, volumeID, volumeRegion, tags) + } else { + return b.createSnapshot(snapshotName, volumeID, volumeAZ, tags) + } +} + +func (b *blockStore) createSnapshot(snapshotName, volumeID, volumeAZ string, tags map[string]string) (string, error) { disk, err := b.gce.Disks.Get(b.project, volumeAZ, volumeID).Do() if err != nil { return "", errors.WithStack(err) @@ -156,6 +224,25 @@ func (b *blockStore) CreateSnapshot(volumeID, volumeAZ string, tags map[string]s return gceSnap.Name, nil } +func (b *blockStore) createRegionSnapshot(snapshotName, volumeID, volumeRegion string, tags map[string]string) (string, error) { + disk, err := b.gce.RegionDisks.Get(b.project, volumeRegion, volumeID).Do() + if err != nil { + return "", errors.WithStack(err) + } + + gceSnap := compute.Snapshot{ + Name: snapshotName, + Description: getSnapshotTags(tags, disk.Description, b.log), + } + + _, err = b.gce.RegionDisks.CreateSnapshot(b.project, volumeRegion, volumeID, &gceSnap).Do() + if err != nil { + return "", errors.WithStack(err) + } + + return gceSnap.Name, nil +} + func getSnapshotTags(arkTags map[string]string, diskDescription string, log logrus.FieldLogger) string { // Kubernetes uses the description field of GCP disks to store a JSON doc containing // tags. diff --git a/pkg/cloudprovider/gcp/block_store_test.go b/pkg/cloudprovider/gcp/block_store_test.go index 79fbb4f6e..dd6f3cd15 100644 --- a/pkg/cloudprovider/gcp/block_store_test.go +++ b/pkg/cloudprovider/gcp/block_store_test.go @@ -20,6 +20,7 @@ import ( "encoding/json" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -152,3 +153,62 @@ func TestGetSnapshotTags(t *testing.T) { }) } } + +func TestRegionHelpers(t *testing.T) { + tests := []struct { + name string + volumeAZ string + expectedRegion string + expectedIsMultiZone bool + expectedError error + }{ + { + name: "valid multizone(2) tag", + volumeAZ: "us-central1-a__us-central1-b", + expectedRegion: "us-central1", + expectedIsMultiZone: true, + expectedError: nil, + }, + { + 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, + }, + { + name: "valid single zone tag", + volumeAZ: "us-central1-a", + expectedRegion: "us-central1", + expectedIsMultiZone: false, + expectedError: nil, + }, + { + 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"), + }, + { + 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"), + }, + } + + 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) + if test.expectedError == nil { + assert.NoError(t, err) + } else { + assert.Equal(t, test.expectedError.Error(), err.Error()) + } + assert.Equal(t, test.expectedRegion, region) + }) + } +}