Remove compute/api.py::update()

This removes the compute/api::update() method which used to support old-style
instance dict updates. The only users of this method were broken unit tests.

Related to blueprint kilo-objects

Change-Id: I53cb012665f9eaea628af2d25a4fc137dcd2313b
This commit is contained in:
Dan Smith 2014-12-02 12:23:10 -08:00
parent de952edb92
commit 01d28bcadd
6 changed files with 31 additions and 93 deletions

View File

@ -1480,31 +1480,6 @@ class API(base.Base):
shutdown_terminate=shutdown_terminate, shutdown_terminate=shutdown_terminate,
check_server_group_quota=check_server_group_quota) check_server_group_quota=check_server_group_quota)
@wrap_check_policy
def update(self, context, instance, **kwargs):
"""Updates the instance in the datastore.
:param context: The security context
:param instance: The instance to update
:param kwargs: All additional keyword args are treated
as data fields of the instance to be
updated
:returns: A reference to the updated instance
"""
refs = self._update(context, instance, **kwargs)
return refs[1]
def _update(self, context, instance, **kwargs):
# Update the instance record and send a state update notification
# if task or vm state changed
old_ref, instance_ref = self.db.instance_update_and_get_original(
context, instance.uuid, kwargs)
notifications.send_update(context, old_ref,
instance_ref, service="api")
return dict(old_ref.iteritems()), dict(instance_ref.iteritems())
def _check_auto_disk_config(self, instance=None, image=None, def _check_auto_disk_config(self, instance=None, image=None,
**extra_instance_updates): **extra_instance_updates):
auto_disk_config = extra_instance_updates.get("auto_disk_config") auto_disk_config = extra_instance_updates.get("auto_disk_config")

View File

@ -205,32 +205,6 @@ class ComputeCellsAPI(compute_api.API):
""" """
pass pass
def update(self, context, instance, **kwargs):
"""Update an instance."""
cell_name = instance.cell_name
if cell_name and self._cell_read_only(cell_name):
raise exception.InstanceInvalidState(
attr="vm_state",
instance_uuid=instance.uuid,
state="temporary_readonly",
method='update')
rv = super(ComputeCellsAPI, self).update(context,
instance, **kwargs)
kwargs_copy = kwargs.copy()
# We need to skip vm_state/task_state updates as the child
# cell is authoritative for these. The admin API does
# support resetting state, but it has been converted to use
# Instance.save() with an appropriate kwarg.
kwargs_copy.pop('vm_state', None)
kwargs_copy.pop('task_state', None)
if kwargs_copy:
try:
self._cast_to_cells(context, instance, 'update',
**kwargs_copy)
except exception.InstanceUnknownCell:
pass
return rv
def soft_delete(self, context, instance): def soft_delete(self, context, instance):
self._handle_cell_delete(context, instance, 'soft_delete') self._handle_cell_delete(context, instance, 'soft_delete')

View File

@ -14,6 +14,7 @@
import uuid import uuid
import mock
from oslo_config import cfg from oslo_config import cfg
import webob import webob
@ -93,10 +94,6 @@ class EvacuateTestV21(test.NoDBTestCase):
controller._evacuate, controller._evacuate,
self.admin_req, uuid or self.UUID, body=body) self.admin_req, uuid or self.UUID, body=body)
def _fake_update(self, inst, context, instance, task_state,
expected_task_state):
return None
def test_evacuate_with_valid_instance(self): def test_evacuate_with_valid_instance(self):
admin_pass = 'MyNewPass' admin_pass = 'MyNewPass'
res = self._get_evacuate_response({'host': 'my-host', res = self._get_evacuate_response({'host': 'my-host',
@ -161,8 +158,6 @@ class EvacuateTestV21(test.NoDBTestCase):
'adminPass': 'MyNewPass'}) 'adminPass': 'MyNewPass'})
def test_evacuate_instance_with_target(self): def test_evacuate_instance_with_target(self):
self.stubs.Set(compute_api.API, 'update', self._fake_update)
admin_pass = 'MyNewPass' admin_pass = 'MyNewPass'
res = self._get_evacuate_response({'host': 'my-host', res = self._get_evacuate_response({'host': 'my-host',
'onSharedStorage': 'False', 'onSharedStorage': 'False',
@ -170,21 +165,21 @@ class EvacuateTestV21(test.NoDBTestCase):
self.assertEqual(admin_pass, res['adminPass']) self.assertEqual(admin_pass, res['adminPass'])
def test_evacuate_shared_and_pass(self): @mock.patch('nova.objects.Instance.save')
self.stubs.Set(compute_api.API, 'update', self._fake_update) def test_evacuate_shared_and_pass(self, mock_save):
self._check_evacuate_failure(webob.exc.HTTPBadRequest, self._check_evacuate_failure(webob.exc.HTTPBadRequest,
{'host': 'bad-host', {'host': 'bad-host',
'onSharedStorage': 'True', 'onSharedStorage': 'True',
'adminPass': 'MyNewPass'}) 'adminPass': 'MyNewPass'})
def test_evacuate_not_shared_pass_generated(self): @mock.patch('nova.objects.Instance.save')
self.stubs.Set(compute_api.API, 'update', self._fake_update) def test_evacuate_not_shared_pass_generated(self, mock_save):
res = self._get_evacuate_response({'host': 'my-host', res = self._get_evacuate_response({'host': 'my-host',
'onSharedStorage': 'False'}) 'onSharedStorage': 'False'})
self.assertEqual(CONF.password_length, len(res['adminPass'])) self.assertEqual(CONF.password_length, len(res['adminPass']))
def test_evacuate_shared(self): @mock.patch('nova.objects.Instance.save')
self.stubs.Set(compute_api.API, 'update', self._fake_update) def test_evacuate_shared(self, mock_save):
self._get_evacuate_response({'host': 'my-host', self._get_evacuate_response({'host': 'my-host',
'onSharedStorage': 'True'}) 'onSharedStorage': 'True'})
@ -208,12 +203,12 @@ class EvacuateTestV21(test.NoDBTestCase):
'adminPass': 'MyNewPass'}, 'adminPass': 'MyNewPass'},
controller=self.controller_no_ext) controller=self.controller_no_ext)
def test_evacuate_instance_with_underscore_in_hostname(self): @mock.patch('nova.objects.Instance.save')
def test_evacuate_instance_with_underscore_in_hostname(self, mock_save):
admin_pass = 'MyNewPass'
# NOTE: The hostname grammar in RFC952 does not allow for # NOTE: The hostname grammar in RFC952 does not allow for
# underscores in hostnames. However, we should test that it # underscores in hostnames. However, we should test that it
# is supported because it sometimes occurs in real systems. # is supported because it sometimes occurs in real systems.
admin_pass = 'MyNewPass'
self.stubs.Set(compute_api.API, 'update', self._fake_update)
res = self._get_evacuate_response({'host': 'underscore_hostname', res = self._get_evacuate_response({'host': 'underscore_hostname',
'onSharedStorage': 'False', 'onSharedStorage': 'False',
'adminPass': admin_pass}) 'adminPass': admin_pass})
@ -226,9 +221,10 @@ class EvacuateTestV21(test.NoDBTestCase):
def test_evacuate_enable_password_return(self): def test_evacuate_enable_password_return(self):
self._test_evacuate_enable_instance_password_conf(True) self._test_evacuate_enable_instance_password_conf(True)
def _test_evacuate_enable_instance_password_conf(self, enable_pass): @mock.patch('nova.objects.Instance.save')
def _test_evacuate_enable_instance_password_conf(self, mock_save,
enable_pass):
self.flags(enable_instance_password=enable_pass) self.flags(enable_instance_password=enable_pass)
self.stubs.Set(compute_api.API, 'update', self._fake_update)
res = self._get_evacuate_response({'host': 'underscore_hostname', res = self._get_evacuate_response({'host': 'underscore_hostname',
'onSharedStorage': 'False'}) 'onSharedStorage': 'False'})

View File

@ -3304,18 +3304,15 @@ class ServersAllExtensionsTestCase(test.TestCase):
def test_update_missing_server(self): def test_update_missing_server(self):
# Test update with malformed body. # Test update with malformed body.
def fake_update(*args, **kwargs):
raise test.TestingException("Should not reach the compute API.")
self.stubs.Set(compute_api.API, 'update', fake_update)
req = fakes.HTTPRequestV3.blank('/servers/1') req = fakes.HTTPRequestV3.blank('/servers/1')
req.method = 'PUT' req.method = 'PUT'
req.content_type = 'application/json' req.content_type = 'application/json'
body = {'foo': {'a': 'b'}} body = {'foo': {'a': 'b'}}
req.body = jsonutils.dumps(body) req.body = jsonutils.dumps(body)
res = req.get_response(self.app) with mock.patch('nova.objects.Instance.save') as mock_save:
res = req.get_response(self.app)
self.assertFalse(mock_save.called)
self.assertEqual(400, res.status_int) self.assertEqual(400, res.status_int)

View File

@ -550,7 +550,6 @@ class _ComputeAPIUnitTestMixIn(object):
self.mox.StubOutWithMock(self.context, 'elevated') self.mox.StubOutWithMock(self.context, 'elevated')
self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api, '_record_action_start')
self.mox.StubOutWithMock(self.compute_api, 'update')
self.mox.StubOutWithMock(inst, 'save') self.mox.StubOutWithMock(inst, 'save')
expected_task_state = [None] expected_task_state = [None]
if reboot_type == 'HARD': if reboot_type == 'HARD':
@ -1410,7 +1409,6 @@ class _ComputeAPIUnitTestMixIn(object):
self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id')
# Should never reach these. # Should never reach these.
self.mox.StubOutWithMock(self.compute_api, '_reserve_quota_delta') self.mox.StubOutWithMock(self.compute_api, '_reserve_quota_delta')
self.mox.StubOutWithMock(self.compute_api, 'update')
self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(quota.QUOTAS, 'commit')
self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api, '_record_action_start')
self.mox.StubOutWithMock(self.compute_api.compute_task_api, self.mox.StubOutWithMock(self.compute_api.compute_task_api,
@ -1424,15 +1422,16 @@ class _ComputeAPIUnitTestMixIn(object):
self.mox.ReplayAll() self.mox.ReplayAll()
self.assertRaises(exception.FlavorNotFound, with mock.patch.object(fake_inst, 'save') as mock_save:
self.compute_api.resize, self.context, self.assertRaises(exception.FlavorNotFound,
fake_inst, flavor_id='flavor-id') self.compute_api.resize, self.context,
fake_inst, flavor_id='flavor-id')
self.assertFalse(mock_save.called)
def test_resize_disabled_flavor_fails(self): def test_resize_disabled_flavor_fails(self):
self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id')
# Should never reach these. # Should never reach these.
self.mox.StubOutWithMock(self.compute_api, '_reserve_quota_delta') self.mox.StubOutWithMock(self.compute_api, '_reserve_quota_delta')
self.mox.StubOutWithMock(self.compute_api, 'update')
self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(quota.QUOTAS, 'commit')
self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api, '_record_action_start')
self.mox.StubOutWithMock(self.compute_api.compute_task_api, self.mox.StubOutWithMock(self.compute_api.compute_task_api,
@ -1447,9 +1446,11 @@ class _ComputeAPIUnitTestMixIn(object):
self.mox.ReplayAll() self.mox.ReplayAll()
self.assertRaises(exception.FlavorNotFound, with mock.patch.object(fake_inst, 'save') as mock_save:
self.compute_api.resize, self.context, self.assertRaises(exception.FlavorNotFound,
fake_inst, flavor_id='flavor-id') self.compute_api.resize, self.context,
fake_inst, flavor_id='flavor-id')
self.assertFalse(mock_save.called)
@mock.patch.object(flavors, 'get_flavor_by_flavor_id') @mock.patch.object(flavors, 'get_flavor_by_flavor_id')
def test_resize_to_zero_disk_flavor_fails(self, get_flavor_by_flavor_id): def test_resize_to_zero_disk_flavor_fails(self, get_flavor_by_flavor_id):
@ -1468,7 +1469,6 @@ class _ComputeAPIUnitTestMixIn(object):
self.mox.StubOutWithMock(self.compute_api, '_upsize_quota_delta') self.mox.StubOutWithMock(self.compute_api, '_upsize_quota_delta')
self.mox.StubOutWithMock(self.compute_api, '_reserve_quota_delta') self.mox.StubOutWithMock(self.compute_api, '_reserve_quota_delta')
# Should never reach these. # Should never reach these.
self.mox.StubOutWithMock(self.compute_api, 'update')
self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(quota.QUOTAS, 'commit')
self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api, '_record_action_start')
self.mox.StubOutWithMock(self.compute_api.compute_task_api, self.mox.StubOutWithMock(self.compute_api.compute_task_api,
@ -1497,9 +1497,11 @@ class _ComputeAPIUnitTestMixIn(object):
self.mox.ReplayAll() self.mox.ReplayAll()
self.assertRaises(exception.TooManyInstances, with mock.patch.object(fake_inst, 'save') as mock_save:
self.compute_api.resize, self.context, self.assertRaises(exception.TooManyInstances,
fake_inst, flavor_id='flavor-id') self.compute_api.resize, self.context,
fake_inst, flavor_id='flavor-id')
self.assertFalse(mock_save.called)
def test_pause(self): def test_pause(self):
# Ensure instance can be paused. # Ensure instance can be paused.

View File

@ -121,12 +121,6 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
self.stubs.Set(self.compute_api, '_validate_cell', self.stubs.Set(self.compute_api, '_validate_cell',
_fake_validate_cell) _fake_validate_cell)
# NOTE(belliott) Don't update the instance state
# for the tests at the API layer. Let it happen after
# the stub cast to cells so that expected_task_states
# match.
self.stubs.Set(self.compute_api, 'update', _nop_update)
deploy_stubs(self.stubs, self.compute_api) deploy_stubs(self.stubs, self.compute_api)
def tearDown(self): def tearDown(self):