From d59d1f1ca10233fd18fb5ad7ebade0e58dd88c0b Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Tue, 12 Oct 2010 14:01:12 -0400 Subject: [PATCH 1/3] Implements Parallax API call to register a new image * Adds unit test for Parallax API controller * Adds stubouts for glance.parallax.db.sqlalchemy.api calls regarding images * Adds --logging-clear-handlers arg to nosetests in run_tests.sh to prevent extra output in running tests when tests complete successfully --- glance/common/wsgi.py | 1 - glance/parallax/controllers.py | 44 +++++++--- glance/parallax/db/sqlalchemy/api.py | 2 +- glance/parallax/db/sqlalchemy/models.py | 28 +++---- glance/teller/controllers.py | 2 +- run_tests.sh | 8 +- tests/stubs.py | 79 ++++++++++++++++++ tests/unit/test_parallax_api.py | 106 ++++++++++++++++++++++++ 8 files changed, 237 insertions(+), 33 deletions(-) create mode 100644 tests/unit/test_parallax_api.py diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index edd0923411..74d0e1cf53 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -224,7 +224,6 @@ def _serialize(self, data, request): Uses self._serialization_metadata if it exists, which is a dict mapping MIME types to information needed to serialize to that type. """ - # FIXME(sirp): type(self) should just be `self` _metadata = getattr(type(self), "_serialization_metadata", {}) serializer = Serializer(request.environ, _metadata) return serializer.to_content_type(data) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index 790dd0d08f..8b6f32946d 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -18,15 +18,20 @@ Parllax Image controller """ +import json import routes +from webob import exc + from glance.common import wsgi from glance.common import exception from glance.parallax import db -from webob import exc class ImageController(wsgi.Controller): """Image Controller """ + + def __init__(self): + super(ImageController, self).__init__() def index(self, req): """Return data for all public, non-deleted images """ @@ -52,8 +57,25 @@ def delete(self, req, id): raise exc.HTTPNotImplemented() def create(self, req): - """Create is not currently supported """ - raise exc.HTTPNotImplemented() + """Registers a new image with the registry. + + :param req: Request body. A JSON-ified dict of information about + the image. + + :retval Returns the newly-created image information as a mapping, + which will include the newly-created image's internal id + in the 'id' field + + """ + image_data = json.loads(req.body) + + # Ensure the image has a status set + if 'status' not in image_data.keys(): + image_data['status'] = 'available' + + context = None + new_image = db.image_create(context, image_data) + return dict(new_image) def update(self, req, id): """Update is not currently supported """ @@ -64,23 +86,23 @@ def _make_image_dict(image): """ Create a dict represenation of an image which we can use to serialize the image. """ - def _fetch_attrs(obj, attrs): - return dict([(a, getattr(obj, a)) for a in attrs]) + def _fetch_attrs(d, attrs): + return dict([(a, d[a]) for a in attrs]) # attributes common to all models base_attrs = set(['id', 'created_at', 'updated_at', 'deleted_at', 'deleted']) file_attrs = base_attrs | set(['location', 'size']) - files = [_fetch_attrs(f, file_attrs) for f in image.files] + files = [_fetch_attrs(f, file_attrs) for f in image['files']] # TODO(sirp): should this be a dict, or a list of dicts? # A plain dict is more convenient, but list of dicts would provide # access to created_at, etc - metadata = dict((m.key, m.value) for m in image.metadata - if not m.deleted) + metadata = dict((m['key'], m['value']) for m in image['metadata'] + if not m['deleted']) - image_attrs = base_attrs | set(['name', 'image_type', 'state', 'public']) + image_attrs = base_attrs | set(['name', 'image_type', 'status', 'is_public']) image_dict = _fetch_attrs(image, image_attrs) image_dict['files'] = files @@ -94,6 +116,6 @@ class API(wsgi.Router): def __init__(self): # TODO(sirp): should we add back the middleware for parallax? mapper = routes.Mapper() - mapper.resource("image", "images", controller=ImageController(), - collection={'detail': 'GET'}) + mapper.resource("image", "images", controller=ImageController()) + mapper.connect("/", controller=ImageController(), action="index") super(API, self).__init__(mapper) diff --git a/glance/parallax/db/sqlalchemy/api.py b/glance/parallax/db/sqlalchemy/api.py index 26ba09889f..9cca2c966e 100644 --- a/glance/parallax/db/sqlalchemy/api.py +++ b/glance/parallax/db/sqlalchemy/api.py @@ -95,7 +95,7 @@ def image_get_all_public(context, public): ).options(joinedload(models.Image.files) ).options(joinedload(models.Image.metadata) ).filter_by(deleted=_deleted(context) - ).filter_by(public=public + ).filter_by(is_public=public ).all() diff --git a/glance/parallax/db/sqlalchemy/models.py b/glance/parallax/db/sqlalchemy/models.py index c637fe5d36..c81fb903aa 100644 --- a/glance/parallax/db/sqlalchemy/models.py +++ b/glance/parallax/db/sqlalchemy/models.py @@ -23,16 +23,13 @@ import sys import datetime -# TODO(vish): clean up these imports -from sqlalchemy.orm import relationship, backref, exc, object_mapper +from sqlalchemy.orm import relationship, backref, exc, object_mapper, validates from sqlalchemy import Column, Integer, String from sqlalchemy import ForeignKey, DateTime, Boolean, Text from sqlalchemy.ext.declarative import declarative_base from glance.common.db.sqlalchemy.session import get_session -# FIXME(sirp): confirm this is not needed -#from common import auth from glance.common import exception from glance.common import flags @@ -40,6 +37,7 @@ BASE = declarative_base() + #TODO(sirp): ModelBase should be moved out so Glance and Nova can share it class ModelBase(object): """Base class for Nova and Glance Models""" @@ -128,18 +126,18 @@ class Image(BASE, ModelBase): id = Column(Integer, primary_key=True) name = Column(String(255)) - image_type = Column(String(255)) - state = Column(String(255)) - public = Column(Boolean, default=False) + image_type = Column(String(30)) + status = Column(String(30)) + is_public = Column(Boolean, default=False) - #@validates('image_type') - #def validate_image_type(self, key, image_type): - # assert(image_type in ('machine', 'kernel', 'ramdisk', 'raw')) - # - #@validates('state') - #def validate_state(self, key, state): - # assert(state in ('available', 'pending', 'disabled')) - # + @validates('image_type') + def validate_image_type(self, key, image_type): + assert(image_type in ('machine', 'kernel', 'ramdisk', 'raw')) + + @validates('status') + def validate_status(self, key, state): + assert(state in ('available', 'pending', 'disabled')) + # TODO(sirp): should these be stored as metadata? #user_id = Column(String(255)) #project_id = Column(String(255)) diff --git a/glance/teller/controllers.py b/glance/teller/controllers.py index 2052f25e48..e13c1bf01a 100644 --- a/glance/teller/controllers.py +++ b/glance/teller/controllers.py @@ -95,7 +95,7 @@ def update(self, req, id): class API(wsgi.Router): - """WSGI entry point for all Parallax requests.""" + """WSGI entry point for all Teller requests.""" def __init__(self): mapper = routes.Mapper() diff --git a/run_tests.sh b/run_tests.sh index 4277613edd..85911529c7 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -41,12 +41,12 @@ process_options $options if [ $never_venv -eq 1 ]; then # Just run the test suites in current environment - python run_tests.py + nosetests --logging-clear-handlers exit fi if [ -e ${venv} ]; then - ${with_venv} nosetests + ${with_venv} nosetests --logging-clear-handlers else if [ $always_venv -eq 1 ]; then # Automatically install the virtualenv @@ -58,9 +58,9 @@ else # Install the virtualenv and run the test suite in it python tools/install_venv.py else - nosetests + nosetests --logging-clear-handlers exit fi fi - ${with_venv} nosetests + ${with_venv} nosetests --logging-clear-handlers fi diff --git a/tests/stubs.py b/tests/stubs.py index 0ba81f39ec..7fcee3df1f 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -17,12 +17,14 @@ """Stubouts, mocks and fixtures for the test suite""" +import datetime import httplib import StringIO import stubout import glance.teller.backends.swift +import glance.parallax.db.sqlalchemy.api def stub_out_http_backend(stubs): """Stubs out the httplib.HTTPRequest.getresponse to return @@ -147,3 +149,80 @@ def lookup(cls, _parsed_uri): fake_parallax_registry = FakeParallax() stubs.Set(glance.teller.registries.Parallax, 'lookup', fake_parallax_registry.lookup) + + +def stub_out_parallax_db_image_api(stubs): + """Stubs out the database set/fetch API calls for Parallax + so the calls are routed to an in-memory dict. This helps us + avoid having to manually clear or flush the SQLite database. + + The "datastore" always starts with this set of image fixtures. + + :param stubs: Set of stubout stubs + + """ + class FakeDatastore(object): + + FIXTURES = [ + {'id': 1, + 'name': 'fake image #1', + 'status': 'available', + 'image_type': 'kernel', + 'is_public': False, + 'created_at': datetime.datetime.utcnow(), + 'updated_at': datetime.datetime.utcnow(), + 'deleted_at': None, + 'deleted': False, + 'files': [], + 'metadata': []}, + {'id': 2, + 'name': 'fake image #2', + 'status': 'available', + 'image_type': 'kernel', + 'is_public': True, + 'created_at': datetime.datetime.utcnow(), + 'updated_at': datetime.datetime.utcnow(), + 'deleted_at': None, + 'deleted': False, + 'files': [], + 'metadata': []}] + + def __init__(self): + self.images = self.FIXTURES + self.next_id = 3 + + def image_create(self, _context, values): + values['id'] = self.next_id + if 'status' not in values.keys(): + values['status'] = 'available' + self.next_id += 1 + self.images.extend(values) + return values + + def image_destroy(self, _context, image_id): + try: + del self.images[image_id] + except KeyError: + new_exc = exception.NotFound("No model for id %s" % image_id) + raise new_exc.__class__, new_exc, sys.exc_info()[2] + + def image_get(self, _context, image_id): + if image_id not in self.images.keys() or self.images[image_id]['deleted']: + new_exc = exception.NotFound("No model for id %s" % image_id) + raise new_exc.__class__, new_exc, sys.exc_info()[2] + else: + return self.images[image_id] + + def image_get_all_public(self, _context, public): + return [f for f in self.images + if f['is_public'] == public] + + fake_datastore = FakeDatastore() + stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_create', + fake_datastore.image_create) + stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_destroy', + fake_datastore.image_destroy) + stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_get', + fake_datastore.image_get) + stubs.Set(glance.parallax.db.sqlalchemy.api, 'image_get_all_public', + fake_datastore.image_get_all_public) diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py new file mode 100644 index 0000000000..523e8e953a --- /dev/null +++ b/tests/unit/test_parallax_api.py @@ -0,0 +1,106 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2010 OpenStack, LLC +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import json +import stubout +import unittest +import webob + +from glance.parallax import controllers +from glance.parallax import db +from tests import stubs + + +class TestImageController(unittest.TestCase): + def setUp(self): + """Establish a clean test environment""" + self.stubs = stubout.StubOutForTesting() + self.image_controller = controllers.ImageController() + stubs.stub_out_parallax_db_image_api(self.stubs) + + def tearDown(self): + """Clear the test environment""" + self.stubs.UnsetAll() + + def test_get_root(self): + """Tests that the root parallax API returns "index", + which is a list of public images + + """ + fixture = {'id': 2, + 'name': 'fake image #2', + 'is_public': True, + 'image_type': 'kernel', + 'status': 'available' + } + req = webob.Request.blank('/') + res = req.get_response(controllers.API()) + res_dict = json.loads(res.body) + self.assertEquals(res.status_int, 200) + + images = res_dict['images'] + self.assertEquals(len(images), 1) + + for k,v in fixture.iteritems(): + self.assertEquals(v, images[0][k]) + + def test_get_images(self): + """Tests that the /images parallax API returns list of + public images + + """ + fixture = {'id': 2, + 'name': 'fake image #2', + 'is_public': True, + 'image_type': 'kernel', + 'status': 'available' + } + req = webob.Request.blank('/images') + res = req.get_response(controllers.API()) + res_dict = json.loads(res.body) + self.assertEquals(res.status_int, 200) + + images = res_dict['images'] + self.assertEquals(len(images), 1) + + for k,v in fixture.iteritems(): + self.assertEquals(v, images[0][k]) + + def test_create_image(self): + """Tests that the /images POST parallax API creates the image""" + fixture = {'id': 3, + 'name': 'fake public image', + 'is_public': True, + 'image_type': 'kernel' + } + + req = webob.Request.blank('/images') + + req.method = 'POST' + req.body = json.dumps(fixture) + + res = req.get_response(controllers.API()) + + self.assertEquals(res.status_int, 200) + + res_dict = json.loads(res.body) + + for k,v in fixture.iteritems(): + self.assertEquals(v, res_dict[k]) + + # Test status was updated properly + self.assertEquals('available', res_dict['status']) From bee23da1c69c94021fc0d72d0933ad9a144ec9e9 Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Tue, 12 Oct 2010 14:26:02 -0400 Subject: [PATCH 2/3] Adds tests for bad status set on image --- glance/parallax/db/sqlalchemy/models.py | 8 ++++++-- tests/stubs.py | 8 ++++++++ tests/unit/test_parallax_api.py | 26 +++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/glance/parallax/db/sqlalchemy/models.py b/glance/parallax/db/sqlalchemy/models.py index c81fb903aa..5b4e75116b 100644 --- a/glance/parallax/db/sqlalchemy/models.py +++ b/glance/parallax/db/sqlalchemy/models.py @@ -132,11 +132,15 @@ class Image(BASE, ModelBase): @validates('image_type') def validate_image_type(self, key, image_type): - assert(image_type in ('machine', 'kernel', 'ramdisk', 'raw')) + if not image_type in ('machine', 'kernel', 'ramdisk', 'raw'): + raise exception.Invalid("Invalid image type '%s' for image." % image_type) + return image_type @validates('status') def validate_status(self, key, state): - assert(state in ('available', 'pending', 'disabled')) + if not state in ('available', 'pending', 'disabled'): + raise exception.Invalid("Invalid status '%s' for image." % status) + return image_type # TODO(sirp): should these be stored as metadata? #user_id = Column(String(255)) diff --git a/tests/stubs.py b/tests/stubs.py index 7fcee3df1f..b22c54f948 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -23,6 +23,7 @@ import stubout +from glance.common import exception import glance.teller.backends.swift import glance.parallax.db.sqlalchemy.api @@ -187,14 +188,21 @@ class FakeDatastore(object): 'files': [], 'metadata': []}] + VALID_STATUSES = ('available', 'disabled', 'pending') + def __init__(self): self.images = self.FIXTURES self.next_id = 3 def image_create(self, _context, values): values['id'] = self.next_id + if 'status' not in values.keys(): values['status'] = 'available' + else: + if not values['status'] in self.VALID_STATUSES: + raise exception.Invalid("Invalid status '%s' for image" % values['status']) + self.next_id += 1 self.images.extend(values) return values diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py index 523e8e953a..366972eb83 100644 --- a/tests/unit/test_parallax_api.py +++ b/tests/unit/test_parallax_api.py @@ -20,6 +20,7 @@ import unittest import webob +from glance.common import exception from glance.parallax import controllers from glance.parallax import db from tests import stubs @@ -82,8 +83,7 @@ def test_get_images(self): def test_create_image(self): """Tests that the /images POST parallax API creates the image""" - fixture = {'id': 3, - 'name': 'fake public image', + fixture = {'name': 'fake public image', 'is_public': True, 'image_type': 'kernel' } @@ -102,5 +102,27 @@ def test_create_image(self): for k,v in fixture.iteritems(): self.assertEquals(v, res_dict[k]) + # Test ID auto-assigned properly + self.assertEquals(3, res_dict['id']) + # Test status was updated properly self.assertEquals('available', res_dict['status']) + + def test_create_image_with_bad_status(self): + """Tests proper exception is raised if a bad status is set""" + fixture = {'id': 3, + 'name': 'fake public image', + 'is_public': True, + 'image_type': 'kernel', + 'status': 'bad status' + } + + req = webob.Request.blank('/images') + + req.method = 'POST' + req.body = json.dumps(fixture) + + # TODO(jaypipes): Port Nova's Fault infrastructure + # over to Glance to support exception catching into + # standard HTTP errors. + self.assertRaises(exception.Invalid, req.get_response, controllers.API()) From fa66de6d9a24ef74176535ac9f6feaedb193617c Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Wed, 13 Oct 2010 13:47:05 -0400 Subject: [PATCH 3/3] Adds a /images/detail route to the Parallax controller, adds a unit test for it, and cleans up Michael's suggestions. --- glance/parallax/controllers.py | 41 +++++++++++++++++++++++++-------- tests/unit/test_parallax_api.py | 28 ++++++++++++++++------ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index 8b6f32946d..c38292c864 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -28,20 +28,41 @@ class ImageController(wsgi.Controller): - """Image Controller """ - def __init__(self): - super(ImageController, self).__init__() + """Image Controller """ def index(self, req): - """Return data for all public, non-deleted images """ + """Return basic information for all public, non-deleted images + + :param req: the Request object coming from the wsgi layer + :retval a mapping of the following form:: + + dict(images=[image_list]) + + Where image_list is a sequence of mappings:: + + {'id': image_id, 'name': image_name} + + """ images = db.image_get_all_public(None) - image_dicts = [self._make_image_dict(i) for i in images] + image_dicts = [dict(id=i['id'], name=i['name']) for i in images] return dict(images=image_dicts) def detail(self, req): - """Detail is not currently supported """ - raise exc.HTTPNotImplemented() + """Return detailed information for all public, non-deleted images + + :param req: the Request object coming from the wsgi layer + :retval a mapping of the following form:: + + dict(images=[image_list]) + + Where image_list is a sequence of mappings containing + all image model fields. + + """ + images = db.image_get_all_public(None) + image_dicts = [self._make_image_dict(i) for i in images] + return dict(images=image_dicts) def show(self, req, id): """Return data about the given image id.""" @@ -70,8 +91,7 @@ def create(self, req): image_data = json.loads(req.body) # Ensure the image has a status set - if 'status' not in image_data.keys(): - image_data['status'] = 'available' + image_data.setdefault('status', 'available') context = None new_image = db.image_create(context, image_data) @@ -116,6 +136,7 @@ class API(wsgi.Router): def __init__(self): # TODO(sirp): should we add back the middleware for parallax? mapper = routes.Mapper() - mapper.resource("image", "images", controller=ImageController()) + mapper.resource("image", "images", controller=ImageController(), + collection={'detail': 'GET'}) mapper.connect("/", controller=ImageController(), action="index") super(API, self).__init__(mapper) diff --git a/tests/unit/test_parallax_api.py b/tests/unit/test_parallax_api.py index 366972eb83..0046e57b7a 100644 --- a/tests/unit/test_parallax_api.py +++ b/tests/unit/test_parallax_api.py @@ -43,11 +43,7 @@ def test_get_root(self): """ fixture = {'id': 2, - 'name': 'fake image #2', - 'is_public': True, - 'image_type': 'kernel', - 'status': 'available' - } + 'name': 'fake image #2'} req = webob.Request.blank('/') res = req.get_response(controllers.API()) res_dict = json.loads(res.body) @@ -59,10 +55,28 @@ def test_get_root(self): for k,v in fixture.iteritems(): self.assertEquals(v, images[0][k]) - def test_get_images(self): + def test_get_index(self): """Tests that the /images parallax API returns list of public images + """ + fixture = {'id': 2, + 'name': 'fake image #2'} + req = webob.Request.blank('/images') + res = req.get_response(controllers.API()) + res_dict = json.loads(res.body) + self.assertEquals(res.status_int, 200) + + images = res_dict['images'] + self.assertEquals(len(images), 1) + + for k,v in fixture.iteritems(): + self.assertEquals(v, images[0][k]) + + def test_get_details(self): + """Tests that the /images/detail parallax API returns + a mapping containing a list of detailed image information + """ fixture = {'id': 2, 'name': 'fake image #2', @@ -70,7 +84,7 @@ def test_get_images(self): 'image_type': 'kernel', 'status': 'available' } - req = webob.Request.blank('/images') + req = webob.Request.blank('/images/detail') res = req.get_response(controllers.API()) res_dict = json.loads(res.body) self.assertEquals(res.status_int, 200)