From 8e34d622aff72d7dd286add31e3d7cd366629bc2 Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Wed, 30 Nov 2022 15:28:29 -0800 Subject: [PATCH] API support for CRUD node.shard - Basic support and testing for CRUD for node.shard. - Policy checking for update node.shard. - New API endpoint: GET /v1/shards - Policy checking for GET /v1/shards - Support for querying for nodes in a list of shards Story: 2010378 Task: 46624 Change-Id: I385594339028c20cfc83fdcc4cbbec107efdacff --- api-ref/source/baremetal-api-v1-nodes.inc | 24 ++- api-ref/source/baremetal-api-v1-shards.inc | 56 +++++++ api-ref/source/index.rst | 1 + api-ref/source/parameters.yaml | 23 +++ .../source/samples/shards-list-response.json | 12 ++ .../contributor/webapi-version-history.rst | 11 +- ironic/api/controllers/v1/__init__.py | 14 +- ironic/api/controllers/v1/node.py | 36 +++-- ironic/api/controllers/v1/shard.py | 59 +++++++ ironic/api/controllers/v1/utils.py | 20 +++ ironic/api/controllers/v1/versions.py | 3 +- ironic/common/policy.py | 17 ++- ironic/common/release_mappings.py | 2 +- ironic/db/sqlalchemy/api.py | 2 +- .../unit/api/controllers/v1/test_node.py | 144 ++++++++++++++++++ .../unit/api/controllers/v1/test_root.py | 4 + .../unit/api/controllers/v1/test_shard.py | 80 ++++++++++ .../unit/api/test_rbac_project_scoped.yaml | 17 +++ .../unit/api/test_rbac_system_scoped.yaml | 24 +++ 19 files changed, 532 insertions(+), 17 deletions(-) create mode 100644 api-ref/source/baremetal-api-v1-shards.inc create mode 100644 api-ref/source/samples/shards-list-response.json create mode 100644 ironic/api/controllers/v1/shard.py create mode 100644 ironic/tests/unit/api/controllers/v1/test_shard.py diff --git a/api-ref/source/baremetal-api-v1-nodes.inc b/api-ref/source/baremetal-api-v1-nodes.inc index 2ebbd2c5d4..4734c1b734 100644 --- a/api-ref/source/baremetal-api-v1-nodes.inc +++ b/api-ref/source/baremetal-api-v1-nodes.inc @@ -104,6 +104,9 @@ supplied when the Node is created, or the resource may be updated later. .. versionadded:: 1.65 Introduced the ``lessee`` field. +.. versionadded:: 1.82 + Introduced the ``shard`` field. + Normal response codes: 201 Error codes: 400,403,406 @@ -135,6 +138,7 @@ Request - owner: owner - description: req_n_description - lessee: lessee + - shard: shard - automated_clean: req_automated_clean - bios_interface: req_bios_interface - chassis_uuid: req_chassis_uuid @@ -161,7 +165,7 @@ and any defaults added for non-specified fields. Most fields default to "null" or "". The list and example below are representative of the response as of API -microversion 1.48. +microversion 1.81. .. rest_parameters:: parameters.yaml @@ -213,6 +217,7 @@ microversion 1.48. - conductor: conductor - owner: owner - lessee: lessee + - shard: shard - description: n_description - allocation_uuid: allocation_uuid - automated_clean: automated_clean @@ -280,6 +285,9 @@ provision state, and maintenance setting for each Node. .. versionadded:: 1.65 Introduced the ``lessee`` field. +.. versionadded:: 1.82 + Introduced the ``shard`` field. + Normal response codes: 200 Error codes: 400,403,406 @@ -300,6 +308,7 @@ Request - fault: r_fault - owner: owner - lessee: lessee + - shard: req_shard - description_contains: r_description_contains - fields: fields - limit: limit @@ -371,6 +380,9 @@ Nova instance, eg. with a request to ``v1/nodes/detail?instance_uuid={NOVA INSTA .. versionadded:: 1.65 Introduced the ``lessee`` field. +.. versionadded:: 1.82 + Introduced the ``shard`` field. + Normal response codes: 200 Error codes: 400,403,406 @@ -391,6 +403,7 @@ Request - conductor: r_conductor - owner: owner - lessee: lessee + - shard: req_shard - description_contains: r_description_contains - limit: limit - marker: marker @@ -450,6 +463,7 @@ Response - protected_reason: protected_reason - owner: owner - lessee: lessee + - shard: shard - description: n_description - conductor: conductor - allocation_uuid: allocation_uuid @@ -508,6 +522,9 @@ only the specified set. .. versionadded:: 1.66 Introduced the ``network_data`` field. +.. versionadded:: 1.82 + Introduced the ``shard`` field. + Normal response codes: 200 Error codes: 400,403,404,406 @@ -573,6 +590,7 @@ Response - protected_reason: protected_reason - owner: owner - lessee: lessee + - shard: shard - description: n_description - conductor: conductor - allocation_uuid: allocation_uuid @@ -600,6 +618,9 @@ managed through sub-resources. .. versionadded:: 1.51 Introduced the ability to set/unset a node's description. +.. versionadded:: 1.82 + Introduced the ability to set/unset a node's shard. + Normal response codes: 200 Error codes: 400,403,404,406,409 @@ -670,6 +691,7 @@ Response - protected_reason: protected_reason - owner: owner - lessee: lessee + - shard: shard - description: n_description - conductor: conductor - allocation_uuid: allocation_uuid diff --git a/api-ref/source/baremetal-api-v1-shards.inc b/api-ref/source/baremetal-api-v1-shards.inc new file mode 100644 index 0000000000..c051e506ef --- /dev/null +++ b/api-ref/source/baremetal-api-v1-shards.inc @@ -0,0 +1,56 @@ +.. -*- rst -*- + +====== +Shards +====== + +This section describes an API endpoint returning the population of shards +among nodes in the Bare Metal Service. Shards are a way to group nodes in the +Bare Metal service. They are used by API clients to separate nodes into groups, +allowing horizontal scaling. + +Shards are not directly added and removed from the Bare Metal service. Instead, +operators can configure a node into a given shard by setting the ``shard`` key +to any unique string value representing the shard. + +.. note:: + The Bare Metal Service does not use shards directly. It instead relies on + API clients and external services to use shards to group nodes into smaller + areas of responsibility. + + +Shards +====== + +.. rest_method:: GET /v1/shards + +.. versionadded:: 1.82 + +The ``/v1/shards`` endpoint exists to allow querying the distribution of nodes +between all defined shards. + +Normal response codes: 200 + +Error response codes: 400 403 404 + +Request +------- + +No request parameters are accepted by this endpoint. + +Response +-------- + +Returns a list of shards and the count of nodes assigned to each. The +list is sorted by descending count. + +.. rest_parameters:: parameters.yaml + + - name: shard_name + - count: shard_count + +Response Example +---------------- + +.. literalinclude:: samples/shards-list-response.json + :language: javascript diff --git a/api-ref/source/index.rst b/api-ref/source/index.rst index 50c6a6d14f..bb41ba6fd1 100644 --- a/api-ref/source/index.rst +++ b/api-ref/source/index.rst @@ -28,6 +28,7 @@ .. include:: baremetal-api-v1-node-allocation.inc .. include:: baremetal-api-v1-deploy-templates.inc .. include:: baremetal-api-v1-nodes-history.inc +.. include:: baremetal-api-v1-shards.inc .. NOTE(dtantsur): keep chassis close to the end since it's semi-deprecated .. include:: baremetal-api-v1-chassis.inc .. NOTE(dtantsur): keep misc last, since it covers internal API diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 361cca531d..930d85f1e6 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1813,6 +1813,13 @@ req_resource_class_create: in: body required: false type: string +req_shard: + description: | + Filter the list of returned Nodes, and only return the ones associated + with nodes in this specific shard(s), or an empty set if not found. + in: body + required: false + type: array req_standalone_ports_supported: description: | Indicates whether ports that are members of this portgroup can be @@ -1938,6 +1945,22 @@ secure_boot: Indicates whether node is currently booted with secure_boot turned on. in: body type: boolean +shard: + description: | + A string indicating the shard this node belongs to. + in: body + type: string +shard_count: + description: | + The number of nodes with this current string as their assigned shard value. + in: body + type: integer +shard_name: + description: | + The name of the shard. A value of "None" indicates the count of nodes with + an empty shard value. + in: body + type: string standalone_ports_supported: description: | Indicates whether ports that are members of this portgroup can be diff --git a/api-ref/source/samples/shards-list-response.json b/api-ref/source/samples/shards-list-response.json new file mode 100644 index 0000000000..776dd324b2 --- /dev/null +++ b/api-ref/source/samples/shards-list-response.json @@ -0,0 +1,12 @@ +{ + "shards": [ + { + "count": 47, + "name": "example_shard1", + }, + { + "count": 46, + "name": "example_shard2" + } + ] +} diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index 51b4a8d039..11e529292f 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,15 @@ REST API Version History ======================== +1.82 (Antelope) +---------------------- + +This version signifies the addition of node sharding endpoints. + +- Adds support for get, set, and delete of shard key on Node object. +- Adds support for ``GET /v1/shards`` which returns a list of all shards and + the count of nodes assigned to each. + 1.81 (Antelope) ---------------------- @@ -12,7 +21,7 @@ Add endpoint to retrieve introspection data for nodes via the REST API. 1.80 (Zed, 21.1) ---------------------- -This verison is a signifier of additional RBAC functionality allowing +This version is a signifier of additional RBAC functionality allowing a project scoped ``admin`` to create or delete nodes in Ironic. 1.79 (Zed, 21.0) diff --git a/ironic/api/controllers/v1/__init__.py b/ironic/api/controllers/v1/__init__.py index a944dec69d..9bd9af9850 100644 --- a/ironic/api/controllers/v1/__init__.py +++ b/ironic/api/controllers/v1/__init__.py @@ -36,6 +36,7 @@ from ironic.api.controllers.v1 import node from ironic.api.controllers.v1 import port from ironic.api.controllers.v1 import portgroup from ironic.api.controllers.v1 import ramdisk +from ironic.api.controllers.v1 import shard from ironic.api.controllers.v1 import utils from ironic.api.controllers.v1 import versions from ironic.api.controllers.v1 import volume @@ -182,6 +183,16 @@ def v1(): 'deploy_templates', '', bookmark=True) ] + if utils.allow_shards_endpoint(): + v1['shards'] = [ + link.make_link('self', + api.request.public_url, + 'shards', ''), + link.make_link('bookmark', + api.request.public_url, + 'shards', '', + bookmark=True) + ] return v1 @@ -200,7 +211,8 @@ class Controller(object): 'conductors': conductor.ConductorsController(), 'allocations': allocation.AllocationsController(), 'events': event.EventsController(), - 'deploy_templates': deploy_template.DeployTemplatesController() + 'deploy_templates': deploy_template.DeployTemplatesController(), + 'shards': shard.ShardController(), } @method.expose() diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 4b2f61b1cc..9a117a95fd 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -268,6 +268,7 @@ PATCH_ALLOWED_FIELDS = [ 'resource_class', 'retired', 'retired_reason', + 'shard', 'storage_interface', 'vendor_interface' ] @@ -2070,7 +2071,7 @@ class NodesController(rest.RestController): fields=None, fault=None, conductor_group=None, detail=None, conductor=None, owner=None, lessee=None, project=None, - description_contains=None): + description_contains=None, shard=None): if self.from_chassis and not chassis_uuid: raise exception.MissingParameterValue( _("Chassis id not specified.")) @@ -2090,6 +2091,12 @@ class NodesController(rest.RestController): # The query parameters for the 'next' URL parameters = {} + + # note(JayF): This is where you resolve differences between the name + # of the filter in the API and the name of the filter in the DB API. + # In the case of lists (args.string_list), you need to append _in to + # the filter name in order to exercise the list-aware logic in the + # lower level. possible_filters = { 'maintenance': maintenance, 'chassis_uuid': chassis_uuid, @@ -2101,6 +2108,7 @@ class NodesController(rest.RestController): 'conductor_group': conductor_group, 'owner': owner, 'lessee': lessee, + 'shard_in': shard, 'project': project, 'description_contains': description_contains, 'retired': retired, @@ -2123,7 +2131,7 @@ class NodesController(rest.RestController): # map the name for the call, as we did not pickup a specific # list of fields to return. obj_fields = fields - # NOTE(TheJulia): When a data set of the nodeds list is being + # NOTE(TheJulia): When a data set of the nodes list is being # requested, this method takes approximately 3-3.5% of the time # when requesting specific fields aligning with Nova's sync # process. (Local DB though) @@ -2248,14 +2256,15 @@ class NodesController(rest.RestController): fault=args.string, conductor_group=args.string, detail=args.boolean, conductor=args.string, owner=args.string, description_contains=args.string, - lessee=args.string, project=args.string) + lessee=args.string, project=args.string, + shard=args.string_list) def get_all(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, retired=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, fields=None, resource_class=None, fault=None, conductor_group=None, detail=None, conductor=None, owner=None, description_contains=None, lessee=None, - project=None): + project=None, shard=None): """Retrieve a list of nodes. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -2290,9 +2299,11 @@ class NodesController(rest.RestController): :param owner: Optional string value that set the owner whose nodes are to be retrurned. :param lessee: Optional string value that set the lessee whose nodes - are to be returned. + are to be returned. :param project: Optional string value that set the project - lessee or owner - whose nodes are to be returned. + :param shard: Optional string value that set the shards whose nodes are + to be returned. :param fields: Optional, a list with a specified set of fields of the resource to be returned. :param fault: Optional string value to get only nodes with that fault. @@ -2313,6 +2324,7 @@ class NodesController(rest.RestController): api_utils.check_allow_filter_by_conductor(conductor) api_utils.check_allow_filter_by_owner(owner) api_utils.check_allow_filter_by_lessee(lessee) + api_utils.check_allow_filter_by_shard(shard) fields = api_utils.get_request_return_fields(fields, detail, _DEFAULT_RETURN_FIELDS) @@ -2329,7 +2341,7 @@ class NodesController(rest.RestController): detail=detail, conductor=conductor, owner=owner, lessee=lessee, - project=project, + shard=shard, project=project, **extra_args) @METRICS.timer('NodesController.detail') @@ -2342,13 +2354,15 @@ class NodesController(rest.RestController): resource_class=args.string, fault=args.string, conductor_group=args.string, conductor=args.string, owner=args.string, description_contains=args.string, - lessee=args.string, project=args.string) + lessee=args.string, project=args.string, + shard=args.string_list) def detail(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, retired=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, resource_class=None, fault=None, conductor_group=None, conductor=None, owner=None, - description_contains=None, lessee=None, project=None): + description_contains=None, lessee=None, project=None, + shard=None): """Retrieve a list of nodes with detail. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -2385,6 +2399,7 @@ class NodesController(rest.RestController): are to be returned. :param project: Optional string value that set the project - lessee or owner - whose nodes are to be returned. + :param shard: Optional - set the shards whose nodes are to be returned. :param description_contains: Optional string value to get only nodes with description field contains matching value. @@ -2405,6 +2420,7 @@ class NodesController(rest.RestController): raise exception.HTTPNotFound() api_utils.check_allow_filter_by_conductor(conductor) + api_utils.check_allow_filter_by_shard(shard) extra_args = {'description_contains': description_contains} return self._get_nodes_collection(chassis_uuid, instance_uuid, @@ -2418,7 +2434,7 @@ class NodesController(rest.RestController): conductor_group=conductor_group, conductor=conductor, owner=owner, lessee=lessee, - project=project, + project=project, shard=shard, **extra_args) @METRICS.timer('NodesController.validate') @@ -2644,6 +2660,8 @@ class NodesController(rest.RestController): policy_checks.append('baremetal:node:update:name') elif p['path'].startswith('/retired'): policy_checks.append('baremetal:node:update:retired') + elif p['path'].startswith('/shard'): + policy_checks.append('baremetal:node:update:shard') else: generic_update = True # always do at least one check diff --git a/ironic/api/controllers/v1/shard.py b/ironic/api/controllers/v1/shard.py new file mode 100644 index 0000000000..7aa0869970 --- /dev/null +++ b/ironic/api/controllers/v1/shard.py @@ -0,0 +1,59 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic_lib import metrics_utils +from oslo_config import cfg +import pecan +from webob import exc as webob_exc + +from ironic import api +from ironic.api.controllers.v1 import utils as api_utils +from ironic.api import method +from ironic.common.i18n import _ + + +CONF = cfg.CONF + +METRICS = metrics_utils.get_metrics_logger(__name__) + + +class ShardController(pecan.rest.RestController): + """REST controller for shards.""" + + @pecan.expose() + def _route(self, argv, request=None): + if not api_utils.allow_shards_endpoint(): + msg = _("The API version does not allow shards") + if api.request.method in "GET": + raise webob_exc.HTTPNotFound(msg) + else: + raise webob_exc.HTTPMethodNotAllowed(msg) + return super(ShardController, self)._route(argv, request) + + @METRICS.timer('ShardController.get_all') + @method.expose() + def get_all(self): + """Retrieve a list of shards. + + :returns: A list of shards. + """ + api_utils.check_policy('baremetal:shards:get') + + return { + 'shards': api.request.dbapi.get_shard_list(), + } + + @METRICS.timer('ShardController.get_one') + @method.expose() + def get_one(self, __): + """Explicitly do not support getting one.""" + pecan.abort(404) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 0494077cc4..1f92b89bca 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -807,6 +807,7 @@ VERSIONED_FIELDS = { 'network_data': versions.MINOR_66_NODE_NETWORK_DATA, 'boot_mode': versions.MINOR_75_NODE_BOOT_MODE, 'secure_boot': versions.MINOR_75_NODE_BOOT_MODE, + 'shard': versions.MINOR_82_NODE_SHARD } for field in V31_FIELDS: @@ -1065,6 +1066,20 @@ def check_allow_filter_by_lessee(lessee): 'opr': versions.MINOR_65_NODE_LESSEE}) +def check_allow_filter_by_shard(shard): + """Check if filtering nodes by shard is allowed. + + Version 1.81 of the API allows filtering nodes by shard. + """ + if (shard is not None and api.request.version.minor + < versions.MINOR_82_NODE_SHARD): + raise exception.NotAcceptable(_( + "Request not acceptable. The minimal required API version " + "should be %(base)s.%(opr)s") % + {'base': versions.BASE_VERSION, + 'opr': versions.MINOR_82_NODE_SHARD}) + + def initial_node_provision_state(): """Return node state to use by default when creating new nodes. @@ -1953,3 +1968,8 @@ def check_allow_clean_disable_ramdisk(target, disable_ramdisk): elif target != "clean": raise exception.BadRequest( _("disable_ramdisk is supported only with manual cleaning")) + + +def allow_shards_endpoint(): + """Check if shards endpoint is available.""" + return api.request.version.minor >= versions.MINOR_82_NODE_SHARD diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 4dcfd7fb8e..aa8131570c 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -201,6 +201,7 @@ MINOR_78_NODE_HISTORY = 78 MINOR_79_ALLOCATION_NODE_NAME = 79 MINOR_80_PROJECT_CREATE_DELETE_NODE = 80 MINOR_81_NODE_INVENTORY = 81 +MINOR_82_NODE_SHARD = 82 # When adding another version, update: # - MINOR_MAX_VERSION @@ -208,7 +209,7 @@ MINOR_81_NODE_INVENTORY = 81 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_81_NODE_INVENTORY +MINOR_MAX_VERSION = MINOR_82_NODE_SHARD # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 39733c7325..b877611af6 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -986,8 +986,21 @@ node_policies = [ # operating context. deprecated_rule=deprecated_node_get ), - - + policy.DocumentedRuleDefault( + name='baremetal:node:update:shard', + check_str=SYSTEM_ADMIN, + scope_types=['system', 'project'], + description='Governs if node shard field can be updated via ' + 'the API clients.', + operations=[{'path': '/nodes/{node_ident}', 'method': 'PATCH'}], + ), + policy.DocumentedRuleDefault( + name='baremetal:shards:get', + check_str=SYSTEM_READER, + scope_types=['system', 'project'], + description='Governs if shards can be read via the API clients.', + operations=[{'path': '/shards', 'method': 'GET'}], + ), ] deprecated_port_reason = """ diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 6d96bf20b7..ad9f8f43a0 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -511,7 +511,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.81', + 'api': '1.82', 'rpc': '1.55', 'objects': { 'Allocation': ['1.1'], diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 7e9254b052..bfc03b9a70 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -398,7 +398,7 @@ class Connection(api.Connection): 'uuid', 'id', 'fault', 'conductor_group', 'owner', 'lessee', 'instance_uuid'} _NODE_IN_QUERY_FIELDS = {'%s_in' % field: field - for field in ('uuid', 'provision_state')} + for field in ('uuid', 'provision_state', 'shard')} _NODE_NON_NULL_FILTERS = {'associated': 'instance_uuid', 'reserved': 'reservation', 'with_power_state': 'power_state'} diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 3c0049a737..e1ef916b9a 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -21,6 +21,7 @@ import sys import tempfile from unittest import mock from urllib import parse as urlparse +import uuid import fixtures from oslo_config import cfg @@ -7968,3 +7969,146 @@ class TestNodeInventory(test_api_base.BaseApiTest): headers={api_base.Version.string: self.version}) self.assertEqual({'inventory': self.fake_inventory_data, 'plugin_data': self.fake_plugin_data}, ret) + + +class TestNodeShardGets(test_api_base.BaseApiTest): + def setUp(self): + super(TestNodeShardGets, self).setUp() + p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', + autospec=True) + self.mock_gtf = p.start() + self.mock_gtf.return_value = 'test-topic' + self.addCleanup(p.stop) + self.mock_get_conductor_for = self.useFixture( + fixtures.MockPatchObject(rpcapi.ConductorAPI, 'get_conductor_for', + autospec=True)).mock + self.mock_get_conductor_for.return_value = 'fake.conductor' + self.node = obj_utils.create_test_node(self.context, shard='foo') + self.headers = {api_base.Version.string: '1.82'} + + def test_get_node_shard_field(self): + result = self.get_json( + '/nodes/%s' % self.node.uuid, headers=self.headers) + self.assertEqual('foo', result['shard']) + + def test_get_node_shard_field_fails_wrong_version(self): + headers = {api_base.Version.string: '1.80'} + result = self.get_json('/nodes/%s' % self.node.uuid, headers=headers) + self.assertNotIn('shard', result) + + def test_filtering_by_shard(self): + result = self.get_json( + '/nodes?shard=foo', fields='shard', headers=self.headers) + self.assertEqual(1, len(result['nodes'])) + self.assertEqual('foo', result['nodes'][0]['shard']) + + def test_filtering_by_shard_fails_wrong_version(self): + headers = {api_base.Version.string: '1.80'} + + result = self.get_json('/nodes?shard=foo', + expect_errors=True, headers=headers) + self.assertEqual(http_client.NOT_ACCEPTABLE, result.status_code) + + def test_filtering_by_single_shard_detail(self): + result = self.get_json('/nodes/detail?shard=foo', headers=self.headers) + self.assertEqual(1, len(result['nodes'])) + self.assertEqual('foo', result['nodes'][0]['shard']) + + def test_filtering_by_multi_shard_detail(self): + obj_utils.create_test_node( + self.context, uuid=uuid.uuid4(), shard='bar') + result = self.get_json( + '/nodes?shard=foo,bar', headers=self.headers) + self.assertEqual(2, len(result['nodes'])) + + def test_filtering_by_shard_detail_fails_wrong_version(self): + headers = {api_base.Version.string: '1.80'} + + result = self.get_json('/nodes/detail?shard=foo', + expect_errors=True, headers=headers) + self.assertEqual(http_client.NOT_ACCEPTABLE, result.status_code) + + +@mock.patch.object(rpcapi.ConductorAPI, 'create_node', + lambda _api, _ctx, node, _topic: _create_node_locally(node)) +class TestNodeShardPost(test_api_base.BaseApiTest): + def setUp(self): + super(TestNodeShardPost, self).setUp() + p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', + autospec=True) + self.mock_gtf = p.start() + self.mock_gtf.return_value = 'test-topic' + self.addCleanup(p.stop) + self.chassis = obj_utils.create_test_chassis(self.context) + + def test_create_node_with_shard(self): + shard = 'foo' + ndict = test_api_utils.post_get_test_node(shard=shard) + headers = {api_base.Version.string: '1.82'} + response = self.post_json('/nodes', ndict, headers=headers) + self.assertEqual(http_client.CREATED, response.status_int) + + result = self.get_json('/nodes/%s' % ndict['uuid'], headers=headers) + self.assertEqual(ndict['uuid'], result['uuid']) + self.assertEqual(shard, result['shard']) + + def test_create_node_with_shard_fail_wrong_version(self): + headers = {api_base.Version.string: '1.80'} + shard = 'foo' + ndict = test_api_utils.post_get_test_node(shard=shard) + response = self.post_json( + '/nodes', ndict, expect_errors=True, headers=headers) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + + +class TestNodeShardPatch(test_api_base.BaseApiTest): + def setUp(self): + super(TestNodeShardPatch, self).setUp() + self.node = obj_utils.create_test_node(self.context, name='node-57.1') + p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', + autospec=True) + self.mock_gtf = p.start() + self.mock_gtf.return_value = 'test-topic' + self.addCleanup(p.stop) + p = mock.patch.object(rpcapi.ConductorAPI, 'update_node', + autospec=True) + self.mock_update_node = p.start() + self.addCleanup(p.stop) + + def test_node_add_shard(self): + self.mock_update_node.return_value = self.node + (self + .mock_update_node + .return_value + .updated_at) = "2013-12-03T06:20:41.184720+00:00" + headers = {api_base.Version.string: '1.82'} + shard = 'shard1' + body = [{ + 'path': '/shard', + 'value': shard, + 'op': 'add', + }] + + response = self.patch_json( + '/nodes/%s' % self.node.uuid, body, headers=headers) + self.assertEqual(http_client.OK, response.status_code) + self.mock_update_node.assert_called_once() + + def test_node_add_shard_fail_wrong_version(self): + self.mock_update_node.return_value = self.node + (self + .mock_update_node + .return_value + .updated_at) = "2013-12-03T06:20:41.184720+00:00" + headers = {api_base.Version.string: '1.80'} + shard = 'shard1' + body = [{ + 'path': '/shard', + 'value': shard, + 'op': 'add', + }] + + response = self.patch_json('/nodes/%s' % self.node.uuid, + body, expect_errors=True, headers=headers) + self.mock_update_node.assert_not_called() + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) diff --git a/ironic/tests/unit/api/controllers/v1/test_root.py b/ironic/tests/unit/api/controllers/v1/test_root.py index 78d3053e40..229e88622f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_root.py +++ b/ironic/tests/unit/api/controllers/v1/test_root.py @@ -147,6 +147,10 @@ class TestV1Routing(api_base.BaseApiTest): {'href': 'http://localhost/v1/ports/', 'rel': 'self'}, {'href': 'http://localhost/ports/', 'rel': 'bookmark'} ], + 'shards': [ + {'href': 'http://localhost/v1/shards/', 'rel': 'self'}, + {'href': 'http://localhost/shards/', 'rel': 'bookmark'} + ], 'volume': [ {'href': 'http://localhost/v1/volume/', 'rel': 'self'}, {'href': 'http://localhost/volume/', 'rel': 'bookmark'} diff --git a/ironic/tests/unit/api/controllers/v1/test_shard.py b/ironic/tests/unit/api/controllers/v1/test_shard.py new file mode 100644 index 0000000000..73d30f1063 --- /dev/null +++ b/ironic/tests/unit/api/controllers/v1/test_shard.py @@ -0,0 +1,80 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +""" +Tests for the API /shards/ methods. +""" + +from http import client as http_client +import uuid + +from ironic.api.controllers import base as api_base +from ironic.api.controllers import v1 as api_v1 +from ironic.tests.unit.api import base as test_api_base +from ironic.tests.unit.objects import utils as obj_utils + + +class TestListShards(test_api_base.BaseApiTest): + headers = {api_base.Version.string: str(api_v1.max_version())} + + def _create_test_shard(self, name, count): + for i in range(count): + obj_utils.create_test_node( + self.context, uuid=uuid.uuid4(), shard=name) + + def test_empty(self): + data = self.get_json('/shards', headers=self.headers) + self.assertEqual([], data['shards']) + + def test_one_shard(self): + shard = 'shard1' + count = 1 + self._create_test_shard(shard, count) + data = self.get_json('/shards', headers=self.headers) + self.assertEqual(shard, data['shards'][0]['name']) + self.assertEqual(count, data['shards'][0]['count']) + + def test_multiple_shards(self): + for i in range(0, 6): + self._create_test_shard('shard{}'.format(i), i) + data = self.get_json('/shards', headers=self.headers) + self.assertEqual(5, len(data['shards'])) + + def test_nodes_but_no_shards(self): + self._create_test_shard(None, 5) + data = self.get_json('/shards', headers=self.headers) + self.assertEqual("None", data['shards'][0]['name']) + self.assertEqual(5, data['shards'][0]['count']) + + def test_fail_wrong_version(self): + headers = {api_base.Version.string: '1.80'} + self._create_test_shard('shard1', 1) + result = self.get_json( + '/shards', expect_errors=True, headers=headers) + self.assertEqual(http_client.NOT_FOUND, result.status_int) + + def test_fail_get_one(self): + # We do not implement a get /v1/shards/ endpoint + # validate it errors properly + self._create_test_shard('shard1', 1) + result = self.get_json( + '/shards/shard1', expect_errors=True, headers=self.headers) + self.assertEqual(http_client.NOT_FOUND, result.status_int) + + def test_fail_post(self): + result = self.post_json( + '/shards', {}, expect_errors=True, headers=self.headers) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, result.status_int) + + def test_fail_put(self): + result = self.put_json( + '/shards', {}, expect_errors=True, headers=self.headers) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, result.status_int) diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml index ad3342e860..0e21076588 100644 --- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml @@ -3871,3 +3871,20 @@ lessee_node_inventory_get_reader: method: get headers: *lessee_reader_headers assert_status: 404 + +# Shard support - system scoped req'd to set on a node or view via /v1/shards +shard_get_shards_disallowed: + path: '/v1/shards' + method: get + headers: *owner_reader_headers + assert_status: 403 + +shard_patch_set_node_shard_disallowed: + path: '/v1/nodes/{owner_node_ident}' + method: patch + headers: *owner_admin_headers + body: + - op: replace + path: /shard + value: 'TestShard' + assert_status: 403 diff --git a/ironic/tests/unit/api/test_rbac_system_scoped.yaml b/ironic/tests/unit/api/test_rbac_system_scoped.yaml index a980fefc7a..16f0fded6c 100644 --- a/ironic/tests/unit/api/test_rbac_system_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_system_scoped.yaml @@ -2289,3 +2289,27 @@ node_history_get_entry_service: method: get headers: *service_headers assert_status: 200 + +# Shard support +shard_get_shards: + path: '/v1/shards' + method: get + headers: *reader_headers + assert_status: 200 + +shard_patch_set_node_shard: + path: '/v1/nodes/{node_ident}' + method: patch + headers: *admin_headers + body: &replace_shard + - op: replace + path: /shard + value: 'TestShard' + assert_status: 503 + +shard_patch_set_node_shard_disallowed: + path: '/v1/nodes/{node_ident}' + method: patch + headers: *scoped_member_headers + body: *replace_shard + assert_status: 403