From 9ace551db2262d1134ad984ae8a10436a02cdac6 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Wed, 20 Apr 2022 19:19:44 +0000 Subject: [PATCH] Check for hybrid plugging in OVS There is a cold migration scenario that leaves some interfaces behind in case port binding changes from hybrid to direct plugging between the nodes. This patch adds functionality that checks presence of intermediate linux bridge and clears things up properly if found. Signed-off-by: Jakub Libosvar Change-Id: Ic5b38a0467b3c18e38bec005d80cd1f5f0e66b28 --- .../OVSVif-hybrid-unplug-f37bf57bc8e913de.yaml | 10 ++++++++++ vif_plug_ovs/ovs.py | 12 ++++++++---- vif_plug_ovs/tests/unit/test_plugin.py | 14 ++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/OVSVif-hybrid-unplug-f37bf57bc8e913de.yaml diff --git a/releasenotes/notes/OVSVif-hybrid-unplug-f37bf57bc8e913de.yaml b/releasenotes/notes/OVSVif-hybrid-unplug-f37bf57bc8e913de.yaml new file mode 100644 index 00000000..11545a07 --- /dev/null +++ b/releasenotes/notes/OVSVif-hybrid-unplug-f37bf57bc8e913de.yaml @@ -0,0 +1,10 @@ +--- + +feature: + - | + The unplug() method for VIFOpenVSwitch type now checks for existence of + linux bridge used in hybrid plugging mechanism. In case it exists, the + interfaces related to the hybrid plugging are deleted too. This is useful + in particular for cold migration use case when target node has updated + port binding that doesn't use hybrid plugging, while the original plugging + was hybrid. diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 3d459635..67e37c6c 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -363,7 +363,7 @@ class OvsPlugin(plugin.PluginBase): constants.OVS_VHOSTUSER_PREFIX, vif.id)) - def _unplug_bridge(self, vif, instance_info): + def _unplug_bridge(self, vif, instance_info, linux_bridge_name): """UnPlug using hybrid strategy Unhook port from OVS, unhook port from bridge, delete @@ -372,7 +372,7 @@ class OvsPlugin(plugin.PluginBase): v1_name, v2_name = self.get_veth_pair_names(vif) - linux_net.delete_bridge(vif.bridge_name, v1_name) + linux_net.delete_bridge(linux_bridge_name, v1_name) self.ovsdb.delete_ovs_vif_port(vif.network.bridge, v2_name) @@ -439,9 +439,13 @@ class OvsPlugin(plugin.PluginBase): if self.config.per_port_bridge: self._unplug_port_bridge(vif, instance_info) else: - self._unplug_vif_generic(vif, instance_info) + linux_bridge_name = self.gen_port_name('qbr', vif.id) + if ip_lib.exists(linux_bridge_name): + self._unplug_bridge(vif, instance_info, linux_bridge_name) + else: + self._unplug_vif_generic(vif, instance_info) elif isinstance(vif, objects.vif.VIFBridge): - self._unplug_bridge(vif, instance_info) + self._unplug_bridge(vif, instance_info, vif.bridge_name) elif isinstance(vif, objects.vif.VIFVHostUser): self._unplug_vhostuser(vif, instance_info) elif isinstance(vif, objects.vif.VIFHostDevice): diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index a0576f7e..15006348 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/vif_plug_ovs/tests/unit/test_plugin.py @@ -588,3 +588,17 @@ class PluginTest(testtools.TestCase): ] delete_ovs_vif_port.assert_has_calls(calls) delete_ovs_bridge.assert_called_once_with('pbb679325f-ca8') + + @mock.patch.object(ip_lib, 'exists', return_value=True) + @mock.patch.object(ovs.OvsPlugin, '_unplug_bridge') + def test_unplug_hybrid_bridge(self, m_unplug_bridge, m_ip_lib_exists): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + plugin.unplug(self.vif_ovs, self.instance) + m_unplug_bridge.assert_called_once() + + @mock.patch.object(ip_lib, 'exists', return_value=False) + @mock.patch.object(ovs.OvsPlugin, '_unplug_vif_generic') + def test_unplug_ovs(self, m_unplug_generic, m_ip_lib_exists): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + plugin.unplug(self.vif_ovs, self.instance) + m_unplug_generic.assert_called_once()