Merge lp:~soren/nova/image-tests into lp:~hudson-openstack/nova/trunk

[Soren Hansen]

Add a generic image service test and run it against the fake image service.

Add a generic image service test and run it against the fake image service.

I've removed a couple of things from the image dicts FakeImageService returns because they're not documented. If something needs them, we should update the documentation and make sure all the other drivers return them as well.

Right now, I just run these tests against the fake image service, but I want to run them against the other ones as well to make sure they all actually behave identically.

[lp:~soren/nova/image-tests]

1381. By Matt Dietz on 2011-08-05

Fixes lp821144

Revert resize broken because an incorrect number of parameters and a bad call at the virt layer

[Josh Kearney (jk0)]

I'm game.

review: Approve

[lp:~soren/nova/image-tests]

1382. By Dan Prince on 2011-08-05

Add exception logging for instance IDs in the __public_instance_is_accessible smoke test function. This should help troubleshoot an intermittent failure.

1383. By Gabe Westmaas on 2011-08-05

Updates v1.1 servers/id/action requests to comply with the 1.1 spec

1384. By Justin Shepherd on 2011-08-05

Implemented @test.skip_unless and @test.skip_if functionality in nova/test.py.

Added nova/tests/test_skip_examples.py which contains example skip case usages.

[Devin Carlen (devcamcar)]

good stuff!

review: Approve

[lp:~soren/nova/image-tests]

1385. By Soren Hansen on 2011-08-05

Add a generic image service test and run it against the fake image service.

=== modified file 'nova/image/fake.py'
--- nova/image/fake.py	2011-07-08 09:35:01 +0000
+++ nova/image/fake.py	2011-08-05 13:46:49 +0000
@@ -45,9 +45,12 @@
                  'name': 'fakeimage123456',
                  'created_at': timestamp,
                  'updated_at': timestamp,
+                 'deleted_at': None,
+                 'deleted': False,
                  'status': 'active',
-                 'container_format': 'ami',
-                 'disk_format': 'raw',
+                 'is_public': False,
+#                 'container_format': 'ami',
+#                 'disk_format': 'raw',
                  'properties': {'kernel_id': FLAGS.null_kernel,
                                 'ramdisk_id': FLAGS.null_kernel,
                                 'architecture': 'x86_64'}}
@@ -56,9 +59,12 @@
                  'name': 'fakeimage123456',
                  'created_at': timestamp,
                  'updated_at': timestamp,
+                 'deleted_at': None,
+                 'deleted': False,
                  'status': 'active',
-                 'container_format': 'ami',
-                 'disk_format': 'raw',
+                 'is_public': True,
+#                 'container_format': 'ami',
+#                 'disk_format': 'raw',
                  'properties': {'kernel_id': FLAGS.null_kernel,
                                 'ramdisk_id': FLAGS.null_kernel}}
 
@@ -66,9 +72,12 @@
                  'name': 'fakeimage123456',
                  'created_at': timestamp,
                  'updated_at': timestamp,
+                 'deleted_at': None,
+                 'deleted': False,
                  'status': 'active',
-                 'container_format': 'ami',
-                 'disk_format': 'raw',
+                 'is_public': True,
+#                 'container_format': 'ami',
+#                 'disk_format': 'raw',
                  'properties': {'kernel_id': FLAGS.null_kernel,
                                 'ramdisk_id': FLAGS.null_kernel}}
 
@@ -76,9 +85,12 @@
                  'name': 'fakeimage123456',
                  'created_at': timestamp,
                  'updated_at': timestamp,
+                 'deleted_at': None,
+                 'deleted': False,
                  'status': 'active',
-                 'container_format': 'ami',
-                 'disk_format': 'raw',
+                 'is_public': True,
+#                 'container_format': 'ami',
+#                 'disk_format': 'raw',
                  'properties': {'kernel_id': FLAGS.null_kernel,
                                 'ramdisk_id': FLAGS.null_kernel}}
 
@@ -86,9 +98,12 @@
                  'name': 'fakeimage123456',
                  'created_at': timestamp,
                  'updated_at': timestamp,
+                 'deleted_at': None,
+                 'deleted': False,
                  'status': 'active',
-                 'container_format': 'ami',
-                 'disk_format': 'raw',
+                 'is_public': True,
+#                 'container_format': 'ami',
+#                 'disk_format': 'raw',
                  'properties': {'kernel_id': FLAGS.null_kernel,
                                 'ramdisk_id': FLAGS.null_kernel}}
 
@@ -101,7 +116,11 @@
 
     def index(self, context, filters=None, marker=None, limit=None):
         """Returns list of images."""
-        return copy.deepcopy(self.images.values())
+        retval = []
+        for img in self.images.values():
+            retval += [dict([(k, v) for k, v in img.iteritems()
+                                                  if k in ['id', 'name']])]
+        return retval
 
     def detail(self, context, filters=None, marker=None, limit=None):
         """Return list of detailed image information."""

=== added file 'nova/tests/test_image.py'
--- nova/tests/test_image.py	1970-01-01 00:00:00 +0000
+++ nova/tests/test_image.py	2011-08-05 13:46:49 +0000
@@ -0,0 +1,132 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+#    Copyright 2011 OpenStack LLC
+#    Author: Soren Hansen
+#
+#    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
+
+from nova import context
+from nova import exception
+from nova import test
+import nova.image
+
+class _ImageTestCase(test.TestCase):
+    def setUp(self):
+        super(_ImageTestCase, self).setUp()
+        self.context = context.get_admin_context()
+
+    def test_index(self):
+        res = self.image_service.index(self.context)
+        for image in res:
+            self.assertEquals(set(image.keys()), set(['id', 'name']))
+
+    def test_detail(self):
+        res = self.image_service.detail(self.context)
+        for image in res:
+            keys = set(image.keys())
+            self.assertEquals(keys, set(['id', 'name', 'created_at',
+                                         'updated_at', 'deleted_at', 'deleted',
+                                         'status', 'is_public', 'properties']))
+            self.assertTrue(isinstance(image['created_at'], datetime.datetime))
+            self.assertTrue(isinstance(image['updated_at'], datetime.datetime))
+
+            if not (isinstance(image['deleted_at'], datetime.datetime) or
+                                      image['deleted_at'] is None):
+                self.fail('image\'s "deleted_at" attribute was neither a '
+                          'datetime object nor None')
+
+            def check_is_bool(image, key):
+                val = image.get('deleted')
+                if not isinstance(val, bool):
+                    self.fail('image\'s "%s" attribute wasn\'t '
+                              'a bool: %r' % (key, val))
+
+            check_is_bool(image, 'deleted')
+            check_is_bool(image, 'is_public')
+
+    def test_index_and_detail_have_same_results(self):
+        index = self.image_service.index(self.context)
+        detail = self.image_service.detail(self.context)
+        index_set = set([(i['id'], i['name']) for i in index])
+        detail_set = set([(i['id'], i['name']) for i in detail])
+        self.assertEqual(index_set, detail_set)
+
+    def test_show_raises_imagenotfound_for_invalid_id(self):
+        self.assertRaises(exception.ImageNotFound,
+                          self.image_service.show,
+                          self.context,
+                          'this image does not exist')
+
+    def test_show_by_name(self):
+        self.assertRaises(exception.ImageNotFound,
+                          self.image_service.show_by_name,
+                          self.context,
+                          'this image does not exist')
+
+    def test_create_adds_id(self):
+        index = self.image_service.index(self.context)
+        image_count = len(index)
+
+        self.image_service.create(self.context, {})
+
+        index = self.image_service.index(self.context)
+        self.assertEquals(len(index), image_count + 1)
+
+        self.assertTrue(index[0]['id'])
+
+    def test_create_keeps_id(self):
+        self.image_service.create(self.context, {'id': '34'})
+        self.image_service.show(self.context, '34')
+
+    def test_create_rejects_duplicate_ids(self):
+        self.image_service.create(self.context, {'id': '34'})
+        self.assertRaises(exception.Duplicate,
+                          self.image_service.create,
+                          self.context,
+                          {'id': '34'})
+
+        # Make sure there's still one left
+        self.image_service.show(self.context, '34')
+
+    def test_update(self):
+        self.image_service.create(self.context,
+                                  {'id': '34', 'foo': 'bar'})
+
+        self.image_service.update(self.context, '34',
+                                  {'id': '34', 'foo': 'baz'})
+
+        img = self.image_service.show(self.context, '34')
+        self.assertEquals(img['foo'], 'baz')
+
+    def test_delete(self):
+        self.image_service.create(self.context, {'id': '34', 'foo': 'bar'})
+        self.image_service.delete(self.context, '34')
+        self.assertRaises(exception.NotFound,
+                          self.image_service.show,
+                          self.context,
+                          '34')
+
+    def test_delete_all(self):
+        self.image_service.create(self.context, {'id': '32', 'foo': 'bar'})
+        self.image_service.create(self.context, {'id': '33', 'foo': 'bar'})
+        self.image_service.create(self.context, {'id': '34', 'foo': 'bar'})
+        self.image_service.delete_all()
+        index = self.image_service.index(self.context)
+        self.assertEquals(len(index), 0)
+
+class FakeImageTestCase(_ImageTestCase):
+    def setUp(self):
+        super(FakeImageTestCase, self).setUp()
+        self.image_service = nova.image.fake.FakeImageService()

=== modified file 'nova/virt/libvirt/connection.py'
--- nova/virt/libvirt/connection.py	2011-08-05 02:22:13 +0000
+++ nova/virt/libvirt/connection.py	2011-08-05 13:46:49 +0000
@@ -392,9 +392,7 @@
             nova.image.get_image_service(image_href)
         snapshot = snapshot_image_service.show(context, snapshot_image_id)
 
-        metadata = {'disk_format': base['disk_format'],
-                    'container_format': base['container_format'],
-                    'is_public': False,
+        metadata = {'is_public': False,
                     'status': 'active',
                     'name': snapshot['name'],
                     'properties': {
@@ -409,6 +407,12 @@
             arch = base['properties']['architecture']
             metadata['properties']['architecture'] = arch
 
+        if 'disk_format' in base:
+            metadata['disk_format'] = base['disk_format']
+
+        if 'container_format' in base:
+            metadata['container_format'] = base['container_format']
+
         # Make the snapshot
         snapshot_name = uuid.uuid4().hex
         snapshot_xml = """