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

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

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')

neutron/db/migration/alembic_migrations/versions/HEAD

@@ -1 +1 @@
-5589aa32bf80
+884573acbf1c

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

neutron/plugins/vmware/dbexts/distributedrouter.py

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

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 Column, Enum, ForeignKey, Integer, String
-from sqlalchemy import orm
-from sqlalchemy import sql
 
-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
+        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'))

neutron/plugins/vmware/dbexts/nsxrouter.py

@@ -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'])

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 = (
+    extra_attributes = (
-        dist_rtr.DistributedRouter_mixin.nsx_attributes + [{
+        l3_dvr_db.L3_NAT_with_dvr_db_mixin.extra_attributes + [{
             'name': servicerouter.SERVICE_ROUTER,
             'default': False
         }])

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):
+class Distributedrouter(dvr.Dvr):
-    if data is not None:
+    """(Deprecated) Extension class supporting distributed router."""
-        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."""
 
     @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 {}

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

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

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)