From 3ef988eddeff6fc05313915d3ef17196e67db023 Mon Sep 17 00:00:00 2001 From: zhongjun Date: Fri, 11 Dec 2015 19:08:03 +0800 Subject: [PATCH] Check share-network in 'share create' API For the moment it is possible to schedule share creation with DHSS=true share type but without share network. But it makes no sense, and expected to fail. So, perform check on API level. APIImpact When create share with share type(DHSS=true) and not input share network, API will return HTTPBadRequest and message: "Share network must be set when the driver_handles_share_servers is true." Closes-Bug: #1525125 Change-Id: Icdfabff7b1d3b6e95dd1dd58a0155de637056657 --- manila/api/v1/shares.py | 24 +++++++++++++++---- manila/tests/api/v1/test_shares.py | 18 ++++++++++++++ .../tests/api/admin/test_quotas_negative.py | 2 +- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index 5b609d4f7b..2ce160df4f 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -356,14 +356,14 @@ class ShareMixin(object): raise exc.HTTPBadRequest(explanation=msg) req_share_type = share.get('share_type', share.get('volume_type')) + share_type = None if req_share_type: try: if not uuidutils.is_uuid_like(req_share_type): - kwargs['share_type'] = \ - share_types.get_share_type_by_name( - context, req_share_type) + share_type = share_types.get_share_type_by_name( + context, req_share_type) else: - kwargs['share_type'] = share_types.get_share_type( + share_type = share_types.get_share_type( context, req_share_type) except exception.ShareTypeNotFound: msg = _("Share type not found.") @@ -371,8 +371,22 @@ class ShareMixin(object): elif not snapshot: def_share_type = share_types.get_default_share_type() if def_share_type: - kwargs['share_type'] = def_share_type + share_type = def_share_type + # Only use in create share feature. Create share from snapshot + # and create share with consistency group features not + # need this check. + if (not share_network_id and not snapshot + and not share.get('consistency_group_id') + and share_type and share_type.get('extra_specs') + and (strutils.bool_from_string(share_type.get('extra_specs'). + get('driver_handles_share_servers')))): + msg = _('Share network must be set when the ' + 'driver_handles_share_servers is true.') + raise exc.HTTPBadRequest(explanation=msg) + + if share_type: + kwargs['share_type'] = share_type new_share = self.share_api.create(context, share_proto, size, diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index 42494dd6d7..0cd172c20a 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -192,6 +192,24 @@ class ShareAPITest(test.TestCase): self.controller.create, req, {'share': self.share}) share_types.get_default_share_type.assert_called_once_with() + def test_share_create_with_dhss_true_and_network_notexist(self): + fake_share_type = { + 'id': 'fake_volume_type_id', + 'name': 'fake_volume_type_name', + 'extra_specs': { + 'driver_handles_share_servers': True, + } + } + self.mock_object( + share_types, 'get_default_share_type', + mock.Mock(return_value=fake_share_type), + ) + CONF.set_default("default_share_type", fake_share_type['name']) + req = fakes.HTTPRequest.blank('/shares') + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, req, {'share': self.share}) + share_types.get_default_share_type.assert_called_once_with() + def test_share_create_with_share_net(self): shr = { "size": 100, diff --git a/manila_tempest_tests/tests/api/admin/test_quotas_negative.py b/manila_tempest_tests/tests/api/admin/test_quotas_negative.py index 985bef4a02..d5452ed33d 100644 --- a/manila_tempest_tests/tests/api/admin/test_quotas_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_quotas_negative.py @@ -102,7 +102,7 @@ class SharesAdminQuotasNegativeTest(base.BaseSharesAdminTest): # try schedule share with size, bigger than gigabytes quota self.assertRaises(lib_exc.OverLimit, - self.shares_client.create_share, + self.create_share, size=overquota) @test.attr(type=["gate", "smoke", "negative"])