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

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 64028bc10968d5feb86289e8bb2b7a4d46095df1..238563c3103207c59717343590e3708d58ab5b69 100644 (file)
--- 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 1dfb0c080a71f87142b3c8926725f94adadb1cef..cffff85fdd8d0e4325ec24353a597ca9a177b1ad 100644 (file)
--- 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