From 70bb142aea276e250a3ca4c8a70bff7885a73fe7 Mon Sep 17 00:00:00 2001
From: Michel Nederlof <mnederlof@cloudvps.com>
Date: Wed, 6 Mar 2024 11:42:17 +0100
Subject: [PATCH] Ensure backup availability zone is populated if empty

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
(cherry picked from commit 9b7eaf2e328400b2ad00a93d8cd602ebcce63731)
---
 cinder/backup/manager.py                      |  3 ++
 cinder/tests/unit/backup/test_backup.py       | 39 ++++++++++++++++++-
 ...ility-zone-object-fix-939f93fda2c539b8.yml |  8 ++++
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/bug-1942154-backup-availability-zone-object-fix-939f93fda2c539b8.yml

diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py
index b31fdad4875..fb07c8b4758 100644
--- a/cinder/backup/manager.py
+++ b/cinder/backup/manager.py
@@ -408,6 +408,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()
 
diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py
index 356bbd166dc..95cd440df0d 100644
--- a/cinder/tests/unit/backup/test_backup.py
+++ b/cinder/tests/unit/backup/test_backup.py
@@ -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)
diff --git a/releasenotes/notes/bug-1942154-backup-availability-zone-object-fix-939f93fda2c539b8.yml b/releasenotes/notes/bug-1942154-backup-availability-zone-object-fix-939f93fda2c539b8.yml
new file mode 100644
index 00000000000..91e6e679d0a
--- /dev/null
+++ b/releasenotes/notes/bug-1942154-backup-availability-zone-object-fix-939f93fda2c539b8.yml
@@ -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.