From: armando-migliaccio Date: Fri, 18 Jul 2014 22:15:57 +0000 (-0700) Subject: NSX: unify the two distributed routing extensions X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=45e8eaa1b12ae1c7b0707d42eb82661c7035e3a4;p=openstack-build%2Fneutron-build.git NSX: unify the two distributed routing extensions This is done by adopting the router_extra_attributes models. However, the NSX 'distributed' extension is slightly different in that it is visible to tenants and prevents router conversion (allow_put=false). PUT requests are made return the correct HTTP code; The access control misalignments will need to be adjusted via rules in policy.json. This will be properly documented. DocImpact Supports-blueprint: neutron-ovs-dvr Change-Id: I35a4b54318d8169eb2c73be77ca2f30bbee08b46 --- diff --git a/neutron/db/l3_attrs_db.py b/neutron/db/l3_attrs_db.py index b04778ab7..d43cdc7b4 100644 --- a/neutron/db/l3_attrs_db.py +++ b/neutron/db/l3_attrs_db.py @@ -36,6 +36,10 @@ class RouterExtraAttributes(model_base.BASEV2): distributed = sa.Column(sa.Boolean, default=False, server_default=sa.sql.false(), nullable=False) + # Whether the router is to be considered a 'service' router + service_router = sa.Column(sa.Boolean, default=False, + server_default=sa.sql.false(), + nullable=False) router = orm.relationship( l3_db.Router, backref=orm.backref("extra_attributes", lazy='joined', diff --git a/neutron/db/migration/alembic_migrations/versions/884573acbf1c_unify_nsx_router_extra_attributes.py b/neutron/db/migration/alembic_migrations/versions/884573acbf1c_unify_nsx_router_extra_attributes.py new file mode 100644 index 000000000..2d1938b9a --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/884573acbf1c_unify_nsx_router_extra_attributes.py @@ -0,0 +1,73 @@ +# Copyright 2014 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""Drop NSX table in favor of the extra_attributes one + +Revision ID: 884573acbf1c +Revises: 5589aa32bf80 +Create Date: 2013-01-07 13:47:29.093160 + +""" + +revision = '884573acbf1c' +down_revision = '5589aa32bf80' + + +from alembic import op +import sqlalchemy as sa + + +def _migrate_data(old_table, new_table): + engine = op.get_bind().engine + if engine.name == 'postgresql': + op.execute(("UPDATE %(new_table)s new_t " + "SET distributed = old_t.distributed, " + "service_router = old_t.service_router " + "FROM %(old_table)s old_t " + "WHERE new_t.router_id = old_t.router_id") % + {'new_table': new_table, 'old_table': old_table}) + else: + op.execute(("UPDATE %(new_table)s new_t " + "INNER JOIN %(old_table)s as old_t " + "ON new_t.router_id = old_t.router_id " + "SET new_t.distributed = old_t.distributed, " + "new_t.service_router = old_t.service_router") % + {'new_table': new_table, 'old_table': old_table}) + + +def upgrade(active_plugins=None, options=None): + op.add_column('router_extra_attributes', + sa.Column('service_router', sa.Boolean(), + nullable=False, + server_default=sa.sql.false())) + _migrate_data('router_extra_attributes', 'nsxrouterextattributess') + op.drop_table('nsxrouterextattributess') + + +def downgrade(active_plugins=None, options=None): + op.create_table( + 'nsxrouterextattributess', + sa.Column('router_id', sa.String(length=36), nullable=False), + sa.Column('distributed', sa.Boolean(), nullable=False, + server_default=sa.sql.false()), + sa.Column('service_router', sa.Boolean(), nullable=False, + server_default=sa.sql.false()), + sa.ForeignKeyConstraint( + ['router_id'], ['routers.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('router_id') + ) + op.execute(("INSERT INTO nsxrouterextattributess " + "SELECT * from router_extra_attributes")) + op.drop_column('router_extra_attributes', 'service_router') diff --git a/neutron/db/migration/alembic_migrations/versions/HEAD b/neutron/db/migration/alembic_migrations/versions/HEAD index 1383ffcb3..f39590d47 100644 --- a/neutron/db/migration/alembic_migrations/versions/HEAD +++ b/neutron/db/migration/alembic_migrations/versions/HEAD @@ -1 +1 @@ -5589aa32bf80 +884573acbf1c diff --git a/neutron/plugins/vmware/common/exceptions.py b/neutron/plugins/vmware/common/exceptions.py index 3f435bd53..85032bfeb 100644 --- a/neutron/plugins/vmware/common/exceptions.py +++ b/neutron/plugins/vmware/common/exceptions.py @@ -119,3 +119,7 @@ class LsnMigrationConflict(n_exc.Conflict): class LsnConfigurationConflict(NsxPluginException): message = _("Configuration conflict on Logical Service Node %(lsn_id)s") + + +class ReadOnlyAttribute(NsxPluginException): + message = _("Cannot update read-only attribute %(attribute)s") diff --git a/neutron/plugins/vmware/dbexts/distributedrouter.py b/neutron/plugins/vmware/dbexts/distributedrouter.py deleted file mode 100644 index 5c6accbd3..000000000 --- a/neutron/plugins/vmware/dbexts/distributedrouter.py +++ /dev/null @@ -1,28 +0,0 @@ -# Copyright 2013 VMware, Inc. All rights reserved. -# -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# - -from neutron.plugins.vmware.dbexts import nsxrouter -from neutron.plugins.vmware.extensions import distributedrouter as dist_rtr - - -class DistributedRouter_mixin(nsxrouter.NsxRouterMixin): - """Mixin class to enable distributed router support.""" - - nsx_attributes = ( - nsxrouter.NsxRouterMixin.nsx_attributes + [{ - 'name': dist_rtr.DISTRIBUTED, - 'default': False - }]) diff --git a/neutron/plugins/vmware/dbexts/models.py b/neutron/plugins/vmware/dbexts/models.py index 53bf24f20..0c87dfa30 100644 --- a/neutron/plugins/vmware/dbexts/models.py +++ b/neutron/plugins/vmware/dbexts/models.py @@ -15,11 +15,8 @@ # under the License. -from sqlalchemy import Boolean, Column, Enum, ForeignKey, Integer, String -from sqlalchemy import orm -from sqlalchemy import sql +from sqlalchemy import Column, Enum, ForeignKey, Integer, String -from neutron.db import l3_db from neutron.db import model_base @@ -118,21 +115,4 @@ class MultiProviderNetworks(model_base.BASEV2): primary_key=True) def __init__(self, network_id): - self.network_id = network_id - - -class NSXRouterExtAttributes(model_base.BASEV2): - """Router attributes managed by NSX plugin extensions.""" - router_id = Column(String(36), - ForeignKey('routers.id', ondelete="CASCADE"), - primary_key=True) - distributed = Column(Boolean, default=False, server_default=sql.false(), - nullable=False) - service_router = Column(Boolean, default=False, server_default=sql.false(), - nullable=False) - # Add a relationship to the Router model in order to instruct - # SQLAlchemy to eagerly load this association - router = orm.relationship( - l3_db.Router, - backref=orm.backref("nsx_attributes", lazy='joined', - uselist=False, cascade='delete')) + self.network_id = network_id \ No newline at end of file diff --git a/neutron/plugins/vmware/dbexts/nsxrouter.py b/neutron/plugins/vmware/dbexts/nsxrouter.py deleted file mode 100644 index 48aa61266..000000000 --- a/neutron/plugins/vmware/dbexts/nsxrouter.py +++ /dev/null @@ -1,66 +0,0 @@ -# Copyright 2013 VMware, Inc. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# - -from neutron.db import db_base_plugin_v2 -from neutron.extensions import l3 -from neutron.openstack.common import log as logging -from neutron.plugins.vmware.dbexts import models - -LOG = logging.getLogger(__name__) - - -class NsxRouterMixin(object): - """Mixin class to enable nsx router support.""" - - nsx_attributes = [] - - def _extend_nsx_router_dict(self, router_res, router_db): - nsx_attrs = router_db['nsx_attributes'] - # Return False if nsx attributes are not definied for this - # neutron router - for attr in self.nsx_attributes: - name = attr['name'] - default = attr['default'] - router_res[name] = ( - nsx_attrs and nsx_attrs[name] or default) - - def _process_nsx_router_create( - self, context, router_db, router_req): - if not router_db['nsx_attributes']: - kwargs = {} - for attr in self.nsx_attributes: - name = attr['name'] - default = attr['default'] - kwargs[name] = router_req.get(name, default) - nsx_attributes = models.NSXRouterExtAttributes( - router_id=router_db['id'], **kwargs) - context.session.add(nsx_attributes) - router_db['nsx_attributes'] = nsx_attributes - else: - # The situation where the record already exists will - # be likely once the NSXRouterExtAttributes model - # will allow for defining several attributes pertaining - # to different extensions - for attr in self.nsx_attributes: - name = attr['name'] - default = attr['default'] - router_db['nsx_attributes'][name] = router_req.get( - name, default) - LOG.debug(_("Nsx router extension successfully processed " - "for router:%s"), router_db['id']) - - # Register dict extend functions for ports - db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs( - l3.ROUTERS, ['_extend_nsx_router_dict']) diff --git a/neutron/plugins/vmware/dbexts/servicerouter.py b/neutron/plugins/vmware/dbexts/servicerouter.py index bc34cd4c3..81ffb714f 100644 --- a/neutron/plugins/vmware/dbexts/servicerouter.py +++ b/neutron/plugins/vmware/dbexts/servicerouter.py @@ -13,15 +13,15 @@ # under the License. # -from neutron.plugins.vmware.dbexts import distributedrouter as dist_rtr +from neutron.db import l3_dvr_db from neutron.plugins.vmware.extensions import servicerouter -class ServiceRouter_mixin(dist_rtr.DistributedRouter_mixin): +class ServiceRouter_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin): """Mixin class to enable service router support.""" - nsx_attributes = ( - dist_rtr.DistributedRouter_mixin.nsx_attributes + [{ + extra_attributes = ( + l3_dvr_db.L3_NAT_with_dvr_db_mixin.extra_attributes + [{ 'name': servicerouter.SERVICE_ROUTER, 'default': False }]) diff --git a/neutron/plugins/vmware/extensions/distributedrouter.py b/neutron/plugins/vmware/extensions/distributedrouter.py index aa6949b82..0ec4af439 100644 --- a/neutron/plugins/vmware/extensions/distributedrouter.py +++ b/neutron/plugins/vmware/extensions/distributedrouter.py @@ -12,28 +12,14 @@ # License for the specific language governing permissions and limitations # under the License. -from neutron.api.v2 import attributes +# TODO(armando-migliaccio): This is deprecated in Juno, and +# to be removed in Kilo. +from neutron.extensions import dvr -def convert_to_boolean_if_not_none(data): - if data is not None: - return attributes.convert_to_boolean(data) - return data - -DISTRIBUTED = 'distributed' -EXTENDED_ATTRIBUTES_2_0 = { - 'routers': { - DISTRIBUTED: {'allow_post': True, 'allow_put': False, - 'convert_to': convert_to_boolean_if_not_none, - 'default': attributes.ATTR_NOT_SPECIFIED, - 'is_visible': True}, - } -} - - -class Distributedrouter(object): - """Extension class supporting distributed router.""" +class Distributedrouter(dvr.Dvr): + """(Deprecated) Extension class supporting distributed router.""" @classmethod def get_name(cls): @@ -45,26 +31,9 @@ class Distributedrouter(object): @classmethod def get_description(cls): - return "Enables configuration of NSX Distributed routers." + return ("Enables configuration of NSX " + "Distributed routers (Deprecated).") @classmethod def get_namespace(cls): return "http://docs.openstack.org/ext/dist-router/api/v1.0" - - @classmethod - def get_updated(cls): - return "2013-08-1T10:00:00-00:00" - - def get_required_extensions(self): - return ["router"] - - @classmethod - def get_resources(cls): - """Returns Ext Resources.""" - return [] - - def get_extended_resources(self, version): - if version == "2.0": - return EXTENDED_ATTRIBUTES_2_0 - else: - return {} diff --git a/neutron/plugins/vmware/plugins/base.py b/neutron/plugins/vmware/plugins/base.py index e59c527ee..6090b0d66 100644 --- a/neutron/plugins/vmware/plugins/base.py +++ b/neutron/plugins/vmware/plugins/base.py @@ -35,6 +35,7 @@ from neutron.db import db_base_plugin_v2 from neutron.db import external_net_db from neutron.db import extraroute_db from neutron.db import l3_db +from neutron.db import l3_dvr_db from neutron.db import l3_gwmode_db from neutron.db import models_v2 from neutron.db import portbindings_db @@ -62,7 +63,6 @@ from neutron.plugins.vmware.common import securitygroups as sg_utils from neutron.plugins.vmware.common import sync from neutron.plugins.vmware.common import utils as c_utils from neutron.plugins.vmware.dbexts import db as nsx_db -from neutron.plugins.vmware.dbexts import distributedrouter as dist_rtr from neutron.plugins.vmware.dbexts import maclearning as mac_db from neutron.plugins.vmware.dbexts import networkgw_db from neutron.plugins.vmware.dbexts import qos_db @@ -88,7 +88,7 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin, agentschedulers_db.DhcpAgentSchedulerDbMixin, db_base_plugin_v2.NeutronDbPluginV2, dhcpmeta_modes.DhcpMetadataAccess, - dist_rtr.DistributedRouter_mixin, + l3_dvr_db.L3_NAT_with_dvr_db_mixin, external_net_db.External_net_db_mixin, extraroute_db.ExtraRoute_db_mixin, l3_gwmode_db.L3_NAT_db_mixin, @@ -101,6 +101,7 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin, supported_extension_aliases = ["allowed-address-pairs", "binding", + "dvr", "dist-router", "ext-gw-mode", "extraroute", @@ -740,6 +741,8 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin, webob.exc.HTTPBadRequest, nsx_exc.NoMorePortsException: webob.exc.HTTPBadRequest, + nsx_exc.ReadOnlyAttribute: + webob.exc.HTTPBadRequest, nsx_exc.MaintenanceInProgress: webob.exc.HTTPServiceUnavailable, nsx_exc.InvalidSecurityCertificate: @@ -1438,7 +1441,7 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin, admin_state_up=r['admin_state_up'], status=lrouter['status']) context.session.add(router_db) - self._process_nsx_router_create(context, router_db, r) + self._process_extra_attr_router_create(context, router_db, r) # Ensure neutron router is moved into the transaction's buffer context.session.flush() # Add mapping between neutron and nsx identifiers @@ -1481,6 +1484,9 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin, self.cluster, nsx_router_id, routes) def update_router(self, context, router_id, router): + if isinstance(router['router'].get('distributed'), bool): + # Router conversion is not supported + raise nsx_exc.ReadOnlyAttribute(attribute='distributed') # Either nexthop is updated or should be kept as it was before r = router['router'] nexthop = None diff --git a/neutron/plugins/vmware/plugins/service.py b/neutron/plugins/vmware/plugins/service.py index 3db8fd8cd..1f5e503b6 100644 --- a/neutron/plugins/vmware/plugins/service.py +++ b/neutron/plugins/vmware/plugins/service.py @@ -142,7 +142,7 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin, router = self._get_router(context, router_id) LOG.debug(_("EDGE: router = %s"), router) - if router['nsx_attributes']['service_router']: + if router['extra_attributes']['service_router']: router_type = ROUTER_TYPE_ADVANCED else: router_type = ROUTER_TYPE_BASIC diff --git a/neutron/tests/unit/vmware/test_nsx_plugin.py b/neutron/tests/unit/vmware/test_nsx_plugin.py index fad5fa056..ecb8ed8b4 100644 --- a/neutron/tests/unit/vmware/test_nsx_plugin.py +++ b/neutron/tests/unit/vmware/test_nsx_plugin.py @@ -28,6 +28,7 @@ from neutron.common import constants from neutron.common import exceptions as ntn_exc import neutron.common.test_lib as test_lib from neutron import context +from neutron.extensions import dvr from neutron.extensions import external_net from neutron.extensions import l3 from neutron.extensions import l3_ext_gw_mode @@ -43,7 +44,6 @@ from neutron.plugins.vmware.common import exceptions as nsx_exc from neutron.plugins.vmware.common import sync from neutron.plugins.vmware.common import utils from neutron.plugins.vmware.dbexts import db as nsx_db -from neutron.plugins.vmware.extensions import distributedrouter as dist_router from neutron.plugins.vmware import nsxlib from neutron.tests.unit import _test_extension_portbindings as test_bindings import neutron.tests.unit.test_db_plugin as test_plugin @@ -408,7 +408,7 @@ class TestL3ExtensionManager(object): l3.RESOURCE_ATTRIBUTE_MAP[key].update( l3_ext_gw_mode.EXTENDED_ATTRIBUTES_2_0.get(key, {})) l3.RESOURCE_ATTRIBUTE_MAP[key].update( - dist_router.EXTENDED_ATTRIBUTES_2_0.get(key, {})) + dvr.EXTENDED_ATTRIBUTES_2_0.get(key, {})) # Finally add l3 resources to the global attribute map attributes.RESOURCE_ATTRIBUTE_MAP.update( l3.RESOURCE_ATTRIBUTE_MAP) @@ -1225,6 +1225,16 @@ class NeutronNsxOutOfSync(NsxPluginV2TestCase, self._test_remove_router_interface_nsx_out_of_sync(unsync_action) + def test_update_router_distributed_bad_request(self): + res = self._create_router('json', 'tenant') + router = self.deserialize('json', res) + req = self.new_update_request( + 'routers', + {'router': {'distributed': True}}, + router['router']['id']) + res = req.get_response(self.ext_api) + self.assertEqual(res.status_int, 400) + def test_update_router_not_in_nsx(self): res = self._create_router('json', 'tenant') router = self.deserialize('json', res)