openstack/manila
Code Issues Proposed changes

Get ports filtered by subnet id on share-server cleanup

Browse Source 
Neutron can filter ports by subnet-id by itself, so:
- Don't send all ports in Neutron on every share-server removal
- Don't send fixed-ips fields (which are nearly half of total data)

Closes-Bug: #1879754
Change-Id: If42c9a0a662a62bbe2a02b6baee6ae13eedd8c97
Signed-off-by: George Melikov <mail@gmelikov.ru>
This commit is contained in:
George Melikov 2020-03-13 18:11:36 +03:00
parent 947315f090
commit d5fc7d2802
3 changed files with 21 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
manila/share/drivers/service_instance.py
View File

@ -840,7 +840,8 @@ class NeutronNetworkHelper(BaseNetworkhelper):
if router_id and subnet_id: if router_id and subnet_id:
ports = self.neutron_api.list_ports( ports = self.neutron_api.list_ports(
fields=['fixed_ips', 'device_id', 'device_owner']) fields=['device_id', 'device_owner'],
fixed_ips=['subnet_id=%s' % subnet_id])
# NOTE(vponomaryov): iterate ports to get to know whether current # NOTE(vponomaryov): iterate ports to get to know whether current
# subnet is used or not. We will not remove it from router if it # subnet is used or not. We will not remove it from router if it
# is used. # is used.
@ -850,12 +851,10 @@ class NeutronNetworkHelper(BaseNetworkhelper):
# we know that it is VM. We continue only if both are 'True'. # we know that it is VM. We continue only if both are 'True'.
if (port['device_id'] and if (port['device_id'] and
port['device_owner'].startswith('compute:')): port['device_owner'].startswith('compute:')):
for fixed_ip in port['fixed_ips']: # NOTE(vponomaryov): There are other share servers
if fixed_ip['subnet_id'] == subnet_id: # exist that use this subnet. So, do not remove it
# NOTE(vponomaryov): There are other share servers # from router.
# exist that use this subnet. So, do not remove it return
# from router.
return
try: try:
# NOTE(vponomaryov): there is no other share servers or # NOTE(vponomaryov): there is no other share servers or
# some VMs that use this subnet. So, remove it from router. # some VMs that use this subnet. So, remove it from router.

26
manila/tests/share/drivers/test_service_instance.py
View File

@ -1680,8 +1680,7 @@ class NeutronNetworkHelperTestCase(test.TestCase):
def test_teardown_network_subnet_is_used(self): def test_teardown_network_subnet_is_used(self):
server_details = dict(subnet_id='foo', router_id='bar') server_details = dict(subnet_id='foo', router_id='bar')
fake_ports = [ fake_ports = [
{'fixed_ips': [{'subnet_id': server_details['subnet_id']}], {'device_id': 'fake_device_id',
'device_id': 'fake_device_id',
'device_owner': 'compute:foo'}, 'device_owner': 'compute:foo'},
] ]
instance = self._init_neutron_network_plugin() instance = self._init_neutron_network_plugin()
@ -1699,19 +1698,16 @@ class NeutronNetworkHelperTestCase(test.TestCase):
service_instance.neutron.API.router_remove_interface.called) service_instance.neutron.API.router_remove_interface.called)
self.assertFalse(service_instance.neutron.API.update_subnet.called) self.assertFalse(service_instance.neutron.API.update_subnet.called)
service_instance.neutron.API.list_ports.assert_called_once_with( service_instance.neutron.API.list_ports.assert_called_once_with(
fields=['fixed_ips', 'device_id', 'device_owner']) fields=['device_id', 'device_owner'], fixed_ips=['subnet_id=foo'])
def test_teardown_network_subnet_not_used(self): def test_teardown_network_subnet_not_used(self):
server_details = dict(subnet_id='foo', router_id='bar') server_details = dict(subnet_id='foo', router_id='bar')
fake_ports = [ fake_ports = [
{'fixed_ips': [{'subnet_id': server_details['subnet_id']}], {'device_id': 'fake_device_id',
'device_id': 'fake_device_id',
'device_owner': 'network:router_interface'}, 'device_owner': 'network:router_interface'},
{'fixed_ips': [{'subnet_id': 'bar' + server_details['subnet_id']}], {'device_id': 'fake_device_id',
'device_id': 'fake_device_id',
'device_owner': 'compute'}, 'device_owner': 'compute'},
{'fixed_ips': [{'subnet_id': server_details['subnet_id']}], {'device_id': '',
'device_id': '',
'device_owner': 'compute'}, 'device_owner': 'compute'},
] ]
instance = self._init_neutron_network_plugin() instance = self._init_neutron_network_plugin()
@ -1730,13 +1726,12 @@ class NeutronNetworkHelperTestCase(test.TestCase):
(service_instance.neutron.API.update_subnet. (service_instance.neutron.API.update_subnet.
assert_called_once_with('foo', '')) assert_called_once_with('foo', ''))
service_instance.neutron.API.list_ports.assert_called_once_with( service_instance.neutron.API.list_ports.assert_called_once_with(
fields=['fixed_ips', 'device_id', 'device_owner']) fields=['device_id', 'device_owner'], fixed_ips=['subnet_id=foo'])
def test_teardown_network_subnet_not_used_and_get_error_404(self): def test_teardown_network_subnet_not_used_and_get_error_404(self):
server_details = dict(subnet_id='foo', router_id='bar') server_details = dict(subnet_id='foo', router_id='bar')
fake_ports = [ fake_ports = [
{'fixed_ips': [{'subnet_id': server_details['subnet_id']}], {'device_id': 'fake_device_id',
'device_id': 'fake_device_id',
'device_owner': 'fake'}, 'device_owner': 'fake'},
] ]
instance = self._init_neutron_network_plugin() instance = self._init_neutron_network_plugin()
@ -1756,13 +1751,12 @@ class NeutronNetworkHelperTestCase(test.TestCase):
(service_instance.neutron.API.update_subnet. (service_instance.neutron.API.update_subnet.
assert_called_once_with('foo', '')) assert_called_once_with('foo', ''))
service_instance.neutron.API.list_ports.assert_called_once_with( service_instance.neutron.API.list_ports.assert_called_once_with(
fields=['fixed_ips', 'device_id', 'device_owner']) fields=['device_id', 'device_owner'], fixed_ips=['subnet_id=foo'])
def test_teardown_network_subnet_not_used_get_unhandled_error(self): def test_teardown_network_subnet_not_used_get_unhandled_error(self):
server_details = dict(subnet_id='foo', router_id='bar') server_details = dict(subnet_id='foo', router_id='bar')
fake_ports = [ fake_ports = [
{'fixed_ips': [{'subnet_id': server_details['subnet_id']}], {'device_id': 'fake_device_id',
'device_id': 'fake_device_id',
'device_owner': 'fake'}, 'device_owner': 'fake'},
] ]
instance = self._init_neutron_network_plugin() instance = self._init_neutron_network_plugin()
@ -1783,7 +1777,7 @@ class NeutronNetworkHelperTestCase(test.TestCase):
assert_called_once_with('bar', 'foo')) assert_called_once_with('bar', 'foo'))
self.assertFalse(service_instance.neutron.API.update_subnet.called) self.assertFalse(service_instance.neutron.API.update_subnet.called)
service_instance.neutron.API.list_ports.assert_called_once_with( service_instance.neutron.API.list_ports.assert_called_once_with(
fields=['fixed_ips', 'device_id', 'device_owner']) fields=['device_id', 'device_owner'], fixed_ips=['subnet_id=foo'])
def test_setup_network_and_connect_share_server_to_tenant_net(self): def test_setup_network_and_connect_share_server_to_tenant_net(self):
def fake_create_port(*aargs, **kwargs): def fake_create_port(*aargs, **kwargs):

5
releasenotes/notes/bug-1879754-teardown-network-d1887cdf6eb83388.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- Fixed unneeded all ports list request to Neutron
in service instance helper module on tearing down
service subnet, Neutron can filter them by subnet_id itself.