Give a way to save why a service has been disabled

Port from nova https://review.openstack.org/#/c/26020/

We added a field to the table of service to log a reason when a
service has been disabled.

We added a new API extension called os-extended-services.

The new extension will extend the os-services extension adding a
method for disabling a service and specify a reason for that:
PUT /v2/{tenant_id}/os-services/disable-log-reason

When the os-extended-extension is loaded the call:
GET /V2/{tenant_id}/os-services
will return the list of services with disable reason information
if that exists.

DocImpact

Change-Id: I885e43132b0e49b63f7858abd3fcffe1b78d3fd8
Implements bp record-reason-for-disabling-service

diff --git a/cinder/api/contrib/extended_services.py b/cinder/api/contrib/extended_services.py
new file mode 100644 (file)
index 0000000..97e95b9
--- /dev/null
+++ b/cinder/api/contrib/extended_services.py
@@ -0,0 +1,25 @@
+# Copyright 2014 IBM Corp.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from cinder.api import extensions
+
+
+class Extended_services(extensions.ExtensionDescriptor):
+    """Extended services support."""
+
+    name = "ExtendedServices"
+    alias = "os-extended-services"
+    namespace = ("http://docs.openstack.org/volume/ext/"
+                 "extended_services/api/v2")
+    updated = "2014-01-10T00:00:00-00:00"

diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py
index 998208665b53c1d637984048830115589a13eaf8..8fde5b874b27b4d3b1cef7f54c7ee8437b61efd3 100644 (file)
--- a/cinder/api/contrib/services.py
+++ b/cinder/api/contrib/services.py
@@ -43,6 +43,7 @@ class ServicesIndexTemplate(xmlutil.TemplateBuilder):
         elem.set('status')
         elem.set('state')
         elem.set('update_at')
+        elem.set('disabled_reason')
 
         return xmlutil.MasterTemplate(root, 1)
 
@@ -59,11 +60,16 @@ class ServicesUpdateTemplate(xmlutil.TemplateBuilder):
         root.set('disabled')
         root.set('binary')
         root.set('status')
+        root.set('disabled_reason')
 
         return xmlutil.MasterTemplate(root, 1)
 
 
 class ServiceController(wsgi.Controller):
+    def __init__(self, ext_mgr=None):
+        self.ext_mgr = ext_mgr
+        super(ServiceController, self).__init__()
+
     @wsgi.serializers(xml=ServicesIndexTemplate)
     def index(self, req):
         """Return a list of all running services.
@@ -72,6 +78,7 @@ class ServiceController(wsgi.Controller):
         """
         context = req.environ['cinder.context']
         authorize(context)
+        detailed = self.ext_mgr.is_loaded('os-extended-services')
         now = timeutils.utcnow()
         services = db.service_get_all(context)
 
@@ -102,22 +109,43 @@ class ServiceController(wsgi.Controller):
             active = 'enabled'
             if svc['disabled']:
                 active = 'disabled'
-            svcs.append({"binary": svc['binary'], 'host': svc['host'],
-                         'zone': svc['availability_zone'],
-                         'status': active, 'state': art,
-                         'updated_at': svc['updated_at']})
+            ret_fields = {'binary': svc['binary'], 'host': svc['host'],
+                          'zone': svc['availability_zone'],
+                          'status': active, 'state': art,
+                          'updated_at': svc['updated_at']}
+            if detailed:
+                ret_fields['disabled_reason'] = svc['disabled_reason']
+            svcs.append(ret_fields)
         return {'services': svcs}
 
+    def _is_valid_as_reason(self, reason):
+        if not reason:
+            return False
+        try:
+            utils.check_string_length(reason.strip(), 'Disabled reason',
+                                      min_length=1, max_length=255)
+        except exception.InvalidInput:
+            return False
+
+        return True
+
     @wsgi.serializers(xml=ServicesUpdateTemplate)
     def update(self, req, id, body):
         """Enable/Disable scheduling for a service."""
         context = req.environ['cinder.context']
         authorize(context)
 
+        ext_loaded = self.ext_mgr.is_loaded('os-extended-services')
+        ret_val = {}
         if id == "enable":
             disabled = False
-        elif id == "disable":
+            status = "enabled"
+            if ext_loaded:
+                ret_val['disabled_reason'] = None
+        elif (id == "disable" or
+                (id == "disable-log-reason" and ext_loaded)):
             disabled = True
+            status = "disabled"
         else:
             raise webob.exc.HTTPNotFound("Unknown action")
 
@@ -126,6 +154,15 @@ class ServiceController(wsgi.Controller):
         except (TypeError, KeyError):
             raise webob.exc.HTTPBadRequest()
 
+        ret_val['disabled'] = disabled
+        if id == "disable-log-reason" and ext_loaded:
+            reason = body.get('disabled_reason')
+            if not self._is_valid_as_reason(reason):
+                msg = _('Disabled reason contains invalid characters '
+                        'or is too long')
+                raise webob.exc.HTTPBadRequest(explanation=msg)
+            ret_val['disabled_reason'] = reason
+
         # NOTE(uni): deprecating service request key, binary takes precedence
         # Still keeping service key here for API compatibility sake.
         service = body.get('service', '')
@@ -139,16 +176,13 @@ class ServiceController(wsgi.Controller):
             if not svc:
                 raise webob.exc.HTTPNotFound('Unknown service')
 
-            db.service_update(context, svc['id'], {'disabled': disabled})
+            db.service_update(context, svc['id'], ret_val)
         except exception.ServiceNotFound:
             raise webob.exc.HTTPNotFound("service not found")
 
-        status = id + 'd'
-        return {'host': host,
-                'service': service,
-                'disabled': disabled,
-                'binary': binary,
-                'status': status}
+        ret_val.update({'host': host, 'service': service,
+                        'binary': binary, 'status': status})
+        return ret_val
 
 
 class Services(extensions.ExtensionDescriptor):
@@ -161,7 +195,7 @@ class Services(extensions.ExtensionDescriptor):
 
     def get_resources(self):
         resources = []
-        resource = extensions.ResourceExtension('os-services',
-                                                ServiceController())
+        controller = ServiceController(self.ext_mgr)
+        resource = extensions.ResourceExtension('os-services', controller)
         resources.append(resource)
         return resources

diff --git a/cinder/api/extensions.py b/cinder/api/extensions.py
index b20323d89de41ae435a0cad43c5e5628178bfd60..8f4292506610dea55401830213c4fd6df059dd67 100644 (file)
--- a/cinder/api/extensions.py
+++ b/cinder/api/extensions.py
@@ -62,6 +62,7 @@ class ExtensionDescriptor(object):
         """Register extension with the extension manager."""
 
         ext_mgr.register(self)
+        self.ext_mgr = ext_mgr
 
     def get_resources(self):
         """List of extensions.ResourceExtension extension objects.

diff --git a/cinder/tests/api/contrib/test_services.py b/cinder/tests/api/contrib/test_services.py
index 420fcd8545561bdf5a0075820238fafb06b4acd1..fbabf65a0523be6253f03c71ed74699336688c02 100644 (file)
--- a/cinder/tests/api/contrib/test_services.py
+++ b/cinder/tests/api/contrib/test_services.py
@@ -14,7 +14,10 @@
 #    under the License.
 
 
+import webob.exc
+
 from cinder.api.contrib import services
+from cinder.api import extensions
 from cinder import context
 from cinder import db
 from cinder import exception
@@ -31,28 +34,32 @@ fake_services_list = [{'binary': 'cinder-scheduler',
                        'id': 1,
                        'disabled': True,
                        'updated_at': datetime(2012, 10, 29, 13, 42, 2),
-                       'created_at': datetime(2012, 9, 18, 2, 46, 27)},
+                       'created_at': datetime(2012, 9, 18, 2, 46, 27),
+                       'disabled_reason': 'test1'},
                       {'binary': 'cinder-volume',
                        'host': 'host1',
                        'availability_zone': 'cinder',
                        'id': 2,
                        'disabled': True,
                        'updated_at': datetime(2012, 10, 29, 13, 42, 5),
-                       'created_at': datetime(2012, 9, 18, 2, 46, 27)},
+                       'created_at': datetime(2012, 9, 18, 2, 46, 27),
+                       'disabled_reason': 'test2'},
                       {'binary': 'cinder-scheduler',
                        'host': 'host2',
                        'availability_zone': 'cinder',
                        'id': 3,
                        'disabled': False,
                        'updated_at': datetime(2012, 9, 19, 6, 55, 34),
-                       'created_at': datetime(2012, 9, 18, 2, 46, 28)},
+                       'created_at': datetime(2012, 9, 18, 2, 46, 28),
+                       'disabled_reason': ''},
                       {'binary': 'cinder-volume',
                        'host': 'host2',
                        'availability_zone': 'cinder',
                        'id': 4,
                        'disabled': True,
                        'updated_at': datetime(2012, 9, 18, 8, 3, 38),
-                       'created_at': datetime(2012, 9, 18, 2, 46, 28)},
+                       'created_at': datetime(2012, 9, 18, 2, 46, 28),
+                       'disabled_reason': 'test4'},
                       ]
 
 
@@ -138,7 +145,9 @@ class ServicesTest(test.TestCase):
         self.stubs.Set(policy, "enforce", fake_policy_enforce)
 
         self.context = context.get_admin_context()
-        self.controller = services.ServiceController()
+        self.ext_mgr = extensions.ExtensionManager()
+        self.ext_mgr.extensions = {}
+        self.controller = services.ServiceController(self.ext_mgr)
 
     def tearDown(self):
         super(ServicesTest, self).tearDown()
@@ -165,6 +174,34 @@ class ServicesTest(test.TestCase):
                      'updated_at': datetime(2012, 9, 18, 8, 3, 38)}]}
         self.assertEqual(res_dict, response)
 
+    def test_services_detail(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        req = FakeRequest()
+        res_dict = self.controller.index(req)
+
+        response = {'services': [{'binary': 'cinder-scheduler',
+                    'host': 'host1', 'zone': 'cinder',
+                    'status': 'disabled', 'state': 'up',
+                    'updated_at': datetime(2012, 10, 29, 13, 42, 2),
+                    'disabled_reason': 'test1'},
+                    {'binary': 'cinder-volume',
+                     'host': 'host1', 'zone': 'cinder',
+                     'status': 'disabled', 'state': 'up',
+                     'updated_at': datetime(2012, 10, 29, 13, 42, 5),
+                     'disabled_reason': 'test2'},
+                    {'binary': 'cinder-scheduler', 'host': 'host2',
+                     'zone': 'cinder',
+                     'status': 'enabled', 'state': 'down',
+                     'updated_at': datetime(2012, 9, 19, 6, 55, 34),
+                     'disabled_reason': ''},
+                    {'binary': 'cinder-volume', 'host': 'host2',
+                     'zone': 'cinder',
+                     'status': 'disabled', 'state': 'down',
+                     'updated_at': datetime(2012, 9, 18, 8, 3, 38),
+                     'disabled_reason': 'test4'}]}
+        self.assertEqual(res_dict, response)
+
     def test_services_list_with_host(self):
         req = FakeRequestWithHost()
         res_dict = self.controller.index(req)
@@ -182,6 +219,27 @@ class ServicesTest(test.TestCase):
                                                          13, 42, 5)}]}
         self.assertEqual(res_dict, response)
 
+    def test_services_detail_with_host(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        req = FakeRequestWithHost()
+        res_dict = self.controller.index(req)
+
+        response = {'services': [{'binary': 'cinder-scheduler',
+                                  'host': 'host1',
+                                  'zone': 'cinder',
+                                  'status': 'disabled', 'state': 'up',
+                                  'updated_at': datetime(2012, 10,
+                                                         29, 13, 42, 2),
+                                  'disabled_reason': 'test1'},
+                                 {'binary': 'cinder-volume', 'host': 'host1',
+                                  'zone': 'cinder',
+                                  'status': 'disabled', 'state': 'up',
+                                  'updated_at': datetime(2012, 10, 29,
+                                                         13, 42, 5),
+                                  'disabled_reason': 'test2'}]}
+        self.assertEqual(res_dict, response)
+
     def test_services_list_with_service(self):
         req = FakeRequestWithService()
         res_dict = self.controller.index(req)
@@ -202,6 +260,30 @@ class ServicesTest(test.TestCase):
                                                          8, 3, 38)}]}
         self.assertEqual(res_dict, response)
 
+    def test_services_detail_with_service(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        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),
+                                  'disabled_reason': 'test2'},
+                                 {'binary': 'cinder-volume',
+                                  'host': 'host2',
+                                  'zone': 'cinder',
+                                  'status': 'disabled',
+                                  'state': 'down',
+                                  'updated_at': datetime(2012, 9, 18,
+                                                         8, 3, 38),
+                                  'disabled_reason': 'test4'}]}
+        self.assertEqual(res_dict, response)
+
     def test_services_list_with_binary(self):
         req = FakeRequestWithBinary()
         res_dict = self.controller.index(req)
@@ -222,6 +304,30 @@ class ServicesTest(test.TestCase):
                                                          8, 3, 38)}]}
         self.assertEqual(res_dict, response)
 
+    def test_services_detail_with_binary(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        req = FakeRequestWithBinary()
+        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),
+                                  'disabled_reason': 'test2'},
+                                 {'binary': 'cinder-volume',
+                                  'host': 'host2',
+                                  'zone': 'cinder',
+                                  'status': 'disabled',
+                                  'state': 'down',
+                                  'updated_at': datetime(2012, 9, 18,
+                                                         8, 3, 38),
+                                  'disabled_reason': 'test4'}]}
+        self.assertEqual(res_dict, response)
+
     def test_services_list_with_host_service(self):
         req = FakeRequestWithHostService()
         res_dict = self.controller.index(req)
@@ -235,6 +341,22 @@ class ServicesTest(test.TestCase):
                                                          13, 42, 5)}]}
         self.assertEqual(res_dict, response)
 
+    def test_services_detail_with_host_service(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        req = FakeRequestWithHostService()
+        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),
+                                  'disabled_reason': 'test2'}]}
+        self.assertEqual(res_dict, response)
+
     def test_services_list_with_host_binary(self):
         req = FakeRequestWithHostBinary()
         res_dict = self.controller.index(req)
@@ -248,6 +370,22 @@ class ServicesTest(test.TestCase):
                                                          13, 42, 5)}]}
         self.assertEqual(res_dict, response)
 
+    def test_services_detail_with_host_binary(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        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),
+                                  'disabled_reason': 'test2'}]}
+        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')
@@ -275,3 +413,40 @@ class ServicesTest(test.TestCase):
         res_dict = self.controller.update(req, "disable", body)
 
         self.assertEqual(res_dict['status'], 'disabled')
+
+    def test_services_disable_log_reason(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        req = (
+            fakes.HTTPRequest.blank('v1/fake/os-services/disable-log-reason'))
+        body = {'host': 'host1',
+                'binary': 'cinder-scheduler',
+                'disabled_reason': 'test-reason',
+                }
+        res_dict = self.controller.update(req, "disable-log-reason", body)
+
+        self.assertEqual(res_dict['status'], 'disabled')
+        self.assertEqual(res_dict['disabled_reason'], 'test-reason')
+
+    def test_services_disable_log_reason_none(self):
+        self.ext_mgr.extensions['os-extended-services'] = True
+        self.controller = services.ServiceController(self.ext_mgr)
+        req = (
+            fakes.HTTPRequest.blank('v1/fake/os-services/disable-log-reason'))
+        body = {'host': 'host1',
+                'binary': 'cinder-scheduler',
+                'disabled_reason': None,
+                }
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update,
+                          req, "disable-log-reason", body)
+
+    def test_invalid_reason_field(self):
+        reason = ' '
+        self.assertFalse(self.controller._is_valid_as_reason(reason))
+        reason = 'a' * 256
+        self.assertFalse(self.controller._is_valid_as_reason(reason))
+        reason = 'it\'s a valid reason.'
+        self.assertTrue(self.controller._is_valid_as_reason(reason))
+        reason = None
+        self.assertFalse(self.controller._is_valid_as_reason(reason))

diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py
index 380c13522267a796316d9671eee38b6908a0f7fb..110e2b6179e54332dc5dcc06128970759c0b75f7 100644 (file)
--- a/cinder/tests/test_utils.py
+++ b/cinder/tests/test_utils.py
@@ -900,3 +900,18 @@ class BrickUtils(test.TestCase):
         utils.brick_get_connector('aoe')
         utils.brick_get_connector('local')
         self.mox.VerifyAll()
+
+
+class StringLengthTestCase(test.TestCase):
+    def test_check_string_length(self):
+        self.assertIsNone(utils.check_string_length(
+                          'test', 'name', max_length=255))
+        self.assertRaises(exception.InvalidInput,
+                          utils.check_string_length,
+                          11, 'name', max_length=255)
+        self.assertRaises(exception.InvalidInput,
+                          utils.check_string_length,
+                          '', 'name', min_length=1)
+        self.assertRaises(exception.InvalidInput,
+                          utils.check_string_length,
+                          'a' * 256, 'name', max_length=255)

diff --git a/cinder/utils.py b/cinder/utils.py
index 707ddf5cbf276cbfd99d6223b604d0f6c2191074..401d05234d3e7ac24b5e4064fcd5ea2df670411e 100644 (file)
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -34,6 +34,7 @@ import tempfile
 from eventlet import pools
 from oslo.config import cfg
 import paramiko
+import six
 from xml.dom import minidom
 from xml.parsers import expat
 from xml import sax
@@ -768,3 +769,25 @@ def get_file_mode(path):
 def get_file_gid(path):
     """This primarily exists to make unit testing easier."""
     return os.stat(path).st_gid
+
+
+def check_string_length(value, name, min_length=0, max_length=None):
+    """Check the length of specified string
+    :param value: the value of the string
+    :param name: the name of the string
+    :param min_length: the min_length of the string
+    :param max_length: the max_length of the string
+    """
+    if not isinstance(value, six.string_types):
+        msg = _("%s is not a string or unicode") % name
+        raise exception.InvalidInput(message=msg)
+
+    if len(value) < min_length:
+        msg = _("%(name)s has a minimum character requirement of "
+                "%(min_length)s.") % {'name': name, 'min_length': min_length}
+        raise exception.InvalidInput(message=msg)
+
+    if max_length and len(value) > max_length:
+        msg = _("%(name)s has more than %(max_length)s "
+                "characters.") % {'name': name, 'max_length': max_length}
+        raise exception.InvalidInput(message=msg)