Merge "Make the MTU attribute not nullable"

neutron/cmd/upgrade_checks/checks.py

@@ -22,6 +22,7 @@ from oslo_upgradecheck import upgradecheck
 from neutron._i18n import _
 from neutron.cmd.upgrade_checks import base
 from neutron.db.models import agent as agent_model
+from neutron.db import models_v2
 
 
 def get_l3_agents():
@@ -33,6 +34,13 @@ def get_l3_agents():
     return query.all()
 
 
+def get_networks():
+    ctx = context.get_admin_context()
+    query = model_query.get_collection_query(ctx,
+                                             models_v2.Network)
+    return query.all()
+
+
 class CoreChecks(base.BaseChecks):
 
     def get_checks(self):
@@ -123,3 +131,28 @@ class CoreChecks(base.BaseChecks):
             return upgradecheck.Result(
                 upgradecheck.Code.SUCCESS,
                 _("L3 agents can use multiple networks as external gateways."))
+
+    @staticmethod
+    def network_mtu_check(checker):
+        if not cfg.CONF.database.connection:
+            return upgradecheck.Result(
+                upgradecheck.Code.WARNING,
+                _("Database connection string is not set. Check of 'mtu' in "
+                  "networks can't be done"))
+
+        networks_with_empty_mtu_attr = []
+        for network in get_networks():
+            mtu = network.get('mtu', None)
+            if not mtu:
+                networks_with_empty_mtu_attr.append(network.get("id"))
+
+        if networks_with_empty_mtu_attr:
+            networks_list = ", ".join(networks_with_empty_mtu_attr)
+            return upgradecheck.Result(
+                upgradecheck.Code.WARNING,
+                _("The 'mtu' attribute of networks %s are not set "
+                  "This attribute can't be null now.") % networks_list)
+        else:
+            return upgradecheck.Result(
+                upgradecheck.Code.SUCCESS,
+                _("The 'mtu' attribute of all networks are set."))

neutron/db/db_base_plugin_v2.py

@@ -404,7 +404,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
             args = {'tenant_id': n['tenant_id'],
                     'id': n.get('id') or uuidutils.generate_uuid(),
                     'name': n['name'],
-                    'mtu': n.get('mtu'),
+                    'mtu': n.get('mtu', constants.DEFAULT_NETWORK_MTU),
                     'admin_state_up': n['admin_state_up'],
                     'status': n.get('status', constants.NET_STATUS_ACTIVE),
                     'description': n.get('description')}

neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD

@@ -1 +1 @@
-c3e9d13c4367
+86274d77933e

neutron/db/migration/alembic_migrations/versions/ussuri/expand/86274d77933e_change_mtu_to_not_null.py

@@ -0,0 +1,55 @@
+# Copyright 2019 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.
+#
+
+from alembic import op
+from neutron_lib import constants
+import sqlalchemy as sa
+
+
+"""change_mtu_to_not_null
+
+Revision ID: 86274d77933e
+Revises: c3e9d13c4367
+Create Date: 2019-08-30 15:52:30.015146
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '86274d77933e'
+down_revision = 'c3e9d13c4367'
+
+
+networks = sa.Table(
+    'networks', sa.MetaData(),
+    sa.Column('id', sa.String(length=36), nullable=False),
+    sa.Column('mtu', sa.Integer(), nullable=True))
+
+
+def upgrade_existing_records():
+    session = sa.orm.Session(bind=op.get_bind())
+    with session.begin(subtransactions=True):
+        for row in session.query(networks):
+            if row[1] is None:
+                session.execute(networks.update().values(
+                    mtu=constants.DEFAULT_NETWORK_MTU).where(
+                    networks.c.id == row[0]))
+    session.commit()
+
+
+def upgrade():
+    upgrade_existing_records()
+    op.alter_column('networks', 'mtu', nullable=False,
+                    server_default=str(constants.DEFAULT_NETWORK_MTU),
+                    existing_type=sa.INTEGER())

neutron/db/models_v2.py

@@ -268,9 +268,9 @@ class Network(standard_attr.HasStandardAttributes, model_base.BASEV2,
                                     lazy='subquery',
                                     cascade='all, delete, delete-orphan')
     availability_zone_hints = sa.Column(sa.String(255))
-    # TODO(ihrachys) provide data migration path to fill in mtus for existing
+    mtu = sa.Column(sa.Integer, nullable=False,
-    # networks in Queens when all controllers run Pike+ code
+                    default=constants.DEFAULT_NETWORK_MTU,
-    mtu = sa.Column(sa.Integer, nullable=True)
+                    server_default=str(constants.DEFAULT_NETWORK_MTU))
     dhcp_agents = orm.relationship(
         'Agent', lazy='subquery', viewonly=True,
         secondary=ndab_model.NetworkDhcpAgentBinding.__table__)

neutron/objects/network.py

@@ -14,7 +14,9 @@
 
 from neutron_lib.api.definitions import availability_zone as az_def
 from neutron_lib.api.validators import availability_zone as az_validator
+from neutron_lib import constants
 from oslo_utils import versionutils
+from oslo_versionedobjects import exception
 from oslo_versionedobjects import fields as obj_fields
 import sqlalchemy as sa
 
@@ -214,7 +216,8 @@ class ExternalNetwork(base.NeutronDbObject):
 @base.NeutronObjectRegistry.register
 class Network(rbac_db.NeutronRbacObject):
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Changed 'mtu' to be not nullable
+    VERSION = '1.1'
 
     rbac_db_cls = NetworkRBAC
     db_model = models_v2.Network
@@ -231,7 +234,7 @@ class Network(rbac_db.NeutronRbacObject):
             nullable=True),
         'shared': obj_fields.BooleanField(default=False),
 
-        'mtu': obj_fields.IntegerField(nullable=True),
+        'mtu': obj_fields.IntegerField(default=constants.DEFAULT_NETWORK_MTU),
 
         # TODO(ihrachys): consider exposing availability zones
 
@@ -344,6 +347,14 @@ class Network(rbac_db.NeutronRbacObject):
         # TODO(ihrachys): provide actual implementation
         return set()
 
+    def obj_make_compatible(self, primitive, target_version):
+        _target_version = versionutils.convert_version_to_tuple(target_version)
+        if _target_version >= (1, 1):
+            if primitive['mtu'] is None:
+                # mtu will not be nullable after
+                raise exception.IncompatibleObjectVersion(
+                    objver=target_version, objname=self.__class__.__name__)
+
 
 @base.NeutronObjectRegistry.register
 class SegmentHostMapping(base.NeutronDbObject):

neutron/plugins/ml2/plugin.py

@@ -1144,12 +1144,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         with db_api.CONTEXT_WRITER.using(context):
             net_db = self._get_network(context, id)
 
-            # NOTE(ihrachys) pre Pike networks may have null mtus; update them
-            # in database if needed
-            # TODO(ihrachys) remove in Queens+ when mtu is not nullable
-            if net_db.mtu is None:
-                net_db.mtu = self._get_network_mtu(net_db, validate=False)
-
             net_data = self._make_network_dict(net_db, context=context)
             self.type_manager.extend_network_dict_provider(context, net_data)
 
@@ -1164,13 +1158,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             nets_db = super(Ml2Plugin, self)._get_networks(
                 context, filters, None, sorts, limit, marker, page_reverse)
 
-            # NOTE(ihrachys) pre Pike networks may have null mtus; update them
-            # in database if needed
-            # TODO(ihrachys) remove in Queens+ when mtu is not nullable
             net_data = []
             for net in nets_db:
-                if net.mtu is None:
-                    net.mtu = self._get_network_mtu(net, validate=False)
                 net_data.append(self._make_network_dict(net, context=context))
 
             self.type_manager.extend_networks_dict_provider(context, net_data)
@@ -1241,12 +1230,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             result, net_db, ipam_sub = self._create_subnet_precommit(
                 context, subnet)
 
-            # NOTE(ihrachys) pre Pike networks may have null mtus; update them
-            # in database if needed
-            # TODO(ihrachys) remove in Queens+ when mtu is not nullable
-            if net_db['mtu'] is None:
-                net_db['mtu'] = self._get_network_mtu(net_db, validate=False)
-
             self.extension_manager.process_create_subnet(
                 context, subnet[subnet_def.RESOURCE_NAME], result)
             network = self._make_network_dict(net_db, context=context)

neutron/tests/unit/cmd/upgrade_checks/test_checks.py

@@ -106,3 +106,25 @@ class TestChecks(base.BaseTestCase):
             self.assertIn('Host B', result.details)
             self.assertNotIn('Host A', result.details)
             self.assertNotIn('Host C', result.details)
+
+    def test_network_mtu_check_good(self):
+        networks = [
+            {'id': 'net-uuid-a', 'mtu': 1500},
+            {'id': 'net-uuid-b', 'mtu': 1450}
+        ]
+        with mock.patch.object(checks, "get_networks", return_value=networks):
+            result = checks.CoreChecks.network_mtu_check(
+                mock.Mock())
+            self.assertEqual(Code.SUCCESS, result.code)
+
+    def test_network_mtu_check_bad(self):
+        networks = [
+            {'id': 'net-uuid-a', 'mtu': None},
+            {'id': 'net-uuid-b', 'mtu': 1500},
+        ]
+        with mock.patch.object(checks, "get_networks", return_value=networks):
+            result = checks.CoreChecks.network_mtu_check(
+                mock.Mock())
+            self.assertEqual(Code.WARNING, result.code)
+            self.assertIn('net-uuid-a', result.details)
+            self.assertNotIn('net-uuid-b', result.details)

neutron/tests/unit/db/test_db_base_plugin_v2.py

@@ -6503,7 +6503,8 @@ class DbModelMixin(object):
         with db_api.CONTEXT_WRITER.using(ctx):
             network = models_v2.Network(name="net_net", status="OK",
                                         admin_state_up=True,
-                                        project_id='fake_project')
+                                        project_id='fake_project',
+                                        mtu=1500)
             ctx.session.add(network)
             with db_api.autonested_transaction(ctx.session):
                 sg = sg_models.SecurityGroup(name='sg', description='sg')
@@ -6516,7 +6517,8 @@ class DbModelMixin(object):
                 project_id=network.project_id,
                 target_tenant='*').create()
             net2 = models_v2.Network(name="net_net2", status="OK",
-                                     admin_state_up=True)
+                                     admin_state_up=True,
+                                     mtu=1500)
             ctx.session.add(net2)
         pl = db_base_plugin_common.DbBasePluginCommon()
         self.assertTrue(pl._make_network_dict(network, context=ctx)['shared'])

neutron/tests/unit/objects/test_objects.py

@@ -57,7 +57,7 @@ object_data = {
     'MeteringLabel': '1.0-cc4b620a3425222447cbe459f62de533',
     'MeteringLabelRule': '1.0-b5c5717e7bab8d1af1623156012a5842',
     'Log': '1.0-6391351c0f34ed34375a19202f361d24',
-    'Network': '1.0-f2f6308f79731a767b92b26b0f4f3849',
+    'Network': '1.1-c3e9ecc0618ee934181d91b143a48901',
     'NetworkDhcpAgentBinding': '1.1-d9443c88809ffa4c45a0a5a48134b54a',
     'NetworkDNSDomain': '1.0-420db7910294608534c1e2e30d6d8319',
     'NetworkPortSecurity': '1.0-b30802391a87945ee9c07582b4ff95e3',

releasenotes/notes/make-mtu-not-nullable-2b2765bc85379545.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    The network ``mtu`` attribute is set to be non-nullable. If the ``mtu`` is
+    empty(create before Pike version), it is set to the default value of 1500.