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

diff --git a/neutron/scheduler/dhcp_agent_scheduler.py b/neutron/scheduler/dhcp_agent_scheduler.py
index 98ec5904e6f25c77ab7c9368f4362683474dc217..ac9474338d65cd989960b902536fc5ac2f7963eb 100644 (file)
--- 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 fd071f73aae7109ebb706b2e3c115e030ea6d9d5..d0282bbc784daaf351eeac5175fb8a8c4bcb4867 100644 (file)
--- 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))