From 01d28bcadd3db9255dd46138caa4bd31837baaf1 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Tue, 2 Dec 2014 12:23:10 -0800 Subject: [PATCH] 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 --- nova/compute/api.py | 25 ---------------- nova/compute/cells_api.py | 26 ---------------- .../compute/contrib/test_evacuate.py | 30 ++++++++----------- .../compute/plugins/v3/test_servers.py | 9 ++---- nova/tests/unit/compute/test_compute_api.py | 28 +++++++++-------- nova/tests/unit/compute/test_compute_cells.py | 6 ---- 6 files changed, 31 insertions(+), 93 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 6494800018..019b856bb5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1480,31 +1480,6 @@ class API(base.Base): shutdown_terminate=shutdown_terminate, 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, **extra_instance_updates): auto_disk_config = extra_instance_updates.get("auto_disk_config") diff --git a/nova/compute/cells_api.py b/nova/compute/cells_api.py index 34217ca79a..03e7e3cc8e 100644 --- a/nova/compute/cells_api.py +++ b/nova/compute/cells_api.py @@ -205,32 +205,6 @@ class ComputeCellsAPI(compute_api.API): """ 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): self._handle_cell_delete(context, instance, 'soft_delete') diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_evacuate.py b/nova/tests/unit/api/openstack/compute/contrib/test_evacuate.py index c01217481b..8f0453a4d5 100644 --- a/nova/tests/unit/api/openstack/compute/contrib/test_evacuate.py +++ b/nova/tests/unit/api/openstack/compute/contrib/test_evacuate.py @@ -14,6 +14,7 @@ import uuid +import mock from oslo_config import cfg import webob @@ -93,10 +94,6 @@ class EvacuateTestV21(test.NoDBTestCase): controller._evacuate, 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): admin_pass = 'MyNewPass' res = self._get_evacuate_response({'host': 'my-host', @@ -161,8 +158,6 @@ class EvacuateTestV21(test.NoDBTestCase): 'adminPass': 'MyNewPass'}) def test_evacuate_instance_with_target(self): - self.stubs.Set(compute_api.API, 'update', self._fake_update) - admin_pass = 'MyNewPass' res = self._get_evacuate_response({'host': 'my-host', 'onSharedStorage': 'False', @@ -170,21 +165,21 @@ class EvacuateTestV21(test.NoDBTestCase): self.assertEqual(admin_pass, res['adminPass']) - def test_evacuate_shared_and_pass(self): - self.stubs.Set(compute_api.API, 'update', self._fake_update) + @mock.patch('nova.objects.Instance.save') + def test_evacuate_shared_and_pass(self, mock_save): self._check_evacuate_failure(webob.exc.HTTPBadRequest, {'host': 'bad-host', 'onSharedStorage': 'True', 'adminPass': 'MyNewPass'}) - def test_evacuate_not_shared_pass_generated(self): - self.stubs.Set(compute_api.API, 'update', self._fake_update) + @mock.patch('nova.objects.Instance.save') + def test_evacuate_not_shared_pass_generated(self, mock_save): res = self._get_evacuate_response({'host': 'my-host', 'onSharedStorage': 'False'}) self.assertEqual(CONF.password_length, len(res['adminPass'])) - def test_evacuate_shared(self): - self.stubs.Set(compute_api.API, 'update', self._fake_update) + @mock.patch('nova.objects.Instance.save') + def test_evacuate_shared(self, mock_save): self._get_evacuate_response({'host': 'my-host', 'onSharedStorage': 'True'}) @@ -208,12 +203,12 @@ class EvacuateTestV21(test.NoDBTestCase): 'adminPass': 'MyNewPass'}, 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 # underscores in hostnames. However, we should test that it # 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', 'onSharedStorage': 'False', 'adminPass': admin_pass}) @@ -226,9 +221,10 @@ class EvacuateTestV21(test.NoDBTestCase): def test_evacuate_enable_password_return(self): 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.stubs.Set(compute_api.API, 'update', self._fake_update) res = self._get_evacuate_response({'host': 'underscore_hostname', 'onSharedStorage': 'False'}) diff --git a/nova/tests/unit/api/openstack/compute/plugins/v3/test_servers.py b/nova/tests/unit/api/openstack/compute/plugins/v3/test_servers.py index 729c05b21f..448b908b87 100644 --- a/nova/tests/unit/api/openstack/compute/plugins/v3/test_servers.py +++ b/nova/tests/unit/api/openstack/compute/plugins/v3/test_servers.py @@ -3304,18 +3304,15 @@ class ServersAllExtensionsTestCase(test.TestCase): def test_update_missing_server(self): # 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.method = 'PUT' req.content_type = 'application/json' body = {'foo': {'a': 'b'}} 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) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index d7001a1f60..afce95ff7d 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -550,7 +550,6 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(self.context, 'elevated') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') - self.mox.StubOutWithMock(self.compute_api, 'update') self.mox.StubOutWithMock(inst, 'save') expected_task_state = [None] if reboot_type == 'HARD': @@ -1410,7 +1409,6 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') # Should never reach these. 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(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_task_api, @@ -1424,15 +1422,16 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.ReplayAll() - self.assertRaises(exception.FlavorNotFound, - self.compute_api.resize, self.context, - fake_inst, flavor_id='flavor-id') + with mock.patch.object(fake_inst, 'save') as mock_save: + self.assertRaises(exception.FlavorNotFound, + self.compute_api.resize, self.context, + fake_inst, flavor_id='flavor-id') + self.assertFalse(mock_save.called) def test_resize_disabled_flavor_fails(self): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') # Should never reach these. 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(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_task_api, @@ -1447,9 +1446,11 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.ReplayAll() - self.assertRaises(exception.FlavorNotFound, - self.compute_api.resize, self.context, - fake_inst, flavor_id='flavor-id') + with mock.patch.object(fake_inst, 'save') as mock_save: + self.assertRaises(exception.FlavorNotFound, + 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') 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, '_reserve_quota_delta') # Should never reach these. - self.mox.StubOutWithMock(self.compute_api, 'update') self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_task_api, @@ -1497,9 +1497,11 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.ReplayAll() - self.assertRaises(exception.TooManyInstances, - self.compute_api.resize, self.context, - fake_inst, flavor_id='flavor-id') + with mock.patch.object(fake_inst, 'save') as mock_save: + self.assertRaises(exception.TooManyInstances, + self.compute_api.resize, self.context, + fake_inst, flavor_id='flavor-id') + self.assertFalse(mock_save.called) def test_pause(self): # Ensure instance can be paused. diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index db26e9dacd..a769e1d97f 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -121,12 +121,6 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase): self.stubs.Set(self.compute_api, '_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) def tearDown(self):