From 7d7dc2659d1ad07ef77892306827ca4a662e315d Mon Sep 17 00:00:00 2001
From: j-griffith <john.griffith8@gmail.com>
Date: Thu, 29 Nov 2018 18:07:58 +0000
Subject: [PATCH] Check Volume Status on attahcment create/update

Currently the new Attachment calls in Cinder aren't checking the status
of the volume when create or update is called.  This isn't good because
it can mask problems with attach processes on the volume.

This patch adds an explicit check of the volume status in both
attachment_create and attachment_update, and if the volume status is in
any error state, the call will result in an InvalidVolume exception.

This is an API change to fix a bug, and is not microversioned.

Closes-Bug: #1805762

Change-Id: I9b32c4db93879dda6490b244643de1a18bddbbf5
---
 .../unit/attachments/test_attachments_api.py  | 34 +++++++++++++++++++
 cinder/volume/api.py                          | 17 ++++++++++
 2 files changed, 51 insertions(+)

diff --git a/cinder/tests/unit/attachments/test_attachments_api.py b/cinder/tests/unit/attachments/test_attachments_api.py
index f0d2202950c..f6c25913ac3 100644
--- a/cinder/tests/unit/attachments/test_attachments_api.py
+++ b/cinder/tests/unit/attachments/test_attachments_api.py
@@ -280,3 +280,37 @@ class AttachmentManagerTestCase(test.TestCase):
         self.assertEqual('ro', aref.attach_mode)
         self.assertEqual(vref.id, aref.volume_id)
         self.assertEqual({}, aref.connection_info)
+
+    def test_attachment_create_volume_in_error_state(self):
+        """Test attachment_create volume in error state."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        vref.status = "error"
+        self.assertRaises(exception.InvalidVolume,
+                          self.volume_api.attachment_create,
+                          self.context,
+                          vref,
+                          fake.UUID2)
+
+    def test_attachment_update_volume_in_error_state(self):
+        """Test attachment_update volumem in error state."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        aref = self.volume_api.attachment_create(self.context,
+                                                 vref,
+                                                 fake.UUID2)
+        self.assertEqual(fake.UUID2, aref.instance_uuid)
+        self.assertIsNone(aref.attach_time)
+        self.assertEqual('reserved', aref.attach_status)
+        self.assertEqual(vref.id, aref.volume_id)
+        self.assertEqual({}, aref.connection_info)
+        vref.status = 'error'
+        vref.save()
+        connector = {'fake': 'connector'}
+        self.assertRaises(exception.InvalidVolume,
+                          self.volume_api.attachment_update,
+                          self.context,
+                          aref,
+                          connector)
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index e8f6b0ba1a4..d0ccbc01ccd 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -2111,6 +2111,15 @@ class API(base.Base):
         """Create an attachment record for the specified volume."""
         ctxt.authorize(attachment_policy.CREATE_POLICY, target_obj=volume_ref)
         connection_info = {}
+        if "error" in volume_ref.status:
+            msg = ('Volume attachments can not be created if the volume '
+                   'is in an error state. '
+                   'The Volume %(volume_id)s currently has a status of: '
+                   '%(volume_status)s ') % {
+                       'volume_id': volume_ref.id,
+                       'volume_status': volume_ref.status}
+            LOG.error(msg)
+            raise exception.InvalidVolume(reason=msg)
         attachment_ref = self._attachment_reserve(ctxt,
                                                   volume_ref,
                                                   instance_uuid)
@@ -2155,6 +2164,14 @@ class API(base.Base):
         ctxt.authorize(attachment_policy.UPDATE_POLICY,
                        target_obj=attachment_ref)
         volume_ref = objects.Volume.get_by_id(ctxt, attachment_ref.volume_id)
+        if "error" in volume_ref.status:
+            msg = ('Volume attachments can not be updated if the volume '
+                   'is in an error state. The Volume %(volume_id)s '
+                   'currently has a status of: %(volume_status)s ') % {
+                       'volume_id': volume_ref.id,
+                       'volume_status': volume_ref.status}
+            LOG.error(msg)
+            raise exception.InvalidVolume(reason=msg)
         connection_info = (
             self.volume_rpcapi.attachment_update(ctxt,
                                                  volume_ref,