Merge "Fix action on services raises TypeError"

cinder/api/contrib/services.py

@@ -112,13 +112,21 @@ class ServiceController(wsgi.Controller):
 
         return True
 
+    def _volume_api_proxy(self, fun, *args):
+        try:
+            return fun(*args)
+        except exception.ServiceNotFound as ex:
+            raise exception.InvalidInput(ex.msg)
+
     def _freeze(self, context, req, body):
         cluster_name, host = common.get_cluster_host(req, body, '3.26')
-        return self.volume_api.freeze_host(context, host, cluster_name)
+        return self._volume_api_proxy(self.volume_api.freeze_host, context,
+                                      host, cluster_name)
 
     def _thaw(self, context, req, body):
         cluster_name, host = common.get_cluster_host(req, body, '3.26')
-        return self.volume_api.thaw_host(context, host, cluster_name)
+        return self._volume_api_proxy(self.volume_api.thaw_host, context,
+                                      host, cluster_name)
 
     def _failover(self, context, req, body, clustered):
         # We set version to None to always get the cluster name from the body,
@@ -126,8 +134,8 @@ class ServiceController(wsgi.Controller):
         # it if the requested version is 3.26 or higher.
         version = '3.26' if clustered else False
         cluster_name, host = common.get_cluster_host(req, body, version)
-        self.volume_api.failover(context, host, cluster_name,
-                                 body.get('backend_id'))
+        self._volume_api_proxy(self.volume_api.failover, context, host,
+                               cluster_name, body.get('backend_id'))
         return webob.Response(status_int=http_client.ACCEPTED)
 
     def update(self, req, id, body):

cinder/tests/unit/api/contrib/test_services.py

@@ -795,6 +795,40 @@ class ServicesTest(test.TestCase):
                                               mock.sentinel.backend_id)
         self.assertEqual(202, res.status_code)
 
+    @ddt.data(('failover_host', {'host': mock.sentinel.host,
+                                 'backend_id': mock.sentinel.backend_id}),
+              ('freeze', {'host': mock.sentinel.host}),
+              ('thaw', {'host': mock.sentinel.host}))
+    @ddt.unpack
+    @mock.patch('cinder.objects.ServiceList.get_all')
+    def test_services_action_host_not_found(self, method, body,
+                                            mock_get_all_services):
+        url = '/v2/%s/os-services/%s' % (fake.PROJECT_ID, method)
+        req = fakes.HTTPRequest.blank(url)
+        mock_get_all_services.return_value = []
+        msg = 'No service found with host=%s' % mock.sentinel.host
+        result = self.assertRaises(exception.InvalidInput,
+                                   self.controller.update,
+                                   req, method, body)
+        self.assertEqual(msg, result.msg)
+
+    @ddt.data(('failover', {'cluster': mock.sentinel.cluster,
+                            'backend_id': mock.sentinel.backend_id}),
+              ('freeze', {'cluster': mock.sentinel.cluster}),
+              ('thaw', {'cluster': mock.sentinel.cluster}))
+    @ddt.unpack
+    @mock.patch('cinder.objects.ServiceList.get_all')
+    def test_services_action_cluster_not_found(self, method, body,
+                                               mock_get_all_services):
+        url = '/v3/%s/os-services/%s' % (fake.PROJECT_ID, method)
+        req = fakes.HTTPRequest.blank(url, version='3.26')
+        mock_get_all_services.return_value = []
+        msg = 'No service found with cluster=%s' % mock.sentinel.cluster
+        result = self.assertRaises(exception.InvalidInput,
+                                   self.controller.update, req,
+                                   method, body)
+        self.assertEqual(msg, result.msg)
+
     def test_services_freeze(self):
         url = '/v2/%s/os-services/freeze' % fake.PROJECT_ID
         req = fakes.HTTPRequest.blank(url)

cinder/volume/api.py

@@ -1718,10 +1718,12 @@ class API(base.Base):
                            'binary': constants.VOLUME_BINARY})
 
         if not services:
-            msg = _('No service found with ') + (
-                'host=%(host)s' if host else 'cluster=%(cluster_name)s')
-            raise exception.ServiceNotFound(msg, host=host,
-                                            cluster_name=cluster_name)
+            if host:
+                msg = _("No service found with host=%s") % host
+            else:
+                msg = _("No service found with cluster=%s") % cluster_name
+
+            raise exception.ServiceNotFound(msg)
 
         cluster = services[0].cluster
         # Check that the host or cluster we received only results in 1 host or