registry team mailing list archive
-
registry team
-
Mailing list archive
-
Message #30256
[Merge] lp:~jaypipes/glance/teller-api into lp:glance
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)
Follow ups
