openstack
/
ironic
Code Issues Proposed changes

Add is_smartnic to Port data model 

To allow the use of Smart NICs on baremetal nodes it is required to mark
an ironic port as a Smart NIC, this change implement the changes required
for adding port attribute 'is_smartnic' in Port object and data model
alongside with migration scripts and unit tests.

Story: #2003346
Change-Id: Ic2ffbd6f1035907ea5a18bda6d2b21e617194195
This commit is contained in:
Hamdy Khader 2019-01-09 21:04:55 +02:00
parent 10bf5d5c2a
commit 86e31bff41
9 changed files with 177 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ironic/common/release_mappings.py
View File

@ -138,7 +138,7 @@ RELEASE_MAPPING = {
'Node': ['1.31', '1.30', '1.29', '1.28'],
'Conductor': ['1.3'],
'Chassis': ['1.3'],
'Port': ['1.8'],
'Port': ['1.9'],
'Portgroup': ['1.4'],
'Trait': ['1.0'],
'TraitList': ['1.0'],

32
ironic/db/sqlalchemy/alembic/versions/9cbeefa3763f_add_port_is_smartnic.py Normal file
View File

@ -0,0 +1,32 @@
# 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.
"""add is_smartnic port attribute
Revision ID: 9cbeefa3763f
Revises: dd67b91a1981
Create Date: 2019-01-13 09:31:13.336479
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = '9cbeefa3763f'
down_revision = 'dd67b91a1981'
def upgrade():
op.add_column('ports', sa.Column('is_smartnic', sa.Boolean(),
default=False))

1
ironic/db/sqlalchemy/models.py
View File

@ -215,6 +215,7 @@ class Port(Base):
pxe_enabled = Column(Boolean, default=True)
internal_info = Column(db_types.JsonEncodedDict)
physical_network = Column(String(64), nullable=True)
is_smartnic = Column(Boolean, nullable=True, default=False)
class Portgroup(Base):

47
ironic/objects/port.py
View File

@ -41,7 +41,7 @@ def migrate_vif_port_id(context, max_count):
# NOTE(rloo): if we introduce newer port versions in the same cycle,
# we could add those versions along with 1.8. This is only so we don't
# duplicate work; it isn't necessary.
db_ports = Port.dbapi.get_not_versions('Port', ['1.8'])
db_ports = Port.dbapi.get_not_versions('Port', ['1.8', '1.9'])
total = len(db_ports)
max_count = max_count or total
done = 0
@ -71,7 +71,8 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
# Version 1.8: Migrate/copy extra['vif_port_id'] to
# internal_info['tenant_vif_port_id'] (not an explicit db
# change)
VERSION = '1.8'
# Version 1.9: Add support for Smart NIC port
VERSION = '1.9'
dbapi = dbapi.get_instance()
@ -87,6 +88,8 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
'pxe_enabled': object_fields.BooleanField(),
'internal_info': object_fields.FlexibleDictField(nullable=True),
'physical_network': object_fields.StringField(nullable=True),
'is_smartnic': object_fields.BooleanField(nullable=True,
default=False),
}
def _convert_to_version(self, target_version,
@ -106,6 +109,9 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
.extra value to internal_info. There is nothing to do here when
downgrading to an older version.
Version 1.9: remove is_smartnic field for unsupported versions if
remove_unavailable_fields is True.
:param target_version: the desired version of the object
:param remove_unavailable_fields: True to remove fields that are
unavailable in the target version; set this to True when
@ -140,6 +146,24 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
# DB: set unavailable fields to their default.
self.physical_network = None
# Convert is_smartnic field.
is_smartnic_set = self.obj_attr_is_set('is_smartnic')
if target_version >= (1, 9):
# Target version supports is_smartnic. Set it to its default
# value if it is not set.
if not is_smartnic_set:
self.is_smartnic = False
# handle is_smartnic field in older version
elif is_smartnic_set:
# Target version does not support is_smartnic, and it is set.
if remove_unavailable_fields:
# (De)serialising: remove unavailable fields.
delattr(self, 'is_smartnic')
elif self.is_smartnic is not False:
# DB: set unavailable fields to their default.
self.is_smartnic = False
# NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
# methods can be used in the future to replace current explicit RPC calls.
# Implications of calling new remote procedures should be thought through.
@ -393,6 +417,15 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
"""
return cls.supports_version((1, 7))
@classmethod
def supports_is_smartnic(cls):
"""Return whether is_smartnic field is supported.
:returns: Whether is_smartnic field is supported
:raises: ovo_exception.IncompatibleObjectVersion
"""
return cls.supports_version((1, 9))
@base.IronicObjectRegistry.register
class PortCRUDNotification(notification.NotificationBase):
@ -410,7 +443,8 @@ class PortCRUDPayload(notification.NotificationPayloadBase):
# Version 1.0: Initial version
# Version 1.1: Add "portgroup_uuid" field
# Version 1.2: Add "physical_network" field
VERSION = '1.2'
# Version 1.3: Add "is_smartnic" field
VERSION = '1.3'
SCHEMA = {
'address': ('port', 'address'),
@ -420,7 +454,8 @@ class PortCRUDPayload(notification.NotificationPayloadBase):
'physical_network': ('port', 'physical_network'),
'created_at': ('port', 'created_at'),
'updated_at': ('port', 'updated_at'),
'uuid': ('port', 'uuid')
'uuid': ('port', 'uuid'),
'is_smartnic': ('port', 'is_smartnic'),
}
fields = {
@ -434,7 +469,9 @@ class PortCRUDPayload(notification.NotificationPayloadBase):
'physical_network': object_fields.StringField(nullable=True),
'created_at': object_fields.DateTimeField(nullable=True),
'updated_at': object_fields.DateTimeField(nullable=True),
'uuid': object_fields.UUIDField()
'uuid': object_fields.UUIDField(),
'is_smartnic': object_fields.BooleanField(nullable=True,
default=False),
}
def __init__(self, port, node_uuid, portgroup_uuid):

3
ironic/tests/unit/api/utils.py
View File

@ -119,6 +119,9 @@ def port_post_data(**kw):
port.pop('version')
port.pop('node_id')
port.pop('portgroup_id')
# TODO(hamdyk): remove when port API can handle this attribute
port.pop('is_smartnic')
internal = port_controller.PortPatchType.internal_attrs()
return remove_internal(port, internal)

9
ironic/tests/unit/db/sqlalchemy/test_migrations.py
View File

@ -842,6 +842,15 @@ class MigrationCheckersMixin(object):
self.assertIsInstance(allocations.c.conductor_affinity.type,
sqlalchemy.types.Integer)
def _check_9cbeefa3763f(self, engine, data):
ports = db_utils.get_table(engine, 'ports')
col_names = [column.name for column in ports.c]
self.assertIn('is_smartnic', col_names)
# in some backends bool type is integer
self.assertIsInstance(ports.c.is_smartnic.type,
(sqlalchemy.types.Boolean,
sqlalchemy.types.Integer))
def test_upgrade_and_version(self):
with patch_with_engine(self.engine):
self.migration_api.upgrade('head')

1
ironic/tests/unit/db/utils.py
View File

@ -269,6 +269,7 @@ def get_test_port(**kw):
'pxe_enabled': kw.get('pxe_enabled', True),
'internal_info': kw.get('internal_info', {"bar": "buzz"}),
'physical_network': kw.get('physical_network'),
'is_smartnic': kw.get('is_smartnic', False),
}

4
ironic/tests/unit/objects/test_objects.py
View File

@ -680,7 +680,7 @@ expected_object_fingerprints = {
'Node': '1.31-1b77c11e94f971a71c76f5f44fb5b3f4',
'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6',
'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905',
'Port': '1.8-898a47921f4a1f53fcdddd4eeb179e0b',
'Port': '1.9-0cb9202a4ec442e8c0d87a324155eaaf',
'Portgroup': '1.4-71923a81a86743b313b190f5c675e258',
'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a',
'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370',
@ -701,7 +701,7 @@ expected_object_fingerprints = {
'NodeCRUDNotification': '1.0-59acc533c11d306f149846f922739c15',
'NodeCRUDPayload': '1.10-49590dee863c5ed1193f5deae0a0a2f2',
'PortCRUDNotification': '1.0-59acc533c11d306f149846f922739c15',
'PortCRUDPayload': '1.2-233d259df442eb15cc584fae1fe81504',
'PortCRUDPayload': '1.3-21235916ed54a91b2a122f59571194e7',
'NodeMaintenanceNotification': '1.0-59acc533c11d306f149846f922739c15',
'NodeConsoleNotification': '1.0-59acc533c11d306f149846f922739c15',
'PortgroupCRUDNotification': '1.0-59acc533c11d306f149846f922739c15',

90
ironic/tests/unit/objects/test_port.py
View File

@ -165,6 +165,20 @@ class TestPortObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
def test_payload_schemas(self):
self._check_payload_schemas(objects.port, objects.Port.fields)
@mock.patch.object(obj_base.IronicObject, 'supports_version',
spec_set=types.FunctionType)
def test_supports_is_smartnic_supported(self, mock_sv):
mock_sv.return_value = True
self.assertTrue(objects.Port.supports_is_smartnic())
mock_sv.assert_called_once_with((1, 9))
@mock.patch.object(obj_base.IronicObject, 'supports_version',
spec_set=types.FunctionType)
def test_supports_is_smartnic_unsupported(self, mock_sv):
mock_sv.return_value = False
self.assertFalse(objects.Port.supports_is_smartnic())
mock_sv.assert_called_once_with((1, 9))
class TestConvertToVersion(db_base.DbTestCase):
@ -255,6 +269,72 @@ class TestConvertToVersion(db_base.DbTestCase):
# no change
self.assertEqual(vif2, port.internal_info['tenant_vif_port_id'])
def test_is_smartnic_unsupported(self):
port = objects.Port(self.context, **self.fake_port)
port._convert_to_version("1.8")
self.assertNotIn('is_smartnic', port)
def test_is_smartnic_supported(self):
port = objects.Port(self.context, **self.fake_port)
port._convert_to_version("1.9")
self.assertIn('is_smartnic', port)
def test_is_smartnic_supported_missing(self):
# is_smartnic is not set, should be set to default.
port = objects.Port(self.context, **self.fake_port)
delattr(port, 'is_smartnic')
port.obj_reset_changes()
port._convert_to_version("1.9")
self.assertFalse(port.is_smartnic)
self.assertIn('is_smartnic', port.obj_get_changes())
self.assertFalse(port.obj_get_changes()['is_smartnic'])
def test_is_smartnic_supported_set(self):
# is_smartnic is set, no change required.
port = objects.Port(self.context, **self.fake_port)
port.is_smartnic = True
port.obj_reset_changes()
port._convert_to_version("1.9")
self.assertTrue(port.is_smartnic)
self.assertNotIn('is_smartnic', port.obj_get_changes())
def test_is_smartnic_unsupported_missing(self):
# is_smartnic is not set, no change required.
port = objects.Port(self.context, **self.fake_port)
delattr(port, 'is_smartnic')
port.obj_reset_changes()
port._convert_to_version("1.8")
self.assertNotIn('is_smartnic', port)
self.assertNotIn('is_smartnic', port.obj_get_changes())
def test_is_smartnic_unsupported_set_remove(self):
# is_smartnic is set, should be removed.
port = objects.Port(self.context, **self.fake_port)
port.is_smartnic = False
port.obj_reset_changes()
port._convert_to_version("1.8")
self.assertNotIn('is_smartnic', port)
self.assertNotIn('is_smartnic', port.obj_get_changes())
def test_is_smartnic_unsupported_set_no_remove_non_default(self):
# is_smartnic is set, should be set to default.
port = objects.Port(self.context, **self.fake_port)
port.is_smartnic = True
port.obj_reset_changes()
port._convert_to_version("1.8", False)
self.assertFalse(port.is_smartnic)
self.assertIn('is_smartnic', port.obj_get_changes())
self.assertFalse(port.obj_get_changes()['is_smartnic'])
def test_is_smartnic_unsupported_set_no_remove_default(self):
# is_smartnic is set, no change required.
port = objects.Port(self.context, **self.fake_port)
port.is_smartnic = False
port.obj_reset_changes()
port._convert_to_version("1.8", False)
self.assertFalse(port.is_smartnic)
self.assertNotIn('is_smartnic', port.obj_get_changes())
class TestMigrateVifPortId(db_base.DbTestCase):
@ -278,9 +358,10 @@ class TestMigrateVifPortId(db_base.DbTestCase):
total, done = objects.port.migrate_vif_port_id(self.context, 0)
self.assertEqual(3, total)
self.assertEqual(3, done)
mock_get_not_versions.assert_called_once_with('Port', ['1.8'])
mock_get_not_versions.assert_called_once_with('Port', ['1.8',
'1.9'])
calls = 3 * [
mock.call(mock.ANY, '1.8', remove_unavailable_fields=False),
mock.call(mock.ANY, '1.9', remove_unavailable_fields=False),
]
self.assertEqual(calls, mock_convert.call_args_list)
@ -292,8 +373,9 @@ class TestMigrateVifPortId(db_base.DbTestCase):
total, done = objects.port.migrate_vif_port_id(self.context, 1)
self.assertEqual(3, total)
self.assertEqual(1, done)
mock_get_not_versions.assert_called_once_with('Port', ['1.8'])
mock_get_not_versions.assert_called_once_with('Port', ['1.8',
'1.9'])
calls = [
mock.call(mock.ANY, '1.8', remove_unavailable_fields=False),
mock.call(mock.ANY, '1.9', remove_unavailable_fields=False),
]
self.assertEqual(calls, mock_convert.call_args_list)