Skip to content

Commit b343c1b

Browse files
zetaabzetaab
authored
[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
1 parent dab0f06 commit b343c1b

File tree

8 files changed

+43
-32
lines changed

8 files changed

+43
-32
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ helm install --namespace kube-system --name cinder-csi ./charts/cinder-csi-plugi
269269
| VolumeSnapshotClass `parameters` | `force-create` | `false` | Enable to support creating snapshot for a volume in in-use status |
270270
| 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 |
271271
| 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 |
272+
| VolumeSnapshotClass `parameters` | `availability` | Same as volume | String. Backup Availability Zone |
272273
| Inline Volume `volumeAttributes` | `capacity` | `1Gi` | volume size for creating inline volumes|
273274
| Inline Volume `VolumeAttributes` | `type` | Empty String | Name/ID of Volume type. Corresponding volume type should exist in cinder |
274275

‎pkg/csi/cinder/controllerserver.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
136136
// In case a snapshot is not found
137137
// check if a Backup with the same ID exists
138138
if backupsAreEnabled && cpoerrors.IsNotFound(err) {
139-
back, err := cloud.GetBackupByID(snapshotID)
139+
var back *backups.Backup
140+
back, err = cloud.GetBackupByID(snapshotID)
140141
if err != nil {
141142
//If there is an error getting the backup as well, fail.
142143
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
154155
if cpoerrors.IsNotFound(err) && snapshotID == "" {
155156
return nil, err
156157
}
157-
158158
}
159159

160160
if content != nil && content.GetVolume() != nil {
@@ -420,10 +420,17 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
420420
}
421421

422422
if len(backups) == 1 {
423-
backup = &backups[0]
423+
// since backup.VolumeID is not part of ListBackups response
424+
// we need fetch single backup to get the full object.
425+
backup, err = cs.Cloud.GetBackupByID(backups[0].ID)
426+
if err != nil {
427+
klog.Errorf("Failed to get backup by ID %s: %v", backup.ID, err)
428+
return nil, status.Error(codes.Internal, "Failed to get backup by ID")
429+
}
424430

425431
// Verify the existing backup has the same VolumeID, otherwise it belongs to another volume
426432
if backup.VolumeID != volumeID {
433+
klog.Errorf("found existing backup for volumeID (%s) but different source volume ID (%s)", volumeID, backup.VolumeID)
427434
return nil, status.Error(codes.AlreadyExists, "Backup with given name already exists, with different source volume ID")
428435
}
429436

@@ -503,7 +510,7 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
503510
return nil, status.Error(codes.Internal, fmt.Sprintf("GetBackupByID failed with error %v", err))
504511
}
505512

506-
err = cs.Cloud.DeleteSnapshot(snap.ID)
513+
err = cs.Cloud.DeleteSnapshot(backup.SnapshotID)
507514
if err != nil && !cpoerrors.IsNotFound(err) {
508515
klog.Errorf("Failed to DeleteSnapshot: %v", err)
509516
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
593600
}
594601
}
595602

596-
backup, err := cs.Cloud.CreateBackup(name, volumeID, snap.ID, properties)
603+
backup, err := cs.Cloud.CreateBackup(name, volumeID, snap.ID, parameters[openstack.SnapshotAvailabilityZone], properties)
597604
if err != nil {
598605
klog.Errorf("Failed to Create backup: %v", err)
599606
return nil, status.Error(codes.Internal, fmt.Sprintf("CreateBackup failed with error %v", err))

‎pkg/csi/cinder/openstack/openstack.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ type IOpenStack interface {
6161
DeleteSnapshot(snapID string) error
6262
GetSnapshotByID(snapshotID string) (*snapshots.Snapshot, error)
6363
WaitSnapshotReady(snapshotID string) (string, error)
64-
CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error)
64+
CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error)
6565
ListBackups(filters map[string]string) ([]backups.Backup, error)
6666
DeleteBackup(backupID string) error
6767
GetBackupByID(backupID string) (*backups.Backup, error)

‎pkg/csi/cinder/openstack/openstack_backups.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const (
4444

4545
// CreateBackup issues a request to create a Backup from the specified Snapshot with the corresponding ID and
4646
// returns the resultant gophercloud Backup Item upon success.
47-
func (os *OpenStack) CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error) {
47+
func (os *OpenStack) CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error) {
4848
blockstorageServiceClient, err := openstack.NewBlockStorageV3(os.blockstorage.ProviderClient, os.epOpts)
4949
if err != nil {
5050
return &backups.Backup{}, err
@@ -63,16 +63,17 @@ func (os *OpenStack) CreateBackup(name, volID string, snapshotID string, tags ma
6363
}
6464

6565
opts := &backups.CreateOpts{
66-
VolumeID: volID,
67-
SnapshotID: snapshotID,
68-
Name: name,
69-
Force: force,
70-
Description: backupDescription,
66+
VolumeID: volID,
67+
SnapshotID: snapshotID,
68+
Name: name,
69+
Force: force,
70+
Description: backupDescription,
71+
AvailabilityZone: availabilityZone,
7172
}
7273

7374
if tags != nil {
74-
// Set openstack microversion to 3.43 to send metadata along with the backup
75-
blockstorageServiceClient.Microversion = "3.43"
75+
// Set openstack microversion to 3.51 to send metadata along with the backup
76+
blockstorageServiceClient.Microversion = "3.51"
7677
opts.Metadata = tags
7778
}
7879

‎pkg/csi/cinder/openstack/openstack_mock.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -315,21 +315,21 @@ func (_m *OpenStackMock) ListBackups(filters map[string]string) ([]backups.Backu
315315
return r0, r1
316316
}
317317

318-
func (_m *OpenStackMock) CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error) {
319-
ret := _m.Called(name, volID, snapshotID, tags)
318+
func (_m *OpenStackMock) CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error) {
319+
ret := _m.Called(name, volID, snapshotID, availabilityZone, tags)
320320

321321
var r0 *backups.Backup
322-
if rf, ok := ret.Get(0).(func(string, string, string, map[string]string) *backups.Backup); ok {
323-
r0 = rf(name, volID, snapshotID, tags)
322+
if rf, ok := ret.Get(0).(func(string, string, string, string, map[string]string) *backups.Backup); ok {
323+
r0 = rf(name, volID, snapshotID, availabilityZone, tags)
324324
} else {
325325
if ret.Get(0) != nil {
326326
r0 = ret.Get(0).(*backups.Backup)
327327
}
328328
}
329329

330330
var r1 error
331-
if rf, ok := ret.Get(1).(func(string, string, string, map[string]string) error); ok {
332-
r1 = rf(name, volID, snapshotID, tags)
331+
if rf, ok := ret.Get(1).(func(string, string, string, string, map[string]string) error); ok {
332+
r1 = rf(name, volID, snapshotID, availabilityZone, tags)
333333
} else {
334334
r1 = ret.Error(1)
335335
}

‎pkg/csi/cinder/openstack/openstack_snapshots.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,10 @@ const (
3737
snapReadyFactor = 1.2
3838
snapReadySteps = 10
3939

40-
snapshotDescription = "Created by OpenStack Cinder CSI driver"
41-
SnapshotForceCreate = "force-create"
42-
SnapshotType = "type"
40+
snapshotDescription = "Created by OpenStack Cinder CSI driver"
41+
SnapshotForceCreate = "force-create"
42+
SnapshotType = "type"
43+
SnapshotAvailabilityZone = "availability"
4344
)
4445

4546
// CreateSnapshot issues a request to take a Snapshot of the specified Volume with the corresponding ID and

‎pkg/csi/cinder/openstack/openstack_volumes.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ func (os *OpenStack) CreateVolume(name string, size int, vtype, availability str
7272
return nil, err
7373
}
7474

75-
// creating volumes from backups is available since 3.47 microversion
75+
// creating volumes from backups and backups cross-az is available since 3.51 microversion
7676
// https://docs.openstack.org/cinder/latest/contributor/api_microversion_history.html#id47
7777
if !os.bsOpts.IgnoreVolumeMicroversion && sourceBackupID != "" {
78-
blockstorageClient.Microversion = "3.47"
78+
blockstorageClient.Microversion = "3.51"
7979
}
8080

8181
mc := metrics.NewMetricContext("volume", "create")

‎tests/sanity/cinder/fakecloud.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -227,15 +227,16 @@ func (cloud *cloud) WaitSnapshotReady(snapshotID string) (string, error) {
227227
return "available", nil
228228
}
229229

230-
func (cloud *cloud) CreateBackup(name, volID string, snapshotID string, tags map[string]string) (*backups.Backup, error) {
230+
func (cloud *cloud) CreateBackup(name, volID, snapshotID, availabilityZone string, tags map[string]string) (*backups.Backup, error) {
231231

232232
backup := &backups.Backup{
233-
ID: randString(10),
234-
Name: name,
235-
Status: "available",
236-
VolumeID: volID,
237-
SnapshotID: snapshotID,
238-
CreatedAt: time.Now(),
233+
ID: randString(10),
234+
Name: name,
235+
Status: "available",
236+
VolumeID: volID,
237+
SnapshotID: snapshotID,
238+
AvailabilityZone: &availabilityZone,
239+
CreatedAt: time.Now(),
239240
}
240241

241242
cloud.backups[backup.ID] = backup

0 commit comments

Comments
 (0)