From: Swaminathan Vasudevan Date: Thu, 19 Feb 2015 00:10:25 +0000 (-0800) Subject: Create/Delete FIP Agent gateway port only if DVR Routers X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fe4fc0ebe21343b4fc7b4238674d80d692dded5d;p=openstack-build%2Fneutron-build.git Create/Delete FIP Agent gateway port only if DVR Routers A recent patch for creating FloatingIP Agent gateway port without RPC dependency was trying to create FloatingIP Agent Gateway port for both Legacy and DVR routers. "Change-id: Ieaa79c8bf2b1e03bc352f9252ce22286703e3715" This patch fixes the problem by checking for the router, before it tries to create or delete a FloatingIP Agent Gateway Port. Change-Id: I588b518d2fffd3943abe6d70fb0308adbe46e974 Closes-Bug: #1423422 --- diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 226b30e34..6b76d479d 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -199,26 +199,40 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, floatingIP disassociation happens. """ fip_port = fip.get('port_id') - if fip_port and floatingip_db['id']: - vm_hostid = self.get_vm_port_hostid( - context, fip_port) - if vm_hostid: - # FIXME (Swami): This FIP Agent Gateway port should be - # created only once and there should not be a duplicate - # for the same host. Until we find a good solution for - # augmenting multiple server requests we should use the - # existing flow. - fip_agent_port = self.create_fip_agent_gw_port_if_not_exists( - context.elevated(), external_port['network_id'], vm_hostid) - LOG.debug("FIP Agent gateway port: %s", fip_agent_port) unused_fip_agent_gw_port = ( fip_port is None and floatingip_db['fixed_port_id']) - if unused_fip_agent_gw_port: + if unused_fip_agent_gw_port and floatingip_db.get('router_id'): admin_ctx = context.elevated() - self.clear_unused_fip_agent_gw_port( - admin_ctx, floatingip_db) + router_dict = self.get_router( + admin_ctx, floatingip_db['router_id']) + # Check if distributed router and then delete the + # FloatingIP agent gateway port + if router_dict.get('distributed'): + self.clear_unused_fip_agent_gw_port( + admin_ctx, floatingip_db) super(L3_NAT_with_dvr_db_mixin, self)._update_fip_assoc( context, fip, floatingip_db, external_port) + associate_fip = fip_port and floatingip_db['id'] + if associate_fip and floatingip_db.get('router_id'): + admin_ctx = context.elevated() + router_dict = self.get_router( + admin_ctx, floatingip_db['router_id']) + # Check if distributed router and then create the + # FloatingIP agent gateway port + if router_dict.get('distributed'): + vm_hostid = self.get_vm_port_hostid( + context, fip_port) + if vm_hostid: + # FIXME (Swami): This FIP Agent Gateway port should be + # created only once and there should not be a duplicate + # for the same host. Until we find a good solution for + # augmenting multiple server requests we should use the + # existing flow. + fip_agent_port = ( + self.create_fip_agent_gw_port_if_not_exists( + admin_ctx, external_port['network_id'], + vm_hostid)) + LOG.debug("FIP Agent gateway port: %s", fip_agent_port) def clear_unused_fip_agent_gw_port( self, context, floatingip_db): diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 08cef8f6f..65d908935 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -342,32 +342,33 @@ class L3DvrTestCase(testlib_api.SqlTestCase): floatingip = { 'id': _uuid(), 'fixed_port_id': 1234, + 'router_id': 'foo_router_id' } + router = {'id': 'foo_router_id', 'distributed': True} with contextlib.nested( + mock.patch.object(self.mixin, + 'get_router'), mock.patch.object(self.mixin, 'clear_unused_fip_agent_gw_port'), mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, '_update_fip_assoc'), - ) as (vf, cf): + ) as (grtr, vf, cf): + grtr.return_value = router self.mixin._update_fip_assoc( self.ctx, fip, floatingip, mock.ANY) self.assertTrue(vf.called) - def test_create_floatingip_agent_gw_port(self): - fip = { - 'id': _uuid(), - 'port_id': _uuid(), - } - floatingip = { - 'id': _uuid(), - 'fixed_port_id': 1234, - } + def _setup_test_create_delete_floatingip( + self, fip, floatingip_db, router_db): port = { - 'id': _uuid(), + 'id': '1234', 'binding:host_id': 'myhost', 'network_id': 'external_net' } + with contextlib.nested( + mock.patch.object(self.mixin, + 'get_router'), mock.patch.object(self.mixin, 'get_vm_port_hostid'), mock.patch.object(self.mixin, @@ -376,11 +377,78 @@ class L3DvrTestCase(testlib_api.SqlTestCase): 'create_fip_agent_gw_port_if_not_exists'), mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, '_update_fip_assoc'), - ) as (vmp, vf, c_fip, cf): + ) as (grtr, vmp, d_fip, c_fip, up_fip): + grtr.return_value = router_db vmp.return_value = 'my-host' self.mixin._update_fip_assoc( - self.ctx, fip, floatingip, port) - self.assertTrue(c_fip.called) + self.ctx, fip, floatingip_db, port) + return d_fip, c_fip + + def test_create_floatingip_agent_gw_port_with_dvr_router(self): + floatingip = { + 'id': _uuid(), + 'router_id': 'foo_router_id' + } + router = {'id': 'foo_router_id', 'distributed': True} + fip = { + 'id': _uuid(), + 'port_id': _uuid() + } + delete_fip, create_fip = ( + self._setup_test_create_delete_floatingip( + fip, floatingip, router)) + self.assertTrue(create_fip.called) + self.assertFalse(delete_fip.called) + + def test_create_floatingip_agent_gw_port_with_non_dvr_router(self): + floatingip = { + 'id': _uuid(), + 'router_id': 'foo_router_id' + } + router = {'id': 'foo_router_id', 'distributed': False} + fip = { + 'id': _uuid(), + 'port_id': _uuid() + } + delete_fip, create_fip = ( + self._setup_test_create_delete_floatingip( + fip, floatingip, router)) + self.assertFalse(create_fip.called) + self.assertFalse(delete_fip.called) + + def test_delete_floatingip_agent_gw_port_with_dvr_router(self): + floatingip = { + 'id': _uuid(), + 'fixed_port_id': 1234, + 'router_id': 'foo_router_id' + } + router = {'id': 'foo_router_id', 'distributed': True} + fip = { + 'id': _uuid(), + 'port_id': None + } + delete_fip, create_fip = ( + self._setup_test_create_delete_floatingip( + fip, floatingip, router)) + self.assertTrue(delete_fip.called) + self.assertFalse(create_fip.called) + + def test_delete_floatingip_agent_gw_port_with_non_dvr_router(self): + floatingip = { + 'id': _uuid(), + 'fixed_port_id': 1234, + 'router_id': 'foo_router_id' + } + router = {'id': 'foo_router_id', 'distributed': False} + fip = { + 'id': _uuid(), + 'port_id': None + } + delete_fip, create_fip = ( + self._setup_test_create_delete_floatingip( + fip, floatingip, router)) + self.assertFalse(create_fip.called) + self.assertFalse(delete_fip.called) def test__validate_router_migration_prevent_check_advanced_svc(self): router = {'name': 'foo_router', 'admin_state_up': True}