diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index c013ed5453..c8067582b7 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -111,13 +111,14 @@ REST_API_VERSION_HISTORY = """ manila. * 2.39 - Added share-type quotas. * 2.40 - Added share group and share group snapshot quotas. + * 2.41 - Added 'description' in share type create/list APIs. """ # 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.40" +_MAX_API_VERSION = "2.41" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index acbd143a26..5a2f22b181 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -226,3 +226,7 @@ user documentation. 2.40 ---- Added share group and share group snapshot quotas. + +2.41 +---- + Added 'description' in share type create/list APIs. diff --git a/manila/api/v2/share_types.py b/manila/api/v2/share_types.py index 80a243f434..2767bf040b 100644 --- a/manila/api/v2/share_types.py +++ b/manila/api/v2/share_types.py @@ -21,6 +21,7 @@ import six import webob from webob import exc +from manila.api.openstack import api_version_request as api_version from manila.api.openstack import wsgi from manila.api.views import types as views_types from manila.common import constants @@ -153,13 +154,21 @@ class ShareTypesController(wsgi.Controller): share_type = body['volume_type'] name = share_type.get('name') specs = share_type.get('extra_specs', {}) + description = share_type.get('description') + if (description and req.api_version_request + < api_version.APIVersionRequest("2.41")): + msg = _("'description' key is not supported by this " + "microversion. Use 2.41 or greater microversion " + "to be able to use 'description' in share type.") + raise webob.exc.HTTPBadRequest(explanation=msg) is_public = share_type.get( 'os-share-type-access:is_public', share_type.get('share_type_access:is_public', True), ) - if name is None or name == "" or len(name) > 255: - msg = _("Type name is not valid.") + if (name is None or name == "" or len(name) > 255 + or (description and len(description) > 255)): + msg = _("Type name or description is not valid.") raise webob.exc.HTTPBadRequest(explanation=msg) # Note(cknight): Set the default extra spec value for snapshot_support @@ -172,7 +181,8 @@ class ShareTypesController(wsgi.Controller): required_extra_specs = ( share_types.get_valid_required_extra_specs(specs) ) - share_types.create(context, name, specs, is_public) + share_types.create(context, name, specs, is_public, + description=description) share_type = share_types.get_share_type_by_name(context, name) share_type['required_extra_specs'] = required_extra_specs req.cache_db_share_type(share_type) diff --git a/manila/api/views/types.py b/manila/api/views/types.py index 823fbcee79..e17f34ef4f 100644 --- a/manila/api/views/types.py +++ b/manila/api/views/types.py @@ -26,6 +26,7 @@ class ViewBuilder(common.ViewBuilder): "add_is_public_attr_core_api_like", "add_is_public_attr_extension_like", "add_inferred_optional_extra_specs", + "add_description_attr", ] def show(self, request, share_type, brief=False): @@ -90,3 +91,7 @@ class ViewBuilder(common.ViewBuilder): def _filter_extra_specs(self, extra_specs, valid_keys): return {key: value for key, value in extra_specs.items() if key in valid_keys} + + @common.ViewBuilder.versioned_method("2.41") + def add_description_attr(self, context, share_type_dict, share_type): + share_type_dict['description'] = share_type.get('description') diff --git a/manila/db/migrations/alembic/versions/27cb96d991fa_add_description_for_share_type.py b/manila/db/migrations/alembic/versions/27cb96d991fa_add_description_for_share_type.py new file mode 100644 index 0000000000..ba84925324 --- /dev/null +++ b/manila/db/migrations/alembic/versions/27cb96d991fa_add_description_for_share_type.py @@ -0,0 +1,50 @@ +# Copyright (c) 2017 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# 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 description for share type + +Revision ID: 27cb96d991fa +Revises: 829a09b0ddd4 +Create Date: 2017-09-16 03:07:15.548947 + +""" + +# revision identifiers, used by Alembic. +revision = '27cb96d991fa' +down_revision = '829a09b0ddd4' + +from alembic import op +from oslo_log import log +import sqlalchemy as sa + +LOG = log.getLogger(__name__) + + +def upgrade(): + try: + op.add_column( + 'share_types', + sa.Column('description', sa.String(255), nullable=True)) + except Exception: + LOG.error("Column share_types.description not created!") + raise + + +def downgrade(): + try: + op.drop_column('share_types', 'description') + except Exception: + LOG.error("Column share_types.description not dropped!") + raise diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 47d26e98a7..e751f9c1b8 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -485,6 +485,7 @@ class ShareTypes(BASE, ManilaBase): id = Column(String(36), primary_key=True) deleted = Column(String(36), default='False') name = Column(String(255)) + description = Column(String(255)) is_public = Column(Boolean, default=True) diff --git a/manila/share/share_types.py b/manila/share/share_types.py index e8a0015510..0aa818cd71 100644 --- a/manila/share/share_types.py +++ b/manila/share/share_types.py @@ -34,7 +34,8 @@ CONF = cfg.CONF LOG = log.getLogger(__name__) -def create(context, name, extra_specs=None, is_public=True, projects=None): +def create(context, name, extra_specs=None, is_public=True, + projects=None, description=None): """Creates share types.""" extra_specs = extra_specs or {} projects = projects or [] @@ -44,10 +45,10 @@ def create(context, name, extra_specs=None, is_public=True, projects=None): get_valid_optional_extra_specs(extra_specs) except exception.InvalidExtraSpec as e: raise exception.InvalidShareType(reason=six.text_type(e)) - try: type_ref = db.share_type_create(context, dict(name=name, + description=description, extra_specs=extra_specs, is_public=is_public), projects=projects) diff --git a/manila/test.py b/manila/test.py index baef99fbbc..70a383bc19 100644 --- a/manila/test.py +++ b/manila/test.py @@ -370,6 +370,10 @@ class TestCase(base_test.BaseTestCase): return (api_version.APIVersionRequest(left) >= api_version.APIVersionRequest(right)) + def is_microversion_lt(self, left, right): + return (api_version.APIVersionRequest(left) < + api_version.APIVersionRequest(right)) + def assert_notify_called(self, mock_notify, calls): for i in range(0, len(calls)): mock_call = mock_notify.call_args_list[i] diff --git a/manila/tests/api/v2/test_share_types.py b/manila/tests/api/v2/test_share_types.py index 74933600e3..a9ab762662 100644 --- a/manila/tests/api/v2/test_share_types.py +++ b/manila/tests/api/v2/test_share_types.py @@ -45,6 +45,7 @@ def stub_share_type(id): return dict( id=id, name='share_type_%s' % str(id), + description='description_%s' % str(id), extra_specs=specs, required_extra_specs={ constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: "true", @@ -88,12 +89,13 @@ def return_share_types_with_volumes_destroy(context, id): pass -def return_share_types_create(context, name, specs, is_public): +def return_share_types_create(context, name, specs, is_public, description): pass def make_create_body(name="test_share_1", extra_specs=None, - spec_driver_handles_share_servers=True): + spec_driver_handles_share_servers=True, + description=None): if not extra_specs: extra_specs = {} @@ -104,9 +106,11 @@ def make_create_body(name="test_share_1", extra_specs=None, body = { "share_type": { "name": name, - "extra_specs": extra_specs + "extra_specs": extra_specs, } } + if description: + body["share_type"].update({"description": description}) return body @@ -235,6 +239,8 @@ class ShareTypesAPITest(test.TestCase): ('2.24', 'share_type_access', False), ('2.27', 'share_type_access', True), ('2.27', 'share_type_access', False), + ('2.41', 'share_type_access', True), + ('2.41', 'share_type_access', False), ) @ddt.unpack def test_view_builder_show(self, version, prefix, admin): @@ -243,6 +249,7 @@ class ShareTypesAPITest(test.TestCase): now = timeutils.utcnow().isoformat() raw_share_type = dict( name='new_type', + description='description_test', deleted=False, created_at=now, updated_at=now, @@ -270,8 +277,10 @@ class ShareTypesAPITest(test.TestCase): for extra_spec in constants.ExtraSpecs.INFERRED_OPTIONAL_MAP: expected_share_type['extra_specs'][extra_spec] = ( constants.ExtraSpecs.INFERRED_OPTIONAL_MAP[extra_spec]) + if self.is_microversion_ge(version, '2.41'): + expected_share_type['description'] = 'description_test' - self.assertDictMatch(output['share_type'], expected_share_type) + self.assertDictMatch(expected_share_type, output['share_type']) @ddt.data( ('1.0', 'os-share-type-access', True), @@ -288,6 +297,8 @@ class ShareTypesAPITest(test.TestCase): ('2.24', 'share_type_access', False), ('2.27', 'share_type_access', True), ('2.27', 'share_type_access', False), + ('2.41', 'share_type_access', True), + ('2.41', 'share_type_access', False), ) @ddt.unpack def test_view_builder_list(self, version, prefix, admin): @@ -306,6 +317,7 @@ class ShareTypesAPITest(test.TestCase): raw_share_types.append( dict( name='new_type', + description='description_test', deleted=False, created_at=now, updated_at=now, @@ -321,16 +333,18 @@ class ShareTypesAPITest(test.TestCase): output = view_builder.index(request, raw_share_types) self.assertIn('share_types', output) + expected_share_type = { + 'name': 'new_type', + 'extra_specs': extra_specs, + '%s:is_public' % prefix: True, + 'required_extra_specs': {}, + } + if self.is_microversion_ge(version, '2.41'): + expected_share_type['description'] = 'description_test' for i in range(0, 10): - expected_share_type = { - 'name': 'new_type', - 'extra_specs': extra_specs, - '%s:is_public' % prefix: True, - 'required_extra_specs': {}, - 'id': 42 + i, - } - self.assertDictMatch(output['share_types'][i], - expected_share_type) + expected_share_type['id'] = 42 + i + self.assertDictMatch(expected_share_type, + output['share_types'][i]) @ddt.data(None, True, 'true', 'false', 'all') def test_parse_is_public_valid(self, value): @@ -373,37 +387,18 @@ class ShareTypesAPITest(test.TestCase): self.controller._delete(req, 1) self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) - @ddt.data(make_create_body("share_type_1"), - make_create_body(spec_driver_handles_share_servers=True), - make_create_body(spec_driver_handles_share_servers=False)) - def test_create_2_23(self, body): + @ddt.data( + (make_create_body("share_type_1"), "2.24"), + (make_create_body(spec_driver_handles_share_servers=True), "2.24"), + (make_create_body(spec_driver_handles_share_servers=False), "2.24"), + (make_create_body("share_type_1"), "2.23"), + (make_create_body(spec_driver_handles_share_servers=True), "2.23"), + (make_create_body(spec_driver_handles_share_servers=False), "2.23"), + (make_create_body(description="description_1"), "2.41")) + @ddt.unpack + def test_create(self, body, version): - req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.23") - self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - - res_dict = self.controller.create(req, body) - - self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) - self.assertEqual(2, len(res_dict)) - self.assertEqual('share_type_1', res_dict['share_type']['name']) - self.assertEqual('share_type_1', res_dict['volume_type']['name']) - for extra_spec in constants.ExtraSpecs.REQUIRED: - self.assertIn(extra_spec, - res_dict['share_type']['required_extra_specs']) - expected_extra_specs = { - constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: True, - constants.ExtraSpecs.SNAPSHOT_SUPPORT: True, - } - expected_extra_specs.update(body['share_type']['extra_specs']) - share_types.create.assert_called_once_with( - mock.ANY, body['share_type']['name'], expected_extra_specs, True) - - @ddt.data(make_create_body("share_type_1"), - make_create_body(spec_driver_handles_share_servers=True), - make_create_body(spec_driver_handles_share_servers=False)) - def test_create_2_24(self, body): - - req = fakes.HTTPRequest.blank('/v2/fake/types', version="2.24") + req = fakes.HTTPRequest.blank('/v2/fake/types', version=version) self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) res_dict = self.controller.create(req, body) @@ -412,15 +407,24 @@ class ShareTypesAPITest(test.TestCase): self.assertEqual(2, len(res_dict)) self.assertEqual('share_type_1', res_dict['share_type']['name']) self.assertEqual('share_type_1', res_dict['volume_type']['name']) + if self.is_microversion_ge(version, '2.41'): + self.assertEqual(body['share_type']['description'], + res_dict['share_type']['description']) + self.assertEqual(body['share_type']['description'], + res_dict['volume_type']['description']) for extra_spec in constants.ExtraSpecs.REQUIRED: self.assertIn(extra_spec, res_dict['share_type']['required_extra_specs']) expected_extra_specs = { constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: True, } + if self.is_microversion_lt(version, '2.24'): + expected_extra_specs[constants.ExtraSpecs.SNAPSHOT_SUPPORT] = True expected_extra_specs.update(body['share_type']['extra_specs']) share_types.create.assert_called_once_with( - mock.ANY, body['share_type']['name'], expected_extra_specs, True) + mock.ANY, body['share_type']['name'], + expected_extra_specs, True, + description=body['share_type'].get('description')) @ddt.data(None, make_create_body(""), @@ -489,6 +493,7 @@ def generate_type(type_id, is_public): return { 'id': type_id, 'name': u'test', + 'description': u'ds_test', 'deleted': False, 'created_at': datetime.datetime(2012, 1, 1, 1, 1, 1, 1), 'updated_at': None, diff --git a/manila/tests/db/migrations/alembic/migrations_data_checks.py b/manila/tests/db/migrations/alembic/migrations_data_checks.py index 4b62ea0f4f..24cf770b50 100644 --- a/manila/tests/db/migrations/alembic/migrations_data_checks.py +++ b/manila/tests/db/migrations/alembic/migrations_data_checks.py @@ -2500,3 +2500,40 @@ class FixProjectShareTypesQuotasUniqueConstraintChecks(BaseMigrationChecks): def check_downgrade(self, engine): pass + + +@map_to_migration('27cb96d991fa') +class NewDescriptionColumnChecks(BaseMigrationChecks): + st_table_name = 'share_types' + st_ids = ['share_type_id_fake_3_%d' % i for i in (1, 2)] + + def setup_upgrade_data(self, engine): + # Create share type + share_type_data = { + 'id': self.st_ids[0], + 'name': 'name_1', + } + st_table = utils.load_table(self.st_table_name, engine) + engine.execute(st_table.insert(share_type_data)) + + def check_upgrade(self, engine, data): + st_table = utils.load_table(self.st_table_name, engine) + for na in engine.execute(st_table.select()): + self.test_case.assertTrue(hasattr(na, 'description')) + + share_type_data_ds = { + 'id': self.st_ids[1], + 'name': 'name_1', + 'description': 'description_1', + } + engine.execute(st_table.insert(share_type_data_ds)) + st = engine.execute(st_table.select().where( + share_type_data_ds['id'] == st_table.c.id)).first() + self.test_case.assertEqual( + share_type_data_ds['description'], st['description']) + + def check_downgrade(self, engine): + table = utils.load_table(self.st_table_name, engine) + db_result = engine.execute(table.select()) + for record in db_result: + self.test_case.assertFalse(hasattr(record, 'description')) diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py index 9e4a16887b..ab76f8f43f 100644 --- a/manila_tempest_tests/config.py +++ b/manila_tempest_tests/config.py @@ -30,7 +30,7 @@ ShareGroup = [ help="The minimum api microversion is configured to be the " "value of the minimum microversion supported by Manila."), cfg.StrOpt("max_api_microversion", - default="2.40", + default="2.41", help="The maximum api microversion is configured to be the " "value of the latest microversion supported by Manila."), cfg.StrOpt("region", diff --git a/manila_tempest_tests/services/share/v2/json/shares_client.py b/manila_tempest_tests/services/share/v2/json/shares_client.py index 89a948ef32..fdcb3cb70a 100644 --- a/manila_tempest_tests/services/share/v2/json/shares_client.py +++ b/manila_tempest_tests/services/share/v2/json/shares_client.py @@ -820,6 +820,8 @@ class SharesV2Client(shares_client.SharesClient): 'extra_specs': kwargs.get('extra_specs'), is_public_keyname: is_public, } + if kwargs.get('description'): + post_body['description'] = kwargs.get('description') post_body = json.dumps({'share_type': post_body}) resp, body = self.post('types', post_body, version=version) self.expected_success(200, resp.status) diff --git a/manila_tempest_tests/tests/api/admin/test_share_types.py b/manila_tempest_tests/tests/api/admin/test_share_types.py index 2df0561448..ea5572f662 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types.py @@ -58,18 +58,30 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): self.assertIn(old_key_name, share_type) self.assertNotIn(new_key_name, share_type) + def _verify_description(self, expect_des, share_type, version): + if utils.is_microversion_ge(version, "2.41"): + self.assertEqual(expect_des, share_type['description']) + else: + self.assertNotIn('description', share_type) + @tc.attr(base.TAG_POSITIVE, base.TAG_API) - @ddt.data('2.0', '2.6', '2.7') + @ddt.data('2.0', '2.6', '2.7', '2.40', '2.41') def test_share_type_create_get(self, version): self.skip_if_microversion_not_supported(version) name = data_utils.rand_name("tempest-manila") + description = None + if utils.is_microversion_ge(version, "2.41"): + description = "Description for share type" extra_specs = self.add_extra_specs_to_dict({"key": "value", }) # Create share type st_create = self.create_share_type( - name, extra_specs=extra_specs, version=version) + name, extra_specs=extra_specs, version=version, + description=description) self.assertEqual(name, st_create['share_type']['name']) + self._verify_description( + description, st_create['share_type'], version) self._verify_is_public_key_name(st_create['share_type'], version) st_id = st_create["share_type"]["id"] @@ -77,6 +89,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): get = self.shares_v2_client.get_share_type(st_id, version=version) self.assertEqual(name, get["share_type"]["name"]) self.assertEqual(st_id, get["share_type"]["id"]) + self._verify_description(description, get['share_type'], version) self.assertEqual(extra_specs, get["share_type"]["extra_specs"]) self._verify_is_public_key_name(get['share_type'], version) @@ -84,16 +97,20 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest): self.assertDictMatch(get["volume_type"], get["share_type"]) @tc.attr(base.TAG_POSITIVE, base.TAG_API) - @ddt.data('2.0', '2.6', '2.7') + @ddt.data('2.0', '2.6', '2.7', '2.40', '2.41') def test_share_type_create_list(self, version): self.skip_if_microversion_not_supported(version) name = data_utils.rand_name("tempest-manila") + description = None + if utils.is_microversion_ge(version, "2.41"): + description = "Description for share type" extra_specs = self.add_extra_specs_to_dict() # Create share type st_create = self.create_share_type( - name, extra_specs=extra_specs, version=version) + name, extra_specs=extra_specs, version=version, + description=description) self._verify_is_public_key_name(st_create['share_type'], version) st_id = st_create["share_type"]["id"] diff --git a/manila_tempest_tests/tests/api/admin/test_share_types_negative.py b/manila_tempest_tests/tests/api/admin/test_share_types_negative.py index b3bf855e49..62d38cceb1 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types_negative.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt from tempest.lib.common.utils import data_utils from tempest.lib import exceptions as lib_exc from testtools import testcase as tc @@ -20,6 +21,7 @@ from testtools import testcase as tc from manila_tempest_tests.tests.api import base +@ddt.ddt class ShareTypesAdminNegativeTest(base.BaseSharesMixedTest): def _create_share_type(self): @@ -48,6 +50,18 @@ class ShareTypesAdminNegativeTest(base.BaseSharesMixedTest): "x" * 256, client=self.admin_shares_v2_client) + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) + @ddt.data('2.0', '2.6', '2.40') + def test_create_share_type_with_description_in_wrong_version( + self, version): + self.assertRaises(lib_exc.BadRequest, + self.create_share_type, + data_utils.rand_name("tempest_type_name"), + extra_specs=self.add_extra_specs_to_dict(), + description="tempest_type_description", + version=version, + client=self.admin_shares_v2_client) + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_get_share_type_by_nonexistent_id(self): self.assertRaises(lib_exc.NotFound,