]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Datapath on L2pop only for agents with tunneling-ip
authorHong Hui Xiao <xiaohhui@cn.ibm.com>
Wed, 28 Oct 2015 07:28:27 +0000 (03:28 -0400)
committerHong Hui Xiao <xiaohhui@cn.ibm.com>
Thu, 19 Nov 2015 06:18:51 +0000 (01:18 -0500)
This patch is a regression issue for patch[1].

Only those agents which expose tunneling-ip will be considered in
determining data-path tunnels in deployments with l2pop ON.

[1] https://review.openstack.org/#/c/236970/

Change-Id: I7c3b911d5e7448b4e8dee15bb50df33a6e9d5cfe
Closes-Bug: #1407959

neutron/plugins/ml2/drivers/l2pop/db.py
neutron/plugins/ml2/drivers/l2pop/mech_driver.py
neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py
neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py

index 0208417f779c7ebfaef68f75aec27738220bd2f0..822e9da0243101c33338b0477b2c34fef09fa504 100644 (file)
@@ -71,8 +71,10 @@ class L2populationDbMixin(base_db.CommonDbMixin):
 
     def get_nondvr_active_network_ports(self, session, network_id):
         query = self._get_active_network_ports(session, network_id)
-        return query.filter(models_v2.Port.device_owner !=
-                            const.DEVICE_OWNER_DVR_INTERFACE)
+        query = query.filter(models_v2.Port.device_owner !=
+                             const.DEVICE_OWNER_DVR_INTERFACE)
+        return [(bind, agent) for bind, agent in query.all()
+                if self.get_agent_ip(agent)]
 
     def get_dvr_active_network_ports(self, session, network_id):
         with session.begin(subtransactions=True):
@@ -87,7 +89,8 @@ class L2populationDbMixin(base_db.CommonDbMixin):
                                  const.PORT_STATUS_ACTIVE,
                                  models_v2.Port.device_owner ==
                                  const.DEVICE_OWNER_DVR_INTERFACE)
-            return query
+        return [(bind, agent) for bind, agent in query.all()
+                if self.get_agent_ip(agent)]
 
     def get_agent_network_active_port_count(self, session, agent_host,
                                             network_id):
index 928ca36f24a79661d517523d5a9b20dc007833e5..14dab900f6b69e1f24ececbf390dddddddb35bb3 100644 (file)
@@ -159,13 +159,11 @@ class L2populationMechanismDriver(api.MechanismDriver,
         session = db_api.get_session()
         agent = self.get_agent_by_host(session, agent_host)
         if not agent:
+            LOG.warning(_LW("Unable to retrieve active L2 agent on host %s"),
+                        agent_host)
             return
 
         agent_ip = self.get_agent_ip(agent)
-        if not agent_ip:
-            LOG.warning(_LW("Unable to retrieve the agent ip, check the agent "
-                            "configuration."))
-            return
 
         segment = context.bottom_bound_segment
         if not segment:
@@ -189,9 +187,9 @@ class L2populationMechanismDriver(api.MechanismDriver,
                               'network_type': segment['network_type'],
                               'ports': {}}}
         tunnel_network_ports = (
-            self.get_dvr_active_network_ports(session, network_id).all())
+            self.get_dvr_active_network_ports(session, network_id))
         fdb_network_ports = (
-            self.get_nondvr_active_network_ports(session, network_id).all())
+            self.get_nondvr_active_network_ports(session, network_id))
         ports = agent_fdb_entries[network_id]['ports']
         ports.update(self._get_tunnels(
             fdb_network_ports + tunnel_network_ports,
index e4bf6dc59e2c880cb9d5d76b5ff7e6859d6feef5..6dc71ece9148ca301bba5baa865fbfbb034e88da 100644 (file)
 
 from neutron.common import constants
 from neutron import context
+from neutron.db import models_v2
+from neutron.extensions import portbindings
 from neutron.plugins.ml2.drivers.l2pop import db as l2pop_db
+from neutron.plugins.ml2 import models
 from neutron.tests.common import helpers
 from neutron.tests.unit import testlib_api
 
@@ -23,6 +26,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):
     def setUp(self):
         super(TestL2PopulationDBTestCase, self).setUp()
         self.db_mixin = l2pop_db.L2populationDbMixin()
+        self.ctx = context.get_admin_context()
 
     def test_get_agent_by_host(self):
         # Register a L2 agent + A bunch of other agents on the same host
@@ -30,7 +34,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):
         helpers.register_dhcp_agent()
         helpers.register_ovs_agent()
         agent = self.db_mixin.get_agent_by_host(
-            context.get_admin_context().session, helpers.HOST)
+            self.ctx.session, helpers.HOST)
         self.assertEqual(constants.AGENT_TYPE_OVS, agent.agent_type)
 
     def test_get_agent_by_host_no_candidate(self):
@@ -38,5 +42,73 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):
         helpers.register_l3_agent()
         helpers.register_dhcp_agent()
         agent = self.db_mixin.get_agent_by_host(
-            context.get_admin_context().session, helpers.HOST)
+            self.ctx.session, helpers.HOST)
         self.assertIsNone(agent)
+
+    def _setup_port_binding(self, network_id='network_id', dvr=True):
+        with self.ctx.session.begin(subtransactions=True):
+            self.ctx.session.add(models_v2.Network(id=network_id))
+            device_owner = constants.DEVICE_OWNER_DVR_INTERFACE if dvr else ''
+            self.ctx.session.add(models_v2.Port(
+                id='port_id',
+                network_id=network_id,
+                mac_address='00:11:22:33:44:55',
+                admin_state_up=True,
+                status=constants.PORT_STATUS_ACTIVE,
+                device_id='',
+                device_owner=device_owner))
+            port_binding_cls = (models.DVRPortBinding if dvr
+                                else models.PortBinding)
+            binding_kwarg = {
+                'port_id': 'port_id',
+                'host': helpers.HOST,
+                'vif_type': portbindings.VIF_TYPE_UNBOUND,
+                'vnic_type': portbindings.VNIC_NORMAL
+            }
+            if dvr:
+                binding_kwarg['router_id'] = 'router_id'
+                binding_kwarg['status'] = constants.PORT_STATUS_DOWN
+
+            self.ctx.session.add(port_binding_cls(**binding_kwarg))
+
+    def test_get_dvr_active_network_ports(self):
+        self._setup_port_binding()
+        # Register a L2 agent + A bunch of other agents on the same host
+        helpers.register_l3_agent()
+        helpers.register_dhcp_agent()
+        helpers.register_ovs_agent()
+        tunnel_network_ports = self.db_mixin.get_dvr_active_network_ports(
+            self.ctx.session, 'network_id')
+        self.assertEqual(1, len(tunnel_network_ports))
+        _, agent = tunnel_network_ports[0]
+        self.assertEqual(constants.AGENT_TYPE_OVS, agent.agent_type)
+
+    def test_get_dvr_active_network_ports_no_candidate(self):
+        self._setup_port_binding()
+        # Register a bunch of non-L2 agents on the same host
+        helpers.register_l3_agent()
+        helpers.register_dhcp_agent()
+        tunnel_network_ports = self.db_mixin.get_dvr_active_network_ports(
+            self.ctx.session, 'network_id')
+        self.assertEqual(0, len(tunnel_network_ports))
+
+    def test_get_nondvr_active_network_ports(self):
+        self._setup_port_binding(dvr=False)
+        # Register a L2 agent + A bunch of other agents on the same host
+        helpers.register_l3_agent()
+        helpers.register_dhcp_agent()
+        helpers.register_ovs_agent()
+        fdb_network_ports = self.db_mixin.get_nondvr_active_network_ports(
+            self.ctx.session, 'network_id')
+        self.assertEqual(1, len(fdb_network_ports))
+        _, agent = fdb_network_ports[0]
+        self.assertEqual(constants.AGENT_TYPE_OVS, agent.agent_type)
+
+    def test_get_nondvr_active_network_ports_no_candidate(self):
+        self._setup_port_binding(dvr=False)
+        # Register a bunch of non-L2 agents on the same host
+        helpers.register_l3_agent()
+        helpers.register_dhcp_agent()
+        fdb_network_ports = self.db_mixin.get_nondvr_active_network_ports(
+            self.ctx.session, 'network_id')
+        self.assertEqual(0, len(fdb_network_ports))
index ccfe85ba3fd0ad2b2b68e97e159be307ef4a7102..7402cc49f57295a044e0b79b94c9dece778c81df 100644 (file)
@@ -818,10 +818,10 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
         tunnels = self._test_get_tunnels(None, exclude_host=False)
         self.assertEqual(0, len(tunnels))
 
-    def _test_create_agent_fdb(self, fdb_network_ports_query, agent_ips):
+    def _test_create_agent_fdb(self, fdb_network_ports, agent_ips):
         mech_driver = l2pop_mech_driver.L2populationMechanismDriver()
-        tunnel_network_ports_query, tunnel_agent = (
-            self._mock_network_ports_query(HOST + '1', None))
+        tunnel_network_ports, tunnel_agent = (
+            self._mock_network_ports(HOST + '1', None))
         agent_ips[tunnel_agent] = '10.0.0.1'
 
         def agent_ip_side_effect(agent):
@@ -832,10 +832,10 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
                                side_effect=agent_ip_side_effect),\
                 mock.patch.object(l2pop_db.L2populationDbMixin,
                                   'get_nondvr_active_network_ports',
-                                  new=fdb_network_ports_query),\
+                                  return_value=fdb_network_ports),\
                 mock.patch.object(l2pop_db.L2populationDbMixin,
                                   'get_dvr_active_network_ports',
-                                  new=tunnel_network_ports_query):
+                                  return_value=tunnel_network_ports):
             session = mock.Mock()
             agent = mock.Mock()
             agent.host = HOST
@@ -845,24 +845,20 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
                                                  segment,
                                                  'network_id')
 
-    def _mock_network_ports_query(self, host_name, binding):
+    def _mock_network_ports(self, host_name, binding):
         agent = mock.Mock()
         agent.host = host_name
-        result = [(binding, agent)]
-        all_mock = mock.Mock()
-        all_mock.all = mock.Mock(return_value=result)
-        mock_query = mock.Mock(return_value=all_mock)
-        return mock_query, agent
+        return [(binding, agent)], agent
 
     def test_create_agent_fdb(self):
         binding = mock.Mock()
         binding.port = {'mac_address': '00:00:DE:AD:BE:EF',
                         'fixed_ips': [{'ip_address': '1.1.1.1'}]}
-        fdb_network_ports_query, fdb_agent = (
-            self._mock_network_ports_query(HOST + '2', binding))
+        fdb_network_ports, fdb_agent = (
+            self._mock_network_ports(HOST + '2', binding))
         agent_ips = {fdb_agent: '20.0.0.1'}
 
-        agent_fdb = self._test_create_agent_fdb(fdb_network_ports_query,
+        agent_fdb = self._test_create_agent_fdb(fdb_network_ports,
                                                 agent_ips)
         result = agent_fdb['network_id']
 
@@ -879,10 +875,7 @@ class TestL2PopulationMechDriver(base.BaseTestCase):
         self.assertEqual(expected_result, result)
 
     def test_create_agent_fdb_only_tunnels(self):
-        all_mock = mock.Mock()
-        all_mock.all = mock.Mock(return_value=[])
-        fdb_network_ports_query = mock.Mock(return_value=all_mock)
-        agent_fdb = self._test_create_agent_fdb(fdb_network_ports_query, {})
+        agent_fdb = self._test_create_agent_fdb([], {})
         result = agent_fdb['network_id']
 
         expected_result = {'segment_id': 1,