From 58ea633fa44d8994596055a23cfde260e6d9d9c9 Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Mon, 30 Nov 2015 19:05:18 +0100 Subject: [PATCH] Refactor OVS-agent tunnel config validate This change transforms validate_local_ip into a sub-method of validate_tunnel_config and raises directly SystemExit instead of indirectly. Related-bug: #1464394 Change-Id: I35addd41e1a8b061bd0e5e6656a1728fb7fe04ce --- .../openvswitch/agent/ovs_neutron_agent.py | 26 ++++------- .../agent/test_ovs_neutron_agent.py | 43 +++++++------------ 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 64028bc1096..238563c3103 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1836,10 +1836,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, def validate_local_ip(local_ip): - """If tunneling is enabled, verify if the ip exists on the agent's host.""" - if not cfg.CONF.AGENT.tunnel_types: - return - + """Verify if the ip exists on the agent's host.""" if not ip_lib.IPWrapper().get_device_by_ip(local_ip): LOG.error(_LE("Tunneling can't be enabled with invalid local_ip '%s'." " IP couldn't be found on this host's interfaces."), @@ -1847,15 +1844,16 @@ def validate_local_ip(local_ip): raise SystemExit(1) -def validate_tunnel_types(tunnel_types, local_ip): - if tunnel_types and not local_ip: - msg = _('Tunneling cannot be enabled without a valid local_ip.') - raise ValueError(msg) +def validate_tunnel_config(tunnel_types, local_ip): + """Verify local ip and tunnel config if tunneling is enabled.""" + if not tunnel_types: + return + validate_local_ip(local_ip) for tun in tunnel_types: if tun not in constants.TUNNEL_NETWORK_TYPES: - msg = _('Invalid tunnel type specified: %s') % tun - raise ValueError(msg) + LOG.error(_LE('Invalid tunnel type specified: %s'), tun) + raise SystemExit(1) def prepare_xen_compute(): @@ -1869,13 +1867,7 @@ def prepare_xen_compute(): def main(bridge_classes): prepare_xen_compute() - validate_local_ip(cfg.CONF.OVS.local_ip) - try: - validate_tunnel_types(cfg.CONF.AGENT.tunnel_types, - cfg.CONF.OVS.local_ip) - except ValueError as e: - LOG.error(_LE("Validation of tunnel types failed. %s"), e) - raise SystemExit(1) + validate_tunnel_config(cfg.CONF.AGENT.tunnel_types, cfg.CONF.OVS.local_ip) try: agent = OVSNeutronAgent(bridge_classes, cfg.CONF) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 1dfb0c080a7..cffff85fdd8 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -69,35 +69,25 @@ class MockFixedIntervalLoopingCall(object): class ValidateTunnelTypes(ovs_test_base.OVSAgentConfigTestBase): + def setUp(self): + super(ValidateTunnelTypes, self).setUp() + self.mock_validate_local_ip = mock.patch.object( + self.mod_agent, 'validate_local_ip').start() + def test_validate_tunnel_types_succeeds(self): cfg.CONF.set_override('local_ip', '10.10.10.10', group='OVS') cfg.CONF.set_override('tunnel_types', [p_const.TYPE_GRE], group='AGENT') - # ValueError will not raise - self.mod_agent.validate_tunnel_types(cfg.CONF.AGENT.tunnel_types, - cfg.CONF.OVS.local_ip) - - def test_validate_tunnel_types_fails_for_invalid_tunnel_config(self): - # An ip address is required for tunneling but there is no default, - # verify this for both gre and vxlan tunnels. - cfg.CONF.set_override('local_ip', None, group='OVS') - cfg.CONF.set_override('tunnel_types', [p_const.TYPE_GRE], - group='AGENT') - with testtools.ExpectedException(ValueError): - self.mod_agent.validate_tunnel_types(cfg.CONF.AGENT.tunnel_types, - cfg.CONF.OVS.local_ip) - cfg.CONF.set_override('tunnel_types', [p_const.TYPE_VXLAN], - group='AGENT') - with testtools.ExpectedException(ValueError): - self.mod_agent.validate_tunnel_types(cfg.CONF.AGENT.tunnel_types, - cfg.CONF.OVS.local_ip) + self.mod_agent.validate_tunnel_config(cfg.CONF.AGENT.tunnel_types, + cfg.CONF.OVS.local_ip) + self.mock_validate_local_ip.assert_called_once_with('10.10.10.10') def test_validate_tunnel_types_fails_for_invalid_tunnel_type(self): cfg.CONF.set_override('local_ip', '10.10.10.10', group='OVS') cfg.CONF.set_override('tunnel_types', ['foobar'], group='AGENT') - with testtools.ExpectedException(ValueError): - self.mod_agent.validate_tunnel_types(cfg.CONF.AGENT.tunnel_types, - cfg.CONF.OVS.local_ip) + with testtools.ExpectedException(SystemExit): + self.mod_agent.validate_tunnel_config(cfg.CONF.AGENT.tunnel_types, + cfg.CONF.OVS.local_ip) class TestOvsNeutronAgent(object): @@ -2674,20 +2664,17 @@ class TestOvsDvrNeutronAgentRyu(TestOvsDvrNeutronAgent, class TestValidateTunnelLocalIP(base.BaseTestCase): - def test_validate_local_ip_no_tunneling(self): - cfg.CONF.set_override('tunnel_types', [], group='AGENT') - # The test will pass simply if no exception is raised by the next call: - ovs_agent.validate_local_ip(FAKE_IP1) - def test_validate_local_ip_with_valid_ip(self): - cfg.CONF.set_override('tunnel_types', ['vxlan'], group='AGENT') mock_get_device_by_ip = mock.patch.object( ip_lib.IPWrapper, 'get_device_by_ip').start() ovs_agent.validate_local_ip(FAKE_IP1) mock_get_device_by_ip.assert_called_once_with(FAKE_IP1) + def test_validate_local_ip_with_none_ip(self): + with testtools.ExpectedException(SystemExit): + ovs_agent.validate_local_ip(None) + def test_validate_local_ip_with_invalid_ip(self): - cfg.CONF.set_override('tunnel_types', ['vxlan'], group='AGENT') mock_get_device_by_ip = mock.patch.object( ip_lib.IPWrapper, 'get_device_by_ip').start() mock_get_device_by_ip.return_value = None