From 727e334950c672b9309ad6a1394488dbdc13952f Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem.os@gmail.com>
Date: Wed, 3 Jan 2018 15:51:37 -0500
Subject: [PATCH] Handle InvalidVolume when creating a volume attachment

If you try to attach a non-multiattach volume to more than
once instance using the 3.27 volume attachments API, the volume
API would return a 500 response because the volume is in-use.

This translates to a 500 in the compute API because nova isn't
expecting a random client exception.

Treat this like how os-reserve could fail in the same situation
and handle InvalidVolume which will be converted to an HTTPBadRequest
by ResourceExceptionHandler elsewhere in the WSGi stack.

Change-Id: I1a8068043159b034d700419675af87fc17e1faf5
Closes-Bug: #1741112
---
 cinder/api/v3/attachments.py                 |  3 ++-
 cinder/tests/unit/api/v3/test_attachments.py | 24 ++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/cinder/api/v3/attachments.py b/cinder/api/v3/attachments.py
index caa005c518e..6c457920f9b 100644
--- a/cinder/api/v3/attachments.py
+++ b/cinder/api/v3/attachments.py
@@ -170,7 +170,8 @@ class AttachmentsController(wsgi.Controller):
                                                   volume_ref,
                                                   instance_uuid,
                                                   connector=connector))
-        except exception.NotAuthorized:
+        except (exception.NotAuthorized,
+                exception.InvalidVolume):
             raise
         except exception.CinderException as ex:
             err_msg = _(
diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py
index a1f50aee2a6..8d38e1c56ce 100644
--- a/cinder/tests/unit/api/v3/test_attachments.py
+++ b/cinder/tests/unit/api/v3/test_attachments.py
@@ -244,6 +244,30 @@ class AttachmentsAPITestCase(test.TestCase):
         self.assertRaises(exception.ValidationError,
                           self.controller.create, req, body=fake_body)
 
+    @mock.patch('cinder.volume.api.API._attachment_reserve')
+    def test_create_attachment_in_use_volume_multiattach_false(self,
+                                                               mock_reserve):
+        """Negative test for creating an attachment on an in-use volume."""
+        req = fakes.HTTPRequest.blank('/v3/%s/attachments' %
+                                      fake.PROJECT_ID,
+                                      version=mv.NEW_ATTACH)
+        body = {
+            "attachment":
+                {
+                    "connector": None,
+                    "instance_uuid": fake.UUID1,
+                    "volume_uuid": self.volume1.id
+                },
+        }
+        mock_reserve.side_effect = (
+            exception.InvalidVolume(
+                reason="Volume %s status must be available or "
+                       "downloading" % self.volume1.id))
+        # Note that if we were using the full WSGi stack, the
+        # ResourceExceptionHandler would convert this to an HTTPBadRequest.
+        self.assertRaises(exception.InvalidVolume,
+                          self.controller.create, req, body=body)
+
     @ddt.data(False, True)
     def test_list_attachments(self, is_detail):
         url = '/v3/%s/attachments' % fake.PROJECT_ID