diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index 4614fdecc..4eeb11fcc 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 4affab94c..ae2a150d8 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 f395a87da..f4a021b43 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 2efd3a744..dd2e0fb17 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 8fd9bfaf7..4527f1495 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 079f2c606..885f277c4 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 85dd854bd..0359b697c 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 5774d98b7..42b9c6bd0 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 aebe309f0..65278c7f2 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=' is an instance UUID.') +@cliutils.arg( + '--fields', + nargs='+', + dest='fields', + metavar='', + 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, '') + 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='', + 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='', help="UUID of the node.") +@cliutils.arg( + '--fields', + nargs='+', + dest='fields', + metavar='', + 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