Browse Source

[OVN] Extra DHCP options validation: Log invalid options

By raising an exception when an option is invalid we broke the Ironic +
OVN + Neutron DHCP agent combination that enabled deploying baremetal
machines.

This patch is changing the approach to just log the invalid options
instead of failing the request so that the OVN driver can still be used
with Ironic.

Change-Id: I5e98297acefb62f9a9c1200ccfaac0672eeeed2c
Closes-Bug: #1888649
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
(cherry picked from commit 8c4b23f812)
changes/60/743160/1
Lucas Alvares Gomes 2 weeks ago
parent
commit
d07e879208
2 changed files with 35 additions and 25 deletions
  1. +3
    -5
      neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
  2. +32
    -20
      neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

+ 3
- 5
neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py View File

@@ -466,12 +466,10 @@ class OVNMechanismDriver(api.MechanismDriver):
return
ipv4_opts = ', '.join(result.invalid_ipv4)
ipv6_opts = ', '.join(result.invalid_ipv6)
msg = (_('The following extra DHCP options for port %(port_id)s '
LOG.info('The following extra DHCP options for port %(port_id)s '
'are not supported by OVN. IPv4: "%(ipv4_opts)s" and '
'IPv6: "%(ipv6_opts)s"') %
{'port_id': port['id'], 'ipv4_opts': ipv4_opts,
'ipv6_opts': ipv6_opts})
raise OVNPortUpdateError(resource='port', msg=msg)
'IPv6: "%(ipv6_opts)s"', {'port_id': port['id'],
'ipv4_opts': ipv4_opts, 'ipv6_opts': ipv6_opts})

def create_port_precommit(self, context):
"""Allocate resources for a new port.


+ 32
- 20
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py View File

@@ -231,16 +231,22 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
self.assertIsNone(
self.mech_driver._validate_port_extra_dhcp_opts(port))

def test__validate_port_extra_dhcp_opts_invalid(self):
@mock.patch.object(mech_driver.LOG, 'info')
def test__validate_port_extra_dhcp_opts_invalid(self, mock_log):
port_id = 'fake-port'
opt = {'opt_name': 'not-valid',
'opt_value': 'spongebob squarepants',
'ip_version': 4}
port = {edo_ext.EXTRADHCPOPTS: [opt], 'id': 'fake-port'}
self.assertRaises(mech_driver.OVNPortUpdateError,
self.mech_driver._validate_port_extra_dhcp_opts,
port)

def test_create_port_invalid_extra_dhcp_opts(self):
port = {edo_ext.EXTRADHCPOPTS: [opt], 'id': port_id}
self.mech_driver._validate_port_extra_dhcp_opts(port)
# Assert the log message contained the invalid DHCP options
expected_call = mock.call(
mock.ANY, {'port_id': port_id, 'ipv4_opts': 'not-valid',
'ipv6_opts': ''})
mock_log.assert_has_calls([expected_call])

@mock.patch.object(mech_driver.LOG, 'info')
def test_create_port_invalid_extra_dhcp_opts(self, mock_log):
extra_dhcp_opts = {
'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'banana',
'opt_value': 'banana'},
@@ -250,15 +256,17 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
with self.network() as n:
with self.subnet(n):
res = self._create_port(self.fmt, n['network']['id'],
arg_list=('extra_dhcp_opts',),
**extra_dhcp_opts)
# Assert 400 (BadRequest) was returned
self.assertEqual(400, res.status_code)
response = self.deserialize(self.fmt, res)
self.assertIn('banana', response['NeutronError']['message'])
self.assertIn('orange', response['NeutronError']['message'])

def test_update_port_invalid_extra_dhcp_opts(self):
arg_list=('extra_dhcp_opts',),
**extra_dhcp_opts)
port_id = self.deserialize(self.fmt, res)['port']['id']
# Assert the log message contained the invalid DHCP options
expected_call = mock.call(
mock.ANY, {'port_id': port_id, 'ipv4_opts': 'banana',
'ipv6_opts': 'orange'})
mock_log.assert_has_calls([expected_call])

@mock.patch.object(mech_driver.LOG, 'info')
def test_update_port_invalid_extra_dhcp_opts(self, mock_log):
data = {
'port': {'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'apple',
'opt_value': 'apple'},
@@ -268,10 +276,14 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
with self.subnet(network=net) as subnet:
with self.port(subnet=subnet,
set_context=True, tenant_id='test') as port:
res = self._update('ports', port['port']['id'], data,
expected_code=400)
self.assertIn('apple', res['NeutronError']['message'])
self.assertIn('grape', res['NeutronError']['message'])
port_id = port['port']['id']
self._update('ports', port_id, data)

# Assert the log message contained the invalid DHCP options
expected_call = mock.call(
mock.ANY, {'port_id': port_id, 'ipv4_opts': 'apple',
'ipv6_opts': 'grape'})
mock_log.assert_has_calls([expected_call])

def test_create_and_update_ignored_fip_port(self):
with self.network(set_context=True, tenant_id='test') as net1:


Loading…
Cancel
Save