From 75a87df739effe840e6cb39c976002e99b49c796 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Wed, 22 Jun 2011 13:31:28 -0500 Subject: Round 1 of backup with rotation. --- nova/compute/api.py | 32 ++++++++++++++++++++++++++++++-- nova/compute/manager.py | 20 ++++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) (limited to 'nova') diff --git a/nova/compute/api.py b/nova/compute/api.py index a7ea88d51..365aa1c5d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -701,18 +701,46 @@ class API(base.Base): raise exception.Error(_("Unable to find host for Instance %s") % instance_id) + def backup(self, context, instance_id, backup_type, rotation): + """Backup the given instance + + instance_id - int - id representing the instance + backup_type - str - whether it's 'daily' or 'weekly' + rotation - int - number of backups to keep around + """ + name = backup_type # daily backups are called 'daily' + recv_meta = self._snapshot(context, instance_id, name, backup_type, + rotation=rotation) + return recv_meta + def snapshot(self, context, instance_id, name): """Snapshot the given instance. :returns: A dict containing image metadata """ - properties = {'instance_id': str(instance_id), + return self._snapshot(context, instance_id, name, 'snapshot') + + def _snapshot(self, context, instance_id, name, image_type, rotation=None): + """Snapshot an instance on this host. + + :param context: security context + :param instance_id: nova.db.sqlalchemy.models.Instance.Id + :param name: string for name of the snapshot + :param image_type: snapshot | daily | weekly + :param rotation: int representing how many backups to keep around; + None if rotation shouldn't be used (as in the case of snapshots) + """ + instance = db.api.instance_get(context, instance_id) + properties = {'instance_uuid': instance['uuid'], 'user_id': str(context.user_id), 'image_state': 'creating'} + if image_type != 'snapshot': + properties['backup_type'] = image_type sent_meta = {'name': name, 'is_public': False, 'status': 'creating', 'properties': properties} recv_meta = self.image_service.create(context, sent_meta) - params = {'image_id': recv_meta['id']} + params = {'image_id': recv_meta['id'], 'image_type': image_type, + 'rotation': rotation} self._cast_compute_message('snapshot_instance', context, instance_id, params=params) return recv_meta diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 4e006e677..bc6981c58 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -473,8 +473,17 @@ class ComputeManager(manager.SchedulerDependentManager): self._update_state(context, instance_id) @exception.wrap_exception - def snapshot_instance(self, context, instance_id, image_id): - """Snapshot an instance on this host.""" + def snapshot_instance(self, context, instance_id, image_id, + image_type='snapshot', rotation=None): + """Snapshot an instance on this host. + + :param context: security context + :param instance_id: nova.db.sqlalchemy.models.Instance.Id + :param image_id: glance.db.sqlalchemy.models.Image.Id + :param image_type: snapshot | daily | weekly + :param rotation: int representing how many backups to keep around; + None if rotation shouldn't be used (as in the case of snapshots) + """ context = context.elevated() instance_ref = self.db.instance_get(context, instance_id) @@ -493,6 +502,13 @@ class ComputeManager(manager.SchedulerDependentManager): 'expected: %(running)s)') % locals()) self.driver.snapshot(instance_ref, image_id) + if rotation: + self.rotate_backups(context, instance_id, image_type, rotation) + + def rotate_backups(self, context, instance_id, image_type, rotation): + """ + """ + pass @exception.wrap_exception @checks_instance_lock -- cgit From ab2a77d0c6f738fe70b5d5a77fa7f97bf1f1f88b Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Wed, 22 Jun 2011 16:14:01 -0500 Subject: Adding backup rotation --- nova/compute/api.py | 7 +++---- nova/compute/manager.py | 40 ++++++++++++++++++++++++++++++++++------ nova/exception.py | 4 ++++ 3 files changed, 41 insertions(+), 10 deletions(-) (limited to 'nova') diff --git a/nova/compute/api.py b/nova/compute/api.py index 365aa1c5d..c0cb2e18a 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -722,7 +722,7 @@ class API(base.Base): def _snapshot(self, context, instance_id, name, image_type, rotation=None): """Snapshot an instance on this host. - + :param context: security context :param instance_id: nova.db.sqlalchemy.models.Instance.Id :param name: string for name of the snapshot @@ -733,9 +733,8 @@ class API(base.Base): instance = db.api.instance_get(context, instance_id) properties = {'instance_uuid': instance['uuid'], 'user_id': str(context.user_id), - 'image_state': 'creating'} - if image_type != 'snapshot': - properties['backup_type'] = image_type + 'image_state': 'creating', + 'image_type': image_type} sent_meta = {'name': name, 'is_public': False, 'status': 'creating', 'properties': properties} recv_meta = self.image_service.create(context, sent_meta) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index bc6981c58..44abd5d89 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -43,6 +43,7 @@ import time import functools from eventlet import greenthread +from operator import itemgetter from nova import exception from nova import flags @@ -476,7 +477,7 @@ class ComputeManager(manager.SchedulerDependentManager): def snapshot_instance(self, context, instance_id, image_id, image_type='snapshot', rotation=None): """Snapshot an instance on this host. - + :param context: security context :param instance_id: nova.db.sqlalchemy.models.Instance.Id :param image_id: glance.db.sqlalchemy.models.Image.Id @@ -502,13 +503,40 @@ class ComputeManager(manager.SchedulerDependentManager): 'expected: %(running)s)') % locals()) self.driver.snapshot(instance_ref, image_id) - if rotation: - self.rotate_backups(context, instance_id, image_type, rotation) + if rotation and image_type == 'snapshot': + raise exception.ImageRotationNotAllowed + elif rotation: + instance_uuid = instance_ref['uuid'] + self.rotate_backups(context, instance_uuid, image_type, rotation) - def rotate_backups(self, context, instance_id, image_type, rotation): - """ + def rotate_backups(self, context, instance_uuid, image_type, rotation): + """Delete excess backups associated to an instance. + + Instances are allowed a fixed number of backups (the rotation number); + this method deletes the oldest backups that exceed the rotation + threshold. + + :param context: security context + :param instance_uuid: string representing uuid of instance + :param image_type: snapshot | daily | weekly + :param rotation: int representing how many backups to keep around; + None if rotation shouldn't be used (as in the case of snapshots) """ - pass + image_service = nova.image.get_default_image_service() + filters = {'property-image-type': image_type, + 'property-instance-uuid': instance_uuid} + images = image_service.detail(context, filters=filters) + if len(images) > rotation: + # Sort oldest (by created_at) to end of list + images.sort(key=itemgetter('created_at'), reverse=True) + + # NOTE(sirp): this deletes all backups that exceed the rotation + # limit + excess = len(images) - rotation + for i in xrange(excess): + image = images.pop() + image_id = image['id'] + image_service.delete(context, image_id) @exception.wrap_exception @checks_instance_lock diff --git a/nova/exception.py b/nova/exception.py index f3a452228..a548a638c 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -549,6 +549,10 @@ class GlobalRoleNotAllowed(NotAllowed): message = _("Unable to use global role %(role_id)s") +class ImageRotationNotAllowed(NovaException): + message = _("Rotation is not allowed for snapshots") + + #TODO(bcwaldon): EOL this exception! class Duplicate(NovaException): pass -- cgit From f6964aadc5b073152d221bb0a4e899c2b17d174c Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Thu, 23 Jun 2011 14:27:13 -0500 Subject: Small refactoring around getting params --- nova/api/openstack/images.py | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) (limited to 'nova') diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 5ffd8e96a..54f8e05a9 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -88,28 +88,53 @@ class Controller(object): return webob.exc.HTTPNoContent() def create(self, req, body): - """Snapshot a server instance and save the image. + """Snapshot or backup a server instance and save the image. + + Images now have an `image_type` associated with them, which can be + 'snapshot' or the backup type, like 'daily' or 'weekly'. + + If the image_type is backup-like, then the rotation factor can be + included and that will cause the oldest backups that exceed the + rotation factor to be deleted. :param req: `wsgi.Request` object """ + def get_param(param): + try: + return body["image"][param] + except KeyError: + raise webob.exc.HTTPBadRequest() + context = req.environ['nova.context'] content_type = req.get_content_type() if not body: raise webob.exc.HTTPBadRequest() + image_type = body["image"].get("image_type", "snapshot") + try: server_id = self._server_id_from_req_data(body) - image_name = body["image"]["name"] except KeyError: raise webob.exc.HTTPBadRequest() - image = self._compute_service.snapshot(context, server_id, image_name) + if image_type == "snapshot": + image_name = get_param("name") + image = self._compute_service.snapshot(context, server_id, + image_name) + else: + if not FLAGS.allow_admin_api: + raise webob.exc.HTTPBadRequest() + + rotation = get_param("rotation") + image = self._compute_service.backup(context, server_id, + image_type, rotation) + return dict(image=self.get_builder(req).build(image, detail=True)) def get_builder(self, request): """Indicates that you must use a Controller subclass.""" - raise NotImplementedError + raise NotImplementedError() def _server_id_from_req_data(self, data): raise NotImplementedError() -- cgit From 63a9216ecbaab20fc7dfb82afb9fe0e2f3fbded4 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Thu, 23 Jun 2011 15:35:26 -0500 Subject: Adding missing import. --- nova/compute/manager.py | 1 + 1 file changed, 1 insertion(+) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 44abd5d89..3c849286e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -47,6 +47,7 @@ from operator import itemgetter from nova import exception from nova import flags +import nova.image from nova import log as logging from nova import manager from nova import network -- cgit From 2028222a5ed47dc82b49f51969d237c4eece50e7 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Thu, 23 Jun 2011 16:17:54 -0500 Subject: Fixed filter property and added logging. --- nova/compute/manager.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 3c849286e..d0ca1ff0d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -524,9 +524,10 @@ class ComputeManager(manager.SchedulerDependentManager): None if rotation shouldn't be used (as in the case of snapshots) """ image_service = nova.image.get_default_image_service() - filters = {'property-image-type': image_type, - 'property-instance-uuid': instance_uuid} + filters = {'property-image_type': image_type, + 'property-instance_uuid': instance_uuid} images = image_service.detail(context, filters=filters) + LOG.debug(_("Found %d images (rotation: %d)" % (len(images), rotation))) if len(images) > rotation: # Sort oldest (by created_at) to end of list images.sort(key=itemgetter('created_at'), reverse=True) @@ -534,9 +535,11 @@ class ComputeManager(manager.SchedulerDependentManager): # NOTE(sirp): this deletes all backups that exceed the rotation # limit excess = len(images) - rotation + LOG.debug(_("Rotating out %d backups" % excess)) for i in xrange(excess): image = images.pop() image_id = image['id'] + LOG.debug(_("Deleting image %d" % image_id)) image_service.delete(context, image_id) @exception.wrap_exception -- cgit From e3c1a6742b16add04d76631b9dbd4f2ef016e0b3 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Thu, 23 Jun 2011 16:19:08 -0500 Subject: PEP8 cleanup. --- nova/compute/manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index d0ca1ff0d..4bd7d434e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -527,7 +527,8 @@ class ComputeManager(manager.SchedulerDependentManager): filters = {'property-image_type': image_type, 'property-instance_uuid': instance_uuid} images = image_service.detail(context, filters=filters) - LOG.debug(_("Found %d images (rotation: %d)" % (len(images), rotation))) + LOG.debug(_("Found %d images (rotation: %d)" % + (len(images), rotation))) if len(images) > rotation: # Sort oldest (by created_at) to end of list images.sort(key=itemgetter('created_at'), reverse=True) -- cgit From 2d0d1e179dd8870967ebf00a82fbc7d21bed6116 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Thu, 23 Jun 2011 16:28:59 -0500 Subject: Cast rotation to int. --- nova/api/openstack/images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 54f8e05a9..d8dbd2360 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -126,7 +126,7 @@ class Controller(object): if not FLAGS.allow_admin_api: raise webob.exc.HTTPBadRequest() - rotation = get_param("rotation") + rotation = int(get_param("rotation")) image = self._compute_service.backup(context, server_id, image_type, rotation) -- cgit From a045cd5fdd00b3e52f46181017077146abe8df9f Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Thu, 23 Jun 2011 16:54:28 -0500 Subject: Fixed syntax errors. --- nova/compute/manager.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 4bd7d434e..ca66d0387 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -527,9 +527,10 @@ class ComputeManager(manager.SchedulerDependentManager): filters = {'property-image_type': image_type, 'property-instance_uuid': instance_uuid} images = image_service.detail(context, filters=filters) - LOG.debug(_("Found %d images (rotation: %d)" % - (len(images), rotation))) - if len(images) > rotation: + num_images = len(images) + LOG.debug(_("Found %(num_images)d images (rotation: %(rotation)d)" + % locals())) + if num_images > rotation: # Sort oldest (by created_at) to end of list images.sort(key=itemgetter('created_at'), reverse=True) -- cgit From c941234c86fc02cf652f2e91ee958260d83fc4d7 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 10:50:09 -0500 Subject: Adding tests for snapshot no-name and backup no-name --- nova/tests/api/openstack/fakes.py | 9 ++++ nova/tests/api/openstack/test_images.py | 88 +++++++++++++++++++++++---------- 2 files changed, 72 insertions(+), 25 deletions(-) (limited to 'nova') diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index f8d158ddd..0a2584910 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -146,6 +146,15 @@ def stub_out_compute_api_snapshot(stubs): stubs.Set(nova.compute.API, 'snapshot', snapshot) +def stub_out_compute_api_backup(stubs): + def backup(self, context, instance_id, backup_type, rotation): + return dict(id='123', status='ACTIVE', + properties=dict(instance_id='123', + image_type=backup_type, + rotation=rotation)) + stubs.Set(nova.compute.API, 'backup', backup) + + def stub_out_glance_add_image(stubs, sent_to_glance): """ We return the metadata sent to glance by modifying the sent_to_glance dict diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index e4204809f..9fabfeae1 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -340,6 +340,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): self.fixtures = self._make_image_fixtures() fakes.stub_out_glance(self.stubs, initial_fixtures=self.fixtures) fakes.stub_out_compute_api_snapshot(self.stubs) + fakes.stub_out_compute_api_backup(self.stubs) def tearDown(self): """Run after each test.""" @@ -364,10 +365,10 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): response_list = response_dict["images"] expected = [{'id': 123, 'name': 'public image'}, - {'id': 124, 'name': 'queued backup'}, - {'id': 125, 'name': 'saving backup'}, - {'id': 126, 'name': 'active backup'}, - {'id': 127, 'name': 'killed backup'}, + {'id': 124, 'name': 'queued snapshot'}, + {'id': 125, 'name': 'saving snapshot'}, + {'id': 126, 'name': 'active snapshot'}, + {'id': 127, 'name': 'killed snapshot'}, {'id': 129, 'name': None}] self.assertDictListMatch(response_list, expected) @@ -617,7 +618,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 124, - 'name': 'queued backup', + 'name': 'queued snapshot', 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -625,7 +626,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 125, - 'name': 'saving backup', + 'name': 'saving snapshot', 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -634,7 +635,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 126, - 'name': 'active backup', + 'name': 'active snapshot', 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -642,7 +643,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 127, - 'name': 'killed backup', + 'name': 'killed snapshot', 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -688,7 +689,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 124, - 'name': 'queued backup', + 'name': 'queued snapshot', 'serverRef': "http://localhost/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -710,7 +711,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 125, - 'name': 'saving backup', + 'name': 'saving snapshot', 'serverRef': "http://localhost/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -733,7 +734,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 126, - 'name': 'active backup', + 'name': 'active snapshot', 'serverRef': "http://localhost/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -755,7 +756,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): }, { 'id': 127, - 'name': 'killed backup', + 'name': 'killed snapshot', 'serverRef': "http://localhost/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, @@ -973,8 +974,43 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): self.assertEqual(res.status_int, 404) def test_create_image(self): + body = dict(image=dict(serverId='123', name='Snapshot 1')) + req = webob.Request.blank('/v1.0/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(200, response.status_int) + + def test_create_snapshot_no_name(self): + """Name is required for snapshots + + If an image_type isn't passed, we default to image_type=snapshot, + thus `name` is required + """ + body = dict(image=dict(serverId='123')) + req = webob.Request.blank('/v1.0/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, response.status_int) - body = dict(image=dict(serverId='123', name='Backup 1')) + def test_create_backup_no_name_with_rotation(self): + """Name isn't required for backups, but rotation is. + + The reason name isn't required is because it defaults to the + image_type. + + Creating a backup is an admin-only operation, as opposed to snapshots + which are available to anybody. + """ + # FIXME(sirp): teardown needed? + FLAGS.allow_admin_api = True + + # FIXME(sirp): should the fact that backups are admin_only be a FLAG + body = dict(image=dict(serverId='123', image_type='daily', + rotation=1)) req = webob.Request.blank('/v1.0/images') req.method = 'POST' req.body = json.dumps(body) @@ -984,7 +1020,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): def test_create_image_no_server_id(self): - body = dict(image=dict(name='Backup 1')) + body = dict(image=dict(name='Snapshot 1')) req = webob.Request.blank('/v1.0/images') req.method = 'POST' req.body = json.dumps(body) @@ -994,7 +1030,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): def test_create_image_v1_1(self): - body = dict(image=dict(serverRef='123', name='Backup 1')) + body = dict(image=dict(serverRef='123', name='Snapshot 1')) req = webob.Request.blank('/v1.1/images') req.method = 'POST' req.body = json.dumps(body) @@ -1004,7 +1040,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): def test_create_image_v1_1_xml_serialization(self): - body = dict(image=dict(serverRef='123', name='Backup 1')) + body = dict(image=dict(serverRef='123', name='Snapshot 1')) req = webob.Request.blank('/v1.1/images') req.method = 'POST' req.body = json.dumps(body) @@ -1037,7 +1073,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): def test_create_image_v1_1_no_server_ref(self): - body = dict(image=dict(name='Backup 1')) + body = dict(image=dict(name='Snapshot 1')) req = webob.Request.blank('/v1.1/images') req.method = 'POST' req.body = json.dumps(body) @@ -1064,18 +1100,20 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): status='active', properties={}) image_id += 1 - # Backup for User 1 - backup_properties = {'instance_id': '42', 'user_id': '1'} + # Snapshot for User 1 + snapshot_properties = {'instance_id': '42', 'user_id': '1'} for status in ('queued', 'saving', 'active', 'killed'): - add_fixture(id=image_id, name='%s backup' % status, + add_fixture(id=image_id, name='%s snapshot' % status, is_public=False, status=status, - properties=backup_properties) + properties=snapshot_properties) image_id += 1 - # Backup for User 2 - other_backup_properties = {'instance_id': '43', 'user_id': '2'} - add_fixture(id=image_id, name='someone elses backup', is_public=False, - status='active', properties=other_backup_properties) + # Snapshot for User 2 + other_snapshot_properties = {'instance_id': '43', 'user_id': '2'} + add_fixture(id=image_id, name='someone elses snapshot', + is_public=False, status='active', + properties=other_snapshot_properties) + image_id += 1 # Image without a name -- cgit From 4a32c971893a22a6451eed7e618291ad86c24510 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 10:50:48 -0500 Subject: Trailing whitespace --- nova/tests/api/openstack/test_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 9fabfeae1..036e510c9 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -1008,7 +1008,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): # FIXME(sirp): teardown needed? FLAGS.allow_admin_api = True - # FIXME(sirp): should the fact that backups are admin_only be a FLAG + # FIXME(sirp): should the fact that backups are admin_only be a FLAG body = dict(image=dict(serverId='123', image_type='daily', rotation=1)) req = webob.Request.blank('/v1.0/images') -- cgit From cbf9f1bef113d54be57e2bb9a79990226afcd90f Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 11:55:43 -0500 Subject: Adding tests for backup no rotation, invalid image type --- nova/api/openstack/images.py | 6 +++++- nova/tests/api/openstack/test_images.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index d8dbd2360..2287ca0f7 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -122,13 +122,17 @@ class Controller(object): image_name = get_param("name") image = self._compute_service.snapshot(context, server_id, image_name) - else: + elif image_type in ("daily", "weekly"): if not FLAGS.allow_admin_api: raise webob.exc.HTTPBadRequest() rotation = int(get_param("rotation")) image = self._compute_service.backup(context, server_id, image_type, rotation) + else: + LOG.error(_("Invalid image_type '%s' passed" % image_type)) + raise webob.exc.HTTPBadRequest() + return dict(image=self.get_builder(req).build(image, detail=True)) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 036e510c9..0fad044f1 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -1018,6 +1018,35 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): response = req.get_response(fakes.wsgi_app()) self.assertEqual(200, response.status_int) + def test_create_backup_no_rotation(self): + """Rotation is required for backup requests""" + # FIXME(sirp): teardown needed? + FLAGS.allow_admin_api = True + + # FIXME(sirp): should the fact that backups are admin_only be a FLAG + body = dict(image=dict(serverId='123', image_type='daily')) + req = webob.Request.blank('/v1.0/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, response.status_int) + + def test_create_image_with_invalid_image_type(self): + """Valid image_types are snapshot | daily | weekly""" + # FIXME(sirp): teardown needed? + FLAGS.allow_admin_api = True + + # FIXME(sirp): should the fact that backups are admin_only be a FLAG + body = dict(image=dict(serverId='123', image_type='monthly', + rotation=1)) + req = webob.Request.blank('/v1.0/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, response.status_int) + def test_create_image_no_server_id(self): body = dict(image=dict(name='Snapshot 1')) -- cgit From 1d3960e3b76e3f75c68f919278a2a227e1f96e48 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 11:56:15 -0500 Subject: Pep8 fix --- nova/api/openstack/images.py | 1 - 1 file changed, 1 deletion(-) (limited to 'nova') diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 2287ca0f7..5f88ede96 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -133,7 +133,6 @@ class Controller(object): LOG.error(_("Invalid image_type '%s' passed" % image_type)) raise webob.exc.HTTPBadRequest() - return dict(image=self.get_builder(req).build(image, detail=True)) def get_builder(self, request): -- cgit From 594d5c7a98f2b4e6ea2d866f10c67cbdaa88ce0c Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 15:03:01 -0500 Subject: Refactored backup rotate. --- nova/api/openstack/images.py | 20 ++++++++++----- nova/compute/api.py | 33 ++++++++++++++---------- nova/compute/manager.py | 29 ++++++++++++++------- nova/exception.py | 4 +++ nova/tests/api/openstack/fakes.py | 5 ++-- nova/tests/api/openstack/test_images.py | 45 ++++++++++++++++++++++++--------- 6 files changed, 93 insertions(+), 43 deletions(-) (limited to 'nova') diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 5f88ede96..c535e4e26 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -118,19 +118,25 @@ class Controller(object): except KeyError: raise webob.exc.HTTPBadRequest() + image_name = get_param("name") + if image_type == "snapshot": - image_name = get_param("name") - image = self._compute_service.snapshot(context, server_id, - image_name) - elif image_type in ("daily", "weekly"): + image = self._compute_service.snapshot( + context, server_id, image_name) + elif image_type == "backup": + # NOTE(sirp): Unlike snapshot, backup is not a customer facing + # API call; rather, it's used by the internal backup scheduler if not FLAGS.allow_admin_api: raise webob.exc.HTTPBadRequest() + backup_type = get_param("backup_type") rotation = int(get_param("rotation")) - image = self._compute_service.backup(context, server_id, - image_type, rotation) + + image = self._compute_service.backup( + context, server_id, image_name, + backup_type, rotation) else: - LOG.error(_("Invalid image_type '%s' passed" % image_type)) + LOG.error(_("Invalid image_type '%s' passed") % image_type) raise webob.exc.HTTPBadRequest() return dict(image=self.get_builder(req).build(image, detail=True)) diff --git a/nova/compute/api.py b/nova/compute/api.py index c0cb2e18a..9c6f0ef9d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -701,32 +701,38 @@ class API(base.Base): raise exception.Error(_("Unable to find host for Instance %s") % instance_id) - def backup(self, context, instance_id, backup_type, rotation): + def backup(self, context, instance_id, name, backup_type, rotation): """Backup the given instance - instance_id - int - id representing the instance - backup_type - str - whether it's 'daily' or 'weekly' - rotation - int - number of backups to keep around - """ + :param instance_id: nova.db.sqlalchemy.models.Instance.Id + :param name: name of the backup or snapshot name = backup_type # daily backups are called 'daily' - recv_meta = self._snapshot(context, instance_id, name, backup_type, - rotation=rotation) + :param rotation: int representing how many backups to keep around; + None if rotation shouldn't be used (as in the case of snapshots) + """ + recv_meta = self._create_image(context, instance_id, name, 'backup', + backup_type=backup_type, rotation=rotation) return recv_meta def snapshot(self, context, instance_id, name): """Snapshot the given instance. + :param instance_id: nova.db.sqlalchemy.models.Instance.Id + :param name: name of the backup or snapshot + :returns: A dict containing image metadata """ - return self._snapshot(context, instance_id, name, 'snapshot') + return self._create_image(context, instance_id, name, 'snapshot') - def _snapshot(self, context, instance_id, name, image_type, rotation=None): - """Snapshot an instance on this host. + def _create_image(self, context, instance_id, name, image_type, + backup_type=None, rotation=None): + """Create snapshot or backup for an instance on this host. :param context: security context :param instance_id: nova.db.sqlalchemy.models.Instance.Id :param name: string for name of the snapshot - :param image_type: snapshot | daily | weekly + :param image_type: snapshot | backup + :param backup_type: daily | weekly :param rotation: int representing how many backups to keep around; None if rotation shouldn't be used (as in the case of snapshots) """ @@ -734,12 +740,13 @@ class API(base.Base): properties = {'instance_uuid': instance['uuid'], 'user_id': str(context.user_id), 'image_state': 'creating', - 'image_type': image_type} + 'image_type': image_type, + 'backup_type': backup_type} sent_meta = {'name': name, 'is_public': False, 'status': 'creating', 'properties': properties} recv_meta = self.image_service.create(context, sent_meta) params = {'image_id': recv_meta['id'], 'image_type': image_type, - 'rotation': rotation} + 'backup_type': backup_type, 'rotation': rotation} self._cast_compute_message('snapshot_instance', context, instance_id, params=params) return recv_meta diff --git a/nova/compute/manager.py b/nova/compute/manager.py index ca66d0387..1458ea41f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -476,13 +476,15 @@ class ComputeManager(manager.SchedulerDependentManager): @exception.wrap_exception def snapshot_instance(self, context, instance_id, image_id, - image_type='snapshot', rotation=None): + image_type='snapshot', backup_type=None, + rotation=None): """Snapshot an instance on this host. :param context: security context :param instance_id: nova.db.sqlalchemy.models.Instance.Id :param image_id: glance.db.sqlalchemy.models.Image.Id - :param image_type: snapshot | daily | weekly + :param image_type: snapshot | backup + :param backup_type: daily | weekly :param rotation: int representing how many backups to keep around; None if rotation shouldn't be used (as in the case of snapshots) """ @@ -504,13 +506,21 @@ class ComputeManager(manager.SchedulerDependentManager): 'expected: %(running)s)') % locals()) self.driver.snapshot(instance_ref, image_id) - if rotation and image_type == 'snapshot': + + if image_type == 'snapshot' and rotation: raise exception.ImageRotationNotAllowed - elif rotation: - instance_uuid = instance_ref['uuid'] - self.rotate_backups(context, instance_uuid, image_type, rotation) + elif image_type == 'backup': + if rotation: + instance_uuid = instance_ref['uuid'] + self.rotate_backups(context, instance_uuid, backup_type, + rotation) + else: + raise exception.RotationRequiredForBackup + else: + raise Exception(_('Image type not recognized %s') % image_type) + - def rotate_backups(self, context, instance_uuid, image_type, rotation): + def rotate_backups(self, context, instance_uuid, backup_type, rotation): """Delete excess backups associated to an instance. Instances are allowed a fixed number of backups (the rotation number); @@ -519,12 +529,13 @@ class ComputeManager(manager.SchedulerDependentManager): :param context: security context :param instance_uuid: string representing uuid of instance - :param image_type: snapshot | daily | weekly + :param backup_type: daily | weekly :param rotation: int representing how many backups to keep around; None if rotation shouldn't be used (as in the case of snapshots) """ image_service = nova.image.get_default_image_service() - filters = {'property-image_type': image_type, + filters = {'property-image_type': 'backup', + 'property-backup_type': backup_type, 'property-instance_uuid': instance_uuid} images = image_service.detail(context, filters=filters) num_images = len(images) diff --git a/nova/exception.py b/nova/exception.py index a548a638c..f3893d239 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -553,6 +553,10 @@ class ImageRotationNotAllowed(NovaException): message = _("Rotation is not allowed for snapshots") +class RotationRequiredForBackup(NovaException): + message = _("Rotation param is required for backup image_type") + + #TODO(bcwaldon): EOL this exception! class Duplicate(NovaException): pass diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 0a2584910..ad9c5067c 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -147,10 +147,11 @@ def stub_out_compute_api_snapshot(stubs): def stub_out_compute_api_backup(stubs): - def backup(self, context, instance_id, backup_type, rotation): + def backup(self, context, instance_id, name, backup_type, rotation): return dict(id='123', status='ACTIVE', properties=dict(instance_id='123', - image_type=backup_type, + name=name, + backup_type=backup_type, rotation=rotation)) stubs.Set(nova.compute.API, 'backup', backup) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 0fad044f1..8ad08080a 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -983,11 +983,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): self.assertEqual(200, response.status_int) def test_create_snapshot_no_name(self): - """Name is required for snapshots - - If an image_type isn't passed, we default to image_type=snapshot, - thus `name` is required - """ + """Name is required for snapshots""" body = dict(image=dict(serverId='123')) req = webob.Request.blank('/v1.0/images') req.method = 'POST' @@ -996,11 +992,19 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): response = req.get_response(fakes.wsgi_app()) self.assertEqual(400, response.status_int) - def test_create_backup_no_name_with_rotation(self): - """Name isn't required for backups, but rotation is. + def test_create_backup_no_name(self): + """Name is also required for backups""" + body = dict(image=dict(serverId='123', image_type='backup', + backup_type='daily', rotation=1)) + req = webob.Request.blank('/v1.0/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, response.status_int) - The reason name isn't required is because it defaults to the - image_type. + def test_create_backup_with_rotation_and_backup_type(self): + """The happy path for creating backups Creating a backup is an admin-only operation, as opposed to snapshots which are available to anybody. @@ -1009,8 +1013,9 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): FLAGS.allow_admin_api = True # FIXME(sirp): should the fact that backups are admin_only be a FLAG - body = dict(image=dict(serverId='123', image_type='daily', - rotation=1)) + body = dict(image=dict(serverId='123', image_type='backup', + name='Backup 1', + backup_type='daily', rotation=1)) req = webob.Request.blank('/v1.0/images') req.method = 'POST' req.body = json.dumps(body) @@ -1024,7 +1029,23 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): FLAGS.allow_admin_api = True # FIXME(sirp): should the fact that backups are admin_only be a FLAG - body = dict(image=dict(serverId='123', image_type='daily')) + body = dict(image=dict(serverId='123', name='daily', + image_type='backup', backup_type='daily')) + req = webob.Request.blank('/v1.0/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, response.status_int) + + def test_create_backup_no_backup_type(self): + """Backup Type (daily or weekly) is required for backup requests""" + # FIXME(sirp): teardown needed? + FLAGS.allow_admin_api = True + + # FIXME(sirp): should the fact that backups are admin_only be a FLAG + body = dict(image=dict(serverId='123', name='daily', + image_type='backup', rotation=1)) req = webob.Request.blank('/v1.0/images') req.method = 'POST' req.body = json.dumps(body) -- cgit From a1b9aea9d12eaa32f869e5a4a59b01788e6c836d Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 15:04:34 -0500 Subject: PEP8 cleanup. --- nova/compute/api.py | 2 +- nova/compute/manager.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'nova') diff --git a/nova/compute/api.py b/nova/compute/api.py index 9c6f0ef9d..efd6d166b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -719,7 +719,7 @@ class API(base.Base): :param instance_id: nova.db.sqlalchemy.models.Instance.Id :param name: name of the backup or snapshot - + :returns: A dict containing image metadata """ return self._create_image(context, instance_id, name, 'snapshot') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 1458ea41f..d4e1d3a1e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -506,7 +506,7 @@ class ComputeManager(manager.SchedulerDependentManager): 'expected: %(running)s)') % locals()) self.driver.snapshot(instance_ref, image_id) - + if image_type == 'snapshot' and rotation: raise exception.ImageRotationNotAllowed elif image_type == 'backup': @@ -519,7 +519,6 @@ class ComputeManager(manager.SchedulerDependentManager): else: raise Exception(_('Image type not recognized %s') % image_type) - def rotate_backups(self, context, instance_uuid, backup_type, rotation): """Delete excess backups associated to an instance. -- cgit From 3b85d8080ee06436873bd2e4d8f358e4686da1bf Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 15:18:05 -0500 Subject: Fixed snapshot logic. --- nova/compute/manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index d4e1d3a1e..8708768fb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -507,8 +507,9 @@ class ComputeManager(manager.SchedulerDependentManager): self.driver.snapshot(instance_ref, image_id) - if image_type == 'snapshot' and rotation: - raise exception.ImageRotationNotAllowed + if image_type == 'snapshot': + if rotation: + raise exception.ImageRotationNotAllowed elif image_type == 'backup': if rotation: instance_uuid = instance_ref['uuid'] -- cgit From 707c64ba5cb86ae3fc72d7bdc64070d9e562d96b Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Fri, 24 Jun 2011 17:19:32 -0500 Subject: PEP8 cleanup. --- .../migrate_repo/versions/027_add_provider_firewall_rules.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py b/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py index 5aa30f7a8..7e51d93b7 100644 --- a/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py +++ b/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py @@ -58,8 +58,7 @@ provider_fw_rules = Table('provider_fw_rules', meta, Column('to_port', Integer()), Column('cidr', String(length=255, convert_unicode=False, assert_unicode=None, - unicode_error=None, _warn_on_bytestring=False)) - ) + unicode_error=None, _warn_on_bytestring=False))) def upgrade(migrate_engine): -- cgit From 883992df19441544deb9aa5f60f2a77ab1f46567 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Mon, 27 Jun 2011 16:50:17 -0500 Subject: Review feedback. --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9d71ff922..156b197e1 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -523,14 +523,14 @@ class ComputeManager(manager.SchedulerDependentManager): if image_type == 'snapshot': if rotation: - raise exception.ImageRotationNotAllowed + raise exception.ImageRotationNotAllowed() elif image_type == 'backup': if rotation: instance_uuid = instance_ref['uuid'] self.rotate_backups(context, instance_uuid, backup_type, rotation) else: - raise exception.RotationRequiredForBackup + raise exception.RotationRequiredForBackup() else: raise Exception(_('Image type not recognized %s') % image_type) -- cgit From ec574986212b694bfed8109545b4b4dc578ec8f4 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Tue, 28 Jun 2011 14:49:40 -0500 Subject: Review feedback. --- nova/compute/manager.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 156b197e1..fc9a89379 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -547,15 +547,30 @@ class ComputeManager(manager.SchedulerDependentManager): :param rotation: int representing how many backups to keep around; None if rotation shouldn't be used (as in the case of snapshots) """ + # NOTE(jk0): Eventually extract this out to the ImageService? + def fetch_images(): + images = [] + offset = 0 + while True: + batch = image_service.detail(context, filters=filters, + offset=offset) + if not batch: + break + images += batch + offset += len(batch) + return images + image_service = nova.image.get_default_image_service() filters = {'property-image_type': 'backup', 'property-backup_type': backup_type, 'property-instance_uuid': instance_uuid} - images = image_service.detail(context, filters=filters) + + images = fetch_images() num_images = len(images) LOG.debug(_("Found %(num_images)d images (rotation: %(rotation)d)" % locals())) if num_images > rotation: + # TODO(jk0): Use db-level sorting in glance when it hits trunk. # Sort oldest (by created_at) to end of list images.sort(key=itemgetter('created_at'), reverse=True) -- cgit From ee2eb1f712a87e73832618be6b79f74301d74a41 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Tue, 28 Jun 2011 14:58:34 -0500 Subject: Whoops. --- nova/compute/manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index fc9a89379..6a7bb73cb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -550,14 +550,14 @@ class ComputeManager(manager.SchedulerDependentManager): # NOTE(jk0): Eventually extract this out to the ImageService? def fetch_images(): images = [] - offset = 0 + marker = 0 while True: batch = image_service.detail(context, filters=filters, - offset=offset) + marker=marker) if not batch: break images += batch - offset += len(batch) + marker += len(batch) return images image_service = nova.image.get_default_image_service() -- cgit From ec1afee8399818db2ba11952a61c924da73f57a0 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Tue, 28 Jun 2011 15:17:23 -0500 Subject: OOPS --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6a7bb73cb..fdb231e9e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -550,14 +550,14 @@ class ComputeManager(manager.SchedulerDependentManager): # NOTE(jk0): Eventually extract this out to the ImageService? def fetch_images(): images = [] - marker = 0 + marker = None while True: batch = image_service.detail(context, filters=filters, marker=marker) if not batch: break images += batch - marker += len(batch) + marker = batch[-1]['id'] return images image_service = nova.image.get_default_image_service() -- cgit From 2916aa40f6dc0b06217ff7d3750ecdd3bb03e4fd Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Tue, 28 Jun 2011 16:03:41 -0500 Subject: Review feedback. --- nova/api/openstack/images.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'nova') diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 44d8c94a4..7ebf58023 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -105,7 +105,8 @@ class Controller(object): try: return body["image"][param] except KeyError: - raise webob.exc.HTTPBadRequest() + raise webob.exc.HTTPBadRequest(explanation="Missing required " + "param: %s" % param) context = req.environ['nova.context'] content_type = req.get_content_type() @@ -131,7 +132,8 @@ class Controller(object): # NOTE(sirp): Unlike snapshot, backup is not a customer facing # API call; rather, it's used by the internal backup scheduler if not FLAGS.allow_admin_api: - raise webob.exc.HTTPBadRequest() + raise webob.exc.HTTPBadRequest( + explanation="Admin API Required") backup_type = get_param("backup_type") rotation = int(get_param("rotation")) @@ -141,7 +143,8 @@ class Controller(object): backup_type, rotation, extra_properties=props) else: LOG.error(_("Invalid image_type '%s' passed") % image_type) - raise webob.exc.HTTPBadRequest() + raise webob.exc.HTTPBadRequest(explanation="Invalue image_type: " + "%s" % image_type) return dict(image=self.get_builder(req).build(image, detail=True)) -- cgit From d0ff8a737111e9155fd59816afa5c4fc2b34bb4c Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Tue, 28 Jun 2011 16:54:25 -0500 Subject: Let glance handle sorting. --- nova/compute/manager.py | 5 ----- 1 file changed, 5 deletions(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index fdb231e9e..f81e793fe 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -43,7 +43,6 @@ import time import functools from eventlet import greenthread -from operator import itemgetter from nova import exception from nova import flags @@ -570,10 +569,6 @@ class ComputeManager(manager.SchedulerDependentManager): LOG.debug(_("Found %(num_images)d images (rotation: %(rotation)d)" % locals())) if num_images > rotation: - # TODO(jk0): Use db-level sorting in glance when it hits trunk. - # Sort oldest (by created_at) to end of list - images.sort(key=itemgetter('created_at'), reverse=True) - # NOTE(sirp): this deletes all backups that exceed the rotation # limit excess = len(images) - rotation -- cgit From 0ca902cb90ea824ef199601b65dbc52e6c713079 Mon Sep 17 00:00:00 2001 From: Josh Kearney Date: Tue, 28 Jun 2011 18:50:17 -0500 Subject: Review feedback --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 404a2176b..40a640083 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -571,7 +571,7 @@ class ComputeManager(manager.SchedulerDependentManager): marker = None while True: batch = image_service.detail(context, filters=filters, - marker=marker) + marker=marker, sort_key='created_at', sort_dir='desc') if not batch: break images += batch -- cgit From 45e5ae28377abc0eefd2e71ef553380b25283c48 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 29 Jun 2011 09:49:19 -0700 Subject: Fanout queues use unique queue names, so the consumer should have exclusive access. This means that they also get auto deleted when we're done with them, so they're not left around on a service restart. Fixes lp:803165 --- nova/rpc.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'nova') diff --git a/nova/rpc.py b/nova/rpc.py index 2e78a31e7..9f0b507fd 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -275,6 +275,11 @@ class FanoutAdapterConsumer(AdapterConsumer): unique = uuid.uuid4().hex self.queue = '%s_fanout_%s' % (topic, unique) self.durable = False + # Fanout creates unique queue names, so we should auto-remove + # them when done, so they're not left around on restart. + # Also, we're the only one that should be consuming. exclusive + # implies auto_delete, so we'll just set that.. + self.exclusive = True LOG.info(_('Created "%(exchange)s" fanout exchange ' 'with "%(key)s" routing key'), dict(exchange=self.exchange, key=self.routing_key)) -- cgit From 74c222b6b4042053cc8c2d0038f37b3f8ee8b9fc Mon Sep 17 00:00:00 2001 From: Mark Washenberger Date: Wed, 29 Jun 2011 14:52:56 -0400 Subject: don't pass zero in to glance image service if no limit or marker are present --- nova/api/openstack/common.py | 37 +++++++++++++++------------------ nova/api/openstack/images.py | 12 +++++------ nova/tests/api/openstack/test_common.py | 12 ++++++++--- nova/tests/api/openstack/test_images.py | 20 +++++++++--------- 4 files changed, 42 insertions(+), 39 deletions(-) (limited to 'nova') diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 4da7ec0ef..aa8911b62 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -45,23 +45,20 @@ def get_pagination_params(request): exc.HTTPBadRequest() exceptions to be raised. """ - try: - marker = int(request.GET.get('marker', 0)) - except ValueError: - raise webob.exc.HTTPBadRequest(_('marker param must be an integer')) - - try: - limit = int(request.GET.get('limit', 0)) - except ValueError: - raise webob.exc.HTTPBadRequest(_('limit param must be an integer')) - - if limit < 0: - raise webob.exc.HTTPBadRequest(_('limit param must be positive')) - - if marker < 0: - raise webob.exc.HTTPBadRequest(_('marker param must be positive')) - - return(marker, limit) + params = {} + for param in ['marker', 'limit']: + if not param in request.GET: + continue + try: + params[param] = int(request.GET[param]) + except ValueError: + msg = _('%s param must be an integer') % param + raise webob.exc.HTTPBadRequest(msg) + if params[param] < 0: + msg = _('%s param must be positive') % param + raise webob.exc.HTTPBadRequest(msg) + + return params def limited(items, request, max_limit=FLAGS.osapi_max_limit): @@ -100,10 +97,10 @@ def limited(items, request, max_limit=FLAGS.osapi_max_limit): def limited_by_marker(items, request, max_limit=FLAGS.osapi_max_limit): """Return a slice of items according to the requested marker and limit.""" - (marker, limit) = get_pagination_params(request) + params = get_pagination_params(request) - if limit == 0: - limit = max_limit + limit = params.get('limit', max_limit) + marker = params.get('marker') limit = min(max_limit, limit) start_index = 0 diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index d43340e10..64d003a0f 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -181,9 +181,9 @@ class ControllerV11(Controller): """ context = req.environ['nova.context'] filters = self._get_filters(req) - (marker, limit) = common.get_pagination_params(req) - images = self._image_service.index( - context, filters=filters, marker=marker, limit=limit) + page_params = common.get_pagination_params(req) + images = self._image_service.index(context, filters=filters, + **page_params) builder = self.get_builder(req).build return dict(images=[builder(image, detail=False) for image in images]) @@ -195,9 +195,9 @@ class ControllerV11(Controller): """ context = req.environ['nova.context'] filters = self._get_filters(req) - (marker, limit) = common.get_pagination_params(req) - images = self._image_service.detail( - context, filters=filters, marker=marker, limit=limit) + page_params = common.get_pagination_params(req) + images = self._image_service.detail(context, filters=filters, + **page_params) builder = self.get_builder(req).build return dict(images=[builder(image, detail=True) for image in images]) diff --git a/nova/tests/api/openstack/test_common.py b/nova/tests/api/openstack/test_common.py index 9a9d9125c..29cb8b944 100644 --- a/nova/tests/api/openstack/test_common.py +++ b/nova/tests/api/openstack/test_common.py @@ -161,12 +161,12 @@ class PaginationParamsTest(test.TestCase): def test_no_params(self): """ Test no params. """ req = Request.blank('/') - self.assertEqual(common.get_pagination_params(req), (0, 0)) + self.assertEqual(common.get_pagination_params(req), {}) def test_valid_marker(self): """ Test valid marker param. """ req = Request.blank('/?marker=1') - self.assertEqual(common.get_pagination_params(req), (1, 0)) + self.assertEqual(common.get_pagination_params(req), {'marker': 1}) def test_invalid_marker(self): """ Test invalid marker param. """ @@ -177,10 +177,16 @@ class PaginationParamsTest(test.TestCase): def test_valid_limit(self): """ Test valid limit param. """ req = Request.blank('/?limit=10') - self.assertEqual(common.get_pagination_params(req), (0, 10)) + self.assertEqual(common.get_pagination_params(req), {'limit': 10}) def test_invalid_limit(self): """ Test invalid limit param. """ req = Request.blank('/?limit=-2') self.assertRaises( webob.exc.HTTPBadRequest, common.get_pagination_params, req) + + def test_valid_limit_and_marker(self): + """ Test valid limit and marker parameters. """ + req = Request.blank('/?limit=20&marker=40') + self.assertEqual(common.get_pagination_params(req), + {'marker': 40, 'limit': 20}) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 446d68e9e..fc4fc84e2 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -802,7 +802,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'name': 'testname'} image_service.index( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images?name=testname') @@ -817,7 +817,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'status': 'ACTIVE'} image_service.index( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images?status=ACTIVE') @@ -832,7 +832,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'property-test': '3'} image_service.index( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images?property-test=3') @@ -847,7 +847,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'status': 'ACTIVE'} image_service.index( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images?status=ACTIVE&UNSUPPORTEDFILTER=testname') @@ -862,7 +862,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {} image_service.index( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images') @@ -877,7 +877,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'name': 'testname'} image_service.detail( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images/detail?name=testname') @@ -892,7 +892,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'status': 'ACTIVE'} image_service.detail( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images/detail?status=ACTIVE') @@ -907,7 +907,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'property-test': '3'} image_service.detail( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images/detail?property-test=3') @@ -922,7 +922,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {'status': 'ACTIVE'} image_service.detail( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images/detail?status=ACTIVE&UNSUPPORTEDFILTER=testname') @@ -937,7 +937,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): context = object() filters = {} image_service.detail( - context, filters=filters, marker=0, limit=0).AndReturn([]) + context, filters=filters).AndReturn([]) mocker.ReplayAll() request = webob.Request.blank( '/v1.1/images/detail') -- cgit