From d5fc7d28023fb060a077e7913c38b2aa59deae4b Mon Sep 17 00:00:00 2001 From: George Melikov Date: Fri, 13 Mar 2020 18:11:36 +0300 Subject: [PATCH] Get ports filtered by subnet id on share-server cleanup 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 --- manila/share/drivers/service_instance.py | 13 +++++----- .../share/drivers/test_service_instance.py | 26 +++++++------------ ...754-teardown-network-d1887cdf6eb83388.yaml | 5 ++++ 3 files changed, 21 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/bug-1879754-teardown-network-d1887cdf6eb83388.yaml diff --git a/manila/share/drivers/service_instance.py b/manila/share/drivers/service_instance.py index 6637cbf277..9831ade907 100644 --- a/manila/share/drivers/service_instance.py +++ b/manila/share/drivers/service_instance.py @@ -840,7 +840,8 @@ class NeutronNetworkHelper(BaseNetworkhelper): if router_id and subnet_id: 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 # subnet is used or not. We will not remove it from router if it # is used. @@ -850,12 +851,10 @@ class NeutronNetworkHelper(BaseNetworkhelper): # we know that it is VM. We continue only if both are 'True'. if (port['device_id'] and port['device_owner'].startswith('compute:')): - for fixed_ip in port['fixed_ips']: - if fixed_ip['subnet_id'] == subnet_id: - # NOTE(vponomaryov): There are other share servers - # exist that use this subnet. So, do not remove it - # from router. - return + # NOTE(vponomaryov): There are other share servers + # exist that use this subnet. So, do not remove it + # from router. + return try: # NOTE(vponomaryov): there is no other share servers or # some VMs that use this subnet. So, remove it from router. diff --git a/manila/tests/share/drivers/test_service_instance.py b/manila/tests/share/drivers/test_service_instance.py index 3c0ad3a0fe..5cbb9c9334 100644 --- a/manila/tests/share/drivers/test_service_instance.py +++ b/manila/tests/share/drivers/test_service_instance.py @@ -1680,8 +1680,7 @@ class NeutronNetworkHelperTestCase(test.TestCase): def test_teardown_network_subnet_is_used(self): server_details = dict(subnet_id='foo', router_id='bar') fake_ports = [ - {'fixed_ips': [{'subnet_id': server_details['subnet_id']}], - 'device_id': 'fake_device_id', + {'device_id': 'fake_device_id', 'device_owner': 'compute:foo'}, ] instance = self._init_neutron_network_plugin() @@ -1699,19 +1698,16 @@ class NeutronNetworkHelperTestCase(test.TestCase): service_instance.neutron.API.router_remove_interface.called) self.assertFalse(service_instance.neutron.API.update_subnet.called) 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): server_details = dict(subnet_id='foo', router_id='bar') 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'}, - {'fixed_ips': [{'subnet_id': 'bar' + server_details['subnet_id']}], - 'device_id': 'fake_device_id', + {'device_id': 'fake_device_id', 'device_owner': 'compute'}, - {'fixed_ips': [{'subnet_id': server_details['subnet_id']}], - 'device_id': '', + {'device_id': '', 'device_owner': 'compute'}, ] instance = self._init_neutron_network_plugin() @@ -1730,13 +1726,12 @@ class NeutronNetworkHelperTestCase(test.TestCase): (service_instance.neutron.API.update_subnet. assert_called_once_with('foo', '')) 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): server_details = dict(subnet_id='foo', router_id='bar') fake_ports = [ - {'fixed_ips': [{'subnet_id': server_details['subnet_id']}], - 'device_id': 'fake_device_id', + {'device_id': 'fake_device_id', 'device_owner': 'fake'}, ] instance = self._init_neutron_network_plugin() @@ -1756,13 +1751,12 @@ class NeutronNetworkHelperTestCase(test.TestCase): (service_instance.neutron.API.update_subnet. assert_called_once_with('foo', '')) 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): server_details = dict(subnet_id='foo', router_id='bar') fake_ports = [ - {'fixed_ips': [{'subnet_id': server_details['subnet_id']}], - 'device_id': 'fake_device_id', + {'device_id': 'fake_device_id', 'device_owner': 'fake'}, ] instance = self._init_neutron_network_plugin() @@ -1783,7 +1777,7 @@ class NeutronNetworkHelperTestCase(test.TestCase): assert_called_once_with('bar', 'foo')) self.assertFalse(service_instance.neutron.API.update_subnet.called) 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 fake_create_port(*aargs, **kwargs): diff --git a/releasenotes/notes/bug-1879754-teardown-network-d1887cdf6eb83388.yaml b/releasenotes/notes/bug-1879754-teardown-network-d1887cdf6eb83388.yaml new file mode 100644 index 0000000000..e71beb11a2 --- /dev/null +++ b/releasenotes/notes/bug-1879754-teardown-network-d1887cdf6eb83388.yaml @@ -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.