azure: update blockstore to allow storing snaps in different resource…

… group

Signed-off-by: Steve Kriss <[email protected]>

docs/api-types/volumesnapshotlocation.md

@@ -48,6 +48,7 @@ The configurable parameters are as follows:
 | Key | Type | Default | Meaning |
 | --- | --- | --- | --- |
 | `apiTimeout` | metav1.Duration | 2m0s | How long to wait for an Azure API request to complete before timeout. |
+| `resourceGroup` | string | Optional | The name of the resource group where volume snapshots should be stored, if different from the cluster's resource group. |
 
 #### GCP
 
@@ -56,4 +57,4 @@ No parameters required.
 [0]: #aws
 [1]: #gcp
 [2]: #azure
-[3]: http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-available-regions
+[3]: http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-available-regions

examples/azure/06-ark-volumesnapshotlocation.yaml

@@ -21,4 +21,4 @@ metadata:
 spec:
   provider: azure
   config:
-    apiTimeout: 2m0s 
+    apiTimeout: 2m0s

pkg/cloudprovider/azure/block_store.go

@@ -40,18 +40,21 @@ import (
 
 const (
 	resourceGroupEnvVar = "AZURE_RESOURCE_GROUP"
+
 	apiTimeoutConfigKey = "apiTimeout"
-	snapshotsResource   = "snapshots"
-	disksResource       = "disks"
+
+	snapshotsResource = "snapshots"
+	disksResource     = "disks"
 )
 
 type blockStore struct {
-	log           logrus.FieldLogger
-	disks         *disk.DisksClient
-	snaps         *disk.SnapshotsClient
-	subscription  string
-	resourceGroup string
-	apiTimeout    time.Duration
+	log                logrus.FieldLogger
+	disks              *disk.DisksClient
+	snaps              *disk.SnapshotsClient
+	subscription       string
+	disksResourceGroup string
+	snapsResourceGroup string
+	apiTimeout         time.Duration
 }
 
 type snapshotIdentifier struct {
@@ -106,7 +109,16 @@ func (b *blockStore) Init(config map[string]string) error {
 	b.disks = &disksClient
 	b.snaps = &snapsClient
 	b.subscription = envVars[subscriptionIDEnvVar]
-	b.resourceGroup = envVars[resourceGroupEnvVar]
+	b.disksResourceGroup = envVars[resourceGroupEnvVar]
+	b.snapsResourceGroup = config[resourceGroupConfigKey]
+
+	// if no resource group was explicitly specified in 'config',
+	// use the value from the env var (i.e. the same one as where
+	// the cluster & disks are)
+	if b.snapsResourceGroup == "" {
+		b.snapsResourceGroup = envVars[resourceGroupEnvVar]
+	}
+
 	b.apiTimeout = apiTimeout
 
 	return nil
@@ -142,7 +154,7 @@ func (b *blockStore) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ s
 	ctx, cancel := context.WithTimeout(context.Background(), b.apiTimeout)
 	defer cancel()
 
-	_, errChan := b.disks.CreateOrUpdate(b.resourceGroup, *disk.Name, disk, ctx.Done())
+	_, errChan := b.disks.CreateOrUpdate(b.disksResourceGroup, *disk.Name, disk, ctx.Done())
 
 	err = <-errChan
 
@@ -153,7 +165,7 @@ func (b *blockStore) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ s
 }
 
 func (b *blockStore) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) {
-	res, err := b.disks.Get(b.resourceGroup, volumeID)
+	res, err := b.disks.Get(b.disksResourceGroup, volumeID)
 	if err != nil {
 		return "", nil, errors.WithStack(err)
 	}
@@ -163,12 +175,12 @@ func (b *blockStore) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, e
 
 func (b *blockStore) CreateSnapshot(volumeID, volumeAZ string, tags map[string]string) (string, error) {
 	// Lookup disk info for its Location
-	diskInfo, err := b.disks.Get(b.resourceGroup, volumeID)
+	diskInfo, err := b.disks.Get(b.disksResourceGroup, volumeID)
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
 
-	fullDiskName := getComputeResourceName(b.subscription, b.resourceGroup, disksResource, volumeID)
+	fullDiskName := getComputeResourceName(b.subscription, b.disksResourceGroup, disksResource, volumeID)
 	// snapshot names must be <= 80 characters long
 	var snapshotName string
 	suffix := "-" + uuid.NewV4().String()
@@ -194,14 +206,14 @@ func (b *blockStore) CreateSnapshot(volumeID, volumeAZ string, tags map[string]s
 	ctx, cancel := context.WithTimeout(context.Background(), b.apiTimeout)
 	defer cancel()
 
-	_, errChan := b.snaps.CreateOrUpdate(b.resourceGroup, *snap.Name, snap, ctx.Done())
+	_, errChan := b.snaps.CreateOrUpdate(b.snapsResourceGroup, *snap.Name, snap, ctx.Done())
 	err = <-errChan
 
 	if err != nil {
 		return "", errors.WithStack(err)
 	}
 
-	return getComputeResourceName(b.subscription, b.resourceGroup, snapshotsResource, snapshotName), nil
+	return getComputeResourceName(b.subscription, b.snapsResourceGroup, snapshotsResource, snapshotName), nil
 }
 
 func getSnapshotTags(arkTags map[string]string, diskTags *map[string]*string) *map[string]*string {
@@ -279,8 +291,11 @@ func (b *blockStore) parseSnapshotName(name string) (*snapshotIdentifier, error)
 	// legacy format - name only (not fully-qualified)
 	case !strings.Contains(name, "/"):
 		return &snapshotIdentifier{
-			subscription:  b.subscription,
-			resourceGroup: b.resourceGroup,
+			subscription: b.subscription,
+			// use the disksResourceGroup here because Ark only
+			// supported storing snapshots in that resource group
+			// when the legacy snapshot format was used.
+			resourceGroup: b.disksResourceGroup,
 			name:          name,
 		}, nil
 	// current format - fully qualified
@@ -341,7 +356,7 @@ func (b *blockStore) SetVolumeID(pv runtime.Unstructured, volumeID string) (runt
 	}
 
 	azure["diskName"] = volumeID
-	azure["diskURI"] = getComputeResourceName(b.subscription, b.resourceGroup, disksResource, volumeID)
+	azure["diskURI"] = getComputeResourceName(b.subscription, b.disksResourceGroup, disksResource, volumeID)
 
 	return pv, nil
 }

pkg/cloudprovider/azure/block_store_test.go

@@ -56,8 +56,8 @@ func TestGetVolumeID(t *testing.T) {
 
 func TestSetVolumeID(t *testing.T) {
 	b := &blockStore{
-		resourceGroup: "rg",
-		subscription:  "sub",
+		disksResourceGroup: "rg",
+		subscription:       "sub",
 	}
 
 	pv := &unstructured.Unstructured{
@@ -99,8 +99,8 @@ func TestSetVolumeID(t *testing.T) {
 // format
 func TestParseSnapshotName(t *testing.T) {
 	b := &blockStore{
-		subscription:  "default-sub",
-		resourceGroup: "default-rg",
+		subscription:       "default-sub",
+		disksResourceGroup: "default-rg-legacy",
 	}
 
 	// invalid name
@@ -123,7 +123,7 @@ func TestParseSnapshotName(t *testing.T) {
 	snap, err = b.parseSnapshotName(fullName)
 	require.NoError(t, err)
 	assert.Equal(t, b.subscription, snap.subscription)
-	assert.Equal(t, b.resourceGroup, snap.resourceGroup)
+	assert.Equal(t, b.disksResourceGroup, snap.resourceGroup)
 	assert.Equal(t, fullName, snap.name)
 
 }

pkg/cloudprovider/azure/common.go

@@ -29,6 +29,8 @@ const (
 	subscriptionIDEnvVar = "AZURE_SUBSCRIPTION_ID"
 	clientIDEnvVar       = "AZURE_CLIENT_ID"
 	clientSecretEnvVar   = "AZURE_CLIENT_SECRET"
+
+	resourceGroupConfigKey = "resourceGroup"
 )
 
 // GetResticEnvVars gets the environment variables that restic

pkg/cloudprovider/azure/object_store.go

@@ -33,7 +33,6 @@ import (
 )
 
 const (
-	resourceGroupConfigKey  = "resourceGroup"
 	storageAccountConfigKey = "storageAccount"
 )