Make delete-vlan-bridge and delete-vlan functions clear

Change delete_vlan_bridge function name to reflect that it doesn't
only work with vlans but also for example with vxlan, the same with
delete_vlan function.

This refactor work is needed to make the code clear.

Change-Id: I76f5f91aa9437aa4478e2876c9bb3dfd75e2ca63
Closes-Bug: #1488298

diff --git a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
index 41503b00e8f0f0aa913998d226b8e573ec7a786b..49eea73119b79b3b6524a6528aa63b1fe3cde2c0 100644 (file)
--- a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
@@ -460,14 +460,14 @@ class LinuxBridgeManager(object):
                                       physical_network, segmentation_id,
                                       tap_device_name)
 
-    def delete_vlan_bridge(self, bridge_name):
+    def delete_bridge(self, bridge_name):
         if ip_lib.device_exists(bridge_name):
             interfaces_on_bridge = self.get_interfaces_on_bridge(bridge_name)
             for interface in interfaces_on_bridge:
                 self.remove_interface(bridge_name, interface)
 
                 if interface.startswith(VXLAN_INTERFACE_PREFIX):
-                    self.delete_vxlan(interface)
+                    self.delete_interface(interface)
                     continue
 
                 for physical_interface in self.interface_mappings.values():
@@ -481,7 +481,7 @@ class LinuxBridgeManager(object):
                                                              bridge_name,
                                                              ips, gateway)
                         elif physical_interface != interface:
-                            self.delete_vlan(interface)
+                            self.delete_interface(interface)
 
             LOG.debug("Deleting bridge %s", bridge_name)
             bridge_device = bridge_lib.BridgeDevice(bridge_name)
@@ -499,7 +499,7 @@ class LinuxBridgeManager(object):
         for network_id in list(self.network_map.keys()):
             bridge_name = self.get_bridge_name(network_id)
             if not self.get_tap_devices_count(bridge_name):
-                self.delete_vlan_bridge(bridge_name)
+                self.delete_bridge(bridge_name)
                 del self.network_map[network_id]
 
     def remove_interface(self, bridge_name, interface_name):
@@ -524,25 +524,14 @@ class LinuxBridgeManager(object):
                        'bridge_name': bridge_name})
             return False
 
-    def delete_vlan(self, interface):
+    def delete_interface(self, interface):
         if ip_lib.device_exists(interface):
-            LOG.debug("Deleting subinterface %s for vlan", interface)
-            if utils.execute(['ip', 'link', 'set', interface, 'down'],
-                             run_as_root=True):
-                return
-            if utils.execute(['ip', 'link', 'delete', interface],
-                             run_as_root=True):
-                return
-            LOG.debug("Done deleting subinterface %s", interface)
-
-    def delete_vxlan(self, interface):
-        if ip_lib.device_exists(interface):
-            LOG.debug("Deleting vxlan interface %s for vlan",
+            LOG.debug("Deleting interface %s",
                       interface)
-            int_vxlan = self.ip.device(interface)
-            int_vxlan.link.set_down()
-            int_vxlan.link.delete()
-            LOG.debug("Done deleting vxlan interface %s", interface)
+            device = self.ip.device(interface)
+            device.link.set_down()
+            device.link.delete()
+            LOG.debug("Done deleting interface %s", interface)
 
     def get_tap_devices(self):
         devices = set()
@@ -583,7 +572,7 @@ class LinuxBridgeManager(object):
         except RuntimeError:
             return False
         finally:
-            self.delete_vxlan(test_iface)
+            self.delete_interface(test_iface)
 
     def vxlan_mcast_supported(self):
         if not cfg.CONF.VXLAN.vxlan_group:
@@ -693,7 +682,7 @@ class LinuxBridgeRpcCallbacks(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         network_id = kwargs.get('network_id')
         bridge_name = self.agent.br_mgr.get_bridge_name(network_id)
         LOG.debug("Delete %s", bridge_name)
-        self.agent.br_mgr.delete_vlan_bridge(bridge_name)
+        self.agent.br_mgr.delete_bridge(bridge_name)
 
     def port_update(self, context, **kwargs):
         port_id = kwargs['port']['id']

diff --git a/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
index 9f80a53af214cc61a444b238c5459c15272cc5e5..a48b513d9349a32775682806c485ec186f1db7e1 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
@@ -716,7 +716,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
             add_tap.assert_called_with("123", p_const.TYPE_VLAN, "physnet-1",
                                        "1", "tap234")
 
-    def test_delete_vlan_bridge(self):
+    def test_delete_bridge(self):
         bridge_device = mock.Mock()
         with mock.patch.object(ip_lib, "device_exists") as de_fn,\
                 mock.patch.object(self.lbm,
@@ -726,22 +726,22 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                                   "get_interface_details") as if_det_fn,\
                 mock.patch.object(self.lbm,
                                   "update_interface_ip_details") as updif_fn,\
-                mock.patch.object(self.lbm, "delete_vxlan") as del_vxlan,\
+                mock.patch.object(self.lbm, "delete_interface") as del_interface,\
                 mock.patch.object(bridge_lib, "BridgeDevice",
                                   return_value=bridge_device):
             de_fn.return_value = False
-            self.lbm.delete_vlan_bridge("br0")
+            self.lbm.delete_bridge("br0")
             self.assertFalse(getif_fn.called)
 
             de_fn.return_value = True
             getif_fn.return_value = ["eth0", "eth1", "vxlan-1002"]
             if_det_fn.return_value = ("ips", "gateway")
             bridge_device.link.set_down.return_value = False
-            self.lbm.delete_vlan_bridge("br0")
+            self.lbm.delete_bridge("br0")
             updif_fn.assert_called_with("eth1", "br0", "ips", "gateway")
-            del_vxlan.assert_called_with("vxlan-1002")
+            del_interface.assert_called_with("vxlan-1002")
 
-    def test_delete_vlan_bridge_with_ip(self):
+    def test_delete_bridge_with_ip(self):
         bridge_device = mock.Mock()
         with mock.patch.object(ip_lib, "device_exists") as de_fn,\
                 mock.patch.object(self.lbm,
@@ -751,18 +751,18 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                                   "get_interface_details") as if_det_fn,\
                 mock.patch.object(self.lbm,
                                   "update_interface_ip_details") as updif_fn,\
-                mock.patch.object(self.lbm, "delete_vlan") as del_vlan,\
+                mock.patch.object(self.lbm, "delete_interface") as del_interface,\
                 mock.patch.object(bridge_lib, "BridgeDevice",
                                   return_value=bridge_device):
             de_fn.return_value = True
             getif_fn.return_value = ["eth0", "eth1.1"]
             if_det_fn.return_value = ("ips", "gateway")
             bridge_device.link.set_down.return_value = False
-            self.lbm.delete_vlan_bridge("br0")
+            self.lbm.delete_bridge("br0")
             updif_fn.assert_called_with("eth1.1", "br0", "ips", "gateway")
-            self.assertFalse(del_vlan.called)
+            self.assertFalse(del_interface.called)
 
-    def test_delete_vlan_bridge_no_ip(self):
+    def test_delete_bridge_no_ip(self):
         bridge_device = mock.Mock()
         with mock.patch.object(ip_lib, "device_exists") as de_fn,\
                 mock.patch.object(self.lbm,
@@ -772,18 +772,18 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                                   "get_interface_details") as if_det_fn,\
                 mock.patch.object(self.lbm,
                                   "update_interface_ip_details") as updif_fn,\
-                mock.patch.object(self.lbm, "delete_vlan") as del_vlan,\
+                mock.patch.object(self.lbm, "delete_interface") as del_interface,\
                 mock.patch.object(bridge_lib, "BridgeDevice",
                                   return_value=bridge_device):
             de_fn.return_value = True
             getif_fn.return_value = ["eth0", "eth1.1"]
             bridge_device.link.set_down.return_value = False
             if_det_fn.return_value = ([], None)
-            self.lbm.delete_vlan_bridge("br0")
-            del_vlan.assert_called_with("eth1.1")
+            self.lbm.delete_bridge("br0")
+            del_interface.assert_called_with("eth1.1")
             self.assertFalse(updif_fn.called)
 
-    def test_delete_vxlan_bridge_no_int_mappings(self):
+    def test_delete_bridge_no_int_mappings(self):
         interface_mappings = {}
         with mock.patch.object(ip_lib.IPWrapper,
                                'get_device_by_ip', return_value=None):
@@ -795,18 +795,18 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                 mock.patch.object(lbm,
                                   "get_interfaces_on_bridge") as getif_fn,\
                 mock.patch.object(lbm, "remove_interface"),\
-                mock.patch.object(lbm, "delete_vxlan") as del_vxlan,\
+                mock.patch.object(lbm, "delete_interface") as del_interface,\
                 mock.patch.object(bridge_lib, "BridgeDevice",
                                   return_value=bridge_device):
             de_fn.return_value = False
-            lbm.delete_vlan_bridge("br0")
+            lbm.delete_bridge("br0")
             self.assertFalse(getif_fn.called)
 
             de_fn.return_value = True
             getif_fn.return_value = ["vxlan-1002"]
             bridge_device.link.set_down.return_value = False
-            lbm.delete_vlan_bridge("br0")
-            del_vxlan.assert_called_with("vxlan-1002")
+            lbm.delete_bridge("br0")
+            del_interface.assert_called_with("vxlan-1002")
 
     def test_remove_empty_bridges(self):
         self.lbm.network_map = {'net1': mock.Mock(), 'net2': mock.Mock()}
@@ -814,7 +814,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
         def tap_count_side_effect(*args):
             return 0 if args[0] == 'brqnet1' else 1
 
-        with mock.patch.object(self.lbm, "delete_vlan_bridge") as del_br_fn,\
+        with mock.patch.object(self.lbm, "delete_bridge") as del_br_fn,\
                 mock.patch.object(self.lbm,
                                   "get_tap_devices_count",
                                   side_effect=tap_count_side_effect):
@@ -843,17 +843,19 @@ class TestLinuxBridgeManager(base.BaseTestCase):
             bridge_device.delif.return_value = False
             self.assertTrue(self.lbm.remove_interface("br0", "eth0"))
 
-    def test_delete_vlan(self):
+    def test_delete_interface(self):
         with mock.patch.object(ip_lib, "device_exists") as de_fn,\
-                mock.patch.object(utils, "execute") as exec_fn:
+                mock.patch.object(ip_lib.IpLinkCommand, "set_down") as down_fn,\
+                mock.patch.object(ip_lib.IpLinkCommand, "delete") as delete_fn:
             de_fn.return_value = False
-            self.lbm.delete_vlan("eth1.1")
-            self.assertFalse(exec_fn.called)
+            self.lbm.delete_interface("eth1.1")
+            self.assertFalse(down_fn.called)
+            self.assertFalse(delete_fn.called)
 
             de_fn.return_value = True
-            exec_fn.return_value = True
-            self.lbm.delete_vlan("eth1.1")
-            self.assertTrue(exec_fn.called)
+            self.lbm.delete_interface("eth1.1")
+            self.assertTrue(down_fn.called)
+            self.assertTrue(delete_fn.called)
 
     def _check_vxlan_support(self, expected, vxlan_ucast_supported,
                              vxlan_mcast_supported):
@@ -892,7 +894,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
         with mock.patch.object(ip_lib, 'device_exists', return_value=False),\
                 mock.patch.object(ip_lib, 'vxlan_in_use', return_value=False),\
                 mock.patch.object(self.lbm,
-                                  'delete_vxlan',
+                                  'delete_interface',
                                   return_value=None),\
                 mock.patch.object(self.lbm,
                                   'ensure_vxlan',
@@ -975,7 +977,7 @@ class TestLinuxBridgeRpcCallbacks(base.BaseTestCase):
         with mock.patch.object(self.lb_rpc.agent.br_mgr,
                                "get_bridge_name") as get_br_fn,\
                 mock.patch.object(self.lb_rpc.agent.br_mgr,
-                                  "delete_vlan_bridge") as del_fn:
+                                  "delete_bridge") as del_fn:
             get_br_fn.return_value = "br0"
             self.lb_rpc.network_delete("anycontext", network_id="123")
             get_br_fn.assert_called_with("123")