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

diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py
index 400d24893722a06ed700ce8a00da2e90c19c752c..b8b9bc9bee87ca4114b620e1b4dd5ab9265a6771 100644 (file)
--- 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 5a8a4e55f31eefa6c350dd6824211a0f1238ef05..5e4bf5524dadafdcef3931c29c343cda485c6071 100644 (file)
--- 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')