From 3c683709e6cf687d7e15e1cfc28373a12a56d9f2 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Fri, 15 Jan 2016 13:44:14 -0600 Subject: [PATCH] Move L2populationDbMixin to module-level functions None of the L2populationDbMixin methods actually use 'self' for anything. As the class is basically just used as a namespace and modules already provide that, this patch gets rid of the mixin. This makes the code simpler and easier to debug as inheritance doesn't buy us anything in this case. Change-Id: Ibf4dfe49a2ebc32d3909d3d7b579d2bb2ea3f61d --- neutron/plugins/ml2/drivers/l2pop/db.py | 155 +++++++++--------- .../plugins/ml2/drivers/l2pop/mech_driver.py | 34 ++-- .../unit/plugins/ml2/drivers/l2pop/test_db.py | 13 +- .../ml2/drivers/l2pop/test_mech_driver.py | 15 +- 4 files changed, 106 insertions(+), 111 deletions(-) diff --git a/neutron/plugins/ml2/drivers/l2pop/db.py b/neutron/plugins/ml2/drivers/l2pop/db.py index f0f8060c886..257b4af59e4 100644 --- a/neutron/plugins/ml2/drivers/l2pop/db.py +++ b/neutron/plugins/ml2/drivers/l2pop/db.py @@ -22,92 +22,93 @@ from neutron.db import models_v2 from neutron.plugins.ml2 import models as ml2_models -class L2populationDbMixin(object): +def get_agent_ip_by_host(session, agent_host): + agent = get_agent_by_host(session, agent_host) + if agent: + return get_agent_ip(agent) - def get_agent_ip_by_host(self, session, agent_host): - agent = self.get_agent_by_host(session, agent_host) - if agent: - return self.get_agent_ip(agent) - def get_agent_ip(self, agent): - configuration = jsonutils.loads(agent.configurations) - return configuration.get('tunneling_ip') +def get_agent_ip(agent): + configuration = jsonutils.loads(agent.configurations) + return configuration.get('tunneling_ip') - def get_agent_uptime(self, agent): - return timeutils.delta_seconds(agent.started_at, - agent.heartbeat_timestamp) - def get_agent_tunnel_types(self, agent): - configuration = jsonutils.loads(agent.configurations) - return configuration.get('tunnel_types') +def get_agent_uptime(agent): + return timeutils.delta_seconds(agent.started_at, + agent.heartbeat_timestamp) - def get_agent_l2pop_network_types(self, agent): - configuration = jsonutils.loads(agent.configurations) - return configuration.get('l2pop_network_types') - def get_agent_by_host(self, session, agent_host): - """Return a L2 agent on the host.""" +def get_agent_tunnel_types(agent): + configuration = jsonutils.loads(agent.configurations) + return configuration.get('tunnel_types') - with session.begin(subtransactions=True): - query = session.query(agents_db.Agent) - query = query.filter(agents_db.Agent.host == agent_host) - 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): - query = session.query(ml2_models.PortBinding, - agents_db.Agent) - query = query.join(agents_db.Agent, - agents_db.Agent.host == - ml2_models.PortBinding.host) - query = query.join(models_v2.Port) - query = query.filter(models_v2.Port.network_id == network_id, - models_v2.Port.status == - const.PORT_STATUS_ACTIVE) - return query +def get_agent_l2pop_network_types(agent): + configuration = jsonutils.loads(agent.configurations) + return configuration.get('l2pop_network_types') - def get_nondvr_active_network_ports(self, session, network_id): - query = self._get_active_network_ports(session, network_id) - query = query.filter(models_v2.Port.device_owner != + +def get_agent_by_host(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) + for agent in query: + if get_agent_ip(agent): + return agent + + +def _get_active_network_ports(session, network_id): + with session.begin(subtransactions=True): + query = session.query(ml2_models.PortBinding, agents_db.Agent) + query = query.join(agents_db.Agent, + agents_db.Agent.host == ml2_models.PortBinding.host) + query = query.join(models_v2.Port) + query = query.filter(models_v2.Port.network_id == network_id, + models_v2.Port.status == const.PORT_STATUS_ACTIVE) + return query + + +def get_nondvr_active_network_ports(session, network_id): + query = _get_active_network_ports(session, network_id) + query = query.filter(models_v2.Port.device_owner != + const.DEVICE_OWNER_DVR_INTERFACE) + return [(bind, agent) for bind, agent in query.all() + if get_agent_ip(agent)] + + +def get_dvr_active_network_ports(session, network_id): + with session.begin(subtransactions=True): + query = session.query(ml2_models.DVRPortBinding, agents_db.Agent) + query = query.join(agents_db.Agent, + agents_db.Agent.host == + ml2_models.DVRPortBinding.host) + query = query.join(models_v2.Port) + query = query.filter(models_v2.Port.network_id == network_id, + models_v2.Port.status == const.PORT_STATUS_ACTIVE, + 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)] + return [(bind, agent) for bind, agent in query.all() + if get_agent_ip(agent)] - def get_dvr_active_network_ports(self, session, network_id): - with session.begin(subtransactions=True): - query = session.query(ml2_models.DVRPortBinding, - agents_db.Agent) - query = query.join(agents_db.Agent, - agents_db.Agent.host == - ml2_models.DVRPortBinding.host) - query = query.join(models_v2.Port) - query = query.filter(models_v2.Port.network_id == network_id, - models_v2.Port.status == - const.PORT_STATUS_ACTIVE, - 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_agent_network_active_port_count(self, session, agent_host, - network_id): - with session.begin(subtransactions=True): - query = session.query(models_v2.Port) - query1 = query.join(ml2_models.PortBinding) - query1 = query1.filter(models_v2.Port.network_id == network_id, - models_v2.Port.status == - const.PORT_STATUS_ACTIVE, - models_v2.Port.device_owner != - const.DEVICE_OWNER_DVR_INTERFACE, - ml2_models.PortBinding.host == agent_host) - query2 = query.join(ml2_models.DVRPortBinding) - query2 = query2.filter(models_v2.Port.network_id == network_id, - ml2_models.DVRPortBinding.status == - const.PORT_STATUS_ACTIVE, - models_v2.Port.device_owner == - const.DEVICE_OWNER_DVR_INTERFACE, - ml2_models.DVRPortBinding.host == - agent_host) - return (query1.count() + query2.count()) +def get_agent_network_active_port_count(session, agent_host, + network_id): + with session.begin(subtransactions=True): + query = session.query(models_v2.Port) + query1 = query.join(ml2_models.PortBinding) + query1 = query1.filter(models_v2.Port.network_id == network_id, + models_v2.Port.status == + const.PORT_STATUS_ACTIVE, + models_v2.Port.device_owner != + const.DEVICE_OWNER_DVR_INTERFACE, + ml2_models.PortBinding.host == agent_host) + query2 = query.join(ml2_models.DVRPortBinding) + query2 = query2.filter(models_v2.Port.network_id == network_id, + ml2_models.DVRPortBinding.status == + const.PORT_STATUS_ACTIVE, + models_v2.Port.device_owner == + const.DEVICE_OWNER_DVR_INTERFACE, + ml2_models.DVRPortBinding.host == agent_host) + return (query1.count() + query2.count()) diff --git a/neutron/plugins/ml2/drivers/l2pop/mech_driver.py b/neutron/plugins/ml2/drivers/l2pop/mech_driver.py index 6e3baaaf7b5..e4ddfe2e801 100644 --- a/neutron/plugins/ml2/drivers/l2pop/mech_driver.py +++ b/neutron/plugins/ml2/drivers/l2pop/mech_driver.py @@ -29,8 +29,7 @@ from neutron.plugins.ml2.drivers.l2pop import rpc as l2pop_rpc LOG = logging.getLogger(__name__) -class L2populationMechanismDriver(api.MechanismDriver, - l2pop_db.L2populationDbMixin): +class L2populationMechanismDriver(api.MechanismDriver): def __init__(self): super(L2populationMechanismDriver, self).__init__() @@ -76,7 +75,8 @@ class L2populationMechanismDriver(api.MechanismDriver, if not agent_host: return - agent_ip = self.get_agent_ip_by_host(db_api.get_session(), agent_host) + agent_ip = l2pop_db.get_agent_ip_by_host(db_api.get_session(), + agent_host) orig_mac_ip = [l2pop_rpc.PortInfo(mac_address=port['mac_address'], ip_address=ip) @@ -159,9 +159,9 @@ class L2populationMechanismDriver(api.MechanismDriver, "to any segment", {'port': port_id, 'agent': agent}) return - network_types = self.get_agent_l2pop_network_types(agent) + network_types = l2pop_db.get_agent_l2pop_network_types(agent) if network_types is None: - network_types = self.get_agent_tunnel_types(agent) + network_types = l2pop_db.get_agent_tunnel_types(agent) if segment['network_type'] not in network_types: return @@ -173,16 +173,16 @@ class L2populationMechanismDriver(api.MechanismDriver, 'network_type': segment['network_type'], 'ports': {}}} tunnel_network_ports = ( - self.get_dvr_active_network_ports(session, network_id)) + l2pop_db.get_dvr_active_network_ports(session, network_id)) fdb_network_ports = ( - self.get_nondvr_active_network_ports(session, network_id)) + l2pop_db.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, agent.host)) for agent_ip, fdbs in ports.items(): for binding, agent in fdb_network_ports: - if self.get_agent_ip(agent) == agent_ip: + if l2pop_db.get_agent_ip(agent) == agent_ip: fdbs.extend(self._get_port_fdb_entries(binding.port)) return agent_fdb_entries @@ -193,7 +193,7 @@ class L2populationMechanismDriver(api.MechanismDriver, if agent.host == exclude_host: continue - ip = self.get_agent_ip(agent) + ip = l2pop_db.get_agent_ip(agent) if not ip: LOG.debug("Unable to retrieve the agent ip, check " "the agent %s configuration.", agent.host) @@ -208,7 +208,7 @@ class L2populationMechanismDriver(api.MechanismDriver, port = context.current agent_host = context.host session = db_api.get_session() - agent = self.get_agent_by_host(session, agent_host) + agent = l2pop_db.get_agent_by_host(session, agent_host) if not agent: LOG.warning(_LW("Unable to retrieve active L2 agent on host %s"), agent_host) @@ -216,10 +216,10 @@ class L2populationMechanismDriver(api.MechanismDriver, network_id = port['network_id'] - agent_active_ports = self.get_agent_network_active_port_count( + agent_active_ports = l2pop_db.get_agent_network_active_port_count( session, agent_host, network_id) - agent_ip = self.get_agent_ip(agent) + agent_ip = l2pop_db.get_agent_ip(agent) segment = self._get_and_validate_segment(context, port['id'], agent) if not segment: return @@ -227,8 +227,8 @@ class L2populationMechanismDriver(api.MechanismDriver, segment, agent_ip, network_id) other_fdb_ports = other_fdb_entries[network_id]['ports'] - if agent_active_ports == 1 or ( - self.get_agent_uptime(agent) < cfg.CONF.l2pop.agent_boot_time): + if agent_active_ports == 1 or (l2pop_db.get_agent_uptime(agent) < + cfg.CONF.l2pop.agent_boot_time): # First port activated on current agent in this network, # we have to provide it with the whole list of fdb entries agent_fdb_entries = self._create_agent_fdb(session, @@ -257,15 +257,15 @@ class L2populationMechanismDriver(api.MechanismDriver, network_id = port['network_id'] session = db_api.get_session() - agent_active_ports = self.get_agent_network_active_port_count( + agent_active_ports = l2pop_db.get_agent_network_active_port_count( session, agent_host, network_id) - agent = self.get_agent_by_host(db_api.get_session(), agent_host) + agent = l2pop_db.get_agent_by_host(db_api.get_session(), agent_host) segment = self._get_and_validate_segment(context, port['id'], agent) if not segment: return - agent_ip = self.get_agent_ip(agent) + agent_ip = l2pop_db.get_agent_ip(agent) other_fdb_entries = self._get_fdb_entries_template( segment, agent_ip, port['network_id']) if agent_active_ports == 0: diff --git a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py index 6dc71ece914..212e4af9215 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py +++ b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py @@ -25,7 +25,6 @@ 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() self.ctx = context.get_admin_context() def test_get_agent_by_host(self): @@ -33,7 +32,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase): helpers.register_l3_agent() helpers.register_dhcp_agent() helpers.register_ovs_agent() - agent = self.db_mixin.get_agent_by_host( + agent = l2pop_db.get_agent_by_host( self.ctx.session, helpers.HOST) self.assertEqual(constants.AGENT_TYPE_OVS, agent.agent_type) @@ -41,7 +40,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase): # 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( + agent = l2pop_db.get_agent_by_host( self.ctx.session, helpers.HOST) self.assertIsNone(agent) @@ -77,7 +76,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase): helpers.register_l3_agent() helpers.register_dhcp_agent() helpers.register_ovs_agent() - tunnel_network_ports = self.db_mixin.get_dvr_active_network_ports( + tunnel_network_ports = l2pop_db.get_dvr_active_network_ports( self.ctx.session, 'network_id') self.assertEqual(1, len(tunnel_network_ports)) _, agent = tunnel_network_ports[0] @@ -88,7 +87,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase): # 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( + tunnel_network_ports = l2pop_db.get_dvr_active_network_ports( self.ctx.session, 'network_id') self.assertEqual(0, len(tunnel_network_ports)) @@ -98,7 +97,7 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase): helpers.register_l3_agent() helpers.register_dhcp_agent() helpers.register_ovs_agent() - fdb_network_ports = self.db_mixin.get_nondvr_active_network_ports( + fdb_network_ports = l2pop_db.get_nondvr_active_network_ports( self.ctx.session, 'network_id') self.assertEqual(1, len(fdb_network_ports)) _, agent = fdb_network_ports[0] @@ -109,6 +108,6 @@ class TestL2PopulationDBTestCase(testlib_api.SqlTestCase): # 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( + fdb_network_ports = l2pop_db.get_nondvr_active_network_ports( self.ctx.session, 'network_id') self.assertEqual(0, len(fdb_network_ports)) diff --git a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py index 9fb4282b50b..912ad92e803 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py @@ -97,8 +97,7 @@ class TestL2PopulationRpcTestCase(test_plugin.Ml2PluginV2TestCase): cast_patch = mock.patch(cast) self.mock_cast = cast_patch.start() - uptime = ('neutron.plugins.ml2.drivers.l2pop.db.L2populationDbMixin.' - 'get_agent_uptime') + uptime = ('neutron.plugins.ml2.drivers.l2pop.db.get_agent_uptime') uptime_patch = mock.patch(uptime, return_value=190) uptime_patch.start() @@ -833,8 +832,7 @@ class TestL2PopulationMechDriver(base.BaseTestCase): agent = mock.Mock() agent.host = HOST network_ports = ((None, agent),) - with mock.patch.object(l2pop_db.L2populationDbMixin, - 'get_agent_ip', + with mock.patch.object(l2pop_db, 'get_agent_ip', return_value=agent_ip): excluded_host = HOST + '-EXCLUDE' if exclude_host else HOST return mech_driver._get_tunnels(network_ports, excluded_host) @@ -860,14 +858,11 @@ class TestL2PopulationMechDriver(base.BaseTestCase): def agent_ip_side_effect(agent): return agent_ips[agent] - with mock.patch.object(l2pop_db.L2populationDbMixin, - 'get_agent_ip', + with mock.patch.object(l2pop_db, 'get_agent_ip', side_effect=agent_ip_side_effect),\ - mock.patch.object(l2pop_db.L2populationDbMixin, - 'get_nondvr_active_network_ports', + mock.patch.object(l2pop_db, 'get_nondvr_active_network_ports', return_value=fdb_network_ports),\ - mock.patch.object(l2pop_db.L2populationDbMixin, - 'get_dvr_active_network_ports', + mock.patch.object(l2pop_db, 'get_dvr_active_network_ports', return_value=tunnel_network_ports): session = mock.Mock() agent = mock.Mock()