Merge "fix netns deletion of broken namespaces" into unmaintained/yoga

Author: Zuul

neutron/agent/linux/ip_lib.py

@@ -260,7 +260,22 @@ def ensure_namespace(self, name):
         return ip
 
     def namespace_is_empty(self):
-        return not self.get_devices()
+        try:
+            return not self.get_devices()
+        except OSError as e:
+            # This can happen if we previously got terminated in the middle of
+            # removing this namespace. In this case the bind mount of the
+            # namespace under /var/run/netns will be removed, but the namespace
+            # file is still there. As the bind mount is gone we can no longer
+            # access the namespace to validate that it is empty. But since it
+            # should have already been removed we are sure that the check has
+            # passed the last time and since the namespace is unuseable that
+            # can not have changed.
+            # Future calls to pyroute2 to remove that namespace will clean up
+            # the leftover file.
+            if e.errno == errno.EINVAL:
+                return True
+            raise e
 
     def garbage_collect_namespace(self):
         """Conditionally destroy the namespace if it is empty."""

neutron/agent/ovn/metadata/agent.py

@@ -371,7 +371,10 @@ def sync(self):
                              ns.startswith(NS_PREFIX) and
                              ns not in metadata_namespaces]
         for ns in unused_namespaces:
-            self.teardown_datapath(self._get_datapath_name(ns))
+            try:
+                self.teardown_datapath(self._get_datapath_name(ns))
+            except Exception:
+                LOG.exception('Error unable to destroy namespace: %s', ns)
 
         # resync all network namespaces based on the associated datapaths,
         # even those that are already running. This is to make sure

neutron/tests/unit/agent/linux/test_ip_lib.py

@@ -356,6 +356,21 @@ def test_garbage_collect_namespace_existing_not_empty(self):
                 self.assertNotIn(mock.call().delete('ns'),
                                  ip_ns_cmd_cls.mock_calls)
 
+    def test_garbage_collect_namespace_existing_broken(self):
+        with mock.patch.object(ip_lib, 'IpNetnsCommand') as ip_ns_cmd_cls:
+            ip_ns_cmd_cls.return_value.exists.return_value = True
+
+            ip = ip_lib.IPWrapper(namespace='ns')
+
+            with mock.patch.object(ip, 'get_devices',
+                                   side_effect=OSError(errno.EINVAL, None)
+                                   ) as mock_get_devices:
+                self.assertTrue(ip.garbage_collect_namespace())
+
+                mock_get_devices.assert_called_once_with()
+                expected = [mock.call().delete('ns')]
+                ip_ns_cmd_cls.assert_has_calls(expected)
+
     @mock.patch.object(priv_lib, 'create_interface')
     def test_add_vlan(self, create):
         retval = ip_lib.IPWrapper().add_vlan('eth0.1', 'eth0', '1')

neutron/tests/unit/agent/ovn/metadata/test_agent.py

@@ -134,6 +134,31 @@ def test_sync_teardown_namespace(self):
             lnn.assert_called_once_with()
             tdp.assert_called_once_with('3')
 
+    def test_sync_teardown_namespace_does_not_crash_on_error(self):
+        """Test that sync tears down unneeded metadata namespaces.
+        Even if that fails it continues to provision other datapaths
+        """
+        with mock.patch.object(
+                self.agent, 'provision_datapath') as pdp,\
+                mock.patch.object(
+                    ip_lib, 'list_network_namespaces',
+                    return_value=['ovnmeta-1', 'ovnmeta-2', 'ovnmeta-3',
+                                  'ns1', 'ns2']) as lnn,\
+                mock.patch.object(
+                    self.agent, 'teardown_datapath',
+                    side_effect=Exception()) as tdp:
+            self.agent.sync()
+
+            pdp.assert_has_calls(
+                [
+                    mock.call(p.datapath)
+                    for p in self.ports
+                ],
+                any_order=True
+            )
+            lnn.assert_called_once_with()
+            tdp.assert_called_once_with('3')
+
     def test_get_networks_datapaths(self):
         """Test get_networks_datapaths returns only datapath objects for the
         networks containing vif ports of type ''(blank) and 'external'.