Merge lp:~rackspace-titan/nova/minram-mindisk into lp:~hudson-openstack/nova/trunk

[Naveed Massjouni]

This addresses Bug #819990 Add minDisk minRam to OSAPI image details. It also enforces the minram and mindisk constraints if they exist. Corresponding tests have been added.

[Brian Waldon (bcwaldon)]

Can you change the new exception names to InstanceTypeDiskTooSmall and InstanceTypeMemoryTooSmall? I think keeping the term 'flavor' limited to the OSAPI view is a good thing to do here.

It looks like we only test passing image filters through to the image service, not that the image service actually does something with them. If you see a way we can test that, I would love to see it :) It's not a blocker for my Approve, though.

review: Needs Fixing

[Naveed Massjouni (ironcamel)]

Good point. Fixed.

[Brian Waldon (bcwaldon)]

Thanks guys.

review: Approve

[Chris Behrens (cbehrens)]

Looks good. Minor thing. I'd like to see the tests that call compute_api.create() (in test_compute) destroy the instances that it created when done, otherwise stale data is left in the sqllite database. You can see examples in the other test_compute tests.

review: Needs Fixing

[Alex Meade (alex-meade)]

good point, I am on it

[Chris Behrens (cbehrens)]

Thanks!

review: Approve

[Dan Prince (dan-prince)]

Looks good.

review: Approve

[Brian Waldon (bcwaldon)]

Merged through gerrit.

Unmerged revisions

1585. By Alex Meade on 2011-09-22

Added the deleting of servers that were created in tests

1584. By Alex Meade on 2011-09-22

fixed incorrect exception names

1583. By Naveed Massjouni on 2011-09-22

Changing FlavorMemoryTooSmall/FlavorDiskTooSmall exceptions to
InstanceTypeMemoryTooSmall/InstanceTypeDiskTooSmall.

1582. By Naveed Massjouni on 2011-09-21

pep8

1581. By Naveed Massjouni on 2011-09-21

More tests.

1580. By Naveed Massjouni on 2011-09-21

Adding tests for minram/mindisk.

1579. By Naveed Massjouni on 2011-09-20

Honor the min_ram and min_disk attributes of images when creating a server

1578. By Naveed Massjouni on 2011-09-20

removing print

1577. By Naveed Massjouni on 2011-09-20

Merge from trunk

1576. By Alex Meade on 2011-09-16

Added support for filtering images with minRam and minDisk

=== modified file 'nova/api/openstack/create_instance_helper.py'
--- nova/api/openstack/create_instance_helper.py	2011-09-15 14:07:58 +0000
+++ nova/api/openstack/create_instance_helper.py	2011-09-22 19:53:25 +0000
@@ -187,6 +187,10 @@
                                   config_drive=config_drive,))
         except quota.QuotaError as error:
             self._handle_quota_error(error)
+        except exception.InstanceTypeMemoryTooSmall as error:
+            raise exc.HTTPBadRequest(explanation=unicode(error))
+        except exception.InstanceTypeDiskTooSmall as error:
+            raise exc.HTTPBadRequest(explanation=unicode(error))
         except exception.ImageNotFound as error:
             msg = _("Can not find requested image")
             raise exc.HTTPBadRequest(explanation=msg)
 
=== modified file 'nova/api/openstack/images.py'
--- nova/api/openstack/images.py	2011-09-20 21:11:49 +0000
+++ nova/api/openstack/images.py	2011-09-22 19:53:25 +0000
@@ -41,6 +41,8 @@
     'changes-since': 'changes-since',
     'server': 'property-instance_ref',
     'type': 'property-image_type',
+    'minRam': 'min_ram',
+    'minDisk': 'min_disk',
 }
 
 
@@ -239,6 +241,10 @@
             image_elem.set('status', str(image_dict['status']))
             if 'progress' in image_dict:
                 image_elem.set('progress', str(image_dict['progress']))
+            if 'minRam' in image_dict:
+                image_elem.set('minRam', str(image_dict['minRam']))
+            if 'minDisk' in image_dict:
+                image_elem.set('minDisk', str(image_dict['minDisk']))
             if 'server' in image_dict:
                 server_elem = self._create_server_node(image_dict['server'])
                 image_elem.append(server_elem)
 
=== modified file 'nova/api/openstack/schemas/v1.1/image.rng'
--- nova/api/openstack/schemas/v1.1/image.rng	2011-08-25 16:05:53 +0000
+++ nova/api/openstack/schemas/v1.1/image.rng	2011-09-22 19:53:25 +0000
@@ -9,6 +9,12 @@
     <attribute name="progress">   <text/> </attribute>
   </optional>
   <optional>
+    <attribute name="minDisk">   <text/> </attribute>
+  </optional>
+  <optional>
+    <attribute name="minRam">   <text/> </attribute>
+  </optional>
+  <optional>
     <element name="server">
       <attribute name="id"> <text/> </attribute>
       <zeroOrMore>
 
=== modified file 'nova/api/openstack/views/images.py'
--- nova/api/openstack/views/images.py	2011-09-20 20:21:06 +0000
+++ nova/api/openstack/views/images.py	2011-09-22 19:53:25 +0000
@@ -161,6 +161,11 @@
 
         if detail:
             image["metadata"] = image_obj.get("properties", {})
+            if 'min_ram' in image_obj:
+                image["minRam"] = image_obj.get("min_ram") or 0
+
+            if 'min_disk' in image_obj:
+                image["minDisk"] = image_obj.get("min_disk") or 0
 
         return image
 
 
=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-09-21 21:00:53 +0000
+++ nova/compute/api.py	2011-09-22 19:53:25 +0000
@@ -219,6 +219,11 @@
                                                                  image_href)
         image = image_service.show(context, image_id)
 
+        if instance_type['memory_mb'] < int(image.get('min_ram', 0)):
+            raise exception.InstanceTypeMemoryTooSmall()
+        if instance_type['local_gb'] < int(image.get('min_disk', 0)):
+            raise exception.InstanceTypeDiskTooSmall()
+
         config_drive_id = None
         if config_drive and config_drive is not True:
             # config_drive is volume id
 
=== modified file 'nova/exception.py'
--- nova/exception.py	2011-09-16 02:53:42 +0000
+++ nova/exception.py	2011-09-22 19:53:25 +0000
@@ -810,3 +810,11 @@
         if message is None:
             message = _("1 or more Zones could not complete the request")
         super(ZoneRequestError, self).__init__(message=message)
+
+
+class InstanceTypeMemoryTooSmall(NovaException):
+    message = _("Instance type's memory is too small for requested image.")
+
+
+class InstanceTypeDiskTooSmall(NovaException):
+    message = _("Instance type's disk is too small for requested image.")
 
=== modified file 'nova/image/glance.py'
--- nova/image/glance.py	2011-09-20 18:56:15 +0000
+++ nova/image/glance.py	2011-09-22 19:53:25 +0000
@@ -404,7 +404,7 @@
                         'container_format', 'checksum', 'id',
                         'name', 'created_at', 'updated_at',
                         'deleted_at', 'deleted', 'status',
-                        'is_public']
+                        'min_disk', 'min_ram', 'is_public']
     output = {}
     for attr in IMAGE_ATTRIBUTES:
         output[attr] = image_meta.get(attr)
 
=== modified file 'nova/tests/api/openstack/test_images.py'
--- nova/tests/api/openstack/test_images.py	2011-09-21 20:17:05 +0000
+++ nova/tests/api/openstack/test_images.py	2011-09-22 19:53:25 +0000
@@ -113,6 +113,7 @@
         self.assertDictMatch(expected_image, actual_image)
 
     def test_get_image_v1_1(self):
+        self.maxDiff = None
         request = webob.Request.blank('/v1.1/fake/images/124')
         app = fakes.wsgi_app(fake_auth_context=self._get_fake_context())
         response = request.get_response(app)
@@ -133,6 +134,8 @@
                 "created": NOW_API_FORMAT,
                 "status": "SAVING",
                 "progress": 0,
+                "minDisk": 0,
+                "minRam": 0,
                 'server': {
                     'id': '42',
                     "links": [{
@@ -542,6 +545,8 @@
             'created': NOW_API_FORMAT,
             'status': 'ACTIVE',
             'progress': 100,
+            'minDisk': 0,
+            'minRam': 0,
             "links": [{
                 "rel": "self",
                 "href": "http://localhost/v1.1/fake/images/123",
@@ -567,6 +572,8 @@
             'created': NOW_API_FORMAT,
             'status': 'SAVING',
             'progress': 0,
+            'minDisk': 0,
+            'minRam': 0,
             'server': {
                 'id': '42',
                 "links": [{
@@ -603,6 +610,8 @@
             'created': NOW_API_FORMAT,
             'status': 'SAVING',
             'progress': 0,
+            'minDisk': 0,
+            'minRam': 0,
             'server': {
                 'id': '42',
                 "links": [{
@@ -639,6 +648,8 @@
             'created': NOW_API_FORMAT,
             'status': 'ACTIVE',
             'progress': 100,
+            'minDisk': 0,
+            'minRam': 0,
             'server': {
                 'id': '42',
                 "links": [{
@@ -675,6 +686,8 @@
             'created': NOW_API_FORMAT,
             'status': 'ERROR',
             'progress': 0,
+            'minDisk': 0,
+            'minRam': 0,
             'server': {
                 'id': '42',
                 "links": [{
@@ -711,6 +724,8 @@
             'created': NOW_API_FORMAT,
             'status': 'DELETED',
             'progress': 0,
+            'minDisk': 0,
+            'minRam': 0,
             'server': {
                 'id': '42',
                 "links": [{
@@ -747,6 +762,8 @@
             'created': NOW_API_FORMAT,
             'status': 'DELETED',
             'progress': 0,
+            'minDisk': 0,
+            'minRam': 0,
             'server': {
                 'id': '42',
                 "links": [{
@@ -780,6 +797,8 @@
             'created': NOW_API_FORMAT,
             'status': 'ACTIVE',
             'progress': 100,
+            'minDisk': 0,
+            'minRam': 0,
             "links": [{
                 "rel": "self",
                 "href": "http://localhost/v1.1/fake/images/130",
@@ -810,6 +829,30 @@
         controller.index(request)
         self.mox.VerifyAll()
 
+    def test_image_filter_with_min_ram(self):
+        image_service = self.mox.CreateMockAnything()
+        context = self._get_fake_context()
+        filters = {'min_ram': '0'}
+        image_service.index(context, filters=filters).AndReturn([])
+        self.mox.ReplayAll()
+        request = webob.Request.blank('/v1.1/images?minRam=0')
+        request.environ['nova.context'] = context
+        controller = images.ControllerV11(image_service=image_service)
+        controller.index(request)
+        self.mox.VerifyAll()
+
+    def test_image_filter_with_min_disk(self):
+        image_service = self.mox.CreateMockAnything()
+        context = self._get_fake_context()
+        filters = {'min_disk': '7'}
+        image_service.index(context, filters=filters).AndReturn([])
+        self.mox.ReplayAll()
+        request = webob.Request.blank('/v1.1/images?minDisk=7')
+        request.environ['nova.context'] = context
+        controller = images.ControllerV11(image_service=image_service)
+        controller.index(request)
+        self.mox.VerifyAll()
+
     def test_image_filter_with_status(self):
         image_service = self.mox.CreateMockAnything()
         context = self._get_fake_context()
@@ -1369,6 +1412,152 @@
         server_root = root.find('{0}server'.format(NS))
         self.assertEqual(server_root, None)
 
+    def test_show_with_min_ram(self):
+        serializer = images.ImageXMLSerializer()
+
+        fixture = {
+            'image': {
+                'id': 1,
+                'name': 'Image1',
+                'created': self.TIMESTAMP,
+                'updated': self.TIMESTAMP,
+                'status': 'ACTIVE',
+                'progress': 80,
+                'minRam': 256,
+                'server': {
+                    'id': '1',
+                    'links': [
+                        {
+                            'href': self.SERVER_HREF,
+                            'rel': 'self',
+                        },
+                        {
+                            'href': self.SERVER_BOOKMARK,
+                            'rel': 'bookmark',
+                        },
+                    ],
+                },
+                'metadata': {
+                    'key1': 'value1',
+                },
+                'links': [
+                    {
+                        'href': self.IMAGE_HREF % 1,
+                        'rel': 'self',
+                    },
+                    {
+                        'href': self.IMAGE_BOOKMARK % 1,
+                        'rel': 'bookmark',
+                    },
+                ],
+            },
+        }
+
+        output = serializer.serialize(fixture, 'show')
+        print output
+        root = etree.XML(output)
+        xmlutil.validate_schema(root, 'image')
+        image_dict = fixture['image']
+
+        for key in ['name', 'id', 'updated', 'created', 'status', 'progress',
+                    'minRam']:
+            self.assertEqual(root.get(key), str(image_dict[key]))
+
+        link_nodes = root.findall('{0}link'.format(ATOMNS))
+        self.assertEqual(len(link_nodes), 2)
+        for i, link in enumerate(image_dict['links']):
+            for key, value in link.items():
+                self.assertEqual(link_nodes[i].get(key), value)
+
+        metadata_root = root.find('{0}metadata'.format(NS))
+        metadata_elems = metadata_root.findall('{0}meta'.format(NS))
+        self.assertEqual(len(metadata_elems), 1)
+        for i, metadata_elem in enumerate(metadata_elems):
+            (meta_key, meta_value) = image_dict['metadata'].items()[i]
+            self.assertEqual(str(metadata_elem.get('key')), str(meta_key))
+            self.assertEqual(str(metadata_elem.text).strip(), str(meta_value))
+
+        server_root = root.find('{0}server'.format(NS))
+        self.assertEqual(server_root.get('id'), image_dict['server']['id'])
+        link_nodes = server_root.findall('{0}link'.format(ATOMNS))
+        self.assertEqual(len(link_nodes), 2)
+        for i, link in enumerate(image_dict['server']['links']):
+            for key, value in link.items():
+                self.assertEqual(link_nodes[i].get(key), value)
+
+    def test_show_with_min_disk(self):
+        serializer = images.ImageXMLSerializer()
+
+        fixture = {
+            'image': {
+                'id': 1,
+                'name': 'Image1',
+                'created': self.TIMESTAMP,
+                'updated': self.TIMESTAMP,
+                'status': 'ACTIVE',
+                'progress': 80,
+                'minDisk': 5,
+                'server': {
+                    'id': '1',
+                    'links': [
+                        {
+                            'href': self.SERVER_HREF,
+                            'rel': 'self',
+                        },
+                        {
+                            'href': self.SERVER_BOOKMARK,
+                            'rel': 'bookmark',
+                        },
+                    ],
+                },
+                'metadata': {
+                    'key1': 'value1',
+                },
+                'links': [
+                    {
+                        'href': self.IMAGE_HREF % 1,
+                        'rel': 'self',
+                    },
+                    {
+                        'href': self.IMAGE_BOOKMARK % 1,
+                        'rel': 'bookmark',
+                    },
+                ],
+            },
+        }
+
+        output = serializer.serialize(fixture, 'show')
+        print output
+        root = etree.XML(output)
+        xmlutil.validate_schema(root, 'image')
+        image_dict = fixture['image']
+
+        for key in ['name', 'id', 'updated', 'created', 'status', 'progress',
+                    'minDisk']:
+            self.assertEqual(root.get(key), str(image_dict[key]))
+
+        link_nodes = root.findall('{0}link'.format(ATOMNS))
+        self.assertEqual(len(link_nodes), 2)
+        for i, link in enumerate(image_dict['links']):
+            for key, value in link.items():
+                self.assertEqual(link_nodes[i].get(key), value)
+
+        metadata_root = root.find('{0}metadata'.format(NS))
+        metadata_elems = metadata_root.findall('{0}meta'.format(NS))
+        self.assertEqual(len(metadata_elems), 1)
+        for i, metadata_elem in enumerate(metadata_elems):
+            (meta_key, meta_value) = image_dict['metadata'].items()[i]
+            self.assertEqual(str(metadata_elem.get('key')), str(meta_key))
+            self.assertEqual(str(metadata_elem.text).strip(), str(meta_value))
+
+        server_root = root.find('{0}server'.format(NS))
+        self.assertEqual(server_root.get('id'), image_dict['server']['id'])
+        link_nodes = server_root.findall('{0}link'.format(ATOMNS))
+        self.assertEqual(len(link_nodes), 2)
+        for i, link in enumerate(image_dict['server']['links']):
+            for key, value in link.items():
+                self.assertEqual(link_nodes[i].get(key), value)
+
     def test_index(self):
         serializer = images.ImageXMLSerializer()
 
 
=== modified file 'nova/tests/image/test_glance.py'
--- nova/tests/image/test_glance.py	2011-09-20 18:56:15 +0000
+++ nova/tests/image/test_glance.py	2011-09-22 19:53:25 +0000
@@ -127,6 +127,8 @@
             'name': 'test image',
             'is_public': False,
             'size': None,
+            'min_disk': None,
+            'min_ram': None,
             'location': None,
             'disk_format': None,
             'container_format': None,
@@ -157,6 +159,8 @@
             'name': 'test image',
             'is_public': False,
             'size': None,
+            'min_disk': None,
+            'min_ram': None,
             'location': None,
             'disk_format': None,
             'container_format': None,
@@ -287,6 +291,8 @@
                 'name': 'TestImage %d' % (i),
                 'properties': {},
                 'size': None,
+                'min_disk': None,
+                'min_ram': None,
                 'location': None,
                 'disk_format': None,
                 'container_format': None,
@@ -330,6 +336,8 @@
                 'name': 'TestImage %d' % (i),
                 'properties': {},
                 'size': None,
+                'min_disk': None,
+                'min_ram': None,
                 'location': None,
                 'disk_format': None,
                 'container_format': None,
@@ -382,6 +390,8 @@
             'name': 'image1',
             'is_public': True,
             'size': None,
+            'min_disk': None,
+            'min_ram': None,
             'location': None,
             'disk_format': None,
             'container_format': None,
@@ -416,6 +426,8 @@
                 'name': 'image10',
                 'is_public': True,
                 'size': None,
+                'min_disk': None,
+                'min_ram': None,
                 'location': None,
                 'disk_format': None,
                 'container_format': None,
 
=== modified file 'nova/tests/test_compute.py'
--- nova/tests/test_compute.py	2011-09-21 20:59:40 +0000
+++ nova/tests/test_compute.py	2011-09-22 19:53:25 +0000
@@ -20,6 +20,8 @@
 Tests For Compute
 """
 
+from copy import copy
+
 from nova import compute
 from nova import context
 from nova import db
@@ -1394,3 +1396,92 @@
         self.assertEqual(self.compute_api._volume_size(inst_type,
                                                        'swap'),
                          swap_size)
+
+
+class ComputeTestMinRamMinDisk(test.TestCase):
+    def setUp(self):
+        super(ComputeTestMinRamMinDisk, self).setUp()
+        self.compute = utils.import_object(FLAGS.compute_manager)
+        self.compute_api = compute.API()
+        self.context = context.RequestContext('fake', 'fake')
+        self.fake_image = {
+            'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1}}
+
+    def test_create_with_too_little_ram(self):
+        """Test an instance type with too little memory"""
+
+        inst_type = instance_types.get_default_instance_type()
+        inst_type['memory_mb'] = 1
+
+        def fake_show(*args):
+            img = copy(self.fake_image)
+            img['min_ram'] = 2
+            return img
+        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+
+        self.assertRaises(exception.InstanceTypeMemoryTooSmall,
+            self.compute_api.create, self.context, inst_type, None)
+
+        # Now increase the inst_type memory and make sure all is fine.
+        inst_type['memory_mb'] = 2
+        ref = self.compute_api.create(self.context, inst_type, None)
+        self.assertTrue(ref)
+
+        db.instance_destroy(self.context, ref[0]['id'])
+
+    def test_create_with_too_little_disk(self):
+        """Test an instance type with too little disk space"""
+
+        inst_type = instance_types.get_default_instance_type()
+        inst_type['local_gb'] = 1
+
+        def fake_show(*args):
+            img = copy(self.fake_image)
+            img['min_disk'] = 2
+            return img
+        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+
+        self.assertRaises(exception.InstanceTypeDiskTooSmall,
+            self.compute_api.create, self.context, inst_type, None)
+
+        # Now increase the inst_type disk space and make sure all is fine.
+        inst_type['local_gb'] = 2
+        ref = self.compute_api.create(self.context, inst_type, None)
+        self.assertTrue(ref)
+
+        db.instance_destroy(self.context, ref[0]['id'])
+
+    def test_create_just_enough_ram_and_disk(self):
+        """Test an instance type with just enough ram and disk space"""
+
+        inst_type = instance_types.get_default_instance_type()
+        inst_type['local_gb'] = 2
+        inst_type['memory_mb'] = 2
+
+        def fake_show(*args):
+            img = copy(self.fake_image)
+            img['min_ram'] = 2
+            img['min_disk'] = 2
+            return img
+        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+
+        ref = self.compute_api.create(self.context, inst_type, None)
+        self.assertTrue(ref)
+
+        db.instance_destroy(self.context, ref[0]['id'])
+
+    def test_create_with_no_ram_and_disk_reqs(self):
+        """Test an instance type with no min_ram or min_disk"""
+
+        inst_type = instance_types.get_default_instance_type()
+        inst_type['local_gb'] = 1
+        inst_type['memory_mb'] = 1
+
+        def fake_show(*args):
+            return copy(self.fake_image)
+        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+
+        ref = self.compute_api.create(self.context, inst_type, None)
+        self.assertTrue(ref)
+
+        db.instance_destroy(self.context, ref[0]['id'])