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

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.

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

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):