Add disabled reason field to service.

update micversion to 2.83
user can set disabled reason for service.

Closes-Bug: #2037700

Change-Id: I3d7c46945366ac9e1d305c2f6de2233859259bf7

manila/api/openstack/api_version_request.py

@@ -200,13 +200,14 @@ REST_API_VERSION_HISTORY = """
     * 2.80 - Added share backup APIs.
     * 2.81 - Added API methods, endpoint /resource-locks.
     * 2.82 - Added lock and restriction to share access rules.
+    * 2.83 - Added 'disabled_reason' field to services.
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.82"
+_MAX_API_VERSION = "2.83"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -444,3 +444,9 @@ user documentation.
 -------------------------------
   Introduce the ability to lock access rules and restrict the visibility of
   sensitive fields.
+
+2.83
+----
+  The ``disabled_reason`` field was added to the service to mark the reason why
+  the user disabled the service. ``disabled`` field will be replaced by
+  ``status`` field.

manila/api/v2/services.py

@@ -14,11 +14,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_utils import strutils
 import webob.exc
 
 from manila.api.openstack import wsgi
 from manila.api.views import services as services_views
 from manila import db
+from manila.i18n import _
 
 
 class ServiceMixin(object):
@@ -46,6 +48,7 @@ class ServiceMixin(object):
                 'host': service['host'],
                 'zone': service['availability_zone']['name'],
                 'status': 'disabled' if service['disabled'] else 'enabled',
+                'disabled_reason': service.get('disabled_reason'),
                 'state': service['state'],
                 'updated_at': service['updated_at'],
             }
@@ -65,17 +68,38 @@ class ServiceMixin(object):
             if len(services) == 0:
                 break
 
-        return self._view_builder.detail_list(services)
+        return self._view_builder.detail_list(req, services)
 
     @wsgi.Controller.authorize("update")
-    def _update(self, req, id, body):
+    def _update(self, req, id, body, support_disabled_reason=True):
         """Enable/Disable scheduling for a service."""
         context = req.environ['manila.context']
+        update_dict = {}
 
         if id == "enable":
             data = {'disabled': False}
+            if support_disabled_reason:
+                update_dict['disabled_reason'] = None
         elif id == "disable":
             data = {'disabled': True}
+            disabled_reason = body.get('disabled_reason')
+            if disabled_reason and not support_disabled_reason:
+                msg = _("'disabled_reason' option is not supported by this "
+                        "microversion. Use 2.83 or greater microversion to "
+                        "be able to set 'disabled_reason'.")
+                raise webob.exc.HTTPBadRequest(explanation=msg)
+            if disabled_reason:
+                try:
+                    strutils.check_string_length(disabled_reason.strip(),
+                                                 name='disabled_reason',
+                                                 min_length=1,
+                                                 max_length=255)
+                except (ValueError, TypeError):
+                    msg = _('Disabled reason contains invalid characters '
+                            'or is too long')
+                    raise webob.exc.HTTPBadRequest(explanation=msg)
+                update_dict['disabled_reason'] = disabled_reason.strip()
+                data['disabled_reason'] = disabled_reason.strip()
         else:
             raise webob.exc.HTTPNotFound("Unknown action '%s'" % id)
 
@@ -86,10 +110,12 @@ class ServiceMixin(object):
             raise webob.exc.HTTPBadRequest()
 
         svc = db.service_get_by_args(context, data['host'], data['binary'])
-        db.service_update(
+        update_dict['disabled'] = data['disabled']
-            context, svc['id'], {'disabled': data['disabled']})
 
-        return self._view_builder.summary(data)
+        db.service_update(context, svc['id'], update_dict)
+        data['status'] = 'disabled' if id == "disable" else 'enabled'
+
+        return self._view_builder.summary(req, data)
 
 
 class ServiceControllerLegacy(ServiceMixin, wsgi.Controller):
@@ -119,8 +145,12 @@ class ServiceController(ServiceMixin, wsgi.Controller):
     def index(self, req):
         return self._index(req)
 
-    @wsgi.Controller.api_version('2.7')
+    @wsgi.Controller.api_version('2.7', '2.82')
     def update(self, req, id, body):
+        return self._update(req, id, body, support_disabled_reason=False)
+
+    @wsgi.Controller.api_version('2.83') # noqa
+    def update(self, req, id, body): # pylint: disable=function-redefined  # noqa F811
         return self._update(req, id, body)
 
 

manila/api/views/services.py

@@ -19,14 +19,33 @@ from manila.api import common
 class ViewBuilder(common.ViewBuilder):
 
     _collection_name = "services"
+    _detail_version_modifiers = [
+        "add_disabled_reason_field",
+    ]
 
-    def summary(self, service):
+    def summary(self, request, service):
         """Summary view of a single service."""
-        keys = 'host', 'binary', 'disabled'
+        keys = 'host', 'binary', 'status',
-        return {key: service.get(key) for key in keys}
+        service_dict = {key: service.get(key) for key in keys}
+        self.update_versioned_resource_dict(request, service_dict, service)
+        return service_dict
 
-    def detail_list(self, services):
+    def detail(self, request, service):
+        """Detailed view of a single service."""
+        keys = ('id', 'binary', 'host', 'zone', 'status',
+                'state', 'updated_at')
+        service_dict = {key: service.get(key) for key in keys}
+        self.update_versioned_resource_dict(request, service_dict, service)
+        return service_dict
+
+    def detail_list(self, request, services):
         """Detailed view of a list of services."""
-        keys = 'id', 'binary', 'host', 'zone', 'status', 'state', 'updated_at'
+        services_list = [self.detail(request, s) for s in services]
-        views = [{key: s.get(key) for key in keys} for s in services]
+        services_dict = dict(services=services_list)
-        return {self._collection_name: views}
+        return services_dict
+
+    @common.ViewBuilder.versioned_method("2.83")
+    def add_disabled_reason_field(self, context, service_dict, service):
+        service_dict.pop('disabled', None)
+        service_dict['status'] = service.get('status')
+        service_dict['disabled_reason'] = service.get('disabled_reason')

manila/db/migrations/alembic/versions/99d328f0a3d2_add_disable_reason_to_service.py

@@ -0,0 +1,49 @@
+# 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.
+
+"""add_disable_reason_to_service
+
+Revision ID: 99d328f0a3d2
+Revises: 2d708a9a3ba9
+Create Date: 2023-10-13 10:50:29.311032
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '99d328f0a3d2'
+down_revision = '2d708a9a3ba9'
+
+from alembic import op
+from oslo_log import log
+import sqlalchemy as sa
+
+
+LOG = log.getLogger(__name__)
+
+
+def upgrade():
+    try:
+        op.add_column(
+            'services', sa.Column(
+                'disabled_reason', sa.String(length=255),
+                nullable=True))
+    except Exception:
+        LOG.error("Column services.disabled_reason not created!")
+        raise
+
+
+def downgrade():
+    try:
+        op.drop_column('services', 'disabled_reason')
+    except Exception:
+        LOG.error("Column services.disabled_reason not dropped!")
+        raise

manila/db/sqlalchemy/models.py

@@ -68,6 +68,7 @@ class Service(BASE, ManilaBase):
     state = Column(String(36))
     report_count = Column(Integer, nullable=False, default=0)
     disabled = Column(Boolean, default=False)
+    disabled_reason = Column(String(255))
     availability_zone_id = Column(String(36),
                                   ForeignKey('availability_zones.id'),
                                   nullable=True)

manila/tests/api/v2/test_services.py

@@ -21,6 +21,7 @@ from unittest import mock
 import ddt
 from oslo_utils import timeutils
 
+from manila.api.openstack import api_version_request as api_version
 from manila.api.v2 import services
 from manila import context
 from manila import db
@@ -37,6 +38,7 @@ fake_services_list = [
         'availability_zone': {'name': 'manila1'},
         'id': 1,
         'disabled': True,
+        'disabled_reason': 'test1',
         'state': 'up',
         'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 2),
         'created_at': datetime.datetime(2012, 9, 18, 2, 46, 27),
@@ -47,6 +49,7 @@ fake_services_list = [
         'availability_zone': {'name': 'manila1'},
         'id': 2,
         'disabled': True,
+        'disabled_reason': 'test2',
         'state': 'up',
         'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 5),
         'created_at': datetime.datetime(2012, 9, 18, 2, 46, 27)},
@@ -56,6 +59,7 @@ fake_services_list = [
         'availability_zone': {'name': 'manila2'},
         'id': 3,
         'disabled': False,
+        'disabled_reason': '',
         'state': 'down',
         'updated_at': datetime.datetime(2012, 9, 19, 6, 55, 34),
         'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28)},
@@ -65,6 +69,7 @@ fake_services_list = [
         'availability_zone': {'name': 'manila2'},
         'id': 4,
         'disabled': True,
+        'disabled_reason': 'test4',
         'state': 'down',
         'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38),
         'created_at': datetime.datetime(2012, 9, 18, 2, 46, 28),
@@ -111,6 +116,49 @@ fake_response_service_list = {'services': [
     },
 ]}
 
+fake_response_service_list_with_disabled_reason = {'services': [
+    {
+        'id': 1,
+        'binary': 'manila-scheduler',
+        'host': 'host1',
+        'zone': 'manila1',
+        'status': 'disabled',
+        'disabled_reason': 'test1',
+        'state': 'up',
+        'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 2),
+    },
+    {
+        'id': 2,
+        'binary': 'manila-share',
+        'host': 'host1',
+        'zone': 'manila1',
+        'status': 'disabled',
+        'disabled_reason': 'test2',
+        'state': 'up',
+        'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 5),
+    },
+    {
+        'id': 3,
+        'binary': 'manila-scheduler',
+        'host': 'host2',
+        'zone': 'manila2',
+        'status': 'enabled',
+        'disabled_reason': '',
+        'state': 'down',
+        'updated_at': datetime.datetime(2012, 9, 19, 6, 55, 34),
+    },
+    {
+        'id': 4,
+        'binary': 'manila-share',
+        'host': 'host2',
+        'zone': 'manila2',
+        'status': 'disabled',
+        'disabled_reason': 'test4',
+        'state': 'down',
+        'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38),
+    },
+]}
+
 
 def fake_service_get_all(context):
     return fake_services_list
@@ -165,6 +213,7 @@ class ServicesTest(test.TestCase):
         ('os-services', '1.0', services.ServiceControllerLegacy),
         ('os-services', '2.6', services.ServiceControllerLegacy),
         ('services', '2.7', services.ServiceController),
+        ('services', '2.83', services.ServiceController),
     )
     @ddt.unpack
     def test_services_list(self, url, version, controller):
@@ -173,7 +222,12 @@ class ServicesTest(test.TestCase):
 
         res_dict = controller().index(req)
 
-        self.assertEqual(fake_response_service_list, res_dict)
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.83')):
+            self.assertEqual(fake_response_service_list_with_disabled_reason,
+                             res_dict)
+        else:
+            self.assertEqual(fake_response_service_list, res_dict)
         self.mock_policy_check.assert_called_once_with(
             req.environ['manila.context'], self.resource_name, 'index')
 
@@ -273,7 +327,7 @@ class ServicesTest(test.TestCase):
 
         res_dict = controller().update(req, "enable", body)
 
-        self.assertFalse(res_dict['disabled'])
+        self.assertEqual('enabled', res_dict['status'])
         self.mock_policy_check.assert_called_once_with(
             req.environ['manila.context'], self.resource_name, 'update')
 
@@ -281,16 +335,24 @@ class ServicesTest(test.TestCase):
         ('os-services', '1.0', services.ServiceControllerLegacy),
         ('os-services', '2.6', services.ServiceControllerLegacy),
         ('services', '2.7', services.ServiceController),
+        ('services', '2.83', services.ServiceController),
     )
     @ddt.unpack
     def test_services_disable(self, url, version, controller):
         req = fakes.HTTPRequest.blank(
             '/fooproject/%s/disable' % url, version=version)
         body = {'host': 'host1', 'binary': 'manila-share'}
+        if (api_version.APIVersionRequest(version) >
+                api_version.APIVersionRequest("2.83")):
+            body['disabled_reason'] = 'test1'
 
         res_dict = controller().update(req, "disable", body)
 
-        self.assertTrue(res_dict['disabled'])
+        self.assertEqual('disabled', res_dict['status'])
+        if (api_version.APIVersionRequest(version) >
+                api_version.APIVersionRequest("2.83")):
+            self.assertEqual(res_dict['disabled_reason'], 'test1')
+
         self.mock_policy_check.assert_called_once_with(
             req.environ['manila.context'], self.resource_name, 'update')
 

manila/tests/db/migrations/alembic/migrations_data_checks.py

@@ -3372,3 +3372,35 @@ class AddResourceLocks(BaseMigrationChecks):
         self.test_case.assertRaises(sa_exc.NoSuchTableError,
                                     utils.load_table,
                                     'resource_locks', engine)
+
+
+@map_to_migration('99d328f0a3d2')
+class ServiceDisabledReason(BaseMigrationChecks):
+    def _get_service_data(self, options):
+        base_dict = {
+            'binary': 'manila-share',
+            'topic': 'share',
+            'disabled': False,
+            'report_count': '100',
+        }
+        base_dict.update(options)
+        return base_dict
+
+    def setup_upgrade_data(self, engine):
+        service_fixture = [
+            self._get_service_data({'host': 'fake1'}),
+            self._get_service_data({'host': 'fake2'}),
+        ]
+        services_table = utils.load_table('services', engine)
+        for fixture in service_fixture:
+            engine.execute(services_table.insert(fixture))
+
+    def check_upgrade(self, engine, data):
+        service_table = utils.load_table('services', engine)
+        for s in engine.execute(service_table.select()):
+            self.test_case.assertTrue(hasattr(s, 'disabled_reason'))
+
+    def check_downgrade(self, engine):
+        service_table = utils.load_table('services', engine)
+        for s in engine.execute(service_table.select()):
+            self.test_case.assertFalse(hasattr(s, 'disabled_reason'))

releasenotes/notes/add_disabled_reason_to_services-8369aaa2985ada25.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - Added a new field 'disabled_reason' to services table. Users can set
+    'disabled_reason' when disabling a service, and query the disabled
+    reason while listing services. When re-enabling the service, the
+    disabled reason will be cleared.
+    See `bug 2037700 <https://bugs.launchpad.net/manila/+bug/2037700>`_
+    for more details.