diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 9122b8e9ff6..1eeaf518513 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -93,58 +93,63 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, self.set_extra_attr_value(context, router_db, 'distributed', dist) def _validate_router_migration(self, context, router_db, router_res): - """Allow centralized -> distributed state transition only.""" - if router_res.get('ha') or router_db.extra_attributes.ha: - # leave HA validation to l3_hamode_db - return - if (router_db.extra_attributes.distributed and - router_res.get('distributed') is False): - LOG.info(_LI("Centralizing distributed router %s " - "is not supported"), router_db['id']) - raise n_exc.BadRequest( - resource='router', - msg=_("Migration from distributed router to centralized is " - "not supported")) - elif (not router_db.extra_attributes.distributed and - router_res.get('distributed')): - # router should be disabled in order for upgrade - if router_db.admin_state_up: - msg = _('Cannot upgrade active router to distributed. Please ' - 'set router admin_state_up to False prior to upgrade.') - raise n_exc.BadRequest(resource='router', msg=msg) + """Allow transition only when admin_state_up=False""" + original_distributed_state = router_db.extra_attributes.distributed + requested_distributed_state = router_res.get('distributed', None) - # Notify advanced services of the imminent state transition - # for the router. - try: - kwargs = {'context': context, 'router': router_db} - registry.notify( - resources.ROUTER, events.BEFORE_UPDATE, self, **kwargs) - except exceptions.CallbackFailure as e: - with excutils.save_and_reraise_exception(): - # NOTE(armax): preserve old check's behavior - if len(e.errors) == 1: - raise e.errors[0].error - raise l3.RouterInUse(router_id=router_db['id'], - reason=e) + distributed_changed = ( + requested_distributed_state is not None and + requested_distributed_state != original_distributed_state) + if not distributed_changed: + return False + if router_db.admin_state_up: + msg = _("Cannot change the 'distributed' attribute of active " + "routers. Please set router admin_state_up to False " + "prior to upgrade") + raise n_exc.BadRequest(resource='router', msg=msg) + + # Notify advanced services of the imminent state transition + # for the router. + try: + kwargs = {'context': context, 'router': router_db} + registry.notify( + resources.ROUTER, events.BEFORE_UPDATE, self, **kwargs) + except exceptions.CallbackFailure as e: + with excutils.save_and_reraise_exception(): + # NOTE(armax): preserve old check's behavior + if len(e.errors) == 1: + raise e.errors[0].error + raise l3.RouterInUse(router_id=router_db['id'], + reason=e) + return True def _handle_distributed_migration(self, resource, event, trigger, context, router_id, router, router_db, **kwargs): """Event handler for router update migration to distributed.""" + if not self._validate_router_migration(context, router_db, router): + return + migrating_to_distributed = ( not router_db.extra_attributes.distributed and router.get('distributed') is True) - self._validate_router_migration(context, router_db, router) + if migrating_to_distributed: self._migrate_router_ports( context, router_db, old_owner=const.DEVICE_OWNER_ROUTER_INTF, new_owner=const.DEVICE_OWNER_DVR_INTERFACE) self.set_extra_attr_value(context, router_db, 'distributed', True) - cur_agents = self.list_l3_agents_hosting_router( - context, router_db['id'])['agents'] - for agent in cur_agents: - self._unbind_router(context, router_db['id'], - agent['id']) + else: + self._migrate_router_ports( + context, router_db, + old_owner=const.DEVICE_OWNER_DVR_INTERFACE, + new_owner=const.DEVICE_OWNER_ROUTER_INTF) + self.set_extra_attr_value(context, router_db, 'distributed', False) + + cur_agents = self.list_l3_agents_hosting_router( + context, router_db['id'])['agents'] + for agent in cur_agents: + self._unbind_router(context, router_db['id'], agent['id']) def _create_snat_interfaces_after_change(self, resource, event, trigger, context, router_id, router, diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 6bb543808d6..47833cf09c4 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -442,34 +442,8 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin, **kwargs): """Event handler on precommit update to validate migration.""" - original_distributed_state = old_router['distributed'] original_ha_state = old_router['ha'] - requested_ha_state = router.get('ha') - requested_distributed_state = router.get('distributed') - # cvr to dvrha - if not original_distributed_state and not original_ha_state: - if (requested_ha_state is True and - requested_distributed_state is True): - raise l3_ha.UpdateToDvrHamodeNotSupported() - - # cvrha to any dvr... - elif not original_distributed_state and original_ha_state: - if requested_distributed_state is True: - raise l3_ha.DVRmodeUpdateOfHaNotSupported() - - # dvr to any ha... - elif original_distributed_state and not original_ha_state: - if requested_ha_state is True: - raise l3_ha.HAmodeUpdateOfDvrNotSupported() - - #dvrha to any cvr... - elif original_distributed_state and original_ha_state: - if requested_distributed_state is False: - raise l3_ha.DVRmodeUpdateOfDvrHaNotSupported() - #elif dvrha to dvr - if requested_ha_state is False: - raise l3_ha.HAmodeUpdateOfDvrHaNotSupported() ha_changed = (requested_ha_state is not None and requested_ha_state != original_ha_state) @@ -478,7 +452,7 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin, if router_db.admin_state_up: msg = _('Cannot change HA attribute of active routers. Please ' - 'set router admin_state_up to False prior to upgrade.') + 'set router admin_state_up to False prior to upgrade') raise n_exc.BadRequest(resource='router', msg=msg) if requested_ha_state: diff --git a/neutron/extensions/l3_ext_ha_mode.py b/neutron/extensions/l3_ext_ha_mode.py index d1e5f375fa9..8c85a9dd7dc 100644 --- a/neutron/extensions/l3_ext_ha_mode.py +++ b/neutron/extensions/l3_ext_ha_mode.py @@ -30,35 +30,6 @@ EXTENDED_ATTRIBUTES_2_0 = { } -class HAmodeUpdateOfDvrNotSupported(NotImplementedError): - message = _("Currently update of HA mode for a distributed router is " - "not supported.") - - -class DVRmodeUpdateOfHaNotSupported(NotImplementedError): - message = _("Currently update of distributed mode for an HA router is " - "not supported.") - - -class HAmodeUpdateOfDvrHaNotSupported(NotImplementedError): - message = _("Currently update of HA mode for a DVR/HA router is " - "not supported.") - - -class DVRmodeUpdateOfDvrHaNotSupported(NotImplementedError): - message = _("Currently update of distributed mode for a DVR/HA router " - "is not supported") - - -class UpdateToDvrHamodeNotSupported(NotImplementedError): - message = _("Currently updating a router to DVR/HA is not supported.") - - -class UpdateToNonDvrHamodeNotSupported(NotImplementedError): - message = _("Currently updating a router from DVR/HA to non-DVR " - " non-HA is not supported.") - - class MaxVRIDAllocationTriesReached(exceptions.NeutronException): message = _("Failed to allocate a VRID in the network %(network_id)s " "for the router %(router_id)s after %(max_tries)s tries.") diff --git a/neutron/tests/functional/services/l3_router/test_l3_dvr_ha_router_plugin.py b/neutron/tests/functional/services/l3_router/test_l3_dvr_ha_router_plugin.py index 3ad0446485e..1bb638b58a9 100644 --- a/neutron/tests/functional/services/l3_router/test_l3_dvr_ha_router_plugin.py +++ b/neutron/tests/functional/services/l3_router/test_l3_dvr_ha_router_plugin.py @@ -15,11 +15,9 @@ import mock from neutron_lib import constants -from neutron.callbacks import exceptions as c_exc from neutron.common import topics from neutron.extensions import external_net from neutron.extensions import l3 -from neutron.extensions import l3_ext_ha_mode from neutron.extensions import portbindings from neutron.tests.common import helpers from neutron.tests.functional.services.l3_router import \ @@ -35,100 +33,77 @@ class L3DvrHATestCase(test_l3_dvr_router_plugin.L3DvrTestCase): host="standby", agent_mode=constants.L3_AGENT_MODE_DVR_SNAT) - def _create_router(self, distributed=True, ha=True): + def _create_router(self, distributed=True, ha=True, admin_state_up=True): return (super(L3DvrHATestCase, self). - _create_router(distributed=distributed, ha=ha)) + _create_router(distributed=distributed, ha=ha, + admin_state_up=admin_state_up)) def test_update_router_db_cvr_to_dvrha(self): - router = self._create_router(distributed=False, ha=False) - e = self.assertRaises( - c_exc.CallbackFailure, - self.l3_plugin.update_router, + router = self._create_router(distributed=False, ha=False, + admin_state_up=False) + self.l3_plugin.update_router( self.context, router['id'], - {'router': {'distributed': True, 'ha': True}} - ) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.UpdateToDvrHamodeNotSupported) + {'router': {'distributed': True, 'ha': True}}) + router = self.l3_plugin.get_router(self.context, router['id']) + self.assertTrue(router['distributed']) + self.assertTrue(router['ha']) + + def test_update_router_db_dvrha_to_cvr(self): + router = self._create_router(distributed=True, ha=True, + admin_state_up=False) + self.l3_plugin.update_router( + self.context, + router['id'], + {'router': {'distributed': False, 'ha': False}}) router = self.l3_plugin.get_router(self.context, router['id']) self.assertFalse(router['distributed']) self.assertFalse(router['ha']) - def test_update_router_db_dvrha_to_cvr(self): - router = self._create_router(distributed=True, ha=True) - e = self.assertRaises( - c_exc.CallbackFailure, - self.l3_plugin.update_router, - self.context, - router['id'], - {'router': {'distributed': False, 'ha': False}} - ) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.DVRmodeUpdateOfDvrHaNotSupported) - router = self.l3_plugin.get_router(self.context, router['id']) - self.assertTrue(router['distributed']) - self.assertTrue(router['ha']) - def test_update_router_db_dvrha_to_dvr(self): - router = self._create_router(distributed=True, ha=True) + router = self._create_router(distributed=True, ha=True, + admin_state_up=False) self.l3_plugin.update_router( self.context, router['id'], {'router': {'admin_state_up': False}}) - e = self.assertRaises( - c_exc.CallbackFailure, - self.l3_plugin.update_router, + self.l3_plugin.update_router( self.context, router['id'], - {'router': {'distributed': True, 'ha': False}} - ) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.HAmodeUpdateOfDvrHaNotSupported) + {'router': {'distributed': True, 'ha': False}}) router = self.l3_plugin.get_router(self.context, router['id']) self.assertTrue(router['distributed']) - self.assertTrue(router['ha']) + self.assertFalse(router['ha']) def test_update_router_db_dvrha_to_cvrha(self): - router = self._create_router(distributed=True, ha=True) - e = self.assertRaises( - c_exc.CallbackFailure, - self.l3_plugin.update_router, + router = self._create_router(distributed=True, ha=True, + admin_state_up=False) + self.l3_plugin.update_router( self.context, router['id'], - {'router': {'distributed': False, 'ha': True}} - ) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.DVRmodeUpdateOfDvrHaNotSupported) + {'router': {'distributed': False, 'ha': True}}) router = self.l3_plugin.get_router(self.context, router['id']) - self.assertTrue(router['distributed']) + self.assertFalse(router['distributed']) self.assertTrue(router['ha']) def test_update_router_db_dvr_to_dvrha(self): - router = self._create_router(distributed=True, ha=False) - e = self.assertRaises( - c_exc.CallbackFailure, - self.l3_plugin.update_router, + router = self._create_router(distributed=True, ha=False, + admin_state_up=False) + self.l3_plugin.update_router( self.context, router['id'], - {'router': {'distributed': True, 'ha': True}} - ) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.HAmodeUpdateOfDvrNotSupported) + {'router': {'distributed': True, 'ha': True}}) router = self.l3_plugin.get_router(self.context, router['id']) self.assertTrue(router['distributed']) - self.assertFalse(router['ha']) + self.assertTrue(router['ha']) def test_update_router_db_cvrha_to_dvrha(self): - router = self._create_router(distributed=False, ha=True) - e = self.assertRaises( - c_exc.CallbackFailure, - self.l3_plugin.update_router, + router = self._create_router(distributed=False, ha=True, + admin_state_up=False) + self.l3_plugin.update_router( self.context, router['id'], - {'router': {'distributed': True, 'ha': True}} - ) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.DVRmodeUpdateOfHaNotSupported) + {'router': {'distributed': True, 'ha': True}}) router = self.l3_plugin.get_router(self.context, router['id']) - self.assertFalse(router['distributed']) + self.assertTrue(router['distributed']) self.assertTrue(router['ha']) def _assert_router_is_hosted_on_both_dvr_snat_agents(self, router): diff --git a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py index 1c11c0f1205..21b9d930568 100644 --- a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py +++ b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py @@ -37,9 +37,10 @@ class L3DvrTestCaseBase(ml2_test_base.ML2TestFramework): self.l3_agent = helpers.register_l3_agent( agent_mode=constants.L3_AGENT_MODE_DVR_SNAT) - def _create_router(self, distributed=True, ha=False): + def _create_router(self, distributed=True, ha=False, admin_state_up=True): return (super(L3DvrTestCaseBase, self). - _create_router(distributed=distributed, ha=ha)) + _create_router(distributed=distributed, ha=ha, + admin_state_up=admin_state_up)) class L3DvrTestCase(L3DvrTestCaseBase): diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 3f76cd20dc1..f7e21cd1e41 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -81,7 +81,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): 'distributed': True } router_db = self._create_router(router) - self.assertIsNone(self.mixin._validate_router_migration( + self.assertFalse(self.mixin._validate_router_migration( self.ctx, router_db, {'name': 'foo_router_2'})) def test__validate_router_migration_raise_error(self): diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 3698dc6f975..a61fea0a14d 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -281,73 +281,63 @@ class L3HATestCase(L3HATestFramework): router = self._create_router(ha=True, admin_state_up=False, distributed=False) self.assertTrue(router['ha']) - e = self.assertRaises(c_exc.CallbackFailure, - self._update_router, - router['id'], - ha=True, - distributed=True) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.DVRmodeUpdateOfHaNotSupported) + + after_update = self._update_router(router['id'], + ha=True, distributed=True) + self.assertTrue(after_update.extra_attributes.ha) + self.assertTrue(after_update.extra_attributes.distributed) def test_migrate_ha_router_to_distributed_and_not_ha(self): router = self._create_router(ha=True, admin_state_up=False, distributed=False) self.assertTrue(router['ha']) - e = self.assertRaises(c_exc.CallbackFailure, - self._update_router, - router['id'], - ha=False, - distributed=True) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.DVRmodeUpdateOfHaNotSupported) + + after_update = self._update_router(router['id'], + ha=False, distributed=True) + self.assertFalse(after_update.extra_attributes.ha) + self.assertTrue(after_update.extra_attributes.distributed) def test_migrate_dvr_router_to_ha_and_not_dvr(self): router = self._create_router(ha=False, admin_state_up=False, distributed=True) self.assertTrue(router['distributed']) - e = self.assertRaises(c_exc.CallbackFailure, - self._update_router, - router['id'], - ha=True, - distributed=True) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.HAmodeUpdateOfDvrNotSupported) + + after_update = self._update_router(router['id'], + ha=True, distributed=False) + self.assertTrue(after_update.extra_attributes.ha) + self.assertFalse(after_update.extra_attributes.distributed) def test_migrate_dvr_router_to_ha_and_dvr(self): router = self._create_router(ha=False, admin_state_up=False, distributed=True) self.assertTrue(router['distributed']) - e = self.assertRaises(c_exc.CallbackFailure, - self._update_router, - router['id'], - ha=True, - distributed=True) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.HAmodeUpdateOfDvrNotSupported) + + after_update = self._update_router(router['id'], + ha=True, distributed=True) + self.assertTrue(after_update.extra_attributes.ha) + self.assertTrue(after_update.extra_attributes.distributed) def test_migrate_distributed_router_to_ha(self): - router = self._create_router(ha=False, distributed=True) + router = self._create_router(ha=False, admin_state_up=False, + distributed=True) self.assertFalse(router['ha']) self.assertTrue(router['distributed']) - e = self.assertRaises(c_exc.CallbackFailure, - self._update_router, - router['id'], - ha=True) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.HAmodeUpdateOfDvrNotSupported) + after_update = self._update_router(router['id'], + ha=True, distributed=False) + self.assertTrue(after_update.extra_attributes.ha) + self.assertFalse(after_update.extra_attributes.distributed) def test_migrate_legacy_router_to_distributed_and_ha(self): - router = self._create_router(ha=False, distributed=False) + router = self._create_router(ha=False, admin_state_up=False, + distributed=False) self.assertFalse(router['ha']) self.assertFalse(router['distributed']) - e = self.assertRaises(c_exc.CallbackFailure, - self._update_router, - router['id'], - ha=True, - distributed=True) - self.assertIsInstance(e.inner_exceptions[0], - l3_ext_ha_mode.UpdateToDvrHamodeNotSupported) + + after_update = self._update_router(router['id'], + ha=True, distributed=True) + self.assertTrue(after_update.extra_attributes.ha) + self.assertTrue(after_update.extra_attributes.distributed) def test_unbind_ha_router(self): router = self._create_router() diff --git a/neutron/tests/unit/plugins/ml2/base.py b/neutron/tests/unit/plugins/ml2/base.py index ddb6e258ba9..c65a56758f1 100644 --- a/neutron/tests/unit/plugins/ml2/base.py +++ b/neutron/tests/unit/plugins/ml2/base.py @@ -34,12 +34,12 @@ class ML2TestFramework(test_plugin.Ml2PluginV2TestCase): self.core_plugin = directory.get_plugin() self.l3_plugin = directory.get_plugin(constants.L3) - def _create_router(self, distributed=False, ha=False): + def _create_router(self, distributed=False, ha=False, admin_state_up=True): return self.l3_plugin.create_router( self.context, {'router': {'name': 'router', - 'admin_state_up': True, + 'admin_state_up': admin_state_up, 'tenant_id': self._tenant_id, 'ha': ha, 'distributed': distributed}})