Merge "Catch non-existent entry failures better in ip_lib" into stable/yoga

Author: Zuul

neutron/privileged/agent/linux/ip_lib.py

@@ -474,6 +474,10 @@ def delete_neigh_entry(ip_version, ip_address, mac_address, device, namespace,
         if e.code == errno.ENOENT:
             return
         raise
+    except OSError as e:
+        if e.errno == errno.ENOENT:
+            raise NetworkNamespaceNotFound(netns_name=namespace)
+        raise
 
 
 @tenacity.retry(
@@ -683,6 +687,11 @@ def delete_ip_rule(namespace, **kwargs):
     try:
         with get_iproute(namespace) as ip:
             ip.rule('del', **kwargs)
+    except netlink_exceptions.NetlinkError as e:
+        # trying to delete a non-existent entry shouldn't raise an error
+        if e.code == errno.ENOENT:
+            return
+        raise
     except OSError as e:
         if e.errno == errno.ENOENT:
             raise NetworkNamespaceNotFound(netns_name=namespace)
@@ -785,6 +794,11 @@ def delete_ip_route(namespace, cidr, ip_version, device=None, via=None,
     try:
         with get_iproute(namespace) as ip:
             ip.route('del', **kwargs)
+    except netlink_exceptions.NetlinkError as e:
+        # trying to delete a non-existent entry shouldn't raise an error
+        if e.code == errno.ESRCH:
+            return
+        raise
     except OSError as e:
         if e.errno == errno.ENOENT:
             raise NetworkNamespaceNotFound(netns_name=namespace)
@@ -819,6 +833,11 @@ def _command_bridge_fdb(command, mac, device, dst_ip=None, namespace=None,
             kwargs['dst'] = dst_ip
         with get_iproute(namespace) as ip:
             return make_serializable(ip.fdb(command, **kwargs))
+    except netlink_exceptions.NetlinkError as e:
+        # trying to delete a non-existent entry shouldn't raise an error
+        if command == 'del' and e.code == errno.ENOENT:
+            return
+        raise
     except OSError as e:
         if e.errno == errno.ENOENT:
             raise NetworkNamespaceNotFound(netns_name=namespace)
@@ -834,20 +853,20 @@ def add_bridge_fdb(mac, device, dst_ip=None, namespace=None, **kwargs):
 
 @privileged.default.entrypoint
 def append_bridge_fdb(mac, device, dst_ip=None, namespace=None, **kwargs):
-    """Add a FDB entry"""
+    """Append a FDB entry"""
     return _command_bridge_fdb('append', mac, device, dst_ip=dst_ip,
                                namespace=namespace, **kwargs)
 
 
 @privileged.default.entrypoint
 def replace_bridge_fdb(mac, device, dst_ip=None, namespace=None, **kwargs):
-    """Add a FDB entry"""
+    """Replace a FDB entry"""
     return _command_bridge_fdb('replace', mac, device, dst_ip=dst_ip,
                                namespace=namespace, **kwargs)
 
 
 @privileged.default.entrypoint
 def delete_bridge_fdb(mac, device, dst_ip=None, namespace=None, **kwargs):
-    """Add a FDB entry"""
+    """Delete a FDB entry"""
     return _command_bridge_fdb('del', mac, device, dst_ip=dst_ip,
                                namespace=namespace, **kwargs)

neutron/tests/functional/agent/linux/test_bridge_lib.py

@@ -211,6 +211,13 @@ def test_add_delete(self):
                                        namespace=self.namespace)
         self._assert_mac(self.MAC1, self.device, present=False)
 
+        try:
+            # This should not raise for a non-existent entry
+            bridge_lib.FdbInterface.delete(self.MAC1, self.device,
+                                           namespace=self.namespace)
+        except Exception:
+            self.fail('Delete FDB entry threw unexpected exception')
+
     def test_add_delete_dst(self):
         self._assert_mac(self.MAC1, self.device_vxlan, present=False)
         bridge_lib.FdbInterface.add(

neutron/tests/functional/privileged/agent/linux/test_ip_lib.py

@@ -239,19 +239,34 @@ def test_get_devices_info_veth_same_namespaces(self):
         self.assertEqual(veth1_2['index'], veth1_1_link)
 
 
-class ListIpRulesTestCase(functional_base.BaseSudoTestCase):
-
-    RULE_TABLES = {'default': 253, 'main': 254, 'local': 255}
+class BaseIpRuleTestCase(functional_base.BaseSudoTestCase):
 
     def setUp(self):
-        super(ListIpRulesTestCase, self).setUp()
+        super().setUp()
         self.namespace = 'ns_test-' + uuidutils.generate_uuid()
         self.ns = priv_ip_lib.create_netns(self.namespace)
         self.addCleanup(self._remove_ns)
 
     def _remove_ns(self):
         priv_ip_lib.remove_netns(self.namespace)
 
+    def _check_rules(self, rules, parameters, values, exception_string=None,
+                     raise_exception=True):
+        for rule in rules:
+            if all(rule.get(parameter) == value
+                   for parameter, value in zip(parameters, values)):
+                return True
+        else:
+            if raise_exception:
+                self.fail('Rule with %s was expected' % exception_string)
+            else:
+                return False
+
+
+class ListIpRulesTestCase(BaseIpRuleTestCase):
+
+    RULE_TABLES = {'default': 253, 'main': 254, 'local': 255}
+
     def test_list_default_rules_ipv4(self):
         rules_ipv4 = priv_ip_lib.list_ip_rules(self.namespace, 4)
         self.assertEqual(3, len(rules_ipv4))
@@ -291,28 +306,7 @@ def test_list_rules_ipv6(self):
             self.fail('Rule added (2001:db8::1/64, table 20) not found')
 
 
-class RuleTestCase(functional_base.BaseSudoTestCase):
-
-    def setUp(self):
-        super(RuleTestCase, self).setUp()
-        self.namespace = 'ns_test-' + uuidutils.generate_uuid()
-        self.ns = priv_ip_lib.create_netns(self.namespace)
-        self.addCleanup(self._remove_ns)
-
-    def _remove_ns(self):
-        priv_ip_lib.remove_netns(self.namespace)
-
-    def _check_rules(self, rules, parameters, values, exception_string=None,
-                     raise_exception=True):
-        for rule in rules:
-            if all(rule.get(parameter) == value
-                   for parameter, value in zip(parameters, values)):
-                return True
-        else:
-            if raise_exception:
-                self.fail('Rule with %s was expected' % exception_string)
-            else:
-                return False
+class AddIpRulesTestCase(BaseIpRuleTestCase):
 
     def test_add_rule_ip(self):
         ip_addresses = ['192.168.200.250', '2001::250']
@@ -433,6 +427,23 @@ def test_add_rule_exists(self):
         self.assertEqual(4, len(rules))
 
 
+class DeleteIpRulesTestCase(BaseIpRuleTestCase):
+
+    def test_delete_rule_no_entry(self):
+        iif = 'iif_device'
+        priv_ip_lib.create_interface(iif, self.namespace, 'dummy')
+
+        try:
+            # This should not raise for a non-existent entry
+            priv_ip_lib.delete_ip_rule(self.namespace, iifname=iif)
+        except Exception:
+            self.fail('Delete IP rule threw unexpected exception')
+
+        rules = ip_lib.list_ip_rules(self.namespace, 4)
+        # There are always 3 rules by default
+        self.assertEqual(3, len(rules))
+
+
 class GetIpAddressesTestCase(functional_base.BaseSudoTestCase):
 
     def _remove_ns(self, namespace):
@@ -659,6 +670,21 @@ def test_add_multipath_route(self):
                                  n_cons.IP_VERSION_4, via=multipath)
         self._check_routes(['192.168.0.0/24'], gateway=multipath)
 
+    def test_delete_route_no_entry(self):
+        cidr = '192.168.0.0/24'
+        self.device.addr.add('10.1.0.1/24')
+        try:
+            # This should not raise for a non-existent entry
+            priv_ip_lib.delete_ip_route(self.namespace, cidr,
+                                        n_cons.IP_VERSION_4,
+                                        device=self.device_name)
+        except Exception:
+            self.fail('Delete IP route threw unexpected exception')
+
+        routes = ip_lib.list_ip_routes(self.namespace, n_cons.IP_VERSION_4)
+        # There will be a single interface route since we added an IP
+        self.assertEqual(1, len(routes))
+
 
 class GetLinkAttributesTestCase(functional_base.BaseSudoTestCase):
 

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

@@ -157,6 +157,17 @@ def test_run_iproute_neigh_namespace_not_exists(self):
                 priv_lib._run_iproute_neigh,
                 "test_cmd", "eth0", None, test_param="test_value")
 
+    def test_run_iproute_neigh_no_entry(self):
+        with mock.patch.object(pyroute2, "IPRoute") as iproute_mock:
+            iproute_mock.side_effect = netlink_exceptions.NetlinkError(
+                code=errno.ENOENT)
+            try:
+                priv_lib._run_iproute_neigh(
+                    "test_cmd", "eth0", None, test_param="test_value")
+                self.fail("NetlinkError exception not raised")
+            except netlink_exceptions.NetlinkError as e:
+                self.assertEqual(errno.ENOENT, e.code)
+
     def test_run_iproute_neigh_error(self):
         with mock.patch.object(pyroute2, "IPRoute") as iproute_mock:
             iproute_mock.side_effect = OSError(