From: Kaiwei Fan Date: Mon, 16 Sep 2013 22:02:34 +0000 (-0700) Subject: Fix for status always in PENDING_CREATE for Edge service router X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d731097843d0b031bcdf6a8d67548f4bdf733016;p=openstack-build%2Fneutron-build.git Fix for status always in PENDING_CREATE for Edge service router The root cause is when deployment finished, we only update router status to active if the status is in pending create. The problem happens when the background sync thread update router status to active, so the status update for vcns_router_binding table is skipped. We fixed this by seperating checking and updating status for router and binding table. Also fixed an issue where Edge is not deleted if neutron service is restarted. The root cause is when neutron service restarts, the cache for router type is empty. And because we delete the router from db before we delete Edge, we're not able to locate the router from db to determine the router type. The fix is to use binding table to determine the router type. Also piggyback a missing attribute for updating Edge interface. It must have been removed by accident when resolving conflict during service plugin merge. Closes-Bug: #1226229 Change-Id: I3d0639d245e71ea2a3faba70fef1a0ebb87e19fd --- diff --git a/neutron/plugins/nicira/NeutronServicePlugin.py b/neutron/plugins/nicira/NeutronServicePlugin.py index 576e04730..9e030469b 100644 --- a/neutron/plugins/nicira/NeutronServicePlugin.py +++ b/neutron/plugins/nicira/NeutronServicePlugin.py @@ -25,6 +25,7 @@ from neutron.db import l3_db from neutron.db.loadbalancer import loadbalancer_db from neutron.db import routedserviceinsertion_db as rsi_db from neutron.extensions import firewall as fw_ext +from neutron.extensions import l3 from neutron.openstack.common import excutils from neutron.openstack.common import log as logging from neutron.plugins.common import constants as service_constants @@ -473,14 +474,10 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, return lrouter def _delete_lrouter(self, context, id): - if not self._is_advanced_service_router(context, id): - super(NvpAdvancedPlugin, self)._delete_lrouter(context, id) - if id in self._router_type: - del self._router_type[id] - return - binding = vcns_db.get_vcns_router_binding(context.session, id) - if binding: + if not binding: + super(NvpAdvancedPlugin, self)._delete_lrouter(context, id) + else: vcns_db.update_vcns_router_binding( context.session, id, status=service_constants.PENDING_DELETE) @@ -499,8 +496,9 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin, } self.vcns_driver.delete_edge(id, edge_id, jobdata=jobdata) - # delete LR - nvplib.delete_lrouter(self.cluster, id) + # delete LR + nvplib.delete_lrouter(self.cluster, id) + if id in self._router_type: del self._router_type[id] @@ -1528,24 +1526,33 @@ class VcnsCallbacks(object): lrouter = jobdata['lrouter'] context = jobdata['context'] name = task.userdata['router_name'] - router_db = self.plugin._get_router(context, lrouter['uuid']) + router_db = None + try: + router_db = self.plugin._get_router(context, lrouter['uuid']) + except l3.RouterNotFound: + # Router might have been deleted before deploy finished + LOG.exception(_("Router %s not found"), lrouter['uuid']) + if task.status == TaskStatus.COMPLETED: LOG.debug(_("Successfully deployed %(edge_id)s for " "router %(name)s"), { 'edge_id': task.userdata['edge_id'], 'name': name}) - if router_db['status'] == service_constants.PENDING_CREATE: + if (router_db and + router_db['status'] == service_constants.PENDING_CREATE): router_db['status'] = service_constants.ACTIVE - binding = vcns_db.get_vcns_router_binding( - context.session, lrouter['uuid']) - # only update status to active if its status is pending create - if binding['status'] == service_constants.PENDING_CREATE: - vcns_db.update_vcns_router_binding( - context.session, lrouter['uuid'], - status=service_constants.ACTIVE) + + binding = vcns_db.get_vcns_router_binding( + context.session, lrouter['uuid']) + # only update status to active if its status is pending create + if binding['status'] == service_constants.PENDING_CREATE: + vcns_db.update_vcns_router_binding( + context.session, lrouter['uuid'], + status=service_constants.ACTIVE) else: LOG.debug(_("Failed to deploy Edge for router %s"), name) - router_db['status'] = service_constants.ERROR + if router_db: + router_db['status'] = service_constants.ERROR vcns_db.update_vcns_router_binding( context.session, lrouter['uuid'], status=service_constants.ERROR) diff --git a/neutron/plugins/nicira/vshield/edge_appliance_driver.py b/neutron/plugins/nicira/vshield/edge_appliance_driver.py index 9d15dc16d..807224918 100644 --- a/neutron/plugins/nicira/vshield/edge_appliance_driver.py +++ b/neutron/plugins/nicira/vshield/edge_appliance_driver.py @@ -101,7 +101,8 @@ class EdgeApplianceDriver(object): } if secondary: address_group['secondaryAddresses'] = { - 'ipAddress': secondary + 'ipAddress': secondary, + 'type': 'IpAddressesDto' } vnic['addressGroups'] = { diff --git a/neutron/tests/unit/nicira/test_edge_router.py b/neutron/tests/unit/nicira/test_edge_router.py index ad772d77a..60f690d8d 100644 --- a/neutron/tests/unit/nicira/test_edge_router.py +++ b/neutron/tests/unit/nicira/test_edge_router.py @@ -166,7 +166,7 @@ class ServiceRouterTestCase(ServiceRouterTest, NvpRouterTestCase): ('admin_state_up', True), ('external_gateway_info', None), ('service_router', True)] - with self.router(name='router1', admin_state_up=True, + with self.router(name=name, admin_state_up=True, tenant_id=tenant_id) as router: expected_value_1 = expected_value + [('status', 'PENDING_CREATE')] for k, v in expected_value_1: @@ -196,6 +196,21 @@ class ServiceRouterTestCase(ServiceRouterTest, NvpRouterTestCase): if lswitch['display_name'] == lswitch_name: self.fail("Integration switch is not deleted") + def test_router_delete_after_plugin_restart(self): + name = 'router1' + tenant_id = _uuid() + with self.router(name=name, admin_state_up=True, + tenant_id=tenant_id): + # clear router type cache to mimic plugin restart + plugin = NeutronManager.get_plugin() + plugin._router_type = {} + + # check an integration lswitch is deleted + lswitch_name = "%s-ls" % name + for lswitch_id, lswitch in self.fc2._lswitches.iteritems(): + if lswitch['display_name'] == lswitch_name: + self.fail("Integration switch is not deleted") + def test_router_show(self): name = 'router1' tenant_id = _uuid()