From dbb854635fdb988e28acf87d221fa278e6cb8c0c Mon Sep 17 00:00:00 2001 From: xiaoxi_chen Date: Thu, 11 Jul 2013 15:22:04 +0800 Subject: [PATCH] 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 --- cinder/api/contrib/backups.py | 2 + cinder/backup/api.py | 24 ++++- cinder/tests/api/contrib/test_backups.py | 106 +++++++++++++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py index ac4bd3552..c41b637d9 100644 --- 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 644de210c..18dc57c2f 100644 --- 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 c757e78bc..a7c5c73fd 100644 --- 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' % -- 2.45.2