Jay Pipes has proposed merging lp:~jaypipes/glance/teller-api into lp:glance.
Requested reviews: Michael Gundlach (gundlach) Rick Harris (rconradharris) Glance Core (glance-core) Adds DELETE call to Teller API -- https://code.launchpad.net/~jaypipes/glance/teller-api/+merge/42858 Your team Glance Core is requested to review the proposed merge of lp:~jaypipes/glance/teller-api into lp:glance.
=== modified file 'glance/client.py' --- glance/client.py 2010-12-02 20:19:26 +0000 +++ glance/client.py 2010-12-06 17:36:56 +0000 @@ -167,6 +167,13 @@ res = self.do_request("GET", "/images/%s" % image_id) return res.read() + def delete_image(self, image_id): + """ + Deletes Tellers's information about an image. + """ + self.do_request("DELETE", "/images/%s" % image_id) + return True + class ParallaxClient(BaseClient): === modified file 'glance/teller/backends/__init__.py' --- glance/teller/backends/__init__.py 2010-10-11 18:28:35 +0000 +++ glance/teller/backends/__init__.py 2010-12-06 17:36:56 +0000 @@ -15,12 +15,19 @@ # License for the specific language governing permissions and limitations # under the License. +import os import urlparse +from glance.common import exception + # TODO(sirp): should this be moved out to common/utils.py ? def _file_iter(f, size): - """ Return an iterator for a file-like object """ + """ + Return an iterator for a file-like object + + :raises NotFound if the file does not exist + """ chunk = f.read(size) while chunk: yield chunk @@ -49,10 +56,35 @@ #FIXME: must prevent attacks using ".." and "." paths with opener(parsed_uri.path) as f: return _file_iter(f, cls.CHUNKSIZE) - - -def get_from_backend(uri, **kwargs): - """ Yields chunks of data from backend specified by uri """ + + @classmethod + def delete(cls, parsed_uri): + """ + Removes a file from the filesystem backend. + + :param parsed_uri: Parsed pieces of URI in form of:: + file:///path/to/filename.ext + + :raises NotFound if file does not exist + :raises NotAuthorized if cannot delete because of permissions + """ + fn = parsed_uri.path + if os.path.exists(fn): + try: + os.unlink(fn) + except OSError: + raise exception.NotAuthorized("You cannot delete file %s" % fn) + else: + raise exception.NotFound("File %s does not exist" % fn) + + +def get_backend_class(backend): + """ + Returns the backend class as designated in the + backend name + + :param backend: Name of backend to create + """ # NOTE(sirp): avoiding circular import from glance.teller.backends.http import HTTPBackend from glance.teller.backends.swift import SwiftBackend @@ -64,12 +96,29 @@ "swift": SwiftBackend } - parsed_uri = urlparse.urlparse(uri) - scheme = parsed_uri.scheme - try: - backend = BACKENDS[scheme] + return BACKENDS[backend] except KeyError: raise UnsupportedBackend("No backend found for '%s'" % scheme) - return backend.get(parsed_uri, **kwargs) + +def get_from_backend(uri, **kwargs): + """Yields chunks of data from backend specified by uri""" + + parsed_uri = urlparse.urlparse(uri) + scheme = parsed_uri.scheme + + backend_class = get_backend_class(scheme) + + return backend_class.get(parsed_uri, **kwargs) + + +def delete_from_backend(uri, **kwargs): + """Removes chunks of data from backend specified by uri""" + + parsed_uri = urlparse.urlparse(uri) + scheme = parsed_uri.scheme + + backend_class = get_backend_class(scheme) + + return backend_class.delete(parsed_uri, **kwargs) === modified file 'glance/teller/controllers.py' --- glance/teller/controllers.py 2010-12-02 20:19:26 +0000 +++ glance/teller/controllers.py 2010-12-06 17:36:56 +0000 @@ -43,18 +43,7 @@ Optionally, we can pass in 'registry' which will use a given RegistryAdapter for the request. This is useful for testing. """ - registry = req.str_GET.get('registry', 'parallax') - - try: - image = registries.lookup_by_registry(registry, id) - except registries.UnknownImageRegistry: - logging.debug("Could not find image registry: %s.", registry) - return exc.HTTPBadRequest(body="Unknown registry '%s'" % registry, - request=req, - content_type="text/plain") - except exception.NotFound: - raise exc.HTTPNotFound(body='Image not found', request=req, - content_type='text/plain') + registry, image = self.get_image_data(req, id) def image_iterator(): for file in image['files']: @@ -73,8 +62,34 @@ raise exc.HTTPNotImplemented() def delete(self, req, id): - """Delete is not currently supported """ - raise exc.HTTPNotImplemented() + """ + Deletes the image and all its chunks from the Teller service. + Note that this DOES NOT delete the image from the image + registry (Parallax or other registry service). The caller + should delete the metadata from the registry if necessary. + + :param request: The WSGI/Webob Request object + :param id: The opaque image identifier + + :raises HttpBadRequest if image registry is invalid + :raises HttpNotFound if image or any chunk is not available + :raises HttpNotAuthorized if image or any chunk is not + deleteable by the requesting user + """ + registry, image = self.get_image_data(req, id) + + try: + for file in image['files']: + backends.delete_from_backend(file['location']) + except exception.NotAuthorized: + raise exc.HTTPNotAuthorized(body='You are not authorized to ' + 'delete image chunk %s' % file, + request=req, + content_type='text/plain') + except exception.NotFound: + raise exc.HTTPNotFound(body='Image chunk %s not found' % + file, request=req, + content_type='text/plain') def create(self, req): """Create is not currently supported """ @@ -84,6 +99,33 @@ """Update is not currently supported """ raise exc.HTTPNotImplemented() + def get_image_data(self, req, id): + """ + Returns the registry used and the image metadata for a + supplied image ID and request object. + + :param req: The WSGI/Webob Request object + :param id: The opaque image identifier + + :raises HttpBadRequest if image registry is invalid + :raises HttpNotFound if image is not available + + :retval tuple with (regitry, image) + """ + registry = req.str_GET.get('registry', 'parallax') + + try: + image = registries.lookup_by_registry(registry, id) + return registry, image + except registries.UnknownImageRegistry: + logging.debug("Could not find image registry: %s.", registry) + raise exc.HTTPBadRequest(body="Unknown registry '%s'" % registry, + request=req, + content_type="text/plain") + except exception.NotFound: + raise exc.HTTPNotFound(body='Image not found', request=req, + content_type='text/plain') + class API(wsgi.Router): === modified file 'tests/stubs.py' --- tests/stubs.py 2010-12-02 20:19:26 +0000 +++ tests/stubs.py 2010-12-06 17:36:56 +0000 @@ -83,7 +83,7 @@ def stub_out_filesystem_backend(stubs): """ Stubs out the Filesystem Teller service to return fake - data from files. + gzipped image data from files. We establish a few fake images in a directory under /tmp/glance-tests and ensure that this directory contains the following files: @@ -104,12 +104,29 @@ @classmethod def get(cls, parsed_uri, expected_size, opener=None): filepath = os.path.join('/', - parsed_uri.netloc, - parsed_uri.path.strip(os.path.sep)) - f = gzip.open(filepath, 'rb') - data = f.read() - f.close() - return data + parsed_uri.netloc.lstrip('/'), + parsed_uri.path.strip(os.path.sep)) + if os.path.exists(filepath): + f = gzip.open(filepath, 'rb') + data = f.read() + f.close() + return data + else: + raise exception.NotFound("File %s does not exist" % filepath) + + @classmethod + def delete(self, parsed_uri): + filepath = os.path.join('/', + parsed_uri.netloc.lstrip('/'), + parsed_uri.path.strip(os.path.sep)) + if os.path.exists(filepath): + try: + os.unlink(filepath) + except OSError: + raise exception.NotAuthorized("You cannot delete file %s" % + filepath) + else: + raise exception.NotFound("File %s does not exist" % filepath) # Establish a clean faked filesystem with dummy images if os.path.exists(FAKE_FILESYSTEM_ROOTDIR): @@ -130,6 +147,8 @@ fake_filesystem_backend = FakeFilesystemBackend() stubs.Set(glance.teller.backends.FilesystemBackend, 'get', fake_filesystem_backend.get) + stubs.Set(glance.teller.backends.FilesystemBackend, 'delete', + fake_filesystem_backend.delete) def stub_out_swift_backend(stubs): === modified file 'tests/unit/test_clients.py' --- tests/unit/test_clients.py 2010-12-01 17:10:25 +0000 +++ tests/unit/test_clients.py 2010-12-06 17:36:56 +0000 @@ -284,6 +284,7 @@ stubs.stub_out_parallax_and_teller_server(self.stubs) stubs.stub_out_filesystem_backend(self.stubs) self.client = client.TellerClient() + self.pclient = client.ParallaxClient() def tearDown(self): """Clear the test environment""" @@ -296,3 +297,35 @@ image = self.client.get_image(2) self.assertEquals(expected, image) + + def test_get_image_not_existing(self): + """Test retrieval of a non-existing image returns a 404""" + + self.assertRaises(exception.NotFound, + self.client.get_image, + 3) + + def test_delete_image(self): + """Tests that image data is deleted properly""" + + expected = 'chunk0chunk42' + image = self.client.get_image(2) + + self.assertEquals(expected, image) + + # Delete image #2 + self.assertTrue(self.client.delete_image(2)) + + # Delete the image metadata for #2 from Parallax + self.assertTrue(self.pclient.delete_image(2)) + + self.assertRaises(exception.NotFound, + self.client.get_image, + 2) + + def test_delete_image_not_existing(self): + """Test deletion of a non-existing image returns a 404""" + + self.assertRaises(exception.NotFound, + self.client.delete_image, + 3) === modified file 'tests/unit/test_teller_api.py' --- tests/unit/test_teller_api.py 2010-12-02 20:19:26 +0000 +++ tests/unit/test_teller_api.py 2010-12-06 17:36:56 +0000 @@ -20,7 +20,8 @@ import stubout import webob -from glance.teller import controllers +from glance.teller import controllers as teller_controllers +from glance.parallax import controllers as parallax_controllers from tests import stubs @@ -31,7 +32,6 @@ stubs.stub_out_parallax_and_teller_server(self.stubs) stubs.stub_out_parallax_db_image_api(self.stubs) stubs.stub_out_filesystem_backend(self.stubs) - self.image_controller = controllers.ImageController() def tearDown(self): """Clear the test environment""" @@ -40,20 +40,46 @@ def test_index_raises_not_implemented(self): req = webob.Request.blank("/images") - res = req.get_response(controllers.API()) + res = req.get_response(teller_controllers.API()) self.assertEquals(res.status_int, webob.exc.HTTPNotImplemented.code) def test_show_image_unrecognized_registry_adapter(self): req = webob.Request.blank("/images/1?registry=unknown") - res = req.get_response(controllers.API()) + res = req.get_response(teller_controllers.API()) self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code) def test_show_image_basic(self): req = webob.Request.blank("/images/2") - res = req.get_response(controllers.API()) + res = req.get_response(teller_controllers.API()) self.assertEqual('chunk0chunk42', res.body) def test_show_non_exists_image(self): req = webob.Request.blank("/images/42") - res = req.get_response(controllers.API()) + res = req.get_response(teller_controllers.API()) + self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code) + + def test_delete_image(self): + req = webob.Request.blank("/images/2") + req.method = 'DELETE' + res = req.get_response(teller_controllers.API()) + self.assertEquals(res.status_int, 200) + + # Deletion from registry is not done from Teller on + # purpose to allow the most flexibility for migrating + # image file/chunk locations while keeping an image + # identifier stable. + req = webob.Request.blank("/images/2") + req.method = 'DELETE' + res = req.get_response(parallax_controllers.API()) + self.assertEquals(res.status_int, 200) + + req = webob.Request.blank("/images/2") + req.method = 'GET' + res = req.get_response(teller_controllers.API()) + self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code, res.body) + + def test_delete_non_exists_image(self): + req = webob.Request.blank("/images/42") + req.method = 'DELETE' + res = req.get_response(teller_controllers.API()) self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code)
_______________________________________________ Mailing list: https://launchpad.net/~registry Post to : registry@lists.launchpad.net Unsubscribe : https://launchpad.net/~registry More help : https://help.launchpad.net/ListHelp