From 5195f95eb6c1deaf1acd2dd0b16025974ed6ec18 Mon Sep 17 00:00:00 2001 From: Jay Lau Date: Sat, 22 Feb 2014 05:47:47 -0500 Subject: [PATCH] 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 --- cinder/api/contrib/extended_services.py | 25 +++ cinder/api/contrib/services.py | 62 ++++++-- cinder/api/extensions.py | 1 + cinder/tests/api/contrib/test_services.py | 185 +++++++++++++++++++++- cinder/tests/test_utils.py | 15 ++ cinder/utils.py | 23 +++ 6 files changed, 292 insertions(+), 19 deletions(-) create mode 100644 cinder/api/contrib/extended_services.py diff --git a/cinder/api/contrib/extended_services.py b/cinder/api/contrib/extended_services.py new file mode 100644 index 000000000..97e95b9e6 --- /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 998208665..8fde5b874 100644 --- 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 b20323d89..8f4292506 100644 --- 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 420fcd854..fbabf65a0 100644 --- 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 380c13522..110e2b617 100644 --- 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 707ddf5cb..401d05234 100644 --- 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) -- 2.45.2