From 9bd97242b65b56ee89c422b56af05a330ad3c521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= Date: Tue, 18 Feb 2020 11:37:33 +0000 Subject: [PATCH] Clear lsp.addresses always if port is OVN LB VIP port. The check _is_virtual_port_supported() prevented us from clearing the addresses field while port was OVN LB VIP port. The virtual port should be set only when port is Octavia Amphorae VIP port. Change-Id: Id6dd29650951855d13498a7206f6d1dde7db7864 Closes-Bug: 1863893 --- .../ovn/mech_driver/ovsdb/ovn_client.py | 16 +++--------- .../ovn/mech_driver/test_mech_driver.py | 26 +++++-------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 309783a8a32..01e22427c0a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -370,14 +370,10 @@ class OVNClient(object): kwargs['ha_chassis_group'] = ( self._get_default_ha_chassis_group()) - # TODO(lucasgomes): Remove this workaround in the future, - # the core OVN version >= 2.12 supports the "virtual" port - # type which deals with these situations. # NOTE(mjozefcz): Do not set addresses if the port is not - # bound and has no device_owner - possibly it is a VirtualIP - # port used for Octavia (VRRP). + # bound, has no device_owner and it is OVN LB VIP port. # For more details check related bug #1789686. - if (not self._is_virtual_port_supported() and + if (port.get('name').startswith(ovn_const.LB_VIP_PORT_PREFIX) and not port.get('device_owner') and port.get(portbindings.VIF_TYPE) == portbindings.VIF_TYPE_UNBOUND): @@ -519,14 +515,10 @@ class OVNClient(object): else: dhcpv6_options = [port_info.dhcpv6_options['uuid']] - # TODO(lucasgomes): Remove this workaround in the future, - # the core OVN version >= 2.12 supports the "virtual" port - # type which deals with these situations. # NOTE(mjozefcz): Do not set addresses if the port is not - # bound and has no device_owner - possibly it is a VirtualIP - # port used for Octavia (VRRP). + # bound, has no device_owner and it is OVN LB VIP port. # For more details check related bug #1789686. - if (not self._is_virtual_port_supported() and + if (port.get('name').startswith(ovn_const.LB_VIP_PORT_PREFIX) and not port.get('device_owner') and port.get(portbindings.VIF_TYPE) == portbindings.VIF_TYPE_UNBOUND): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 2759a3c8162..a805fdccce3 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -495,16 +495,11 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): old_mac]), called_args_dict.get('addresses')) - def test_create_port_possible_vip(self): - """Test if just created LSP has no adresses set. - - This could be potential VIP port. If not - next - port update will set the adresses corectly during - binding process. - """ + def test_create_port_ovn_octavia_vip(self): with (self.network(set_context=True, tenant_id='test')) as net1, ( self.subnet(network=net1)) as subnet1, ( - self.port(subnet=subnet1, set_context=True, tenant_id='test')): + self.port(name=ovn_const.LB_VIP_PORT_PREFIX + 'foo', + subnet=subnet1, set_context=True, tenant_id='test')): self.assertTrue(self.nb_ovn.create_lswitch_port.called) called_args_dict = ( @@ -711,32 +706,25 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): def _test_update_port_vip(self, is_vip=True): kwargs = {} - if not is_vip: - # NOTE(mjozefcz): Lets pretend this is nova port to not - # be treated as VIP. - kwargs['device_owner'] = 'compute:nova' with ( self.network(set_context=True, tenant_id='test')) as net1, ( self.subnet(network=net1)) as subnet1, ( self.port(subnet=subnet1, set_context=True, tenant_id='test', **kwargs)) as port1: - fake_lsp = ( fakes.FakeOVNPort.from_neutron_port( port1['port'])) self.nb_ovn.lookup.return_value = fake_lsp - - # Update the port name. self.nb_ovn.set_lswitch_port.reset_mock() - data = {'port': {'name': 'rtheis'}} + if is_vip: + data = {'port': {'name': ovn_const.LB_VIP_PORT_PREFIX + 'foo'}} + else: + data = {'port': {}} self._update('ports', port1['port']['id'], data) self.assertEqual( 1, self.nb_ovn.set_lswitch_port.call_count) called_args_dict = ( self.nb_ovn.set_lswitch_port.call_args_list[0][1]) - self.assertEqual( - 'rtheis', - called_args_dict['external_ids']['neutron:port_name']) if is_vip: self.assertEqual([], called_args_dict.get('addresses'))