From 6b44f269c10c9e9e77d0242a70ac80a30f5ee2f6 Mon Sep 17 00:00:00 2001 From: Kaifeng Wang Date: Fri, 23 Mar 2018 16:59:45 +0800 Subject: [PATCH] Power fault recovery: db and rpc implementation This adds a fault field to the node table of database, and necessary rpc object change and version bumping. Story: #1596107 Task: #10469 Change-Id: I5539aa0406dbfbde25bc9aa91d5c1e615875e50e --- ironic/common/release_mappings.py | 2 +- .../fb3f10dd262e_add_fault_to_node_table.py | 31 +++++++++ ironic/db/sqlalchemy/api.py | 5 +- ironic/db/sqlalchemy/models.py | 1 + ironic/objects/node.py | 20 +++++- ironic/tests/unit/api/utils.py | 2 + .../unit/db/sqlalchemy/test_migrations.py | 7 ++ ironic/tests/unit/db/test_nodes.py | 14 ++++ ironic/tests/unit/db/utils.py | 1 + ironic/tests/unit/objects/test_node.py | 66 +++++++++++++++++-- ironic/tests/unit/objects/test_objects.py | 2 +- 11 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 ironic/db/sqlalchemy/alembic/versions/fb3f10dd262e_add_fault_to_node_table.py diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index f8471cb690..50eba32689 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -103,7 +103,7 @@ RELEASE_MAPPING = { 'api': '1.39', 'rpc': '1.44', 'objects': { - 'Node': ['1.24'], + 'Node': ['1.25'], 'Conductor': ['1.2'], 'Chassis': ['1.3'], 'Port': ['1.8'], diff --git a/ironic/db/sqlalchemy/alembic/versions/fb3f10dd262e_add_fault_to_node_table.py b/ironic/db/sqlalchemy/alembic/versions/fb3f10dd262e_add_fault_to_node_table.py new file mode 100644 index 0000000000..cc577d363f --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/fb3f10dd262e_add_fault_to_node_table.py @@ -0,0 +1,31 @@ +# 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 fault to node table + +Revision ID: fb3f10dd262e +Revises: 2d13bc3d6bba +Create Date: 2018-03-23 14:10:52.142016 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'fb3f10dd262e' +down_revision = '2d13bc3d6bba' + + +def upgrade(): + op.add_column('nodes', sa.Column('fault', sa.String(length=255), + nullable=True)) diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index e8a2a4bc95..5ce30c7530 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -224,14 +224,15 @@ class Connection(api.Connection): 'resource_class', 'provision_state', 'uuid', 'id', 'chassis_uuid', 'associated', 'reserved', 'reserved_by_any_of', 'provisioned_before', - 'inspection_started_before'} + 'inspection_started_before', 'fault'} unsupported_filters = set(filters).difference(supported_filters) if unsupported_filters: msg = _("SqlAlchemy API does not support " "filtering by %s") % ', '.join(unsupported_filters) raise ValueError(msg) for field in ['console_enabled', 'maintenance', 'driver', - 'resource_class', 'provision_state', 'uuid', 'id']: + 'resource_class', 'provision_state', 'uuid', 'id', + 'fault']: if field in filters: query = query.filter_by(**{field: filters[field]}) if 'chassis_uuid' in filters: diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index a42181c228..f548758dc4 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -165,6 +165,7 @@ class Node(Base): maintenance = Column(Boolean, default=False) maintenance_reason = Column(Text, nullable=True) + fault = Column(String(255), nullable=True) console_enabled = Column(Boolean, default=False) inspection_finished_at = Column(DateTime, nullable=True) inspection_started_at = Column(DateTime, nullable=True) diff --git a/ironic/objects/node.py b/ironic/objects/node.py index c68f5f45e4..255d0e7d5b 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -60,7 +60,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.22: Add rescue_interface field # Version 1.23: Add traits field # Version 1.24: Add bios_interface field - VERSION = '1.24' + # Version 1.25: Add fault field + VERSION = '1.25' dbapi = db_api.get_instance() @@ -106,6 +107,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 'maintenance': object_fields.BooleanField(), 'maintenance_reason': object_fields.StringField(nullable=True), + 'fault': object_fields.StringField(nullable=True), 'console_enabled': object_fields.BooleanField(), # Any error from the most recent (last) asynchronous transaction @@ -463,6 +465,18 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node = cls._from_db_object(context, cls(), db_node) return node + def _convert_fault_field(self, target_version, + remove_unavailable_fields=True): + fault_is_set = self.obj_attr_is_set('fault') + if target_version >= (1, 25): + if not fault_is_set: + self.fault = None + elif fault_is_set: + if remove_unavailable_fields: + delattr(self, 'fault') + elif self.fault is not None: + self.fault = None + def _convert_to_version(self, target_version, remove_unavailable_fields=True): """Convert to the target version. @@ -480,6 +494,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): Version 1.24: bios_interface field was added. Its default value is None. For versions prior to this, it should be set to None (or removed). + Version 1.25: fault field was added. For versions prior to + this, it should be removed. :param target_version: the desired version of the object :param remove_unavailable_fields: True to remove fields that are @@ -530,6 +546,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # DB: set unavailable field to the default of None. self.bios_interface = None + self._convert_fault_field(target_version, remove_unavailable_fields) + @base.IronicObjectRegistry.register class NodePayload(notification.NotificationPayloadBase): diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index 595200e6de..233bea1a5d 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -110,6 +110,8 @@ def node_post_data(**kw): node.pop(name) if 'resource_class' not in kw: node.pop('resource_class') + if 'fault' not in kw: + node.pop('fault') internal = node_controller.NodePatchType.internal_attrs() return remove_internal(node, internal) diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index b231c5d1ef..94d408d66a 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -712,6 +712,13 @@ class MigrationCheckersMixin(object): self.assertIsInstance(nodes.c.bios_interface.type, sqlalchemy.types.String) + def _check_fb3f10dd262e(self, engine, data): + nodes_tbl = db_utils.get_table(engine, 'nodes') + col_names = [column.name for column in nodes_tbl.c] + self.assertIn('fault', col_names) + self.assertIsInstance(nodes_tbl.c.fault.type, + sqlalchemy.types.String) + def test_upgrade_and_version(self): with patch_with_engine(self.engine): self.migration_api.upgrade('head') diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index 765a8b1f06..590b224209 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/ironic/tests/unit/db/test_nodes.py @@ -143,6 +143,7 @@ class DbNodeTestCase(base.DbTestCase): driver='driver-two', uuid=uuidutils.generate_uuid(), maintenance=True, + fault='boom', resource_class='foo') node3 = utils.create_test_node( driver='driver-one', @@ -177,6 +178,12 @@ class DbNodeTestCase(base.DbTestCase): self.assertEqual(sorted([node1.id, node3.id]), sorted([r.id for r in res])) + res = self.dbapi.get_nodeinfo_list(filters={'fault': 'boom'}) + self.assertEqual([node2.id], [r.id for r in res]) + + res = self.dbapi.get_nodeinfo_list(filters={'fault': 'moob'}) + self.assertEqual([], [r.id for r in res]) + res = self.dbapi.get_nodeinfo_list(filters={'resource_class': 'foo'}) self.assertEqual([node2.id], [r.id for r in res]) @@ -284,6 +291,7 @@ class DbNodeTestCase(base.DbTestCase): uuid=uuidutils.generate_uuid(), chassis_id=ch2['id'], maintenance=True, + fault='boom', resource_class='foo') res = self.dbapi.get_node_list(filters={'chassis_uuid': ch1['uuid']}) @@ -316,6 +324,12 @@ class DbNodeTestCase(base.DbTestCase): res = self.dbapi.get_node_list(filters={'maintenance': False}) self.assertEqual([node1.id], [r.id for r in res]) + res = self.dbapi.get_nodeinfo_list(filters={'fault': 'boom'}) + self.assertEqual([node2.id], [r.id for r in res]) + + res = self.dbapi.get_nodeinfo_list(filters={'fault': 'moob'}) + self.assertEqual([], [r.id for r in res]) + res = self.dbapi.get_node_list(filters={'resource_class': 'foo'}) self.assertEqual([node2.id], [r.id for r in res]) diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 57d4563425..b0d2970382 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -188,6 +188,7 @@ def get_test_node(**kw): 'reservation': kw.get('reservation'), 'maintenance': kw.get('maintenance', False), 'maintenance_reason': kw.get('maintenance_reason'), + 'fault': kw.get('fault'), 'console_enabled': kw.get('console_enabled', False), 'extra': kw.get('extra', {}), 'updated_at': kw.get('updated_at'), diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index ef4df2942e..5149f6d64a 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -408,6 +408,7 @@ class TestConvertToVersion(db_base.DbTestCase): node = obj_utils.get_test_node(self.ctxt, **self.fake_node) node.rescue_interface = 'fake' + node.fault = None node.obj_reset_changes() node._convert_to_version("1.21", False) self.assertIsNone(node.rescue_interface) @@ -420,6 +421,7 @@ class TestConvertToVersion(db_base.DbTestCase): node.rescue_interface = None node.traits = None + node.fault = None node.obj_reset_changes() node._convert_to_version("1.21", False) self.assertIsNone(node.rescue_interface) @@ -466,6 +468,7 @@ class TestConvertToVersion(db_base.DbTestCase): # traits not set, should be set to default. node = obj_utils.get_test_node(self.ctxt, **self.fake_node) delattr(node, 'traits') + node.fault = None node.obj_reset_changes() node._convert_to_version("1.22", False) @@ -478,18 +481,19 @@ class TestConvertToVersion(db_base.DbTestCase): node = obj_utils.get_test_node(self.ctxt, **self.fake_node) node.traits = objects.TraitList(self.ctxt) node.traits.obj_reset_changes() + node.fault = None node.obj_reset_changes() node._convert_to_version("1.22", False) self.assertIsNone(node.traits) - self.assertEqual({'traits': None}, - node.obj_get_changes()) + self.assertEqual({'traits': None}, node.obj_get_changes()) def test_trait_unsupported_set_no_remove_default(self): # traits set, no change required. node = obj_utils.get_test_node(self.ctxt, **self.fake_node) node.traits = None + node.fault = None node.obj_reset_changes() node._convert_to_version("1.22", False) @@ -544,22 +548,76 @@ class TestConvertToVersion(db_base.DbTestCase): node = obj_utils.get_test_node(self.ctxt, **self.fake_node) node.bios_interface = 'fake' + node.fault = None node.obj_reset_changes() node._convert_to_version("1.23", False) self.assertIsNone(node.bios_interface) - self.assertEqual({'bios_interface': None}, - node.obj_get_changes()) + self.assertEqual({'bios_interface': None}, node.obj_get_changes()) def test_bios_unsupported_set_no_remove_default(self): # bios_interface set, no change required. node = obj_utils.get_test_node(self.ctxt, **self.fake_node) node.bios_interface = None + node.fault = None node.obj_reset_changes() node._convert_to_version("1.23", False) self.assertIsNone(node.bios_interface) self.assertEqual({}, node.obj_get_changes()) + def test_fault_supported_missing(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + delattr(node, 'fault') + node.obj_reset_changes() + + node._convert_to_version("1.25") + + self.assertIsNone(node.fault) + self.assertEqual({'fault': None}, node.obj_get_changes()) + + def test_fault_supported_untouched(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + node.maintenance = True + node.fault = 'a fake fault' + node.obj_reset_changes() + + node._convert_to_version("1.25") + + self.assertEqual('a fake fault', node.fault) + self.assertEqual({}, node.obj_get_changes()) + + def test_fault_unsupported_missing(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + delattr(node, 'fault') + node.obj_reset_changes() + + node._convert_to_version("1.24") + + self.assertNotIn('fault', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_fault_unsupported_set_remove(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + node.maintenance = True + node.fault = 'some fake fault' + node.obj_reset_changes() + + node._convert_to_version("1.24") + + self.assertNotIn('fault', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_fault_unsupported_set_remove_in_maintenance(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + node.maintenance = True + node.fault = 'some fake type' + node.obj_reset_changes() + + node._convert_to_version("1.24", False) + + self.assertIsNone(node.fault) + self.assertEqual({'fault': None}, node.obj_get_changes()) + class TestNodePayloads(db_base.DbTestCase): diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 486084ea19..4f2c80cc9b 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -663,7 +663,7 @@ class TestObject(_LocalTest, _TestObject): # version bump. It is an MD5 hash of the object fields and remotable methods. # The fingerprint values should only be changed if there is a version bump. expected_object_fingerprints = { - 'Node': '1.24-7d3d504e5e0d2535b2390d558b27196a', + 'Node': '1.25-3a468b3e88d0a8fe7709f822fc654e4b', 'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.8-898a47921f4a1f53fcdddd4eeb179e0b',