Host selection in backup service

With scaling backup service, it introduces a config called
backup_use_same_backend which indicates whether customers use
same backup driver in their environments. If the value is
set to True, Cinder selects any backup host to do backup task. If
the value is False, cinder can only select backup.host for a backup
task. Currently the default value is set to False.

This patch is to change above config to backup_use_same_host which
means whether selecting same host for tasks of a backup. The
default value is set to False. It indicates that we don't care
about backup.host when running backup task, and choose any available
host in same az.

As currently Cinder doesn't support multiple backup drivers, the change
is to make the config more sensible.

Change-Id: I1a43df251a18006363162ee8e7c5aa891f44fa3a
Closes-bug: #1554845

diff --git a/cinder/backup/api.py b/cinder/backup/api.py
index 9ac21ebab11931ecd024de147a3200a4f2839ba9..c447859ef6515d07efe5ae4ea6815abb2aab8cc3 100644 (file)
--- a/cinder/backup/api.py
+++ b/cinder/backup/api.py
@@ -43,7 +43,7 @@ import cinder.volume
 from cinder.volume import utils as volume_utils
 
 backup_api_opts = [
-    cfg.BoolOpt('backup_use_same_backend',
+    cfg.BoolOpt('backup_use_same_host',
                 default=False,
                 help='Backup services use same backend.')
 ]
@@ -209,10 +209,10 @@ class API(base.Base):
                 raise exception.ServiceNotFound(service_id='cinder-backup')
 
         backup_host = None
-        if host and self._is_backup_service_enabled(az, host):
-            backup_host = host
-        if not backup_host and (not host or CONF.backup_use_same_backend):
+        if (not host or not CONF.backup_use_same_host):
             backup_host = self._get_any_available_backup_service(az)
+        elif self._is_backup_service_enabled(az, host):
+            backup_host = host
         if not backup_host:
             raise exception.ServiceNotFound(service_id='cinder-backup')
         return backup_host

diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py
index beb6cb1f494bbadade0777da397cd27f43f0b4d8..6860a133bc6e545309243bf80daafb13f7bca39a 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_admin_actions.py
+++ b/cinder/tests/unit/api/contrib/test_admin_actions.py
@@ -106,9 +106,9 @@ class AdminActionsTest(BaseAdminTest):
         req.headers['content-type'] = 'application/json'
         req.body = jsonutils.dump_as_bytes({'os-reset_status': updated_status})
         req.environ['cinder.context'] = ctx
-        with mock.patch('cinder.backup.api.API._is_backup_service_enabled') \
-                as mock_is_service_available:
-            mock_is_service_available.return_value = True
+        with mock.patch('cinder.backup.api.API._get_available_backup_service_host') \
+                as mock_get_backup_host:
+            mock_get_backup_host.return_value = 'testhost'
             resp = req.get_response(app())
             return resp
 

diff --git a/cinder/tests/unit/api/contrib/test_backups.py b/cinder/tests/unit/api/contrib/test_backups.py
index 031ffb4cdbcd65b71e4f98bc1806a042a7406c38..33b6c4ff709dc95d330cf80fc36e739b5a1800f7 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_backups.py
+++ b/cinder/tests/unit/api/contrib/test_backups.py
@@ -1064,30 +1064,44 @@ class BackupsAPITestCase(test.TestCase):
     def test_get_available_backup_service(self,
                                           _mock_service_get_all_by_topic):
         _mock_service_get_all_by_topic.return_value = [
-            {'availability_zone': 'az1', 'host': 'testhost',
+            {'availability_zone': 'az1', 'host': 'testhost1',
              'disabled': 0, 'updated_at': timeutils.utcnow()},
-            {'availability_zone': 'az2', 'host': 'fakehost',
-             'disabled': 0, 'updated_at': timeutils.utcnow()}]
+            {'availability_zone': 'az2', 'host': 'testhost2',
+             'disabled': 0, 'updated_at': timeutils.utcnow()},
+            {'availability_zone': 'az2', 'host': 'testhost3',
+             'disabled': 0, 'updated_at': timeutils.utcnow()}, ]
         actual_host = self.backup_api._get_available_backup_service_host(
-            'testhost', 'az1')
-        self.assertEqual('testhost', actual_host)
-        self.assertRaises(exception.ServiceNotFound,
-                          self.backup_api._get_available_backup_service_host,
-                          'testhost', 'az2')
-        self.assertRaises(exception.ServiceNotFound,
-                          self.backup_api._get_available_backup_service_host,
-                          'testhost2', 'az1')
-        self.override_config('backup_use_same_backend', True)
+            None, 'az1')
+        self.assertEqual('testhost1', actual_host)
+        actual_host = self.backup_api._get_available_backup_service_host(
+            'testhost2', 'az2')
+        self.assertIn(actual_host, ['testhost2', 'testhost3'])
+        actual_host = self.backup_api._get_available_backup_service_host(
+            'testhost4', 'az1')
+        self.assertEqual('testhost1', actual_host)
+
+    @mock.patch('cinder.db.service_get_all_by_topic')
+    def test_get_available_backup_service_with_same_host(
+            self, _mock_service_get_all_by_topic):
+        _mock_service_get_all_by_topic.return_value = [
+            {'availability_zone': 'az1', 'host': 'testhost1',
+             'disabled': 0, 'updated_at': timeutils.utcnow()},
+            {'availability_zone': 'az2', 'host': 'testhost2',
+             'disabled': 0, 'updated_at': timeutils.utcnow()}, ]
+        self.override_config('backup_use_same_host', True)
         actual_host = self.backup_api._get_available_backup_service_host(
             None, 'az1')
-        self.assertEqual('testhost', actual_host)
+        self.assertEqual('testhost1', actual_host)
         actual_host = self.backup_api._get_available_backup_service_host(
-            'testhost2', 'az1')
-        self.assertEqual('testhost', actual_host)
+            'testhost2', 'az2')
+        self.assertEqual('testhost2', actual_host)
+        self.assertRaises(exception.ServiceNotFound,
+                          self.backup_api._get_available_backup_service_host,
+                          'testhost4', 'az1')
 
     @mock.patch('cinder.db.service_get_all_by_topic')
-    def test_delete_backup_available(self,
-                                     _mock_service_get_all_by_topic):
+    def test_delete_backup_available(
+            self, _mock_service_get_all_by_topic):
         _mock_service_get_all_by_topic.return_value = [
             {'availability_zone': 'az1', 'host': 'testhost',
              'disabled': 0, 'updated_at': timeutils.utcnow()}]
@@ -1219,10 +1233,10 @@ class BackupsAPITestCase(test.TestCase):
 
         db.backup_destroy(context.get_admin_context(), backup_id)
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     def test_restore_backup_volume_id_specified_json(
-            self, _mock_is_service_enabled):
-        _mock_is_service_enabled.return_value = True
+            self, _mock_get_backup_host):
+        _mock_get_backup_host.return_value = 'testhost'
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE)
         # need to create the volume referenced below first
         volume_name = 'test1'
@@ -1244,10 +1258,10 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual(volume_id, res_dict['restore']['volume_id'])
         self.assertEqual(volume_name, res_dict['restore']['volume_name'])
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     def test_restore_backup_volume_id_specified_xml(
-            self, _mock_is_service_enabled):
-        _mock_is_service_enabled.return_value = True
+            self, _mock_get_backup_host):
+        _mock_get_backup_host.return_value = 'testhost'
         volume_name = 'test1'
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE)
         volume_id = utils.create_volume(self.context,
@@ -1314,19 +1328,20 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual("Missing required element 'restore' in request body.",
                          res_dict['badRequest']['message'])
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.db.service_get_all_by_topic')
     @mock.patch('cinder.volume.api.API.create')
-    def test_restore_backup_volume_id_unspecified(self,
-                                                  _mock_volume_api_create,
-                                                  _mock_is_service_enabled):
-
+    def test_restore_backup_volume_id_unspecified(
+            self, _mock_volume_api_create,
+            _mock_service_get_all_by_topic):
         # intercept volume creation to ensure created volume
         # has status of available
         def fake_volume_api_create(context, size, name, description):
             volume_id = utils.create_volume(self.context, size=size)['id']
             return db.volume_get(context, volume_id)
 
-        _mock_is_service_enabled.return_value = True
+        _mock_service_get_all_by_topic.return_value = [
+            {'availability_zone': 'az1', 'host': 'testhost',
+             'disabled': 0, 'updated_at': timeutils.utcnow()}]
         _mock_volume_api_create.side_effect = fake_volume_api_create
 
         backup_id = self._create_backup(size=5,
@@ -1344,11 +1359,11 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual(202, res.status_int)
         self.assertEqual(backup_id, res_dict['restore']['backup_id'])
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.db.service_get_all_by_topic')
     @mock.patch('cinder.volume.api.API.create')
     def test_restore_backup_name_specified(self,
                                            _mock_volume_api_create,
-                                           _mock_is_service_enabled):
+                                           _mock_service_get_all_by_topic):
         # Intercept volume creation to ensure created volume
         # has status of available
         def fake_volume_api_create(context, size, name, description):
@@ -1357,7 +1372,9 @@ class BackupsAPITestCase(test.TestCase):
             return db.volume_get(context, volume_id)
 
         _mock_volume_api_create.side_effect = fake_volume_api_create
-        _mock_is_service_enabled.return_value = True
+        _mock_service_get_all_by_topic.return_value = [
+            {'availability_zone': 'az1', 'host': 'testhost',
+             'disabled': 0, 'updated_at': timeutils.utcnow()}]
 
         backup_id = self._create_backup(size=5,
                                         status=fields.BackupStatus.AVAILABLE)
@@ -1382,10 +1399,10 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual(202, res.status_int)
         self.assertEqual(backup_id, res_dict['restore']['backup_id'])
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     def test_restore_backup_name_volume_id_specified(
-            self, _mock_is_service_enabled):
-        _mock_is_service_enabled.return_value = True
+            self, _mock_get_backup_host):
+        _mock_get_backup_host.return_value = 'testhost'
         backup_id = self._create_backup(size=5,
                                         status=fields.BackupStatus.AVAILABLE)
         orig_vol_name = "vol-00"
@@ -1602,12 +1619,11 @@ class BackupsAPITestCase(test.TestCase):
         db.volume_destroy(context.get_admin_context(), volume_id)
         db.backup_destroy(context.get_admin_context(), backup_id)
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
-    def test_restore_backup_to_oversized_volume(
-            self, _mock_is_service_enabled):
-        _mock_is_service_enabled.return_value = True
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
+    def test_restore_backup_to_oversized_volume(self, _mock_get_backup_host):
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE,
                                         size=10)
+        _mock_get_backup_host.return_value = 'testhost'
         # need to create the volume referenced below first
         volume_name = 'test1'
         volume_id = utils.create_volume(self.context,
@@ -1632,8 +1648,8 @@ class BackupsAPITestCase(test.TestCase):
         db.backup_destroy(context.get_admin_context(), backup_id)
 
     @mock.patch('cinder.backup.rpcapi.BackupAPI.restore_backup')
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
-    def test_restore_backup_with_different_host(self, mock_is_backup_available,
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
+    def test_restore_backup_with_different_host(self, _mock_get_backup_host,
                                                 mock_restore_backup):
         volume_name = 'test1'
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE,
@@ -1642,7 +1658,7 @@ class BackupsAPITestCase(test.TestCase):
                                         host='HostB@BackendB#PoolB',
                                         display_name=volume_name)['id']
 
-        mock_is_backup_available.return_value = True
+        _mock_get_backup_host.return_value = 'testhost'
         body = {"restore": {"volume_id": volume_id, }}
         req = webob.Request.blank('/v2/fake/backups/%s/restore' %
                                   backup_id)
@@ -1656,7 +1672,7 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual(backup_id, res_dict['restore']['backup_id'])
         self.assertEqual(volume_id, res_dict['restore']['volume_id'])
         self.assertEqual(volume_name, res_dict['restore']['volume_name'])
-        mock_restore_backup.assert_called_once_with(mock.ANY, u'HostA',
+        mock_restore_backup.assert_called_once_with(mock.ANY, u'testhost',
                                                     mock.ANY, volume_id)
         # Manually check if restore_backup was called with appropriate backup.
         self.assertEqual(backup_id, mock_restore_backup.call_args[0][2].id)
@@ -1676,11 +1692,11 @@ class BackupsAPITestCase(test.TestCase):
         # request is not authorized
         self.assertEqual(403, res.status_int)
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.export_record')
     def test_export_backup_record_id_specified_json(self,
                                                     _mock_export_record_rpc,
-                                                    _mock_service_enabled):
+                                                    _mock_get_backup_host):
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE,
                                         size=10)
         ctx = context.RequestContext('admin', 'fake', is_admin=True)
@@ -1689,7 +1705,7 @@ class BackupsAPITestCase(test.TestCase):
         _mock_export_record_rpc.return_value = \
             {'backup_service': backup_service,
              'backup_url': backup_url}
-        _mock_service_enabled.return_value = True
+        _mock_get_backup_host.return_value = 'testhost'
         req = webob.Request.blank('/v2/fake/backups/%s/export_record' %
                                   backup_id)
         req.method = 'GET'
@@ -1705,11 +1721,11 @@ class BackupsAPITestCase(test.TestCase):
                          res_dict['backup-record']['backup_url'])
         db.backup_destroy(context.get_admin_context(), backup_id)
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.export_record')
     def test_export_record_backup_id_specified_xml(self,
                                                    _mock_export_record_rpc,
-                                                   _mock_service_enabled):
+                                                   _mock_get_backup_host):
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE,
                                         size=10)
         ctx = context.RequestContext('admin', 'fake', is_admin=True)
@@ -1718,7 +1734,7 @@ class BackupsAPITestCase(test.TestCase):
         _mock_export_record_rpc.return_value = \
             {'backup_service': backup_service,
              'backup_url': backup_url}
-        _mock_service_enabled.return_value = True
+        _mock_get_backup_host.return_value = 'testhost'
         req = webob.Request.blank('/v2/fake/backups/%s/export_record' %
                                   backup_id)
         req.method = 'GET'
@@ -1769,15 +1785,15 @@ class BackupsAPITestCase(test.TestCase):
                          res_dict['badRequest']['message'])
         db.backup_destroy(context.get_admin_context(), backup_id)
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.export_record')
     def test_export_record_with_unavailable_service(self,
                                                     _mock_export_record_rpc,
-                                                    _mock_service_enabled):
+                                                    _mock_get_backup_host):
         msg = 'fake unavailable service'
         _mock_export_record_rpc.side_effect = \
             exception.InvalidBackup(reason=msg)
-        _mock_service_enabled.return_value = True
+        _mock_get_backup_host.return_value = 'testhost'
         backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE)
         ctx = context.RequestContext('admin', 'fake', is_admin=True)
         req = webob.Request.blank('/v2/fake/backups/%s/export_record' %

diff --git a/cinder/tests/unit/test_backup.py b/cinder/tests/unit/test_backup.py
index 7e7152ed7a268e815d127d3ec2b7ce970ca96450..8c33e13fd42963ff0844b0f43e9a53df11b6a7ea 100644 (file)
--- a/cinder/tests/unit/test_backup.py
+++ b/cinder/tests/unit/test_backup.py
@@ -1361,16 +1361,16 @@ class BackupAPITestCase(BaseBackupTest):
                           volume_id=volume_id,
                           container='volumebackups')
 
-    @mock.patch('cinder.backup.api.API._is_backup_service_enabled')
+    @mock.patch('cinder.backup.api.API._get_available_backup_service_host')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.restore_backup')
     def test_restore_volume(self,
                             mock_rpcapi_restore,
-                            mock_is_service_enabled):
+                            mock_get_backup_host):
         volume_id = self._create_volume_db_entry(status='available',
                                                  size=1)
         backup = self._create_backup_db_entry(size=1,
                                               status='available')
-        mock_is_service_enabled.return_value = True
+        mock_get_backup_host.return_value = 'testhost'
         self.api.restore(self.ctxt, backup.id, volume_id)
         backup = objects.Backup.get_by_id(self.ctxt, backup.id)
         self.assertEqual(volume_id, backup.restore_volume_id)