diff options
| -rw-r--r-- | nova/compute/api.py | 62 | ||||
| -rw-r--r-- | nova/compute/manager.py | 5 | ||||
| -rw-r--r-- | nova/context.py | 6 | ||||
| -rw-r--r-- | nova/exception.py | 4 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/test_server_actions.py | 12 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/test_server_metadata.py | 6 | ||||
| -rw-r--r-- | nova/tests/compute/test_compute.py | 2 |
7 files changed, 82 insertions, 15 deletions
diff --git a/nova/compute/api.py b/nova/compute/api.py index 4684fc54f..c0c7c099d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -94,6 +94,21 @@ def check_instance_state(vm_state=None, task_state=(None,)): return outer +def check_instance_lock(function): + @functools.wraps(function) + def inner(self, context, instance, *args, **kwargs): + if instance['locked'] and not context.is_admin: + raise exception.InstanceIsLocked(instance_uuid=instance['uuid']) + # NOTE(danms): at this point, we have verified that either + # theinstance is not locked, or the user is suffiently endowed + # that it doesn't matter. While the following statement may be + # interpreted as the "the instance is not locked", it actually + # refers to the whole condition. + context.instance_lock_checked = True + return function(self, context, instance, *args, **kwargs) + return inner + + def policy_decorator(scope): """Check corresponding policy prior of wrapped method to execution""" def outer(func): @@ -838,6 +853,7 @@ class API(base.Base): return dict(instance_ref.iteritems()) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=None, task_state=None) def soft_delete(self, context, instance): """Terminate an instance.""" @@ -915,6 +931,7 @@ class API(base.Base): # NOTE(maoy): we allow delete to be called no matter what vm_state says. @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=None, task_state=None) def delete(self, context, instance): """Terminate an instance.""" @@ -926,6 +943,7 @@ class API(base.Base): self._delete(context, instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.SOFT_DELETED]) def restore(self, context, instance): """Restore a previously deleted (but not reclaimed) instance.""" @@ -943,12 +961,14 @@ class API(base.Base): deleted_at=None) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.SOFT_DELETED]) def force_delete(self, context, instance): """Force delete a previously deleted (but not reclaimed) instance.""" self._delete(context, instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED, vm_states.ERROR, vm_states.STOPPED], task_state=[None]) @@ -965,6 +985,7 @@ class API(base.Base): self.compute_rpcapi.stop_instance(context, instance, cast=do_cast) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.STOPPED]) def start(self, context, instance): """Start an instance.""" @@ -1223,6 +1244,7 @@ class API(base.Base): return min_ram, min_disk @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, vm_states.RESCUED], task_state=[None]) @@ -1244,6 +1266,7 @@ class API(base.Base): return image_service.show(context, image_id) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED], task_state=[None]) def rebuild(self, context, instance, image_href, admin_password, **kwargs): @@ -1309,6 +1332,7 @@ class API(base.Base): image_ref=image_href, orig_image_ref=orig_image_ref) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.RESIZED]) def revert_resize(self, context, instance): """Reverts a resize, deleting the 'new' instance in the process.""" @@ -1331,6 +1355,7 @@ class API(base.Base): {'status': 'reverted'}) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.RESIZED]) def confirm_resize(self, context, instance): """Confirms a migration/resize and deletes the 'old' instance.""" @@ -1354,6 +1379,7 @@ class API(base.Base): {'status': 'confirmed'}) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED], task_state=[None]) def resize(self, context, instance, flavor_id=None, **kwargs): @@ -1427,18 +1453,21 @@ class API(base.Base): self.scheduler_rpcapi.prep_resize(context, **args) @wrap_check_policy + @check_instance_lock def add_fixed_ip(self, context, instance, network_id): """Add fixed_ip from specified network to given instance.""" self.compute_rpcapi.add_fixed_ip_to_instance(context, instance=instance, network_id=network_id) @wrap_check_policy + @check_instance_lock def remove_fixed_ip(self, context, instance, address): """Remove fixed_ip from specified network to given instance.""" self.compute_rpcapi.remove_fixed_ip_from_instance(context, instance=instance, address=address) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED]) def pause(self, context, instance): """Pause the given instance.""" @@ -1449,6 +1478,7 @@ class API(base.Base): self.compute_rpcapi.pause_instance(context, instance=instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.PAUSED]) def unpause(self, context, instance): """Unpause the given instance.""" @@ -1464,6 +1494,7 @@ class API(base.Base): return self.compute_rpcapi.get_diagnostics(context, instance=instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED]) def suspend(self, context, instance): """Suspend the given instance.""" @@ -1474,6 +1505,7 @@ class API(base.Base): self.compute_rpcapi.suspend_instance(context, instance=instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.SUSPENDED]) def resume(self, context, instance): """Resume the given instance.""" @@ -1484,6 +1516,7 @@ class API(base.Base): self.compute_rpcapi.resume_instance(context, instance=instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED]) def rescue(self, context, instance, rescue_password=None): """Rescue the given instance.""" @@ -1496,6 +1529,7 @@ class API(base.Base): rescue_password=rescue_password) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.RESCUED]) def unrescue(self, context, instance): """Unrescue the given instance.""" @@ -1506,6 +1540,7 @@ class API(base.Base): self.compute_rpcapi.unrescue_instance(context, instance=instance) @wrap_check_policy + @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE]) def set_admin_password(self, context, instance, password=None): """Set the root/admin password for the given instance.""" @@ -1518,6 +1553,7 @@ class API(base.Base): new_pass=password) @wrap_check_policy + @check_instance_lock def inject_file(self, context, instance, path, file_contents): """Write a file to the given instance.""" self.compute_rpcapi.inject_file(context, instance=instance, path=path, @@ -1563,16 +1599,19 @@ class API(base.Base): return self.get(context, instance['uuid'])['locked'] @wrap_check_policy + @check_instance_lock def reset_network(self, context, instance): """Reset networking on the instance.""" self.compute_rpcapi.reset_network(context, instance=instance) @wrap_check_policy + @check_instance_lock def inject_network_info(self, context, instance): """Inject network info for the instance.""" self.compute_rpcapi.inject_network_info(context, instance=instance) @wrap_check_policy + @check_instance_lock def attach_volume(self, context, instance, volume_id, device): """Attach an existing volume to an existing instance.""" if not re.match("^/dev/x{0,1}[a-z]d[a-z]+$", device): @@ -1583,6 +1622,17 @@ class API(base.Base): self.compute_rpcapi.attach_volume(context, instance=instance, volume_id=volume_id, mountpoint=device) + @check_instance_lock + def _detach_volume(self, context, instance, volume_id): + check_policy(context, 'detach_volume', instance) + + volume = self.volume_api.get(context, volume_id) + self.volume_api.check_detach(context, volume) + + self.compute_rpcapi.detach_volume(context, instance=instance, + volume_id=volume_id) + return instance + # FIXME(comstud): I wonder if API should pull in the instance from # the volume ID via volume API and pass it and the volume object here def detach_volume(self, context, volume_id): @@ -1593,15 +1643,7 @@ class API(base.Base): instance_uuid) if not instance: raise exception.VolumeUnattached(volume_id=volume_id) - - check_policy(context, 'detach_volume', instance) - - volume = self.volume_api.get(context, volume_id) - self.volume_api.check_detach(context, volume) - - self.compute_rpcapi.detach_volume(context, instance=instance, - volume_id=volume_id) - return instance + self._detach_volume(context, instance, volume_id) @wrap_check_policy def get_instance_metadata(self, context, instance): @@ -1610,6 +1652,7 @@ class API(base.Base): return dict(rv.iteritems()) @wrap_check_policy + @check_instance_lock def delete_instance_metadata(self, context, instance, key): """Delete the given metadata item from an instance.""" self.db.instance_metadata_delete(context, instance['uuid'], key) @@ -1618,6 +1661,7 @@ class API(base.Base): diff={key: ['-']}) @wrap_check_policy + @check_instance_lock def update_instance_metadata(self, context, instance, metadata, delete=False): """Updates or creates instance metadata. diff --git a/nova/compute/manager.py b/nova/compute/manager.py index e85062623..8da98767f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -164,7 +164,10 @@ def checks_instance_lock(function): # and the function may get either an instance_uuid or an instance. def _checks_instance_lock_core(self, cb, context, *args, **kwargs): instance_uuid = kwargs['instance_uuid'] - locked = self._get_lock(context, instance_uuid) + if context.instance_lock_checked: + locked = False # Implied, since we wouldn't be here otherwise + else: + locked = self._get_lock(context, instance_uuid) admin = context.is_admin LOG.info(_("check_instance_lock: locked: |%s|"), locked, diff --git a/nova/context.py b/nova/context.py index 5712193fb..66697b567 100644 --- a/nova/context.py +++ b/nova/context.py @@ -45,7 +45,7 @@ class RequestContext(object): roles=None, remote_address=None, timestamp=None, request_id=None, auth_token=None, overwrite=True, quota_class=None, user_name=None, project_name=None, - service_catalog=None, **kwargs): + service_catalog=None, instance_lock_checked=False, **kwargs): """ :param read_deleted: 'no' indicates deleted records are hidden, 'yes' indicates deleted records are visible, 'only' indicates that @@ -81,6 +81,7 @@ class RequestContext(object): self.request_id = request_id self.auth_token = auth_token self.service_catalog = service_catalog + self.instance_lock_checked = instance_lock_checked # NOTE(markmc): this attribute is currently only used by the # rs_limits turnstile pre-processor. @@ -123,7 +124,8 @@ class RequestContext(object): 'quota_class': self.quota_class, 'user_name': self.user_name, 'service_catalog': self.service_catalog, - 'project_name': self.project_name} + 'project_name': self.project_name, + 'instance_lock_checked': self.instance_lock_checked} @classmethod def from_dict(cls, values): diff --git a/nova/exception.py b/nova/exception.py index 1e9cc56ef..e4e738e85 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1127,6 +1127,10 @@ class TaskNotRunning(NovaException): message = _("Task %(task_name) is not running on host %(host)") +class InstanceIsLocked(InstanceInvalidState): + message = _("Instance %(instance_uuid)s is locked") + + def get_context_from_function_and_args(function, args, kwargs): """Find an arg of type RequestContext and return it. diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py index ef3995649..851fb57f2 100644 --- a/nova/tests/api/openstack/compute/test_server_actions.py +++ b/nova/tests/api/openstack/compute/test_server_actions.py @@ -708,6 +708,18 @@ class ServerActionsControllerTest(test.TestCase): self.controller._action_create_image, req, FAKE_UUID, body) + def test_locked(self): + def fake_locked(context, instance_uuid): + return {"name": "foo", + "uuid": FAKE_UUID, + "locked": True} + self.stubs.Set(nova.db, 'instance_get_by_uuid', fake_locked) + body = dict(reboot=dict(type="HARD")) + req = fakes.HTTPRequest.blank(self.url) + self.assertRaises(webob.exc.HTTPConflict, + self.controller._action_reboot, + req, FAKE_UUID, body) + class TestServerActionXMLDeserializer(test.TestCase): diff --git a/nova/tests/api/openstack/compute/test_server_metadata.py b/nova/tests/api/openstack/compute/test_server_metadata.py index 29eb60cad..5c1f5a67b 100644 --- a/nova/tests/api/openstack/compute/test_server_metadata.py +++ b/nova/tests/api/openstack/compute/test_server_metadata.py @@ -73,13 +73,15 @@ def stub_max_server_metadata(): def return_server(context, server_id): return {'id': server_id, 'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64', - 'name': 'fake'} + 'name': 'fake', + 'locked': False} def return_server_by_uuid(context, server_uuid): return {'id': 1, 'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64', - 'name': 'fake'} + 'name': 'fake', + 'locked': False} def return_server_nonexistant(context, server_id): diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index d59753d58..747fcb12e 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -3709,7 +3709,7 @@ class ComputeAPITestCase(BaseTestCase): self.assertRaises(exception.InvalidDevicePath, self.compute_api.attach_volume, self.context, - None, + {'locked': False}, None, '/dev/invalid') |