From 9fc9dbb4139135bc37964f2a9ba04c27209cf219 Mon Sep 17 00:00:00 2001 From: Ashish Amarnath Date: Sun, 24 Sep 2017 18:45:36 -0700 Subject: [PATCH] Preserve PV's AZ info when snapshotting and restoring PVs. - Read PV's AZ info from fault-domain label of the PV object for snapshotting. - Store PV's AZ info in the VolumeInfo. - Add tests for reading the label from the PV object. - Remove availability zone validation in AWS and GCP BlockStorageAdaptor. - Add volumeAZ as a parameter to methods in the BlockStorageAdapter interface. - Get AZ from VolumeInfo when restoring PV snapshot. - Remove references to PV availability zone in docs. Signed-off-by: Ashish Amarnath --- docs/cloud-provider-specifics.md | 4 +- docs/config-definition.md | 43 ++++++++----------- examples/aws/00-ark-config.yaml | 1 - examples/gcp/00-ark-config.yaml | 1 - pkg/apis/ark/v1/backup.go | 4 ++ pkg/apis/ark/v1/config.go | 2 - pkg/backup/volume_snapshot_action.go | 23 +++++++--- pkg/backup/volume_snapshot_action_test.go | 35 ++++++++++----- .../aws/block_storage_adapter.go | 32 +++----------- .../azure/block_storage_adapter.go | 8 ++-- .../gcp/block_storage_adapter.go | 29 ++++++------- pkg/cloudprovider/snapshot_service.go | 20 ++++----- pkg/cloudprovider/storage_interfaces.go | 8 ++-- pkg/cmd/server/server.go | 4 +- pkg/restore/restorers/pv_restorer.go | 2 +- pkg/util/test/fake_snapshot_service.go | 13 +++--- 16 files changed, 114 insertions(+), 115 deletions(-) diff --git a/docs/cloud-provider-specifics.md b/docs/cloud-provider-specifics.md index 105f0e271..889d448d5 100644 --- a/docs/cloud-provider-specifics.md +++ b/docs/cloud-provider-specifics.md @@ -89,7 +89,7 @@ Now that you have your IAM user credentials stored in a Secret, you need to repl * In file `examples/aws/00-ark-config.yaml`: - * Replace ``, ``, and ``. See the [Config definition][6] for details. + * Replace `` and ``. See the [Config definition][6] for details. * In file `examples/common/10-deployment.yaml`: @@ -167,7 +167,7 @@ Now that you have your Google Cloud credentials stored in a Secret, you need to * In file `examples/gcp/00-ark-config.yaml`: - * Replace ``, `` and ``. See the [Config definition][7] for details. + * Replace `` and ``. See the [Config definition][7] for details. * In file `examples/common/10-deployment.yaml`: diff --git a/docs/config-definition.md b/docs/config-definition.md index b98d32317..5e19cdfed 100644 --- a/docs/config-definition.md +++ b/docs/config-definition.md @@ -1,9 +1,9 @@ # Ark Config definition -* [Overview][10] -* [Example][11] -* [Parameter Reference][8] - * [Main config][9] +* [Overview][8] +* [Example][9] +* [Parameter Reference][6] + * [Main config][7] * [AWS][0] * [GCP][1] * [Azure][2] @@ -26,7 +26,6 @@ metadata: persistentVolumeProvider: aws: region: us-west-2 - availabilityZone: us-west-2a backupStorageProvider: bucket: ark aws: @@ -65,55 +64,51 @@ The configurable parameters are as follows: | `region` | string | Required Field | *Example*: "us-east-1"

See [AWS documentation][3] for the full list. | | `disableSSL` | bool | `false` | Set this to `true` if you are using Minio (or another local, S3-compatible storage service) and your deployment is not secured. | | `s3ForcePathStyle` | bool | `false` | Set this to `true` if you are using a local storage service like Minio. | -| `s3Url` | string | Required field for non-AWS-hosted storage| *Example*: http://minio:9000

You can specify the AWS S3 URL here for explicitness, but Ark can already generate it from `region`, `availabilityZone`, and `bucket`. This field is primarily for local storage services like Minio.| -| `kmsKeyID` | string | Empty | *Example*: "502b409c-4da1-419f-a16e-eif453b3i49f"

Specify an [AWS KMS key][12] id to enable encryption of the backups stored in S3. Only works with AWS S3 and may require explicitly granting key usage rights.| +| `s3Url` | string | Required field for non-AWS-hosted storage| *Example*: http://minio:9000

You can specify the AWS S3 URL here for explicitness, but Ark can already generate it from `region`, and `bucket`. This field is primarily for local storage services like Minio.| +| `kmsKeyID` | string | Empty | *Example*: "502b409c-4da1-419f-a16e-eif453b3i49f"

Specify an [AWS KMS key][10] id to enable encryption of the backups stored in S3. Only works with AWS S3 and may require explicitly granting key usage rights.| #### persistentVolumeProvider (AWS Only) | Key | Type | Default | Meaning | | --- | --- | --- | --- | | `region` | string | Required Field | *Example*: "us-east-1"

See [AWS documentation][3] for the full list. | -| `availabilityZone` | string | Required Field | *Example*: "us-east-1a"

See [AWS documentation][4] for details. | ### GCP #### backupStorageProvider -No parameters required; specify an empty object per [example file][13]. +No parameters required; specify an empty object per [example file][11]. #### persistentVolumeProvider | Key | Type | Default | Meaning | | --- | --- | --- | --- | -| `project` | string | Required Field | *Example*: "project-example-3jsn23"

See the [Project ID documentation][5] for details. | -| `zone` | string | Required Field | *Example*: "us-central1-a"

See [GCP documentation][6] for the full list. | +| `project` | string | Required Field | *Example*: "project-example-3jsn23"

See the [Project ID documentation][4] for details. | ### Azure #### backupStorageProvider -No parameters required; specify an empty object per [example file][14]. +No parameters required; specify an empty object per [example file][12]. #### persistentVolumeProvider | Key | Type | Default | Meaning | | --- | --- | --- | --- | -| `location` | string | Required Field | *Example*: "Canada East"

See [the list of available locations][7] (note that this particular page refers to them as "Regions"). | +| `location` | string | Required Field | *Example*: "Canada East"

See [the list of available locations][5] (note that this particular page refers to them as "Regions"). | | `apiTimeout` | metav1.Duration | 2m0s | How long to wait for an Azure API request to complete before timeout. | [0]: #aws [1]: #gcp [2]: #azure [3]: http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-available-regions -[4]: http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-availability-zones -[5]: https://cloud.google.com/resource-manager/docs/creating-managing-projects#identifying_projects -[6]: https://cloud.google.com/compute/docs/regions-zones/regions-zones -[7]: https://azure.microsoft.com/en-us/regions/ -[8]: #parameter-reference -[9]: #main-config-parameters -[10]: #overview -[11]: #example -[12]: http://docs.aws.amazon.com/kms/latest/developerguide/overview.html -[13]: ../examples/gcp/00-ark-config.yaml -[14]: ../examples/azure/10-ark-config.yaml +[4]: https://cloud.google.com/resource-manager/docs/creating-managing-projects#identifying_projects +[5]: https://azure.microsoft.com/en-us/regions/ +[6]: #parameter-reference +[7]: #main-config-parameters +[8]: #overview +[9]: #example +[10]: http://docs.aws.amazon.com/kms/latest/developerguide/overview.html +[11]: ../examples/gcp/00-ark-config.yaml +[12]: ../examples/azure/10-ark-config.yaml diff --git a/examples/aws/00-ark-config.yaml b/examples/aws/00-ark-config.yaml index 183f983c8..6b668fb8e 100644 --- a/examples/aws/00-ark-config.yaml +++ b/examples/aws/00-ark-config.yaml @@ -21,7 +21,6 @@ metadata: persistentVolumeProvider: aws: region: - availabilityZone: backupStorageProvider: bucket: aws: diff --git a/examples/gcp/00-ark-config.yaml b/examples/gcp/00-ark-config.yaml index 3b7579847..6616e9656 100644 --- a/examples/gcp/00-ark-config.yaml +++ b/examples/gcp/00-ark-config.yaml @@ -21,7 +21,6 @@ metadata: persistentVolumeProvider: gcp: project: - zone: backupStorageProvider: bucket: gcp: {} diff --git a/pkg/apis/ark/v1/backup.go b/pkg/apis/ark/v1/backup.go index 7dd266ad5..55e544ae4 100644 --- a/pkg/apis/ark/v1/backup.go +++ b/pkg/apis/ark/v1/backup.go @@ -109,6 +109,10 @@ type VolumeBackupInfo struct { // API. Type string `json:"type"` + // AvailabilityZone is the where the volume is provisioned + // in the cloud provider. + AvailabilityZone string `json:"availabilityZone,omitempty"` + // Iops is the optional value of provisioned IOPS for the // disk/volume in the cloud provider API. Iops *int64 `json:"iops,omitempty"` diff --git a/pkg/apis/ark/v1/config.go b/pkg/apis/ark/v1/config.go index 29ad9b1f3..28d65b864 100644 --- a/pkg/apis/ark/v1/config.go +++ b/pkg/apis/ark/v1/config.go @@ -94,7 +94,6 @@ type ObjectStorageProviderConfig struct { // AWSConfig is configuration information for connecting to AWS. type AWSConfig struct { Region string `json:"region"` - AvailabilityZone string `json:"availabilityZone"` DisableSSL bool `json:"disableSSL"` S3ForcePathStyle bool `json:"s3ForcePathStyle"` S3Url string `json:"s3Url"` @@ -104,7 +103,6 @@ type AWSConfig struct { // GCPConfig is configuration information for connecting to GCP. type GCPConfig struct { Project string `json:"project"` - Zone string `json:"zone"` } // AzureConfig is configuration information for connecting to Azure. diff --git a/pkg/backup/volume_snapshot_action.go b/pkg/backup/volume_snapshot_action.go index 2aebd4bf2..42a72f188 100644 --- a/pkg/backup/volume_snapshot_action.go +++ b/pkg/backup/volume_snapshot_action.go @@ -24,6 +24,7 @@ import ( api "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/cloudprovider" + "github.com/heptio/ark/pkg/util/collections" kubeutil "github.com/heptio/ark/pkg/util/kube" ) @@ -59,6 +60,17 @@ func (a *volumeSnapshotAction) Execute(ctx ActionContext, volume map[string]inte metadata := volume["metadata"].(map[string]interface{}) name := metadata["name"].(string) + var pvfailureDomainZone string + labelsMap, err := collections.GetMap(metadata, "labels") + if err == nil { + if labelsMap["failure-domain.beta.kubernetes.io/zone"] != nil { + pvfailureDomainZone = labelsMap["failure-domain.beta.kubernetes.io/zone"].(string) + } else { + ctx.log("error getting 'failure-domain.beta.kubernetes.io/zone' label on PersistentVolume %q for backup %q.\n", name, backupName) + } + } else { + ctx.log("error getting labels on PersistentVolume %q for backup %q. ", name, backupName) + } volumeID, err := kubeutil.GetVolumeID(volume) // non-nil error means it's a supported PV source but volume ID can't be found @@ -75,13 +87,13 @@ func (a *volumeSnapshotAction) Execute(ctx ActionContext, volume map[string]inte ctx.log("Backup %q: snapshotting PersistentVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration) - snapshotID, err := a.snapshotService.CreateSnapshot(volumeID) + snapshotID, err := a.snapshotService.CreateSnapshot(volumeID, pvfailureDomainZone) if err != nil { ctx.log("error creating snapshot for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err) return err } - volumeType, iops, err := a.snapshotService.GetVolumeInfo(volumeID) + volumeType, iops, err := a.snapshotService.GetVolumeInfo(volumeID, pvfailureDomainZone) if err != nil { ctx.log("error getting volume info for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err) return err @@ -92,9 +104,10 @@ func (a *volumeSnapshotAction) Execute(ctx ActionContext, volume map[string]inte } backup.Status.VolumeBackups[name] = &api.VolumeBackupInfo{ - SnapshotID: snapshotID, - Type: volumeType, - Iops: iops, + SnapshotID: snapshotID, + Type: volumeType, + Iops: iops, + AvailabilityZone: pvfailureDomainZone, } return nil diff --git a/pkg/backup/volume_snapshot_action_test.go b/pkg/backup/volume_snapshot_action_test.go index e524a5955..99df26bf8 100644 --- a/pkg/backup/volume_snapshot_action_test.go +++ b/pkg/backup/volume_snapshot_action_test.go @@ -77,49 +77,49 @@ func TestVolumeSnapshotAction(t *testing.T) { { name: "aws - simple volume id", snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "vol-abc123"}}}`, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/zone": "us-east-1c"}}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-east-1c/vol-abc123"}}}`, expectError: false, expectedSnapshotsTaken: 1, expectedVolumeID: "vol-abc123", ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ - "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, + "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1", AvailabilityZone: "us-east-1c"}, }, }, { name: "aws - simple volume id with provisioned IOPS", snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "vol-abc123"}}}`, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/zone": "us-east-1c"}}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-east-1c/vol-abc123"}}}`, expectError: false, expectedSnapshotsTaken: 1, expectedVolumeID: "vol-abc123", ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ - "vol-abc123": v1.VolumeBackupInfo{Type: "io1", Iops: &iops, SnapshotID: "snap-1"}, + "vol-abc123": v1.VolumeBackupInfo{Type: "io1", Iops: &iops, SnapshotID: "snap-1", AvailabilityZone: "us-east-1c"}, }, }, { name: "aws - dynamically provisioned volume id", snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-west-2a/vol-abc123"}}}`, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/zone": "us-west-2a"}}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-west-2a/vol-abc123"}}}`, expectError: false, expectedSnapshotsTaken: 1, expectedVolumeID: "vol-abc123", ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ - "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, + "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1", AvailabilityZone: "us-west-2a"}, }, }, { name: "gce", snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/zone": "gcp-zone2"}}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, expectError: false, expectedSnapshotsTaken: 1, expectedVolumeID: "pd-abc123", ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ - "pd-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, + "pd-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1", AvailabilityZone: "gcp-zone2"}, }, }, { @@ -155,6 +155,18 @@ func TestVolumeSnapshotAction(t *testing.T) { pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, expectError: true, }, + { + name: "PV with label metadata but no failureDomainZone", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/region": "us-east-1"}}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-east-1c/vol-abc123"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "vol-abc123", + ttl: 5 * time.Minute, + volumeInfo: map[string]v1.VolumeBackupInfo{ + "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, + }, + }, } for _, test := range tests { @@ -216,9 +228,10 @@ func TestVolumeSnapshotAction(t *testing.T) { snapshotID, _ := snapshotService.SnapshotsTaken.PopAny() expectedVolumeBackups["mypv"] = &v1.VolumeBackupInfo{ - SnapshotID: snapshotID, - Type: test.volumeInfo[test.expectedVolumeID].Type, - Iops: test.volumeInfo[test.expectedVolumeID].Iops, + SnapshotID: snapshotID, + Type: test.volumeInfo[test.expectedVolumeID].Type, + Iops: test.volumeInfo[test.expectedVolumeID].Iops, + AvailabilityZone: test.volumeInfo[test.expectedVolumeID].AvailabilityZone, } if e, a := expectedVolumeBackups, backup.Status.VolumeBackups; !reflect.DeepEqual(e, a) { diff --git a/pkg/cloudprovider/aws/block_storage_adapter.go b/pkg/cloudprovider/aws/block_storage_adapter.go index 052422147..621c6f7c8 100644 --- a/pkg/cloudprovider/aws/block_storage_adapter.go +++ b/pkg/cloudprovider/aws/block_storage_adapter.go @@ -33,7 +33,6 @@ var _ cloudprovider.BlockStorageAdapter = &blockStorageAdapter{} type blockStorageAdapter struct { ec2 *ec2.EC2 - az string } func getSession(config *aws.Config) (*session.Session, error) { @@ -49,13 +48,10 @@ func getSession(config *aws.Config) (*session.Session, error) { return sess, nil } -func NewBlockStorageAdapter(region, availabilityZone string) (cloudprovider.BlockStorageAdapter, error) { +func NewBlockStorageAdapter(region string) (cloudprovider.BlockStorageAdapter, error) { if region == "" { return nil, errors.New("missing region in aws configuration in config file") } - if availabilityZone == "" { - return nil, errors.New("missing availabilityZone in aws configuration in config file") - } awsConfig := aws.NewConfig().WithRegion(region) @@ -64,22 +60,8 @@ func NewBlockStorageAdapter(region, availabilityZone string) (cloudprovider.Bloc return nil, err } - // validate the availabilityZone - var ( - ec2Client = ec2.New(sess) - azReq = &ec2.DescribeAvailabilityZonesInput{ZoneNames: []*string{&availabilityZone}} - ) - res, err := ec2Client.DescribeAvailabilityZones(azReq) - if err != nil { - return nil, err - } - if len(res.AvailabilityZones) == 0 { - return nil, fmt.Errorf("availability zone %q not found", availabilityZone) - } - return &blockStorageAdapter{ - ec2: ec2Client, - az: availabilityZone, + ec2: ec2.New(sess), }, nil } @@ -88,10 +70,10 @@ func NewBlockStorageAdapter(region, availabilityZone string) (cloudprovider.Bloc // from snapshot. var iopsVolumeTypes = sets.NewString("io1") -func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType string, iops *int64) (volumeID string, err error) { +func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (volumeID string, err error) { req := &ec2.CreateVolumeInput{ SnapshotId: &snapshotID, - AvailabilityZone: &op.az, + AvailabilityZone: &volumeAZ, VolumeType: &volumeType, } @@ -107,7 +89,7 @@ func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType s return *res.VolumeId, nil } -func (op *blockStorageAdapter) GetVolumeInfo(volumeID string) (string, *int64, error) { +func (op *blockStorageAdapter) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { req := &ec2.DescribeVolumesInput{ VolumeIds: []*string{&volumeID}, } @@ -139,7 +121,7 @@ func (op *blockStorageAdapter) GetVolumeInfo(volumeID string) (string, *int64, e return volumeType, iops, nil } -func (op *blockStorageAdapter) IsVolumeReady(volumeID string) (ready bool, err error) { +func (op *blockStorageAdapter) IsVolumeReady(volumeID, volumeAZ string) (ready bool, err error) { req := &ec2.DescribeVolumesInput{ VolumeIds: []*string{&volumeID}, } @@ -181,7 +163,7 @@ func (op *blockStorageAdapter) ListSnapshots(tagFilters map[string]string) ([]st return ret, nil } -func (op *blockStorageAdapter) CreateSnapshot(volumeID string, tags map[string]string) (string, error) { +func (op *blockStorageAdapter) CreateSnapshot(volumeID, volumeAZ string, tags map[string]string) (string, error) { req := &ec2.CreateSnapshotInput{ VolumeId: &volumeID, } diff --git a/pkg/cloudprovider/azure/block_storage_adapter.go b/pkg/cloudprovider/azure/block_storage_adapter.go index 62cad92fc..a9325c9de 100644 --- a/pkg/cloudprovider/azure/block_storage_adapter.go +++ b/pkg/cloudprovider/azure/block_storage_adapter.go @@ -130,7 +130,7 @@ func NewBlockStorageAdapter(location string, apiTimeout time.Duration) (cloudpro }, nil } -func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType string, iops *int64) (string, error) { +func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (string, error) { fullSnapshotName := getFullSnapshotName(op.subscription, op.resourceGroup, snapshotID) diskName := "restore-" + uuid.NewV4().String() @@ -159,7 +159,7 @@ func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType s return diskName, nil } -func (op *blockStorageAdapter) GetVolumeInfo(volumeID string) (string, *int64, error) { +func (op *blockStorageAdapter) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { res, err := op.disks.Get(op.resourceGroup, volumeID) if err != nil { return "", nil, err @@ -168,7 +168,7 @@ func (op *blockStorageAdapter) GetVolumeInfo(volumeID string) (string, *int64, e return string(res.AccountType), nil, nil } -func (op *blockStorageAdapter) IsVolumeReady(volumeID string) (ready bool, err error) { +func (op *blockStorageAdapter) IsVolumeReady(volumeID, volumeAZ string) (ready bool, err error) { res, err := op.disks.Get(op.resourceGroup, volumeID) if err != nil { return false, err @@ -215,7 +215,7 @@ Snapshot: return ret, nil } -func (op *blockStorageAdapter) CreateSnapshot(volumeID string, tags map[string]string) (string, error) { +func (op *blockStorageAdapter) CreateSnapshot(volumeID, volumeAZ string, tags map[string]string) (string, error) { fullDiskName := getFullDiskName(op.subscription, op.resourceGroup, volumeID) // snapshot names must be <= 80 characters long var snapshotName string diff --git a/pkg/cloudprovider/gcp/block_storage_adapter.go b/pkg/cloudprovider/gcp/block_storage_adapter.go index 8664e476a..04fb8ab81 100644 --- a/pkg/cloudprovider/gcp/block_storage_adapter.go +++ b/pkg/cloudprovider/gcp/block_storage_adapter.go @@ -35,18 +35,14 @@ import ( type blockStorageAdapter struct { gce *compute.Service project string - zone string } var _ cloudprovider.BlockStorageAdapter = &blockStorageAdapter{} -func NewBlockStorageAdapter(project, zone string) (cloudprovider.BlockStorageAdapter, error) { +func NewBlockStorageAdapter(project string) (cloudprovider.BlockStorageAdapter, error) { if project == "" { return nil, errors.New("missing project in gcp configuration in config file") } - if zone == "" { - return nil, errors.New("missing zone in gcp configuration in config file") - } client, err := google.DefaultClient(oauth2.NoContext, compute.ComputeScope) if err != nil { @@ -58,24 +54,23 @@ func NewBlockStorageAdapter(project, zone string) (cloudprovider.BlockStorageAda return nil, err } - // validate project & zone - res, err := gce.Zones.Get(project, zone).Do() + // validate project + res, err := gce.Projects.Get(project).Do() if err != nil { return nil, err } if res == nil { - return nil, fmt.Errorf("zone %q not found for project %q", project, zone) + return nil, fmt.Errorf("error getting project %q", project) } return &blockStorageAdapter{ gce: gce, project: project, - zone: zone, }, nil } -func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID string, volumeType string, iops *int64) (volumeID string, err error) { +func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (volumeID string, err error) { res, err := op.gce.Snapshots.Get(op.project, snapshotID).Do() if err != nil { return "", err @@ -87,15 +82,15 @@ func (op *blockStorageAdapter) CreateVolumeFromSnapshot(snapshotID string, volum Type: volumeType, } - if _, err = op.gce.Disks.Insert(op.project, op.zone, disk).Do(); err != nil { + if _, err = op.gce.Disks.Insert(op.project, volumeAZ, disk).Do(); err != nil { return "", err } return disk.Name, nil } -func (op *blockStorageAdapter) GetVolumeInfo(volumeID string) (string, *int64, error) { - res, err := op.gce.Disks.Get(op.project, op.zone, volumeID).Do() +func (op *blockStorageAdapter) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { + res, err := op.gce.Disks.Get(op.project, volumeAZ, volumeID).Do() if err != nil { return "", nil, err } @@ -103,8 +98,8 @@ func (op *blockStorageAdapter) GetVolumeInfo(volumeID string) (string, *int64, e return res.Type, nil, nil } -func (op *blockStorageAdapter) IsVolumeReady(volumeID string) (ready bool, err error) { - disk, err := op.gce.Disks.Get(op.project, op.zone, volumeID).Do() +func (op *blockStorageAdapter) IsVolumeReady(volumeID, volumeAZ string) (ready bool, err error) { + disk, err := op.gce.Disks.Get(op.project, volumeAZ, volumeID).Do() if err != nil { return false, err } @@ -140,7 +135,7 @@ func (op *blockStorageAdapter) ListSnapshots(tagFilters map[string]string) ([]st return ret, nil } -func (op *blockStorageAdapter) CreateSnapshot(volumeID string, tags map[string]string) (string, error) { +func (op *blockStorageAdapter) CreateSnapshot(volumeID, volumeAZ string, tags map[string]string) (string, error) { // snapshot names must adhere to RFC1035 and be 1-63 characters // long var snapshotName string @@ -156,7 +151,7 @@ func (op *blockStorageAdapter) CreateSnapshot(volumeID string, tags map[string]s Name: snapshotName, } - _, err := op.gce.Disks.CreateSnapshot(op.project, op.zone, volumeID, &gceSnap).Do() + _, err := op.gce.Disks.CreateSnapshot(op.project, volumeAZ, volumeID, &gceSnap).Do() if err != nil { return "", err } diff --git a/pkg/cloudprovider/snapshot_service.go b/pkg/cloudprovider/snapshot_service.go index 34165ee01..ae2e7a8a8 100644 --- a/pkg/cloudprovider/snapshot_service.go +++ b/pkg/cloudprovider/snapshot_service.go @@ -32,19 +32,19 @@ type SnapshotService interface { // CreateSnapshot triggers a snapshot for the specified cloud volume and tags it with metadata. // it returns the cloud snapshot ID, or an error if a problem is encountered triggering the snapshot via // the cloud API. - CreateSnapshot(volumeID string) (string, error) + CreateSnapshot(volumeID, volumeAZ string) (string, error) // CreateVolumeFromSnapshot triggers a restore operation to create a new cloud volume from the specified // snapshot and volume characteristics. Returns the cloud volume ID, or an error if a problem is // encountered triggering the restore via the cloud API. - CreateVolumeFromSnapshot(snapshotID, volumeType string, iops *int64) (string, error) + CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (string, error) // DeleteSnapshot triggers a deletion of the specified Ark snapshot via the cloud API. It returns an // error if a problem is encountered triggering the deletion via the cloud API. DeleteSnapshot(snapshotID string) error // GetVolumeInfo gets the type and IOPS (if applicable) from the cloud API. - GetVolumeInfo(volumeID string) (string, *int64, error) + GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) } const ( @@ -67,8 +67,8 @@ func NewSnapshotService(blockStorage BlockStorageAdapter) SnapshotService { } } -func (sr *snapshotService) CreateVolumeFromSnapshot(snapshotID string, volumeType string, iops *int64) (string, error) { - volumeID, err := sr.blockStorage.CreateVolumeFromSnapshot(snapshotID, volumeType, iops) +func (sr *snapshotService) CreateVolumeFromSnapshot(snapshotID string, volumeType string, volumeAZ string, iops *int64) (string, error) { + volumeID, err := sr.blockStorage.CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ, iops) if err != nil { return "", err } @@ -84,7 +84,7 @@ func (sr *snapshotService) CreateVolumeFromSnapshot(snapshotID string, volumeTyp case <-timeout.C: return "", fmt.Errorf("timeout reached waiting for volume %v to be ready", volumeID) case <-ticker.C: - if ready, err := sr.blockStorage.IsVolumeReady(volumeID); err == nil && ready { + if ready, err := sr.blockStorage.IsVolumeReady(volumeID, volumeAZ); err == nil && ready { return volumeID, nil } } @@ -104,18 +104,18 @@ func (sr *snapshotService) GetAllSnapshots() ([]string, error) { return res, nil } -func (sr *snapshotService) CreateSnapshot(volumeID string) (string, error) { +func (sr *snapshotService) CreateSnapshot(volumeID, volumeAZ string) (string, error) { tags := map[string]string{ snapshotTagKey: snapshotTagVal, } - return sr.blockStorage.CreateSnapshot(volumeID, tags) + return sr.blockStorage.CreateSnapshot(volumeID, volumeAZ, tags) } func (sr *snapshotService) DeleteSnapshot(snapshotID string) error { return sr.blockStorage.DeleteSnapshot(snapshotID) } -func (sr *snapshotService) GetVolumeInfo(volumeID string) (string, *int64, error) { - return sr.blockStorage.GetVolumeInfo(volumeID) +func (sr *snapshotService) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { + return sr.blockStorage.GetVolumeInfo(volumeID, volumeAZ) } diff --git a/pkg/cloudprovider/storage_interfaces.go b/pkg/cloudprovider/storage_interfaces.go index bc439f37a..9804461e9 100644 --- a/pkg/cloudprovider/storage_interfaces.go +++ b/pkg/cloudprovider/storage_interfaces.go @@ -53,21 +53,21 @@ type ObjectStorageAdapter interface { type BlockStorageAdapter interface { // CreateVolumeFromSnapshot creates a new block volume, initialized from the provided snapshot, // and with the specified type and IOPS (if using provisioned IOPS). - CreateVolumeFromSnapshot(snapshotID, volumeType string, iops *int64) (volumeID string, err error) + CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (volumeID string, err error) // GetVolumeInfo returns the type and IOPS (if using provisioned IOPS) for a specified block // volume. - GetVolumeInfo(volumeID string) (string, *int64, error) + GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) // IsVolumeReady returns whether the specified volume is ready to be used. - IsVolumeReady(volumeID string) (ready bool, err error) + IsVolumeReady(volumeID, volumeAZ string) (ready bool, err error) // ListSnapshots returns a list of all snapshots matching the specified set of tag key/values. ListSnapshots(tagFilters map[string]string) ([]string, error) // CreateSnapshot creates a snapshot of the specified block volume, and applies the provided // set of tags to the snapshot. - CreateSnapshot(volumeID string, tags map[string]string) (snapshotID string, err error) + CreateSnapshot(volumeID, volumeAZ string, tags map[string]string) (snapshotID string, err error) // DeleteSnapshot deletes the specified volume snapshot. DeleteSnapshot(snapshotID string) error diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 6da718e45..b6ce6d1ff 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -375,9 +375,9 @@ func getBlockStorageProvider(cloudConfig api.CloudProviderConfig, field string) switch { case cloudConfig.AWS != nil: - blockStorage, err = arkaws.NewBlockStorageAdapter(cloudConfig.AWS.Region, cloudConfig.AWS.AvailabilityZone) + blockStorage, err = arkaws.NewBlockStorageAdapter(cloudConfig.AWS.Region) case cloudConfig.GCP != nil: - blockStorage, err = gcp.NewBlockStorageAdapter(cloudConfig.GCP.Project, cloudConfig.GCP.Zone) + blockStorage, err = gcp.NewBlockStorageAdapter(cloudConfig.GCP.Project) case cloudConfig.Azure != nil: blockStorage, err = azure.NewBlockStorageAdapter(cloudConfig.Azure.Location, cloudConfig.Azure.APITimeout.Duration) } diff --git a/pkg/restore/restorers/pv_restorer.go b/pkg/restore/restorers/pv_restorer.go index d255738e3..a358a5079 100644 --- a/pkg/restore/restorers/pv_restorer.go +++ b/pkg/restore/restorers/pv_restorer.go @@ -100,7 +100,7 @@ func (sr *persistentVolumeRestorer) Prepare(obj runtime.Unstructured, restore *a if restoreFromSnapshot { backupInfo := backup.Status.VolumeBackups[pvName] - volumeID, err := sr.snapshotService.CreateVolumeFromSnapshot(backupInfo.SnapshotID, backupInfo.Type, backupInfo.Iops) + volumeID, err := sr.snapshotService.CreateVolumeFromSnapshot(backupInfo.SnapshotID, backupInfo.Type, backupInfo.AvailabilityZone, backupInfo.Iops) if err != nil { return nil, nil, err } diff --git a/pkg/util/test/fake_snapshot_service.go b/pkg/util/test/fake_snapshot_service.go index f940ec6f8..82777e87b 100644 --- a/pkg/util/test/fake_snapshot_service.go +++ b/pkg/util/test/fake_snapshot_service.go @@ -39,7 +39,7 @@ func (s *FakeSnapshotService) GetAllSnapshots() ([]string, error) { return s.SnapshotsTaken.List(), nil } -func (s *FakeSnapshotService) CreateSnapshot(volumeID string) (string, error) { +func (s *FakeSnapshotService) CreateSnapshot(volumeID, volumeAZ string) (string, error) { if _, exists := s.SnapshottableVolumes[volumeID]; !exists { return "", errors.New("snapshottable volume not found") } @@ -52,11 +52,12 @@ func (s *FakeSnapshotService) CreateSnapshot(volumeID string) (string, error) { return s.SnapshottableVolumes[volumeID].SnapshotID, nil } -func (s *FakeSnapshotService) CreateVolumeFromSnapshot(snapshotID, volumeType string, iops *int64) (string, error) { +func (s *FakeSnapshotService) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (string, error) { key := api.VolumeBackupInfo{ - SnapshotID: snapshotID, - Type: volumeType, - Iops: iops, + SnapshotID: snapshotID, + Type: volumeType, + Iops: iops, + AvailabilityZone: volumeAZ, } return s.RestorableVolumes[key], nil @@ -72,7 +73,7 @@ func (s *FakeSnapshotService) DeleteSnapshot(snapshotID string) error { return nil } -func (s *FakeSnapshotService) GetVolumeInfo(volumeID string) (string, *int64, error) { +func (s *FakeSnapshotService) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { if volumeInfo, exists := s.SnapshottableVolumes[volumeID]; !exists { return "", nil, errors.New("VolumeID not found") } else {