Merge "Set stage for objects to handle selected field lists."

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'}
+        use_fields = set(fields or self.fields) - {'traits'}
-        super(Node, self)._set_from_db_object(context, db_object, fields)
+        super(Node, self)._set_from_db_object(context, db_object, use_fields)
-        self.traits = object_base.obj_make_list(
+        if not fields or 'traits' in fields:
-            context, objects.TraitList(context),
+            # NOTE(TheJulia): No reason to do additional work on a field
-            objects.Trait, db_object['traits'])
+            # selected query, unless they are seeking the traits themselves.
-        self.traits.obj_reset_changes()
+            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

ironic/tests/unit/api/controllers/v1/test_node.py

@@ -497,6 +497,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)

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',
+                fields=['id', 'name', 'uuid', 'provision_state', 'version',
-                        'owner', 'lessee', 'driver', 'conductor_group'])
+                        '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',