From b661698224a4f17d9128c9763d11017d685eabc8 Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Tue, 20 Oct 2015 17:42:57 -0400 Subject: [PATCH] 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. (Cherry-picked from commit 204eb2af1752cbdd4d1bfc42aa4d61debdeb7551) Change-Id: Ib3072966140b7f6ca954d7847ad9835aa1191998 Closes-Bug: #1508205 --- neutron/plugins/ml2/drivers/l2pop/db.py | 6 ++- neutron/tests/fullstack/test_connectivity.py | 6 ++- .../unit/plugins/ml2/drivers/l2pop/test_db.py | 42 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py diff --git a/neutron/plugins/ml2/drivers/l2pop/db.py b/neutron/plugins/ml2/drivers/l2pop/db.py index 8023434c401..0208417f779 100644 --- 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 5137fa310fc..9915af571c6 100644 --- 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 index 00000000000..e4bf6dc59e2 --- /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)