openstack
/
networking-ovn
Code Issues Proposed changes

Merge "clean stale port dhcpv4 options within port op"

changes/11/370811/1 1.0.0.0rc1
Jenkins 2016-09-14 18:32:34 +00:00 committed by Gerrit Code Review
parent ac2100e8fa 022b99026b
commit 899c1ea256
6 changed files with 66 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
networking_ovn/common/utils.py
View File

@ -56,6 +56,8 @@ def ovn_addrset_name(sg_id, ip_version):
def get_lsp_dhcpv4_opts(port):
# Get dhcpv4 options from Neutron port, for setting DHCP_Options row
# in OVN.
lsp_dhcp_disabled = False
lsp_dhcpv4_opts = {}
if port['device_owner'].startswith(const.DEVICE_OWNER_PREFIXES):

52
networking_ovn/ml2/mech_driver.py
View File

@ -28,7 +28,6 @@ from neutron.callbacks import resources
from neutron.common import utils as n_utils
from neutron import context as n_context
from neutron.db import provisioning_blocks
from neutron.extensions import extra_dhcp_opt as edo_ext
from neutron.extensions import portbindings
from neutron.extensions import portsecurity as psec
from neutron.extensions import providernet as pnet
@ -780,26 +779,6 @@ class OVNMechanismDriver(driver_api.MechanismDriver):
addrs_add=addr_add,
addrs_remove=addr_remove))
cmd = self._get_clean_stale_port_dhcpv4_options_cmd(
ovn_port_info.dhcpv4_options, port, original_port)
if cmd:
txn.add(cmd)
def _get_clean_stale_port_dhcpv4_options_cmd(self, ovn_port_dhcpv4_opts,
port, original_port):
if (not original_port.get(edo_ext.EXTRADHCPOPTS)
or original_port['device_owner'].startswith(
const.DEVICE_OWNER_PREFIXES)):
return
if (port['device_owner'].startswith(const.DEVICE_OWNER_PREFIXES)
or not ovn_port_dhcpv4_opts
or not port.get(edo_ext.EXTRADHCPOPTS)):
# Extra DHCP options were define for this port. Delete the
# DHCP_Options row created for this port earlier if exists,
# since this port no longer refers it.
return self._get_delete_lsp_dhcpv4_options_cmd(original_port)
def _get_delete_lsp_dhcpv4_options_cmd(self, port):
lsp_dhcp_options = None
for fixed_ip in port['fixed_ips']:
@ -810,12 +789,12 @@ class OVNMechanismDriver(driver_api.MechanismDriver):
break
if lsp_dhcp_options:
# Delete the DHCP_Options row created for this port.
# A separate DHCP_Options row would have be created since the port
# has extra DHCP options defined.
# Extra DHCP options were defined for this port. Delete the
# DHCP_Options row created for this port earlier if exists,
# since this port no longer refers it.
return self._nb_ovn.delete_dhcp_options(lsp_dhcp_options['uuid'])
def get_port_dhcpv4_options(self, port, original_port=None):
def get_port_dhcpv4_options(self, port):
lsp_dhcp_disabled, lsp_dhcpv4_opts = utils.get_lsp_dhcpv4_opts(port)
if lsp_dhcp_disabled:
@ -844,10 +823,16 @@ class OVNMechanismDriver(driver_api.MechanismDriver):
# This port has extra DHCP options defined.
# So we need to create a new row in DHCP_Options table for this
# port.
#
# TODO(numans) In cases where the below transaction is successful
# but the Logical_Switch_Port create or update transaction fails
# we need to delete the DHCP_Options row created else it will be
# an orphan row.
#
# NOTE(lizk) In cases where the below transaction is successful, but
# the Logical_Switch_Port get deleted before setting port dhcp options
# to it, we will delete the DHCP_Options row created to make sure
# no orphan left behind.
subnet_dhcp_options['options'].update(lsp_dhcpv4_opts)
subnet_dhcp_options['external_ids'].update(
{'port_id': port['id']})
@ -891,17 +876,12 @@ class OVNMechanismDriver(driver_api.MechanismDriver):
addrs_add=None,
addrs_remove=addresses[ip_version]))
# Delete the DHCP_Options row if created for this port.
# A separate DHCP_Options row would have be created if the port
# has extra DHCP options defined.
for fixed_ip in port['fixed_ips']:
if netaddr.IPAddress(fixed_ip['ip_address']).version == 4:
lsp_dhcp_options = self._nb_ovn.get_port_dhcp_options(
fixed_ip['subnet_id'], port['id'])
if lsp_dhcp_options:
txn.add(self._nb_ovn.delete_dhcp_options(
lsp_dhcp_options['uuid']))
break
# NOTE(lizk): Always try to clean port dhcp options, to make sure
# no orphaned DHCP_Options row related to port left behind, which
# may be created in get_port_dhcpv4_options.
cmd = self._get_delete_lsp_dhcpv4_options_cmd(port)
if cmd:
txn.add(cmd)
def bind_port(self, context):
"""Attempt to bind a port.

22
networking_ovn/ovsdb/commands.py
View File

@ -74,6 +74,17 @@ def _updatevalues_in_list(row, column, new_values=None, old_values=None):
setattr(row, column, column_values)
def get_lsp_dhcpv4_options_uuids(lsp, lsp_name):
# Get dhcpv4_options uuids from Logical_Switch_Port, which are references
# of port dhcpv4 options in DHCP_Options table.
uuids = set()
for dhcp_opts in getattr(lsp, 'dhcpv4_options', []):
external_ids = getattr(dhcp_opts, 'external_ids', {})
if external_ids.get('port_id') == lsp_name:
uuids.add(dhcp_opts.uuid)
return uuids
class AddLSwitchCommand(commands.BaseCommand):
def __init__(self, api, name, may_exist, **columns):
super(AddLSwitchCommand, self).__init__(api)
@ -185,6 +196,17 @@ class SetLSwitchPortCommand(commands.BaseCommand):
msg = _("Logical Switch Port %s does not exist") % self.lport
raise RuntimeError(msg)
# Delete DHCP_Options records no longer refered by this port.
# The table rows should be consistent for the same transaction.
# After we get a DHCP_Options row uuid from port dhcpv4_options
# reference, the row shouldn't disappear for this transaction,
# before we delete it.
cur_port_dhcp_opts = get_lsp_dhcpv4_options_uuids(
port, self.lport)
new_port_dhcp_opts = set(self.columns.get('dhcpv4_options', []))
for uuid in cur_port_dhcp_opts - new_port_dhcp_opts:
self.api._tables['DHCP_Options'].rows[uuid].delete()
for col, val in self.columns.items():
setattr(port, col, val)

21
networking_ovn/tests/functional/test_ovn_db_resources.py
View File

@ -15,6 +15,7 @@
import mock
from networking_ovn.ovsdb import commands as cmd
from networking_ovn.tests.functional import base
from neutron.agent.ovsdb.native import idlutils
from neutron.common import utils as n_utils
@ -411,3 +412,23 @@ class TestNBDbResources(base.TestOVNFunctionalBase):
# The Logical_Switch_Port.dhcpv4_options for this port should be
# empty.
self._verify_dhcp_option_row_for_port(p1['port']['id'], {})
# Add orphaned DHCP_Options row for this port.
with self.nb_idl_transaction(self.fake_api, check_error=True) as txn:
txn.add(cmd.AddDHCPOptionsCommand(
self.fake_api, subnet['subnet']['id'],
port_id=p1['port']['id'],
cidr='10.0.0.0/24',
options={'server_id': '10.0.0.1',
'server_mac': '01:02:03:04:05:06',
'lease_time': str(12 * 60 * 60),
'mtu': '1200',
'router': subnet['subnet']['gateway_ip'],
'tftp_server': '8.8.8.8'},
external_ids={'subnet_id': subnet['subnet']['id'],
'port_id': p1['port']['id']}))
# Port deletion will have a final checking to make sure no orphaned
# DHCP_Options related to port left behind.
port_req = self.new_delete_request('ports', p1['port']['id'])
port_req.get_response(self.api)
self._verify_dhcp_option_rows(expected_dhcp_options_rows)

7
networking_ovn/tests/functional/test_ovn_db_sync.py
View File

@ -393,7 +393,10 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
for dhcp_opts in self.dirty_dhcpv4_options:
txn.add(cmd.AddDHCPOptionsCommand(
fake_api, dhcp_opts['subnet_id'],
port_id=dhcp_opts.get('port_id'), options={'foo': 'bar'}))
port_id=dhcp_opts.get('port_id'),
external_ids={'subnet_id': dhcp_opts['subnet_id'],
'port_id': dhcp_opts.get('port_id')},
options={'foo': 'bar'}))
for port_id in self.lport_dhcpv4_disabled:
txn.add(cmd.SetLSwitchPortCommand(
@ -405,7 +408,7 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
if dhcp_opts['port_id'] in self.orphaned_lport_dhcpv4_options:
continue
uuid = self.mech_driver._nb_ovn.get_port_dhcp_options(
dhcp_opts['subnet_id'], dhcp_opts['port_id'])
dhcp_opts['subnet_id'], dhcp_opts['port_id'])['uuid']
txn.add(cmd.SetLSwitchPortCommand(fake_api, lport_name, True,
dhcpv4_options=[uuid]))

82
networking_ovn/tests/unit/ml2/test_mech_driver.py
View File

@ -935,85 +935,3 @@ class TestOVNMechansimDriverDHCPOptions(OVNMechanismDriverTestCase):
self.assertIsNone(
self.mech_driver._get_delete_lsp_dhcpv4_options_cmd(port))
self.mech_driver._nb_ovn.delete_dhcp_options.assert_not_called()
def test__get_clean_stale_dhcpv4_options_cmd_owner_changed(self):
port = {'device_owner': 'neutron:router_interface'}
original_port = {
'device_owner': 'compute:None',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'mtu',
'opt_value': '1200'}]}
fake_cmd = 'foo'
with mock.patch.object(self.mech_driver,
'_get_delete_lsp_dhcpv4_options_cmd',
return_value=fake_cmd):
self.assertEqual(
fake_cmd,
self.mech_driver._get_clean_stale_port_dhcpv4_options_cmd(
mock.ANY, port, original_port))
def test__get_clean_stale_dhcpv4_options_dhcp_disabled(self):
port = {
'device_owner': 'compute:None',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'dhcp_disabled',
'opt_value': 'True'}]}
original_port = {
'device_owner': 'compute:None',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'mtu',
'opt_value': '1200'}]}
fake_cmd = 'foo'
with mock.patch.object(self.mech_driver,
'_get_delete_lsp_dhcpv4_options_cmd',
return_value=fake_cmd):
self.assertEqual(
fake_cmd,
self.mech_driver._get_clean_stale_port_dhcpv4_options_cmd(
None, port, original_port))
def test__get_clean_stale_dhcpv4_options_extra_dhcp_opts_removed(self):
port = {'device_owner': 'compute:None'}
original_port = {
'device_owner': 'compute:None',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'mtu',
'opt_value': '1200'}]}
fake_cmd = 'foo'
with mock.patch.object(self.mech_driver,
'_get_delete_lsp_dhcpv4_options_cmd',
return_value=fake_cmd):
self.assertEqual(
fake_cmd,
self.mech_driver._get_clean_stale_port_dhcpv4_options_cmd(
mock.ANY, port, original_port))
def test__get_clean_stale_dhcpv4_options_cmd_negative1(self):
# Test case no extra dhcp options assigned to port before.
original_port = {'device_owner': 'compute:None'}
self.assertIsNone(
self.mech_driver._get_clean_stale_port_dhcpv4_options_cmd(
mock.ANY, mock.ANY, original_port))
def test__get_clean_stale_dhcpv4_options_cmd_negative2(self):
# Test case owner changed from neutron managed port type.
original_port = {
'device_owner': 'neutron:XX',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'mtu',
'opt_value': '1200'}]}
self.assertIsNone(
self.mech_driver._get_clean_stale_port_dhcpv4_options_cmd(
mock.ANY, mock.ANY, original_port))
def test__get_clean_stale_dhcpv4_options_cmd_negative3(self):
# Test case extra dhcp options updated.
original_port = {
'device_owner': 'compute:None',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'mtu',
'opt_value': '1200'}]}
port = {
'device_owner': 'compute:None',
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'mtu',
'opt_value': '1250'}]}
self.assertIsNone(
self.mech_driver._get_clean_stale_port_dhcpv4_options_cmd(
mock.ANY, port, original_port))