From 852b9307ac86b20cbe870aa5bbfaf121226f5440 Mon Sep 17 00:00:00 2001 From: Brian Lamar Date: Mon, 26 Sep 2011 23:41:28 -0400 Subject: [PATCH] Snapshots/backups can no longer happen simultaneously. Tests included. Implemented exception.InstanceBusy when attempting to snapshot/backup an instance which is already snapshotting or being currently backed up. Fixes bug 727502. (Patch Set 2) 3 new exceptions: InstanceBusy, InstanceSnapshotting, and InstanceBackingUp (Patch Set 3) Oops. New exceptions now inherit from InstanceBusy (Patch Set 4) Tests now tear down created instances (cherry picked from commit b5abd8e7415c28630852107da7755045f6522b50) Change-Id: I42a04254bd96d5f7a92b71a5d3e79f0350bbda5b --- nova/api/openstack/images.py | 13 +++++++--- nova/api/openstack/servers.py | 12 ++++++--- nova/compute/api.py | 8 ++++++ nova/exception.py | 12 +++++++++ nova/tests/api/openstack/test_servers.py | 17 +++++++++++++ nova/tests/test_compute.py | 31 ++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 8 deletions(-) diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 1c8fc10c973..b6dfa22ebeb 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -124,10 +124,15 @@ def create(self, req, body): context = req.environ["nova.context"] props = {'instance_id': instance_id} - image = self._compute_service.snapshot(context, - instance_id, - image_name, - extra_properties=props) + + try: + image = self._compute_service.snapshot(context, + instance_id, + image_name, + extra_properties=props) + except exception.InstanceBusy: + msg = _("Server is currently creating an image. Please wait.") + raise webob.exc.HTTPConflict(explanation=msg) return dict(image=self.get_builder(req).build(image, detail=True)) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index d084ac36092..94c4e998bb0 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -811,10 +811,14 @@ def _action_create_image(self, input_dict, req, instance_id): msg = _("Invalid metadata") raise webob.exc.HTTPBadRequest(explanation=msg) - image = self.compute_api.snapshot(context, - instance_id, - image_name, - extra_properties=props) + try: + image = self.compute_api.snapshot(context, + instance_id, + image_name, + extra_properties=props) + except exception.InstanceBusy: + msg = _("Server is currently creating an image. Please wait.") + raise webob.exc.HTTPConflict(explanation=msg) # build location of newly-created image entity image_id = str(image['id']) diff --git a/nova/compute/api.py b/nova/compute/api.py index 96d5fed8e59..0d6e16d1571 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1028,6 +1028,14 @@ def _create_image(self, context, instance_id, name, image_type, """ instance = self.db.instance_get(context, instance_id) + task_state = instance["task_state"] + + if task_state == task_states.IMAGE_BACKUP: + raise exception.InstanceBackingUp(instance_id=instance_id) + + if task_state == task_states.IMAGE_SNAPSHOT: + raise exception.InstanceSnapshotting(instance_id=instance_id) + properties = {'instance_uuid': instance['uuid'], 'user_id': str(context.user_id), 'image_state': 'creating', diff --git a/nova/exception.py b/nova/exception.py index ba67f098dfe..79b1bf3a21b 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -182,6 +182,18 @@ class AdminRequired(NotAuthorized): message = _("User does not have admin privileges") +class InstanceBusy(NovaException): + message = _("Instance %(instance_id)s is busy. (%(task_state)s)") + + +class InstanceSnapshotting(InstanceBusy): + message = _("Instance %(instance_id)s is currently snapshotting.") + + +class InstanceBackingUp(InstanceBusy): + message = _("Instance %(instance_id)s is currently being backed up.") + + class Invalid(NovaException): message = _("Unacceptable parameters.") diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 821a02eeab0..817b8a3a282 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1626,6 +1626,23 @@ def test_create_instance_no_name(self): res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 400) + def test_create_image_conflict_snapshot_v1_1(self): + """Attempt to create image when image is already being created.""" + def snapshot(*args, **kwargs): + raise exception.InstanceSnapshotting + self.stubs.Set(nova.compute.API, 'snapshot', snapshot) + + req = webob.Request.blank('/v1.1/fakes/servers/1/action') + req.method = 'POST' + req.body = json.dumps({ + "createImage": { + "name": "test_snapshot", + }, + }) + req.headers["content-type"] = "application/json" + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 409) + def test_create_instance_nonstring_name(self): self._setup_for_create_instance() diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 050d1395389..67dbe5e2bde 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -24,6 +24,7 @@ from nova.compute import instance_types from nova.compute import manager as compute_manager from nova.compute import power_state +from nova.compute import task_states from nova.compute import vm_states from nova import context from nova import db @@ -352,6 +353,36 @@ def test_snapshot(self): self.compute.snapshot_instance(self.context, instance_id, name) self.compute.terminate_instance(self.context, instance_id) + def test_snapshot_conflict_backup(self): + """Can't backup an instance which is already being backed up.""" + instance_id = self._create_instance() + instance_values = {'task_state': task_states.IMAGE_BACKUP} + db.instance_update(self.context, instance_id, instance_values) + + self.assertRaises(exception.InstanceBackingUp, + self.compute_api.backup, + self.context, + instance_id, + None, + None, + None) + + db.instance_destroy(self.context, instance_id) + + def test_snapshot_conflict_snapshot(self): + """Can't snapshot an instance which is already being snapshotted.""" + instance_id = self._create_instance() + instance_values = {'task_state': task_states.IMAGE_SNAPSHOT} + db.instance_update(self.context, instance_id, instance_values) + + self.assertRaises(exception.InstanceSnapshotting, + self.compute_api.snapshot, + self.context, + instance_id, + None) + + db.instance_destroy(self.context, instance_id) + def test_console_output(self): """Make sure we can get console output from instance""" instance_id = self._create_instance()