From 56eb877d250794a38bcf4f6b94aee1add4723ae0 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Wed, 9 Dec 2015 17:58:14 +0300 Subject: [PATCH] Remove duplicate for check_ports_exist_on_l3agent This removes check_ports_on_host_and_subnet which mostly duplicates what check_ports_exist_on_l3agent is doing. Also rename check_ports_exist_on_l3agent to check_dvr_serviceable_ports_on_host for more clarity. Closes-Bug: #1524291 Change-Id: Ie02c68279c2bbafffc7be4d9a81fe25a0e983d58 --- neutron/db/l3_agentschedulers_db.py | 22 ++- neutron/db/l3_dvr_db.py | 4 +- neutron/db/l3_dvrscheduler_db.py | 28 +--- .../tests/unit/db/test_agentschedulers_db.py | 4 +- neutron/tests/unit/db/test_l3_dvr_db.py | 4 +- .../unit/scheduler/test_l3_agent_scheduler.py | 131 +++--------------- 6 files changed, 42 insertions(+), 151 deletions(-) diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index d6ab00ead..30eb39389 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -456,10 +456,16 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, if agentschedulers_db.AgentSchedulerDbMixin.is_eligible_agent( active, l3_agent)] - def check_ports_exist_on_l3agent(self, context, l3_agent, subnet_ids): - """ - This function checks for existence of dvr serviceable - ports on the host, running the input l3agent. + def check_dvr_serviceable_ports_on_host( + self, context, host, subnet_ids, except_port=None): + """Check for existence of dvr serviceable ports on host + + :param context: request context + :param host: host to look ports on + :param subnet_ids: IDs of subnets to look ports on + :param except_port: ID of the port to ignore (used when checking if + DVR router should be removed from host before actual port remove) + :return: """ # db query will return ports for all subnets if subnet_ids is empty, # so need to check first @@ -468,13 +474,15 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, core_plugin = manager.NeutronManager.get_plugin() filters = {'fixed_ips': {'subnet_id': subnet_ids}, - portbindings.HOST_ID: [l3_agent['host']]} + portbindings.HOST_ID: [host]} ports_query = core_plugin._get_ports_query(context, filters=filters) owner_filter = or_( models_v2.Port.device_owner.startswith( constants.DEVICE_OWNER_COMPUTE_PREFIX), models_v2.Port.device_owner.in_( n_utils.get_other_dvr_serviced_device_owners())) + if except_port: + ports_query = ports_query.filter(models_v2.Port.id != except_port) ports_query = ports_query.filter(owner_filter) return ports_query.first() is not None @@ -514,8 +522,8 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, candidates.append(l3_agent) elif (is_router_distributed and subnet_ids and agent_mode.startswith(constants.L3_AGENT_MODE_DVR) and ( - self.check_ports_exist_on_l3agent( - context, l3_agent, subnet_ids))): + self.check_dvr_serviceable_ports_on_host( + context, l3_agent['host'], subnet_ids))): candidates.append(l3_agent) return candidates diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index e77d665f0..ccb759508 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -369,8 +369,8 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, context, router['id']) if subnet_ids: for l3_agent in l3_agents: - if not plugin.check_ports_exist_on_l3agent( - context, l3_agent, subnet_ids): + if not plugin.check_dvr_serviceable_ports_on_host( + context, l3_agent['host'], subnet_ids): plugin.remove_router_from_l3_agent( context, l3_agent['id'], router['id']) router_interface_info = self._make_router_interface_info( diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index fdce6b0b8..497546725 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -156,22 +156,6 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin): 'for router %s', router_id) return subnet_ids - def check_ports_on_host_and_subnet(self, context, host, - port_id, subnet_id): - """Check if there are any dvr service ports on the subnet_id.""" - filter_sub = {'fixed_ips': {'subnet_id': [subnet_id]}} - ports = self._core_plugin.get_ports(context, filters=filter_sub) - for port in ports: - if (n_utils.is_dvr_serviced(port['device_owner']) - and port[portbindings.HOST_ID] == host - and port['id'] != port_id): - LOG.debug('DVR: %(port_status)s port exists for subnet ' - '%(subnet_id)s on host %(host)s', - {'port_status': port['status'], - 'subnet_id': subnet_id, 'host': host}) - return True - return False - def dvr_deletens_if_no_port(self, context, port_id, port_host=None): """Delete the DVR namespace if no dvr serviced port exists.""" admin_context = context.elevated() @@ -192,16 +176,8 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin): for router_id in router_ids: subnet_ids = self.get_subnet_ids_on_router(admin_context, router_id) - port_exists_on_subnet = False - for subnet in subnet_ids: - if self.check_ports_on_host_and_subnet(admin_context, - port_host, - port_id, - subnet): - port_exists_on_subnet = True - break - - if port_exists_on_subnet: + if self.check_dvr_serviceable_ports_on_host( + admin_context, port_host, subnet_ids, except_port=port_id): continue filter_rtr = {'device_id': [router_id], 'device_owner': diff --git a/neutron/tests/unit/db/test_agentschedulers_db.py b/neutron/tests/unit/db/test_agentschedulers_db.py index 078ef145e..c878422e6 100644 --- a/neutron/tests/unit/db/test_agentschedulers_db.py +++ b/neutron/tests/unit/db/test_agentschedulers_db.py @@ -778,7 +778,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase): with self.subnet() as s, \ mock.patch.object( self.l3plugin, - 'check_ports_exist_on_l3agent') as port_exists: + 'check_dvr_serviceable_ports_on_host') as port_exists: net_id = s['subnet']['network_id'] self._set_net_external(net_id) router = {'name': 'router1', @@ -1095,7 +1095,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase): {'router': router}) with mock.patch.object( self.l3plugin, - 'check_ports_exist_on_l3agent') as ports_exist: + 'check_dvr_serviceable_ports_on_host') as ports_exist: # emulating dvr serviceable ports exist on compute node ports_exist.return_value = True self.l3plugin.schedule_router( diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 6954f8e70..6dc0df3bf 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -486,7 +486,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): return_value=[mock.MagicMock()]) plugin.get_subnet_ids_on_router = mock.Mock( return_value=interface_info) - plugin.check_ports_exist_on_l3agent = mock.Mock( + plugin.check_dvr_serviceable_ports_on_host = mock.Mock( return_value=False) plugin.remove_router_from_l3_agent = mock.Mock( return_value=None) @@ -515,7 +515,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): self.mixin.remove_router_interface( self.ctx, mock.Mock(), interface_info) self.assertTrue(plugin.get_l3_agents_hosting_routers.called) - self.assertTrue(plugin.check_ports_exist_on_l3agent.called) + self.assertTrue(plugin.check_dvr_serviceable_ports_on_host.called) self.assertTrue(plugin.remove_router_from_l3_agent.called) def test_remove_router_interface_csnat_ports_removal(self): diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index 75beca526..80a5db52b 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py @@ -646,7 +646,7 @@ class L3SchedulerTestBaseMixin(object): # test dvr agent_mode case only dvr agent should be candidate router['distributed'] = True self.get_subnet_ids_on_router = mock.Mock() - self.check_ports_exist_on_l3agent = mock.Mock(return_value=True) + self.check_dvr_serviceable_ports_on_host = mock.Mock(return_value=True) self._check_get_l3_agent_candidates(router, agent_list, HOST_DVR) def test_get_l3_agent_candidates_dvr_no_vms(self): @@ -660,7 +660,8 @@ class L3SchedulerTestBaseMixin(object): router['distributed'] = True # Test no VMs present case self.get_subnet_ids_on_router = mock.Mock() - self.check_ports_exist_on_l3agent = mock.Mock(return_value=False) + self.check_dvr_serviceable_ports_on_host = mock.Mock( + return_value=False) self._check_get_l3_agent_candidates( router, agent_list, HOST_DVR, count=0) @@ -675,7 +676,7 @@ class L3SchedulerTestBaseMixin(object): agent_list = [self.l3_dvr_snat_agent] self.get_subnet_ids_on_router = mock.Mock() - self.check_ports_exist_on_l3agent = mock.Mock(return_value=True) + self.check_dvr_serviceable_ports_on_host = mock.Mock(return_value=True) self._check_get_l3_agent_candidates(router, agent_list, HOST_DVR_SNAT) def test_get_l3_agent_candidates_dvr_snat_no_vms(self): @@ -688,10 +689,11 @@ class L3SchedulerTestBaseMixin(object): router['distributed'] = True agent_list = [self.l3_dvr_snat_agent] - self.check_ports_exist_on_l3agent = mock.Mock(return_value=False) + self.check_dvr_serviceable_ports_on_host = mock.Mock( + return_value=False) # Test no VMs present case self.get_subnet_ids_on_router = mock.Mock() - self.check_ports_exist_on_l3agent.return_value = False + self.check_dvr_serviceable_ports_on_host.return_value = False self._check_get_l3_agent_candidates( router, agent_list, HOST_DVR_SNAT, count=0) @@ -1229,85 +1231,6 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): r1['id']) self.assertEqual(len(sub_ids), 0) - def _test_check_ports_on_host_and_subnet_base(self, port_status): - dvr_port = { - 'id': 'fake_id', - 'device_id': 'r1', - 'status': port_status, - portbindings.HOST_ID: 'thisHost', - 'device_owner': DEVICE_OWNER_COMPUTE_NOVA, - 'fixed_ips': [ - { - 'subnet_id': '80947d4a-fbc8-484b-9f92-623a6bfcf3e0', - 'ip_address': '10.10.10.1' - } - ] - } - r1 = { - 'id': 'r1', - 'distributed': True, - } - with mock.patch( - 'neutron.db.db_base_plugin_v2.NeutronDbPluginV2' '.get_ports', - return_value=[dvr_port]),\ - mock.patch( - 'neutron.manager.NeutronManager.get_service_plugins', - return_value=mock.Mock()),\ - mock.patch('neutron.db.l3_db.L3_NAT_db_mixin.get_router', - return_value=r1),\ - mock.patch('neutron.api.rpc.agentnotifiers.l3_rpc_agent_api' - '.L3AgentNotifyAPI'): - sub_ids = self.dut.get_subnet_ids_on_router(self.adminContext, - r1['id']) - result = self.dut.check_ports_on_host_and_subnet( - self.adminContext, - 'thisHost', 'dvr_port1', - sub_ids) - self.assertTrue(result) - - def test_check_ports_on_host_and_subnet_with_active_port(self): - self._test_check_ports_on_host_and_subnet_base('ACTIVE') - - def test_check_ports_on_host_and_subnet_with_build_port(self): - self._test_check_ports_on_host_and_subnet_base('BUILD') - - def test_check_ports_on_host_and_subnet_with_down_port(self): - self._test_check_ports_on_host_and_subnet_base('DOWN') - - def _test_dvr_serviced_port_exists_on_subnet(self, port): - with mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.' - 'get_ports', return_value=[port]): - result = self.dut.check_ports_on_host_and_subnet( - self.adminContext, - 'thisHost', - 'dvr1-intf-id', - 'my-subnet-id') - self.assertTrue(result) - - def _test_dvr_serviced_vip_port_exists_on_subnet(self, device_owner): - vip_port = { - 'id': 'lbaas-vip-port1', - 'device_id': 'vip-pool-id', - 'status': 'ACTIVE', - portbindings.HOST_ID: 'thisHost', - 'device_owner': device_owner, - 'fixed_ips': [ - { - 'subnet_id': 'my-subnet-id', - 'ip_address': '10.10.10.1' - } - ] - } - self._test_dvr_serviced_port_exists_on_subnet(port=vip_port) - - def test_dvr_serviced_lbaas_vip_port_exists_on_subnet(self): - self._test_dvr_serviced_vip_port_exists_on_subnet( - device_owner=constants.DEVICE_OWNER_LOADBALANCER) - - def test_dvr_serviced_lbaasv2_vip_port_exists_on_subnet(self): - self._test_dvr_serviced_vip_port_exists_on_subnet( - device_owner=constants.DEVICE_OWNER_LOADBALANCERV2) - def _create_port(self, port_name, tenant_id, host, subnet_id, ip_address, status='ACTIVE', device_owner=DEVICE_OWNER_COMPUTE_NOVA): @@ -1375,11 +1298,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): status='INACTIVE') deleted_vm_port_id = deleted_vm_port['id'] - running_vm_port = self._create_port( - 'running-vn', 'tenant-2', vm_port_host, - shared_subnet_id, '10.10.10.33') - - fakePortDB = FakePortDB([running_vm_port, deleted_vm_port, dvr_port]) + fakePortDB = FakePortDB([deleted_vm_port, dvr_port]) vm_port_binding = { 'port_id': deleted_vm_port_id, @@ -1397,10 +1316,15 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.' 'get_ports', side_effect=fakePortDB.get_ports) as\ mock_get_ports,\ + mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.' + '_get_ports_query'),\ mock.patch('neutron.plugins.ml2.db.' 'get_dvr_port_binding_by_host', return_value=vm_port_binding) as\ - mock_get_dvr_port_binding_by_host: + mock_get_dvr_port_binding_by_host,\ + mock.patch.object(self.dut, + 'check_dvr_serviceable_ports_on_host', + return_value=True): routers = self.dut.dvr_deletens_if_no_port( my_context, deleted_vm_port_id) @@ -1437,11 +1361,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): status='INACTIVE') deleted_vm_port_id = deleted_vm_port['id'] - running_vm_port = self._create_port( - 'running-vn', vm_tenant, 'compute-node-2', - shared_subnet_id, '10.10.10.33') - - fakePortDB = FakePortDB([running_vm_port, dvr_port, deleted_vm_port]) + fakePortDB = FakePortDB([dvr_port, deleted_vm_port]) dvr_port_binding = { 'port_id': dvr_port_id, 'host': vm_port_host @@ -1469,7 +1389,10 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): mock_get_dvr_port_binding_by_host,\ mock.patch('neutron.db.agents_db.AgentDbMixin.' '_get_agent_by_type_and_host', - return_value=l3_agent_on_vm_host): + return_value=l3_agent_on_vm_host),\ + mock.patch.object(self.dut, + 'check_dvr_serviceable_ports_on_host', + return_value=False): routers = self.dut.dvr_deletens_if_no_port( my_context, deleted_vm_port_id) @@ -1499,22 +1422,6 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): self._test_dvr_deletens_if_no_ports_delete_routers( 'tenant-1', 'tenant-1') - def test_dvr_serviced_dhcp_port_exists_on_subnet(self): - dhcp_port = { - 'id': 'dhcp-port1', - 'device_id': 'dhcp-net-id', - 'status': 'ACTIVE', - portbindings.HOST_ID: 'thisHost', - 'device_owner': constants.DEVICE_OWNER_DHCP, - 'fixed_ips': [ - { - 'subnet_id': 'my-subnet-id', - 'ip_address': '10.10.10.2' - } - ] - } - self._test_dvr_serviced_port_exists_on_subnet(port=dhcp_port) - def _prepare_schedule_snat_tests(self): agent = agents_db.Agent() agent.admin_state_up = True -- 2.45.2