From 0a0dec37e42e220ffa750650027714bd7289faa1 Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Fri, 2 Jun 2023 11:52:59 +0000 Subject: [PATCH] Don't break traffic if port already exists Nova compute service plugs VIF on service init, it triggers rules reinitialization for iptables filter. But it breaks traffic in case isolate_vif is used by assigning dead VLAN to the port. This patch prevents dead VLAN assingment for ports that exists. Co-Authored-By: aarefiev@mirantis.com Closes-Bug: #2023300 Change-Id: Ib1b7467fce9facfbfcd698bf6e9f950c0cead650 --- ...art-with-isolate_vif-71617a41741b33e8.yaml | 7 ++++ vif_plug_ovs/ovs.py | 15 +++++-- vif_plug_ovs/tests/unit/test_plugin.py | 39 ++++++++++++++++++- 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/fix-broken-dataplane-on-nova-restart-with-isolate_vif-71617a41741b33e8.yaml diff --git a/releasenotes/notes/fix-broken-dataplane-on-nova-restart-with-isolate_vif-71617a41741b33e8.yaml b/releasenotes/notes/fix-broken-dataplane-on-nova-restart-with-isolate_vif-71617a41741b33e8.yaml new file mode 100644 index 00000000..fa9485be --- /dev/null +++ b/releasenotes/notes/fix-broken-dataplane-on-nova-restart-with-isolate_vif-71617a41741b33e8.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + An issue when after nova-compute service restart configured with + `isolate_vif=True` instances lost connectivity unless neutron + rebinds ports. Which is normally happening by agent restart or port + update. diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 1c43a695..4a719ea7 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -186,6 +186,13 @@ class OvsPlugin(plugin.PluginBase): return True + def _isolate_vif(self, vif_name, bridge): + # NOTE(vsaienko): don't break traffic if port already exists, + # we assume it is called when nova-compute is initialized and + # since port is present it should be bound already. + return (self.config.isolate_vif and + not self.ovsdb.port_exists(vif_name, bridge)) + def _create_vif_port(self, vif, vif_name, instance_info, **kwargs): mtu = self._get_mtu(vif) # NOTE(sean-k-mooney): As part of a partial fix to bug #1734320 @@ -200,7 +207,8 @@ class OvsPlugin(plugin.PluginBase): # TODO(sean-k-mooney): Extend neutron to record what ml2 driver # bound the interface in the vif binding details so isolation # can be enabled automatically in the future. - if self.config.isolate_vif: + bridge = kwargs.pop('bridge', vif.network.bridge) + if self._isolate_vif(vif_name, bridge): kwargs['tag'] = constants.DEAD_VLAN qos_type = self._get_qos_type(vif) if qos_type is not None: @@ -215,7 +223,6 @@ class OvsPlugin(plugin.PluginBase): # for more details. if not self.ovsdb.port_exists(vif_name, vif.network.bridge): kwargs['qos_type'] = qos_type - bridge = kwargs.pop('bridge', vif.network.bridge) self.ovsdb.create_ovs_vif_port( bridge, vif_name, @@ -302,7 +309,9 @@ class OvsPlugin(plugin.PluginBase): vif, vif.vif_name, instance_info, bridge=port_bridge_name, set_ids=False ) - tag = constants.DEAD_VLAN if self.config.isolate_vif else None + tag = (constants.DEAD_VLAN + if self._isolate_vif(int_bridge_patch, int_bridge_name) + else None) iface_id = vif.id mac = vif.address instance_id = instance_info.uuid diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index 968c67d2..83cc9c3c 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/vif_plug_ovs/tests/unit/test_plugin.py @@ -187,8 +187,28 @@ class PluginTest(testtools.TestCase): interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) @mock.patch.object(ovsdb_lib.BaseOVS, 'create_ovs_vif_port') - def test_create_vif_port_isolate(self, mock_create_ovs_vif_port): + @mock.patch.object(ovsdb_lib.BaseOVS, 'port_exists') + def test_create_vif_port_isolate_port_no_isolate_vif_no_port( + self, mock_port_exists, mock_create_ovs_vif_port): plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + mock_port_exists.return_value = False + with mock.patch.object(plugin.config, 'isolate_vif', False): + plugin._create_vif_port( + self.vif_ovs, mock.sentinel.vif_name, self.instance, + interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) + mock_create_ovs_vif_port.assert_called_once_with( + self.vif_ovs.network.bridge, mock.sentinel.vif_name, + self.vif_ovs.port_profile.interface_id, + self.vif_ovs.address, self.instance.uuid, + mtu=plugin.config.network_device_mtu, + interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) + + @mock.patch.object(ovsdb_lib.BaseOVS, 'create_ovs_vif_port') + @mock.patch.object(ovsdb_lib.BaseOVS, 'port_exists') + def test_create_vif_port_isolate_port_isolate_vif_no_port( + self, mock_port_exists, mock_create_ovs_vif_port): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + mock_port_exists.return_value = False with mock.patch.object(plugin.config, 'isolate_vif', True): plugin._create_vif_port( self.vif_ovs, mock.sentinel.vif_name, self.instance, @@ -201,6 +221,23 @@ class PluginTest(testtools.TestCase): interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE, tag=constants.DEAD_VLAN) + @mock.patch.object(ovsdb_lib.BaseOVS, 'create_ovs_vif_port') + @mock.patch.object(ovsdb_lib.BaseOVS, 'port_exists') + def test_create_vif_port_isolate_port_isolate_vif_port_exists( + self, mock_port_exists, mock_create_ovs_vif_port): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + mock_port_exists.return_value = True + with mock.patch.object(plugin.config, 'isolate_vif', True): + plugin._create_vif_port( + self.vif_ovs, mock.sentinel.vif_name, self.instance, + interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) + mock_create_ovs_vif_port.assert_called_once_with( + self.vif_ovs.network.bridge, mock.sentinel.vif_name, + self.vif_ovs.port_profile.interface_id, + self.vif_ovs.address, self.instance.uuid, + mtu=plugin.config.network_device_mtu, + interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) + @mock.patch.object(ovs, 'sys') @mock.patch.object(ovs.OvsPlugin, '_plug_vif_generic') def test_plug_ovs_port_bridge_false(self, plug_vif_generic, mock_sys):