Prevent rescue for volume-backed instances

This patch prevents rescuing of volume_backed instances, by checking
for it in the API layer and raising an exception if instance on which a
rescue was attempted is volume backed.

Rescue is supposed to just be a way to log into a wayward instance
if something goes wrong with the base image that may have had some data
(logfiles etc.) and make it possible to grab that - block devices are
assumed to be accessible by re-attaching them, and are considered
persistant so no need for rescue there.

Fixes bug: #1067744
blueprint: improve-boot-from-volume

Change-Id: I8a4b1ccff7406837de3086aa413034e8e647b8fa

5 files changed, 70 insertions, 12 deletions

diff --git a/nova/api/openstack/compute/contrib/rescue.py b/nova/api/openstack/compute/contrib/rescue.py
index b6cf3a918..d8699e0e0 100644
--- a/nova/api/openstack/compute/contrib/rescue.py
+++ b/nova/api/openstack/compute/contrib/rescue.py
@@ -63,6 +63,9 @@ class RescueController(wsgi.Controller):
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                                                                   'rescue')
+        except exception.InstanceNotRescuable as non_rescuable:
+            raise exc.HTTPBadRequest(explanation=unicode(non_rescuable))
+
         return {'adminPass': password}
 
     @wsgi.action('unrescue')
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 83ea98c8f..dc90748a4 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2142,6 +2142,14 @@ class API(base.Base):
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
     def rescue(self, context, instance, rescue_password=None):
         """Rescue the given instance."""
+        # TODO(ndipanov): This check can be generalized as a decorator to
+        # check for valid combinations of src and dests - for now check
+        # if it's booted from volume only
+        if self.is_volume_backed_instance(context, instance, None):
+            reason = _("Cannot rescue a volume-backed instance")
+            raise exception.InstanceNotRescuable(instance_id=instance['uuid'],
+                                                 reason=reason)
+
         self.update(context,
                     instance,
                     vm_state=vm_states.ACTIVE,
@@ -2414,6 +2422,9 @@ class API(base.Base):
                 instance['uuid'])
 
     def is_volume_backed_instance(self, context, instance, bdms):
+        if not instance['image_ref']:
+            return True
+
         if bdms is None:
             bdms = self.get_instance_bdms(context, instance)
 
diff --git a/nova/exception.py b/nova/exception.py
index 0e6c004d6..046df24c9 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -315,6 +315,10 @@ class InstanceNotInRescueMode(Invalid):
     message = _("Instance %(instance_id)s is not in rescue mode")
 
 
+class InstanceNotRescuable(Invalid):
+    message = _("Instance %(instance_id)s cannot be rescued: %(reason)s")
+
+
 class InstanceNotReady(Invalid):
     message = _("Instance %(instance_id)s is not ready")
 
diff --git a/nova/tests/api/openstack/compute/contrib/test_rescue.py b/nova/tests/api/openstack/compute/contrib/test_rescue.py
index 0d2e9e5c8..ea0a96cbf 100644
--- a/nova/tests/api/openstack/compute/contrib/test_rescue.py
+++ b/nova/tests/api/openstack/compute/contrib/test_rescue.py
@@ -113,3 +113,18 @@ class RescueTest(test.TestCase):
 
         resp = req.get_response(self.app)
         self.assertEqual(resp.status_int, 409)
+
+    def test_rescue_raises_unrescuable(self):
+        body = dict(rescue=None)
+
+        def fake_rescue(*args, **kwargs):
+            raise exception.InstanceNotRescuable('fake message')
+
+        self.stubs.Set(compute.api.API, "rescue", fake_rescue)
+        req = webob.Request.blank('/v2/fake/servers/test_inst/action')
+        req.method = "POST"
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        resp = req.get_response(self.app)
+        self.assertEqual(resp.status_int, 400)
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index c295ed75a..e94d8b788 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -951,18 +951,6 @@ class ComputeTestCase(BaseTestCase):
 
         self.compute.terminate_instance(self.context, instance=instance)
 
-    def test_rescue_no_image(self):
-        params = {'image_ref': ''}
-        instance = self._create_fake_instance(params)
-        instance_uuid = instance['uuid']
-        self.compute.run_instance(self.context, instance=instance)
-        db.instance_update(self.context, instance_uuid,
-                           {"task_state": task_states.RESCUING})
-        self.compute.rescue_instance(self.context, instance=instance)
-        db.instance_update(self.context, instance_uuid,
-                           {"task_state": task_states.UNRESCUING})
-        self.compute.unrescue_instance(self.context, instance=instance)
-
     def test_power_on(self):
         # Ensure instance can be powered on.
 
@@ -4789,6 +4777,43 @@ class ComputeAPITestCase(BaseTestCase):
         self.compute.terminate_instance(self.context,
                 instance=jsonutils.to_primitive(instance))
 
+    def test_rescue_volume_backed(self):
+        # Instance started without an image
+        volume_backed_inst_1 = jsonutils.to_primitive(
+            self._create_fake_instance({'image_ref': ''}))
+
+        # Instance started with a placeholder image (for metadata)
+        volume_backed_inst_2 = jsonutils.to_primitive(
+            self._create_fake_instance(
+                {'image_ref': 'my_placeholder_img',
+                 'root_device_name': '/dev/vda'})
+            )
+        volume_backed_uuid_1 = volume_backed_inst_1['uuid']
+        volume_backed_uuid_2 = volume_backed_inst_2['uuid']
+
+        def fake_get_instance_bdms(*args, **kwargs):
+            return [{'device_name': '/dev/vda'}]
+
+        self.stubs.Set(self.compute_api, 'get_instance_bdms',
+                       fake_get_instance_bdms)
+
+        self.compute.run_instance(self.context,
+                                  instance=volume_backed_inst_1)
+        self.compute.run_instance(self.context,
+                                  instance=volume_backed_inst_2)
+
+        self.assertRaises(exception.InstanceNotRescuable,
+                          self.compute_api.rescue, self.context,
+                          volume_backed_inst_1)
+        self.assertRaises(exception.InstanceNotRescuable,
+                          self.compute_api.rescue, self.context,
+                          volume_backed_inst_2)
+
+        self.compute.terminate_instance(self.context,
+                instance=jsonutils.to_primitive(volume_backed_inst_1))
+        self.compute.terminate_instance(self.context,
+                instance=jsonutils.to_primitive(volume_backed_inst_2))
+
     def test_snapshot(self):
         # Ensure a snapshot of an instance can be created.
         instance = self._create_fake_instance()