Check NVP router's status before deploying a service

With NVP advanced service plugin, router creation is asynchronous while
all service call is synchronous, so it is possible that advanced service
request is called before edge deployment completed.
The solution is to check the router status before deploying an advanced service.
If the router is not in ACTIVE status, the service deployment request would return
"Router not in 'ACTIVE' status" error.

Change-Id: Idde71c37f5d5c113ac04376ed607e0c156b07477
Closes-Bug: #1298865
This commit is contained in:
berlin 2014-03-31 14:46:56 +08:00
parent 7b15b03fcb
commit 25103df197
6 changed files with 128 additions and 55 deletions

View File

@ -90,6 +90,11 @@ class VcnsDriverException(NsxPluginException):
message = _("Error happened in NSX VCNS Driver: %(err_msg)s")
class AdvRouterServiceUnavailable(n_exc.ServiceUnavailable):
message = _("Router %(router_id)s is not in 'ACTIVE' "
"status, thus unable to provide advanced service")
class ServiceClusterUnavailable(NsxPluginException):
message = _("Service cluster: '%(cluster_id)s' is unavailable. Please, "
"check NSX setup and/or configuration")

View File

@ -520,6 +520,17 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin,
router_id=router_id,
firewall_id=firewalls[0]['id'])
def check_router(self, context, router_id):
if not router_id:
msg = _("router_id is not provided!")
raise n_exc.BadRequest(resource='router', msg=msg)
router = self._get_router(context, router_id)
if not self._is_advanced_service_router(context, router=router):
msg = _("router_id:%s is not an advanced router!") % router['id']
raise n_exc.BadRequest(resource='router', msg=msg)
if router['status'] != service_constants.ACTIVE:
raise nsx_exc.AdvRouterServiceUnavailable(router_id=router['id'])
def _delete_lrouter(self, context, router_id, nsx_router_id):
binding = vcns_db.get_vcns_router_binding(context.session, router_id)
if not binding:
@ -903,14 +914,7 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin,
def create_firewall(self, context, firewall):
LOG.debug(_("create_firewall() called"))
router_id = firewall['firewall'].get(vcns_const.ROUTER_ID)
if not router_id:
msg = _("router_id is not provided!")
LOG.error(msg)
raise n_exc.BadRequest(resource='router', msg=msg)
if not self._is_advanced_service_router(context, router_id):
msg = _("router_id:%s is not an advanced router!") % router_id
LOG.error(msg)
raise n_exc.BadRequest(resource='router', msg=msg)
self.check_router(context, router_id)
if self._get_resource_router_id_binding(
context, firewall_db.Firewall, router_id=router_id):
msg = _("A firewall is already associated with the router")
@ -1219,16 +1223,7 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin,
def create_vip(self, context, vip):
LOG.debug(_("create_vip() called"))
router_id = vip['vip'].get(vcns_const.ROUTER_ID)
if not router_id:
msg = _("router_id is not provided!")
LOG.error(msg)
raise n_exc.BadRequest(resource='router', msg=msg)
if not self._is_advanced_service_router(context, router_id):
msg = _("router_id: %s is not an advanced router!") % router_id
LOG.error(msg)
raise nsx_exc.NsxPluginException(err_msg=msg)
self.check_router(context, router_id)
#Check whether the vip port is an external port
subnet_id = vip['vip']['subnet_id']
network_id = self.get_subnet(context, subnet_id)['network_id']
@ -1607,11 +1602,7 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin,
def create_vpnservice(self, context, vpnservice):
LOG.debug(_("create_vpnservice() called"))
router_id = vpnservice['vpnservice'].get('router_id')
if not self._is_advanced_service_router(context, router_id):
msg = _("router_id:%s is not an advanced router!") % router_id
LOG.warning(msg)
raise exceptions.VcnsBadRequest(resource='router', msg=msg)
self.check_router(context, router_id)
if self.get_vpnservices(context, filters={'router_id': [router_id]}):
msg = _("a vpnservice is already associated with the router: %s"
) % router_id

View File

@ -21,9 +21,11 @@ from oslo.config import cfg
from neutron.api.v2 import attributes
from neutron import context
from neutron.db import l3_db
from neutron.extensions import l3
from neutron import manager as n_manager
from neutron.openstack.common import uuidutils
from neutron.plugins.common import constants as service_constants
from neutron.plugins.vmware.common import utils
from neutron.plugins.vmware.plugins import service as nsp
from neutron.tests import base
@ -140,10 +142,11 @@ class ServiceRouterTest(test_nsx_plugin.L3NatTest,
if admin_state_up:
data['router']['admin_state_up'] = admin_state_up
for arg in (('admin_state_up', 'tenant_id') + (arg_list or ())):
# Arg must be present and not empty
if arg in kwargs and kwargs[arg]:
# Arg must be present
if arg in kwargs:
data['router'][arg] = kwargs[arg]
data['router']['service_router'] = True
if data['router'].get('service_router') is None:
data['router']['service_router'] = True
router_req = self.new_create_request('routers', data, fmt)
if set_context and tenant_id:
# create a specific auth context for this request
@ -152,6 +155,22 @@ class ServiceRouterTest(test_nsx_plugin.L3NatTest,
return router_req.get_response(self.ext_api)
def _create_and_get_router(self, active_set=True, **kwargs):
"""Create advanced service router for services."""
req = self._create_router(self.fmt, self._tenant_id, **kwargs)
res = self.deserialize(self.fmt, req)
router_id = res['router']['id']
# manually set router status ACTIVE to pass through the router check,
# else mimic router creation ERROR condition.
status = (service_constants.ACTIVE if active_set
else service_constants.ERROR)
self.plugin._resource_set_status(
context.get_admin_context(),
l3_db.Router,
router_id,
status)
return router_id
class ServiceRouterTestCase(ServiceRouterTest,
test_nsx_plugin.TestL3NatTestCase):

View File

@ -100,11 +100,6 @@ class FirewallPluginTestCase(test_db_firewall.FirewallPluginDbTestCase,
self.ext_api = None
self.plugin = None
def _create_and_get_router(self):
req = self._create_router(self.fmt, self._tenant_id)
res = self.deserialize(self.fmt, req)
return res['router']['id']
def _create_firewall(self, fmt, name, description, firewall_policy_id,
admin_state_up=True, expected_res_status=None,
**kwargs):
@ -141,21 +136,46 @@ class FirewallPluginTestCase(test_db_firewall.FirewallPluginDbTestCase,
for k, v in attrs.iteritems():
self.assertEqual(fw['firewall'][k], v)
def test_create_firewall_without_policy(self):
def test_create_firewall_without_policy(self, **kwargs):
name = "new_fw"
attrs = self._get_test_firewall_attrs(name)
attrs['router_id'] = self._create_and_get_router()
if 'router_id' in kwargs:
attrs['router_id'] = kwargs.pop('router_id')
else:
attrs['router_id'] = self._create_and_get_router()
with self.firewall(name=name,
router_id=attrs['router_id'],
admin_state_up=
test_db_firewall.ADMIN_STATE_UP,
expected_res_status=201) as fw:
**kwargs) as fw:
attrs = self._replace_firewall_status(
attrs, const.PENDING_CREATE, const.ACTIVE)
for k, v in attrs.iteritems():
self.assertEqual(fw['firewall'][k], v)
def test_create_firewall_with_invalid_router(self):
name = "new_fw"
attrs = self._get_test_firewall_attrs(name)
attrs['router_id'] = self._create_and_get_router()
self.assertRaises(webob.exc.HTTPClientError,
self.test_create_firewall_without_policy,
router_id=None)
self.assertRaises(webob.exc.HTTPClientError,
self.test_create_firewall_without_policy,
router_id='invalid_id')
router_id = self._create_and_get_router(
arg_list=('service_router',), service_router=False)
self.assertRaises(webob.exc.HTTPClientError,
self.test_create_firewall_without_policy,
router_id=router_id)
router_id = self._create_and_get_router(active_set=False)
self.assertRaises(webob.exc.HTTPClientError,
self.test_create_firewall_without_policy,
router_id=router_id)
def test_update_firewall(self):
name = "new_fw"
attrs = self._get_test_firewall_attrs(name)

View File

@ -108,11 +108,6 @@ class TestLoadbalancerPlugin(
self.ext_api = None
self.plugin = None
def _create_and_get_router(self):
req = self._create_router(self.fmt, self._tenant_id)
res = self.deserialize(self.fmt, req)
return res['router']['id']
def _get_vip_optional_args(self):
args = super(TestLoadbalancerPlugin, self)._get_vip_optional_args()
return args + ('router_id',)
@ -152,7 +147,7 @@ class TestLoadbalancerPlugin(
for k, v in keys:
self.assertEqual(res['health_monitor'][k], v)
def test_create_vip(self, **extras):
def test_create_vip(self, **kwargs):
expected = {
'name': 'vip1',
'description': '',
@ -161,11 +156,14 @@ class TestLoadbalancerPlugin(
'connection_limit': -1,
'admin_state_up': True,
'status': 'ACTIVE',
'router_id': self._create_and_get_router(),
'tenant_id': self._tenant_id,
}
if 'router_id' in kwargs:
expected['router_id'] = kwargs.pop('router_id')
else:
expected['router_id'] = self._create_and_get_router()
expected.update(extras)
expected.update(kwargs)
name = expected['name']
@ -183,7 +181,7 @@ class TestLoadbalancerPlugin(
)
with self.vip(
router_id=expected['router_id'], name=name,
pool=pool, subnet=subnet, **extras) as vip:
pool=pool, subnet=subnet, **kwargs) as vip:
for k in ('id', 'address', 'port_id', 'pool_id'):
self.assertTrue(vip['vip'].get(k, None))
self.assertEqual(
@ -192,6 +190,23 @@ class TestLoadbalancerPlugin(
expected
)
def test_create_vip_with_invalid_router(self):
self.assertRaises(
web_exc.HTTPClientError,
self.test_create_vip, router_id=None)
self.assertRaises(
web_exc.HTTPClientError,
self.test_create_vip, router_id='invalid_router_id')
router_id = self._create_and_get_router(
arg_list=('service_router',), service_router=False)
self.assertRaises(
web_exc.HTTPClientError,
self.test_create_vip, router_id=router_id)
router_id = self._create_and_get_router(active_set=False)
self.assertRaises(
web_exc.HTTPClientError,
self.test_create_vip, router_id=router_id)
def test_create_vip_with_session_persistence(self):
self.test_create_vip(session_persistence={'type': 'HTTP_COOKIE'})

View File

@ -79,22 +79,18 @@ class TestVpnPlugin(test_db_vpnaas.VPNTestMixin,
self.plugin = None
@contextlib.contextmanager
def router(self, vlan_id=None):
def router(self, vlan_id=None, do_delete=True, **kwargs):
with self._create_l3_ext_network(vlan_id) as net:
with self.subnet(cidr='100.0.0.0/24', network=net) as s:
data = {'router': {'tenant_id': self._tenant_id}}
data['router']['service_router'] = True
router_req = self.new_create_request('routers', data, self.fmt)
res = router_req.get_response(self.ext_api)
router = self.deserialize(self.fmt, res)
router_id = self._create_and_get_router(**kwargs)
self._add_external_gateway_to_router(
router['router']['id'],
s['subnet']['network_id'])
router = self._show('routers', router['router']['id'])
router_id, s['subnet']['network_id'])
router = self._show('routers', router_id)
yield router
self._delete('routers', router['router']['id'])
if do_delete:
self._remove_external_gateway_from_router(
router_id, s['subnet']['network_id'])
self._delete('routers', router_id)
def test_create_vpnservice(self, **extras):
"""Test case to create a vpnservice."""
@ -135,6 +131,33 @@ class TestVpnPlugin(test_db_vpnaas.VPNTestMixin,
self.assertEqual(
res.status_int, webob.exc.HTTPConflict.code)
def test_create_vpnservice_with_invalid_router(self):
"""Test case to create vpnservices with invalid router."""
with self.subnet(cidr='10.2.0.0/24') as subnet:
with contextlib.nested(
self.router(arg_list=('service_router',),
service_router=False),
self.router(active_set=False)) as (r1, r2):
res = self._create_vpnservice(
'json', 'vpnservice', True,
router_id='invalid_id',
subnet_id=(subnet['subnet']['id']))
self.assertEqual(
res.status_int, webob.exc.HTTPBadRequest.code)
res = self._create_vpnservice(
'json', 'vpnservice', True,
router_id=r1['router']['id'],
subnet_id=(subnet['subnet']['id']))
self.assertEqual(
res.status_int, webob.exc.HTTPBadRequest.code)
res = self._create_vpnservice(
'json', 'vpnservice', True,
router_id=r2['router']['id'],
subnet_id=(subnet['subnet']['id']))
self.assertEqual(
res.status_int,
webob.exc.HTTPServiceUnavailable.code)
def test_update_vpnservice(self):
"""Test case to update a vpnservice."""
name = 'new_vpnservice1'