From 1aff17ff0f554b1892eeb0c6b24eefaee045dc05 Mon Sep 17 00:00:00 2001 From: Michael Davies Date: Thu, 12 Mar 2015 18:46:33 +1030 Subject: [PATCH] New field 'name' not supported in port REST API Add support for referring to nodes by their logical name in the port section of the REST API. As part of this, refactored some code out of node.py into utils.py, along with moving the corresponding tests. Change-Id: I5dd61582e6e009f44dd3eed2ce014d0000c94b19 Closes-Bug: #1431204 --- ironic/api/controllers/v1/node.py | 76 +++++------------- ironic/api/controllers/v1/port.py | 69 +++++++++++----- ironic/api/controllers/v1/utils.py | 46 +++++++++++ ironic/tests/api/v1/test_nodes.py | 80 +------------------ ironic/tests/api/v1/test_ports.py | 74 +++++++++++++++++ ironic/tests/api/v1/test_utils.py | 122 ++++++++++++++++++++++++++++- 6 files changed, 313 insertions(+), 154 deletions(-) diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 5549d870ed..93d87db00b 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -34,7 +34,6 @@ from ironic.api.controllers.v1 import utils as api_utils from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states as ir_states -from ironic.common import utils from ironic import objects from ironic.openstack.common import log @@ -61,7 +60,7 @@ def hide_fields_in_newer_versions(obj): if pecan.request.version.minor < 3: obj.driver_internal_info = wsme.Unset - if not allow_logical_names(): + if not api_utils.allow_node_logical_names(): obj.name = wsme.Unset # if requested version is < 1.6, hide inspection_*_at fields @@ -88,40 +87,6 @@ def check_allow_management_verbs(verb): raise exception.NotAcceptable() -def allow_logical_names(): - # v1.5 added logical name aliases - return pecan.request.version.minor >= 5 - - -def is_valid_name(name): - return utils.is_hostname_safe(name) and (not uuidutils.is_uuid_like(name)) - - -def _get_rpc_node(node_ident): - """Get the RPC node from the node uuid or logical name. - - :param node_ident: the UUID or logical name of a node. - - :returns: The RPC Node. - :raises: InvalidUuidOrName if the name or uuid provided is not valid. - :raises: NodeNotFound if the node is not found. - - """ - # Check to see if the node_ident is a valid UUID. If it is, treat it - # as a UUID. - if uuidutils.is_uuid_like(node_ident): - return objects.Node.get_by_uuid(pecan.request.context, node_ident) - - # We can refer to nodes by their name, if the client supports it - if allow_logical_names(): - if utils.is_hostname_safe(node_ident): - return objects.Node.get_by_name(pecan.request.context, node_ident) - raise exception.InvalidUuidOrName(name=node_ident) - - # Ensure we raise the same exception as we did for the Juno release - raise exception.NodeNotFound(node=node_ident) - - class NodePatchType(types.JsonPatchType): @staticmethod @@ -158,7 +123,7 @@ class BootDeviceController(rest.RestController): boot devices. """ - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) if supported: return pecan.request.rpcapi.get_supported_boot_devices( @@ -182,7 +147,7 @@ class BootDeviceController(rest.RestController): Default: False. """ - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) pecan.request.rpcapi.set_boot_device(pecan.request.context, rpc_node.uuid, @@ -248,7 +213,7 @@ class NodeConsoleController(rest.RestController): :param node_ident: UUID or logical name of a node. """ - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) try: console = pecan.request.rpcapi.get_console_information( @@ -269,7 +234,7 @@ class NodeConsoleController(rest.RestController): :param enabled: Boolean value; whether to enable or disable the console. """ - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) pecan.request.rpcapi.set_console_mode(pecan.request.context, rpc_node.uuid, enabled, topic) @@ -346,7 +311,7 @@ class NodeStatesController(rest.RestController): # NOTE(lucasagomes): All these state values come from the # DB. Ironic counts with a periodic task that verify the current # power states of the nodes and update the DB accordingly. - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) return NodeStates.convert(rpc_node) @wsme_pecan.wsexpose(None, types.uuid_or_name, wtypes.text, @@ -364,7 +329,7 @@ class NodeStatesController(rest.RestController): """ # TODO(lucasagomes): Test if it's able to transition to the # target state from the current one - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) if target not in [ir_states.POWER_ON, @@ -413,7 +378,7 @@ class NodeStatesController(rest.RestController): not allow the requested state transition. """ check_allow_management_verbs(target) - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) # Normally, we let the task manager recognize and deal with @@ -710,7 +675,7 @@ class NodeVendorPassthruController(rest.RestController): :raises: NodeNotFound if the node is not found. """ # Raise an exception if node is not found - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) if rpc_node.driver not in _VENDOR_METHODS: topic = pecan.request.rpcapi.get_topic_for(rpc_node) @@ -730,7 +695,7 @@ class NodeVendorPassthruController(rest.RestController): :param data: body of data to supply to the specified method. """ # Raise an exception if node is not found - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) # Raise an exception if method is not specified @@ -751,7 +716,7 @@ class NodeVendorPassthruController(rest.RestController): class NodeMaintenanceController(rest.RestController): def _set_maintenance(self, node_ident, maintenance_mode, reason=None): - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) rpc_node.maintenance = maintenance_mode rpc_node.maintenance_reason = reason @@ -942,10 +907,11 @@ class NodesController(rest.RestController): if node: # We're invoking this interface using positional notation, or # explicitly using 'node'. Try and determine which one. - if not allow_logical_names() and not uuidutils.is_uuid_like(node): + if (not api_utils.allow_node_logical_names() and + not uuidutils.is_uuid_like(node)): raise exception.NotAcceptable() - rpc_node = _get_rpc_node(node_uuid or node) + rpc_node = api_utils.get_rpc_node(node_uuid or node) topic = pecan.request.rpcapi.get_topic_for(rpc_node) return pecan.request.rpcapi.validate_driver_interfaces( @@ -960,7 +926,7 @@ class NodesController(rest.RestController): if self.from_chassis: raise exception.OperationNotPermitted - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) return Node.convert_with_links(rpc_node) @wsme_pecan.wsexpose(Node, body=Node, status_code=201) @@ -991,9 +957,9 @@ class NodesController(rest.RestController): # Verify that if we're creating a new node with a 'name' set # that it is a valid name if node.name: - if not allow_logical_names(): + if not api_utils.allow_node_logical_names(): raise exception.NotAcceptable() - if not is_valid_name(node.name): + if not api_utils.is_valid_node_name(node.name): msg = _("Cannot create node with invalid name %(name)s") raise wsme.exc.ClientSideError(msg % {'name': node.name}, status_code=400) @@ -1016,7 +982,7 @@ class NodesController(rest.RestController): if self.from_chassis: raise exception.OperationNotPermitted - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) # Check if node is transitioning state, although nodes in some states # can be updated. @@ -1040,9 +1006,9 @@ class NodesController(rest.RestController): # Verify that if we're patching 'name' that it is a valid name = api_utils.get_patch_value(patch, '/name') if name: - if not allow_logical_names(): + if not api_utils.allow_node_logical_names(): raise exception.NotAcceptable() - if not is_valid_name(name): + if not api_utils.is_valid_node_name(name): msg = _("Node %(node)s: Cannot change name to invalid " "name '%(name)s'") raise wsme.exc.ClientSideError(msg % {'node': node_ident, @@ -1109,7 +1075,7 @@ class NodesController(rest.RestController): if self.from_chassis: raise exception.OperationNotPermitted - rpc_node = _get_rpc_node(node_ident) + rpc_node = api_utils.get_rpc_node(node_ident) try: topic = pecan.request.rpcapi.get_topic_for(rpc_node) diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index eaf5aacfbc..c841a337d0 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -15,6 +15,7 @@ import datetime +from oslo_utils import uuidutils import pecan from pecan import rest import wsme @@ -175,12 +176,12 @@ class PortsController(rest.RestController): 'detail': ['GET'], } - def _get_ports_collection(self, node_uuid, address, marker, limit, + def _get_ports_collection(self, node_ident, address, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): - if self.from_nodes and not node_uuid: + if self.from_nodes and not node_ident: raise exception.MissingParameterValue(_( - "Node id not specified.")) + "Node identifier not specified.")) limit = api_utils.validate_limit(limit) sort_dir = api_utils.validate_sort_dir(sort_dir) @@ -190,12 +191,12 @@ class PortsController(rest.RestController): marker_obj = objects.Port.get_by_uuid(pecan.request.context, marker) - if node_uuid: + if node_ident: # FIXME(comstud): Since all we need is the node ID, we can # make this more efficient by only querying # for that column. This will get cleaned up # as we move to the object interface. - node = objects.Node.get_by_uuid(pecan.request.context, node_uuid) + node = api_utils.get_rpc_node(node_ident) ports = objects.Port.list_by_node_id(pecan.request.context, node.id, limit, marker_obj, sort_key=sort_key, @@ -227,13 +228,20 @@ class PortsController(rest.RestController): except exception.PortNotFound: return [] - @wsme_pecan.wsexpose(PortCollection, types.uuid, types.macaddress, - types.uuid, int, wtypes.text, wtypes.text) - def get_all(self, node_uuid=None, address=None, marker=None, limit=None, - sort_key='id', sort_dir='asc'): + @wsme_pecan.wsexpose(PortCollection, types.uuid_or_name, types.uuid, + types.macaddress, types.uuid, int, wtypes.text, + wtypes.text) + def get_all(self, node=None, node_uuid=None, address=None, marker=None, + limit=None, sort_key='id', sort_dir='asc'): """Retrieve a list of ports. - :param node_uuid: UUID of a node, to get only ports for that node. + Note that the 'node_uuid' interface is deprecated in favour + of the 'node' interface + + :param node: UUID or name of a node, to get only ports for that + node. + :param node_uuid: UUID of a node, to get only ports for that + node. :param address: MAC address of a port, to get the port which has this MAC address. :param marker: pagination marker for large data sets. @@ -241,16 +249,31 @@ class PortsController(rest.RestController): :param sort_key: column to sort results by. Default: id. :param sort_dir: direction to sort. "asc" or "desc". Default: asc. """ - return self._get_ports_collection(node_uuid, address, marker, limit, - sort_key, sort_dir) + if not node_uuid and node: + # We're invoking this interface using positional notation, or + # explicitly using 'node'. Try and determine which one. + # Make sure only one interface, node or node_uuid is used + if (not api_utils.allow_node_logical_names() and + not uuidutils.is_uuid_like(node)): + raise exception.NotAcceptable() - @wsme_pecan.wsexpose(PortCollection, types.uuid, types.macaddress, - types.uuid, int, wtypes.text, wtypes.text) - def detail(self, node_uuid=None, address=None, marker=None, limit=None, - sort_key='id', sort_dir='asc'): + return self._get_ports_collection(node_uuid or node, address, marker, + limit, sort_key, sort_dir) + + @wsme_pecan.wsexpose(PortCollection, types.uuid_or_name, types.uuid, + types.macaddress, types.uuid, int, wtypes.text, + wtypes.text) + def detail(self, node=None, node_uuid=None, address=None, marker=None, + limit=None, sort_key='id', sort_dir='asc'): """Retrieve a list of ports with detail. - :param node_uuid: UUID of a node, to get only ports for that node. + Note that the 'node_uuid' interface is deprecated in favour + of the 'node' interface + + :param node: UUID or name of a node, to get only ports for that + node. + :param node_uuid: UUID of a node, to get only ports for that + node. :param address: MAC address of a port, to get the port which has this MAC address. :param marker: pagination marker for large data sets. @@ -258,6 +281,14 @@ class PortsController(rest.RestController): :param sort_key: column to sort results by. Default: id. :param sort_dir: direction to sort. "asc" or "desc". Default: asc. """ + if not node_uuid and node: + # We're invoking this interface using positional notation, or + # explicitly using 'node'. Try and determine which one. + # Make sure only one interface, node or node_uuid is used + if (not api_utils.allow_node_logical_names() and + not uuidutils.is_uuid_like(node)): + raise exception.NotAcceptable() + # NOTE(lucasagomes): /detail should only work against collections parent = pecan.request.path.split('/')[:-1][-1] if parent != "ports": @@ -265,8 +296,8 @@ class PortsController(rest.RestController): expand = True resource_url = '/'.join(['ports', 'detail']) - return self._get_ports_collection(node_uuid, address, marker, limit, - sort_key, sort_dir, expand, + return self._get_ports_collection(node_uuid or node, address, marker, + limit, sort_key, sort_dir, expand, resource_url) @wsme_pecan.wsexpose(Port, types.uuid) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 0c06a6edff..6132e120f2 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -15,9 +15,15 @@ import jsonpatch from oslo_config import cfg +from oslo_utils import uuidutils +import pecan import wsme +from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import utils +from ironic import objects + CONF = cfg.CONF @@ -56,3 +62,43 @@ def get_patch_value(patch, path): for p in patch: if p['path'] == path: return p['value'] + + +def allow_node_logical_names(): + # v1.5 added logical name aliases + return pecan.request.version.minor >= 5 + + +def get_rpc_node(node_ident): + """Get the RPC node from the node uuid or logical name. + + :param node_ident: the UUID or logical name of a node. + + :returns: The RPC Node. + :raises: InvalidUuidOrName if the name or uuid provided is not valid. + :raises: NodeNotFound if the node is not found. + """ + # Check to see if the node_ident is a valid UUID. If it is, treat it + # as a UUID. + if uuidutils.is_uuid_like(node_ident): + return objects.Node.get_by_uuid(pecan.request.context, node_ident) + + # We can refer to nodes by their name, if the client supports it + if allow_node_logical_names(): + if utils.is_hostname_safe(node_ident): + return objects.Node.get_by_name(pecan.request.context, node_ident) + raise exception.InvalidUuidOrName(name=node_ident) + + # Ensure we raise the same exception as we did for the Juno release + raise exception.NodeNotFound(node=node_ident) + + +def is_valid_node_name(name): + """Determine if the provided name is a valid node name. + + Check to see that the provided node name is valid, and isn't a UUID. + + :param: name: the node name to check. + :returns: True if the name is valid, False otherwise. + """ + return utils.is_hostname_safe(name) and (not uuidutils.is_uuid_like(name)) diff --git a/ironic/tests/api/v1/test_nodes.py b/ironic/tests/api/v1/test_nodes.py index 285cfa603a..8950ec224a 100644 --- a/ironic/tests/api/v1/test_nodes.py +++ b/ironic/tests/api/v1/test_nodes.py @@ -22,7 +22,6 @@ import mock from oslo_config import cfg from oslo_utils import timeutils from oslo_utils import uuidutils -import pecan from six.moves.urllib import parse as urlparse from testtools.matchers import HasLength from wsme import types as wtypes @@ -30,6 +29,7 @@ from wsme import types as wtypes from ironic.api.controllers import base as api_base from ironic.api.controllers import v1 as api_v1 from ironic.api.controllers.v1 import node as api_node +from ironic.api.controllers.v1 import utils as api_utils from ironic.common import boot_devices from ironic.common import exception from ironic.common import states @@ -52,82 +52,6 @@ def post_get_test_node(**kw): return node -class TestTopLevelFunctions(base.TestCase): - - def setUp(self): - super(TestTopLevelFunctions, self).setUp() - self.valid_name = 'my-host' - self.valid_uuid = uuidutils.generate_uuid() - self.invalid_name = 'Mr Plow' - self.invalid_uuid = '636-555-3226-' - self.node = post_get_test_node() - - def test_is_valid_name(self): - self.assertTrue(api_node.is_valid_name(self.valid_name)) - self.assertFalse(api_node.is_valid_name(self.invalid_name)) - self.assertFalse(api_node.is_valid_name(self.valid_uuid)) - self.assertFalse(api_node.is_valid_name(self.invalid_uuid)) - - @mock.patch.object(pecan, 'request') - @mock.patch.object(api_node, 'allow_logical_names') - @mock.patch.object(objects.Node, 'get_by_uuid') - @mock.patch.object(objects.Node, 'get_by_name') - def test__get_rpc_node_expect_uuid(self, mock_gbn, mock_gbu, mock_aln, - mock_pr): - mock_aln.return_value = True - self.node['uuid'] = self.valid_uuid - mock_gbu.return_value = self.node - self.assertEqual(self.node, api_node._get_rpc_node(self.valid_uuid)) - self.assertEqual(1, mock_gbu.call_count) - self.assertEqual(0, mock_gbn.call_count) - - @mock.patch.object(pecan, 'request') - @mock.patch.object(api_v1.node, 'allow_logical_names') - @mock.patch.object(objects.Node, 'get_by_uuid') - @mock.patch.object(objects.Node, 'get_by_name') - def test__get_rpc_node_expect_name(self, mock_gbn, mock_gbu, mock_aln, - mock_pr): - mock_aln.return_value = True - self.node['name'] = self.valid_name - mock_gbn.return_value = self.node - self.assertEqual(self.node, api_node._get_rpc_node(self.valid_name)) - self.assertEqual(0, mock_gbu.call_count) - self.assertEqual(1, mock_gbn.call_count) - - @mock.patch.object(pecan, 'request') - @mock.patch.object(api_v1.node, 'allow_logical_names') - @mock.patch.object(objects.Node, 'get_by_uuid') - @mock.patch.object(objects.Node, 'get_by_name') - def test__get_rpc_node_invalid_name(self, mock_gbn, mock_gbu, - mock_aln, mock_pr): - mock_aln.return_value = True - self.assertRaises(exception.InvalidUuidOrName, - api_node._get_rpc_node, - self.invalid_name) - - @mock.patch.object(pecan, 'request') - @mock.patch.object(api_v1.node, 'allow_logical_names') - @mock.patch.object(objects.Node, 'get_by_uuid') - @mock.patch.object(objects.Node, 'get_by_name') - def test__get_rpc_node_invalid_name_2(self, mock_gbn, mock_gbu, - mock_aln, mock_pr): - mock_aln.return_value = False - self.assertRaises(exception.NodeNotFound, - api_node._get_rpc_node, - self.invalid_name) - - @mock.patch.object(pecan, 'request') - @mock.patch.object(api_v1.node, 'allow_logical_names') - @mock.patch.object(objects.Node, 'get_by_uuid') - @mock.patch.object(objects.Node, 'get_by_name') - def test__get_rpc_node_invalid_uuid(self, mock_gbn, mock_gbu, - mock_aln, mock_pr): - mock_aln.return_value = True - self.assertRaises(exception.InvalidUuidOrName, - api_node._get_rpc_node, - self.invalid_uuid) - - class TestNodeObject(base.TestCase): def test_node_init(self): @@ -1180,7 +1104,7 @@ class TestPatch(test_api_base.FunctionalTest): self.assertEqual(409, response.status_code) self.assertTrue(response.json['error_message']) - @mock.patch.object(api_node, '_get_rpc_node') + @mock.patch.object(api_utils, 'get_rpc_node') def test_patch_update_drive_console_enabled(self, mock_rpc_node): self.node.console_enabled = True mock_rpc_node.return_value = self.node diff --git a/ironic/tests/api/v1/test_ports.py b/ironic/tests/api/v1/test_ports.py index 9672dd1c92..fd5a163769 100644 --- a/ironic/tests/api/v1/test_ports.py +++ b/ironic/tests/api/v1/test_ports.py @@ -26,7 +26,9 @@ from six.moves.urllib import parse as urlparse from testtools.matchers import HasLength from wsme import types as wtypes +from ironic.api.controllers import base as api_controller from ironic.api.controllers.v1 import port as api_port +from ironic.api.controllers.v1 import utils as api_utils from ironic.common import exception from ironic.conductor import rpcapi from ironic.tests.api import base as api_base @@ -181,6 +183,78 @@ class TestListPorts(api_base.FunctionalTest): self.assertEqual('application/json', response.content_type) self.assertIn(invalid_address, response.json['error_message']) + @mock.patch.object(api_utils, 'get_rpc_node') + def test_get_all_by_node_name_ok(self, mock_get_rpc_node): + # GET /v1/ports specifying node_name - success + mock_get_rpc_node.return_value = self.node + for i in range(5): + if i < 3: + node_id = self.node.id + else: + node_id = 100000 + i + obj_utils.create_test_port(self.context, + node_id=node_id, + uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:3%s' % i) + data = self.get_json("/ports?node=%s" % 'test-node', + headers={api_controller.Version.string: '1.5'}) + self.assertEqual(3, len(data['ports'])) + + @mock.patch.object(api_utils, 'get_rpc_node') + def test_get_all_by_node_uuid_and_name(self, mock_get_rpc_node): + # GET /v1/ports specifying node and uuid - should only use node_uuid + mock_get_rpc_node.return_value = self.node + obj_utils.create_test_port(self.context, node_id=self.node.id) + self.get_json('/ports/detail?node_uuid=%s&node=%s' % + (self.node.uuid, 'node-name')) + mock_get_rpc_node.assert_called_once_with(self.node.uuid) + + @mock.patch.object(api_utils, 'get_rpc_node') + def test_get_all_by_node_name_not_supported(self, mock_get_rpc_node): + # GET /v1/ports specifying node_name - name not supported + mock_get_rpc_node.side_effect = exception.InvalidUuidOrName( + name=self.node.uuid) + for i in range(3): + obj_utils.create_test_port(self.context, + node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:3%s' % i) + data = self.get_json("/ports?node=%s" % 'test-node', + expect_errors=True) + self.assertEqual(0, mock_get_rpc_node.call_count) + self.assertEqual(406, data.status_int) + + @mock.patch.object(api_utils, 'get_rpc_node') + def test_detail_by_node_name_ok(self, mock_get_rpc_node): + # GET /v1/ports/detail specifying node_name - success + mock_get_rpc_node.return_value = self.node + port = obj_utils.create_test_port(self.context, node_id=self.node.id) + data = self.get_json('/ports/detail?node=%s' % 'test-node', + headers={api_controller.Version.string: '1.5'}) + self.assertEqual(port.uuid, data['ports'][0]['uuid']) + self.assertEqual(self.node.uuid, data['ports'][0]['node_uuid']) + + @mock.patch.object(api_utils, 'get_rpc_node') + def test_detail_by_node_name_not_supported(self, mock_get_rpc_node): + # GET /v1/ports/detail specifying node_name - name not supported + mock_get_rpc_node.side_effect = exception.InvalidUuidOrName( + name=self.node.uuid) + obj_utils.create_test_port(self.context, node_id=self.node.id) + data = self.get_json('/ports/detail?node=%s' % 'test-node', + expect_errors=True) + self.assertEqual(0, mock_get_rpc_node.call_count) + self.assertEqual(406, data.status_int) + + @mock.patch.object(api_port.PortsController, '_get_ports_collection') + def test_detail_with_incorrect_api_usage(self, mock_gpc): + # GET /v1/ports/detail specifying node and node_uuid. In this case + # we expect the node_uuid interface to be used. + self.get_json('/ports/detail?node=%s&node_uuid=%s' % + ('test-node', self.node.uuid)) + mock_gpc.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY, + mock.ANY, mock.ANY, mock.ANY, + mock.ANY, mock.ANY) + @mock.patch.object(rpcapi.ConductorAPI, 'update_port') class TestPatch(api_base.FunctionalTest): diff --git a/ironic/tests/api/v1/test_utils.py b/ironic/tests/api/v1/test_utils.py index dda776b970..c8794707b8 100644 --- a/ironic/tests/api/v1/test_utils.py +++ b/ironic/tests/api/v1/test_utils.py @@ -13,16 +13,32 @@ # License for the specific language governing permissions and limitations # under the License. +import mock +from oslo_config import cfg +from oslo_utils import uuidutils +import pecan import wsme from ironic.api.controllers.v1 import utils +from ironic.common import exception +from ironic import objects +from ironic.tests.api import utils as test_api_utils from ironic.tests import base - -from oslo_config import cfg +from ironic.tests.db import utils as dbutils CONF = cfg.CONF +# NOTE(lucasagomes): When creating a node via API (POST) +# we have to use chassis_uuid +def post_get_test_node(**kw): + node = test_api_utils.node_post_data(**kw) + chassis = dbutils.get_test_chassis() + node['chassis_id'] = None + node['chassis_uuid'] = kw.get('chassis_uuid', chassis['uuid']) + return node + + class TestApiUtils(base.TestCase): def test_validate_limit(self): @@ -47,3 +63,105 @@ class TestApiUtils(base.TestCase): self.assertRaises(wsme.exc.ClientSideError, utils.validate_sort_dir, 'fake-sort') + + +class TestNodeIdent(base.TestCase): + + def setUp(self): + super(TestNodeIdent, self).setUp() + self.valid_name = 'my-host' + self.valid_uuid = uuidutils.generate_uuid() + self.invalid_name = 'Mr Plow' + self.invalid_uuid = '636-555-3226-' + self.node = post_get_test_node() + + @mock.patch.object(pecan, 'request') + def test_allow_node_logical_names_pre_name(self, mock_pecan_req): + mock_pecan_req.version.minor = 1 + self.assertFalse(utils.allow_node_logical_names()) + + @mock.patch.object(pecan, 'request') + def test_allow_node_logical_names_post_name(self, mock_pecan_req): + mock_pecan_req.version.minor = 5 + self.assertTrue(utils.allow_node_logical_names()) + + def test_is_valid_node_name(self): + self.assertTrue(utils.is_valid_node_name(self.valid_name)) + self.assertFalse(utils.is_valid_node_name(self.invalid_name)) + self.assertFalse(utils.is_valid_node_name(self.valid_uuid)) + self.assertFalse(utils.is_valid_node_name(self.invalid_uuid)) + + @mock.patch.object(pecan, 'request') + @mock.patch.object(utils, 'allow_node_logical_names') + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(objects.Node, 'get_by_name') + def test_get_rpc_node_expect_uuid(self, mock_gbn, mock_gbu, mock_anln, + mock_pr): + mock_anln.return_value = True + self.node['uuid'] = self.valid_uuid + mock_gbu.return_value = self.node + self.assertEqual(self.node, utils.get_rpc_node(self.valid_uuid)) + self.assertEqual(1, mock_gbu.call_count) + self.assertEqual(0, mock_gbn.call_count) + + @mock.patch.object(pecan, 'request') + @mock.patch.object(utils, 'allow_node_logical_names') + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(objects.Node, 'get_by_name') + def test_get_rpc_node_expect_name(self, mock_gbn, mock_gbu, mock_anln, + mock_pr): + mock_anln.return_value = True + self.node['name'] = self.valid_name + mock_gbn.return_value = self.node + self.assertEqual(self.node, utils.get_rpc_node(self.valid_name)) + self.assertEqual(0, mock_gbu.call_count) + self.assertEqual(1, mock_gbn.call_count) + + @mock.patch.object(pecan, 'request') + @mock.patch.object(utils, 'allow_node_logical_names') + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(objects.Node, 'get_by_name') + def test_get_rpc_node_invalid_name(self, mock_gbn, mock_gbu, + mock_anln, mock_pr): + mock_anln.return_value = True + self.assertRaises(exception.InvalidUuidOrName, + utils.get_rpc_node, + self.invalid_name) + + @mock.patch.object(pecan, 'request') + @mock.patch.object(utils, 'allow_node_logical_names') + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(objects.Node, 'get_by_name') + def test_get_rpc_node_invalid_uuid(self, mock_gbn, mock_gbu, + mock_anln, mock_pr): + mock_anln.return_value = True + self.assertRaises(exception.InvalidUuidOrName, + utils.get_rpc_node, + self.invalid_uuid) + + @mock.patch.object(pecan, 'request') + @mock.patch.object(utils, 'allow_node_logical_names') + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(objects.Node, 'get_by_name') + def test_get_rpc_node_by_uuid_no_logical_name(self, mock_gbn, mock_gbu, + mock_anln, mock_pr): + # allow_node_logical_name() should have no effect + mock_anln.return_value = False + self.node['uuid'] = self.valid_uuid + mock_gbu.return_value = self.node + self.assertEqual(self.node, utils.get_rpc_node(self.valid_uuid)) + self.assertEqual(1, mock_gbu.call_count) + self.assertEqual(0, mock_gbn.call_count) + + @mock.patch.object(pecan, 'request') + @mock.patch.object(utils, 'allow_node_logical_names') + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(objects.Node, 'get_by_name') + def test_get_rpc_node_by_name_no_logical_name(self, mock_gbn, mock_gbu, + mock_anln, mock_pr): + mock_anln.return_value = False + self.node['name'] = self.valid_name + mock_gbn.return_value = self.node + self.assertRaises(exception.NodeNotFound, + utils.get_rpc_node, + self.valid_name)