From 0e9a476610dcc54be88413b5f642c01c47606b3e Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Mon, 3 Jul 2023 11:12:14 -0700 Subject: [PATCH] Node sharding support - Bumps known API version high enough for shards to exist - Implements support for querying for sharded nodes, or by a list of shards - Adds basic unit testing. - Adds support for all of this to OSC Change-Id: Ie66bd3a6d68ff4051f3d52dfb2bca26b1053187e --- ironicclient/common/http.py | 2 +- ironicclient/osc/v1/baremetal_node.py | 48 +++++++- .../tests/unit/osc/v1/test_baremetal_node.py | 109 +++++++++++++++++- ironicclient/tests/unit/v1/test_node.py | 35 +++++- ironicclient/v1/node.py | 15 ++- .../node-shard-support-774ebfe6719fc7c2.yaml | 4 + 6 files changed, 201 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/node-shard-support-774ebfe6719fc7c2.yaml diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index 513d30332..262861923 100644 --- a/ironicclient/common/http.py +++ b/ironicclient/common/http.py @@ -37,7 +37,7 @@ from ironicclient import exc # http://specs.openstack.org/openstack/ironic-specs/specs/kilo/api-microversions.html # noqa # for full details. DEFAULT_VER = '1.9' -LAST_KNOWN_API_VERSION = 81 +LAST_KNOWN_API_VERSION = 82 LATEST_VERSION = '1.{}'.format(LAST_KNOWN_API_VERSION) LOG = logging.getLogger(__name__) diff --git a/ironicclient/osc/v1/baremetal_node.py b/ironicclient/osc/v1/baremetal_node.py index 3f3ecc578..1935d407d 100755 --- a/ironicclient/osc/v1/baremetal_node.py +++ b/ironicclient/osc/v1/baremetal_node.py @@ -523,6 +523,10 @@ class CreateBaremetalNode(command.ShowOne): '--description', metavar='', help=_("Description for the node.")) + parser.add_argument( + '--shard', + metavar='', + help=_("Shard for the node.")) return parser @@ -534,8 +538,9 @@ class CreateBaremetalNode(command.ShowOne): field_list = ['automated_clean', 'chassis_uuid', 'driver', 'driver_info', 'properties', 'extra', 'uuid', 'name', 'conductor_group', 'owner', 'description', 'lessee', - 'resource_class'] + ['%s_interface' % iface - for iface in SUPPORTED_INTERFACES] + 'shard', 'resource_class' + ] + ['%s_interface' % iface + for iface in SUPPORTED_INTERFACES] fields = dict((k, v) for (k, v) in vars(parsed_args).items() if k in field_list and not (v is None)) fields = utils.args_array_to_dict(fields, 'driver_info') @@ -748,6 +753,24 @@ class ListBaremetalNode(command.Lister): metavar='', help=_("Limit list to nodes with description contains " "")) + sharded_group = parser.add_mutually_exclusive_group(required=False) + sharded_group.add_argument( + '--sharded', + dest='sharded', + help=_("List only nodes that are sharded."), + default=None, + action='store_true') + sharded_group.add_argument( + '--unsharded', + dest='sharded', + help=_("List only nodes that are not sharded."), + default=None, + action='store_false') + parser.add_argument( + '--shards', + nargs='+', + metavar='', + help=_("List only nodes that are in shards .")) display_group = parser.add_mutually_exclusive_group(required=False) display_group.add_argument( '--long', @@ -785,12 +808,14 @@ class ListBaremetalNode(command.Lister): params['associated'] = True if parsed_args.unassociated: params['associated'] = False - for field in ['maintenance', 'fault', 'conductor_group', 'retired']: + + for field in ['maintenance', 'fault', 'conductor_group', 'retired', + 'sharded']: if getattr(parsed_args, field) is not None: params[field] = getattr(parsed_args, field) for field in ['provision_state', 'driver', 'resource_class', 'chassis', 'conductor', 'owner', 'lessee', - 'description_contains']: + 'description_contains', 'shards']: if getattr(parsed_args, field): params[field] = getattr(parsed_args, field) if parsed_args.long: @@ -1399,6 +1424,11 @@ class SetBaremetalNode(command.Command): metavar='', help=_('Set the description for the node'), ) + parser.add_argument( + "--shard", + metavar='', + help=_('Set the shard for the node'), + ) return parser @@ -1429,7 +1459,7 @@ class SetBaremetalNode(command.Command): 'chassis_uuid', 'driver', 'resource_class', 'conductor_group', 'protected', 'protected_reason', 'retired', 'retired_reason', 'owner', 'lessee', - 'description']: + 'description', 'shard']: value = getattr(parsed_args, field) if value: properties.extend(utils.args_array_to_patch( @@ -1745,6 +1775,11 @@ class UnsetBaremetalNode(command.Command): action="store_true", help=_('Unset the description field of the node'), ) + parser.add_argument( + "--shard", + action="store_true", + help=_('Unset the shard field of the node'), + ) return parser @@ -1769,7 +1804,8 @@ class UnsetBaremetalNode(command.Command): 'power_interface', 'raid_interface', 'rescue_interface', 'storage_interface', 'vendor_interface', 'protected', 'protected_reason', 'retired', - 'retired_reason', 'owner', 'lessee', 'description']: + 'retired_reason', 'owner', 'lessee', 'description', + 'shard', ]: if getattr(parsed_args, field): properties.extend(utils.args_array_to_patch('remove', [field])) diff --git a/ironicclient/tests/unit/osc/v1/test_baremetal_node.py b/ironicclient/tests/unit/osc/v1/test_baremetal_node.py index 01c7db1e2..807f28dca 100644 --- a/ironicclient/tests/unit/osc/v1/test_baremetal_node.py +++ b/ironicclient/tests/unit/osc/v1/test_baremetal_node.py @@ -731,6 +731,11 @@ class TestBaremetalCreate(TestBaremetal): [('lessee', 'lessee 1')], {'lessee': 'lessee 1'}) + def test_baremetal_create_with_shard(self): + self.check_with_options(['--shard', 'myshard'], + [('shard', 'myshard')], + {'shard': 'myshard'}) + class TestBaremetalDelete(TestBaremetal): def setUp(self): @@ -1404,7 +1409,7 @@ class TestBaremetalList(TestBaremetal): parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args=parsed_args) # Set expected values kwargs = { @@ -1430,6 +1435,69 @@ class TestBaremetalList(TestBaremetal): self.check_parser, self.cmd, arglist, verifylist) + def test_baremetal_list_by_shards(self): + arglist = ['--shards', 'myshard1', 'myshard2'] + verifylist = [ + ('shards', ['myshard1', 'myshard2']) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + kwargs = { + 'marker': None, + 'limit': None, + 'shards': ['myshard1', 'myshard2'], + } + + self.baremetal_mock.node.list.assert_called_with( + **kwargs + ) + + def test_baremetal_list_sharded(self): + arglist = ['--sharded'] + verifylist = [('sharded', True)] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + kwargs = { + 'marker': None, + 'limit': None, + 'sharded': True, + } + + self.baremetal_mock.node.list.assert_called_with( + **kwargs + ) + + def test_baremetal_list_unsharded(self): + arglist = ['--unsharded'] + verifylist = [('sharded', False)] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + kwargs = { + 'marker': None, + 'limit': None, + 'sharded': False, + } + + self.baremetal_mock.node.list.assert_called_with( + **kwargs + ) + + def test_baremetal_list_sharded_unsharded_fail(self): + arglist = ['--sharded', '--unsharded'] + verifylist = [('sharded', True), ('sharded', False)] + + self.assertRaises(oscutils.ParserException, self.check_parser, + self.cmd, arglist, verifylist) + class TestBaremetalMaintenanceSet(TestBaremetal): def setUp(self): @@ -3133,6 +3201,26 @@ class TestBaremetalSet(TestBaremetal): reset_interfaces=None ) + def test_baremetal_set_shard(self): + arglist = [ + 'node_uuid', + '--shard', 'myshard', + ] + verifylist = [ + ('nodes', ['node_uuid']), + ('shard', 'myshard') + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.baremetal_mock.node.update.assert_called_once_with( + 'node_uuid', [{'op': 'add', + 'path': '/shard', + 'value': 'myshard'}], + reset_interfaces=None + ) + @mock.patch.object(commonutils, 'get_from_stdin', autospec=True) @mock.patch.object(commonutils, 'handle_json_or_file_arg', autospec=True) def test_baremetal_set_network_data(self, mock_handle, mock_stdin): @@ -3775,6 +3863,25 @@ class TestBaremetalUnset(TestBaremetal): [{'path': '/lessee', 'op': 'remove'}] ) + def test_baremetal_unset_shard(self): + arglist = [ + 'node_uuid', + '--shard', + ] + verifylist = [ + ('nodes', ['node_uuid']), + ('shard', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + self.baremetal_mock.node.update.assert_called_once_with( + 'node_uuid', + [{'path': '/shard', 'op': 'remove'}] + ) + def test_baremetal_unset_network_data(self): arglist = [ 'node_uuid', diff --git a/ironicclient/tests/unit/v1/test_node.py b/ironicclient/tests/unit/v1/test_node.py index 416bb52c5..ec086a97f 100644 --- a/ironicclient/tests/unit/v1/test_node.py +++ b/ironicclient/tests/unit/v1/test_node.py @@ -52,7 +52,8 @@ NODE2 = {'uuid': '66666666-7777-8888-9999-111111111111', 'extra': {}, 'owner': '33333333-2222-1111-0000-111111111111', 'retired': True, - 'lessee': '77777777-8888-5555-2222-999999999999'} + 'lessee': '77777777-8888-5555-2222-999999999999', + 'shard': 'myshard'} PORT = {'uuid': '11111111-2222-3333-4444-555555555555', 'node_uuid': '66666666-7777-8888-9999-000000000000', 'address': 'AA:AA:AA:AA:AA:AA', @@ -267,6 +268,20 @@ fake_responses = { {"nodes": [NODE2]}, ) }, + '/v1/nodes/?sharded=False': + { + 'GET': ( + {}, + {"nodes": [NODE1]}, + ) + }, + '/v1/nodes/?shard=myshard': + { + 'GET': ( + {}, + {"nodes": [NODE2]} + ) + }, '/v1/nodes/detail?instance_uuid=%s' % NODE2['instance_uuid']: { 'GET': ( @@ -983,6 +998,24 @@ class NodeManagerTest(testtools.TestCase): self.assertThat(nodes, HasLength(1)) self.assertEqual(NODE2['uuid'], getattr(nodes[0], 'uuid')) + def test_node_list_not_sharded(self): + nodes = self.mgr.list(sharded=False) + expect = [ + ('GET', '/v1/nodes/?sharded=False', {}, None), + ] + self.assertEqual(expect, self.api.calls) + self.assertThat(nodes, HasLength(1)) + self.assertEqual(NODE1['uuid'], getattr(nodes[0], 'uuid')) + + def test_node_list_by_shard(self): + nodes = self.mgr.list(shards=["myshard"]) + expect = [ + ('GET', '/v1/nodes/?shard=myshard', {}, None), + ] + self.assertEqual(expect, self.api.calls) + self.assertThat(nodes, HasLength(1)) + self.assertEqual(NODE2['uuid'], getattr(nodes[0], 'uuid')) + def test_node_list_detail(self): nodes = self.mgr.list(detail=True) expect = [ diff --git a/ironicclient/v1/node.py b/ironicclient/v1/node.py index dfad79bbb..6a5287b60 100644 --- a/ironicclient/v1/node.py +++ b/ironicclient/v1/node.py @@ -62,7 +62,7 @@ class NodeManager(base.CreateManager): resource_class=None, chassis=None, fault=None, os_ironic_api_version=None, conductor_group=None, conductor=None, owner=None, retired=None, lessee=None, - global_request_id=None): + shards=None, sharded=None, global_request_id=None): """Retrieve a list of nodes. :param associated: Optional. Either a Boolean or a string @@ -126,9 +126,14 @@ class NodeManager(base.CreateManager): :param conductor: Optional. String value to get only nodes mapped to the given conductor. :param owner: Optional. String value to get only nodes - mapped to a specific owner. + mapped to a specific owner. :param lessee: Optional. String value to get only nodes - mapped to a specific lessee. + mapped to a specific lessee. + :param shards: Optional. A list with a specified set of shards + to limit node returns to. + :param sharded: Optional. Boolean value, when true get only nodes + with a non-null node.shard value, when false get only + nodes with a null node.shard value. None is a noop. :returns: A list of nodes. @@ -166,6 +171,10 @@ class NodeManager(base.CreateManager): filters.append('owner=%s' % owner) if lessee is not None: filters.append('lessee=%s' % lessee) + if sharded is not None: + filters.append('sharded=%s' % sharded) + if shards is not None: + filters.append('shard=%s' % ','.join(shards)) path = '' if detail: diff --git a/releasenotes/notes/node-shard-support-774ebfe6719fc7c2.yaml b/releasenotes/notes/node-shard-support-774ebfe6719fc7c2.yaml new file mode 100644 index 000000000..46da81f2a --- /dev/null +++ b/releasenotes/notes/node-shard-support-774ebfe6719fc7c2.yaml @@ -0,0 +1,4 @@ +features: + - Add support for Node shard field. + - Add support for listing nodes by one or more shards. + - Add support for filtering nodes by if they are sharded. \ No newline at end of file