openstack/neutron
Code Issues Proposed changes

Allow all migration of routers

Browse Source 
This patch proposes the allowance of changing a router's 'distributed'
or 'ha' attributes without any restriction as to the previous state of
these attributes, as long as the router's admin_state_up is False.

Closes-Bug: #1654991
Change-Id: Iad4dd77f74ba01876e0091112a1e24a3463993d1
This commit is contained in:
John Schwarz 2016-09-26 17:36:53 +03:00 committed by Brian Haley
parent 4f705137e9
commit 318650ab80
8 changed files with 121 additions and 205 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
neutron/db/l3_dvr_db.py
View File

@ -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,

28
neutron/db/l3_hamode_db.py
View File

@ -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:

29
neutron/extensions/l3_ext_ha_mode.py
View File

@ -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.")

101
neutron/tests/functional/services/l3_router/test_l3_dvr_ha_router_plugin.py
View File

@ -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):

5
neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py
View File

@ -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):

2
neutron/tests/unit/db/test_l3_dvr_db.py
View File

@ -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):

76
neutron/tests/unit/db/test_l3_hamode_db.py
View File

@ -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()

4
neutron/tests/unit/plugins/ml2/base.py
View File

@ -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}})