Add availability zone checking in the api service

This patch adds availability zone checking for api request period.
If the availability zone the request specified isn't exist in the
Cinder services, the response returned to the user will give an
error immediately.

Fixes bug: 1185714

Change-Id: I842eec7b500c3ff6a7e5d8fc5e397d61617b0eea

diff --git a/cinder/tests/api/v1/stubs.py b/cinder/tests/api/v1/stubs.py
index 72d91dbf1b83c6b427bce546bc6a60c8289f1c85..6bd658589f3995f237ca41b59ddeefb4b382ef46 100644 (file)
--- a/cinder/tests/api/v1/stubs.py
+++ b/cinder/tests/api/v1/stubs.py
@@ -129,3 +129,7 @@ def stub_snapshot_get_all_by_project(self, context):
 
 def stub_snapshot_update(self, context, *args, **param):
     pass
+
+
+def stub_service_get_all_by_topic(context, topic):
+    return [{'availability_zone': "zone1:host1"}]

diff --git a/cinder/tests/api/v1/test_volume_metadata.py b/cinder/tests/api/v1/test_volume_metadata.py
index 4dc09a874464dd10e65fe7a7c4e3005ae2fd402c..dc742858a7568d8fe4b72ace0de14b7f80597261 100644 (file)
--- a/cinder/tests/api/v1/test_volume_metadata.py
+++ b/cinder/tests/api/v1/test_volume_metadata.py
@@ -28,6 +28,7 @@ from cinder import exception
 from cinder.openstack.common import jsonutils
 from cinder import test
 from cinder.tests.api import fakes
+from cinder.tests.api.v1 import stubs
 
 
 CONF = cfg.CONF
@@ -95,6 +96,8 @@ class volumeMetaDataTest(test.TestCase):
         self.stubs.Set(cinder.db, 'volume_get', return_volume)
         self.stubs.Set(cinder.db, 'volume_metadata_get',
                        return_volume_metadata)
+        self.stubs.Set(cinder.db, 'service_get_all_by_topic',
+                       stubs.stub_service_get_all_by_topic)
 
         self.stubs.Set(self.volume_api, 'update_volume_metadata',
                        fake_update_volume_metadata)

diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py
index b199ca40e84489036e69c71d426163119d5c14d1..e72bfb59d0ef033f77ede023dd2a4f72ecd790c0 100644 (file)
--- a/cinder/tests/api/v1/test_volumes.py
+++ b/cinder/tests/api/v1/test_volumes.py
@@ -61,6 +61,8 @@ class VolumeApiTest(test.TestCase):
         self.stubs.Set(db, 'volume_get_all', stubs.stub_volume_get_all)
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(db, 'service_get_all_by_topic',
+                       stubs.stub_service_get_all_by_topic)
         self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete)
 
@@ -123,6 +125,17 @@ class VolumeApiTest(test.TestCase):
                           req,
                           body)
 
+    def test_volume_creation_fails_with_bad_availability_zone(self):
+        vol = {"size": '1',
+               "name": "Volume Test Name",
+               "description": "Volume Test Desc",
+               "availability_zone": "zonen:hostn"}
+        body = {"volume": vol}
+        req = fakes.HTTPRequest.blank('/v2/volumes')
+        self.assertRaises(exception.InvalidInput,
+                          self.controller.create,
+                          req, body)
+
     def test_volume_create_with_image_id(self):
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
         self.ext_mgr.extensions = {'os-image-create': 'fake'}

diff --git a/cinder/tests/api/v2/stubs.py b/cinder/tests/api/v2/stubs.py
index c787ef78c2068d426afbbcf9fd9cc999e7692268..d3242c7a5f12384befab63ed7af97d7fb7851ca3 100644 (file)
--- a/cinder/tests/api/v2/stubs.py
+++ b/cinder/tests/api/v2/stubs.py
@@ -131,3 +131,7 @@ def stub_snapshot_get_all_by_project(self, context):
 
 def stub_snapshot_update(self, context, *args, **param):
     pass
+
+
+def stub_service_get_all_by_topic(context, topic):
+    return [{'availability_zone': "zone1:host1"}]

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index 7264c30a4977e62c537c2cf2036ad2beae825b00..f06504662d398eb77859ac98d6368ce28fa80b41 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -65,6 +65,8 @@ class VolumeApiTest(test.TestCase):
                        stubs.stub_volume_get_all_by_project)
         self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete)
+        self.stubs.Set(db, 'service_get_all_by_topic',
+                       stubs.stub_service_get_all_by_topic)
         self.maxDiff = None
 
     def test_volume_create(self):
@@ -137,6 +139,17 @@ class VolumeApiTest(test.TestCase):
                           req,
                           body)
 
+    def test_volume_creation_fails_with_bad_availability_zone(self):
+        vol = {"size": '1',
+               "name": "Volume Test Name",
+               "description": "Volume Test Desc",
+               "availability_zone": "zonen:hostn"}
+        body = {"volume": vol}
+        req = fakes.HTTPRequest.blank('/v2/volumes')
+        self.assertRaises(exception.InvalidInput,
+                          self.controller.create,
+                          req, body)
+
     def test_volume_create_with_image_id(self):
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
         self.ext_mgr.extensions = {'os-image-create': 'fake'}

diff --git a/cinder/tests/integrated/test_volumes.py b/cinder/tests/integrated/test_volumes.py
index 6f82b54aada72d3a12d7eb5dd82974a6b2e884bb..72a9387c36945fc4f69a18166c1fb1b9b046cc0a 100644 (file)
--- a/cinder/tests/integrated/test_volumes.py
+++ b/cinder/tests/integrated/test_volumes.py
@@ -163,7 +163,7 @@ class VolumesTest(integrated_helpers._IntegratedTestBase):
         """Creates a volume in availability_zone."""
 
         # Create volume
-        availability_zone = 'zone1:host1'
+        availability_zone = 'nova'
         created_volume = self.api.post_volume(
             {'volume': {'size': 1,
                         'availability_zone': availability_zone}})

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 05c22388c40ad701015b39e9a4a37ba820503aba..4cb6be526dad1fc806743662b59134aeb21f5e31 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -24,6 +24,7 @@ import functools
 
 from oslo.config import cfg
 
+from cinder import context
 from cinder.db import base
 from cinder import exception
 from cinder import flags
@@ -83,6 +84,7 @@ class API(base.Base):
                               glance.get_default_image_service())
         self.scheduler_rpcapi = scheduler_rpcapi.SchedulerAPI()
         self.volume_rpcapi = volume_rpcapi.VolumeAPI()
+        self.availability_zones = set()
         super(API, self).__init__(db_driver)
 
     def create(self, context, size, name, description, snapshot=None,
@@ -184,6 +186,8 @@ class API(base.Base):
 
         if availability_zone is None:
             availability_zone = FLAGS.storage_availability_zone
+        else:
+            self._check_availabilty_zone(availability_zone)
 
         if not volume_type and not source_volume:
             volume_type = volume_types.get_default_volume_type()
@@ -291,6 +295,25 @@ class API(base.Base):
                 request_spec=request_spec,
                 filter_properties=filter_properties)
 
+    def _check_availabilty_zone(self, availability_zone):
+        if availability_zone in self.availability_zones:
+            return
+
+        ctxt = context.get_admin_context()
+        topic = FLAGS.volume_topic
+        volume_services = self.db.service_get_all_by_topic(ctxt, topic)
+
+        # NOTE(haomai): In case of volume services isn't init or
+        # availability_zones is updated in the backend
+        self.availability_zones = set()
+        for service in volume_services:
+            self.availability_zones.add(service['availability_zone'])
+
+        if availability_zone not in self.availability_zones:
+            msg = _("Availability zone is invalid")
+            LOG.warn(msg)
+            raise exception.InvalidInput(reason=msg)
+
     @wrap_check_policy
     def delete(self, context, volume, force=False):
         if context.is_admin and context.project_id != volume['project_id']: