Merge lp:~jaypipes/nova/glance-image-service into lp:~hudson-openstack/nova/trunk

[Jay Pipes]

Adds stubs and tests for GlanceImageService and LocalImageService.
Adds basic plumbing for ParallaxClient and TellerClient and hooks that into the GlanceImageService.

Fixes lp654843

Adds stubs and tests for GlanceImageService and LocalImageService.
Adds basic plumbing for ParallaxClient and TellerClient and hooks that into the GlanceImageService.

Fixes lp654843

[Christopher MacGown (0x44)]

Looks good to me.

review: Approve

[Rick Harris (rconradharris)]

Overall, looks excellent.

I think the Parallax get_image and get_image_metadata might need a slight modification. Currently, Parallax returns the JSON with the form dict(image=image_info) or dict(images=image_list). So, you probably want to

try:
    return json.loads(resp.read())['images']
except KeyError:
     raise ImageServiceException('Received malformed json from Parallax')

(The reason I return the json as dict(image=image_info) rather than just returning the image_info as a dict directly-- as would make sense-- is that, when we do support XML, we'll need a name for the root tag. By doing it this way, JSON and XML can be derived from the exact same dict-representation.)

A teensy correction:

186 + # TODO(jaypipes): return or raise HTTP error?
187 + return []

get_image_metadata should return None if image is not found/available.

review: Needs Fixing

[Jay Pipes (jaypipes)]

Got it, thanks Rick. Will fix up shortly.

[Jay Pipes (jaypipes)]

OK, changes made and a new unit test added...

[Rick Harris (rconradharris)]

Looks good, thanks :)

review: Approve

[lp:~jaypipes/nova/glance-image-service]

323. By Jay Pipes on 2010-10-05

Merge trunk

324. By Jay Pipes on 2010-10-05

Merge cerberus and trunk

325. By Jay Pipes on 2010-10-05

Merge overwrote import_object() load of image service.

[Michael Gundlach (gundlach)]

lgtm

review: Approve

=== modified file 'nova/api/rackspace/images.py'
--- nova/api/rackspace/images.py	2010-09-29 16:27:56 +0000
+++ nova/api/rackspace/images.py	2010-10-05 20:21:00 +0000
@@ -17,12 +17,17 @@
 
 from webob import exc
 
+from nova import flags
+from nova import utils
 from nova import wsgi
 from nova.api.rackspace import _id_translator
 import nova.api.rackspace
 import nova.image.service
 from nova.api.rackspace import faults
 
+
+FLAGS = flags.FLAGS
+
 class Controller(wsgi.Controller):
 
     _serialization_metadata = {
@@ -35,7 +40,7 @@
     }
 
     def __init__(self):
-        self._service = nova.image.service.ImageService.load()
+        self._service = utils.import_object(FLAGS.image_service)
         self._id_translator = _id_translator.RackspaceAPIIdTranslator(
                 "image", self._service.__class__.__name__)
 

=== modified file 'nova/api/rackspace/servers.py'
--- nova/api/rackspace/servers.py	2010-10-05 20:07:11 +0000
+++ nova/api/rackspace/servers.py	2010-10-05 20:21:00 +0000
@@ -42,7 +42,7 @@
 
 def _image_service():
     """ Helper method for initializing the image id translator """
-    service = nova.image.service.ImageService.load()
+    service = utils.import_object(FLAGS.image_service)
     return (service, _id_translator.RackspaceAPIIdTranslator(
             "image", service.__class__.__name__))
 

=== modified file 'nova/flags.py'
--- nova/flags.py	2010-09-29 00:53:27 +0000
+++ nova/flags.py	2010-10-05 20:21:00 +0000
@@ -222,6 +222,10 @@
 DEFINE_string('scheduler_manager', 'nova.scheduler.manager.SchedulerManager',
               'Manager for scheduler')
 
+# The service to use for image search and retrieval
+DEFINE_string('image_service', 'nova.image.service.LocalImageService',
+              'The service to use for retrieving and searching for images.')
+
 DEFINE_string('host', socket.gethostname(),
               'name of this node')
 

=== modified file 'nova/image/service.py'
--- nova/image/service.py	2010-09-13 15:53:53 +0000
+++ nova/image/service.py	2010-10-05 20:21:00 +0000
@@ -16,38 +16,215 @@
 #    under the License.
 
 import cPickle as pickle
+import httplib
+import json
+import logging
 import os.path
 import random
 import string
-
-class ImageService(object):
-    """Provides storage and retrieval of disk image objects."""
-
-    @staticmethod
-    def load():
-        """Factory method to return image service."""
-        #TODO(gundlach): read from config.
-        class_ = LocalImageService
-        return class_()
+import urlparse
+
+import webob.exc
+
+from nova import utils
+from nova import flags
+from nova import exception
+
+
+FLAGS = flags.FLAGS
+
+
+flags.DEFINE_string('glance_teller_address', 'http://127.0.0.1',
+                    'IP address or URL where Glance\'s Teller service resides')
+flags.DEFINE_string('glance_teller_port', '9191',
+                    'Port for Glance\'s Teller service')
+flags.DEFINE_string('glance_parallax_address', 'http://127.0.0.1',
+                    'IP address or URL where Glance\'s Parallax service resides')
+flags.DEFINE_string('glance_parallax_port', '9292',
+                    'Port for Glance\'s Parallax service')
+
+
+class BaseImageService(object):
+
+    """Base class for providing image search and retrieval services"""
 
     def index(self):
         """
         Return a dict from opaque image id to image data.
         """
+        raise NotImplementedError
 
     def show(self, id):
         """
         Returns a dict containing image data for the given opaque image id.
-        """
-
-
-class GlanceImageService(ImageService):
+
+        :raises NotFound if the image does not exist
+        """
+        raise NotImplementedError
+
+    def create(self, data):
+        """
+        Store the image data and return the new image id.
+
+        :raises AlreadyExists if the image already exist.
+
+        """
+        raise NotImplementedError
+
+    def update(self, image_id, data):
+        """Replace the contents of the given image with the new data.
+
+        :raises NotFound if the image does not exist.
+
+        """
+        raise NotImplementedError
+
+    def delete(self, image_id):
+        """
+        Delete the given image. 
+        
+        :raises NotFound if the image does not exist.
+        
+        """
+        raise NotImplementedError
+
+
+class TellerClient(object):
+
+    def __init__(self):
+        self.address = FLAGS.glance_teller_address
+        self.port = FLAGS.glance_teller_port
+        url = urlparse.urlparse(self.address)
+        self.netloc = url.netloc
+        self.connection_type = {'http': httplib.HTTPConnection,
+                                'https': httplib.HTTPSConnection}[url.scheme]
+
+
+class ParallaxClient(object):
+
+    def __init__(self):
+        self.address = FLAGS.glance_parallax_address
+        self.port = FLAGS.glance_parallax_port
+        url = urlparse.urlparse(self.address)
+        self.netloc = url.netloc
+        self.connection_type = {'http': httplib.HTTPConnection,
+                                'https': httplib.HTTPSConnection}[url.scheme]
+
+    def get_images(self):
+        """
+        Returns a list of image data mappings from Parallax
+        """
+        try:
+            c = self.connection_type(self.netloc, self.port)
+            c.request("GET", "images")
+            res = c.getresponse()
+            if res.status == 200:
+                # Parallax returns a JSONified dict(images=image_list)
+                data = json.loads(res.read())['images']
+                return data
+            else:
+                logging.warn("Parallax returned HTTP error %d from "
+                             "request for /images", res.status_int)
+                return []
+        finally:
+            c.close()
+
+    def get_image_metadata(self, image_id):
+        """
+        Returns a mapping of image metadata from Parallax
+        """
+        try:
+            c = self.connection_type(self.netloc, self.port)
+            c.request("GET", "images/%s" % image_id)
+            res = c.getresponse()
+            if res.status == 200:
+                # Parallax returns a JSONified dict(image=image_info)
+                data = json.loads(res.read())['image']
+                return data
+            else:
+                # TODO(jaypipes): log the error?
+                return None
+        finally:
+            c.close()
+
+    def add_image_metadata(self, image_metadata):
+        """
+        Tells parallax about an image's metadata
+        """
+        pass
+
+    def update_image_metadata(self, image_id, image_metadata):
+        """
+        Updates Parallax's information about an image
+        """
+        pass
+
+    def delete_image_metadata(self, image_id):
+        """
+        Deletes Parallax's information about an image
+        """
+        pass
+
+
+class GlanceImageService(BaseImageService):
+    
     """Provides storage and retrieval of disk image objects within Glance."""
-    # TODO(gundlach): once Glance has an API, build this.
-    pass
-
-
-class LocalImageService(ImageService):
+
+    def __init__(self):
+        self.teller = TellerClient()
+        self.parallax = ParallaxClient()
+
+    def index(self):
+        """
+        Calls out to Parallax for a list of images available
+        """
+        images = self.parallax.get_images()
+        return images
+
+    def show(self, id):
+        """
+        Returns a dict containing image data for the given opaque image id.
+        """
+        image = self.parallax.get_image_metadata(id)
+        if image:
+            return image
+        raise exception.NotFound
+
+    def create(self, data):
+        """
+        Store the image data and return the new image id.
+
+        :raises AlreadyExists if the image already exist.
+
+        """
+        return self.parallax.add_image_metadata(data)
+
+    def update(self, image_id, data):
+        """Replace the contents of the given image with the new data.
+
+        :raises NotFound if the image does not exist.
+
+        """
+        self.parallax.update_image_metadata(image_id, data)
+
+    def delete(self, image_id):
+        """
+        Delete the given image. 
+        
+        :raises NotFound if the image does not exist.
+        
+        """
+        self.parallax.delete_image_metadata(image_id)
+
+    def delete_all(self):
+        """
+        Clears out all images
+        """
+        pass
+
+
+class LocalImageService(BaseImageService):
+
     """Image service storing images to local disk."""
 
     def __init__(self):
@@ -68,7 +245,10 @@
         return [ self.show(id) for id in self._ids() ]
 
     def show(self, id):
-        return pickle.load(open(self._path_to(id))) 
+        try:
+            return pickle.load(open(self._path_to(id))) 
+        except IOError:
+            raise exception.NotFound
 
     def create(self, data):
         """
@@ -81,10 +261,23 @@
 
     def update(self, image_id, data):
         """Replace the contents of the given image with the new data."""
-        pickle.dump(data, open(self._path_to(image_id), 'w'))
+        try:
+            pickle.dump(data, open(self._path_to(image_id), 'w'))
+        except IOError:
+            raise exception.NotFound
 
     def delete(self, image_id):
         """
         Delete the given image.  Raises OSError if the image does not exist.
         """
-        os.unlink(self._path_to(image_id))
+        try:
+            os.unlink(self._path_to(image_id))
+        except IOError:
+            raise exception.NotFound
+
+    def delete_all(self):
+        """
+        Clears out all images in local directory
+        """
+        for f in os.listdir(self._path):
+            os.unlink(self._path_to(f))

=== modified file 'nova/tests/api/rackspace/fakes.py'
--- nova/tests/api/rackspace/fakes.py	2010-10-01 18:02:51 +0000
+++ nova/tests/api/rackspace/fakes.py	2010-10-05 20:21:00 +0000
@@ -1,5 +1,24 @@
+# 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 datetime
 import json
+import random
+import string
 
 import webob
 import webob.dec
@@ -7,6 +26,7 @@
 from nova import auth
 from nova import utils
 from nova import flags
+from nova import exception as exc
 import nova.api.rackspace.auth
 import nova.api.rackspace._id_translator
 from nova.image import service
@@ -105,6 +125,60 @@
     FLAGS.FAKE_subdomain = 'rs'
 
 
+def stub_out_glance(stubs):
+
+    class FakeParallaxClient:
+
+        def __init__(self):
+            self.fixtures = {}
+
+        def fake_get_images(self):
+            return self.fixtures
+
+        def fake_get_image_metadata(self, image_id):
+            for k, f in self.fixtures.iteritems():
+                if k == image_id:
+                    return f
+            return None
+
+        def fake_add_image_metadata(self, image_data):
+            id = ''.join(random.choice(string.letters) for _ in range(20))
+            image_data['id'] = id
+            self.fixtures[id] = image_data
+            return id
+
+        def fake_update_image_metadata(self, image_id, image_data):
+            
+            if image_id not in self.fixtures.keys():
+                raise exc.NotFound
+
+            self.fixtures[image_id].update(image_data)
+
+        def fake_delete_image_metadata(self, image_id):
+            
+            if image_id not in self.fixtures.keys():
+                raise exc.NotFound
+
+            del self.fixtures[image_id]
+
+        def fake_delete_all(self):
+            self.fixtures = {}
+
+    fake_parallax_client = FakeParallaxClient()
+    stubs.Set(nova.image.service.ParallaxClient, 'get_images',
+              fake_parallax_client.fake_get_images)
+    stubs.Set(nova.image.service.ParallaxClient, 'get_image_metadata',
+              fake_parallax_client.fake_get_image_metadata)
+    stubs.Set(nova.image.service.ParallaxClient, 'add_image_metadata',
+              fake_parallax_client.fake_add_image_metadata)
+    stubs.Set(nova.image.service.ParallaxClient, 'update_image_metadata',
+              fake_parallax_client.fake_update_image_metadata)
+    stubs.Set(nova.image.service.ParallaxClient, 'delete_image_metadata',
+              fake_parallax_client.fake_delete_image_metadata)
+    stubs.Set(nova.image.service.GlanceImageService, 'delete_all',
+              fake_parallax_client.fake_delete_all)
+
+
 class FakeAuthDatabase(object):
     data = {}
 

=== modified file 'nova/tests/api/rackspace/test_images.py'
--- nova/tests/api/rackspace/test_images.py	2010-10-01 18:02:51 +0000
+++ nova/tests/api/rackspace/test_images.py	2010-10-05 20:21:00 +0000
@@ -15,25 +15,127 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import logging
 import unittest
 
 import stubout
 
+from nova import exception
+from nova import utils
 from nova.api.rackspace import images
-
-
-class ImagesTest(unittest.TestCase):
-    def setUp(self):
-        self.stubs = stubout.StubOutForTesting()
-
-    def tearDown(self):
-        self.stubs.UnsetAll()
-
-    def test_get_image_list(self):
-        pass
-
-    def test_delete_image(self):
-        pass
-    
-    def test_create_image(self):
-        pass
+from nova.tests.api.rackspace import fakes
+
+
+class BaseImageServiceTests():
+
+    """Tasks to test for all image services"""
+
+    def test_create(self):
+
+        fixture = {'name': 'test image',
+                   'updated': None,
+                   'created': None,
+                   'status': None,
+                   'serverId': None,
+                   'progress': None}
+
+        num_images = len(self.service.index())
+
+        id = self.service.create(fixture)
+
+        self.assertNotEquals(None, id)
+        self.assertEquals(num_images + 1, len(self.service.index()))
+
+    def test_create_and_show_non_existing_image(self):
+
+        fixture = {'name': 'test image',
+                   'updated': None,
+                   'created': None,
+                   'status': None,
+                   'serverId': None,
+                   'progress': None}
+
+        num_images = len(self.service.index())
+
+        id = self.service.create(fixture)
+
+        self.assertNotEquals(None, id)
+
+        self.assertRaises(exception.NotFound,
+                          self.service.show,
+                          'bad image id')
+
+    def test_update(self):
+
+        fixture = {'name': 'test image',
+                   'updated': None,
+                   'created': None,
+                   'status': None,
+                   'serverId': None,
+                   'progress': None}
+
+        id = self.service.create(fixture)
+
+        fixture['status'] = 'in progress'
+        
+        self.service.update(id, fixture)
+        new_image_data = self.service.show(id)
+        self.assertEquals('in progress', new_image_data['status'])
+
+    def test_delete(self):
+
+        fixtures = [
+                    {'name': 'test image 1',
+                     'updated': None,
+                     'created': None,
+                     'status': None,
+                     'serverId': None,
+                     'progress': None},
+                    {'name': 'test image 2',
+                     'updated': None,
+                     'created': None,
+                     'status': None,
+                     'serverId': None,
+                     'progress': None}]
+
+        ids = []
+        for fixture in fixtures:
+            new_id = self.service.create(fixture)
+            ids.append(new_id)
+
+        num_images = len(self.service.index())
+        self.assertEquals(2, num_images)
+        
+        self.service.delete(ids[0])
+
+        num_images = len(self.service.index())
+        self.assertEquals(1, num_images)
+
+
+class LocalImageServiceTest(unittest.TestCase,
+                            BaseImageServiceTests):
+
+    """Tests the local image service"""
+
+    def setUp(self):
+        self.stubs = stubout.StubOutForTesting()
+        self.service = utils.import_object('nova.image.service.LocalImageService')
+
+    def tearDown(self):
+        self.service.delete_all()
+        self.stubs.UnsetAll()
+
+
+class GlanceImageServiceTest(unittest.TestCase,
+                             BaseImageServiceTests):
+
+    """Tests the local image service"""
+
+    def setUp(self):
+        self.stubs = stubout.StubOutForTesting()
+        fakes.stub_out_glance(self.stubs)
+        self.service = utils.import_object('nova.image.service.GlanceImageService')
+
+    def tearDown(self):
+        self.service.delete_all()
+        self.stubs.UnsetAll()

=== modified file 'nova/tests/api/rackspace/test_servers.py'
--- nova/tests/api/rackspace/test_servers.py	2010-10-05 19:37:15 +0000
+++ nova/tests/api/rackspace/test_servers.py	2010-10-05 20:21:00 +0000
@@ -33,6 +33,7 @@
 
 FLAGS = flags.FLAGS
 
+FLAGS.verbose = True
 
 def return_server(context, id):
     return stub_instance(id)