do not always plug ovs ports.

As part of resolving bug #1734320 the os-vif ovs plugin
was modified to always plug the vif to ovs in all code paths.
This change reverts that behavior as it has been determined
that libvirt will recreate the port which introduces new
issues for neutron and breaks the intent of the os-vif change.

Change-Id: I76a2de2e8077d8a931af0056690dad4a569e228a
Related-Bug: #1728600
Related-Bug: #1808171
Related-Bug: #1811405

releasenotes/notes/revert-always-plug-port-for-ovs-hybrid-plug-false-dc015985cbc5443b.yaml

@@ -0,0 +1,17 @@
+---
+security:
+  - |
+    In 1.13.0 it was reported that bug #1734320 was partially resolved by
+    change Iaf15fa7a678ec2624f7c12f634269c465fbad930. It has since emerged
+    that that change introduced another bug due to an interaction with
+    libvirt. It was understood that libvirt would not recreate the ovs port
+    if it was present on the ovs bridge when spawning a vm however on
+    inspection of the libvirt code this is not the case. In this release
+    we have reverted the change to os-vif and libvirt will be the only
+    entity to create the ovs port when vif_type is set to ovs and
+    hybrid_plug is set to false in the neutron port binding details.
+    Bug #1734320 is not expected to be present if hybrid_plug=true or
+    vif_type vhost-user is used on linux. On windows if hybrid_plug is false
+    on bug #1734320 is also not expected to be present. A new mitigation to
+    bug #1734320 will be developed for the remaining case of
+    hybrid_plug=false on linux.

vif_plug_ovs/ovs.py

@@ -68,7 +68,7 @@ class OvsPlugin(plugin.PluginBase):
                    choices=list(ovsdb_api.interface_map),
                    default='vsctl',
                    help='The interface for interacting with the OVSDB'),
-        # Note(sean-k-mooney): This value is a bool for two reasons.
+        # NOTE(sean-k-mooney): This value is a bool for two reasons.
         # First I want to allow this config option to be reusable with
         # non ml2/ovs deployment in the future if required, as such I do not
         # want to encode how the isolation is done in the config option.
@@ -145,7 +145,7 @@ class OvsPlugin(plugin.PluginBase):
 
     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
+        # NOTE(sean-k-mooney): As part of a partial fix to bug #1734320
         # we introduced the isolate_vif config option to enable isolation
         # of the vif prior to neutron wiring up the interface. To do
         # this we take advantage of the fact the ml2/ovs uses the
@@ -232,7 +232,20 @@ class OvsPlugin(plugin.PluginBase):
         """Create a per-VIF OVS port."""
         self.ovsdb.ensure_ovs_bridge(vif.network.bridge,
                                      self._get_vif_datapath_type(vif))
-        self._create_vif_port(vif, vif.vif_name, instance_info)
+        # NOTE(sean-k-mooney): as part of a partial revert of
+        # change Iaf15fa7a678ec2624f7c12f634269c465fbad930
+        # (always create ovs port during plug), we stopped calling
+        # self._create_vif_port(vif, vif.vif_name, instance_info).
+        # Calling _create_vif_port here was intended to ensure
+        # that the vif was wired up by neutron before the vm was
+        # spawned on boot or live migration to partially resolve
+        # #1734320. When the "always create ovs port during plug" change
+        # was written it was understood by me that libvirt would not
+        # modify ovs if the port exists but in fact it deletes and
+        # recreates the port. This both undoes the effort to resolve
+        # bug #1734320 and intoduces other issues for neutron.
+        # this comment will be removed when we actully fix #1734320 in
+        # all cases.
 
     def _plug_vf_passthrough(self, vif, instance_info):
         self.ovsdb.ensure_ovs_bridge(
@@ -294,6 +307,9 @@ class OvsPlugin(plugin.PluginBase):
 
     def _unplug_vif_generic(self, vif, instance_info):
         """Remove port from OVS."""
+        # NOTE(sean-k-mooney): even with the partial revert of change
+        # Iaf15fa7a678ec2624f7c12f634269c465fbad930 this should be correct
+        # so this is not removed.
         self.ovsdb.delete_ovs_vif_port(vif.network.bridge, vif.vif_name)
 
     def _unplug_vf_passthrough(self, vif, instance_info):

vif_plug_ovs/tests/unit/test_plugin.py

@@ -176,8 +176,9 @@ class PluginTest(testtools.TestCase):
         plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME)
         plugin._plug_vif_generic(self.vif_ovs, self.instance)
         ensure_bridge.assert_called_once()
-        create_port.assert_called_once_with(self.vif_ovs,
-            self.vif_ovs.vif_name, self.instance)
+        # NOTE(sean-k-mooney): the interface will be plugged
+        # by libvirt so we assert _create_vif_port is not called.
+        create_port.assert_not_called()
 
     @mock.patch.object(linux_net, 'set_interface_state')
     @mock.patch.object(ovsdb_lib.BaseOVS, 'ensure_ovs_bridge')