Fix server instance subnet cleanup

For server with no router specified in instance detail
during teardown network, subnet will not be clean as code
for clean subnet never executed.
This change allows to cleanup subnet correctly.

Change-Id: I856966f5994926424ae5421aba373daa999376d6
Signed-off-by: ricolin <rlin@vexxhost.com>
(cherry picked from commit b4fe639297)

manila/share/drivers/service_instance.py

@@ -865,7 +865,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])
@@ -882,17 +882,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(

manila/tests/share/drivers/test_service_instance.py

@@ -1622,6 +1622,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')
@@ -1762,6 +1771,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 = [

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.