diff --git a/neutron/cmd/upgrade_checks/checks.py b/neutron/cmd/upgrade_checks/checks.py index 07439a101d0..637e338b31c 100644 --- a/neutron/cmd/upgrade_checks/checks.py +++ b/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.")) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index b47ee562e3d..f75104c00eb 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/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')} diff --git a/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD b/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD index e7189617eb7..67d8fc4faeb 100644 --- a/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD +++ b/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD @@ -1 +1 @@ -c3e9d13c4367 +86274d77933e diff --git a/neutron/db/migration/alembic_migrations/versions/ussuri/expand/86274d77933e_change_mtu_to_not_null.py b/neutron/db/migration/alembic_migrations/versions/ussuri/expand/86274d77933e_change_mtu_to_not_null.py new file mode 100644 index 00000000000..2a048a39faf --- /dev/null +++ b/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()) diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index cd82965c087..e56eb4a81a9 100644 --- a/neutron/db/models_v2.py +++ b/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 - # networks in Queens when all controllers run Pike+ code - mtu = sa.Column(sa.Integer, nullable=True) + mtu = sa.Column(sa.Integer, nullable=False, + default=constants.DEFAULT_NETWORK_MTU, + server_default=str(constants.DEFAULT_NETWORK_MTU)) dhcp_agents = orm.relationship( 'Agent', lazy='subquery', viewonly=True, secondary=ndab_model.NetworkDhcpAgentBinding.__table__) diff --git a/neutron/objects/network.py b/neutron/objects/network.py index 22f0bade516..560bcb2bd3a 100644 --- a/neutron/objects/network.py +++ b/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): diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 3edc649ac8f..167a82de975 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/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) diff --git a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py b/neutron/tests/unit/cmd/upgrade_checks/test_checks.py index fd018bd4ba9..64a48b47136 100644 --- a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py +++ b/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) diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 47ff036a4e4..ea3cdd288a6 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/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']) diff --git a/neutron/tests/unit/objects/test_objects.py b/neutron/tests/unit/objects/test_objects.py index a72590353b4..647ea723a77 100644 --- a/neutron/tests/unit/objects/test_objects.py +++ b/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', diff --git a/releasenotes/notes/make-mtu-not-nullable-2b2765bc85379545.yaml b/releasenotes/notes/make-mtu-not-nullable-2b2765bc85379545.yaml new file mode 100644 index 00000000000..668d2817582 --- /dev/null +++ b/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.