Browse Source

Do not set port addresses on LSP while port not bound

FIP that points to VIP port could not have addresses
specified [1]. Router pipeline will try to resolve
ARP requests internally from LSP instead looking for
actual MAC address from LSP where VIP exists at this moment.

Lets not set this address till the port is bound.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1707241

Change-Id: I36261701be393584ad4b00ced96273736339b03c
Closes-Bug: #1789686
changes/86/659286/3
Maciej Józefczyk 2 years ago
parent
commit
33fd553158
  1. 48
      networking_ovn/common/ovn_client.py
  2. 69
      networking_ovn/tests/unit/ml2/test_mech_driver.py

48
networking_ovn/common/ovn_client.py

@ -300,19 +300,33 @@ class OVNClient(object):
# The lport_name *must* be neutron port['id']. It must match the
# iface-id set in the Interfaces table of the Open_vSwitch
# database which nova sets to be the port ID.
kwargs = {
'lport_name': port['id'],
'lswitch_name': lswitch_name,
'addresses': port_info.addresses,
'external_ids': external_ids,
'parent_name': port_info.parent_name,
'tag': port_info.tag,
'enabled': port.get('admin_state_up'),
'options': port_info.options,
'type': port_info.type,
'port_security': port_info.port_security,
'dhcpv4_options': dhcpv4_options,
'dhcpv6_options': dhcpv6_options
}
# 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).
# For more details check related bug #1789686.
if (not port.get('device_owner') and
port.get(portbindings.VIF_TYPE) ==
portbindings.VIF_TYPE_UNBOUND):
kwargs['addresses'] = []
port_cmd = txn.add(self._nb_idl.create_lswitch_port(
lport_name=port['id'],
lswitch_name=lswitch_name,
addresses=port_info.addresses,
external_ids=external_ids,
parent_name=port_info.parent_name,
tag=port_info.tag,
enabled=port.get('admin_state_up'),
options=port_info.options,
type=port_info.type,
port_security=port_info.port_security,
dhcpv4_options=dhcpv4_options,
dhcpv6_options=dhcpv6_options))
**kwargs))
# Handle ACL's for this port. If we're not using Port Groups
# because either the schema doesn't support it or we didn't
@ -417,6 +431,16 @@ class OVNClient(object):
dhcpv6_options = txn.add(port_info.dhcpv6_options['cmd'])
else:
dhcpv6_options = [port_info.dhcpv6_options['uuid']]
# 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).
# For more details check related bug #1789686.
if (not port.get('device_owner') and
port.get(portbindings.VIF_TYPE) ==
portbindings.VIF_TYPE_UNBOUND):
columns_dict['addresses'] = []
# NOTE(lizk): Fail port updating if port doesn't exist. This
# prevents any new inserted resources to be orphan, such as port
# dhcp options or ACL rules for port, e.g. a port was created

69
networking_ovn/tests/unit/ml2/test_mech_driver.py

@ -393,7 +393,10 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
called_args_dict.get('port_security'))
def test_create_port_with_disabled_security(self):
kwargs = {'port_security_enabled': False}
# NOTE(mjozefcz): Lets pretend this is nova port to not
# be treated as VIP.
kwargs = {'port_security_enabled': False,
'device_owner': 'compute:nova'}
with self.network(set_context=True, tenant_id='test') as net1:
with self.subnet(network=net1) as subnet1:
with self.port(subnet=subnet1,
@ -440,10 +443,13 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
called_args_dict.get('addresses'))
def test_create_port_security_allowed_address_pairs(self):
# NOTE(mjozefcz): Lets pretend this is nova port to not
# be treated as VIP.
kwargs = {'allowed_address_pairs':
[{"ip_address": "1.1.1.1"},
{"ip_address": "2.2.2.2",
"mac_address": "22:22:22:22:22:22"}]}
"mac_address": "22:22:22:22:22:22"}],
'device_owner': 'compute:nova'}
with self.network(set_context=True, tenant_id='test') as net1:
with self.subnet(network=net1) as subnet1:
with self.port(subnet=subnet1,
@ -493,6 +499,24 @@ 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.
"""
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.assertTrue(self.nb_ovn.create_lswitch_port.called)
called_args_dict = (
self.nb_ovn.create_lswitch_port.call_args_list[0][1])
self.assertEqual([],
called_args_dict.get('addresses'))
def _create_fake_network_context(self,
network_type,
physical_network=None,
@ -672,6 +696,47 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
self.assertEqual(
1, self.nb_ovn.update_address_set.call_count)
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'}}
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'))
else:
self.assertNotEqual([],
called_args_dict.get('addresses'))
def test_update_port_not_vip_port(self):
self._test_update_port_vip(is_vip=False)
def test_update_port_vip_port(self):
self._test_update_port_vip()
def test_delete_port_without_security_groups(self):
kwargs = {'security_groups': []}
with self.network(set_context=True, tenant_id='test') as net1:

Loading…
Cancel
Save