Merge "Allow specifying a set of fields of the Node resource"

9 files changed, 249 insertions, 25 deletions

diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py
index 4614fde..4eeb11f 100644
--- a/ironicclient/common/http.py
+++ b/ironicclient/common/http.py
@@ -36,7 +36,7 @@ from ironicclient import exc
 #             microversion support in the client properly! See
 #             http://specs.openstack.org/openstack/ironic-specs/specs/kilo/api-microversions.html # noqa
 #             for full details.
-DEFAULT_VER = '1.7'
+DEFAULT_VER = '1.8'
 
 
 LOG = logging.getLogger(__name__)
diff --git a/ironicclient/common/utils.py b/ironicclient/common/utils.py
index 4affab9..ae2a150 100644
--- a/ironicclient/common/utils.py
+++ b/ironicclient/common/utils.py
@@ -167,16 +167,24 @@ def common_params_for_list(args, fields, field_labels):
 
     params['detail'] = args.detail
 
+    if hasattr(args, 'fields'):
+        requested_fields = args.fields[0] if args.fields else None
+        if requested_fields is not None:
+            params['fields'] = requested_fields
+
     return params
 
 
-def common_filters(marker=None, limit=None, sort_key=None, sort_dir=None):
+def common_filters(marker=None, limit=None, sort_key=None, sort_dir=None,
+                   fields=None):
     """Generate common filters for any list request.
 
     :param marker: entity ID from which to start returning entities.
     :param limit: maximum number of entities to return.
     :param sort_key: field to use for sorting.
     :param sort_dir: direction of sorting: 'asc' or 'desc'.
+    :param fields: a list with a specified set of fields of the resource
+                   to be returned.
     :returns: list of string filters.
     """
     filters = []
@@ -188,6 +196,8 @@ def common_filters(marker=None, limit=None, sort_key=None, sort_dir=None):
         filters.append('sort_key=%s' % sort_key)
     if sort_dir is not None:
         filters.append('sort_dir=%s' % sort_dir)
+    if fields is not None:
+        filters.append('fields=%s' % ','.join(fields))
     return filters
 
 
@@ -277,3 +287,19 @@ def bool_argument_value(arg_name, bool_str, strict=True, default=False):
         raise exc.CommandError(_("argument %(arg)s: %(err)s.")
                                % {'arg': arg_name, 'err': e})
     return val
+
+
+def check_for_invalid_fields(fields, valid_fields):
+    """Check for invalid fields.
+
+    :param fields: A list of fields specified by the user.
+    :param valid_fields: A list of valid fields.
+    raises: CommandError: If invalid fields were specified by the user.
+    """
+    if not fields:
+        return
+
+    invalid_attr = set(fields) - set(valid_fields)
+    if invalid_attr:
+        raise exc.CommandError(_('Invalid field(s): %s') %
+                               ', '.join(invalid_attr))
diff --git a/ironicclient/tests/unit/test_utils.py b/ironicclient/tests/unit/test_utils.py
index f395a87..f4a021b 100644
--- a/ironicclient/tests/unit/test_utils.py
+++ b/ironicclient/tests/unit/test_utils.py
@@ -114,13 +114,19 @@ class UtilsTest(test_utils.BaseTestCase):
         self.assertEqual('foo', utils.bool_argument_value('arg', 'ee',
                          strict=False, default='foo'))
 
+    def test_check_for_invalid_fields(self):
+        self.assertIsNone(utils.check_for_invalid_fields(
+                          ['a', 'b'], ['a', 'b', 'c']))
+        # 'd' is not a valid field
+        self.assertRaises(exc.CommandError, utils.check_for_invalid_fields,
+                          ['a', 'd'], ['a', 'b', 'c'])
+
 
 class CommonParamsForListTest(test_utils.BaseTestCase):
     def setUp(self):
         super(CommonParamsForListTest, self).setUp()
-        self.args = mock.Mock(marker=None, limit=None,
-                              sort_key=None, sort_dir=None)
-        self.args.detail = False
+        self.args = mock.Mock(marker=None, limit=None, sort_key=None,
+                              sort_dir=None, detail=False, spec=True)
         self.expected_params = {'detail': False}
 
     def test_nothing_set(self):
@@ -179,6 +185,12 @@ class CommonParamsForListTest(test_utils.BaseTestCase):
         self.assertEqual(self.expected_params,
                          utils.common_params_for_list(self.args, [], []))
 
+    def test_fields(self):
+        self.args.fields = [['a', 'b', 'c']]
+        self.expected_params.update({'fields': ['a', 'b', 'c']})
+        self.assertEqual(self.expected_params,
+                         utils.common_params_for_list(self.args, [], []))
+
 
 class CommonFiltersTest(test_utils.BaseTestCase):
     def test_limit(self):
@@ -194,6 +206,10 @@ class CommonFiltersTest(test_utils.BaseTestCase):
             result = utils.common_filters(**{key: 'test'})
             self.assertEqual(['%s=test' % key], result)
 
+    def test_fields(self):
+        result = utils.common_filters(fields=['a', 'b', 'c'])
+        self.assertEqual(['fields=a,b,c'], result)
+
 
 @mock.patch.object(subprocess, 'Popen')
 class MakeConfigDriveTest(test_utils.BaseTestCase):
diff --git a/ironicclient/tests/unit/v1/test_chassis_shell.py b/ironicclient/tests/unit/v1/test_chassis_shell.py
index 2efd3a7..dd2e0fb 100644
--- a/ironicclient/tests/unit/v1/test_chassis_shell.py
+++ b/ironicclient/tests/unit/v1/test_chassis_shell.py
@@ -23,7 +23,7 @@ import ironicclient.v1.chassis_shell as c_shell
 class ChassisShellTest(utils.BaseTestCase):
     def _get_client_mock_args(self, chassis=None, marker=None, limit=None,
                               sort_dir=None, sort_key=None, detail=False):
-        args = mock.MagicMock()
+        args = mock.MagicMock(spec=True)
         args.chassis = chassis
         args.marker = marker
         args.limit = limit
diff --git a/ironicclient/tests/unit/v1/test_node.py b/ironicclient/tests/unit/v1/test_node.py
index 8fd9bfa..4527f14 100644
--- a/ironicclient/tests/unit/v1/test_node.py
+++ b/ironicclient/tests/unit/v1/test_node.py
@@ -103,6 +103,13 @@ fake_responses = {
             {"nodes": [NODE1, NODE2]}
         ),
     },
+    '/v1/nodes/?fields=uuid,extra':
+    {
+        'GET': (
+            {},
+            {"nodes": [NODE1]}
+        ),
+    },
     '/v1/nodes/?associated=False':
     {
         'GET': (
@@ -160,6 +167,13 @@ fake_responses = {
             UPDATED_NODE,
         ),
     },
+    '/v1/nodes/%s?fields=uuid,extra' % NODE1['uuid']:
+    {
+        'GET': (
+            {},
+            NODE1,
+        ),
+    },
     '/v1/nodes/%s' % NODE2['uuid']:
     {
         'GET': (
@@ -188,6 +202,13 @@ fake_responses = {
             {"ports": [PORT]},
         ),
     },
+    '/v1/nodes/%s/ports?fields=uuid,address' % NODE1['uuid']:
+    {
+        'GET': (
+            {},
+            {"ports": [PORT]},
+        ),
+    },
     '/v1/nodes/%s/maintenance' % NODE1['uuid']:
     {
         'PUT': (
@@ -478,6 +499,18 @@ class NodeManagerTest(testtools.TestCase):
         self.assertEqual(2, len(nodes))
         self.assertEqual(nodes[0].extra, {})
 
+    def test_node_list_fields(self):
+        nodes = self.mgr.list(fields=['uuid', 'extra'])
+        expect = [
+            ('GET', '/v1/nodes/?fields=uuid,extra', {}, None),
+        ]
+        self.assertEqual(expect, self.api.calls)
+        self.assertEqual(1, len(nodes))
+
+    def test_node_list_detail_and_fields_fail(self):
+        self.assertRaises(exc.InvalidAttribute, self.mgr.list,
+                          detail=True, fields=['uuid', 'extra'])
+
     def test_node_show(self):
         node = self.mgr.get(NODE1['uuid'])
         expect = [
@@ -503,6 +536,15 @@ class NodeManagerTest(testtools.TestCase):
         self.assertEqual(expect, self.api.calls)
         self.assertEqual(NODE1['uuid'], node.uuid)
 
+    def test_node_show_fields(self):
+        node = self.mgr.get(NODE1['uuid'], fields=['uuid', 'extra'])
+        expect = [
+            ('GET', '/v1/nodes/%s?fields=uuid,extra' %
+             NODE1['uuid'], {}, None),
+        ]
+        self.assertEqual(expect, self.api.calls)
+        self.assertEqual(NODE1['uuid'], node.uuid)
+
     def test_create(self):
         node = self.mgr.create(**CREATE_NODE)
         expect = [
@@ -605,6 +647,19 @@ class NodeManagerTest(testtools.TestCase):
         self.assertEqual(expect, self.api.calls)
         self.assertEqual(1, len(ports))
 
+    def test_node_port_list_fields(self):
+        ports = self.mgr.list_ports(NODE1['uuid'], fields=['uuid', 'address'])
+        expect = [
+            ('GET', '/v1/nodes/%s/ports?fields=uuid,address' %
+             NODE1['uuid'], {}, None),
+        ]
+        self.assertEqual(expect, self.api.calls)
+        self.assertEqual(1, len(ports))
+
+    def test_node_port_list_detail_and_fields_fail(self):
+        self.assertRaises(exc.InvalidAttribute, self.mgr.list_ports,
+                          NODE1['uuid'], detail=True, fields=['uuid', 'extra'])
+
     def test_node_set_maintenance_true(self):
         maintenance = self.mgr.set_maintenance(NODE1['uuid'], 'true',
                                                maint_reason='reason')
diff --git a/ironicclient/tests/unit/v1/test_node_shell.py b/ironicclient/tests/unit/v1/test_node_shell.py
index 079f2c6..885f277 100644
--- a/ironicclient/tests/unit/v1/test_node_shell.py
+++ b/ironicclient/tests/unit/v1/test_node_shell.py
@@ -160,9 +160,10 @@ class NodeShellTest(utils.BaseTestCase):
         args = mock.MagicMock()
         args.node = 'node_uuid'
         args.instance_uuid = False
+        args.fields = None
 
         n_shell.do_node_show(client_mock, args)
-        client_mock.node.get.assert_called_once_with('node_uuid')
+        client_mock.node.get.assert_called_once_with('node_uuid', fields=None)
         # assert get_by_instance_uuid() wasn't called
         self.assertFalse(client_mock.node.get_by_instance_uuid.called)
 
@@ -171,10 +172,11 @@ class NodeShellTest(utils.BaseTestCase):
         args = mock.MagicMock()
         args.node = 'instance_uuid'
         args.instance_uuid = True
+        args.fields = None
 
         n_shell.do_node_show(client_mock, args)
         client_mock.node.get_by_instance_uuid.assert_called_once_with(
-            'instance_uuid')
+            'instance_uuid', fields=None)
         # assert get() wasn't called
         self.assertFalse(client_mock.node.get.called)
 
@@ -214,6 +216,25 @@ class NodeShellTest(utils.BaseTestCase):
                           n_shell.do_node_show,
                           client_mock, args)
 
+    def test_do_node_show_fields(self):
+        client_mock = mock.MagicMock()
+        args = mock.MagicMock()
+        args.node = 'node_uuid'
+        args.instance_uuid = False
+        args.fields = [['uuid', 'power_state']]
+        n_shell.do_node_show(client_mock, args)
+        client_mock.node.get.assert_called_once_with(
+            'node_uuid', fields=['uuid', 'power_state'])
+
+    def test_do_node_show_invalid_fields(self):
+        client_mock = mock.MagicMock()
+        args = mock.MagicMock()
+        args.node = 'node_uuid'
+        args.instance_uuid = False
+        args.fields = [['foo', 'bar']]
+        self.assertRaises(exceptions.CommandError,
+                          n_shell.do_node_show, client_mock, args)
+
     def test_do_node_set_maintenance_true(self):
         client_mock = mock.MagicMock()
         args = mock.MagicMock()
@@ -464,10 +485,12 @@ class NodeShellTest(utils.BaseTestCase):
         client_mock.node.get_supported_boot_devices.assert_called_once_with(
             'node_uuid')
 
-    def _get_client_mock_args(self, associated=None, maintenance=None,
-                              marker=None, limit=None, sort_dir=None,
-                              sort_key=None, detail=False):
+    def _get_client_mock_args(self, node=None, associated=None,
+                              maintenance=None, marker=None, limit=None,
+                              sort_dir=None, sort_key=None, detail=False,
+                              fields=None):
         args = mock.MagicMock()
+        args.node = node
         args.associated = associated
         args.maintenance = maintenance
         args.marker = marker
@@ -475,6 +498,7 @@ class NodeShellTest(utils.BaseTestCase):
         args.sort_dir = sort_dir
         args.sort_key = sort_key
         args.detail = detail
+        args.fields = fields
 
         return args
 
@@ -530,6 +554,19 @@ class NodeShellTest(utils.BaseTestCase):
                           client_mock, args)
         self.assertFalse(client_mock.node.list.called)
 
+    def test_do_node_list_fields(self):
+        client_mock = mock.MagicMock()
+        args = self._get_client_mock_args(fields=[['uuid', 'provision_state']])
+        n_shell.do_node_list(client_mock, args)
+        client_mock.node.list.assert_called_once_with(
+            fields=['uuid', 'provision_state'], detail=False)
+
+    def test_do_node_list_invalid_fields(self):
+        client_mock = mock.MagicMock()
+        args = self._get_client_mock_args(fields=[['foo', 'bar']])
+        self.assertRaises(exceptions.CommandError,
+                          n_shell.do_node_list, client_mock, args)
+
     def test_do_node_show_states(self):
         client_mock = mock.MagicMock()
         args = mock.MagicMock()
@@ -537,3 +574,20 @@ class NodeShellTest(utils.BaseTestCase):
 
         n_shell.do_node_show_states(client_mock, args)
         client_mock.node.states.assert_called_once_with('node_uuid')
+
+    def test_do_node_port_list_fields(self):
+        client_mock = mock.MagicMock()
+        node_mock = mock.MagicMock(spec_set=[])
+        args = self._get_client_mock_args(node=node_mock,
+                                          fields=[['uuid', 'address']])
+        n_shell.do_node_port_list(client_mock, args)
+        client_mock.node.list_ports.assert_called_once_with(
+            node_mock, fields=['uuid', 'address'], detail=False)
+
+    def test_do_node_port_list_invalid_fields(self):
+        client_mock = mock.MagicMock()
+        node_mock = mock.MagicMock(spec_set=[])
+        args = self._get_client_mock_args(node=node_mock,
+                                          fields=[['foo', 'bar']])
+        self.assertRaises(exceptions.CommandError,
+                          n_shell.do_node_port_list, client_mock, args)
diff --git a/ironicclient/tests/unit/v1/test_port_shell.py b/ironicclient/tests/unit/v1/test_port_shell.py
index 85dd854..0359b69 100644
--- a/ironicclient/tests/unit/v1/test_port_shell.py
+++ b/ironicclient/tests/unit/v1/test_port_shell.py
@@ -87,7 +87,7 @@ class PortShellTest(utils.BaseTestCase):
 
     def _get_client_mock_args(self, address=None, marker=None, limit=None,
                               sort_dir=None, sort_key=None, detail=False):
-        args = mock.MagicMock()
+        args = mock.MagicMock(spec=True)
         args.address = address
         args.marker = marker
         args.limit = limit
diff --git a/ironicclient/v1/node.py b/ironicclient/v1/node.py
index 5774d98..42b9c6b 100644
--- a/ironicclient/v1/node.py
+++ b/ironicclient/v1/node.py
@@ -38,7 +38,7 @@ class NodeManager(base.Manager):
         return '/v1/nodes/%s' % id if id else '/v1/nodes'
 
     def list(self, associated=None, maintenance=None, marker=None, limit=None,
-             detail=False, sort_key=None, sort_dir=None):
+             detail=False, sort_key=None, sort_dir=None, fields=None):
         """Retrieve a list of nodes.
 
         :param associated: Optional. Either a Boolean or a string
@@ -70,13 +70,22 @@ class NodeManager(base.Manager):
         :param sort_dir: Optional, direction of sorting, either 'asc' (the
                          default) or 'desc'.
 
+        :param fields: Optional, a list with a specified set of fields
+                       of the resource to be returned. Can not be used
+                       when 'detail' is set.
+
         :returns: A list of nodes.
 
         """
         if limit is not None:
             limit = int(limit)
 
-        filters = utils.common_filters(marker, limit, sort_key, sort_dir)
+        if detail and fields:
+            raise exc.InvalidAttribute(_("Can't fetch a subset of fields "
+                                         "with 'detail' set"))
+
+        filters = utils.common_filters(marker, limit, sort_key, sort_dir,
+                                       fields)
         if associated is not None:
             filters.append('associated=%s' % associated)
         if maintenance is not None:
@@ -95,7 +104,7 @@ class NodeManager(base.Manager):
                                          limit=limit)
 
     def list_ports(self, node_id, marker=None, limit=None, sort_key=None,
-                   sort_dir=None, detail=False):
+                   sort_dir=None, detail=False, fields=None):
         """List all the ports for a given node.
 
         :param node_id: The UUID of the node.
@@ -119,13 +128,22 @@ class NodeManager(base.Manager):
         :param detail: Optional, boolean whether to return detailed information
                        about ports.
 
+        :param fields: Optional, a list with a specified set of fields
+                       of the resource to be returned. Can not be used
+                       when 'detail' is set.
+
         :returns: A list of ports.
 
         """
         if limit is not None:
             limit = int(limit)
 
-        filters = utils.common_filters(marker, limit, sort_key, sort_dir)
+        if detail and fields:
+            raise exc.InvalidAttribute(_("Can't fetch a subset of fields "
+                                         "with 'detail' set"))
+
+        filters = utils.common_filters(marker, limit, sort_key, sort_dir,
+                                       fields)
 
         path = "%s/ports" % node_id
         if detail:
@@ -140,14 +158,23 @@ class NodeManager(base.Manager):
             return self._list_pagination(self._path(path), "ports",
                                          limit=limit)
 
-    def get(self, node_id):
+    def get(self, node_id, fields=None):
+        if fields is not None:
+            node_id = '%s?fields=' % node_id
+            node_id += ','.join(fields)
+
         try:
             return self._list(self._path(node_id))[0]
         except IndexError:
             return None
 
-    def get_by_instance_uuid(self, instance_uuid):
-        path = "detail?instance_uuid=%s" % instance_uuid
+    def get_by_instance_uuid(self, instance_uuid, fields=None):
+        path = '?instance_uuid=%s' % instance_uuid
+        if fields is not None:
+            path += '&fields=' + ','.join(fields)
+        else:
+            path = 'detail' + path
+
         nodes = self._list(self._path(path), 'nodes')
         # get all the details of the node assuming that
         # filtering by instance_uuid returns a collection
diff --git a/ironicclient/v1/node_shell.py b/ironicclient/v1/node_shell.py
index aebe309..65278c7 100644
--- a/ironicclient/v1/node_shell.py
+++ b/ironicclient/v1/node_shell.py
@@ -24,10 +24,12 @@ from ironicclient.openstack.common import cliutils
 from ironicclient.v1 import resource_fields as res_fields
 
 
-def _print_node_show(node):
+def _print_node_show(node, fields=None):
+    if fields is None:
+        fields = res_fields.NODE_DETAILED_RESOURCE.fields
+
     data = dict(
-        [(f, getattr(node, f, ''))
-         for f in res_fields.NODE_DETAILED_RESOURCE.fields])
+        [(f, getattr(node, f, '')) for f in fields])
     cliutils.print_dict(data, wrap=72)
 
 
@@ -42,14 +44,26 @@ def _print_node_show(node):
     action='store_true',
     default=False,
     help='<id> is an instance UUID.')
+@cliutils.arg(
+    '--fields',
+    nargs='+',
+    dest='fields',
+    metavar='<field>',
+    action='append',
+    default=[],
+    help="One or more node fields. Only these fields will be fetched from "
+         "the server.")
 def do_node_show(cc, args):
     """Show detailed information about a node."""
+    fields = args.fields[0] if args.fields else None
     utils.check_empty_arg(args.node, '<id>')
+    utils.check_for_invalid_fields(
+        fields, res_fields.NODE_DETAILED_RESOURCE.fields)
     if args.instance_uuid:
-        node = cc.node.get_by_instance_uuid(args.node)
+        node = cc.node.get_by_instance_uuid(args.node, fields=fields)
     else:
-        node = cc.node.get(args.node)
-    _print_node_show(node)
+        node = cc.node.get(args.node, fields=fields)
+    _print_node_show(node, fields=fields)
 
 
 @cliutils.arg(
@@ -87,6 +101,15 @@ def do_node_show(cc, args):
     action='store_true',
     default=False,
     help="Show detailed information about the nodes.")
+@cliutils.arg(
+    '--fields',
+    nargs='+',
+    dest='fields',
+    metavar='<field>',
+    action='append',
+    default=[],
+    help="One or more node fields. Only these fields will be fetched from "
+         "the server. Can not be used when '--detail' is specified.")
 def do_node_list(cc, args):
     """List the nodes which are registered with the Ironic service."""
     params = {}
@@ -103,6 +126,14 @@ def do_node_list(cc, args):
         field_labels = res_fields.NODE_DETAILED_RESOURCE.labels
         sort_fields = res_fields.NODE_DETAILED_RESOURCE.sort_fields
         sort_field_labels = res_fields.NODE_DETAILED_RESOURCE.sort_labels
+    elif args.fields:
+        utils.check_for_invalid_fields(
+            args.fields[0], res_fields.NODE_DETAILED_RESOURCE.fields)
+        resource = res_fields.Resource(args.fields[0])
+        fields = resource.fields
+        field_labels = resource.labels
+        sort_fields = res_fields.NODE_DETAILED_RESOURCE.sort_fields
+        sort_field_labels = res_fields.NODE_DETAILED_RESOURCE.sort_labels
     else:
         fields = res_fields.NODE_RESOURCE.fields
         field_labels = res_fields.NODE_RESOURCE.labels
@@ -276,11 +307,26 @@ def do_node_vendor_passthru(cc, args):
     choices=['asc', 'desc'],
     help='Sort direction: "asc" (the default) or "desc".')
 @cliutils.arg('node', metavar='<node>', help="UUID of the node.")
+@cliutils.arg(
+    '--fields',
+    nargs='+',
+    dest='fields',
+    metavar='<field>',
+    action='append',
+    default=[],
+    help="One or more port fields. Only these fields will be fetched from "
+         "the server. Can not be used when '--detail' is specified.")
 def do_node_port_list(cc, args):
     """List the ports associated with a node."""
     if args.detail:
         fields = res_fields.PORTS_DETAILED_RESOURCE.fields
         field_labels = res_fields.PORTS_DETAILED_RESOURCE.labels
+    elif args.fields:
+        utils.check_for_invalid_fields(
+            args.fields[0], res_fields.PORT_DETAILED_RESOURCE.fields)
+        resource = res_fields.Resource(args.fields[0])
+        fields = resource.fields
+        field_labels = resource.labels
     else:
         fields = res_fields.PORT_RESOURCE.fields
         field_labels = res_fields.PORT_RESOURCE.labels