Merge "Move volume size validation to api layer."

4 files changed, 67 insertions, 18 deletions

diff --git a/nova/api/openstack/volume/volumes.py b/nova/api/openstack/volume/volumes.py
index ec9a47a74..8c5652084 100644
--- a/nova/api/openstack/volume/volumes.py
+++ b/nova/api/openstack/volume/volumes.py
@@ -219,15 +219,7 @@ class VolumeController(object):
 
         volume = body['volume']
 
-        def as_int(s):
-            try:
-                return int(s)
-            except ValueError:
-                return s
-
-        # NOTE(eglynn): we're tolerant of non-int sizes here, as type
-        # integrity is enforced later in the creation codepath
-        size = as_int(volume['size'])
+        size = volume['size']
 
         LOG.audit(_("Create volume of %s GB"), size, context=context)
 
diff --git a/nova/tests/api/openstack/volume/test_volumes.py b/nova/tests/api/openstack/volume/test_volumes.py
index 1a51c8b67..6f9a6557b 100644
--- a/nova/tests/api/openstack/volume/test_volumes.py
+++ b/nova/tests/api/openstack/volume/test_volumes.py
@@ -42,10 +42,10 @@ class VolumeApiTest(test.TestCase):
         self.stubs.Set(volume_api.API, 'get', fakes.stub_volume_get)
         self.stubs.Set(volume_api.API, 'delete', fakes.stub_volume_delete)
 
-    def _do_test_volume_create(self, size):
+    def test_volume_create(self):
         self.stubs.Set(volume_api.API, "create", fakes.stub_volume_create)
 
-        vol = {"size": size,
+        vol = {"size": 100,
                "display_name": "Volume Test Name",
                "display_description": "Volume Test Desc",
                "availability_zone": "zone1:host1"}
@@ -71,12 +71,6 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(res.code, 200)
         self.assertTrue('location' in res.headers)
 
-    def test_volume_create_int_size(self):
-        self._do_test_volume_create(100)
-
-    def test_volume_create_str_size(self):
-        self._do_test_volume_create('100')
-
     def test_volume_creation_fails_with_bad_size(self):
         vol = {"size": '',
                "display_name": "Volume Test Name",
diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py
index 3bed09d7f..56ee53ee6 100644
--- a/nova/tests/test_volume.py
+++ b/nova/tests/test_volume.py
@@ -452,6 +452,59 @@ class VolumeTestCase(test.TestCase):
         self.assertTrue('created_at' in payload)
         self.volume.delete_volume(self.context, volume_id)
 
+    def _do_test_create_volume_with_size(self, size):
+        def fake_reserve(context, expire=None, **deltas):
+            return ["RESERVATION"]
+
+        def fake_commit(context, reservations):
+            pass
+
+        def fake_rollback(context, reservations):
+            pass
+
+        self.stubs.Set(QUOTAS, "reserve", fake_reserve)
+        self.stubs.Set(QUOTAS, "commit", fake_commit)
+        self.stubs.Set(QUOTAS, "rollback", fake_rollback)
+
+        volume_api = nova.volume.api.API()
+
+        volume = volume_api.create(self.context,
+                                   size,
+                                   'name',
+                                   'description')
+        self.assertEquals(volume['size'], int(size))
+
+    def test_create_volume_int_size(self):
+        """Test volume creation with int size."""
+        self._do_test_create_volume_with_size(2)
+
+    def test_create_volume_string_size(self):
+        """Test volume creation with string size."""
+        self._do_test_create_volume_with_size('2')
+
+    def test_create_volume_with_bad_size(self):
+        def fake_reserve(context, expire=None, **deltas):
+            return ["RESERVATION"]
+
+        def fake_commit(context, reservations):
+            pass
+
+        def fake_rollback(context, reservations):
+            pass
+
+        self.stubs.Set(QUOTAS, "reserve", fake_reserve)
+        self.stubs.Set(QUOTAS, "commit", fake_commit)
+        self.stubs.Set(QUOTAS, "rollback", fake_rollback)
+
+        volume_api = nova.volume.api.API()
+
+        self.assertRaises(exception.InvalidInput,
+                          volume_api.create,
+                          self.context,
+                          '2Gb',
+                          'name',
+                          'description')
+
 
 class DriverTestCase(test.TestCase):
     """Base Test class for Drivers."""
diff --git a/nova/volume/api.py b/nova/volume/api.py
index d98c8d6eb..bbb3149b0 100644
--- a/nova/volume/api.py
+++ b/nova/volume/api.py
@@ -86,8 +86,18 @@ class API(base.Base):
         else:
             snapshot_id = None
 
+        def as_int(s):
+            try:
+                return int(s)
+            except ValueError:
+                return s
+
+        # tolerate size as stringified int
+        size = as_int(size)
+
         if not isinstance(size, int) or size <= 0:
-            msg = _('Volume size must be an integer and greater than 0')
+            msg = (_("Volume size '%s' must be an integer and greater than 0")
+                   % size)
             raise exception.InvalidInput(reason=msg)
         try:
             reservations = QUOTAS.reserve(context, volumes=1, gigabytes=size)