From 251687de151813e57428300d443ba5510cbb61d1 Mon Sep 17 00:00:00 2001 From: Qiu Yu Date: Tue, 2 Jul 2013 15:14:36 +0800 Subject: [PATCH] Make os-services API extension consistent Updates the os-services API extension so that it is consistent internally (index and update return similar formats), and so that it works with the following cinderclient code changes which sends the the following request body format: {"binary": "cinder-volume", "host": "host1"} It addresses a similar issue which happens in nova os-services extention before. https://bugs.launchpad.net/nova/+bug/1147746 This change added 'binary' key in request and 'status' key in response while still keeping 'service' ane 'disabled' key for API compatibility sake. Implements blueprint os-services-extension Change-Id: I7f3fa889294ca6caebdf46b8689345bcac1cdf54 --- cinder/api/contrib/services.py | 36 +++++++++-- cinder/tests/api/contrib/test_services.py | 73 +++++++++++++++++++++-- 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py index 400d24893..b8b9bc9be 100644 --- a/cinder/api/contrib/services.py +++ b/cinder/api/contrib/services.py @@ -51,10 +51,16 @@ class ServicesIndexTemplate(xmlutil.TemplateBuilder): class ServicesUpdateTemplate(xmlutil.TemplateBuilder): def construct(self): + # TODO(uni): template elements of 'host', 'service' and 'disabled' + # should be deprecated to make ServicesUpdateTemplate consistent + # with ServicesIndexTemplate. Still keeping it here for API + # compability sake. root = xmlutil.TemplateElement('host') root.set('host') root.set('service') root.set('disabled') + root.set('binary') + root.set('status') return xmlutil.MasterTemplate(root, 1) @@ -76,10 +82,18 @@ class ServiceController(object): service = '' if 'service' in req.GET: service = req.GET['service'] + LOG.deprecated(_("Query by service parameter is deprecated. " + "Please use binary parameter instead.")) + binary = '' + if 'binary' in req.GET: + binary = req.GET['binary'] + if host: services = [s for s in services if s['host'] == host] - if service: - services = [s for s in services if s['binary'] == service] + # NOTE(uni): deprecating service request key, binary takes precedence + binary_key = binary or service + if binary_key: + services = [s for s in services if s['binary'] == binary_key] svcs = [] for svc in services: @@ -110,12 +124,19 @@ class ServiceController(object): try: host = body['host'] - service = body['service'] except (TypeError, KeyError): raise webob.exc.HTTPBadRequest() + # NOTE(uni): deprecating service request key, binary takes precedence + # Still keeping service key here for API compability sake. + service = body.get('service', '') + binary = body.get('binary', '') + binary_key = binary or service + if not binary_key: + raise webob.exc.HTTPBadRequest() + try: - svc = db.service_get_by_args(context, host, service) + svc = db.service_get_by_args(context, host, binary_key) if not svc: raise webob.exc.HTTPNotFound('Unknown service') @@ -123,7 +144,12 @@ class ServiceController(object): except exception.ServiceNotFound: raise webob.exc.HTTPNotFound("service not found") - return {'host': host, 'service': service, 'disabled': disabled} + status = id + 'd' + return {'host': host, + 'service': service, + 'disabled': disabled, + 'binary': binary, + 'status': status} class Services(extensions.ExtensionDescriptor): diff --git a/cinder/tests/api/contrib/test_services.py b/cinder/tests/api/contrib/test_services.py index 5a8a4e55f..5e4bf5524 100644 --- a/cinder/tests/api/contrib/test_services.py +++ b/cinder/tests/api/contrib/test_services.py @@ -61,21 +61,35 @@ class FakeRequest(object): GET = {} -class FakeRequestWithSevice(object): +# NOTE(uni): deprecating service request key, binary takes precedence +# Still keeping service key here for API compability sake. +class FakeRequestWithService(object): environ = {"cinder.context": context.get_admin_context()} GET = {"service": "cinder-volume"} +class FakeRequestWithBinary(object): + environ = {"cinder.context": context.get_admin_context()} + GET = {"binary": "cinder-volume"} + + class FakeRequestWithHost(object): environ = {"cinder.context": context.get_admin_context()} GET = {"host": "host1"} +# NOTE(uni): deprecating service request key, binary takes precedence +# Still keeping service key here for API compability sake. class FakeRequestWithHostService(object): environ = {"cinder.context": context.get_admin_context()} GET = {"host": "host1", "service": "cinder-volume"} +class FakeRequestWithHostBinary(object): + environ = {"cinder.context": context.get_admin_context()} + GET = {"host": "host1", "binary": "cinder-volume"} + + def fake_service_get_all(context): return fake_services_list @@ -169,7 +183,27 @@ class ServicesTest(test.TestCase): self.assertEqual(res_dict, response) def test_services_list_with_service(self): - req = FakeRequestWithSevice() + req = FakeRequestWithService() + res_dict = self.controller.index(req) + + response = {'services': [{'binary': 'cinder-volume', + 'host': 'host1', + 'zone': 'cinder', + 'status': 'disabled', + 'state': 'up', + 'updated_at': datetime(2012, 10, 29, + 13, 42, 5)}, + {'binary': 'cinder-volume', + 'host': 'host2', + 'zone': 'cinder', + 'status': 'disabled', + 'state': 'down', + 'updated_at': datetime(2012, 9, 18, + 8, 3, 38)}]} + self.assertEqual(res_dict, response) + + def test_services_list_with_binary(self): + req = FakeRequestWithBinary() res_dict = self.controller.index(req) response = {'services': [{'binary': 'cinder-volume', @@ -201,16 +235,43 @@ class ServicesTest(test.TestCase): 13, 42, 5)}]} self.assertEqual(res_dict, response) - def test_services_enable(self): + def test_services_list_with_host_binary(self): + req = FakeRequestWithHostBinary() + res_dict = self.controller.index(req) + + response = {'services': [{'binary': 'cinder-volume', + 'host': 'host1', + 'zone': 'cinder', + 'status': 'disabled', + 'state': 'up', + 'updated_at': datetime(2012, 10, 29, + 13, 42, 5)}]} + self.assertEqual(res_dict, response) + + def test_services_enable_with_service_key(self): body = {'host': 'host1', 'service': 'cinder-volume'} req = fakes.HTTPRequest.blank('/v1/fake/os-services/enable') res_dict = self.controller.update(req, "enable", body) - self.assertEqual(res_dict['disabled'], False) + self.assertEqual(res_dict['status'], 'enabled') + + def test_services_enable_with_binary_key(self): + body = {'host': 'host1', 'binary': 'cinder-volume'} + req = fakes.HTTPRequest.blank('/v1/fake/os-services/enable') + res_dict = self.controller.update(req, "enable", body) + + self.assertEqual(res_dict['status'], 'enabled') - def test_services_disable(self): + def test_services_disable_with_service_key(self): req = fakes.HTTPRequest.blank('/v1/fake/os-services/disable') body = {'host': 'host1', 'service': 'cinder-volume'} res_dict = self.controller.update(req, "disable", body) - self.assertEqual(res_dict['disabled'], True) + self.assertEqual(res_dict['status'], 'disabled') + + def test_services_disable_with_binary_key(self): + req = fakes.HTTPRequest.blank('/v1/fake/os-services/disable') + body = {'host': 'host1', 'binary': 'cinder-volume'} + res_dict = self.controller.update(req, "disable", body) + + self.assertEqual(res_dict['status'], 'disabled') -- 2.45.2