Check enabled backup service before rpc request

In previous code we didn't check whether we have an enabled
backup service there before we send out the rpc request,
results that if no enabled backup service there,the volume will
stays in "backing-up" state and the backup will stays in "creating".

This patch fixed this issue, we exam whether at least an appropriate
(same host,az as the volume and is_alive) backup service available
before we do the rpc request.

Fixes: bug #1200040
Change-Id: I77154528f489ed20f7b784e6fcefccf15dc81d1d

diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py
index ac4bd3552c4dabe320dc77aaf4e45f5c8993b03a..c41b637d95d8cf8a2aa755255e675eae9ee56e8b 100644 (file)
--- a/cinder/api/contrib/backups.py
+++ b/cinder/api/contrib/backups.py
@@ -210,6 +210,8 @@ class BackupsController(wsgi.Controller):
             raise exc.HTTPBadRequest(explanation=unicode(error))
         except exception.VolumeNotFound as error:
             raise exc.HTTPNotFound(explanation=unicode(error))
+        except exception.ServiceNotFound as error:
+            raise exc.HTTPInternalServerError(explanation=unicode(error))
 
         retval = self._view_builder.summary(req, dict(new_backup.iteritems()))
         return retval

diff --git a/cinder/backup/api.py b/cinder/backup/api.py
index 644de210cb06d2a0b996ff6d3cc35e78923e735e..18dc57c2f32a9edf2530662a157a059f382975c5 100644 (file)
--- a/cinder/backup/api.py
+++ b/cinder/backup/api.py
@@ -17,16 +17,22 @@
 Handles all requests relating to the volume backups service.
 """
 
+
 from eventlet import greenthread
 
+from oslo.config import cfg
+
 from cinder.backup import rpcapi as backup_rpcapi
+from cinder import context
 from cinder.db import base
 from cinder import exception
 from cinder.openstack.common import log as logging
+from cinder import utils
+
 import cinder.policy
 import cinder.volume
 
-
+CONF = cfg.CONF
 LOG = logging.getLogger(__name__)
 
 
@@ -78,6 +84,20 @@ class API(base.Base):
 
         return backups
 
+    def _check_backup_service(self, volume):
+        """
+        Check if there is an backup service available
+        """
+        topic = CONF.backup_topic
+        ctxt = context.get_admin_context()
+        services = self.db.service_get_all_by_topic(ctxt, topic)
+        for srv in services:
+            if (srv['availability_zone'] == volume['availability_zone'] and
+                    srv['host'] == volume['host'] and not srv['disabled'] and
+                    utils.service_is_up(srv)):
+                return True
+        return False
+
     def create(self, context, name, description, volume_id,
                container, availability_zone=None):
         """
@@ -103,6 +123,8 @@ class API(base.Base):
                    'host': volume['host'], }
 
         backup = self.db.backup_create(context, options)
+        if not self._check_backup_service(volume):
+            raise exception.ServiceNotFound(service_id='cinder-backup')
 
         #TODO(DuncanT): In future, when we have a generic local attach,
         #               this can go via the scheduler, which enables

diff --git a/cinder/tests/api/contrib/test_backups.py b/cinder/tests/api/contrib/test_backups.py
index c757e78bc64543a06041466ae5dfb784889a094a..a7c5c73fd1d3334005ae24ded74aea828b5959ce 100644 (file)
--- a/cinder/tests/api/contrib/test_backups.py
+++ b/cinder/tests/api/contrib/test_backups.py
@@ -28,6 +28,7 @@ from cinder import context
 from cinder import db
 from cinder import exception
 from cinder.openstack.common import log as logging
+from cinder.openstack.common import timeutils
 from cinder import test
 from cinder.tests.api import fakes
 # needed for stubs to work
@@ -42,6 +43,8 @@ class BackupsAPITestCase(test.TestCase):
 
     def setUp(self):
         super(BackupsAPITestCase, self).setUp()
+        self.volume_api = cinder.volume.API()
+        self.backup_api = cinder.backup.API()
 
     def tearDown(self):
         super(BackupsAPITestCase, self).tearDown()
@@ -78,6 +81,8 @@ class BackupsAPITestCase(test.TestCase):
     def _create_volume(display_name='test_volume',
                        display_description='this is a test volume',
                        status='creating',
+                       availability_zone='fake_az',
+                       host='fake_host',
                        size=1):
         """Create a volume object."""
         vol = {}
@@ -88,8 +93,15 @@ class BackupsAPITestCase(test.TestCase):
         vol['display_name'] = display_name
         vol['display_description'] = display_description
         vol['attach_status'] = 'detached'
+        vol['availability_zone'] = availability_zone
+        vol['host'] = host
         return db.volume_create(context.get_admin_context(), vol)['id']
 
+    @staticmethod
+    def _stub_service_get_all_by_topic(context, topic):
+        return [{'availability_zone': "fake_az", 'host': 'fake_host',
+                 'disabled': 0, 'updated_at': timeutils.utcnow()}]
+
     def test_show_backup(self):
         volume_id = self._create_volume(size=5)
         backup_id = self._create_backup(volume_id)
@@ -343,6 +355,8 @@ class BackupsAPITestCase(test.TestCase):
         db.backup_destroy(context.get_admin_context(), backup_id1)
 
     def test_create_backup_json(self):
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic',
+                       self._stub_service_get_all_by_topic)
         volume_id = self._create_volume(status='available', size=5)
         body = {"backup": {"display_name": "nightly001",
                            "display_description":
@@ -366,6 +380,8 @@ class BackupsAPITestCase(test.TestCase):
         db.volume_destroy(context.get_admin_context(), volume_id)
 
     def test_create_backup_xml(self):
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic',
+                       self._stub_service_get_all_by_topic)
         volume_size = 2
         volume_id = self._create_volume(status='available', size=volume_size)
 
@@ -466,6 +482,96 @@ class BackupsAPITestCase(test.TestCase):
                          'Invalid volume: Volume to be backed up must'
                          ' be available')
 
+    def test_create_backup_WithOUT_enabled_backup_service(self):
+        # need an enabled backup service available
+        def stub_empty_service_get_all_by_topic(ctxt, topic):
+            return []
+
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic',
+                       stub_empty_service_get_all_by_topic)
+        volume_size = 2
+        volume_id = self._create_volume(status='available', size=volume_size)
+
+        req = webob.Request.blank('/v2/fake/backups')
+        body = {"backup": {"display_name": "nightly001",
+                           "display_description":
+                           "Nightly Backup 03-Sep-2012",
+                           "volume_id": volume_id,
+                           "container": "nightlybackups",
+                           }
+                }
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        req.headers['Accept'] = 'application/json'
+        req.body = json.dumps(body)
+
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+        self.assertEqual(res.status_int, 500)
+        self.assertEqual(res_dict['computeFault']['code'], 500)
+        self.assertEqual(res_dict['computeFault']['message'],
+                         'Service cinder-backup could not be found.')
+
+    def test_check_backup_service(self):
+        def empty_service(ctxt, topic):
+            return []
+
+        #service host not match with volume's host
+        def host_not_match(context, topic):
+            return [{'availability_zone': "fake_az", 'host': 'strange_host',
+                     'disabled': 0, 'updated_at': timeutils.utcnow()}]
+
+        #service az not match with volume's az
+        def az_not_match(context, topic):
+            return [{'availability_zone': "strange_az", 'host': 'fake_host',
+                     'disabled': 0, 'updated_at': timeutils.utcnow()}]
+
+        #service disabled
+        def disabled_service(context, topic):
+            return [{'availability_zone': "fake_az", 'host': 'fake_host',
+                     'disabled': 1, 'updated_at': timeutils.utcnow()}]
+
+        #dead service that last reported at 20th centry
+        def dead_service(context, topic):
+            return [{'availability_zone': "fake_az", 'host': 'strange_host',
+                     'disabled': 0, 'updated_at': '1989-04-16 02:55:44'}]
+
+        #first service's host not match but second one works.
+        def multi_services(context, topic):
+            return [{'availability_zone': "fake_az", 'host': 'strange_host',
+                     'disabled': 0, 'updated_at': timeutils.utcnow()},
+                    {'availability_zone': "fake_az", 'host': 'fake_host',
+                     'disabled': 0, 'updated_at': timeutils.utcnow()}]
+
+        volume_id = self._create_volume(status='available', size=2,
+                                        availability_zone='fake_az',
+                                        host='fake_host')
+        volume = self.volume_api.get(context.get_admin_context(), volume_id)
+
+        #test empty service
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic', empty_service)
+        self.assertEqual(self.backup_api._check_backup_service(volume), False)
+
+        #test host not match service
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic', host_not_match)
+        self.assertEqual(self.backup_api._check_backup_service(volume), False)
+
+        #test az not match service
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic', az_not_match)
+        self.assertEqual(self.backup_api._check_backup_service(volume), False)
+
+        #test disabled service
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic', disabled_service)
+        self.assertEqual(self.backup_api._check_backup_service(volume), False)
+
+        #test dead service
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic', dead_service)
+        self.assertEqual(self.backup_api._check_backup_service(volume), False)
+
+        #test multi services and the last service matches
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic', multi_services)
+        self.assertEqual(self.backup_api._check_backup_service(volume), True)
+
     def test_delete_backup_available(self):
         backup_id = self._create_backup(status='available')
         req = webob.Request.blank('/v2/fake/backups/%s' %