Merge "Prevent volume-attach/detach from instances in rescue state"

4 files changed, 74 insertions, 9 deletions

diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index 93d76495f..b16d61852 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -402,6 +402,9 @@ class VolumeAttachmentController(wsgi.Controller):
                                                     volume_id, device)
         except exception.NotFound:
             raise exc.HTTPNotFound()
+        except exception.InstanceInvalidState as state_error:
+            common.raise_http_conflict_for_instance_invalid_state(state_error,
+                    'attach_volume')
 
         # The attach is async
         attachment = {}
@@ -446,12 +449,16 @@ class VolumeAttachmentController(wsgi.Controller):
             raise exc.HTTPNotFound()
 
         found = False
-        for bdm in bdms:
-            if bdm['volume_id'] == volume_id:
-                self.compute_api.detach_volume(context,
-                    volume_id=volume_id)
-                found = True
-                break
+        try:
+            for bdm in bdms:
+                if bdm['volume_id'] == volume_id:
+                    self.compute_api.detach_volume(context,
+                        volume_id=volume_id)
+                    found = True
+                    break
+        except exception.InstanceInvalidState as state_error:
+            common.raise_http_conflict_for_instance_invalid_state(state_error,
+                    'detach_volume')
 
         if not found:
             raise exc.HTTPNotFound()
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 4abb5e886..3bb4919f5 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2285,6 +2285,10 @@ class API(base.Base):
 
     @wrap_check_policy
     @check_instance_lock
+    @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
+                                    vm_states.SUSPENDED, vm_states.STOPPED,
+                                    vm_states.RESIZED, vm_states.SOFT_DELETED],
+                          task_state=None)
     def attach_volume(self, context, instance, volume_id, device=None):
         """Attach an existing volume to an existing instance."""
         # NOTE(vish): Fail fast if the device is not going to pass. This
@@ -2314,6 +2318,10 @@ class API(base.Base):
         return device
 
     @check_instance_lock
+    @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
+                                    vm_states.SUSPENDED, vm_states.STOPPED,
+                                    vm_states.RESIZED, vm_states.SOFT_DELETED],
+                          task_state=None)
     def _detach_volume(self, context, instance, volume_id):
         check_policy(context, 'detach_volume', instance)
 
@@ -2364,7 +2372,7 @@ class API(base.Base):
     @wrap_check_policy
     @check_instance_lock
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
-                          vm_states.SUSPENDED, vm_states.STOPPED],
+                                    vm_states.SUSPENDED, vm_states.STOPPED],
                           task_state=None)
     def delete_instance_metadata(self, context, instance, key):
         """Delete the given metadata item from an instance."""
@@ -2378,7 +2386,7 @@ class API(base.Base):
     @wrap_check_policy
     @check_instance_lock
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
-                          vm_states.SUSPENDED, vm_states.STOPPED],
+                                    vm_states.SUSPENDED, vm_states.STOPPED],
                           task_state=None)
     def update_instance_metadata(self, context, instance,
                                  metadata, delete=False):
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 570c7fdea..21b5b5c96 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -6157,10 +6157,56 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertRaises(exception.InvalidDevicePath,
                 self.compute_api.attach_volume,
                 self.context,
-                {'locked': False},
+                {'locked': False, 'vm_state': vm_states.ACTIVE},
                 None,
                 '/invalid')
 
+    def test_no_attach_volume_in_rescue_state(self):
+        def fake(*args, **kwargs):
+            pass
+
+        def fake_volume_get(self, context, volume_id):
+            return {'id': volume_id}
+
+        self.stubs.Set(cinder.API, 'get', fake_volume_get)
+        self.stubs.Set(cinder.API, 'check_attach', fake)
+        self.stubs.Set(cinder.API, 'reserve_volume', fake)
+
+        self.assertRaises(exception.InstanceInvalidState,
+                self.compute_api.attach_volume,
+                self.context,
+                {'uuid': 'fake_uuid', 'locked': False,
+                'vm_state': vm_states.RESCUED},
+                None,
+                '/dev/vdb')
+
+    def test_no_detach_volume_in_rescue_state(self):
+        # Ensure volume can be detached from instance
+
+        params = {'vm_state': vm_states.RESCUED}
+        instance = self._create_fake_instance(params=params)
+
+        def fake(*args, **kwargs):
+            pass
+
+        def fake_volume_get(self, context, volume_id):
+            pass
+            return {'id': volume_id, 'attach_status': 'in-use',
+                    'instance_uuid': instance['uuid']}
+
+        def fake_rpc_detach_volume(self, context, **kwargs):
+            pass
+
+        self.stubs.Set(cinder.API, 'get', fake_volume_get)
+        self.stubs.Set(cinder.API, 'check_detach', fake)
+        self.stubs.Set(cinder.API, 'begin_detaching', fake)
+        self.stubs.Set(compute_rpcapi.ComputeAPI, 'detach_volume',
+                       fake_rpc_detach_volume)
+
+        self.assertRaises(exception.InstanceInvalidState,
+                self.compute_api.detach_volume,
+                self.context, 1)
+
     def test_vnc_console(self):
         # Make sure we can a vnc console for an instance.
 
diff --git a/nova/tests/compute/test_compute_cells.py b/nova/tests/compute/test_compute_cells.py
index 78100bcc3..0b93652f9 100644
--- a/nova/tests/compute/test_compute_cells.py
+++ b/nova/tests/compute/test_compute_cells.py
@@ -190,6 +190,10 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
         self.skipTest("This test is failing due to TypeError: "
                       "detach_volume() takes exactly 3 arguments (4 given).")
 
+    def test_no_detach_volume_in_rescue_state(self):
+        self.skipTest("This test is failing due to TypeError: "
+                      "detach_volume() takes exactly 3 arguments (4 given).")
+
     def test_evacuate(self):
         self.skipTest("Test is incompatible with cells.")