openstack
/
manila
Code Issues Proposed changes

Release network resources properly 

If we try to create share with backend that uses share-server,
its network resources won't be released because of improper param provided.
Method "deallocate_network" of module "neutron_network_plugin" expects
"share_server" as second param but "share_network" is provided indeed.

Make it be called only when share_server deleted and errored after network
resources provided and with proper params.

Change-Id: I8f79724dacb3dd7efc75dc459529a0c3185f999f
Closes-Bug: #1399732
This commit is contained in:
Valeriy Ponomaryov 2014-12-05 19:34:11 +02:00 committed by Ponomaryov Valeriy
parent 2023930fd7
commit 8b083605fb
5 changed files with 69 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
manila/network/__init__.py
View File

@ -41,7 +41,7 @@ class NetworkBaseAPI(object):
pass
@abc.abstractmethod
def deallocate_network(self, context, share_network):
def deallocate_network(self, context, share_server_id):
pass
@abc.abstractmethod

8
manila/network/neutron/neutron_network_plugin.py
View File

@ -64,18 +64,18 @@ class NeutronNetworkPlugin(manila_network.NetworkBaseAPI, db_base.Base):
return ports
def deallocate_network(self, context, share_server):
"""Deallocate neutron network resources for the given network info.
def deallocate_network(self, context, share_server_id):
"""Deallocate neutron network resources for the given share server.
Delete previously allocated neutron ports, delete manila db
records for deleted ports.
:param context: RequestContext object
:param share_network: share network data
:param share_server_id: id of share server
:rtype: None
"""
ports = self.db.network_allocations_get_for_share_server(
context, share_server['id'])
context, share_server_id)
for port in ports:
self._delete_port(context, port)

20
manila/share/manager.py
View File

@ -436,9 +436,14 @@ class ShareManager(manager.SchedulerDependentManager):
return network_info
def _setup_server(self, context, share_server, metadata=None):
share_network = self.db.share_network_get(
context, share_server['share_network_id'])
# NOTE(vponomaryov): set network_allocations to 0 before 'try' block
# for case we get exception calling appropriate method. This value will
# be used in exception handling and for case 'setup_server' method was
# not called we won't make redundant actions.
allocation_number = 0
try:
share_network = self.db.share_network_get(
context, share_server['share_network_id'])
allocation_number = self.driver.get_network_allocations_number()
if allocation_number:
self.network_api.allocate_network(
@ -478,7 +483,9 @@ class ShareManager(manager.SchedulerDependentManager):
self.db.share_server_update(context, share_server['id'],
{'status': constants.STATUS_ERROR})
self.network_api.deallocate_network(context, share_network)
if allocation_number:
self.network_api.deallocate_network(
context, share_server['id'])
def delete_share_server(self, context, share_server):
@ -519,4 +526,9 @@ class ShareManager(manager.SchedulerDependentManager):
_teardown_server()
LOG.info(_LI("Share server deleted successfully."))
self.network_api.deallocate_network(context, share_server)
# NOTE(vponomaryov): share servers created by Nova do not need
# explicit network allocations release. It is done by Nova itself.
# So, all drivers that use 'service_instance' module do not need
# following operation.
if self.driver.get_network_allocations_number():
self.network_api.deallocate_network(context, share_server['id'])

4
manila/tests/fake_driver.py
View File

@ -76,7 +76,9 @@ class FakeShareDriver(driver.ShareDriver):
pass
def get_network_allocations_number(self):
pass
# NOTE(vponomaryov): Simulate drivers that use share servers and
# do not use 'service_instance' module.
return 2
@staticmethod
def fake_execute(cmd, *_args, **_kwargs):

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

@ -952,8 +952,6 @@ class ShareManagerTestCase(test.TestCase):
mock.Mock(side_effect=exception.ManilaException()))
self.stubs.Set(self.share_manager.db, 'share_server_update',
mock.Mock())
self.stubs.Set(self.share_manager.network_api, 'deallocate_network',
mock.Mock())
# execute method _setup_server
self.assertRaises(
@ -968,70 +966,25 @@ class ShareManagerTestCase(test.TestCase):
assert_called_once_with()
self.share_manager.db.share_server_update.assert_called_once_with(
context, share_server['id'], {'status': constants.STATUS_ERROR})
self.share_manager.network_api.deallocate_network.\
assert_called_once_with(context, share_network)
def test_setup_server_exception_in_driver(self):
def setup_server_raise_exception(self, allocation_number,
detail_data_proper):
# Setup required test data
context = "fake_context"
share_server = {
'id': 'fake_id',
'share_network_id': 'fake_sn_id',
}
share_network = {'id': 'fake_sn_id'}
server_info = {'details_key': 'value'}
network_info = {'fake_network_info_key': 'fake_network_info_value'}
allocation_number = 0
# Mock required parameters
self.stubs.Set(self.share_manager.db, 'share_network_get',
mock.Mock(return_value=share_network))
self.stubs.Set(self.share_manager.driver,
'get_network_allocations_number',
mock.Mock(return_value=allocation_number))
self.stubs.Set(self.share_manager.db, 'share_server_update',
mock.Mock())
self.stubs.Set(self.share_manager.network_api, 'deallocate_network',
mock.Mock())
self.stubs.Set(self.share_manager, '_form_server_setup_info',
mock.Mock(return_value=network_info))
self.stubs.Set(self.share_manager.db,
'share_server_backend_details_set',
mock.Mock())
self.stubs.Set(self.share_manager.driver, 'setup_server',
mock.Mock(side_effect=exception.ManilaException(
detail_data={'server_details': server_info})))
# execute method _setup_server
self.assertRaises(
exception.ManilaException,
self.share_manager._setup_server,
context,
share_server,
)
self.share_manager.db.share_network_get.assert_called_once_with(
context, share_server['share_network_id'])
self.share_manager.driver.get_network_allocations_number.\
assert_called_once_with()
self.share_manager._form_server_setup_info.assert_called_once_with(
context, share_server, share_network)
self.share_manager.db.share_server_backend_details_set.\
assert_called_once_with(context, share_server['id'], server_info)
self.share_manager.db.share_server_update.assert_called_once_with(
context, share_server['id'], {'status': constants.STATUS_ERROR})
self.share_manager.network_api.deallocate_network.\
assert_called_once_with(context, share_network)
def test_setup_server_incorrect_detail_data(self):
# Setup required test data
context = "fake_context"
share_server = {
'id': 'fake_id',
'share_network_id': 'fake_sn_id',
}
share_network = {'id': 'fake_sn_id'}
network_info = {'fake_network_info_key': 'fake_network_info_value'}
allocation_number = 0
if detail_data_proper:
detail_data = {'server_details': server_info}
self.stubs.Set(self.share_manager.db,
'share_server_backend_details_set',
mock.Mock())
else:
detail_data = 'not dictionary detail data'
# Mock required parameters
self.stubs.Set(self.share_manager.db, 'share_network_get',
@ -1041,8 +994,9 @@ class ShareManagerTestCase(test.TestCase):
mock.Mock(return_value=allocation_number))
self.stubs.Set(self.share_manager.db, 'share_server_update',
mock.Mock())
self.stubs.Set(self.share_manager.network_api, 'deallocate_network',
mock.Mock())
if allocation_number:
for m in ['deallocate_network', 'allocate_network']:
self.stubs.Set(self.share_manager.network_api, m, mock.Mock())
self.stubs.Set(self.share_manager, '_form_server_setup_info',
mock.Mock(return_value=network_info))
self.stubs.Set(self.share_manager.db,
@ -1050,7 +1004,7 @@ class ShareManagerTestCase(test.TestCase):
mock.Mock())
self.stubs.Set(self.share_manager.driver, 'setup_server',
mock.Mock(side_effect=exception.ManilaException(
detail_data='not dictionary detail data')))
detail_data=detail_data)))
# execute method _setup_server
self.assertRaises(
@ -1059,13 +1013,41 @@ class ShareManagerTestCase(test.TestCase):
context,
share_server,
)
self.share_manager.db.share_network_get.assert_called_once_with(
context, share_server['share_network_id'])
if detail_data_proper:
self.share_manager.db.share_server_backend_details_set.\
assert_called_once_with(
context, share_server['id'], server_info)
self.share_manager.driver.get_network_allocations_number.\
assert_called_once_with()
self.share_manager._form_server_setup_info.assert_called_once_with(
context, share_server, share_network)
self.share_manager.db.share_server_update.assert_called_once_with(
context, share_server['id'], {'status': constants.STATUS_ERROR})
self.share_manager.network_api.deallocate_network.\
assert_called_once_with(context, share_network)
if allocation_number:
self.share_manager.db.share_network_get.assert_has_calls([
mock.call(context, share_server['share_network_id']),
mock.call(context, share_server['share_network_id'])])
self.share_manager.network_api.allocate_network.assert_has_calls([
mock.call(context, share_server, share_network,
count=allocation_number)])
self.share_manager.network_api.deallocate_network.assert_has_calls(
mock.call(context, share_server['id']))
else:
self.share_manager.db.share_network_get.assert_called_once_with(
context, share_server['share_network_id'])
def test_setup_server_incorrect_detail_data_an2(self):
# an2 - allocation number has value -> 2
self.setup_server_raise_exception(2, detail_data_proper=False)
def test_setup_server_incorrect_detail_data_an0(self):
# an0 - allocation number has value -> 0
self.setup_server_raise_exception(0, detail_data_proper=False)
def test_setup_server_exception_in_driver_an2(self):
# an2 - allocation number has value -> 2
self.setup_server_raise_exception(2, detail_data_proper=True)
def test_setup_server_exception_in_driver_an0(self):
# an0 - allocation number has value -> 0
self.setup_server_raise_exception(2, detail_data_proper=True)