Do not ignore availability_zone in backup creation

This patch corrects a regression introduced by change
Ie2afb57c4861c4.

Change-Id: I8b1b09ecf2ad1e19a557a664516251cae3ddf2ae
Closes-Bug: #1912624

cinder/backup/api.py

@@ -349,7 +349,8 @@ class API(base.Base):
             'data_timestamp': data_timestamp,
             'parent': parent,
             'host': latest_host,
-            'metadata': metadata or {}
+            'metadata': metadata or {},
+            'availability_zone': availability_zone
         }
         try:
             backup = objects.Backup(context=context, **kwargs)

cinder/scheduler/driver.py

@@ -155,7 +155,7 @@ class Scheduler(object):
         raise NotImplementedError(_(
             "Must implement schedule_get_pools"))
 
-    def get_backup_host(self, volume, driver=None):
+    def get_backup_host(self, volume, availability_zone, driver=None):
         """Must override schedule method for scheduler to work."""
         raise NotImplementedError(_(
             "Must implement get_backup_host"))

cinder/scheduler/filter_scheduler.py

@@ -22,7 +22,7 @@ Weighing Functions.
 
 from __future__ import annotations
 
-from typing import Optional
+from typing import (Optional, Union)  # noqa: H301
 
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -604,5 +604,10 @@ class FilterScheduler(driver.Scheduler):
         LOG.debug("Choosing %s", backend_state.backend_id)
         return top_backend
 
-    def get_backup_host(self, volume: objects.Volume, driver=None):
-        return self.host_manager.get_backup_host(volume, driver)
+    def get_backup_host(self,
+                        volume: objects.Volume,
+                        availability_zone: Union[str, None],
+                        driver=None):
+        return self.host_manager.get_backup_host(volume,
+                                                 availability_zone,
+                                                 driver)

cinder/scheduler/host_manager.py

@@ -990,12 +990,26 @@ class HostManager(object):
         # we just convert them into string to compare them.
         return str(value) == str(capability)
 
-    def get_backup_host(self, volume: objects.Volume, driver=None) -> str:
+    def get_az(self,
+               volume: objects.Volume,
+               availability_zone: Union[str, None]) -> Union[str, None]:
+        if availability_zone:
+            az = availability_zone
+        elif volume:
+            az = volume.availability_zone
+        else:
+            az = None
+        return az
+
+    def get_backup_host(self,
+                        volume: objects.Volume,
+                        availability_zone: Union[str, None],
+                        driver=None) -> str:
         if volume:
             volume_host = volume_utils.extract_host(volume.host, 'host')
         else:
             volume_host = None
-        az = volume.availability_zone if volume else None
+        az = self.get_az(volume, availability_zone)
         return self._get_available_backup_service_host(volume_host, az, driver)
 
     def _get_any_available_backup_service(self, availability_zone,

cinder/scheduler/manager.py

@@ -635,11 +635,14 @@ class SchedulerManager(manager.CleanableManager, manager.Manager):
         return requested, not_requested
 
     def create_backup(self, context, backup):
+        availability_zone = backup.availability_zone
         volume_id = backup.volume_id
         volume = self.db.volume_get(context, volume_id)
         try:
+            # Bug #1952805: an incremental backup will already have a host set,
+            # and we must respect it
             if not backup.host:
-                host = self.driver.get_backup_host(volume)
+                host = self.driver.get_backup_host(volume, availability_zone)
                 backup.host = host
                 backup.save()
             self.backup_api.create_backup(context, backup)

cinder/tests/unit/backup/test_backup.py

@@ -2295,3 +2295,28 @@ class BackupAPITestCase(BaseBackupTest):
                           volume_id, None,
                           incremental=is_incremental)
         mock_rollback.assert_called_with(self.ctxt, "fake_reservation")
+
+    @mock.patch('cinder.scheduler.rpcapi.SchedulerAPI.create_backup')
+    @mock.patch.object(api.API, '_get_available_backup_service_host',
+                       return_value='fake_host')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
+    def test_create_backup_with_specified_az(
+            self, mock_reserve, mock_rollback, mock_get_service,
+            mock_create):
+
+        self.ctxt.user_id = 'fake_user'
+        self.ctxt.project_id = 'fake_project'
+        mock_reserve.return_value = 'fake_reservation'
+        mock_get_service.return_value = 'host_1'
+
+        volume_id = self._create_volume_db_entry(status='available',
+                                                 host='testhost#rbd',
+                                                 size=1)
+        backup = self.api.create(self.ctxt,
+                                 name='test',
+                                 description='test',
+                                 volume_id=volume_id,
+                                 container='test',
+                                 availability_zone='test_az')
+        self.assertEqual('test_az', backup.availability_zone)

cinder/tests/unit/scheduler/test_scheduler.py

@@ -29,6 +29,7 @@ from cinder import exception
 from cinder.message import message_field
 from cinder import objects
 from cinder.scheduler import driver
+from cinder.scheduler import host_manager
 from cinder.scheduler import manager
 from cinder.tests.unit.backup import fake_backup
 from cinder.tests.unit import fake_constants as fake
@@ -601,7 +602,7 @@ class SchedulerManagerTestCase(test.TestCase):
         self.manager.create_backup(self.context, backup=backup)
 
         mock_save.assert_called_once()
-        mock_host.assert_called_once_with(volume)
+        mock_host.assert_called_once_with(volume, None)
         mock_volume_get.assert_called_once_with(self.context, backup.volume_id)
         mock_create.assert_called_once_with(self.context, backup)
 
@@ -642,7 +643,7 @@ class SchedulerManagerTestCase(test.TestCase):
 
         self.manager.create_backup(self.context, backup=backup)
 
-        mock_host.assert_called_once_with(volume)
+        mock_host.assert_called_once_with(volume, None)
         mock_volume_get.assert_called_once_with(self.context, backup.volume_id)
         mock_volume_update.assert_called_once_with(
             self.context,
@@ -652,6 +653,14 @@ class SchedulerManagerTestCase(test.TestCase):
         mock_error.assert_called_once_with(
             backup, 'Service not found for creating backup.')
 
+    def test_get_az(self):
+        volume = fake_volume.fake_db_volume()
+        volume['status'] = 'backing-up'
+        volume['previous_status'] = 'available'
+        hm = host_manager.HostManager()
+        az = hm.get_az(volume, availability_zone='test_az')
+        self.assertEqual('test_az', az)
+
 
 class SchedulerTestCase(test.TestCase):
     """Test case for base scheduler driver class."""

releasenotes/notes/bug-1912624-bakup-a-z-regression-452f4bc9dfd41871.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1912624 <https://bugs.launchpad.net/cinder/+bug/1912624>`_:
+    Corrected regression introduced by the refactoring of the backup
+    service in the ussuri release, which prevented the creation of
+    a volume backup in a different availability zone.