From 66e1a8b47544e225721f0ccd3a33580dde337793 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 28 Nov 2012 15:43:37 -0500 Subject: Convert datetimes for conductor instance_update. This patch ensures that jsonutils.to_primitive() is used on the updates being sent to conductor's instance_update method. This is primarily so that we convert datetimes to a string using a very specific format defined in the timeutils module. Then, in the conductor manager, convert these fields back into datetime objects using timeutils. The conversion to a string was already happening implicitly if you were using the kombu rpc driver. The reason is that kombu uses anyjson, which we hook in to and will automatically use our own to_primitive() to fix serialization problems for kombu. Doing it explicitly is better, and is required for the other rpc drivers to work. This is to address a problem with sqlite where it would raise an exception when we try to do an update to a datetime field with a string. Fix bug 1084240. Change-Id: I2d703a92df1e85f0f3a7a793b301fe16cbd81ff5 --- nova/conductor/manager.py | 11 ++++++++++- nova/conductor/rpcapi.py | 3 ++- nova/tests/conductor/test_conductor.py | 6 +++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 4cb6672f4..aa732b1d3 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -18,10 +18,13 @@ from nova import manager from nova import notifications from nova.openstack.common import jsonutils from nova.openstack.common import log as logging +from nova.openstack.common import timeutils LOG = logging.getLogger(__name__) +# Instead of having a huge list of arguments to instance_update(), we just +# accept a dict of fields to update and use this whitelist to validate it. allowed_updates = ['task_state', 'vm_state', 'expected_task_state', 'power_state', 'access_ip_v4', 'access_ip_v6', 'launched_at', 'terminated_at', 'host', 'node', @@ -31,6 +34,9 @@ allowed_updates = ['task_state', 'vm_state', 'expected_task_state', 'default_swap_device', 'root_device_name', ] +# Fields that we want to convert back into a datetime object. +datetime_fields = ['launched_at', 'terminated_at'] + class ConductorManager(manager.SchedulerDependentManager): """Mission: TBD""" @@ -42,11 +48,14 @@ class ConductorManager(manager.SchedulerDependentManager): *args, **kwargs) def instance_update(self, context, instance_uuid, updates): - for key in updates: + for key, value in updates.iteritems(): if key not in allowed_updates: LOG.error(_("Instance update attempted for " "'%(key)s' on %(instance_uuid)s") % locals()) raise KeyError("unexpected update keyword '%s'" % key) + if key in datetime_fields and isinstance(value, basestring): + updates[key] = timeutils.parse_strtime(value) + old_ref, instance_ref = self.db.instance_update_and_get_original( context, instance_uuid, updates) notifications.send_update(context, old_ref, instance_ref) diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index 15750da76..f73547925 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -38,10 +38,11 @@ class ConductorAPI(nova.openstack.common.rpc.proxy.RpcProxy): default_version=self.BASE_RPC_API_VERSION) def instance_update(self, context, instance_uuid, updates): + updates_p = jsonutils.to_primitive(updates) return self.call(context, self.make_msg('instance_update', instance_uuid=instance_uuid, - updates=updates)) + updates=updates_p)) def migration_update(self, context, migration, status): migration_p = jsonutils.to_primitive(migration) diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py index d3c555822..3dcfe0209 100644 --- a/nova/tests/conductor/test_conductor.py +++ b/nova/tests/conductor/test_conductor.py @@ -25,6 +25,7 @@ from nova import db from nova.db.sqlalchemy import models from nova import notifications from nova.openstack.common import jsonutils +from nova.openstack.common import timeutils from nova import test @@ -156,7 +157,10 @@ class ConductorPolicyTest(test.TestCase): conductor = conductor_api.LocalAPI() updates = {} for key in conductor_manager.allowed_updates: - updates[key] = 'foo' + if key in conductor_manager.datetime_fields: + updates[key] = timeutils.utcnow() + else: + updates[key] = 'foo' conductor.instance_update(ctxt, 'fake-instance', **updates) def test_allowed_keys_are_real(self): -- cgit