openstack/ironic
Code Issues Proposed changes

Add RPC methods for updating nodes.

Browse Source 
By having the API service use an RPC call to the Conductor,
we can take an exclusive task lock on the node being updated.
This will prevent updates to a node while a task is in process on it.

This is the first method implemented using RPC of a versioned
object to provide distributed locking between API and Conductor
instances.

This patch also clarifies the description of node state changes
in states.py, and does a minimal amount of reasonable checking
to ensure that nodes do not get put into inconsistent situations.

It also adds some unit tests to the API for both the new method
and for get_all, which already existed.

Change-Id: I8364ef19bfe177d56ec7bc1c0f1e166125a20ec5
This commit is contained in:
Devananda van der Veen 2013-06-22 13:15:39 -07:00
parent 724bc09f39
commit 55222ce07b
13 changed files with 398 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ironic/api/controllers/v1/base.py
View File

@ -21,7 +21,15 @@ from wsme import types as wtypes
class APIBase(wtypes.Base):
def as_dict(self):
"""Render this object as a dict of its fields."""
return dict((k, getattr(self, k))
for k in self.fields
if hasattr(self, k) and
getattr(self, k) != wsme.Unset)
def as_terse_dict(self):
"""Render this object as a dict of its non-None fields."""
return dict((k, getattr(self, k))
for k in self.fields
if hasattr(self, k) and
getattr(self, k) not in [wsme.Unset, None])

62
ironic/api/controllers/v1/node.py
View File

@ -23,6 +23,7 @@ from wsme import types as wtypes
import wsmeext.pecan as wsme_pecan
from ironic.api.controllers.v1 import base
from ironic.common import exception
from ironic import objects
from ironic.openstack.common import log
@ -95,24 +96,55 @@ class NodesController(rest.RestController):
return new_node
@wsme.validate(Node)
@wsme_pecan.wsexpose(Node, unicode, body=Node)
def put(self, uuid, delta_node):
"""Update an existing node."""
node = objects.Node.get_by_uuid(pecan.request.context, uuid)
# NOTE: delta_node will be a full API Node instance, but only user-
# supplied fields will be set, so we extract those by converting
# the object to a dict, then scanning for non-None values, and
# only applying those changes to the Node object instance.
items = delta_node.as_dict().items()
for k, v in [(k, v) for (k, v) in items if v]:
node[k] = v
@wsme_pecan.wsexpose(Node, unicode, body=Node, status=200)
def patch(self, node_id, node_data):
"""Update an existing node.
# TODO(deva): catch exceptions here if node_obj refuses to save.
node.save()
TODO(deva): add exception handling
"""
# NOTE: WSME is creating an api v1 Node object with all fields
# so we eliminate non-supplied fields by converting
# to a dict and stripping keys with value=None
delta = node_data.as_terse_dict()
return node
# NOTE: state transitions are separate from informational changes
# so don't pass a task_state to update_node.
new_state = delta.pop('task_state', None)
response = wsme.api.Response(Node(), status_code=200)
try:
node = objects.Node.get_by_uuid(
pecan.request.context, node_id)
for k in delta.keys():
node[k] = delta[k]
node = pecan.request.rpcapi.update_node(
pecan.request.context, node)
response.obj = node
except exception.InvalidParameterValue:
response.status_code = 400
except exception.NodeInWrongPowerState:
response.status_code = 409
except exception.IronicException as e:
LOG.exception(e)
response.status_code = 500
if new_state:
# NOTE: state change is async, so change the REST response
response.status_code = 202
pecan.request.rpcapi.start_state_change(pecan.request.context,
node, new_state)
# TODO(deva): return the response object instead of raising
# after wsme 0.5b3 is released
if response.status_code not in [200, 202]:
raise wsme.exc.ClientSideError(_(
"Error updating node %s") % node_id)
return response.obj
@wsme_pecan.wsexpose()
def delete(self, node_id):
"""Delete a node."""
"""Delete a node.
TODO(deva): don't allow deletion of an associated node.
"""
pecan.request.dbapi.destroy_node(node_id)

4
ironic/api/hooks.py
View File

@ -65,8 +65,8 @@ class ContextHook(hooks.PecanHook):
user_id = state.request.headers.get('X-User', user_id)
tenant = state.request.headers.get('X-Tenant-Id')
tenant = state.request.headers.get('X-Tenant', tenant)
auth_token = state.request.headers.get('X-Auth-Token')
creds = {'roles': state.request.headers.get('X-Roles').split(',')}
auth_token = state.request.headers.get('X-Auth-Token', None)
creds = {'roles': state.request.headers.get('X-Roles', '').split(',')}
is_admin = policy.check('is_admin', state.request.headers, creds)
state.request.context = context.RequestContext(

15
ironic/common/exception.py
View File

@ -262,6 +262,21 @@ class ExclusiveLockRequired(NotAuthorized):
"but the current context has a shared lock.")
class NodeInUse(IronicException):
message = _("Unable to complete the requested action because node "
"%(node)s is currently in use by another process.")
class NodeInWrongPowerState(IronicException):
message = _("Can not change instance association while node "
"%(node)s is in power state %(pstate)s.")
class NodeNotConfigured(IronicException):
message = _("Can not change power state because node %(node)s "
"is not fully configured.")
class IPMIFailure(IronicException):
message = _("IPMI command failed: %(cmd)s.")

33
ironic/common/states.py
View File

@ -17,13 +17,34 @@
# under the License.
"""
Possible baremetal node states for instances.
Mapping of bare metal node states.
Compute instance baremetal states represent the state of an instance as it
pertains to a user or administrator. When combined with task states
(task_states.py), a better picture can be formed regarding the instance's
health.
A node may have empty {} `properties` and `driver_info` in which case, it is
said to be "initialized" but "not available", and the state is NOSTATE.
When updating `properties`, any data will be rejected if the data fails to be
validated by the driver. Any node with non-empty `properties` is said to be
"initialized", and the state is INIT.
When the driver has received both `properties` and `driver_info`, it will check
the power status of the node and update the `task_state` accordingly. If the
driver fails to read the the power state from the node, it will reject the
`driver_info` change, and the state will remain as INIT. If the power status
check succeeds, `task_state` will change to one of POWER_ON or POWER_OFF,
accordingly.
At this point, the power state may be changed via the API, a console
may be started, and a tenant may be associated.
The `task_state` for a node which fails to transition will be set to ERROR.
When `instance_uuid` is set to a non-empty / non-None value, the node is said
to be "associated" with a tenant.
An associated node can not be deleted.
The `instance_uuid` field may be unset only if the node is in POWER_OFF or
ERROR states.
"""
NOSTATE = None
@ -39,5 +60,5 @@ ERROR = 'error'
POWER_ON = 'power on'
POWER_OFF = 'power off'
REBOOTING = 'rebooting'
REBOOT = 'rebooting'
SUSPEND = 'suspended'

61
ironic/conductor/manager.py
View File

@ -36,9 +36,12 @@ node; these locks are represented by the
:py:class:`ironic.conductor.task_manager.TaskManager` class.
"""
from ironic.common import exception
from ironic.common import service
from ironic.common import states
from ironic.conductor import task_manager
from ironic.db import api as dbapi
from ironic.objects import base as objects_base
from ironic.openstack.common import log
MANAGER_TOPIC = 'ironic.conductor_manager'
@ -49,10 +52,12 @@ LOG = log.getLogger(__name__)
class ConductorManager(service.PeriodicService):
"""Ironic Conductor service main class."""
RPC_API_VERSION = '1.0'
RPC_API_VERSION = '1.1'
def __init__(self, host, topic):
super(ConductorManager, self).__init__(host, topic)
serializer = objects_base.IronicObjectSerializer()
super(ConductorManager, self).__init__(host, topic,
serializer=serializer)
def start(self):
super(ConductorManager, self).start()
@ -79,4 +84,54 @@ class ConductorManager(service.PeriodicService):
state = driver.power.get_power_state(task, node)
return state
# TODO(deva)
def update_node(self, context, node_obj):
"""Update a node with the supplied data.
This method is the main "hub" for PUT and PATCH requests in the API.
It ensures that the requested change is safe to perform,
validates the parameters with the node's driver, if necessary.
:param context: an admin context
:param node_obj: a changed (but not saved) node object.
"""
node_id = node_obj.get('uuid')
LOG.debug("RPC update_node called for node %s." % node_id)
delta = node_obj.obj_what_changed()
if 'task_state' in delta:
raise exception.IronicException(_(
"Invalid method call: update_node can not change node state."))
with task_manager.acquire(node_id, shared=False) as task:
# NOTE(deva): invalid driver parameters should not be saved
# don't proceed if these checks fail
if 'driver_info' in delta:
task.driver.deploy.validate(node_obj)
task.driver.power.validate(node_obj)
node_obj['task_state'] = task.driver.power.get_power_state
# TODO(deva): Determine what value will be passed by API when
# instance_uuid needs to be unset, and handle it.
if 'instance_uuid' in delta:
if node_obj['task_state'] != states.POWER_OFF:
raise exception.NodeInWrongPowerState(
node=node_id,
pstate=node_obj['task_state'])
# update any remaining parameters, then save
node_obj.save(context)
return node_obj
def start_state_change(self, context, node_obj, new_state):
"""RPC method to encapsulate changes to a node's state.
Perform actions such as power on, power off, deploy, and cleanup.
TODO
:param context: an admin context
:param node_obj: an RPC-style node object
:param new_state: the desired state of the node
"""
pass

40
ironic/conductor/rpcapi.py
View File

@ -19,6 +19,7 @@
Client side of the conductor RPC API.
"""
from ironic.objects import base as objects_base
import ironic.openstack.common.rpc.proxy
MANAGER_TOPIC = 'ironic.conductor_manager'
@ -30,9 +31,11 @@ class ConductorAPI(ironic.openstack.common.rpc.proxy.RpcProxy):
API version history:
1.0 - Initial version.
Included get_node_power_status
1.1 - Added update_node and start_state_change.
"""
RPC_API_VERSION = '1.0'
RPC_API_VERSION = '1.1'
def __init__(self, topic=None):
if topic is None:
@ -40,10 +43,11 @@ class ConductorAPI(ironic.openstack.common.rpc.proxy.RpcProxy):
super(ConductorAPI, self).__init__(
topic=topic,
serializer=objects_base.IronicObjectSerializer(),
default_version=self.RPC_API_VERSION)
def get_node_power_state(self, context, node_id):
"""Ask a manager for the node power state.
"""Ask a conductor for the node power state.
:param context: request context.
:param node_id: node id or uuid.
@ -52,3 +56,35 @@ class ConductorAPI(ironic.openstack.common.rpc.proxy.RpcProxy):
return self.call(context,
self.make_msg('get_node_power_state',
node_id=node_id))
def update_node(self, context, node_obj):
"""Synchronously, have a conductor update the node's information.
Update the node's information in the database and return a node object.
The conductor will lock the node while it validates the supplied
information. If driver_info is passed, it will be validated by
the core drivers. If instance_uuid is passed, it will be set or unset
only if the node is properly configured.
Note that task_state should not be passed via this method.
Use start_state_change for initiating driver actions.
:param context: request context.
:param node_obj: a changed (but not saved) node object.
:returns: updated node object, including all fields.
"""
return self.call(context,
self.make_msg('update_node',
node_obj=node_obj))
def start_state_change(self, context, node_obj, new_state):
"""Asynchronously perform an action on a node.
:param context: request context.
:param node_obj: an RPC_style node object.
:param new_state: one of ironic.common.states power state values
"""
self.cast(context,
self.make_msg('start_state_change',
node_obj=node_obj,
new_state=new_state))

9
ironic/tests/api/base.py
View File

@ -24,6 +24,7 @@ import pecan
import pecan.testing
from ironic.api import acl
from ironic.db import api as dbapi
from ironic.tests.db import base
@ -43,6 +44,7 @@ class FunctionalTest(base.DbTestCase):
cfg.CONF.set_override("policy_file",
self.path_get('tests/policy.json'))
self.app = self._make_app()
self.dbapi = dbapi.get_instance()
def _make_app(self, enable_acl=False):
# Determine where we are so we can set up paths in the config
@ -64,6 +66,13 @@ class FunctionalTest(base.DbTestCase):
super(FunctionalTest, self).tearDown()
pecan.set_config({}, overwrite=True)
def patch_json(self, path, params, expect_errors=False, headers=None,
extra_environ=None, status=None):
return self.post_json(path=path, params=params,
expect_errors=expect_errors,
headers=headers, extra_environ=extra_environ,
status=status, method="patch")
def put_json(self, path, params, expect_errors=False, headers=None,
extra_environ=None, status=None):
return self.post_json(path=path, params=params,

24
ironic/tests/api/test_list_nodes.py
View File

@ -1,24 +0,0 @@
# vim: tabstop=4 shiftwidth=4 softtabstop=4
# Copyright 2013 Hewlett-Packard Development Company, L.P.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from ironic.tests.api import base
class TestListNodes(base.FunctionalTest):
def test_empty(self):
data = self.get_json('/nodes', headers={'X-Roles': 'admin'})
self.assertEqual([], data)

112
ironic/tests/api/test_nodes.py Normal file
View File

@ -0,0 +1,112 @@
# vim: tabstop=4 shiftwidth=4 softtabstop=4
# Copyright 2013 Hewlett-Packard Development Company, L.P.
# All Rights Reserved.
#
# 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 mox
from ironic.common import context
from ironic.common import exception
from ironic.conductor import rpcapi
from ironic.tests.api import base
from ironic.tests.db import utils as dbutils
class TestListNodes(base.FunctionalTest):
def test_empty(self):
data = self.get_json('/nodes')
self.assertEqual([], data)
def test_one(self):
ndict = dbutils.get_test_node()
node = self.dbapi.create_node(ndict)
data = self.get_json('/nodes')
self.assertEqual([node['uuid']], data)
def test_many(self):
nodes = []
for id in xrange(5):
ndict = dbutils.get_test_node(id=id)
node = self.dbapi.create_node(ndict)
nodes.append(node['uuid'])
data = self.get_json('/nodes')
self.assertEqual(nodes.sort(), data.sort())
class TestPatch(base.FunctionalTest):
def setUp(self):
super(TestPatch, self).setUp()
ndict = dbutils.get_test_node()
self.context = context.get_admin_context()
self.node = self.dbapi.create_node(ndict)
self.mox.StubOutWithMock(rpcapi.ConductorAPI, 'update_node')
self.mox.StubOutWithMock(rpcapi.ConductorAPI, 'start_state_change')
def test_update_ok(self):
rpcapi.ConductorAPI.update_node(mox.IgnoreArg(), mox.IgnoreArg()).\
AndReturn(self.node)
self.mox.ReplayAll()
response = self.patch_json('/nodes/%s' % self.node['uuid'],
{'instance_uuid': 'fake instance uuid'})
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, 200)
self.mox.VerifyAll()
def test_update_state(self):
rpcapi.ConductorAPI.update_node(mox.IgnoreArg(), mox.IgnoreArg()).\
AndReturn(self.node)
rpcapi.ConductorAPI.start_state_change(mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg())
self.mox.ReplayAll()
response = self.patch_json('/nodes/%s' % self.node['uuid'],
{'task_state': 'new state'})
self.assertEqual(response.content_type, 'application/json')
# TODO(deva): change to 202 when wsme 0.5b3 is released
self.assertEqual(response.status_code, 200)
self.mox.VerifyAll()
def test_update_fails_bad_driver_info(self):
fake_err = 'Fake Error Message'
rpcapi.ConductorAPI.update_node(mox.IgnoreArg(), mox.IgnoreArg()).\
AndRaise(exception.InvalidParameterValue(fake_err))
self.mox.ReplayAll()
response = self.patch_json('/nodes/%s' % self.node['uuid'],
{'driver_info': {'this': 'foo', 'that': 'bar'}},
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, 400)
self.mox.VerifyAll()
def test_update_fails_bad_state(self):
fake_err = 'Fake Power State'
rpcapi.ConductorAPI.update_node(mox.IgnoreArg(), mox.IgnoreArg()).\
AndRaise(exception.NodeInWrongPowerState(
node=self.node['uuid'], pstate=fake_err))
self.mox.ReplayAll()
response = self.patch_json('/nodes/%s' % self.node['uuid'],
{'instance_uuid': 'fake instance uuid'},
expect_errors=True)
print "======================"
print response
self.assertEqual(response.content_type, 'application/json')
# TODO(deva): change to 409 when wsme 0.5b3 released
self.assertEqual(response.status_code, 400)
self.mox.VerifyAll()

64
ironic/tests/conductor/test_manager.py
View File

@ -20,9 +20,12 @@
import mox
from ironic.common import exception
from ironic.common import states
from ironic.conductor import manager
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
from ironic.tests.db import base
@ -65,3 +68,64 @@ class ManagerTestCase(base.DbTestCase):
self.assertEqual(state, states.POWER_ON)
self.mox.VerifyAll()
def test_update_node(self):
ndict = utils.get_test_node(driver='fake', extra={'test': 'one'})
node = self.dbapi.create_node(ndict)
# check that ManagerService.update_node actually updates the node
node['extra'] = {'test': 'two'}
res = self.service.update_node(self.context, node)
self.assertEqual(res['extra'], {'test': 'two'})
def test_update_node_already_locked(self):
ndict = utils.get_test_node(driver='fake', extra={'test': 'one'})
node = self.dbapi.create_node(ndict)
# check that it fails if something else has locked it already
with task_manager.acquire(node['id'], shared=False):
node['extra'] = {'test': 'two'}
self.assertRaises(exception.NodeLocked,
self.service.update_node,
self.context,
node)
# verify change did not happen
res = objects.Node.get_by_uuid(self.context, node['uuid'])
self.assertEqual(res['extra'], {'test': 'one'})
def test_update_node_invalid_state(self):
ndict = utils.get_test_node(driver='fake', extra={'test': 'one'},
instance_uuid=None, task_state=states.POWER_ON)
node = self.dbapi.create_node(ndict)
# check that it fails because state is POWER_ON
node['instance_uuid'] = 'fake-uuid'
self.assertRaises(exception.NodeInWrongPowerState,
self.service.update_node,
self.context,
node)
# verify change did not happen
res = objects.Node.get_by_uuid(self.context, node['uuid'])
self.assertEqual(res['instance_uuid'], None)
def test_update_node_invalid_driver_info(self):
# TODO(deva)
pass
def test_update_node_get_power_state_failure(self):
# TODO(deva)
pass
def test_udpate_node_set_driver_info_and_power_state(self):
# TODO(deva)
pass
def test_update_node_associate_instance(self):
# TODO(deva)
pass
def test_update_node_unassociate_instance(self):
# TODO(deva)
pass

20
ironic/tests/conductor/test_rpcapi.py
View File

@ -21,8 +21,10 @@ Unit Tests for :py:class:`ironic.conductor.rpcapi.ConductorAPI`.
from oslo.config import cfg
from ironic.common import states
from ironic.conductor import rpcapi as conductor_rpcapi
from ironic.db import api as dbapi
from ironic import objects
from ironic.openstack.common import context
from ironic.openstack.common import jsonutils as json
from ironic.openstack.common import rpc
@ -38,9 +40,10 @@ class RPCAPITestCase(base.DbTestCase):
super(RPCAPITestCase, self).setUp()
self.context = context.get_admin_context()
self.dbapi = dbapi.get_instance()
self.fake_node = json.to_primitive(dbutils.get_test_node(
control_driver='fake',
deploy_driver='fake'))
self.fake_node = json.to_primitive(dbutils.get_test_node())
self.fake_node_obj = objects.Node._from_db_object(
objects.Node(),
self.fake_node)
def test_serialized_instance_has_uuid(self):
self.assertTrue('uuid' in self.fake_node)
@ -81,3 +84,14 @@ class RPCAPITestCase(base.DbTestCase):
self._test_rpcapi('get_node_power_state',
'call',
node_id=123)
def test_update_node(self):
self._test_rpcapi('update_node',
'call',
node_obj=self.fake_node)
def test_start_state_change(self):
self._test_rpcapi('start_state_change',
'cast',
node_obj=self.fake_node,
new_state=states.POWER_ON)

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

@ -32,9 +32,7 @@ from ironic.tests.db import utils
def create_fake_node(i):
dbh = dbapi.get_instance()
node = utils.get_test_node(id=i,
uuid=uuidutils.generate_uuid(),
control_driver='fake',
deploy_driver='fake')
uuid=uuidutils.generate_uuid())
dbh.create_node(node)
return node['uuid']