openstack
/
manila
Code Issues Proposed changes

Missing check in ShareManager::manage_existing() 

In ShareManager::manage_existing(), there is a check for the DHSS mode of
the driver but there is no check for the DHSS mode in the specified share
type. This omission requires redundant checks in the drivers. This patch
adds the missing check to the share manager, removes the redundant checks
from the drivers, and updates all relevant unit tests.  Also, fix a
couple minor tempest resource cleanup issues discovered while
debugging this issue.

Change-Id: Ib579fd0558e59c28777342bb9d36def12f6bf4da
Closes-Bug: #1493869
This commit is contained in:
Clinton Knight 2015-09-09 11:27:40 -04:00
parent 3e0284b524
commit fd492fc11d
9 changed files with 53 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
manila/share/drivers/generic.py
View File

@ -24,7 +24,6 @@ from oslo_config import cfg
from oslo_log import log
from oslo_utils import excutils
from oslo_utils import importutils
from oslo_utils import strutils
from oslo_utils import units
import retrying
import six
@ -39,7 +38,6 @@ from manila.i18n import _LI
from manila.i18n import _LW
from manila.share import driver
from manila.share.drivers import service_instance
from manila.share import share_types
from manila import utils
from manila import volume
@ -831,22 +829,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
:param driver_options: Empty dict or dict with 'volume_id' option.
:return: dict with share size, example: {'size': 1}
"""
if self.driver_handles_share_servers:
msg = _('Operation "manage" for shares is supported only when '
'driver does not handle share servers.')
raise exception.InvalidDriverMode(msg)
helper = self._get_helper(share)
driver_mode = share_types.get_share_type_extra_specs(
share['share_type_id'],
const.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS)
if strutils.bool_from_string(driver_mode):
msg = _("%(mode)s != False") % {
'mode': const.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
}
raise exception.ManageExistingShareTypeMismatch(reason=msg)
share_server = self.service_instance_manager.get_common_server()
server_details = share_server['backend_details']

20
manila/share/drivers/hitachi/hds_hnas.py
View File

@ -15,17 +15,13 @@
from oslo_config import cfg
from oslo_log import log
from oslo_utils import strutils
import six
from manila.common import constants as const
from manila import exception
from manila.i18n import _
from manila.i18n import _LE
from manila.i18n import _LI
from manila.share import driver
from manila.share.drivers.hitachi import ssh
from manila.share import share_types
LOG = log.getLogger(__name__)
@ -379,22 +375,6 @@ class HDSHNASDriver(driver.ShareDriver):
:returns: Returns a dict with size of share managed
and its location (your path in file-system).
"""
if self.driver_handles_share_servers:
msg = (_("DHSS = %s") % self.driver_handles_share_servers)
LOG.error(_LE("Operation 'manage' for shares is supported only "
"when driver does not handle share servers."))
raise exception.InvalidDriverMode(driver_mode=msg)
driver_mode = share_types.get_share_type_extra_specs(
share['share_type_id'],
const.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS)
if strutils.bool_from_string(driver_mode):
msg = _("%(mode)s != False.") % {
'mode': const.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
}
raise exception.ManageExistingShareTypeMismatch(reason=msg)
share_id = self._get_hnas_share_id(share['id'])
LOG.info(_LI("Share %(shr_path)s will be managed with ID %(shr_id)s."),

12
manila/share/drivers/huawei/v3/connection.py
View File

@ -16,7 +16,6 @@
import time
from oslo_log import log
from oslo_utils import strutils
from oslo_utils import units
from manila.common import constants as common_constants
@ -29,7 +28,6 @@ from manila.share.drivers.huawei import constants
from manila.share.drivers.huawei import huawei_utils
from manila.share.drivers.huawei.v3 import helper
from manila.share.drivers.huawei.v3 import smartx
from manila.share import share_types
from manila.share import utils as share_utils
@ -495,16 +493,6 @@ class V3StorageConnection(driver.HuaweiBase):
def manage_existing(self, share, driver_options):
"""Manage existing share."""
driver_mode = share_types.get_share_type_extra_specs(
share['share_type_id'],
common_constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS)
if strutils.bool_from_string(driver_mode):
msg = _("%(mode)s != False") % {
'mode':
common_constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
}
raise exception.ManageExistingShareTypeMismatch(reason=msg)
share_proto = share['share_proto']
share_name = share['name']

14
manila/share/manager.py
View File

@ -28,6 +28,7 @@ from oslo_serialization import jsonutils
from oslo_service import periodic_task
from oslo_utils import excutils
from oslo_utils import importutils
from oslo_utils import strutils
from oslo_utils import timeutils
import six
@ -44,6 +45,7 @@ import manila.share.configuration
from manila.share import drivers_private_data
from manila.share import migration
from manila.share import rpcapi as share_rpcapi
from manila.share import share_types
from manila.share import utils as share_utils
from manila import utils
@ -783,7 +785,17 @@ class ShareManager(manager.SchedulerDependentManager):
if self.driver.driver_handles_share_servers:
msg = _("Manage share is not supported for "
"driver_handles_share_servers=True mode.")
raise exception.InvalidShare(reason=msg)
raise exception.InvalidDriverMode(driver_mode=msg)
driver_mode = share_types.get_share_type_extra_specs(
share_instance['share_type_id'],
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS)
if strutils.bool_from_string(driver_mode):
msg = _("%(mode)s != False") % {
'mode': constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
}
raise exception.ManageExistingShareTypeMismatch(reason=msg)
share_update = (
self.driver.manage_existing(share_instance, driver_options)

29
manila/tests/share/drivers/hitachi/test_hds_hnas.py
View File

@ -327,29 +327,6 @@ class HDSHNASTestCase(test.TestCase):
CONF._unset_defaults_and_overrides()
def test_manage_share_type_dhss_true(self):
driver_op = 'fake'
self.mock_object(share_types, 'get_share_type_extra_specs',
mock.Mock(return_value='True'))
self.assertRaises(exception.ManageExistingShareTypeMismatch,
self._driver.manage_existing,
self.share, driver_op)
share_types.get_share_type_extra_specs.assert_called_once_with(
self.share['share_type_id'], self.const_dhss)
def test_manage_conf_dhss_true(self):
driver_op = 'fake'
CONF.set_default('driver_handles_share_servers', True)
self.mock_object(share_types, 'get_share_type_extra_specs',
mock.Mock(return_value='True'))
self.assertRaises(exception.InvalidDriverMode,
self._driver.manage_existing,
self.share, driver_op)
def test_manage_invalid_host(self):
driver_op = 'fake'
self.share_invalid_host = {
@ -369,8 +346,6 @@ class HDSHNASTestCase(test.TestCase):
self.assertRaises(exception.ShareBackendException,
self._driver.manage_existing,
self.share_invalid_host, driver_op)
share_types.get_share_type_extra_specs.assert_called_once_with(
self.share_invalid_host['share_type_id'], self.const_dhss)
def test_manage_invalid_path(self):
driver_op = 'fake'
@ -391,8 +366,6 @@ class HDSHNASTestCase(test.TestCase):
self.assertRaises(exception.ShareBackendException,
self._driver.manage_existing,
self.share_invalid_path, driver_op)
share_types.get_share_type_extra_specs.assert_called_once_with(
self.share_invalid_path['share_type_id'], self.const_dhss)
def test_manage_invalid_evs_ip(self):
driver_op = 'fake'
@ -413,8 +386,6 @@ class HDSHNASTestCase(test.TestCase):
self.assertRaises(exception.ShareBackendException,
self._driver.manage_existing,
self.share_invalid_ip, driver_op)
share_types.get_share_type_extra_specs.assert_called_once_with(
self.share_invalid_ip['share_type_id'], self.const_dhss)
def test_unmanage(self):
self._driver.unmanage(self.share)

27
manila/tests/share/drivers/huawei/test_huawei_nas.py
View File

@ -1483,33 +1483,6 @@ class HuaweiShareDriverTestCase(test.TestCase):
self.share_nfs,
self.driver_options)
def test_manage_existing_share_type_mismatch(self):
fake_extra_specs = {
'driver_handles_share_servers': 'True',
}
fake_share_type_id = 'fake_id'
fake_type_mismatch_extra = {
'test_with_extra': {
'created_at': 'fake_time',
'deleted': '0',
'deleted_at': None,
'extra_specs': fake_extra_specs,
'required_extra_specs': {},
'id': fake_share_type_id,
'name': 'test_with_extra',
'updated_at': None
}
}
share_type = fake_type_mismatch_extra['test_with_extra']
self.mock_object(db,
'share_type_get',
mock.Mock(return_value=share_type))
self.driver.plugin.helper.login()
self.assertRaises(exception.ManageExistingShareTypeMismatch,
self.driver.manage_existing,
self.share_nfs,
self.driver_options)
def test_get_pool_success(self):
self.driver.plugin.helper.login()
pool_name = self.driver.get_pool(self.share_nfs_host_not_exist)

18
manila/tests/share/drivers/test_generic.py
View File

@ -1309,12 +1309,6 @@ class GenericShareDriverTestCase(test.TestCase):
self.assertEqual(True, result['driver_handles_share_servers'])
self.assertEqual('Open Source', result['vendor_name'])
def test_manage_invalid_driver_mode(self):
CONF.set_default('driver_handles_share_servers', True)
self.assertRaises(exception.InvalidDriverMode,
self._driver.manage_existing, 'fake', {})
def _setup_manage_mocks(self,
get_share_type_extra_specs='False',
is_device_mounted=True,
@ -1340,18 +1334,6 @@ class GenericShareDriverTestCase(test.TestCase):
self.assertRaises(exception.InvalidShare,
self._driver.manage_existing, share, {})
def test_manage_share_type_mismatch(self):
share = {'share_proto': 'NFS', 'share_type_id': 'fake'}
self._setup_manage_mocks(get_share_type_extra_specs='True')
self.assertRaises(exception.ManageExistingShareTypeMismatch,
self._driver.manage_existing, share, {})
share_types.get_share_type_extra_specs.assert_called_once_with(
share['share_type_id'],
const.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
)
def test_manage_not_mounted_share(self):
share = get_fake_manage_share()
fake_path = '/foo/bar'

42
manila/tests/share/test_manager.py
View File

@ -33,6 +33,7 @@ from manila.share import drivers_private_data
from manila.share import manager
from manila.share import migration
from manila.share import rpcapi
from manila.share import share_types
from manila import test
from manila.tests import db_utils
from manila.tests import utils as test_utils
@ -833,12 +834,33 @@ class ShareManagerTestCase(test.TestCase):
def test_manage_share_invalid_driver(self):
self.mock_object(self.share_manager, 'driver', mock.Mock())
self.share_manager.driver.driver_handles_share_servers = True
self.mock_object(share_types,
'get_share_type_extra_specs',
mock.Mock(return_value='False'))
self.mock_object(self.share_manager.db, 'share_update', mock.Mock())
share = db_utils.create_share()
share_id = share['id']
self.assertRaises(
exception.InvalidShare,
exception.InvalidDriverMode,
self.share_manager.manage_share, self.context, share_id, {})
self.share_manager.db.share_update.assert_called_once_with(
utils.IsAMatcher(context.RequestContext), share_id,
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
def test_manage_share_invalid_share_type(self):
self.mock_object(self.share_manager, 'driver', mock.Mock())
self.share_manager.driver.driver_handles_share_servers = False
self.mock_object(share_types,
'get_share_type_extra_specs',
mock.Mock(return_value='True'))
self.mock_object(self.share_manager.db, 'share_update', mock.Mock())
share = db_utils.create_share()
share_id = share['id']
self.assertRaises(
exception.ManageExistingShareTypeMismatch,
self.share_manager.manage_share, self.context, share_id, {})
self.share_manager.db.share_update.assert_called_once_with(
@ -849,9 +871,12 @@ class ShareManagerTestCase(test.TestCase):
CustomException = type('CustomException', (Exception,), dict())
self.mock_object(self.share_manager, 'driver', mock.Mock())
self.share_manager.driver.driver_handles_share_servers = False
self.mock_object(
self.share_manager.driver,
"manage_existing", mock.Mock(side_effect=CustomException))
self.mock_object(self.share_manager.driver,
'manage_existing',
mock.Mock(side_effect=CustomException))
self.mock_object(share_types,
'get_share_type_extra_specs',
mock.Mock(return_value='False'))
self.mock_object(self.share_manager.db, 'share_update', mock.Mock())
share = db_utils.create_share()
share_id = share['id']
@ -872,6 +897,9 @@ class ShareManagerTestCase(test.TestCase):
def test_manage_share_invalid_size(self):
self.mock_object(self.share_manager, 'driver')
self.share_manager.driver.driver_handles_share_servers = False
self.mock_object(share_types,
'get_share_type_extra_specs',
mock.Mock(return_value='False'))
self.mock_object(self.share_manager.driver,
"manage_existing",
mock.Mock(return_value=None))
@ -894,6 +922,9 @@ class ShareManagerTestCase(test.TestCase):
def test_manage_share_quota_error(self):
self.mock_object(self.share_manager, 'driver')
self.share_manager.driver.driver_handles_share_servers = False
self.mock_object(share_types,
'get_share_type_extra_specs',
mock.Mock(return_value='False'))
self.mock_object(self.share_manager.driver,
"manage_existing",
mock.Mock(return_value={'size': 3}))
@ -934,6 +965,9 @@ class ShareManagerTestCase(test.TestCase):
mock.Mock(side_effect=(
self.share_manager.db.share_export_locations_update)))
self.share_manager.driver.driver_handles_share_servers = False
self.mock_object(share_types,
'get_share_type_extra_specs',
mock.Mock(return_value='False'))
self.mock_object(self.share_manager.driver,
"manage_existing",
mock.Mock(return_value=driver_data))

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

@ -100,7 +100,7 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
# Add managed share to cleanup queue
self.method_resources.insert(
0, {'type': 'share_type', 'id': share['id'],
0, {'type': 'share', 'id': share['id'],
'client': self.shares_client})
# Wait for success
@ -137,7 +137,7 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
# Add managed share to cleanup queue
self.method_resources.insert(
0, {'type': 'share_type', 'id': share['id'],
0, {'type': 'share', 'id': share['id'],
'client': self.shares_client})
# Wait for success