diff --git a/ironic/objects/node.py b/ironic/objects/node.py index c7076b2869..3c39961964 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -213,12 +213,16 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): raise exception.InvalidParameterValue(msg) def _set_from_db_object(self, context, db_object, fields=None): - fields = set(fields or self.fields) - {'traits'} - super(Node, self)._set_from_db_object(context, db_object, fields) - self.traits = object_base.obj_make_list( - context, objects.TraitList(context), - objects.Trait, db_object['traits']) - self.traits.obj_reset_changes() + use_fields = set(fields or self.fields) - {'traits'} + super(Node, self)._set_from_db_object(context, db_object, use_fields) + if not fields or 'traits' in fields: + # NOTE(TheJulia): No reason to do additional work on a field + # selected query, unless they are seeking the traits themselves. + self.traits = object_base.obj_make_list( + context, objects.TraitList(context), + objects.Trait, db_object['traits'], + fields=['trait', 'version']) + self.traits.obj_reset_changes() # 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. @@ -328,7 +332,6 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): automatically included. These are: id, version, updated_at, created_at, owner, and lessee. :returns: a list of :class:`Node` object. - """ if fields: # All requests must include version, updated_at, created_at diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 3408bc3b6e..e5abdf3333 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -498,6 +498,38 @@ class TestListNodes(test_api_base.BaseApiTest): # We always append "links" self.assertCountEqual(['uuid', 'instance_info', 'links'], node) + def test_get_collection_fields_for_nova(self): + # Unit test which explicitly attempts to request traits along with + # all columns which nova would normally request as part of it's sync + # process. + fields = ('uuid,power_state,target_power_state,provision_state,' + 'target_provision_state,last_error,maintenance,' + 'instance_uuid,traits,resource_class') + trait_list = ['CUSTOM_TRAIT1', 'CUSTOM_RAID5'] + for i in range(3): + node = obj_utils.create_test_node( + self.context, uuid=uuidutils.generate_uuid(), + instance_uuid=uuidutils.generate_uuid()) + self.dbapi.set_node_traits(node.id, trait_list, '1.0') + + data = self.get_json( + '/nodes?fields=%s' % fields, + headers={api_base.Version.string: str(api_v1.max_version())}) + + self.assertEqual(3, len(data['nodes'])) + for node in data['nodes']: + # We always append "links" + self.assertCountEqual([ + 'uuid', 'power_state', 'target_power_state', + 'provision_state', 'target_provision_state', 'last_error', + 'maintenance', 'instance_uuid', 'traits', 'resource_class', + 'links'], node) + # Ensure traits exists that have been created on the node. + # Sorted traits list is an artifact of the data being inserted + # as distinct rows and then collected back into a list. + # There is no contract around ordering for the results of traits. + self.assertEqual(sorted(trait_list, reverse=False), node['traits']) + def test_get_custom_fields_invalid_fields(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index 8d8b2a8e5f..6cbda0ba5a 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -396,17 +396,68 @@ class TestNodeObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): self.assertThat(nodes, matchers.HasLength(1)) self.assertIsInstance(nodes[0], objects.Node) self.assertEqual(self.context, nodes[0]._context) + self.assertIsInstance(nodes[0].traits, objects.TraitList) def test_list_with_fields(self): with mock.patch.object(self.dbapi, 'get_node_list', autospec=True) as mock_get_list: mock_get_list.return_value = [self.fake_node] - objects.Node.list(self.context, fields=['name']) + nodes = objects.Node.list(self.context, + fields=['name', 'uuid', + 'provision_state']) mock_get_list.assert_called_with( filters=None, limit=None, marker=None, sort_key=None, sort_dir=None, - fields=['id', 'name', 'version', 'updated_at', 'created_at', - 'owner', 'lessee', 'driver', 'conductor_group']) + fields=['id', 'name', 'uuid', 'provision_state', 'version', + 'updated_at', 'created_at', 'owner', 'lessee', + 'driver', 'conductor_group']) + self.assertThat(nodes, matchers.HasLength(1)) + self.assertEqual(self.fake_node['uuid'], nodes[0].uuid) + self.assertEqual(self.fake_node['provision_state'], + nodes[0].provision_state) + self.assertIsInstance(nodes[0].traits, objects.TraitList) + # Random assortment of fields which should not be present. + for field in ['power_state', 'instance_info', 'resource_class', + 'automated_clean', 'properties', 'driver', 'traits']: + self.assertFalse(field not in dir(nodes[0])) + + def test_list_with_fields_traits(self): + with mock.patch.object(self.dbapi, 'get_node_list', + autospec=True) as mock_get_list: + # Trait objects and ultimately rows have an underlying + # version. Since we've never changed it, it should just + # be one, but this is required for the oslo versioned + # objects code path to handle objects properly and + # navigate mid-upgrade cases. + self.fake_node['traits'] = [{ + 'trait': 'traitx', 'version': "1"}] + mock_get_list.return_value = [self.fake_node] + nodes = objects.Node.list(self.context, + fields=['uuid', 'provision_state', + 'traits']) + self.assertThat(nodes, matchers.HasLength(1)) + self.assertEqual(self.fake_node['uuid'], nodes[0].uuid) + self.assertEqual(self.fake_node['provision_state'], + nodes[0].provision_state) + self.assertIsInstance(nodes[0].traits, objects.TraitList) + self.assertEqual('traitx', nodes[0].traits[0].trait) + for field in ['power_state', 'instance_info', 'resource_class', + 'automated_clean', 'properties', 'driver']: + self.assertFalse(field not in dir(nodes[0])) + + def test_list_with_fields_empty_trait_present(self): + with mock.patch.object(self.dbapi, 'get_node_list', + autospec=True) as mock_get_list: + mock_get_list.return_value = [self.fake_node] + nodes = objects.Node.list(self.context, + fields=['uuid', 'provision_state', + 'traits']) + self.assertThat(nodes, matchers.HasLength(1)) + self.assertEqual(self.fake_node['uuid'], nodes[0].uuid) + self.assertEqual(self.fake_node['provision_state'], + nodes[0].provision_state) + self.assertIsInstance(nodes[0].traits, objects.TraitList) + self.assertIn('traits', nodes[0]) def test_reserve(self): with mock.patch.object(self.dbapi, 'reserve_node',