openstack/manila
Code Issues Proposed changes

Allow to set share visibility using "manage" API

Browse Source 
This fix allows manila manage command to explicitly
set visibility (key '--public') similar to using API
"create".

Change-Id: I8725719b8c7ff5557fa20ebbb5314f3e770ffcc0
Closes-Bug: #1436865
This commit is contained in:
nidhimittalhada 2015-12-02 16:47:15 +05:30
parent e011644e1c
commit 2315e563b7
9 changed files with 93 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
manila/api/openstack/api_version_request.py
View File

@ -53,14 +53,14 @@ REST_API_VERSION_HISTORY = """
* 2.5 - Share Migration admin API
* 2.6 - Return share_type UUID instead of name in Share API
* 2.7 - Rename old extension-like API URLs to core-API-like
* 2.8 - Attr "is_public" can be set for share using API "manage"
"""
# The minimum and maximum versions of the API supported
# The default api version request is defined to be the
# the minimum version of the API supported.
_MIN_API_VERSION = "2.0"
_MAX_API_VERSION = "2.7"
_MAX_API_VERSION = "2.8"
DEFAULT_API_VERSION = _MIN_API_VERSION

4
manila/api/openstack/rest_api_version_history.rst
View File

@ -65,3 +65,7 @@ user documentation.
2.7
---
Rename old extension-like API URLs to core-API-like.
2.8
---
Allow to set share visibility explicitly using "manage" API.

4
manila/api/v1/share_manage.py
View File

@ -49,6 +49,9 @@ class ShareManageMixin(object):
'display_description': description,
}
if share_data.get('is_public') is not None:
share['is_public'] = share_data['is_public']
driver_options = share_data.get('driver_options', {})
try:
@ -125,6 +128,7 @@ class ShareManageController(ShareManageMixin, wsgi.Controller):
Should be removed when minimum API version becomes equal to or
greater than v2.7
"""
body.get('share', {}).pop('is_public', None)
return self._manage(req, body)

11
manila/api/v2/shares.py
View File

@ -136,9 +136,16 @@ class ShareController(shares.ShareMixin,
"""Shrink size of a share."""
return self._shrink(req, id, body)
@wsgi.Controller.api_version('2.7')
@wsgi.Controller.api_version('2.7', '2.7')
def manage(self, req, body):
return self._manage(req, body)
body.get('share', {}).pop('is_public', None)
detail = self._manage(req, body)
return detail
@wsgi.Controller.api_version("2.8") # noqa
def manage(self, req, body): # pylint: disable=E0102
detail = self._manage(req, body)
return detail
@wsgi.Controller.api_version('2.7')
@wsgi.action('unmanage')

2
manila/tests/api/v1/test_share_manage.py
View File

@ -54,7 +54,6 @@ class ShareManageTest(test.TestCase):
policy, 'check_policy', mock.Mock(return_value=True))
@ddt.data({},
{'share': None},
{'shares': {}},
{'share': get_fake_manage_body('', None, None)})
def test_share_manage_invalid_body(self, body):
@ -173,6 +172,7 @@ class ShareManageTest(test.TestCase):
'display_name': 'foo',
'display_description': 'bar',
}
data['share']['is_public'] = 'foo'
driver_options = data['share'].get('driver_options', {})
actual_result = self.controller.create(self.request, data)

28
manila/tests/api/v2/test_shares.py
View File

@ -30,6 +30,7 @@ from manila.common import constants
from manila import context
from manila import db
from manila import exception
from manila import policy
from manila.share import api as share_api
from manila.share import share_types
from manila import test
@ -1299,8 +1300,11 @@ class ShareManageTest(test.TestCase):
def setUp(self):
super(self.__class__, self).setUp()
self.controller = shares.ShareController()
self.resource_name = self.controller.resource_name
self.request = fakes.HTTPRequest.blank(
'/v2/share/manage', use_admin_context=True, version='2.7')
'/v2/shares/manage', use_admin_context=True, version='2.7')
self.mock_policy_check = self.mock_object(
policy, 'check_policy', mock.Mock(return_value=True))
def _setup_manage_mocks(self, service_is_up=True):
self.mock_object(db, 'service_get_by_host_and_topic', mock.Mock(
@ -1318,7 +1322,6 @@ class ShareManageTest(test.TestCase):
mock.Mock(side_effect=exception.ServiceIsDown(service='fake')))
@ddt.data({},
{'share': None},
{'shares': {}},
{'share': get_fake_manage_body('', None, None)})
def test_share_manage_invalid_body(self, body):
@ -1404,6 +1407,16 @@ class ShareManageTest(test.TestCase):
driver_options=dict(volume_id='quuz')),
)
def test_share_manage(self, data):
self._test_share_manage(data, "2.7")
@ddt.data(
get_fake_manage_body(name='foo', description='bar', is_public=True),
get_fake_manage_body(name='foo', description='bar', is_public=False)
)
def test_share_manage_with_is_public(self, data):
self._test_share_manage(data, "2.8")
def _test_share_manage(self, data, version):
self._setup_manage_mocks()
return_share = {'share_type_id': '', 'id': 'fake'}
self.mock_object(
@ -1418,11 +1431,20 @@ class ShareManageTest(test.TestCase):
}
driver_options = data['share'].get('driver_options', {})
actual_result = self.controller.manage(self.request, data)
if (api_version.APIVersionRequest(version) >=
api_version.APIVersionRequest('2.8')):
share['is_public'] = data['share']['is_public']
req = fakes.HTTPRequest.blank('/v2/shares/manage', version=version,
use_admin_context=True)
actual_result = self.controller.manage(req, data)
share_api.API.manage.assert_called_once_with(
mock.ANY, share, driver_options)
self.assertIsNotNone(actual_result)
self.mock_policy_check.assert_called_once_with(
req.environ['manila.context'], self.resource_name, 'manage')
def test_wrong_permissions(self):
body = get_fake_manage_body()

2
manila_tempest_tests/config.py
View File

@ -36,7 +36,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.7",
default="2.8",
help="The maximum api microversion is configured to be the "
"value of the latest microversion supported by Manila."),
cfg.StrOpt("region",

4
manila_tempest_tests/services/share/v2/json/shares_client.py
View File

@ -323,7 +323,8 @@ class SharesV2Client(shares_client.SharesClient):
def manage_share(self, service_host, protocol, export_path,
share_type_id, name=None, description=None,
version=LATEST_MICROVERSION, url=None):
is_public=False, version=LATEST_MICROVERSION,
url=None):
post_body = {
"share": {
"export_path": export_path,
@ -332,6 +333,7 @@ class SharesV2Client(shares_client.SharesClient):
"share_type": share_type_id,
"name": name,
"description": description,
"is_public": is_public,
}
}
if url is None:

66
manila_tempest_tests/tests/api/admin/test_share_manage.py
View File

@ -75,31 +75,37 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
'share_type_id': cls.st['share_type']['id'],
'share_protocol': cls.protocol,
}}
# Create two shares in parallel
cls.shares = cls.create_shares([creation_data, creation_data])
# Data for creating shares in parallel
data = [creation_data, creation_data]
if float(CONF.share.max_api_microversion) >= 2.8:
data.append(creation_data)
shares_created = cls.create_shares(data)
cls.shares = []
# Load all share data (host, etc.)
cls.share1 = cls.shares_v2_client.get_share(cls.shares[0]['id'])
cls.share2 = cls.shares_v2_client.get_share(cls.shares[1]['id'])
for share in shares_created:
# Unmanage shares from manila
cls.shares.append(cls.shares_client.get_share(share['id']))
cls.shares_client.unmanage_share(share['id'])
cls.shares_client.wait_for_resource_deletion(
share_id=share['id'])
# Unmanage shares from manila
for share_id in (cls.share1['id'], cls.share2['id']):
cls.shares_v2_client.unmanage_share(share_id)
cls.shares_v2_client.wait_for_resource_deletion(share_id=share_id)
@test.attr(type=["gate", "smoke"])
def test_manage(self):
name = "Name for 'managed' share that had ID %s" % self.share1["id"]
def _test_manage(self, share, is_public=False,
version=CONF.share.max_api_microversion):
name = "Name for 'managed' share that had ID %s" % \
share['id']
description = "Description for 'managed' share"
# Manage share
share = self.shares_v2_client.manage_share(
service_host=self.share1['host'],
export_path=self.share1['export_locations'][0],
protocol=self.share1['share_proto'],
service_host=share['host'],
export_path=share['export_locations'][0],
protocol=share['share_proto'],
share_type_id=self.st['share_type']['id'],
name=name,
description=description,
is_public=is_public,
)
# Add managed share to cleanup queue
@ -114,12 +120,17 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
get = self.shares_v2_client.get_share(share['id'], version="2.5")
self.assertEqual(name, get['name'])
self.assertEqual(description, get['description'])
self.assertEqual(self.share1['host'], get['host'])
self.assertEqual(self.share1['share_proto'], get['share_proto'])
self.assertEqual(share['host'], get['host'])
self.assertEqual(share['share_proto'], get['share_proto'])
self.assertEqual(self.st['share_type']['name'], get['share_type'])
get = self.shares_v2_client.get_share(share['id'], version="2.6")
self.assertEqual(self.st['share_type']['id'], get['share_type'])
share = self.shares_v2_client.get_share(share['id'], version="2.6")
self.assertEqual(self.st['share_type']['id'], share['share_type'])
if float(version) >= 2.8:
self.assertEqual(is_public, share['is_public'])
else:
self.assertFalse(share['is_public'])
# Delete share
self.shares_v2_client.delete_share(share['id'])
@ -128,6 +139,17 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
self.shares_v2_client.get_share,
share['id'])
@testtools.skipIf(
float(CONF.share.max_api_microversion) < 2.8,
"Only for API Microversion >= 2.8")
@test.attr(type=["gate", "smoke"])
def test_manage_with_is_public_True(self):
self._test_manage(share=self.shares[2], is_public=True)
@test.attr(type=["gate", "smoke"])
def test_manage(self):
self._test_manage(share=self.shares[0])
@test.attr(type=["gate", "smoke"])
def test_manage_retry(self):
# Manage share with invalid parameters
@ -137,9 +159,9 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
for share_type_id, status in parameters:
share = self.shares_v2_client.manage_share(
service_host=self.share2['host'],
export_path=self.share2['export_locations'][0],
protocol=self.share2['share_proto'],
service_host=self.shares[1]['host'],
export_path=self.shares[1]['export_locations'][0],
protocol=self.shares[1]['share_proto'],
share_type_id=share_type_id)
# Add managed share to cleanup queue