openstack/cinder
Code Issues Proposed changes

Ensure backup availability zone is populated if empty

Browse Source 
If the availability zone is not provided when creating the backup,
the availability zone is still None after scheduling.

So just before starting the backup, we can fill the availability_zone
property with the actual az of the backup service doing the backup.

This will also fix selecting the proper host when restoring a backup.
If the az field is empty, it would randomly pick a cinder-backup service,
while it would make sense that only the services in the zone of the backup
would have access to the backup resource.

Closes-Bug: #1942154
Change-Id: Ida930463228bb60f71bb4b80d3f193dde8398a70
This commit is contained in:
Michel Nederlof 2024-03-06 11:42:17 +01:00
parent d94ecc22c0
commit 9b7eaf2e32
3 changed files with 49 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
cinder/backup/manager.py
View File

@ -406,6 +406,9 @@ class BackupManager(manager.SchedulerDependentManager):
detail=message_field.Detail.BACKUP_SERVICE_DOWN)
raise exception.InvalidBackup(reason=err)
if not backup.availability_zone:
backup.availability_zone = self.az
backup.service = self.driver_name
backup.save()

39
cinder/tests/unit/backup/test_backup.py
View File

@ -90,6 +90,7 @@ class BaseBackupTest(test.TestCase):
snapshot_id=None,
metadata=None,
parent_id=None,
availability_zone='1',
encryption_key_id=None):
"""Create a backup entry in the DB.
@ -101,7 +102,7 @@ class BaseBackupTest(test.TestCase):
kwargs['user_id'] = str(uuid.uuid4())
kwargs['project_id'] = project_id
kwargs['host'] = 'testhost'
kwargs['availability_zone'] = '1'
kwargs['availability_zone'] = availability_zone
kwargs['display_name'] = display_name
kwargs['display_description'] = display_description
kwargs['container'] = container
@ -2339,3 +2340,39 @@ class BackupAPITestCase(BaseBackupTest):
container='test',
availability_zone='test_az')
self.assertEqual('test_az', backup.availability_zone)
def test_create_backup_set_az_if_empty(self):
'''Test populating the availability_zone field if it was empty'''
vol_size = 1
vol_id = self._create_volume_db_entry(size=vol_size)
backup = self._create_backup_db_entry(volume_id=vol_id,
parent_id='mock',
availability_zone=None)
with mock.patch.object(self.backup_mgr, '_start_backup') as \
mock__start_backup:
self.backup_mgr.az = 'test_az'
self.backup_mgr.create_backup(self.ctxt, backup)
mock__start_backup.assert_called_once()
backup = db.backup_get(self.ctxt, backup.id)
self.assertEqual('test_az', backup.availability_zone)
def test_create_backup_set_az_if_provided(self):
'''Test backup availability_zone field remains populated'''
vol_size = 1
vol_id = self._create_volume_db_entry(size=vol_size)
backup = self._create_backup_db_entry(volume_id=vol_id,
parent_id='mock',
availability_zone='backup_az')
with mock.patch.object(self.backup_mgr, '_start_backup') as \
mock__start_backup:
self.backup_mgr.az = 'test_az'
self.backup_mgr.create_backup(self.ctxt, backup)
mock__start_backup.assert_called_once()
backup = db.backup_get(self.ctxt, backup.id)
self.assertEqual('backup_az', backup.availability_zone)

8
releasenotes/notes/bug-1942154-backup-availability-zone-object-fix-939f93fda2c539b8.yml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Backup `Bug #1942154 <https://bugs.launchpad.net/cinder/+bug/1942154>`_:
Fixed backup.availability_zone to be populated with availability zone of
the service that is creating the backup, if it was not provided as argument
when creating the backup. This indirectly fixes selecting the proper host
when restoring the backup as the availability zone field is now populated.