Fix l2pop regression

Patch https://review.openstack.org/#/c/236970/ introduced an issue
where get_agent_by_host can return a random host (Including L3,
DHCP or metadata agents), not only L2 agents. The caller then
tries to get tunneling_ip, which might not exist on the returned
agent, causing l2pop code to bail out with a WARNING:
'Unable to retrieve the agent ip...'.

The issue was found by manual introspection of the code, and
verified by modifying the l2pop fullstack test to register L3
agents. Both a unit test was added, as well as modifying the
fullstack connectivity test to register L3 agents if l2pop
is enabled.

The code will now check for agents with a tunneling_ip key
in their configurations dict, which is required for l2pop
to work correctly, essentially a form of duck typing.

Change-Id: Ib3072966140b7f6ca954d7847ad9835aa1191998
Closes-Bug: #1508205

diff --git a/neutron/plugins/ml2/drivers/l2pop/db.py b/neutron/plugins/ml2/drivers/l2pop/db.py
index 8023434c40171f1f71aca105176aa8a25794c12b..0208417f779c7ebfaef68f75aec27738220bd2f0 100644 (file)
--- a/neutron/plugins/ml2/drivers/l2pop/db.py
+++ b/neutron/plugins/ml2/drivers/l2pop/db.py
@@ -47,10 +47,14 @@ class L2populationDbMixin(base_db.CommonDbMixin):
         return configuration.get('l2pop_network_types')
 
     def get_agent_by_host(self, session, agent_host):
+        """Return a L2 agent on the host."""
+
         with session.begin(subtransactions=True):
             query = session.query(agents_db.Agent)
             query = query.filter(agents_db.Agent.host == agent_host)
-            return query.first()
+        for agent in query:
+            if self.get_agent_ip(agent):
+                return agent
 
     def _get_active_network_ports(self, session, network_id):
         with session.begin(subtransactions=True):

diff --git a/neutron/tests/fullstack/test_connectivity.py b/neutron/tests/fullstack/test_connectivity.py
index 5137fa310fce5559c5e7180e6ad97e7e6781460b..9915af571c694447eedffe1189a1f027f3b7836e 100644 (file)
--- a/neutron/tests/fullstack/test_connectivity.py
+++ b/neutron/tests/fullstack/test_connectivity.py
@@ -36,7 +36,11 @@ class TestConnectivitySameNetwork(base.BaseFullStackTestCase):
 
     def setUp(self):
         host_descriptions = [
-            environment.HostDescription() for _ in range(2)]
+            # There's value in enabling L3 agents registration when l2pop
+            # is enabled, because l2pop code makes assumptions about the
+            # agent types present on machines.
+            environment.HostDescription(
+                l3_agent=self.l2_pop) for _ in range(2)]
         env = environment.Environment(
             environment.EnvironmentDescription(
                 network_type=self.network_type,

diff --git a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py
new file mode 100644 (file)
index 0000000..e4bf6dc
--- /dev/null
+++ b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py
@@ -0,0 +1,42 @@
+# Copyright 2015 Red Hat, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron.common import constants
+from neutron import context
+from neutron.plugins.ml2.drivers.l2pop import db as l2pop_db
+from neutron.tests.common import helpers
+from neutron.tests.unit import testlib_api
+
+
+class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):
+    def setUp(self):
+        super(TestL2PopulationDBTestCase, self).setUp()
+        self.db_mixin = l2pop_db.L2populationDbMixin()
+
+    def test_get_agent_by_host(self):
+        # 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()
+        agent = self.db_mixin.get_agent_by_host(
+            context.get_admin_context().session, helpers.HOST)
+        self.assertEqual(constants.AGENT_TYPE_OVS, agent.agent_type)
+
+    def test_get_agent_by_host_no_candidate(self):
+        # Register a bunch of non-L2 agents on the same host
+        helpers.register_l3_agent()
+        helpers.register_dhcp_agent()
+        agent = self.db_mixin.get_agent_by_host(
+            context.get_admin_context().session, helpers.HOST)
+        self.assertIsNone(agent)