NetApp cDOT driver is too strict in delete workflows

If a share or share server is not created successfully,
sometimes Manila will not save the vserver info in the
share or share server object. In such cases, when asked
to delete such a share or share server, the cDOT driver
should not protest about the lack of vserver info but
instead should log a warning and not raise an exception
(which leaves the object in an error_deleting state).

This patch addresses a number of issues in the delete,
share-server-delete, and snapshot-delete workflows where
the cDOT driver could unnecessarily raise an exception
when it should merely do nothing and allow the workflow
to proceed.

Change-Id: I54cf96b8a24ac5272b37bce2f5118551504a1699
Closes-Bug: #1438893
This commit is contained in:
Clinton Knight 2015-04-03 11:08:58 -04:00
parent 52750a0824
commit 6999c8d26b
12 changed files with 369 additions and 32 deletions

View File

@ -498,12 +498,32 @@ class ServiceInstanceUnavailable(ServiceInstanceException):
message = _("Service instance is not available.")
class StorageResourceException(ManilaException):
message = _("Storage resource exception.")
class StorageResourceNotFound(StorageResourceException):
message = _("Storage resource %(name)s not found.")
class SnapshotNotFound(StorageResourceNotFound):
message = _("Snapshot %(name)s not found.")
class SnapshotUnavailable(StorageResourceException):
message = _("Snapshot %(name)s info not available.")
class NetAppException(ManilaException):
message = _("Exception due to NetApp failure.")
class VserverUnavailable(NetAppException):
message = _("Vserver %(vserver)s is not available.")
class VserverNotFound(NetAppException):
message = _("Vserver %(vserver)s not found.")
class VserverNotSpecified(NetAppException):
message = _("Vserver not specified.")
class EMCVnxXMLAPIError(Invalid):

View File

@ -33,10 +33,12 @@ LOG = log.getLogger(__name__)
EONTAPI_EINVAL = '22'
EAPINOTFOUND = '13005'
ESNAPSHOTNOTALLOWED = '13023'
EVOLUMEOFFLINE = '13042'
EINTERNALERROR = '13114'
EDUPLICATEENTRY = '13130'
EVOLNOTCLONE = '13170'
EVOL_NOT_MOUNTED = '14716'
ESIS_CLONE_NOT_LICENSED = '14956'
EOBJECTNOTFOUND = '15661'

View File

@ -899,7 +899,12 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
@na_utils.trace
def offline_volume(self, volume_name):
"""Offlines a volume."""
self.send_request('volume-offline', {'name': volume_name})
try:
self.send_request('volume-offline', {'name': volume_name})
except netapp_api.NaApiError as e:
if e.code == netapp_api.EVOLUMEOFFLINE:
return
raise
@na_utils.trace
def unmount_volume(self, volume_name, force=False):
@ -908,7 +913,12 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
'volume-name': volume_name,
'force': six.text_type(force).lower(),
}
self.send_request('volume-unmount', api_args)
try:
self.send_request('volume-unmount', api_args)
except netapp_api.NaApiError as e:
if e.code == netapp_api.EVOL_NOT_MOUNTED:
return
raise
@na_utils.trace
def delete_volume(self, volume_name):
@ -944,11 +954,33 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
}
result = self.send_request('snapshot-get-iter', api_args)
error_record_list = result.get_child_by_name(
'volume-errors') or netapp_api.NaElement('none')
errors = error_record_list.get_children()
if errors:
error = errors[0]
error_code = error.get_child_content('errno')
error_reason = error.get_child_content('reason')
msg = _('Could not read information for snapshot %(name)s. '
'Code: %(code)s. Reason: %(reason)s')
msg_args = {
'name': snapshot_name,
'code': error_code,
'reason': error_reason
}
if error_code == netapp_api.ESNAPSHOTNOTALLOWED:
raise exception.SnapshotUnavailable(msg % msg_args)
else:
raise exception.NetAppException(msg % msg_args)
attributes_list = result.get_child_by_name(
'attributes-list') or netapp_api.NaElement('none')
snapshot_info_list = attributes_list.get_children()
if not self._has_records(result) or len(snapshot_info_list) != 1:
if not self._has_records(result):
raise exception.SnapshotNotFound(name=snapshot_name)
elif len(snapshot_info_list) > 1:
msg = _('Could not find unique snapshot %(snap)s on '
'volume %(vol)s.')
msg_args = {'snap': snapshot_name, 'vol': volume_name}

View File

@ -28,7 +28,7 @@ from oslo_utils import units
import six
from manila import exception
from manila.i18n import _, _LE, _LI
from manila.i18n import _, _LE, _LI, _LW
from manila.openstack.common import loopingcall
from manila.share.drivers.netapp.dataontap.client import client_cmode
from manila.share.drivers.netapp.dataontap.protocols import cifs_cmode
@ -459,8 +459,19 @@ class NetAppCmodeFileStorageLibrary(object):
@na_utils.trace
def delete_share(self, context, share, share_server=None):
"""Deletes share."""
try:
vserver, vserver_client = self._get_vserver(
share_server=share_server)
except (exception.InvalidInput,
exception.VserverNotSpecified,
exception.VserverNotFound) as error:
LOG.warning(_LW("Could not determine share server for share being "
"deleted: %(share)s. Deletion of share record "
"will proceed anyway. Error: %(error)s"),
{'share': share['id'], 'error': error})
return
share_name = self._get_valid_share_name(share['id'])
vserver, vserver_client = self._get_vserver(share_server=share_server)
if self._share_exists(share_name, vserver_client):
self._remove_export(share, vserver_client)
self._deallocate_container(share_name, vserver_client)
@ -516,11 +527,27 @@ class NetAppCmodeFileStorageLibrary(object):
@na_utils.trace
def delete_snapshot(self, context, snapshot, share_server=None):
"""Deletes a snapshot of a share."""
vserver, vserver_client = self._get_vserver(share_server=share_server)
try:
vserver, vserver_client = self._get_vserver(
share_server=share_server)
except (exception.InvalidInput,
exception.VserverNotSpecified,
exception.VserverNotFound) as error:
LOG.warning(_LW("Could not determine share server for snapshot "
"being deleted: %(snap)s. Deletion of snapshot "
"record will proceed anyway. Error: %(error)s"),
{'snap': snapshot['id'], 'error': error})
return
share_name = self._get_valid_share_name(snapshot['share_id'])
snapshot_name = self._get_valid_snapshot_name(snapshot['id'])
self._handle_busy_snapshot(vserver_client, share_name, snapshot_name)
try:
self._handle_busy_snapshot(vserver_client, share_name,
snapshot_name)
except exception.SnapshotNotFound:
LOG.info(_LI("Snapshot %s does not exist."), snapshot_name)
return
LOG.debug('Deleting snapshot %(snap)s for share %(share)s.',
{'snap': snapshot_name, 'share': share_name})

View File

@ -80,7 +80,8 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
def _get_vserver(self, share_server=None):
if not share_server:
raise exception.NetAppException(_('Share server not provided.'))
msg = _('Share server not provided')
raise exception.InvalidInput(reason=msg)
backend_details = share_server.get('backend_details')
vserver = backend_details.get(
@ -89,10 +90,10 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
if not vserver:
msg = _('Vserver name is absent in backend details. Please '
'check whether Vserver was created properly.')
raise exception.NetAppException(msg)
raise exception.VserverNotSpecified(msg)
if not self._client.vserver_exists(vserver):
raise exception.VserverUnavailable(vserver=vserver)
raise exception.VserverNotFound(vserver=vserver)
vserver_client = self._get_api_client(vserver)
return vserver, vserver_client
@ -205,8 +206,23 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
@na_utils.trace
def teardown_server(self, server_details, security_services=None):
"""Teardown share network."""
vserver = server_details['vserver_name']
"""Teardown share server."""
vserver = server_details.get(
'vserver_name') if server_details else None
if not vserver:
LOG.warning(_LW("Vserver not specified for share server being "
"deleted. Deletion of share server record will "
"proceed anyway."))
return
elif not self._client.vserver_exists(vserver):
LOG.warning(_LW("Could not find Vserver for share server being "
"deleted: %s. Deletion of share server "
"record will proceed anyway."), vserver)
return
vserver_client = self._get_api_client(vserver=vserver)
self._client.delete_vserver(vserver, vserver_client,
self._client.delete_vserver(vserver,
vserver_client,
security_services=security_services)

View File

@ -53,7 +53,7 @@ class NetAppCmodeSingleSVMFileStorageLibrary(
# Ensure vserver exists.
if not self._client.vserver_exists(self._vserver):
raise exception.VserverUnavailable(vserver=self._vserver)
raise exception.VserverNotFound(vserver=self._vserver)
# If we have vserver credentials, ensure the vserver they connect
# to matches the vserver specified in the configuration.
@ -95,7 +95,7 @@ class NetAppCmodeSingleSVMFileStorageLibrary(
raise exception.InvalidInput(reason=msg)
if not self._client.vserver_exists(self._vserver):
raise exception.VserverUnavailable(vserver=self._vserver)
raise exception.VserverNotFound(vserver=self._vserver)
vserver_client = self._get_api_client(self._vserver)
return self._vserver, vserver_client

View File

@ -1019,6 +1019,62 @@ SNAPSHOT_GET_ITER_BUSY_RESPONSE = etree.XML("""
</results>
""" % {'snap': SNAPSHOT_NAME, 'volume': SHARE_NAME, 'vserver': VSERVER_NAME})
SNAPSHOT_GET_ITER_NOT_UNIQUE_RESPONSE = etree.XML("""
<results status="passed">
<attributes-list>
<snapshot-info>
<busy>false</busy>
<name>%(snap)s</name>
<volume>%(volume)s</volume>
<vserver>%(vserver)s</vserver>
</snapshot-info>
<snapshot-info>
<busy>false</busy>
<name>%(snap)s</name>
<volume>%(root_volume)s</volume>
<vserver>%(admin_vserver)s</vserver>
</snapshot-info>
</attributes-list>
<num-records>1</num-records>
</results>
""" % {
'snap': SNAPSHOT_NAME,
'volume': SHARE_NAME,
'vserver': VSERVER_NAME,
'root_volume': ROOT_VOLUME_NAME,
'admin_vserver': ADMIN_VSERVER_NAME,
})
SNAPSHOT_GET_ITER_UNAVAILABLE_RESPONSE = etree.XML("""
<results status="passed">
<num-records>0</num-records>
<volume-errors>
<volume-error>
<errno>13023</errno>
<name>%(volume)s</name>
<reason>Unable to get information for Snapshot copies of volume \
"%(volume)s" on Vserver "%(vserver)s". Reason: Volume not online.</reason>
<vserver>%(vserver)s</vserver>
</volume-error>
</volume-errors>
</results>
""" % {'volume': SHARE_NAME, 'vserver': VSERVER_NAME})
SNAPSHOT_GET_ITER_OTHER_ERROR_RESPONSE = etree.XML("""
<results status="passed">
<num-records>0</num-records>
<volume-errors>
<volume-error>
<errno>99999</errno>
<name>%(volume)s</name>
<reason>Unable to get information for Snapshot copies of volume \
"%(volume)s" on Vserver "%(vserver)s".</reason>
<vserver>%(vserver)s</vserver>
</volume-error>
</volume-errors>
</results>
""" % {'volume': SHARE_NAME, 'vserver': VSERVER_NAME})
NFS_EXPORT_RULES = ('10.10.10.10', '10.10.10.20')
NFS_EXPORTFS_LIST_RULES_2_NO_RULES_RESPONSE = etree.XML("""

View File

@ -1594,6 +1594,30 @@ class NetAppClientCmodeTestCase(test.TestCase):
self.client.send_request.assert_has_calls([
mock.call('volume-offline', volume_offline_args)])
def test_offline_volume_already_offline(self):
self.mock_object(self.client,
'send_request',
mock.Mock(side_effect=self._mock_api_error(
netapp_api.EVOLUMEOFFLINE)))
self.client.offline_volume(fake.SHARE_NAME)
volume_offline_args = {'name': fake.SHARE_NAME}
self.client.send_request.assert_has_calls([
mock.call('volume-offline', volume_offline_args)])
def test_offline_volume_api_error(self):
self.mock_object(self.client,
'send_request',
mock.Mock(side_effect=self._mock_api_error()))
self.assertRaises(netapp_api.NaApiError,
self.client.offline_volume,
fake.SHARE_NAME)
def test_unmount_volume(self):
self.mock_object(self.client, 'send_request')
@ -1614,14 +1638,36 @@ class NetAppClientCmodeTestCase(test.TestCase):
self.client.unmount_volume(fake.SHARE_NAME, force=True)
volume_unmount_args = {
'volume-name': fake.SHARE_NAME,
'force': 'true'
}
volume_unmount_args = {'volume-name': fake.SHARE_NAME, 'force': 'true'}
self.client.send_request.assert_has_calls([
mock.call('volume-unmount', volume_unmount_args)])
def test_unmount_volume_already_unmounted(self):
self.mock_object(self.client,
'send_request',
mock.Mock(side_effect=self._mock_api_error(
netapp_api.EVOL_NOT_MOUNTED)))
self.client.unmount_volume(fake.SHARE_NAME, force=True)
volume_unmount_args = {'volume-name': fake.SHARE_NAME, 'force': 'true'}
self.client.send_request.assert_has_calls([
mock.call('volume-unmount', volume_unmount_args)])
def test_unmount_volume_api_error(self):
self.mock_object(self.client,
'send_request',
mock.Mock(side_effect=self._mock_api_error()))
self.assertRaises(netapp_api.NaApiError,
self.client.unmount_volume,
fake.SHARE_NAME,
force=True)
def test_delete_volume(self):
self.mock_object(self.client, 'send_request')
@ -1696,14 +1742,28 @@ class NetAppClientCmodeTestCase(test.TestCase):
mock.call('snapshot-get-iter', snapshot_get_iter_args)])
self.assertDictEqual(expected, result)
def test_get_snapshot_not_found(self):
@ddt.data({
'api_response_xml': fake.NO_RECORDS_RESPONSE,
'raised_exception': exception.SnapshotNotFound,
}, {
'api_response_xml': fake.SNAPSHOT_GET_ITER_NOT_UNIQUE_RESPONSE,
'raised_exception': exception.NetAppException,
}, {
'api_response_xml': fake.SNAPSHOT_GET_ITER_UNAVAILABLE_RESPONSE,
'raised_exception': exception.SnapshotUnavailable,
}, {
'api_response_xml': fake.SNAPSHOT_GET_ITER_OTHER_ERROR_RESPONSE,
'raised_exception': exception.NetAppException,
})
@ddt.unpack
def test_get_snapshot_error(self, api_response_xml, raised_exception):
api_response = netapp_api.NaElement(fake.NO_RECORDS_RESPONSE)
api_response = netapp_api.NaElement(api_response_xml)
self.mock_object(self.client,
'send_request',
mock.Mock(return_value=api_response))
self.assertRaises(exception.NetAppException,
self.assertRaises(raised_exception,
self.client.get_snapshot,
fake.SHARE_NAME,
fake.SNAPSHOT_NAME)

View File

@ -20,6 +20,7 @@ import copy
import socket
import time
import ddt
import mock
from oslo_log import log
@ -36,6 +37,7 @@ from manila import test
from manila.tests.share.drivers.netapp.dataontap import fakes as fake
@ddt.ddt
class NetAppFileStorageLibraryTestCase(test.TestCase):
def setUp(self):
@ -49,6 +51,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.mock_object(lib_base.LOG,
'info',
mock.Mock(side_effect=mock_logger.info))
self.mock_object(lib_base.LOG,
'warning',
mock.Mock(side_effect=mock_logger.warning))
self.mock_object(lib_base.LOG,
'error',
mock.Mock(side_effect=mock_logger.error))
@ -754,6 +759,30 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
vserver_client)
self.assertEqual(0, lib_base.LOG.info.call_count)
@ddt.data(exception.InvalidInput(reason='fake_reason'),
exception.VserverNotSpecified(),
exception.VserverNotFound(vserver='fake_vserver'))
def test_delete_share_no_share_server(self, get_vserver_exception):
self.mock_object(self.library,
'_get_vserver',
mock.Mock(side_effect=get_vserver_exception))
mock_share_exists = self.mock_object(self.library,
'_share_exists',
mock.Mock(return_value=False))
mock_remove_export = self.mock_object(self.library, '_remove_export')
mock_deallocate_container = self.mock_object(self.library,
'_deallocate_container')
self.library.delete_share(self.context,
fake.SHARE,
share_server=fake.SHARE_SERVER)
self.assertFalse(mock_share_exists.called)
self.assertFalse(mock_remove_export.called)
self.assertFalse(mock_deallocate_container.called)
self.assertEqual(1, lib_base.LOG.warning.call_count)
def test_delete_share_not_found(self):
vserver_client = mock.Mock()
@ -891,6 +920,64 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
vserver_client.delete_snapshot.assert_called_once_with(share_name,
snapshot_name)
@ddt.data(exception.InvalidInput(reason='fake_reason'),
exception.VserverNotSpecified(),
exception.VserverNotFound(vserver='fake_vserver'))
def test_delete_snapshot_no_share_server(self, get_vserver_exception):
self.mock_object(self.library,
'_get_vserver',
mock.Mock(side_effect=get_vserver_exception))
mock_handle_busy_snapshot = self.mock_object(self.library,
'_handle_busy_snapshot')
self.library.delete_snapshot(self.context,
fake.SNAPSHOT,
share_server=fake.SHARE_SERVER)
self.assertFalse(mock_handle_busy_snapshot.called)
self.assertEqual(1, lib_base.LOG.warning.call_count)
def test_delete_snapshot_not_found(self):
vserver_client = mock.Mock()
self.mock_object(self.library,
'_get_vserver',
mock.Mock(return_value=(fake.VSERVER1,
vserver_client)))
mock_handle_busy_snapshot = self.mock_object(
self.library, '_handle_busy_snapshot',
mock.Mock(side_effect=exception.SnapshotNotFound(
name=fake.SNAPSHOT_NAME)))
self.library.delete_snapshot(self.context,
fake.SNAPSHOT,
share_server=fake.SHARE_SERVER)
self.assertTrue(mock_handle_busy_snapshot.called)
self.assertFalse(vserver_client.delete_snapshot.called)
def test_delete_snapshot_busy(self):
vserver_client = mock.Mock()
self.mock_object(self.library,
'_get_vserver',
mock.Mock(return_value=(fake.VSERVER1,
vserver_client)))
mock_handle_busy_snapshot = self.mock_object(
self.library, '_handle_busy_snapshot',
mock.Mock(side_effect=exception.ShareSnapshotIsBusy(
snapshot_name=fake.SNAPSHOT_NAME)))
self.assertRaises(exception.ShareSnapshotIsBusy,
self.library.delete_snapshot,
self.context,
fake.SNAPSHOT,
share_server=fake.SHARE_SERVER)
self.assertTrue(mock_handle_busy_snapshot.called)
self.assertFalse(vserver_client.delete_snapshot.called)
def test_handle_busy_snapshot_not_busy(self):
vserver_client = mock.Mock()
@ -904,6 +991,18 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.assertEqual(1, vserver_client.get_snapshot.call_count)
self.assertEqual(0, lib_base.LOG.debug.call_count)
def test_handle_busy_snapshot_not_found(self):
vserver_client = mock.Mock()
vserver_client.get_snapshot.side_effect = exception.SnapshotNotFound(
name=fake.SNAPSHOT_NAME)
self.assertRaises(exception.SnapshotNotFound,
self.library._handle_busy_snapshot,
vserver_client,
fake.SHARE_NAME,
fake.SNAPSHOT_NAME)
def test_handle_busy_snapshot_not_clone_dependency(self):
snapshot = copy.deepcopy(fake.CDOT_SNAPSHOT_BUSY_VOLUME_CLONE)

View File

@ -121,7 +121,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
def test_get_vserver_no_share_server(self):
self.assertRaises(exception.NetAppException,
self.assertRaises(exception.InvalidInput,
self.library._get_vserver)
def test_get_vserver_no_backend_details(self):
@ -130,7 +130,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
fake_share_server.pop('backend_details')
kwargs = {'share_server': fake_share_server}
self.assertRaises(exception.NetAppException,
self.assertRaises(exception.VserverNotSpecified,
self.library._get_vserver,
**kwargs)
@ -140,7 +140,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
fake_share_server['backend_details'] = None
kwargs = {'share_server': fake_share_server}
self.assertRaises(exception.NetAppException,
self.assertRaises(exception.VserverNotSpecified,
self.library._get_vserver,
**kwargs)
@ -150,7 +150,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
fake_share_server['backend_details'].pop('vserver_name')
kwargs = {'share_server': fake_share_server}
self.assertRaises(exception.NetAppException,
self.assertRaises(exception.VserverNotSpecified,
self.library._get_vserver,
**kwargs)
@ -160,7 +160,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
fake_share_server['backend_details']['vserver_name'] = None
kwargs = {'share_server': fake_share_server}
self.assertRaises(exception.NetAppException,
self.assertRaises(exception.VserverNotSpecified,
self.library._get_vserver,
**kwargs)
@ -169,7 +169,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.library._client.vserver_exists.return_value = False
kwargs = {'share_server': fake.SHARE_SERVER}
self.assertRaises(exception.VserverUnavailable,
self.assertRaises(exception.VserverNotFound,
self.library._get_vserver,
**kwargs)
@ -432,12 +432,36 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
self.mock_object(self.library,
'_get_api_client',
mock.Mock(return_value=vserver_client))
self.library._client.vserver_exists.return_value = True
self.library.teardown_server(
fake.SHARE_SERVER['backend_details'],
security_services=fake.NETWORK_INFO['security_services'])
self.library._client.vserver_exists.assert_called_once_with(
fake.VSERVER1)
self.library._client.delete_vserver.assert_called_once_with(
fake.VSERVER1,
vserver_client,
security_services=fake.NETWORK_INFO['security_services'])
@ddt.data(None, {}, {'vserver_name': None})
def test_teardown_server_no_share_server(self, server_details):
self.library.teardown_server(server_details)
self.assertFalse(self.library._client.delete_vserver.called)
self.assertTrue(lib_multi_svm.LOG.warning.called)
def test_teardown_server_no_vserver(self):
self.library._client.vserver_exists.return_value = False
self.library.teardown_server(
fake.SHARE_SERVER['backend_details'],
security_services=fake.NETWORK_INFO['security_services'])
self.library._client.vserver_exists.assert_called_once_with(
fake.VSERVER1)
self.assertFalse(self.library._client.delete_vserver.called)
self.assertTrue(lib_multi_svm.LOG.warning.called)

View File

@ -83,7 +83,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
def test_check_for_setup_error_vserver_not_found(self):
self.library._client.vserver_exists.return_value = False
self.assertRaises(exception.VserverUnavailable,
self.assertRaises(exception.VserverNotFound,
self.library.check_for_setup_error)
def test_check_for_setup_error_cluster_creds_vserver_match(self):
@ -145,7 +145,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
def test_get_vserver_vserver_not_found(self):
self.library._client.vserver_exists.return_value = False
self.assertRaises(exception.VserverUnavailable,
self.assertRaises(exception.VserverNotFound,
self.library._get_vserver)
def test_find_matching_aggregates(self):

View File

@ -179,6 +179,7 @@ NETWORK_INFO = {
NETWORK_INFO_NETMASK = '255.255.255.0'
SHARE_SERVER = {
'share_network_id': 'c5b3a865-56d0-4d88-abe5-879965e099c9',
'backend_details': {
'vserver_name': VSERVER1
}