From 8eb5af2d4086438a678abb22d720dbd3ebb9a02d Mon Sep 17 00:00:00 2001
From: Mark Washenberger <mark.washenberger@rackspace.com>
Date: Tue, 6 Dec 2011 14:44:43 -0500
Subject: vm_state:=error on driver exceptions during resize

Set vm_state to error if a driver raises an exception (of any type)
during resize_instance and finish_resize.

I want to highlight that there is a slight behavioral change here.
Before, on a MigrationError the exception was not reraised in the
resize_instance function. However, based on my understanding it is not
a big deal to make this change. The worst case is log duplication in
a fairly rare error state.

Change-Id: Ifd1ea66cf3f2e59ee06714f666e357a99936b4e3
---
 nova/compute/manager.py    | 27 ++++++++++++++++++---------
 nova/tests/test_compute.py | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index c086f21c2..625d2c94b 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1080,12 +1080,13 @@ class ComputeManager(manager.SchedulerDependentManager):
             disk_info = self.driver.migrate_disk_and_power_off(
                     context, instance_ref, migration_ref['dest_host'],
                     instance_type_ref)
-        except exception.MigrationError, error:
-            LOG.error(_('%s. Setting instance vm_state to ERROR') % (error,))
-            self._instance_update(context,
-                                  instance_uuid,
-                                  vm_state=vm_states.ERROR)
-            return
+        except Exception, error:
+            with utils.save_and_reraise_exception():
+                msg = _('%s. Setting instance vm_state to ERROR')
+                LOG.error(msg % error)
+                self._instance_update(context,
+                                      instance_uuid,
+                                      vm_state=vm_states.ERROR)
 
         self.db.migration_update(context,
                                  migration_id,
@@ -1136,9 +1137,17 @@ class ComputeManager(manager.SchedulerDependentManager):
         # Have to look up image here since we depend on disk_format later
         image_meta = _get_image_meta(context, instance_ref['image_ref'])
 
-        self.driver.finish_migration(context, migration_ref, instance_ref,
-                                     disk_info, network_info, image_meta,
-                                     resize_instance)
+        try:
+            self.driver.finish_migration(context, migration_ref, instance_ref,
+                                         disk_info, network_info, image_meta,
+                                         resize_instance)
+        except Exception, error:
+            with utils.save_and_reraise_exception():
+                msg = _('%s. Setting instance vm_state to ERROR')
+                LOG.error(msg % error)
+                self._instance_update(context,
+                                      instance_uuid,
+                                      vm_state=vm_states.ERROR)
 
         self._instance_update(context,
                               instance_uuid,
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index bccc590c3..a40a3eee4 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -713,6 +713,31 @@ class ComputeTestCase(BaseTestCase):
 
         self.compute.terminate_instance(self.context, instance['uuid'])
 
+    def test_finish_resize_handles_error(self):
+        """Make sure we don't leave the instance in RESIZE on error"""
+
+        def throw_up(*args, **kwargs):
+            raise Exception()
+
+        def fake(*args, **kwargs):
+            pass
+
+        self.stubs.Set(self.compute.driver, 'finish_migration', throw_up)
+        self.stubs.Set(self.compute.network_api, 'get_instance_nw_info', fake)
+        context = self.context.elevated()
+        instance = self._create_fake_instance()
+        self.compute.prep_resize(context, instance['uuid'], 1)
+        migration_ref = db.migration_get_by_instance_and_status(context,
+                instance['uuid'], 'pre-migrating')
+
+        self.assertRaises(Exception, self.compute.finish_resize,
+                          context, instance['uuid'],
+                          int(migration_ref['id']), {})
+
+        instance = db.instance_get_by_uuid(context, instance['uuid'])
+        self.assertEqual(instance['vm_state'], vm_states.ERROR)
+        self.compute.terminate_instance(self.context, instance['uuid'])
+
     def test_resize_instance_notification(self):
         """Ensure notifications on instance migrate/resize"""
         instance = self._create_fake_instance()
@@ -831,9 +856,9 @@ class ComputeTestCase(BaseTestCase):
         self.compute.terminate_instance(self.context, instance['uuid'])
 
     def test_resize_instance_handles_migration_error(self):
-        """Ensure vm_state is ERROR when MigrationError occurs"""
+        """Ensure vm_state is ERROR when error occurs"""
         def raise_migration_failure(*args):
-            raise exception.MigrationError(reason='test failure')
+            raise Exception(reason='test failure')
         self.stubs.Set(self.compute.driver,
                 'migrate_disk_and_power_off',
                 raise_migration_failure)
@@ -846,9 +871,8 @@ class ComputeTestCase(BaseTestCase):
         self.compute.prep_resize(context, inst_ref['uuid'], 1)
         migration_ref = db.migration_get_by_instance_and_status(context,
                 inst_ref['uuid'], 'pre-migrating')
-        self.compute.resize_instance(context,
-                                     inst_ref['uuid'],
-                                     migration_ref['id'])
+        self.assertRaises(Exception, self.compute.resize_instance,
+                          context, inst_ref['uuid'], migration_ref['id'])
         inst_ref = db.instance_get_by_uuid(context, inst_ref['uuid'])
         self.assertEqual(inst_ref['vm_state'], vm_states.ERROR)
         self.compute.terminate_instance(context, inst_ref['uuid'])
-- 
cgit