diff --git a/releasenotes/notes/revert-always-plug-port-for-ovs-hybrid-plug-false-dc015985cbc5443b.yaml b/releasenotes/notes/revert-always-plug-port-for-ovs-hybrid-plug-false-dc015985cbc5443b.yaml new file mode 100644 index 00000000..ba4d7460 --- /dev/null +++ b/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. diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index ccd962aa..83fefcdb 100644 --- a/vif_plug_ovs/ovs.py +++ b/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): diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index 545198a0..4b7377a0 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/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')