From d07e87920826c5b4a1077939bbf5f9c9870d6511 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Thu, 23 Jul 2020 13:07:42 +0100 Subject: [PATCH] [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 (cherry picked from commit 8c4b23f8128fa396164293d74837fa3b391e9974) --- .../drivers/ovn/mech_driver/mech_driver.py | 8 ++-- .../ovn/mech_driver/test_mech_driver.py | 48 ++++++++++++------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index c4bab290e60..54268ef6025 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -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. 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 e4e61cc3832..4a39254b6ff 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 @@ -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) + 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]) - def test_create_port_invalid_extra_dhcp_opts(self): + @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']) + 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]) - def test_update_port_invalid_extra_dhcp_opts(self): + @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: