Validate volume type during volume create

Currently when configuring multiple cinder stores,
glance validates the volume types configured in glance-api.conf
during service startup.
This check however has a dependency on cinder API service to be
up and running.
During certain deployment scenarios, it is not always mandatory
that cinder is already running when we are starting glance hence
this check will fail the glance service hence the deployment
This patch removes the startup exceptions and instead logs a warning.
Also the check is done during volume create so handling for NotFound
exception from cinder is also handled.

Change-Id: I468523b947ad6fcff4b074d2ed1cb7a9e52b15ca
Closes-Bug: #1915163

4 files changed, 104 insertions, 46 deletions

diff --git a/glance_store/_drivers/cinder.py b/glance_store/_drivers/cinder.py
index 10d4727..583467d 100644
--- a/glance_store/_drivers/cinder.py
+++ b/glance_store/_drivers/cinder.py
@@ -33,7 +33,7 @@ from glance_store import capabilities
 from glance_store.common import utils
 import glance_store.driver
 from glance_store import exceptions
-from glance_store.i18n import _, _LE, _LI
+from glance_store.i18n import _, _LE, _LI, _LW
 import glance_store.location
 
 try:
@@ -405,38 +405,26 @@ class Store(glance_store.driver.Store):
 
     def configure_add(self):
         """
-        Configure the Store to use the stored configuration options
-        Any store that needs special configuration should implement
-        this method. If the store was not able to successfully configure
-        itself, it should raise `exceptions.BadStoreConfiguration`
-        :raises: `exceptions.BadStoreConfiguration` if multiple stores are
-                 defined and particular store wasn't able to configure
-                 successfully
-        :raises: `exceptions.BackendException` if single store is defined and
-                 it wasn't able to configure successfully
+        Check to verify if the volume types configured for the cinder store
+        exist in deployment and if not, log a warning.
         """
-        if self.backend_group:
-            cinder_volume_type = self.store_conf.cinder_volume_type
-            if cinder_volume_type:
-                # NOTE: `cinder_volume_type` is configured, check
-                # configured volume_type is available in cinder or not
-                cinder_client = self.get_cinderclient()
-                try:
-                    # We don't even need the volume type object, as long
-                    # as this returns clean, we know the name is good.
-                    cinder_client.volume_types.find(name=cinder_volume_type)
-                    # No need to worry NoUniqueMatch as volume type name is
-                    # unique
-                except cinder_exception.NotFound:
-                    reason = _("Invalid `cinder_volume_type %s`"
-                               % cinder_volume_type)
-                    if len(self.conf.enabled_backends) > 1:
-                        LOG.error(reason)
-                        raise exceptions.BadStoreConfiguration(
-                            store_name=self.backend_group, reason=reason)
-                    else:
-                        LOG.critical(reason)
-                        raise exceptions.BackendException(reason)
+        cinder_volume_type = self.store_conf.cinder_volume_type
+        if cinder_volume_type:
+            # NOTE: `cinder_volume_type` is configured, check
+            # configured volume_type is available in cinder or not
+            cinder_client = self.get_cinderclient()
+            try:
+                # We don't even need the volume type object, as long
+                # as this returns clean, we know the name is good.
+                cinder_client.volume_types.find(name=cinder_volume_type)
+                # No need to worry about a NoUniqueMatch as volume type name
+                # is unique
+            except cinder_exception.NotFound:
+                reason = (_LW("Invalid `cinder_volume_type %s`"
+                              % cinder_volume_type))
+                LOG.warning(reason)
+            except cinder_exception.ClientException:
+                pass
 
     def is_image_associated_with_store(self, context, volume_id):
         """
@@ -849,8 +837,18 @@ class Store(glance_store.driver.Store):
             LOG.info(_LI("Since image size is zero, we will be doing "
                          "resize-before-write for each GB which "
                          "will be considerably slower than normal."))
-        volume = client.volumes.create(size_gb, name=name, metadata=metadata,
-                                       volume_type=volume_type)
+        try:
+            volume = client.volumes.create(size_gb, name=name,
+                                           metadata=metadata,
+                                           volume_type=volume_type)
+        except cinder_exception.NotFound:
+            LOG.error(_LE("Invalid volume type %s configured. Please check "
+                          "the `cinder_volume_type` configuration parameter."
+                          % volume_type))
+            msg = (_("Failed to create image-volume due to invalid "
+                     "`cinder_volume_type` configured."))
+            raise exceptions.BackendException(msg)
+
         volume = self._wait_volume_status(volume, 'creating', 'available')
         size_gb = volume.size
 
diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py
index 2137f42..8b1aa17 100644
--- a/glance_store/tests/unit/test_cinder_store.py
+++ b/glance_store/tests/unit/test_cinder_store.py
@@ -411,3 +411,23 @@ class TestCinderStore(base.StoreBaseTest,
 
     def test_set_url_prefix(self):
         self.assertEqual('cinder://', self.store._url_prefix)
+
+    def test_configure_add(self):
+
+        def fake_volume_type(name):
+            if name != 'some_type':
+                raise cinder.cinder_exception.NotFound(code=404)
+
+        with mock.patch.object(self.store, 'get_cinderclient') as mocked_cc:
+            mocked_cc.return_value = FakeObject(volume_types=FakeObject(
+                find=fake_volume_type))
+            self.config(cinder_volume_type='some_type')
+            # If volume type exists, no exception is raised
+            self.store.configure_add()
+            # setting cinder_volume_type to non-existent value will log a
+            # warning
+            self.config(cinder_volume_type='some_random_type')
+            with mock.patch.object(cinder, 'LOG') as mock_log:
+                self.store.configure_add()
+                mock_log.warning.assert_called_with(
+                    "Invalid `cinder_volume_type some_random_type`")
diff --git a/glance_store/tests/unit/test_multistore_cinder.py b/glance_store/tests/unit/test_multistore_cinder.py
index d35dfc9..e5b62ef 100644
--- a/glance_store/tests/unit/test_multistore_cinder.py
+++ b/glance_store/tests/unit/test_multistore_cinder.py
@@ -271,27 +271,55 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
 
         self.store._check_context(FakeObject(service_catalog='fake'))
 
-    def test_cinder_configure_add(self):
+    def test_configure_add(self):
+
+        def fake_volume_type_check(name):
+            if name != 'some_type':
+                raise cinder.cinder_exception.NotFound(code=404)
+
         with mock.patch.object(self.store, 'get_cinderclient') as mocked_cc:
-            def raise_(ex):
-                raise ex
             mocked_cc.return_value = FakeObject(volume_types=FakeObject(
-                find=lambda name: 'some_type' if name == 'some_type'
-                else raise_(cinder.cinder_exception.NotFound(code=404))))
+                find=fake_volume_type_check))
             self.config(cinder_volume_type='some_type',
                         group=self.store.backend_group)
             # If volume type exists, no exception is raised
             self.store.configure_add()
-            # setting cinder_volume_type to non-existent value will raise
-            # BadStoreConfiguration exception
+            # setting cinder_volume_type to non-existent value will log a
+            # warning
             self.config(cinder_volume_type='some_random_type',
                         group=self.store.backend_group)
+            with mock.patch.object(cinder, 'LOG') as mock_log:
+                self.store.configure_add()
+                mock_log.warning.assert_called_with(
+                    "Invalid `cinder_volume_type some_random_type`")
 
-            self.assertRaises(exceptions.BadStoreConfiguration,
-                              self.store.configure_add)
-            # when only 1 store is configured, BackendException is raised
-            self.config(enabled_backends={'cinder1': 'cinder'})
-            self.assertRaises(exceptions.BackendException,
+    def test_configure_add_cinder_service_down(self):
+
+        def fake_volume_type_check(name):
+            raise cinder.cinder_exception.ClientException(code=503)
+
+        self.config(cinder_volume_type='some_type',
+                    group=self.store.backend_group)
+        with mock.patch.object(self.store, 'get_cinderclient') as mocked_cc:
+            mocked_cc.return_value = FakeObject(volume_types=FakeObject(
+                find=fake_volume_type_check))
+            # We handle the ClientException to pass so no exception is raised
+            # in this case
+            self.store.configure_add()
+
+    def test_configure_add_authorization_failed(self):
+
+        def fake_volume_type_check(name):
+            raise cinder.exceptions.AuthorizationFailure(code=401)
+
+        self.config(cinder_volume_type='some_type',
+                    group=self.store.backend_group)
+        with mock.patch.object(self.store, 'get_cinderclient') as mocked_cc:
+            mocked_cc.return_value = FakeObject(volume_types=FakeObject(
+                find=fake_volume_type_check))
+            # Anything apart from invalid volume type or cinder service
+            # down will raise an exception
+            self.assertRaises(cinder.exceptions.AuthorizationFailure,
                               self.store.configure_add)
 
     def test_is_image_associated_with_store(self):
diff --git a/releasenotes/notes/volume-type-validation-check-011a400d7fb3b307.yaml b/releasenotes/notes/volume-type-validation-check-011a400d7fb3b307.yaml
new file mode 100644
index 0000000..7ff169b
--- /dev/null
+++ b/releasenotes/notes/volume-type-validation-check-011a400d7fb3b307.yaml
@@ -0,0 +1,12 @@
+---
+upgrade:
+  - |
+    Previously, during service startup, the check to validate volume types
+    used to raise ``BackendException`` or ``BadStoreConfiguration`` exceptions
+    when an invalid volume type was configured hence failing the service
+    startup. It now logs a warning and the glance service starts normally.
+fixes:
+  - |
+    `Bug #1915163 <https://bugs.launchpad.net/glance-store/+bug/1915163>`_:
+    Added handling to log and raise proper exception during image create when
+    an invalid volume type is configured.