[cinder-csi-plugin] define availability zone for snapshot backup (kubernetes#2569)

* define availability zone for snapshot backup

* fix to volume backup & restore creation

* add doc

Author: zetaab

docs/cinder-csi-plugin/using-cinder-csi-plugin.md

@@ -269,6 +269,7 @@ helm install --namespace kube-system --name cinder-csi ./charts/cinder-csi-plugi
 | VolumeSnapshotClass `parameters` | `force-create`    | `false`         | Enable to support creating snapshot for a volume in in-use status |
 | VolumeSnapshotClass `parameters` | `type`            | Empty String    | `snapshot` creates a VolumeSnapshot object linked to a Cinder volume snapshot. `backup` creates a VolumeSnapshot object linked to a cinder volume backup. Defaults to `snapshot` if not defined |
 | VolumeSnapshotClass `parameters` | `backup-max-duration-seconds-per-gb`  | `20`    | Defines the amount of time to wait for a backup to complete in seconds per GB of volume size |
+| VolumeSnapshotClass `parameters`  | `availability`          | Same as volume | String. Backup Availability Zone |
 | Inline Volume `volumeAttributes`   | `capacity`              | `1Gi`       | volume size for creating inline volumes| 
 | Inline Volume `VolumeAttributes`   | `type`              | Empty String  | Name/ID of Volume type. Corresponding volume type should exist in cinder |
 

pkg/csi/cinder/controllerserver.go

@@ -136,7 +136,8 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		// In case a snapshot is not found
 		// check if a Backup with the same ID exists
 		if backupsAreEnabled && cpoerrors.IsNotFound(err) {
-			back, err := cloud.GetBackupByID(snapshotID)
+			var back *backups.Backup
+			back, err = cloud.GetBackupByID(snapshotID)
 			if err != nil {
 				//If there is an error getting the backup as well, fail.
 				return nil, status.Errorf(codes.NotFound, "VolumeContentSource Snapshot or Backup with ID %s not found", snapshotID)
@@ -154,7 +155,6 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		if cpoerrors.IsNotFound(err) && snapshotID == "" {
 			return nil, err
 		}
-
 	}
 
 	if content != nil && content.GetVolume() != nil {
@@ -420,10 +420,17 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		}
 
 		if len(backups) == 1 {
-			backup = &backups[0]
+			// since backup.VolumeID is not part of ListBackups response
+			// we need fetch single backup to get the full object.
+			backup, err = cs.Cloud.GetBackupByID(backups[0].ID)
+			if err != nil {
+				klog.Errorf("Failed to get backup by ID %s: %v", backup.ID, err)
+				return nil, status.Error(codes.Internal, "Failed to get backup by ID")
+			}
 
 			// Verify the existing backup has the same VolumeID, otherwise it belongs to another volume
 			if backup.VolumeID != volumeID {
+				klog.Errorf("found existing backup for volumeID (%s) but different source volume ID (%s)", volumeID, backup.VolumeID)
 				return nil, status.Error(codes.AlreadyExists, "Backup with given name already exists, with different source volume ID")
 			}
 
@@ -503,7 +510,7 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 			return nil, status.Error(codes.Internal, fmt.Sprintf("GetBackupByID failed with error %v", err))
 		}
 
-		err = cs.Cloud.DeleteSnapshot(snap.ID)
+		err = cs.Cloud.DeleteSnapshot(backup.SnapshotID)
 		if err != nil && !cpoerrors.IsNotFound(err) {
 			klog.Errorf("Failed to DeleteSnapshot: %v", err)
 			return nil, status.Error(codes.Internal, fmt.Sprintf("DeleteSnapshot failed with error %v", err))
@@ -593,7 +600,7 @@ func (cs *controllerServer) createBackup(name string, volumeID string, snap *sna
 		}
 	}
 
-	backup, err := cs.Cloud.CreateBackup(name, volumeID, snap.ID, properties)
+	backup, err := cs.Cloud.CreateBackup(name, volumeID, snap.ID, parameters[openstack.SnapshotAvailabilityZone], properties)
 	if err != nil {
 		klog.Errorf("Failed to Create backup: %v", err)
 		return nil, status.Error(codes.Internal, fmt.Sprintf("CreateBackup failed with error %v", err))

pkg/csi/cinder/openstack/openstack.go

@@ -61,7 +61,7 @@ type IOpenStack interface {
 	DeleteSnapshot(snapID string) error
 	GetSnapshotByID(snapshotID string) (*snapshots.Snapshot, error)
 	WaitSnapshotReady(snapshotID string) (string, error)
-	CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error)
+	CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error)
 	ListBackups(filters map[string]string) ([]backups.Backup, error)
 	DeleteBackup(backupID string) error
 	GetBackupByID(backupID string) (*backups.Backup, error)

pkg/csi/cinder/openstack/openstack_backups.go

@@ -44,7 +44,7 @@ const (
 
 // CreateBackup issues a request to create a Backup from the specified Snapshot with the corresponding ID and
 // returns the resultant gophercloud Backup Item upon success.
-func (os *OpenStack) CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error) {
+func (os *OpenStack) CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error) {
 	blockstorageServiceClient, err := openstack.NewBlockStorageV3(os.blockstorage.ProviderClient, os.epOpts)
 	if err != nil {
 		return &backups.Backup{}, err
@@ -63,16 +63,17 @@ func (os *OpenStack) CreateBackup(name, volID string, snapshotID string, tags ma
 	}
 
 	opts := &backups.CreateOpts{
-		VolumeID:    volID,
-		SnapshotID:  snapshotID,
-		Name:        name,
-		Force:       force,
-		Description: backupDescription,
+		VolumeID:         volID,
+		SnapshotID:       snapshotID,
+		Name:             name,
+		Force:            force,
+		Description:      backupDescription,
+		AvailabilityZone: availabilityZone,
 	}
 
 	if tags != nil {
-		// Set openstack microversion to 3.43 to send metadata along with the backup
-		blockstorageServiceClient.Microversion = "3.43"
+		// Set openstack microversion to 3.51 to send metadata along with the backup
+		blockstorageServiceClient.Microversion = "3.51"
 		opts.Metadata = tags
 	}
 

pkg/csi/cinder/openstack/openstack_mock.go

@@ -315,21 +315,21 @@ func (_m *OpenStackMock) ListBackups(filters map[string]string) ([]backups.Backu
 	return r0, r1
 }
 
-func (_m *OpenStackMock) CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error) {
-	ret := _m.Called(name, volID, snapshotID, tags)
+func (_m *OpenStackMock) CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error) {
+	ret := _m.Called(name, volID, snapshotID, availabilityZone, tags)
 
 	var r0 *backups.Backup
-	if rf, ok := ret.Get(0).(func(string, string, string, map[string]string) *backups.Backup); ok {
-		r0 = rf(name, volID, snapshotID, tags)
+	if rf, ok := ret.Get(0).(func(string, string, string, string, map[string]string) *backups.Backup); ok {
+		r0 = rf(name, volID, snapshotID, availabilityZone, tags)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*backups.Backup)
 		}
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(string, string, string, map[string]string) error); ok {
-		r1 = rf(name, volID, snapshotID, tags)
+	if rf, ok := ret.Get(1).(func(string, string, string, string, map[string]string) error); ok {
+		r1 = rf(name, volID, snapshotID, availabilityZone, tags)
 	} else {
 		r1 = ret.Error(1)
 	}

pkg/csi/cinder/openstack/openstack_snapshots.go

@@ -37,9 +37,10 @@ const (
 	snapReadyFactor     = 1.2
 	snapReadySteps      = 10
 
-	snapshotDescription = "Created by OpenStack Cinder CSI driver"
-	SnapshotForceCreate = "force-create"
-	SnapshotType        = "type"
+	snapshotDescription      = "Created by OpenStack Cinder CSI driver"
+	SnapshotForceCreate      = "force-create"
+	SnapshotType             = "type"
+	SnapshotAvailabilityZone = "availability"
 )
 
 // CreateSnapshot issues a request to take a Snapshot of the specified Volume with the corresponding ID and

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -72,10 +72,10 @@ func (os *OpenStack) CreateVolume(name string, size int, vtype, availability str
 		return nil, err
 	}
 
-	// creating volumes from backups is available since 3.47 microversion
+	// creating volumes from backups and backups cross-az is available since 3.51 microversion
 	// https://docs.openstack.org/cinder/latest/contributor/api_microversion_history.html#id47
 	if !os.bsOpts.IgnoreVolumeMicroversion && sourceBackupID != "" {
-		blockstorageClient.Microversion = "3.47"
+		blockstorageClient.Microversion = "3.51"
 	}
 
 	mc := metrics.NewMetricContext("volume", "create")

tests/sanity/cinder/fakecloud.go

@@ -227,15 +227,16 @@ func (cloud *cloud) WaitSnapshotReady(snapshotID string) (string, error) {
 	return "available", nil
 }
 
-func (cloud *cloud) CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error) {
+func (cloud *cloud) CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error) {
 
 	backup := &backups.Backup{
-		ID:         randString(10),
-		Name:       name,
-		Status:     "available",
-		VolumeID:   volID,
-		SnapshotID: snapshotID,
-		CreatedAt:  time.Now(),
+		ID:               randString(10),
+		Name:             name,
+		Status:           "available",
+		VolumeID:         volID,
+		SnapshotID:       snapshotID,
+		AvailabilityZone: &availabilityZone,
+		CreatedAt:        time.Now(),
 	}
 
 	cloud.backups[backup.ID] = backup