diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 554da81929..99c8b78c5e 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -53,7 +53,10 @@ class Port(base.APIBase): def _set_node_uuid(self, value): if value and self._node_uuid != value: try: - node = objects.Node.get_by_uuid(pecan.request.context, value) + # FIXME(comstud): One should only allow UUID here, but + # there seems to be a bug in that tests are passing an + # ID. See bug #1301046 for more details. + node = objects.Node.get(pecan.request.context, value) self._node_uuid = node.uuid # NOTE(lucasagomes): Create the node_id attribute on-the-fly # to satisfy the api -> rpc object @@ -179,7 +182,7 @@ class PortsController(rest.RestController): # make this more efficient by only querying # for that column. This will get cleaned up # as we move to the object interface. - node = pecan.request.dbapi.get_node(node_uuid) + node = objects.Node.get_by_uuid(pecan.request.context, node_uuid) ports = pecan.request.dbapi.get_ports_by_node_id(node.id, limit, marker_obj, sort_key=sort_key, @@ -300,8 +303,8 @@ class PortsController(rest.RestController): if rpc_port[field] != getattr(port, field): rpc_port[field] = getattr(port, field) - rpc_node = objects.Node.get_by_uuid(pecan.request.context, - rpc_port.node_id) + rpc_node = objects.Node.get_by_id(pecan.request.context, + rpc_port.node_id) topic = pecan.request.rpcapi.get_topic_for(rpc_node) new_port = pecan.request.rpcapi.update_port( diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 96d504ddd7..cf4e63d220 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -57,6 +57,7 @@ from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils from ironic.db import api as dbapi +from ironic import objects from ironic.objects import base as objects_base from ironic.openstack.common import excutils from ironic.openstack.common import lockutils @@ -605,7 +606,7 @@ class ConductorManager(service.PeriodicService): try: if not self._mapped_to_this_conductor(node_uuid, driver): continue - node = self.dbapi.get_node(node_id) + node = objects.Node.get_by_id(context, node_id) if (node.provision_state == states.DEPLOYWAIT or node.maintenance or node.reservation is not None): continue diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index 69389cc050..a0e4b6984c 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -83,6 +83,7 @@ from ironic.openstack.common import excutils from ironic.common import driver_factory from ironic.common import exception from ironic.db import api as dbapi +from ironic import objects CONF = cfg.CONF @@ -164,7 +165,7 @@ class TaskManager(object): node = self.dbapi.reserve_nodes(CONF.host, [id])[0] locked_node_list.append(node.id) else: - node = self.dbapi.get_node(id) + node = objects.Node.get(context, id) ports = self.dbapi.get_ports_by_node_id(node.id) driver = driver_factory.get_driver(driver_name or node.driver) diff --git a/ironic/db/api.py b/ironic/db/api.py index bc92d21c78..90f4064764 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -141,10 +141,18 @@ class Connection(object): """ @abc.abstractmethod - def get_node(self, node_id): + def get_node_by_id(self, node_id): """Return a node. - :param node_id: The id or uuid of a node. + :param node_id: The id of a node. + :returns: A node. + """ + + @abc.abstractmethod + def get_node_by_uuid(self, node_uuid): + """Return a node. + + :param node_uuid: The uuid of a node. :returns: A node. """ diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 41ab6d0113..bc1eb25ab6 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -315,17 +315,19 @@ class Connection(api.Connection): node.save() return node - @objects.objectify(objects.Node) - def get_node(self, node_id): - query = model_query(models.Node) - query = add_identity_filter(query, node_id) - + def get_node_by_id(self, node_id): + query = model_query(models.Node).filter_by(id=node_id) try: - result = query.one() + return query.one() except NoResultFound: raise exception.NodeNotFound(node=node_id) - return result + def get_node_by_uuid(self, node_uuid): + query = model_query(models.Node).filter_by(uuid=node_uuid) + try: + return query.one() + except NoResultFound: + raise exception.NodeNotFound(node=node_uuid) @objects.objectify(objects.Node) def get_node_by_instance(self, instance): diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 4b1de79b1f..74af8c41f6 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -13,52 +13,56 @@ # License for the specific language governing permissions and limitations # under the License. +from ironic.common import exception +from ironic.common import utils from ironic.db import api as db_api from ironic.objects import base -from ironic.objects import utils +from ironic.objects import utils as obj_utils class Node(base.IronicObject): # Version 1.0: Initial version # Version 1.1: Added instance_info - VERSION = '1.1' + # Version 1.2: Add get() and get_by_id() and make get_by_uuid() + # only work with a uuid + VERSION = '1.2' dbapi = db_api.get_instance() fields = { 'id': int, - 'uuid': utils.str_or_none, - 'chassis_id': utils.int_or_none, - 'instance_uuid': utils.str_or_none, + 'uuid': obj_utils.str_or_none, + 'chassis_id': obj_utils.int_or_none, + 'instance_uuid': obj_utils.str_or_none, - 'driver': utils.str_or_none, - 'driver_info': utils.dict_or_none, + 'driver': obj_utils.str_or_none, + 'driver_info': obj_utils.dict_or_none, - 'instance_info': utils.dict_or_none, - 'properties': utils.dict_or_none, - 'reservation': utils.str_or_none, + 'instance_info': obj_utils.dict_or_none, + 'properties': obj_utils.dict_or_none, + 'reservation': obj_utils.str_or_none, # One of states.POWER_ON|POWER_OFF|NOSTATE|ERROR - 'power_state': utils.str_or_none, + 'power_state': obj_utils.str_or_none, # Set to one of states.POWER_ON|POWER_OFF when a power operation # starts, and set to NOSTATE when the operation finishes # (successfully or unsuccessfully). - 'target_power_state': utils.str_or_none, + 'target_power_state': obj_utils.str_or_none, - 'provision_state': utils.str_or_none, - 'provision_updated_at': utils.datetime_or_str_or_none, - 'target_provision_state': utils.str_or_none, + 'provision_state': obj_utils.str_or_none, + 'provision_updated_at': obj_utils.datetime_or_str_or_none, + 'target_provision_state': obj_utils.str_or_none, 'maintenance': bool, 'console_enabled': bool, # Any error from the most recent (last) asynchronous transaction # that started but failed to finish. - 'last_error': utils.str_or_none, + 'last_error': obj_utils.str_or_none, - 'extra': utils.dict_or_none, + 'extra': obj_utils.dict_or_none, } @staticmethod @@ -66,10 +70,37 @@ class Node(base.IronicObject): """Converts a database entity to a formal object.""" for field in node.fields: node[field] = db_node[field] - node.obj_reset_changes() return node + @base.remotable_classmethod + def get(cls, context, node_id): + """Find a node based on its id or uuid and return a Node object. + + :param node_id: the id *or* uuid of a node. + :returns: a :class:`Node` object. + """ + if utils.is_int_like(node_id): + return cls.get_by_id(context, node_id) + elif utils.is_uuid_like(node_id): + return cls.get_by_uuid(context, node_id) + else: + raise exception.InvalidIdentity(identity=node_id) + + @base.remotable_classmethod + def get_by_id(cls, context, node_id): + """Find a node based on its integer id and return a Node object. + + :param node_id: the id of a node. + :returns: a :class:`Node` object. + """ + db_node = cls.dbapi.get_node_by_id(node_id) + node = Node._from_db_object(cls(), db_node) + # FIXME(comstud): Setting of the context should be moved to + # _from_db_object(). + node._context = context + return node + @base.remotable_classmethod def get_by_uuid(cls, context, uuid): """Find a node based on uuid and return a Node object. @@ -77,11 +108,15 @@ class Node(base.IronicObject): :param uuid: the uuid of a node. :returns: a :class:`Node` object. """ - db_node = cls.dbapi.get_node(uuid) - return Node._from_db_object(cls(), db_node) + db_node = cls.dbapi.get_node_by_uuid(uuid) + node = Node._from_db_object(cls(), db_node) + # FIXME(comstud): Setting of the context should be moved to + # _from_db_object(). + node._context = context + return node @base.remotable - def save(self, context): + def save(self, context=None): """Save updates to this Node. Column-wise updates will be made based on the result of @@ -89,16 +124,21 @@ class Node(base.IronicObject): it will be checked against the in-database copy of the node before updates are made. - :param context: Security context + :param context: Security context. NOTE: This is only used + internally by the indirection_api. """ updates = self.obj_get_changes() self.dbapi.update_node(self.uuid, updates) - self.obj_reset_changes() @base.remotable - def refresh(self, context): - current = self.__class__.get_by_uuid(context, uuid=self.uuid) + def refresh(self, context=None): + """Refresh the object by re-fetching from the DB. + + :param context: Security context. NOTE: This is only used + internally by the indirection_api. + """ + current = self.__class__.get_by_uuid(self._context, self.uuid) for field in self.fields: if (hasattr(self, base.get_attrname(field)) and self[field] != current[field]): diff --git a/ironic/tests/api/test_acl.py b/ironic/tests/api/test_acl.py index e939e946a7..413601ff23 100644 --- a/ironic/tests/api/test_acl.py +++ b/ironic/tests/api/test_acl.py @@ -54,7 +54,7 @@ class TestACL(base.FunctionalTest): self.assertEqual(401, response.status_int) def test_authenticated(self): - with mock.patch.object(self.dbapi, 'get_node', + with mock.patch.object(self.dbapi, 'get_node_by_uuid', autospec=True) as mock_get_node: mock_get_node.return_value = self.fake_db_node diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index e126c2c7a1..1fb016f464 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -830,7 +830,7 @@ class ManagerTestCase(tests_db_base.DbTestCase): node = self.dbapi.create_node(ndict) self.service.destroy_node(self.context, node.uuid) self.assertRaises(exception.NodeNotFound, - self.dbapi.get_node, + self.dbapi.get_node_by_uuid, node.uuid) def test_destroy_node_reserved(self): @@ -1396,7 +1396,7 @@ class ManagerDoSyncPowerStateTestCase(tests_base.TestCase): @mock.patch.object(manager.ConductorManager, '_do_sync_power_state') @mock.patch.object(task_manager, 'acquire') @mock.patch.object(manager.ConductorManager, '_mapped_to_this_conductor') -@mock.patch.object(dbapi.IMPL, 'get_node') +@mock.patch.object(objects.Node, 'get_by_id') @mock.patch.object(dbapi.IMPL, 'get_nodeinfo_list') class ManagerSyncPowerStatesTestCase(tests_base.TestCase): def setUp(self): @@ -1480,7 +1480,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(acquire_mock.called) self.assertFalse(sync_mock.called) @@ -1496,7 +1496,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(acquire_mock.called) self.assertFalse(sync_mock.called) @@ -1512,7 +1512,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(acquire_mock.called) self.assertFalse(sync_mock.called) @@ -1528,7 +1528,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(acquire_mock.called) self.assertFalse(sync_mock.called) @@ -1546,7 +1546,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) acquire_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(sync_mock.called) @@ -1566,7 +1566,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) acquire_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(sync_mock.called) @@ -1585,7 +1585,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) acquire_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(sync_mock.called) @@ -1604,7 +1604,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) acquire_mock.assert_called_once_with(self.context, self.node.id) self.assertFalse(sync_mock.called) @@ -1622,7 +1622,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver) - get_node_mock.assert_called_once_with(self.node.id) + get_node_mock.assert_called_once_with(self.context, self.node.id) acquire_mock.assert_called_once_with(self.context, self.node.id) sync_mock.assert_called_once_with(task) @@ -1667,7 +1667,7 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): self._create_task(dict(id=10, maintenance=True)), self._create_task(dict(id=11))] - def _get_node_side_effect(node_id): + def _get_node_side_effect(ctxt, node_id): if node_id == 6: # Make this node disappear raise exception.NodeNotFound(node=node_id) @@ -1688,7 +1688,8 @@ class ManagerSyncPowerStatesTestCase(tests_base.TestCase): columns=self.columns, filters=self.filters) mapped_calls = [mock.call(n.uuid, n.driver) for n in nodes] self.assertEqual(mapped_calls, mapped_mock.call_args_list) - get_node_calls = [mock.call(n.id) for n in nodes[:1] + nodes[2:]] + get_node_calls = [mock.call(self.context, n.id) + for n in nodes[:1] + nodes[2:]] self.assertEqual(get_node_calls, get_node_mock.call_args_list) acquire_calls = [mock.call(self.context, n.id) diff --git a/ironic/tests/conductor/test_task_manager.py b/ironic/tests/conductor/test_task_manager.py index 41be2b1ce6..de79ef6dae 100644 --- a/ironic/tests/conductor/test_task_manager.py +++ b/ironic/tests/conductor/test_task_manager.py @@ -24,6 +24,7 @@ from ironic.common import exception from ironic.common import utils as ironic_utils from ironic.conductor import task_manager from ironic.db import api as dbapi +from ironic import objects from ironic.openstack.common import context from ironic.tests.conductor import utils as mgr_utils @@ -72,7 +73,7 @@ class TaskManagerTestCase(TaskManagerSetup): def test_task_manager_updates_db(self): node_uuid = self.uuids[0] - node = self.dbapi.get_node(node_uuid) + node = objects.Node.get_by_uuid(self.context, node_uuid) self.assertIsNone(node.reservation) with task_manager.acquire(self.context, node_uuid) as task: @@ -93,7 +94,7 @@ class TaskManagerTestCase(TaskManagerSetup): # Ensure all reservations are cleared for uuid in self.uuids: - node = self.dbapi.get_node(uuid) + node = objects.Node.get_by_uuid(self.context, uuid) self.assertIsNone(node.reservation) def test_get_nodes_nested(self): @@ -132,7 +133,7 @@ class TaskManagerTestCase(TaskManagerSetup): self.assertRaises(exception.NodeLocked, task_manager.TaskManager, self.context, node_uuid) - node = self.dbapi.get_node(node_uuid) + node = objects.Node.get_by_uuid(self.context, node_uuid) self.assertEqual('test-host', node.reservation) def test_get_many_nodes_some_already_locked(self): @@ -146,10 +147,10 @@ class TaskManagerTestCase(TaskManagerSetup): task_manager.TaskManager, self.context, self.uuids) - node = self.dbapi.get_node(locked_node_uuid) + node = objects.Node.get_by_uuid(self.context, locked_node_uuid) self.assertEqual('test-host', node.reservation) for uuid in unlocked_node_uuids: - node = self.dbapi.get_node(uuid) + node = objects.Node.get_by_uuid(self.context, uuid) self.assertIsNone(node.reservation) def test_get_one_node_driver_load_exception(self): @@ -160,7 +161,7 @@ class TaskManagerTestCase(TaskManagerSetup): driver_name='no-such-driver') # Check that db node reservation is not set. - node = self.dbapi.get_node(node_uuid) + node = objects.Node.get_by_uuid(self.context, node_uuid) self.assertIsNone(node.reservation) @@ -188,7 +189,7 @@ class ExclusiveLockDecoratorTestCase(TaskManagerSetup): do_state_change(task) for uuid in self.uuids: - res = self.dbapi.get_node(uuid) + res = objects.Node.get_by_uuid(self.context, uuid) self.assertEqual('test-state', res.power_state) @task_manager.require_exclusive_lock @@ -209,7 +210,7 @@ class ExclusiveLockDecoratorTestCase(TaskManagerSetup): self._do_state_change(task) for uuid in self.uuids: - res = self.dbapi.get_node(uuid) + res = objects.Node.get_by_uuid(self.context, uuid) self.assertEqual('test-state', res.power_state) def test_one_node_per_task_properties(self): diff --git a/ironic/tests/db/test_nodes.py b/ironic/tests/db/test_nodes.py index 6a32ffff94..239621c95b 100644 --- a/ironic/tests/db/test_nodes.py +++ b/ironic/tests/db/test_nodes.py @@ -80,22 +80,22 @@ class DbNodeTestCase(base.DbTestCase): def test_get_node_by_id(self): n = self._create_test_node() - res = self.dbapi.get_node(n['id']) + res = self.dbapi.get_node_by_id(n['id']) + self.assertEqual(n['id'], res.id) self.assertEqual(n['uuid'], res.uuid) def test_get_node_by_uuid(self): n = self._create_test_node() - res = self.dbapi.get_node(n['uuid']) + res = self.dbapi.get_node_by_uuid(n['uuid']) self.assertEqual(n['id'], res.id) + self.assertEqual(n['uuid'], res.uuid) def test_get_node_that_does_not_exist(self): self.assertRaises(exception.NodeNotFound, - self.dbapi.get_node, 99) + self.dbapi.get_node_by_id, 99) self.assertRaises(exception.NodeNotFound, - self.dbapi.get_node, + self.dbapi.get_node_by_uuid, '12345678-9999-0000-aaaa-123456789012') - self.assertRaises(exception.InvalidIdentity, - self.dbapi.get_node, 'not-a-uuid') def test_get_nodeinfo_list_defaults(self): for i in range(1, 6): @@ -269,14 +269,14 @@ class DbNodeTestCase(base.DbTestCase): self.dbapi.destroy_node(n['id']) self.assertRaises(exception.NodeNotFound, - self.dbapi.get_node, n['id']) + self.dbapi.get_node_by_id, n['id']) def test_destroy_node_by_uuid(self): n = self._create_test_node() self.dbapi.destroy_node(n['uuid']) self.assertRaises(exception.NodeNotFound, - self.dbapi.get_node, n['uuid']) + self.dbapi.get_node_by_uuid, n['uuid']) def test_destroy_node_that_does_not_exist(self): self.assertRaises(exception.NodeNotFound, @@ -363,7 +363,7 @@ class DbNodeTestCase(base.DbTestCase): self.dbapi.reserve_nodes(r1, [uuid]) # check reservation - res = self.dbapi.get_node(uuid) + res = self.dbapi.get_node_by_uuid(uuid) self.assertEqual(r1, res.reservation) def test_release_reservation(self): @@ -375,7 +375,7 @@ class DbNodeTestCase(base.DbTestCase): # release reservation self.dbapi.release_nodes(r1, [uuid]) - res = self.dbapi.get_node(uuid) + res = self.dbapi.get_node_by_uuid(uuid) self.assertIsNone(res.reservation) def test_reservation_of_reserved_node_fails(self): @@ -408,7 +408,7 @@ class DbNodeTestCase(base.DbTestCase): # another host succeeds self.dbapi.reserve_nodes(r2, [uuid]) - res = self.dbapi.get_node(uuid) + res = self.dbapi.get_node_by_uuid(uuid) self.assertEqual(r2, res.reservation) def test_reserve_many_nodes(self): @@ -418,7 +418,7 @@ class DbNodeTestCase(base.DbTestCase): self.dbapi.reserve_nodes(r1, uuids) for uuid in uuids: - res = self.dbapi.get_node(uuid) + res = self.dbapi.get_node_by_uuid(uuid) self.assertEqual(r1, res.reservation) def test_reserve_overlaping_ranges_fails(self): @@ -446,7 +446,7 @@ class DbNodeTestCase(base.DbTestCase): self.dbapi.reserve_nodes(r2, uuids[3:]) for i in range(0, len(uuids)): - res = self.dbapi.get_node(uuids[i]) + res = self.dbapi.get_node_by_uuid(uuids[i]) reservation = r1 if i < 3 else r2 self.assertEqual(reservation, res.reservation) @@ -492,5 +492,5 @@ class DbNodeTestCase(base.DbTestCase): self.dbapi.release_nodes(r2, uuids[3:]) for uuid in uuids: - res = self.dbapi.get_node(uuid) + res = self.dbapi.get_node_by_uuid(uuid) self.assertIsNone(res.reservation) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index f6a380a185..01e3c43972 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -361,7 +361,7 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase): self.assertEqual(pxe_config_template, pxe_config) # test that deploy_key saved - db_node = self.dbapi.get_node(self.node['uuid']) + db_node = self.dbapi.get_node_by_uuid(self.node['uuid']) db_key = db_node['driver_info'].get('pxe_deploy_key') self.assertEqual(fake_key, db_key) diff --git a/ironic/tests/drivers/test_ssh.py b/ironic/tests/drivers/test_ssh.py index 3e8ba847f8..56e90381ab 100644 --- a/ironic/tests/drivers/test_ssh.py +++ b/ironic/tests/drivers/test_ssh.py @@ -658,7 +658,9 @@ class SSHDriverTestCase(db_base.DbTestCase): self.get_mac_addr_patcher.stop() self.get_mac_addr_mock = None - new_node = self.dbapi.create_node(db_utils.get_test_node(id=321, + new_node = self.dbapi.create_node( + db_utils.get_test_node( + id=321, uuid='aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', driver='fake_ssh', driver_info=db_utils.get_test_ssh_info())) diff --git a/ironic/tests/objects/test_node.py b/ironic/tests/objects/test_node.py index ef2af956ba..cea5445a02 100644 --- a/ironic/tests/objects/test_node.py +++ b/ironic/tests/objects/test_node.py @@ -17,6 +17,7 @@ import datetime import mock +from ironic.common import exception from ironic.db import api as db_api from ironic.db.sqlalchemy import models from ironic import objects @@ -32,25 +33,39 @@ class TestNodeObject(base.DbTestCase): self.fake_node = utils.get_test_node() self.dbapi = db_api.get_instance() - def test_load(self): - uuid = self.fake_node['uuid'] - with mock.patch.object(self.dbapi, 'get_node', + def test_get_by_id(self): + node_id = self.fake_node['id'] + with mock.patch.object(self.dbapi, 'get_node_by_id', autospec=True) as mock_get_node: mock_get_node.return_value = self.fake_node - objects.Node.get_by_uuid(self.context, uuid) + objects.Node.get(self.context, node_id) + + mock_get_node.assert_called_once_with(node_id) + + def test_get_by_uuid(self): + uuid = self.fake_node['uuid'] + with mock.patch.object(self.dbapi, 'get_node_by_uuid', + autospec=True) as mock_get_node: + mock_get_node.return_value = self.fake_node + + objects.Node.get(self.context, uuid) mock_get_node.assert_called_once_with(uuid) + def test_get_bad_id_and_uuid(self): + self.assertRaises(exception.InvalidIdentity, + objects.Node.get, self.context, 'not-a-uuid') + def test_save(self): uuid = self.fake_node['uuid'] - with mock.patch.object(self.dbapi, 'get_node', + with mock.patch.object(self.dbapi, 'get_node_by_uuid', autospec=True) as mock_get_node: mock_get_node.return_value = self.fake_node with mock.patch.object(self.dbapi, 'update_node', autospec=True) as mock_update_node: - n = objects.Node.get_by_uuid(self.context, uuid) + n = objects.Node.get(self.context, uuid) n.properties = {"fake": "property"} n.save() @@ -63,9 +78,10 @@ class TestNodeObject(base.DbTestCase): returns = [dict(self.fake_node, properties={"fake": "first"}), dict(self.fake_node, properties={"fake": "second"})] expected = [mock.call(uuid), mock.call(uuid)] - with mock.patch.object(self.dbapi, 'get_node', side_effect=returns, + with mock.patch.object(self.dbapi, 'get_node_by_uuid', + side_effect=returns, autospec=True) as mock_get_node: - n = objects.Node.get_by_uuid(self.context, uuid) + n = objects.Node.get(self.context, uuid) self.assertEqual({"fake": "first"}, n.properties) n.refresh() self.assertEqual({"fake": "second"}, n.properties)