Do not count dvr agents while creating HA ports

Presently dvr agents are not excluded when a
ha router interface is created, because of which
an interface is created even in the dvr agent namespace.

This patch fixes the issue by supporting a filter - 'agent_modes'
in the function L3AgentSchedulerDbMixin.get_l3_agents.
If this filter is defined, only the l3 agents whose modes
defined in this filter are returned

Change-Id: I9383e2a30ddc4f2c3e04ff0821726b8dc3ec89a0
Closes-bug: #1397209

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index 4284346a9a7a342c017159fe2eb58abf2275429e..104c4f6024a163780b205e739ea13061e8523959 100644 (file)
--- a/neutron/db/l3_agentschedulers_db.py
+++ b/neutron/db/l3_agentschedulers_db.py
@@ -22,6 +22,7 @@ from oslo import messaging
 from oslo.utils import timeutils
 import sqlalchemy as sa
 from sqlalchemy import func
+from sqlalchemy import or_
 from sqlalchemy import orm
 from sqlalchemy.orm import exc
 from sqlalchemy.orm import joinedload
@@ -382,6 +383,15 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
                 if column:
                     query = query.filter(column.in_(value))
 
+            agent_modes = filters.get('agent_modes', [])
+            if agent_modes:
+                agent_mode_key = '\"agent_mode\": \"'
+                configuration_filter = (
+                    [agents_db.Agent.configurations.contains('%s%s\"' %
+                     (agent_mode_key, agent_mode))
+                     for agent_mode in agent_modes])
+                query = query.filter(or_(*configuration_filter))
+
         return [l3_agent
                 for l3_agent in query
                 if agentschedulers_db.AgentSchedulerDbMixin.is_eligible_agent(

diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py
index d5e343e6e90b17dbd7bb3249456f2ed1e1e4b2fd..d3215bb47c02773882633663127d8e6d65239baf 100644 (file)
--- a/neutron/db/l3_hamode_db.py
+++ b/neutron/db/l3_hamode_db.py
@@ -255,7 +255,8 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin):
         """
 
         min_agents = cfg.CONF.min_l3_agents_per_router
-        num_agents = len(self.get_l3_agents(context))
+        num_agents = len(self.get_l3_agents(context,
+            filters={'agent_modes': ['legacy', 'dvr_snat']}))
         max_agents = cfg.CONF.max_l3_agents_per_router
         if max_agents:
             if max_agents > num_agents:

diff --git a/neutron/tests/unit/db/test_l3_ha_db.py b/neutron/tests/unit/db/test_l3_ha_db.py
index 5084a78507ce3843f8dc6511ceaf886a77fffd9a..c8625c06232db2ef64699168d9b1e8ba74518dfc 100644 (file)
--- a/neutron/tests/unit/db/test_l3_ha_db.py
+++ b/neutron/tests/unit/db/test_l3_ha_db.py
@@ -58,11 +58,13 @@ class L3HATestFramework(testlib_api.SqlTestCase,
             'agent_type': constants.AGENT_TYPE_L3,
             'binary': 'neutron-l3-agent',
             'host': 'l3host',
-            'topic': 'N/A'
+            'topic': 'N/A',
+            'configurations': {'agent_mode': 'legacy'}
         }
 
         self.plugin.create_or_update_agent(self.admin_ctx, agent_status)
         agent_status['host'] = 'l3host_2'
+        agent_status['configurations'] = {'agent_mode': 'dvr_snat'}
         self.plugin.create_or_update_agent(self.admin_ctx, agent_status)
         self.agent1, self.agent2 = self.plugin.get_agents(self.admin_ctx)
 
@@ -395,6 +397,27 @@ class L3HATestCase(L3HATestFramework):
         routers_after = self.plugin.get_routers(self.admin_ctx)
         self.assertEqual(routers_before, routers_after)
 
+    def test_exclude_dvr_agents_for_ha_candidates(self):
+        """Test dvr agents are not counted in the ha candidates.
+
+        This test case tests that when get_number_of_agents_for_scheduling
+        is called, it doesn't count dvr agents.
+        """
+        # Test setup registers two l3 agents.
+        # Register another l3 agent with dvr mode and assert that
+        # get_number_of_ha_agent_candidates return 2.
+        dvr_agent_status = {
+            'agent_type': constants.AGENT_TYPE_L3,
+            'binary': 'neutron-l3-agent',
+            'host': 'l3host_3',
+            'topic': 'N/A',
+            'configurations': {'agent_mode': 'dvr'}
+        }
+        self.plugin.create_or_update_agent(self.admin_ctx, dvr_agent_status)
+        num_ha_candidates = self.plugin.get_number_of_agents_for_scheduling(
+            self.admin_ctx)
+        self.assertEqual(2, num_ha_candidates)
+
 
 class L3HAUserTestCase(L3HATestFramework):
 

diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py
index 3f0e3e649af0784863b2fe253e9628d9ea5573ff..9e6034251116cbe6dd948ca6f9458131056729ee 100644 (file)
--- a/neutron/tests/unit/test_l3_schedulers.py
+++ b/neutron/tests/unit/test_l3_schedulers.py
@@ -18,6 +18,8 @@ import datetime
 import uuid
 
 import mock
+import testscenarios
+
 from oslo.config import cfg
 from oslo.db import exception as db_exc
 from oslo.utils import importutils
@@ -44,45 +46,13 @@ from neutron.tests.unit import test_l3_plugin
 from neutron.tests.unit import testlib_api
 from neutron.tests.unit import testlib_plugin
 
-HOST = 'my_l3_host'
-FIRST_L3_AGENT = {
-    'binary': 'neutron-l3-agent',
-    'host': HOST,
-    'topic': topics.L3_AGENT,
-    'configurations': {},
-    'agent_type': constants.AGENT_TYPE_L3,
-    'start_flag': True
-}
-
-HOST_2 = 'my_l3_host_2'
-SECOND_L3_AGENT = {
-    'binary': 'neutron-l3-agent',
-    'host': HOST_2,
-    'topic': topics.L3_AGENT,
-    'configurations': {},
-    'agent_type': constants.AGENT_TYPE_L3,
-    'start_flag': True
-}
-
-HOST_3 = 'my_l3_host_3'
-THIRD_L3_AGENT = {
-    'binary': 'neutron-l3-agent',
-    'host': HOST_3,
-    'topic': topics.L3_AGENT,
-    'configurations': {},
-    'agent_type': constants.AGENT_TYPE_L3,
-    'start_flag': True
-}
-
-HOST_4 = 'my_l3_host_4'
-FOURTH_L3_AGENT = {
-    'binary': 'neutron-l3-agent',
-    'host': HOST_4,
-    'topic': topics.L3_AGENT,
-    'configurations': {},
-    'agent_type': constants.AGENT_TYPE_L3,
-    'start_flag': True
-}
+# the below code is required for the following reason
+# (as documented in testscenarios)
+"""Multiply tests depending on their 'scenarios' attribute.
+   This can be assigned to 'load_tests' in any test module to make this
+   automatically work across tests in the module.
+"""
+load_tests = testscenarios.load_tests_apply_scenarios
 
 HOST_DVR = 'my_l3_host_dvr'
 DVR_L3_AGENT = {
@@ -267,10 +237,18 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
 
 class L3SchedulerBaseMixin(object):
 
-    def _register_l3_agent(self, agent, plugin=None):
+    def _register_l3_agent(self, host, agent_mode='legacy', plugin=None):
         if not plugin:
             plugin = self.plugin
 
+        agent = {
+            'binary': 'neutron-l3-agent',
+            'host': host,
+            'topic': topics.L3_AGENT,
+            'configurations': {'agent_mode': agent_mode},
+            'agent_type': constants.AGENT_TYPE_L3,
+            'start_flag': True
+        }
         callback = agents_db.AgentExtRpcCallback()
         callback.report_state(self.adminContext,
                               agent_state={'agent_state': agent},
@@ -280,10 +258,10 @@ class L3SchedulerBaseMixin(object):
         return agent_db[0]
 
     def _register_l3_agents(self, plugin=None):
-        self.agent1 = self._register_l3_agent(FIRST_L3_AGENT, plugin)
+        self.agent1 = self._register_l3_agent('host_1', plugin=plugin)
         self.agent_id1 = self.agent1.id
 
-        self.agent2 = self._register_l3_agent(SECOND_L3_AGENT, plugin)
+        self.agent2 = self._register_l3_agent('host_2', plugin=plugin)
         self.agent_id2 = self.agent2.id
 
     def _register_l3_dvr_agents(self):
@@ -597,7 +575,7 @@ class L3SchedulerTestBaseMixin(object):
 
         # test legacy agent_mode case: only legacy agent should be candidate
         router['distributed'] = False
-        exp_host = FIRST_L3_AGENT.get('host')
+        exp_host = 'host_1'
         self._check_get_l3_agent_candidates(router, agent_list, exp_host)
 
     def test_get_l3_agent_candidates_dvr(self):
@@ -676,7 +654,7 @@ class L3SchedulerTestBaseMixin(object):
     def _prepare_check_ports_exist_tests(self):
         l3_agent = agents_db.Agent()
         l3_agent.admin_state_up = True
-        l3_agent.host = HOST
+        l3_agent.host = 'host_1'
         router = self._make_router(self.fmt,
                                    tenant_id=str(uuid.uuid4()),
                                    name='r2')
@@ -714,7 +692,7 @@ class L3SchedulerTestBaseMixin(object):
             getp.return_value = self.plugin
             # matching subnet
             port = {'subnet_id': str(uuid.uuid4()),
-                    'binding:host_id': HOST,
+                    'binding:host_id': 'host_1',
                     'device_owner': 'compute:',
                     'id': 1234}
             self.plugin.get_ports.return_value = [port]
@@ -796,13 +774,13 @@ class L3AgentChanceSchedulerTestCase(L3SchedulerTestCase):
                 self._set_l3_agent_admin_state(self.adminContext,
                                                self.agent_id1, True)
                 self.plugin.auto_schedule_routers(self.adminContext,
-                                                  FIRST_L3_AGENT['host'],
+                                                  'host_1',
                                                   [r1['router']['id']])
 
                 agents = self.get_l3_agents_hosting_routers(
                     self.adminContext, [r1['router']['id']],
                     admin_state_up=True)
-                self.assertEqual(FIRST_L3_AGENT['host'], agents[0]['host'])
+                self.assertEqual('host_1', agents[0]['host'])
 
 
 class L3AgentLeastRoutersSchedulerTestCase(L3SchedulerTestCase):
@@ -1168,10 +1146,10 @@ class L3_HA_scheduler_db_mixinTestCase(L3HATestCaseMixin):
         super(L3_HA_scheduler_db_mixinTestCase,
               self)._register_l3_agents(plugin=plugin)
 
-        self.agent3 = self._register_l3_agent(THIRD_L3_AGENT, plugin)
+        self.agent3 = self._register_l3_agent('host_3', plugin=plugin)
         self.agent_id3 = self.agent3.id
 
-        self.agent4 = self._register_l3_agent(FOURTH_L3_AGENT, plugin)
+        self.agent4 = self._register_l3_agent('host_4', plugin=plugin)
         self.agent_id4 = self.agent4.id
 
     def test_get_ha_routers_l3_agents_count(self):
@@ -1275,11 +1253,11 @@ class L3HAChanceSchedulerTestCase(L3HATestCaseMixin):
         self.assertIn(self.agent_id1, agent_ids)
         self.assertIn(self.agent_id2, agent_ids)
 
-        agent = self._register_l3_agent(THIRD_L3_AGENT)
+        agent = self._register_l3_agent('host_3')
         self.agent_id3 = agent.id
         routers_to_auto_schedule = [router['id']] if specific_router else []
         self.plugin.auto_schedule_routers(self.adminContext,
-                                          THIRD_L3_AGENT['host'],
+                                          'host_3',
                                           routers_to_auto_schedule)
 
         agents = self.plugin.get_l3_agents_hosting_routers(
@@ -1289,7 +1267,7 @@ class L3HAChanceSchedulerTestCase(L3HATestCaseMixin):
 
         # Simulate agent restart to make sure we don't try to re-bind
         self.plugin.auto_schedule_routers(self.adminContext,
-                                          THIRD_L3_AGENT['host'],
+                                          'host_3',
                                           routers_to_auto_schedule)
 
     def test_scheduler_with_ha_enabled_not_enough_agent(self):
@@ -1320,10 +1298,10 @@ class L3HALeastRoutersSchedulerTestCase(L3HATestCaseMixin):
         super(L3HALeastRoutersSchedulerTestCase,
               self)._register_l3_agents(plugin=plugin)
 
-        agent = self._register_l3_agent(THIRD_L3_AGENT, plugin)
+        agent = self._register_l3_agent('host_3', plugin=plugin)
         self.agent_id3 = agent.id
 
-        agent = self._register_l3_agent(FOURTH_L3_AGENT, plugin)
+        agent = self._register_l3_agent('host_4', plugin=plugin)
         self.agent_id4 = agent.id
 
     def setUp(self):
@@ -1366,3 +1344,78 @@ class L3HALeastRoutersSchedulerTestCase(L3HATestCaseMixin):
         agent_ids = [agent['id'] for agent in agents]
         self.assertIn(self.agent_id3, agent_ids)
         self.assertIn(self.agent_id4, agent_ids)
+
+
+class TestGetL3AgentsWithAgentModeFilter(testlib_api.SqlTestCase,
+                                         testlib_plugin.PluginSetupHelper,
+                                         L3SchedulerBaseMixin):
+    """Test cases to test get_l3_agents.
+
+    This class tests the L3AgentSchedulerDbMixin.get_l3_agents()
+    for the 'agent_mode' filter with various values.
+
+    5 l3 agents are registered in the order - legacy, dvr_snat, dvr, fake_mode
+    and legacy
+    """
+
+    scenarios = [
+        ('no filter',
+            dict(agent_modes=[],
+                 expected_agent_modes=['legacy', 'dvr_snat', 'dvr',
+                                       'fake_mode', 'legacy'])),
+
+        ('legacy',
+            dict(agent_modes=['legacy'],
+                 expected_agent_modes=['legacy', 'legacy'])),
+
+        ('dvr_snat',
+            dict(agent_modes=['dvr_snat'],
+                 expected_agent_modes=['dvr_snat'])),
+
+        ('dvr ',
+            dict(agent_modes=['dvr'],
+                 expected_agent_modes=['dvr'])),
+
+        ('legacy and dvr snat',
+            dict(agent_modes=['legacy', 'dvr_snat', 'legacy'],
+                 expected_agent_modes=['legacy', 'dvr_snat', 'legacy'])),
+
+        ('legacy and dvr',
+            dict(agent_modes=['legacy', 'dvr'],
+                 expected_agent_modes=['legacy', 'dvr', 'legacy'])),
+
+        ('dvr_snat and dvr',
+            dict(agent_modes=['dvr_snat', 'dvr'],
+                 expected_agent_modes=['dvr_snat', 'dvr'])),
+
+        ('legacy, dvr_snat and dvr',
+            dict(agent_modes=['legacy', 'dvr_snat', 'dvr'],
+                 expected_agent_modes=['legacy', 'dvr_snat', 'dvr',
+                                       'legacy'])),
+
+        ('invalid',
+            dict(agent_modes=['invalid'],
+                 expected_agent_modes=[])),
+    ]
+
+    def setUp(self):
+        super(TestGetL3AgentsWithAgentModeFilter, self).setUp()
+        self.plugin = L3HAPlugin()
+        self.setup_coreplugin('neutron.plugins.ml2.plugin.Ml2Plugin')
+        self.adminContext = q_context.get_admin_context()
+        hosts = ['host_1', 'host_2', 'host_3', 'host_4', 'host_5']
+        agent_modes = ['legacy', 'dvr_snat', 'dvr', 'fake_mode', 'legacy']
+        for host, agent_mode in zip(hosts, agent_modes):
+            self._register_l3_agent(host, agent_mode, self.plugin)
+
+    def _get_agent_mode(self, agent):
+        agent_conf = self.plugin.get_configuration_dict(agent)
+        return agent_conf.get('agent_mode', 'None')
+
+    def test_get_l3_agents(self):
+        l3_agents = self.plugin.get_l3_agents(
+            self.adminContext, filters={'agent_modes': self.agent_modes})
+        self.assertEqual(len(self.expected_agent_modes), len(l3_agents))
+        returned_agent_modes = [self._get_agent_mode(agent)
+                                for agent in l3_agents]
+        self.assertEqual(self.expected_agent_modes, returned_agent_modes)