diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index faefd50ab5..174151117a 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,12 @@ REST API Version History ======================== +1.49 (Stein, master) +-------------------- + +Added new endpoints for retrieving conductors information, and added a +``conductor`` field to node object. + 1.48 (Stein, master) -------------------- diff --git a/ironic/api/controllers/v1/__init__.py b/ironic/api/controllers/v1/__init__.py index 4af57f94b8..074f6d84ba 100644 --- a/ironic/api/controllers/v1/__init__.py +++ b/ironic/api/controllers/v1/__init__.py @@ -26,6 +26,7 @@ from wsme import types as wtypes from ironic.api.controllers import base from ironic.api.controllers import link from ironic.api.controllers.v1 import chassis +from ironic.api.controllers.v1 import conductor from ironic.api.controllers.v1 import driver from ironic.api.controllers.v1 import node from ironic.api.controllers.v1 import port @@ -100,6 +101,9 @@ class V1(base.APIBase): heartbeat = [link.Link] """Links to the heartbeat resource""" + conductors = [link.Link] + """Links to the conductors resource""" + version = version.Version """Version discovery information.""" @@ -178,6 +182,15 @@ class V1(base.APIBase): 'heartbeat', '', bookmark=True) ] + if utils.allow_expose_conductors(): + v1.conductors = [link.Link.make_link('self', + pecan.request.public_url, + 'conductors', ''), + link.Link.make_link('bookmark', + pecan.request.public_url, + 'conductors', '', + bookmark=True) + ] v1.version = version.default_version() return v1 @@ -193,6 +206,7 @@ class Controller(rest.RestController): volume = volume.VolumeController() lookup = ramdisk.LookupController() heartbeat = ramdisk.HeartbeatController() + conductors = conductor.ConductorsController() @expose.expose(V1) def get(self): diff --git a/ironic/api/controllers/v1/collection.py b/ironic/api/controllers/v1/collection.py index 9f82bd1bfe..0328194415 100644 --- a/ironic/api/controllers/v1/collection.py +++ b/ironic/api/controllers/v1/collection.py @@ -29,6 +29,10 @@ class Collection(base.APIBase): def collection(self): return getattr(self, self._type) + @classmethod + def get_key_field(cls): + return 'uuid' + def has_next(self, limit): """Return whether collection has more items.""" return len(self.collection) and len(self.collection) == limit @@ -42,7 +46,7 @@ class Collection(base.APIBase): q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs]) next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % { 'args': q_args, 'limit': limit, - 'marker': self.collection[-1].uuid} + 'marker': getattr(self.collection[-1], self.get_key_field())} return link.Link.make_link('next', pecan.request.public_url, resource_url, next_args).href diff --git a/ironic/api/controllers/v1/conductor.py b/ironic/api/controllers/v1/conductor.py new file mode 100644 index 0000000000..b405e3e106 --- /dev/null +++ b/ironic/api/controllers/v1/conductor.py @@ -0,0 +1,251 @@ +# 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. + +import datetime + +from ironic_lib import metrics_utils +from oslo_log import log +from oslo_utils import timeutils +import pecan +from pecan import rest +import wsme +from wsme import types as wtypes + +from ironic.api.controllers import base +from ironic.api.controllers import link +from ironic.api.controllers.v1 import collection +from ironic.api.controllers.v1 import types +from ironic.api.controllers.v1 import utils as api_utils +from ironic.api import expose +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common import policy +import ironic.conf +from ironic import objects + +CONF = ironic.conf.CONF +LOG = log.getLogger(__name__) +METRICS = metrics_utils.get_metrics_logger(__name__) + +_DEFAULT_RETURN_FIELDS = ('hostname', 'conductor_group', 'alive') + + +class Conductor(base.APIBase): + """API representation of a bare metal conductor.""" + + hostname = wsme.wsattr(wtypes.text) + """The hostname for this conductor""" + + conductor_group = wsme.wsattr(wtypes.text) + """The conductor group this conductor belongs to""" + + alive = types.boolean + """Indicates whether this conductor is considered alive""" + + drivers = wsme.wsattr([wtypes.text]) + """The drivers enabled on this conductor""" + + links = wsme.wsattr([link.Link]) + """A list containing a self link and associated conductor links""" + + def __init__(self, **kwargs): + self.fields = [] + fields = list(objects.Conductor.fields) + # NOTE(kaifeng): alive is not part of objects.Conductor.fields + # because it's an API-only attribute. + fields.append('alive') + + for field in fields: + # Skip fields we do not expose. + if not hasattr(self, field): + continue + self.fields.append(field) + setattr(self, field, kwargs.get(field, wtypes.Unset)) + + @staticmethod + def _convert_with_links(conductor, url, fields=None): + conductor.links = [link.Link.make_link('self', url, 'conductors', + conductor.hostname), + link.Link.make_link('bookmark', url, 'conductors', + conductor.hostname, + bookmark=True)] + return conductor + + @classmethod + def convert_with_links(cls, rpc_conductor, fields=None): + conductor = Conductor(**rpc_conductor.as_dict()) + conductor.alive = not timeutils.is_older_than( + conductor.updated_at, CONF.conductor.heartbeat_timeout) + + if fields is not None: + api_utils.check_for_invalid_fields(fields, conductor.as_dict()) + + conductor = cls._convert_with_links(conductor, + pecan.request.public_url, + fields=fields) + conductor.sanitize(fields) + return conductor + + def sanitize(self, fields): + """Removes sensitive and unrequested data. + + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + if fields is not None: + self.unset_fields_except(fields) + + @classmethod + def sample(cls, expand=True): + time = datetime.datetime(2000, 1, 1, 12, 0, 0) + sample = cls(hostname='computer01', + conductor_group='', + alive=True, + drivers=['ipmi'], + created_at=time, + updated_at=time) + fields = None if expand else _DEFAULT_RETURN_FIELDS + return cls._convert_with_links(sample, 'http://localhost:6385', + fields=fields) + + +class ConductorCollection(collection.Collection): + """API representation of a collection of conductors.""" + + conductors = [Conductor] + """A list containing conductor objects""" + + def __init__(self, **kwargs): + self._type = 'conductors' + + # NOTE(kaifeng) Override because conductors use hostname instead of uuid. + @classmethod + def get_key_field(cls): + return 'hostname' + + @staticmethod + def convert_with_links(conductors, limit, url=None, fields=None, **kwargs): + collection = ConductorCollection() + collection.conductors = [Conductor.convert_with_links(c, fields=fields) + for c in conductors] + collection.next = collection.get_next(limit, url=url, **kwargs) + + for conductor in collection.conductors: + conductor.sanitize(fields) + + return collection + + @classmethod + def sample(cls): + sample = cls() + conductor = Conductor.sample(expand=False) + sample.conductors = [conductor] + return sample + + +class ConductorsController(rest.RestController): + """REST controller for conductors.""" + + invalid_sort_key_list = ['alive', 'drivers'] + + def _get_conductors_collection(self, marker, limit, sort_key, sort_dir, + resource_url=None, fields=None, + detail=None): + + limit = api_utils.validate_limit(limit) + sort_dir = api_utils.validate_sort_dir(sort_dir) + + if sort_key in self.invalid_sort_key_list: + raise exception.InvalidParameterValue( + _("The sort_key value %(key)s is an invalid field for " + "sorting") % {'key': sort_key}) + + marker_obj = None + if marker: + marker_obj = objects.Conductor.get_by_hostname( + pecan.request.context, marker, online=None) + + conductors = objects.Conductor.list(pecan.request.context, limit=limit, + marker=marker_obj, + sort_key=sort_key, + sort_dir=sort_dir) + + parameters = {'sort_key': sort_key, 'sort_dir': sort_dir} + + if detail is not None: + parameters['detail'] = detail + + return ConductorCollection.convert_with_links(conductors, limit, + url=resource_url, + fields=fields, + **parameters) + + @METRICS.timer('ConductorsController.get_all') + @expose.expose(ConductorCollection, types.name, int, wtypes.text, + wtypes.text, types.listtype, types.boolean) + def get_all(self, marker=None, limit=None, sort_key='id', sort_dir='asc', + fields=None, detail=None): + """Retrieve a list of conductors. + + :param marker: pagination marker for large data sets. + :param limit: maximum number of resources to return in a single result. + This value cannot be larger than the value of max_limit + in the [api] section of the ironic configuration, or only + max_limit resources will be returned. + :param sort_key: column to sort results by. Default: id. + :param sort_dir: direction to sort. "asc" or "desc". Default: asc. + :param fields: Optional, a list with a specified set of fields + of the resource to be returned. + :param detail: Optional, boolean to indicate whether retrieve a list + of conductors with detail. + """ + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:conductor:get', cdict, cdict) + + if not api_utils.allow_expose_conductors(): + raise exception.NotFound() + + api_utils.check_allow_specify_fields(fields) + api_utils.check_allowed_fields(fields) + api_utils.check_allowed_fields([sort_key]) + + fields = api_utils.get_request_return_fields(fields, detail, + _DEFAULT_RETURN_FIELDS) + + return self._get_conductors_collection(marker, limit, sort_key, + sort_dir, fields=fields, + detail=detail) + + @METRICS.timer('ConductorsController.get_one') + @expose.expose(Conductor, types.name, types.listtype) + def get_one(self, hostname, fields=None): + """Retrieve information about the given conductor. + + :param hostname: hostname of a conductor. + :param fields: Optional, a list with a specified set of fields + of the resource to be returned. + """ + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:conductor:get', cdict, cdict) + + if not api_utils.allow_expose_conductors(): + raise exception.NotFound() + + api_utils.check_allow_specify_fields(fields) + api_utils.check_allowed_fields(fields) + + conductor = objects.Conductor.get_by_hostname(pecan.request.context, + hostname, online=None) + return Conductor.convert_with_links(conductor, fields=fields) diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 933a33122a..41c890e41e 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1072,6 +1072,9 @@ class Node(base.APIBase): protected_reason = wsme.wsattr(wtypes.text) """Indicates reason for protecting the node.""" + conductor = wsme.wsattr(wtypes.text, readonly=True) + """Represent the conductor currently serving the node""" + # NOTE(deva): "conductor_affinity" shouldn't be presented on the # API because it's an internal value. Don't add it here. @@ -1081,6 +1084,8 @@ class Node(base.APIBase): # NOTE(lucasagomes): chassis_uuid is not part of objects.Node.fields # because it's an API-only attribute. fields.append('chassis_uuid') + # NOTE(kaifeng) conductor is not part of objects.Node.fields too. + fields.append('conductor') for k in fields: # Add fields we expose. if hasattr(self, k): @@ -1149,6 +1154,18 @@ class Node(base.APIBase): def convert_with_links(cls, rpc_node, fields=None, sanitize=True): node = Node(**rpc_node.as_dict()) + if (api_utils.allow_expose_conductors() and + (fields is None or 'conductor' in fields)): + # NOTE(kaifeng) It is possible a node gets orphaned in certain + # circumstances, set conductor to None in such case. + try: + host = pecan.request.rpcapi.get_conductor_for(rpc_node) + node.conductor = host + except (exception.NoValidHost, exception.TemporaryFailure): + LOG.debug('Currently there is no conductor servicing node ' + '%(node)s.', {'node': rpc_node.uuid}) + node.conductor = None + if fields is not None: api_utils.check_for_invalid_fields(fields, node.as_dict()) @@ -1286,7 +1303,7 @@ class NodePatchType(types.JsonPatchType): '/inspection_started_at', '/clean_step', '/deploy_step', '/raid_config', '/target_raid_config', - '/fault'] + '/fault', '/conductor'] class NodeCollection(collection.Collection): @@ -1564,12 +1581,43 @@ class NodesController(rest.RestController): if subcontroller: return subcontroller(node_ident=ident), remainder[1:] + def _filter_by_conductor(self, nodes, conductor): + filtered_nodes = [] + for n in nodes: + host = pecan.request.rpcapi.get_conductor_for(n) + if host == conductor: + filtered_nodes.append(n) + return filtered_nodes + + def _create_node_filters(self, chassis_uuid=None, associated=None, + maintenance=None, provision_state=None, + driver=None, resource_class=None, fault=None, + conductor_group=None): + filters = {} + if chassis_uuid: + filters['chassis_uuid'] = chassis_uuid + if associated is not None: + filters['associated'] = associated + if maintenance is not None: + filters['maintenance'] = maintenance + if provision_state: + filters['provision_state'] = provision_state + if driver: + filters['driver'] = driver + if resource_class is not None: + filters['resource_class'] = resource_class + if fault is not None: + filters['fault'] = fault + if conductor_group is not None: + filters['conductor_group'] = conductor_group + return filters + def _get_nodes_collection(self, chassis_uuid, instance_uuid, associated, maintenance, provision_state, marker, limit, sort_key, sort_dir, driver=None, resource_class=None, resource_url=None, fields=None, fault=None, conductor_group=None, - detail=None): + detail=None, conductor=None): if self.from_chassis and not chassis_uuid: raise exception.MissingParameterValue( _("Chassis id not specified.")) @@ -1577,16 +1625,16 @@ class NodesController(rest.RestController): limit = api_utils.validate_limit(limit) sort_dir = api_utils.validate_sort_dir(sort_dir) - marker_obj = None - if marker: - marker_obj = objects.Node.get_by_uuid(pecan.request.context, - marker) - if sort_key in self.invalid_sort_key_list: raise exception.InvalidParameterValue( _("The sort_key value %(key)s is an invalid field for " "sorting") % {'key': sort_key}) + marker_obj = None + if marker: + marker_obj = objects.Node.get_by_uuid(pecan.request.context, + marker) + # The query parameters for the 'next' URL parameters = {} @@ -1602,28 +1650,18 @@ class NodesController(rest.RestController): # be generated, which we don't want. limit = 0 else: - filters = {} - if chassis_uuid: - filters['chassis_uuid'] = chassis_uuid - if associated is not None: - filters['associated'] = associated - if maintenance is not None: - filters['maintenance'] = maintenance - if provision_state: - filters['provision_state'] = provision_state - if driver: - filters['driver'] = driver - if resource_class is not None: - filters['resource_class'] = resource_class - if fault is not None: - filters['fault'] = fault - if conductor_group is not None: - filters['conductor_group'] = conductor_group - + filters = self._create_node_filters(chassis_uuid, associated, + maintenance, provision_state, + driver, resource_class, fault, + conductor_group) nodes = objects.Node.list(pecan.request.context, limit, marker_obj, sort_key=sort_key, sort_dir=sort_dir, filters=filters) + # Special filtering on results based on conductor field + if conductor: + nodes = self._filter_by_conductor(nodes, conductor) + parameters = {'sort_key': sort_key, 'sort_dir': sort_dir} if associated: parameters['associated'] = associated @@ -1726,12 +1764,12 @@ class NodesController(rest.RestController): @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean, types.boolean, wtypes.text, types.uuid, int, wtypes.text, wtypes.text, wtypes.text, types.listtype, wtypes.text, - wtypes.text, wtypes.text, types.boolean) + wtypes.text, wtypes.text, types.boolean, wtypes.text) def get_all(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=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_group=None, detail=None, conductor=None): """Retrieve a list of nodes. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -1759,6 +1797,8 @@ class NodesController(rest.RestController): that resource_class. :param conductor_group: Optional string value to get only nodes with that conductor_group. + :param conductor: Optional string value to get only nodes managed by + that conductor. :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. @@ -1774,6 +1814,7 @@ class NodesController(rest.RestController): api_utils.check_allow_specify_resource_class(resource_class) api_utils.check_allow_filter_by_fault(fault) api_utils.check_allow_filter_by_conductor_group(conductor_group) + api_utils.check_allow_filter_by_conductor(conductor) fields = api_utils.get_request_return_fields(fields, detail, _DEFAULT_RETURN_FIELDS) @@ -1786,17 +1827,19 @@ class NodesController(rest.RestController): resource_class=resource_class, fields=fields, fault=fault, conductor_group=conductor_group, - detail=detail) + detail=detail, + conductor=conductor) @METRICS.timer('NodesController.detail') @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean, types.boolean, wtypes.text, types.uuid, int, wtypes.text, wtypes.text, wtypes.text, wtypes.text, wtypes.text, - wtypes.text) + wtypes.text, wtypes.text) def detail(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, - resource_class=None, fault=None, conductor_group=None): + resource_class=None, fault=None, conductor_group=None, + conductor=None): """Retrieve a list of nodes with detail. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -1840,6 +1883,8 @@ class NodesController(rest.RestController): if parent != "nodes": raise exception.HTTPNotFound() + api_utils.check_allow_filter_by_conductor(conductor) + resource_url = '/'.join(['nodes', 'detail']) return self._get_nodes_collection(chassis_uuid, instance_uuid, associated, maintenance, @@ -1849,7 +1894,8 @@ class NodesController(rest.RestController): resource_class=resource_class, resource_url=resource_url, fault=fault, - conductor_group=conductor_group) + conductor_group=conductor_group, + conductor=conductor) @METRICS.timer('NodesController.validate') @expose.expose(wtypes.text, types.uuid_or_name, types.uuid) @@ -1913,6 +1959,10 @@ class NodesController(rest.RestController): if self.from_chassis: raise exception.OperationNotPermitted() + if node.conductor is not wtypes.Unset: + msg = _("Cannot specify conductor on node creation.") + raise exception.Invalid(msg) + reject_fields_in_newer_versions(node) if node.traits is not wtypes.Unset: diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 73c84a2469..bed014844f 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -378,6 +378,7 @@ VERSIONED_FIELDS = { 'automated_clean': versions.MINOR_47_NODE_AUTOMATED_CLEAN, 'protected': versions.MINOR_48_NODE_PROTECTED, 'protected_reason': versions.MINOR_48_NODE_PROTECTED, + 'conductor': versions.MINOR_49_CONDUCTORS, } for field in V31_FIELDS: @@ -924,3 +925,25 @@ def get_request_return_fields(fields, detail, default_fields): if fields is None and not detail: return default_fields return fields + + +def allow_expose_conductors(): + """Check if accessing conductor endpoints is allowed. + + Version 1.48 of the API exposed conductor endpoints and conductor field + for the node. + """ + return pecan.request.version.minor >= versions.MINOR_49_CONDUCTORS + + +def check_allow_filter_by_conductor(conductor): + """Check if filtering nodes by conductor is allowed. + + Version 1.48 of the API allows filtering nodes by conductor. + """ + if conductor is not None and not allow_expose_conductors(): + raise exception.NotAcceptable(_( + "Request not acceptable. The minimal required API version " + "should be %(base)s.%(opr)s") % + {'base': versions.BASE_VERSION, + 'opr': versions.MINOR_49_CONDUCTORS}) diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 7b6c61c044..826df55d17 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -136,6 +136,7 @@ MINOR_45_RESET_INTERFACES = 45 MINOR_46_NODE_CONDUCTOR_GROUP = 46 MINOR_47_NODE_AUTOMATED_CLEAN = 47 MINOR_48_NODE_PROTECTED = 48 +MINOR_49_CONDUCTORS = 49 # When adding another version, update: # - MINOR_MAX_VERSION @@ -143,7 +144,7 @@ MINOR_48_NODE_PROTECTED = 48 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_48_NODE_PROTECTED +MINOR_MAX_VERSION = MINOR_49_CONDUCTORS # 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 0d57876d4c..a01708b1e9 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -395,6 +395,15 @@ volume_policies = [ 'method': 'PATCH'}]), ] +conductor_policies = [ + policy.DocumentedRuleDefault( + 'baremetal:conductor:get', + 'rule:is_admin or rule:is_observer', + 'Retrieve Conductor records', + [{'path': '/conductors', 'method': 'GET'}, + {'path': '/conductors/{hostname}', 'method': 'GET'}]), +] + def list_policies(): policies = itertools.chain( @@ -406,7 +415,8 @@ def list_policies(): driver_policies, vendor_passthru_policies, utility_policies, - volume_policies + volume_policies, + conductor_policies ) return policies diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 87806367b3..9579c94f0e 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -131,7 +131,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.48', + 'api': '1.49', 'rpc': '1.47', 'objects': { 'Node': ['1.29', '1.28'], diff --git a/ironic/tests/unit/api/controllers/v1/test_conductor.py b/ironic/tests/unit/api/controllers/v1/test_conductor.py new file mode 100644 index 0000000000..a09a18d592 --- /dev/null +++ b/ironic/tests/unit/api/controllers/v1/test_conductor.py @@ -0,0 +1,231 @@ +# 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 /conductors/ methods. +""" + +import datetime + +import mock +from oslo_config import cfg +from oslo_utils import timeutils +from oslo_utils import uuidutils +from six.moves import http_client + +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 TestListConductors(test_api_base.BaseApiTest): + + def test_empty(self): + data = self.get_json( + '/conductors', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual([], data['conductors']) + + def test_list(self): + obj_utils.create_test_conductor(self.context, hostname='why care') + obj_utils.create_test_conductor(self.context, hostname='why not') + data = self.get_json( + '/conductors', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(2, len(data['conductors'])) + for c in data['conductors']: + self.assertIn('hostname', c) + self.assertIn('conductor_group', c) + self.assertIn('alive', c) + self.assertNotIn('drivers', c) + self.assertEqual(data['conductors'][0]['hostname'], 'why care') + self.assertEqual(data['conductors'][1]['hostname'], 'why not') + + def test_list_with_detail(self): + obj_utils.create_test_conductor(self.context, hostname='why care') + obj_utils.create_test_conductor(self.context, hostname='why not') + data = self.get_json( + '/conductors?detail=true', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(2, len(data['conductors'])) + for c in data['conductors']: + self.assertIn('hostname', c) + self.assertIn('drivers', c) + self.assertIn('conductor_group', c) + self.assertIn('alive', c) + self.assertIn('drivers', c) + self.assertEqual(data['conductors'][0]['hostname'], 'why care') + self.assertEqual(data['conductors'][1]['hostname'], 'why not') + + def test_list_with_invalid_api(self): + response = self.get_json( + '/conductors', headers={api_base.Version.string: '1.48'}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, response.status_int) + + def test_get_one(self): + obj_utils.create_test_conductor(self.context, hostname='rocky.rocks') + data = self.get_json( + '/conductors/rocky.rocks', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('hostname', data) + self.assertIn('drivers', data) + self.assertIn('conductor_group', data) + self.assertIn('alive', data) + self.assertIn('drivers', data) + self.assertEqual(data['hostname'], 'rocky.rocks') + self.assertTrue(data['alive']) + + @mock.patch.object(timeutils, 'utcnow', autospec=True) + def test_get_one_conductor_offline(self, mock_utcnow): + self.config(heartbeat_timeout=10, group='conductor') + + _time = datetime.datetime(2000, 1, 1, 0, 0) + mock_utcnow.return_value = _time + + obj_utils.create_test_conductor(self.context, hostname='rocky.rocks') + + mock_utcnow.return_value = _time + datetime.timedelta(seconds=30) + + data = self.get_json( + '/conductors/rocky.rocks', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('hostname', data) + self.assertIn('drivers', data) + self.assertIn('conductor_group', data) + self.assertIn('alive', data) + self.assertIn('drivers', data) + self.assertEqual(data['hostname'], 'rocky.rocks') + self.assertFalse(data['alive']) + + def test_get_one_with_invalid_api(self): + response = self.get_json( + '/conductors/rocky.rocks', + headers={api_base.Version.string: '1.48'}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, response.status_int) + + def test_get_one_custom_fields(self): + obj_utils.create_test_conductor(self.context, hostname='rocky.rocks') + fields = 'hostname,alive' + data = self.get_json( + '/conductors/rocky.rocks?fields=%s' % fields, + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertItemsEqual(['hostname', 'alive', 'links'], data) + + def test_get_collection_custom_fields(self): + obj_utils.create_test_conductor(self.context, hostname='rocky.rocks') + obj_utils.create_test_conductor(self.context, hostname='stein.rocks') + fields = 'hostname,alive' + + data = self.get_json( + '/conductors?fields=%s' % fields, + headers={api_base.Version.string: str(api_v1.max_version())}) + + self.assertEqual(2, len(data['conductors'])) + for c in data['conductors']: + self.assertItemsEqual(['hostname', 'alive', 'links'], c) + + def test_get_custom_fields_invalid_fields(self): + obj_utils.create_test_conductor(self.context, hostname='rocky.rocks') + fields = 'hostname,spongebob' + response = self.get_json( + '/conductors/rocky.rocks?fields=%s' % fields, + headers={api_base.Version.string: str(api_v1.max_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertIn('spongebob', response.json['error_message']) + + def _test_links(self, public_url=None): + cfg.CONF.set_override('public_endpoint', public_url, 'api') + obj_utils.create_test_conductor(self.context, hostname='rocky.rocks') + headers = {api_base.Version.string: str(api_v1.max_version())} + data = self.get_json( + '/conductors/rocky.rocks', + headers=headers) + self.assertIn('links', data) + self.assertEqual(2, len(data['links'])) + self.assertIn('rocky.rocks', data['links'][0]['href']) + for l in data['links']: + bookmark = l['rel'] == 'bookmark' + self.assertTrue(self.validate_link(l['href'], bookmark=bookmark, + headers=headers)) + + if public_url is not None: + expected = [{'href': '%s/v1/conductors/rocky.rocks' % public_url, + 'rel': 'self'}, + {'href': '%s/conductors/rocky.rocks' % public_url, + 'rel': 'bookmark'}] + for i in expected: + self.assertIn(i, data['links']) + + def test_links(self): + self._test_links() + + def test_links_public_url(self): + self._test_links(public_url='http://foo') + + def test_collection_links(self): + conductors = [] + for id in range(5): + hostname = uuidutils.generate_uuid() + conductor = obj_utils.create_test_conductor(self.context, + hostname=hostname) + conductors.append(conductor.hostname) + data = self.get_json( + '/conductors/?limit=3', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(3, len(data['conductors'])) + + next_marker = data['conductors'][-1]['hostname'] + self.assertIn(next_marker, data['next']) + + def test_collection_links_default_limit(self): + cfg.CONF.set_override('max_limit', 3, 'api') + conductors = [] + for id in range(5): + hostname = uuidutils.generate_uuid() + conductor = obj_utils.create_test_conductor(self.context, + hostname=hostname) + conductors.append(conductor.hostname) + data = self.get_json( + '/conductors', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(3, len(data['conductors'])) + + next_marker = data['conductors'][-1]['hostname'] + self.assertIn(next_marker, data['next']) + + def test_sort_key(self): + conductors = [] + for id in range(5): + hostname = uuidutils.generate_uuid() + conductor = obj_utils.create_test_conductor(self.context, + hostname=hostname) + conductors.append(conductor.hostname) + data = self.get_json( + '/conductors?sort_key=hostname', + headers={api_base.Version.string: str(api_v1.max_version())}) + hosts = [n['hostname'] for n in data['conductors']] + self.assertEqual(sorted(conductors), hosts) + + def test_sort_key_invalid(self): + invalid_keys_list = ['alive', 'drivers'] + headers = {api_base.Version.string: str(api_v1.max_version())} + for invalid_key in invalid_keys_list: + response = self.get_json('/conductors?sort_key=%s' % invalid_key, + headers=headers, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertIn(invalid_key, response.json['error_message']) diff --git a/ironic/tests/unit/api/controllers/v1/test_expose.py b/ironic/tests/unit/api/controllers/v1/test_expose.py index 3de1130ccb..84130e961f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_expose.py +++ b/ironic/tests/unit/api/controllers/v1/test_expose.py @@ -76,3 +76,6 @@ class TestExposedAPIMethodsCheckPolicy(test_base.TestCase): def test_ramdisk_api_policy(self): self._test('ironic.api.controllers.v1.ramdisk') + + def test_conductor_api_policy(self): + self._test('ironic.api.controllers.v1.conductor') diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index a9b12d24f8..16484ae6db 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -16,6 +16,7 @@ Tests for the API /nodes/ methods. import datetime import json +import fixtures import mock from oslo_config import cfg from oslo_utils import timeutils @@ -63,6 +64,10 @@ class TestListNodes(test_api_base.BaseApiTest): 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' def _create_association_test_nodes(self): # create some unassociated nodes @@ -298,6 +303,10 @@ class TestListNodes(test_api_base.BaseApiTest): self._test_node_field_hidden_in_lower_version('protected_reason', '1.47', '1.48') + def test_node_conductor_hidden_in_lower_version(self): + self._test_node_field_hidden_in_lower_version('conductor', + '1.48', '1.49') + def test_node_protected(self): for value in (True, False): node = obj_utils.create_test_node(self.context, protected=value, @@ -489,6 +498,25 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.48'}) self.assertIn('protected', response) + def test_get_conductor_field_invalid_api_version(self): + node = obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + fields = 'conductor' + response = self.get_json( + '/nodes/%s?fields=%s' % (node.uuid, fields), + headers={api_base.Version.string: '1.48'}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + + def test_get_conductor_field(self): + node = obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + fields = 'conductor' + response = self.get_json( + '/nodes/%s?fields=%s' % (node.uuid, fields), + headers={api_base.Version.string: '1.49'}) + self.assertIn('conductor', response) + def test_detail(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -1545,6 +1573,41 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) self.assertTrue(response.json['error_message']) + def test_get_nodes_by_conductor_not_allowed(self): + response = self.get_json('/nodes?conductor=rocky.rocks', + headers={api_base.Version.string: "1.47"}, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + self.assertTrue(response.json['error_message']) + + def test_get_nodes_by_conductor(self): + node1 = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + node2 = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + + response = self.get_json('/nodes?conductor=rocky.rocks', + headers={api_base.Version.string: "1.49"}) + uuids = [n['uuid'] for n in response['nodes']] + self.assertFalse(uuids) + + response = self.get_json('/nodes?conductor=fake.conductor', + headers={api_base.Version.string: "1.49"}) + uuids = [n['uuid'] for n in response['nodes']] + self.assertEqual(2, len(uuids)) + self.assertIn(node1.uuid, uuids) + self.assertIn(node2.uuid, uuids) + + self.mock_get_conductor_for.side_effect = ['rocky.rocks', + 'fake.conductor'] + response = self.get_json('/nodes?conductor=fake.conductor', + headers={api_base.Version.string: "1.49"}) + uuids = [n['uuid'] for n in response['nodes']] + self.assertEqual(1, len(uuids)) + self.assertNotIn(node1.uuid, uuids) + self.assertIn(node2.uuid, uuids) + def test_get_console_information(self): node = obj_utils.create_test_node(self.context) expected_console_info = {'test': 'test-data'} @@ -2739,6 +2802,19 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + def test_patch_conductor_forbidden(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/conductor', + 'op': 'replace', + 'value': 'why care'}], + headers={api_base.Version.string: "1.49"}, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertTrue(response.json['error_message']) + def _create_node_locally(node): driver_factory.check_and_update_node_interfaces(node) diff --git a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py index ec667634f6..466af77765 100644 --- a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py +++ b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py @@ -15,6 +15,7 @@ Tests for the API /lookup/ methods. """ +import fixtures import mock from oslo_config import cfg from oslo_utils import uuidutils @@ -43,6 +44,10 @@ class TestLookup(test_api_base.BaseApiTest): uuid=uuidutils.generate_uuid(), provision_state='available') CONF.set_override('agent_backend', 'statsd', 'metrics') + 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' def _check_config(self, data): expected_metrics = { diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 6e3b7e1c62..9be77ff8e3 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -217,6 +217,7 @@ def get_test_node(**kw): 'automated_clean': kw.get('automated_clean', None), 'protected': kw.get('protected', False), 'protected_reason': kw.get('protected_reason', None), + 'conductor': kw.get('conductor'), } for iface in drivers_base.ALL_INTERFACES: diff --git a/ironic/tests/unit/objects/utils.py b/ironic/tests/unit/objects/utils.py index 99fb6c18d9..412880b71c 100644 --- a/ironic/tests/unit/objects/utils.py +++ b/ironic/tests/unit/objects/utils.py @@ -255,6 +255,16 @@ def create_test_bios_setting(ctxt, **kw): return bios_setting +def create_test_conductor(ctxt, **kw): + """Register and return a test conductor object.""" + args = db_utils.get_test_conductor(**kw) + conductor = objects.Conductor.register(ctxt, args['hostname'], + args['drivers'], + args['conductor_group'], + update_existing=True) + return conductor + + def get_payloads_with_schemas(from_module): """Get the Payload classes with SCHEMAs defined. diff --git a/releasenotes/notes/expose-conductor-d13c9c4ef9d9de86.yaml b/releasenotes/notes/expose-conductor-d13c9c4ef9d9de86.yaml new file mode 100644 index 0000000000..f48ac985c3 --- /dev/null +++ b/releasenotes/notes/expose-conductor-d13c9c4ef9d9de86.yaml @@ -0,0 +1,18 @@ +--- +features: + - | + Adds support to retrieve the information of conductors known + by ironic: + + * a new endpoint ``GET /v1/conductors`` for listing conductor resources. + * a new endpoint ``GET /v1/conductors/{hostname}`` for showing a + conductor resource. + + Adds a read-only ``conductor`` field to the Node, which represents the + conductor currently servicing a node, and can be retrieved from following + node endpoints: + + * ``GET /v1/nodes?detail=true`` or ``GET /v1/nodes/detail`` + * ``GET /v1/nodes/`` + * ``POST /v1/nodes`` + * ``PATCH /v1/nodes/``