openstack
/
ironic
Code Issues Proposed changes

Add reserve() and release() to Node object 

What it says. Also make the code and tests use them vs direct calls
to dbapi.

Change-Id: I0158be53ff683ec3569007bc6f0cee6395241062
Partial-Bug: #1314732
This commit is contained in:
Lucas Alvares Gomes 2014-09-10 17:53:45 +01:00
parent 526bb53528
commit f575ae7ea3
5 changed files with 105 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ironic/conductor/task_manager.py
View File

@ -93,7 +93,6 @@ from oslo.utils import excutils
from ironic.common import driver_factory
from ironic.common import exception
from ironic.common.i18n import _LW
from ironic.db import api as dbapi
from ironic import objects
from ironic.openstack.common import log as logging
@ -161,7 +160,6 @@ class TaskManager(object):
"""
self._dbapi = dbapi.get_instance()
self._spawn_method = None
self._on_error_method = None
@ -179,7 +177,7 @@ class TaskManager(object):
def reserve_node():
LOG.debug("Attempting to reserve node %(node)s",
{'node': node_id})
self.node = self._dbapi.reserve_node(CONF.host, node_id)
self.node = objects.Node.reserve(context, CONF.host, node_id)
try:
if not self.shared:
@ -226,7 +224,7 @@ class TaskManager(object):
if not self.shared:
try:
if self.node:
self._dbapi.release_node(CONF.host, self.node.id)
objects.Node.release(self.context, CONF.host, self.node.id)
except exception.NodeNotFound:
# squelch the exception if the node was deleted
# within the task's context.

2
ironic/db/sqlalchemy/api.py
View File

@ -32,7 +32,6 @@ from ironic.common import states
from ironic.common import utils
from ironic.db import api
from ironic.db.sqlalchemy import models
from ironic import objects
from ironic.openstack.common import log
CONF = cfg.CONF
@ -213,7 +212,6 @@ class Connection(api.Connection):
return _paginate_query(models.Node, limit, marker,
sort_key, sort_dir, query)
@objects.objectify(objects.Node)
def reserve_node(self, tag, node_id):
session = get_session()
with session.begin():

36
ironic/objects/node.py
View File

@ -28,7 +28,8 @@ class Node(base.IronicObject):
# Version 1.3: Add create() and destroy()
# Version 1.4: Add get_by_instance_uuid()
# Version 1.5: Add list()
VERSION = '1.5'
# Version 1.6: Add reserve() and release()
VERSION = '1.6'
dbapi = db_api.get_instance()
@ -158,6 +159,39 @@ class Node(base.IronicObject):
node_list.append(node)
return node_list
@base.remotable_classmethod
def reserve(cls, context, tag, node_id):
"""Get and reserve a node.
To prevent other ManagerServices from manipulating the given
Node while a Task is performed, mark it reserved by this host.
:param context: Security context.
:param tag: A string uniquely identifying the reservation holder.
:param node_id: A node id or uuid.
:raises: NodeNotFound if the node is not found.
:returns: a :class:`Node` object.
"""
db_node = cls.dbapi.reserve_node(tag, 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 release(cls, context, tag, node_id):
"""Release the reservation on a node.
:param context: Security context.
:param tag: A string uniquely identifying the reservation holder.
:param node_id: A node id or uuid.
:raises: NodeNotFound if the node is not found.
"""
cls.dbapi.release_node(tag, node_id)
@base.remotable
def create(self, context=None):
"""Create a Node record in the DB.

51
ironic/tests/conductor/test_task_manager.py
View File

@ -23,15 +23,16 @@ import mock
from ironic.common import driver_factory
from ironic.common import exception
from ironic.common import utils
from ironic.conductor import task_manager
from ironic.db import api as dbapi
from ironic import objects
from ironic.tests import base as tests_base
from ironic.tests.objects import utils as obj_utils
@mock.patch.object(objects.Node, 'get')
@mock.patch.object(dbapi.IMPL, 'release_node')
@mock.patch.object(dbapi.IMPL, 'reserve_node')
@mock.patch.object(objects.Node, 'release')
@mock.patch.object(objects.Node, 'reserve')
@mock.patch.object(driver_factory, 'get_driver')
@mock.patch.object(objects.Port, 'list_by_node_id')
class TaskManagerTestCase(tests_base.TestCase):
@ -42,7 +43,7 @@ class TaskManagerTestCase(tests_base.TestCase):
self.config(node_locked_retry_attempts=1, group='conductor')
self.config(node_locked_retry_interval=0, group='conductor')
self.context = mock.sentinel.context
self.node = mock.Mock(spec_set=objects.Node)
self.node = obj_utils.create_test_node(self.context)
def test_excl_lock(self, get_ports_mock, get_driver_mock,
reserve_mock, release_mock, node_get_mock):
@ -54,10 +55,12 @@ class TaskManagerTestCase(tests_base.TestCase):
self.assertEqual(get_driver_mock.return_value, task.driver)
self.assertFalse(task.shared)
reserve_mock.assert_called_once_with(self.host, 'fake-node-id')
reserve_mock.assert_called_once_with(self.context, self.host,
'fake-node-id')
get_ports_mock.assert_called_once_with(self.context, self.node.id)
get_driver_mock.assert_called_once_with(self.node.driver)
release_mock.assert_called_once_with(self.host, self.node.id)
release_mock.assert_called_once_with(self.context, self.host,
self.node.id)
self.assertFalse(node_get_mock.called)
def test_excl_lock_with_driver(self, get_ports_mock, get_driver_mock,
@ -72,16 +75,21 @@ class TaskManagerTestCase(tests_base.TestCase):
self.assertEqual(get_driver_mock.return_value, task.driver)
self.assertFalse(task.shared)
reserve_mock.assert_called_once_with(self.host, 'fake-node-id')
reserve_mock.assert_called_once_with(self.context, self.host,
'fake-node-id')
get_ports_mock.assert_called_once_with(self.context, self.node.id)
get_driver_mock.assert_called_once_with('fake-driver')
release_mock.assert_called_once_with(self.host, self.node.id)
release_mock.assert_called_once_with(self.context, self.host,
self.node.id)
self.assertFalse(node_get_mock.called)
def test_excl_nested_acquire(self, get_ports_mock, get_driver_mock,
reserve_mock, release_mock,
node_get_mock):
node2 = mock.Mock(spec_set=objects.Node)
node2 = obj_utils.create_test_node(self.context,
id=2,
uuid=utils.generate_uuid(),
driver='fake')
reserve_mock.return_value = self.node
get_ports_mock.return_value = mock.sentinel.ports1
@ -103,8 +111,8 @@ class TaskManagerTestCase(tests_base.TestCase):
self.assertEqual(mock.sentinel.driver2, task2.driver)
self.assertFalse(task2.shared)
self.assertEqual([mock.call(self.host, 'node-id1'),
mock.call(self.host, 'node-id2')],
self.assertEqual([mock.call(self.context, self.host, 'node-id1'),
mock.call(self.context, self.host, 'node-id2')],
reserve_mock.call_args_list)
self.assertEqual([mock.call(self.context, self.node.id),
mock.call(self.context, node2.id)],
@ -113,8 +121,8 @@ class TaskManagerTestCase(tests_base.TestCase):
mock.call(node2.driver)],
get_driver_mock.call_args_list)
# release should be in reverse order
self.assertEqual([mock.call(self.host, node2.id),
mock.call(self.host, self.node.id)],
self.assertEqual([mock.call(self.context, self.host, node2.id),
mock.call(self.context, self.host, self.node.id)],
release_mock.call_args_list)
self.assertFalse(node_get_mock.called)
@ -133,7 +141,7 @@ class TaskManagerTestCase(tests_base.TestCase):
with task_manager.TaskManager(self.context, 'fake-node-id') as task:
self.assertFalse(task.shared)
reserve_mock.assert_called(self.host, 'fake-node-id')
reserve_mock.assert_called(self.context, self.host, 'fake-node-id')
self.assertEqual(2, reserve_mock.call_count)
def test_excl_lock_reserve_exception(self, get_ports_mock,
@ -150,7 +158,8 @@ class TaskManagerTestCase(tests_base.TestCase):
self.context,
'fake-node-id')
reserve_mock.assert_called_with(self.host, 'fake-node-id')
reserve_mock.assert_called_with(self.context, self.host,
'fake-node-id')
self.assertEqual(retry_attempts, reserve_mock.call_count)
self.assertFalse(get_ports_mock.called)
self.assertFalse(get_driver_mock.called)
@ -168,10 +177,12 @@ class TaskManagerTestCase(tests_base.TestCase):
self.context,
'fake-node-id')
reserve_mock.assert_called_once_with(self.host, 'fake-node-id')
reserve_mock.assert_called_once_with(self.context, self.host,
'fake-node-id')
get_ports_mock.assert_called_once_with(self.context, self.node.id)
self.assertFalse(get_driver_mock.called)
release_mock.assert_called_once_with(self.host, self.node.id)
release_mock.assert_called_once_with(self.context, self.host,
self.node.id)
self.assertFalse(node_get_mock.called)
def test_excl_lock_get_driver_exception(self, get_ports_mock,
@ -186,10 +197,12 @@ class TaskManagerTestCase(tests_base.TestCase):
self.context,
'fake-node-id')
reserve_mock.assert_called_once_with(self.host, 'fake-node-id')
reserve_mock.assert_called_once_with(self.context, self.host,
'fake-node-id')
get_ports_mock.assert_called_once_with(self.context, self.node.id)
get_driver_mock.assert_called_once_with(self.node.driver)
release_mock.assert_called_once_with(self.host, self.node.id)
release_mock.assert_called_once_with(self.context, self.host,
self.node.id)
self.assertFalse(node_get_mock.called)
def test_shared_lock(self, get_ports_mock, get_driver_mock,

36
ironic/tests/objects/test_node.py
View File

@ -142,3 +142,39 @@ class TestNodeObject(base.DbTestCase):
nodes = objects.Node.list(self.context)
self.assertThat(nodes, HasLength(1))
self.assertIsInstance(nodes[0], objects.Node)
def test_reserve(self):
with mock.patch.object(self.dbapi, 'reserve_node',
autospec=True) as mock_reserve:
mock_reserve.return_value = self.fake_node
node_id = self.fake_node['id']
fake_tag = 'fake-tag'
node = objects.Node.reserve(self.context, fake_tag, node_id)
self.assertIsInstance(node, objects.Node)
mock_reserve.assert_called_once_with(fake_tag, node_id)
def test_reserve_node_not_found(self):
with mock.patch.object(self.dbapi, 'reserve_node',
autospec=True) as mock_reserve:
node_id = 'non-existent'
mock_reserve.side_effect = exception.NodeNotFound(node=node_id)
self.assertRaises(exception.NodeNotFound,
objects.Node.reserve, self.context, 'fake-tag',
node_id)
def test_release(self):
with mock.patch.object(self.dbapi, 'release_node',
autospec=True) as mock_release:
node_id = self.fake_node['id']
fake_tag = 'fake-tag'
objects.Node.release(self.context, fake_tag, node_id)
mock_release.assert_called_once_with(fake_tag, node_id)
def test_release_node_not_found(self):
with mock.patch.object(self.dbapi, 'release_node',
autospec=True) as mock_release:
node_id = 'non-existent'
mock_release.side_effect = exception.NodeNotFound(node=node_id)
self.assertRaises(exception.NodeNotFound,
objects.Node.release, self.context,
'fake-tag', node_id)