diff --git a/manila/share/drivers/service_instance.py b/manila/share/drivers/service_instance.py index a28a5121eb..b873a6b844 100644 --- a/manila/share/drivers/service_instance.py +++ b/manila/share/drivers/service_instance.py @@ -856,7 +856,7 @@ class NeutronNetworkHelper(BaseNetworkhelper): LOG.debug("Failed to delete port %(port_id)s with error: " "\n %(exc)s", {"port_id": port_id, "exc": e}) - if router_id and subnet_id: + if subnet_id: ports = self.neutron_api.list_ports( fields=['device_id', 'device_owner'], fixed_ips=['subnet_id=%s' % subnet_id]) @@ -873,17 +873,18 @@ class NeutronNetworkHelper(BaseNetworkhelper): # 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. - self.neutron_api.router_remove_interface( - router_id, subnet_id) - except exception.NetworkException as e: - if e.kwargs['code'] != 404: - raise - LOG.debug('Subnet %(subnet_id)s is not attached to the ' - 'router %(router_id)s.', - {'subnet_id': subnet_id, 'router_id': router_id}) + if router_id: + try: + # NOTE(vponomaryov): there is no other share servers or + # some VMs that use this subnet. So, remove it from router. + self.neutron_api.router_remove_interface( + router_id, subnet_id) + except exception.NetworkException as e: + if e.kwargs['code'] != 404: + raise + LOG.debug('Subnet %(subnet_id)s is not attached to the ' + 'router %(router_id)s.', + {'subnet_id': subnet_id, 'router_id': router_id}) self.neutron_api.update_subnet(subnet_id, '') @utils.synchronized( diff --git a/manila/tests/share/drivers/test_service_instance.py b/manila/tests/share/drivers/test_service_instance.py index 69b4616fc2..6ecc5d832d 100644 --- a/manila/tests/share/drivers/test_service_instance.py +++ b/manila/tests/share/drivers/test_service_instance.py @@ -1617,6 +1617,15 @@ class NeutronNetworkHelperTestCase(test.TestCase): @ddt.data(dict(), dict(subnet_id='foo'), dict(router_id='bar')) def test_teardown_network_no_service_data(self, server_details): + fake_ports = [ + {'device_id': 'fake_device_id', + 'device_owner': 'compute:foo'}, + ] + self.mock_object( + service_instance.neutron.API, 'update_subnet') + self.mock_object( + service_instance.neutron.API, 'list_ports', + mock.Mock(return_value=fake_ports)) instance = self._init_neutron_network_plugin() self.mock_object( service_instance.neutron.API, 'router_remove_interface') @@ -1757,6 +1766,31 @@ class NeutronNetworkHelperTestCase(test.TestCase): service_instance.neutron.API.list_ports.assert_called_once_with( fields=['device_id', 'device_owner'], fixed_ips=['subnet_id=foo']) + def test_teardown_network_subnet_not_used_with_no_router_id(self): + server_details = dict(subnet_id='foo') + fake_ports = [ + {'device_id': 'fake_device_id', + 'device_owner': 'compute'}, + {'device_id': '', + 'device_owner': 'compute'}, + ] + instance = self._init_neutron_network_plugin() + self.mock_object( + service_instance.neutron.API, 'router_remove_interface') + self.mock_object( + service_instance.neutron.API, 'update_subnet') + self.mock_object( + service_instance.neutron.API, 'list_ports', + mock.Mock(return_value=fake_ports)) + + instance.teardown_network(server_details) + self.assertFalse( + service_instance.neutron.API.router_remove_interface.called) + (service_instance.neutron.API.update_subnet. + assert_called_once_with('foo', '')) + service_instance.neutron.API.list_ports.assert_called_once_with( + 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 = [ diff --git a/releasenotes/notes/fix-no-router-server-0d5bf587063f22fc.yaml b/releasenotes/notes/fix-no-router-server-0d5bf587063f22fc.yaml new file mode 100644 index 0000000000..25fd3f6b74 --- /dev/null +++ b/releasenotes/notes/fix-no-router-server-0d5bf587063f22fc.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fix subnet cleanup for server instances without routers. + Previously, when tearing down a server instance that had no router + specified in its details, the associated subnet was not cleaned up because + the subnet cleanup code was never executed.