openstack/neutron
Code Issues Proposed changes

Make the MTU attribute not nullable

Browse Source 
This patch sets the MTU attribute to non-nullable, the code
that get the MTU and update the network in some methods can
be removed. If the MTU is empty before the pike version, it
is set to the default value of 1500.

Change-Id: Id4d738dde7fa4b7caccabad0aac542b82b4d7af1
Closes-Bug: #1842261
This commit is contained in:
zhanghao 2019-10-26 16:57:27 -04:00
parent f5bdaec2c7
commit 68625686a4
11 changed files with 138 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
neutron/cmd/upgrade_checks/checks.py
View File

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

2
neutron/db/db_base_plugin_v2.py
View File

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

2
neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD
View File

@ -1 +1 @@
c3e9d13c4367
86274d77933e

55
neutron/db/migration/alembic_migrations/versions/ussuri/expand/86274d77933e_change_mtu_to_not_null.py Normal file
View File

@ -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())

6
neutron/db/models_v2.py
View File

@ -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__)

15
neutron/objects/network.py
View File

@ -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):

17
neutron/plugins/ml2/plugin.py
View File

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

22
neutron/tests/unit/cmd/upgrade_checks/test_checks.py
View File

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

6
neutron/tests/unit/db/test_db_base_plugin_v2.py
View File

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

2
neutron/tests/unit/objects/test_objects.py
View File

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

5
releasenotes/notes/make-mtu-not-nullable-2b2765bc85379545.yaml Normal file
View File

@ -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.