From 3867174bc82b7fd85dd79bc0cc5625a15df2d8fb Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Wed, 18 Jun 2014 17:02:42 +0400 Subject: [PATCH] Fix auto_schedule_networks to resist DBDuplicateEntry This exception may happen if API and RPC workers are in different processes. Also make minor refactoring of auto_schedule_networks method to avoid unnecessary db queries. Add missing unit tests and adjust unit test naming style Change-Id: I6460744e2cffec0b9f009da071597374d8c027f6 Closes-Bug: #1331456 --- neutron/scheduler/dhcp_agent_scheduler.py | 25 ++++++----- neutron/tests/unit/test_dhcp_scheduler.py | 53 +++++++++++++++++++---- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/neutron/scheduler/dhcp_agent_scheduler.py b/neutron/scheduler/dhcp_agent_scheduler.py index 98ec5904e6f..ac9474338d6 100644 --- a/neutron/scheduler/dhcp_agent_scheduler.py +++ b/neutron/scheduler/dhcp_agent_scheduler.py @@ -99,7 +99,16 @@ class ChanceScheduler(object): the specified host. """ agents_per_network = cfg.CONF.dhcp_agents_per_network + # a list of (agent, net_ids) tuples + bindings_to_add = [] with context.session.begin(subtransactions=True): + fields = ['network_id', 'enable_dhcp'] + subnets = plugin.get_subnets(context, fields=fields) + net_ids = set(s['network_id'] for s in subnets + if s['enable_dhcp']) + if not net_ids: + LOG.debug(_('No non-hosted networks')) + return False query = context.session.query(agents_db.Agent) query = query.filter(agents_db.Agent.agent_type == constants.AGENT_TYPE_DHCP, @@ -111,13 +120,6 @@ class ChanceScheduler(object): dhcp_agent.heartbeat_timestamp): LOG.warn(_('DHCP agent %s is not active'), dhcp_agent.id) continue - fields = ['network_id', 'enable_dhcp'] - subnets = plugin.get_subnets(context, fields=fields) - net_ids = set(s['network_id'] for s in subnets - if s['enable_dhcp']) - if not net_ids: - LOG.debug(_('No non-hosted networks')) - return False for net_id in net_ids: agents = plugin.get_dhcp_agents_hosting_networks( context, [net_id], active=True) @@ -125,8 +127,9 @@ class ChanceScheduler(object): continue if any(dhcp_agent.id == agent.id for agent in agents): continue - binding = agentschedulers_db.NetworkDhcpAgentBinding() - binding.dhcp_agent = dhcp_agent - binding.network_id = net_id - context.session.add(binding) + bindings_to_add.append((dhcp_agent, net_id)) + # do it outside transaction so particular scheduling results don't + # make other to fail + for agent, net_id in bindings_to_add: + self._schedule_bind_network(context, [agent], net_id) return True diff --git a/neutron/tests/unit/test_dhcp_scheduler.py b/neutron/tests/unit/test_dhcp_scheduler.py index fd071f73aae..d0282bbc784 100644 --- a/neutron/tests/unit/test_dhcp_scheduler.py +++ b/neutron/tests/unit/test_dhcp_scheduler.py @@ -61,7 +61,7 @@ class DhcpSchedulerTestCase(base.BaseTestCase): with self.ctx.session.begin(subtransactions=True): self.ctx.session.add(models_v2.Network(id=network_id)) - def _test__schedule_bind_network(self, agents, network_id): + def _test_schedule_bind_network(self, agents, network_id): scheduler = dhcp_agent_scheduler.ChanceScheduler() scheduler._schedule_bind_network(self.ctx, agents, network_id) results = ( @@ -71,20 +71,57 @@ class DhcpSchedulerTestCase(base.BaseTestCase): for result in results: self.assertEqual(network_id, result.network_id) - def test__schedule_bind_network_single_agent(self): + def test_schedule_bind_network_single_agent(self): agents = self._get_agents(['host-a']) self._save_agents(agents) - self._test__schedule_bind_network(agents, self.network_id) + self._test_schedule_bind_network(agents, self.network_id) - def test__schedule_bind_network_multi_agents(self): + def test_schedule_bind_network_multi_agents(self): agents = self._get_agents(['host-a', 'host-b']) self._save_agents(agents) - self._test__schedule_bind_network(agents, self.network_id) + self._test_schedule_bind_network(agents, self.network_id) - def test__schedule_bind_network_multi_agent_fail_one(self): + def test_schedule_bind_network_multi_agent_fail_one(self): agents = self._get_agents(['host-a']) self._save_agents(agents) - self._test__schedule_bind_network(agents, self.network_id) + self._test_schedule_bind_network(agents, self.network_id) with mock.patch.object(dhcp_agent_scheduler.LOG, 'info') as fake_log: - self._test__schedule_bind_network(agents, self.network_id) + self._test_schedule_bind_network(agents, self.network_id) self.assertEqual(1, fake_log.call_count) + + def test_auto_schedule_networks_no_networks(self): + plugin = mock.MagicMock() + plugin.get_networks.return_value = [] + scheduler = dhcp_agent_scheduler.ChanceScheduler() + self.assertFalse(scheduler.auto_schedule_networks(plugin, + self.ctx, "host-a")) + + def test_auto_schedule_networks(self): + plugin = mock.MagicMock() + plugin.get_subnets.return_value = [{"network_id": self.network_id, + "enable_dhcp": True}] + agents = self._get_agents(['host-a']) + self._save_agents(agents) + scheduler = dhcp_agent_scheduler.ChanceScheduler() + + self.assertTrue(scheduler.auto_schedule_networks(plugin, + self.ctx, "host-a")) + results = ( + self.ctx.session.query(agentschedulers_db.NetworkDhcpAgentBinding) + .all()) + self.assertEqual(1, len(results)) + + def test_auto_schedule_networks_network_already_scheduled(self): + plugin = mock.MagicMock() + plugin.get_subnets.return_value = [{"network_id": self.network_id, + "enable_dhcp": True}] + agents = self._get_agents(['host-a']) + self._save_agents(agents) + scheduler = dhcp_agent_scheduler.ChanceScheduler() + self._test_schedule_bind_network(agents, self.network_id) + self.assertTrue(scheduler.auto_schedule_networks(plugin, + self.ctx, "host-a")) + results = ( + self.ctx.session.query(agentschedulers_db.NetworkDhcpAgentBinding) + .all()) + self.assertEqual(1, len(results))