From 86e31bff417b2ec9412e9bd431edb91d11b6d42c Mon Sep 17 00:00:00 2001 From: Hamdy Khader Date: Wed, 9 Jan 2019 21:04:55 +0200 Subject: [PATCH] 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 --- ironic/common/release_mappings.py | 2 +- .../9cbeefa3763f_add_port_is_smartnic.py | 32 +++++++ ironic/db/sqlalchemy/models.py | 1 + ironic/objects/port.py | 47 ++++++++-- ironic/tests/unit/api/utils.py | 3 + .../unit/db/sqlalchemy/test_migrations.py | 9 ++ ironic/tests/unit/db/utils.py | 1 + ironic/tests/unit/objects/test_objects.py | 4 +- ironic/tests/unit/objects/test_port.py | 90 ++++++++++++++++++- 9 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 ironic/db/sqlalchemy/alembic/versions/9cbeefa3763f_add_port_is_smartnic.py diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 0486993ac7..00e3f3205a 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -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'], diff --git a/ironic/db/sqlalchemy/alembic/versions/9cbeefa3763f_add_port_is_smartnic.py b/ironic/db/sqlalchemy/alembic/versions/9cbeefa3763f_add_port_is_smartnic.py new file mode 100644 index 0000000000..726abf9088 --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/9cbeefa3763f_add_port_is_smartnic.py @@ -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)) diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index db76a9dbde..ddd300a832 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -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): diff --git a/ironic/objects/port.py b/ironic/objects/port.py index 4aee841d2d..5c50678599 100644 --- a/ironic/objects/port.py +++ b/ironic/objects/port.py @@ -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): diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index f2c11bda19..f6f95aaacb 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -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) diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index 9593f26057..83b60ba277 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -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') diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index f3f8380003..c7f983aa76 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -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), } diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index bd88085061..f6c32fcf41 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -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', diff --git a/ironic/tests/unit/objects/test_port.py b/ironic/tests/unit/objects/test_port.py index ac6a0bdf80..2e69487730 100644 --- a/ironic/tests/unit/objects/test_port.py +++ b/ironic/tests/unit/objects/test_port.py @@ -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)