diff --git a/neutron/plugins/vmware/common/exceptions.py b/neutron/plugins/vmware/common/exceptions.py index 83cc05bf4ad..3f435bd531c 100644 --- a/neutron/plugins/vmware/common/exceptions.py +++ b/neutron/plugins/vmware/common/exceptions.py @@ -90,11 +90,6 @@ 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") diff --git a/neutron/plugins/vmware/plugins/service.py b/neutron/plugins/vmware/plugins/service.py index 4a5b8e9578a..ca1dca82ece 100644 --- a/neutron/plugins/vmware/plugins/service.py +++ b/neutron/plugins/vmware/plugins/service.py @@ -520,17 +520,6 @@ 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: @@ -914,7 +903,14 @@ 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) - self.check_router(context, 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) if self._get_resource_router_id_binding( context, firewall_db.Firewall, router_id=router_id): msg = _("A firewall is already associated with the router") @@ -1223,7 +1219,16 @@ 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) - self.check_router(context, 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) + #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'] @@ -1602,7 +1607,11 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin, def create_vpnservice(self, context, vpnservice): LOG.debug(_("create_vpnservice() called")) router_id = vpnservice['vpnservice'].get('router_id') - self.check_router(context, 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) + if self.get_vpnservices(context, filters={'router_id': [router_id]}): msg = _("a vpnservice is already associated with the router: %s" ) % router_id diff --git a/neutron/tests/unit/vmware/vshield/test_edge_router.py b/neutron/tests/unit/vmware/vshield/test_edge_router.py index e19de25026a..c43e9b34ba2 100644 --- a/neutron/tests/unit/vmware/vshield/test_edge_router.py +++ b/neutron/tests/unit/vmware/vshield/test_edge_router.py @@ -21,11 +21,9 @@ 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 @@ -142,11 +140,10 @@ 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 - if arg in kwargs: + # Arg must be present and not empty + if arg in kwargs and kwargs[arg]: data['router'][arg] = kwargs[arg] - if data['router'].get('service_router') is None: - data['router']['service_router'] = True + 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 @@ -155,22 +152,6 @@ 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): diff --git a/neutron/tests/unit/vmware/vshield/test_fwaas_plugin.py b/neutron/tests/unit/vmware/vshield/test_fwaas_plugin.py index 19bf692f29c..acd8e7da79f 100644 --- a/neutron/tests/unit/vmware/vshield/test_fwaas_plugin.py +++ b/neutron/tests/unit/vmware/vshield/test_fwaas_plugin.py @@ -100,6 +100,11 @@ 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): @@ -136,46 +141,21 @@ 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, **kwargs): + def test_create_firewall_without_policy(self): name = "new_fw" attrs = self._get_test_firewall_attrs(name) - if 'router_id' in kwargs: - attrs['router_id'] = kwargs.pop('router_id') - else: - attrs['router_id'] = self._create_and_get_router() + 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, - **kwargs) as fw: + expected_res_status=201) 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) diff --git a/neutron/tests/unit/vmware/vshield/test_lbaas_plugin.py b/neutron/tests/unit/vmware/vshield/test_lbaas_plugin.py index fdb0be4345a..a6737cfb880 100644 --- a/neutron/tests/unit/vmware/vshield/test_lbaas_plugin.py +++ b/neutron/tests/unit/vmware/vshield/test_lbaas_plugin.py @@ -108,6 +108,11 @@ 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',) @@ -147,7 +152,7 @@ class TestLoadbalancerPlugin( for k, v in keys: self.assertEqual(res['health_monitor'][k], v) - def test_create_vip(self, **kwargs): + def test_create_vip(self, **extras): expected = { 'name': 'vip1', 'description': '', @@ -156,14 +161,11 @@ 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(kwargs) + expected.update(extras) name = expected['name'] @@ -181,7 +183,7 @@ class TestLoadbalancerPlugin( ) with self.vip( router_id=expected['router_id'], name=name, - pool=pool, subnet=subnet, **kwargs) as vip: + pool=pool, subnet=subnet, **extras) as vip: for k in ('id', 'address', 'port_id', 'pool_id'): self.assertTrue(vip['vip'].get(k, None)) self.assertEqual( @@ -190,23 +192,6 @@ 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'}) diff --git a/neutron/tests/unit/vmware/vshield/test_vpnaas_plugin.py b/neutron/tests/unit/vmware/vshield/test_vpnaas_plugin.py index 7ef1a544512..8d8f115ec96 100644 --- a/neutron/tests/unit/vmware/vshield/test_vpnaas_plugin.py +++ b/neutron/tests/unit/vmware/vshield/test_vpnaas_plugin.py @@ -79,18 +79,22 @@ class TestVpnPlugin(test_db_vpnaas.VPNTestMixin, self.plugin = None @contextlib.contextmanager - def router(self, vlan_id=None, do_delete=True, **kwargs): + def router(self, vlan_id=None): with self._create_l3_ext_network(vlan_id) as net: with self.subnet(cidr='100.0.0.0/24', network=net) as s: - router_id = self._create_and_get_router(**kwargs) + 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) self._add_external_gateway_to_router( - router_id, s['subnet']['network_id']) - router = self._show('routers', router_id) + router['router']['id'], + s['subnet']['network_id']) + router = self._show('routers', router['router']['id']) yield router - if do_delete: - self._remove_external_gateway_from_router( - router_id, s['subnet']['network_id']) - self._delete('routers', router_id) + + self._delete('routers', router['router']['id']) def test_create_vpnservice(self, **extras): """Test case to create a vpnservice.""" @@ -131,33 +135,6 @@ 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'