Allow reboot or rebuild from vm_state=Error

In general most operations are only valid on an
instance that has booted successfully at least once
so this change extends the instance state check
logic to include evidence that the instance has
been successfully started at least once.

This enables more operations to be allowed in
against instances in an Error state.  For example
reboot and rebuild are useful recover options for
an instance which has reached an error state, but
not if the instance failed during its initial build.

With this change the only actions allowed on an
instance which has never booted successfully are
delete and force_delete.  Soft delete is not
allowed, as the restore is in effect a start unless
there is specific virt driver support.

In addition the following actions are now allowed
against instances in an Error state providing the
instance has booted at least once:  Reboot, Rebuild,
and Rescue.

Fixes bug:   1183946

Change-Id: I63fd8d2a182df5cf12754892e8076933b3b1e79f

nova/api/openstack/common.py

@@ -358,9 +358,12 @@ def raise_http_conflict_for_instance_invalid_state(exc, action):
     """
     attr = exc.kwargs.get('attr')
     state = exc.kwargs.get('state')
+    not_launched = exc.kwargs.get('not_launched')
     if attr and state:
         msg = _("Cannot '%(action)s' while instance is in %(attr)s "
                 "%(state)s") % {'action': action, 'attr': attr, 'state': state}
+    elif not_launched:
+        msg = _("Cannot '%s' an instance which has never been active") % action
     else:
         # At least give some meaningful message
         msg = _("Instance is in an invalid state for '%s'") % action

nova/compute/api.py

@@ -110,10 +110,12 @@
 SM_IMAGE_PROP_PREFIX = "image_"
 
 
-def check_instance_state(vm_state=None, task_state=(None,)):
+def check_instance_state(vm_state=None, task_state=(None,),
+                         must_have_launched=True):
     """Decorator to check VM and/or task state before entry to API functions.
 
-    If the instance is in the wrong state, the wrapper will raise an exception.
+    If the instance is in the wrong state, or has not been sucessfully started
+    at least once the wrapper will raise an exception.
     """
 
     if vm_state is not None and not isinstance(vm_state, set):
@@ -137,6 +139,13 @@ def inner(self, context, instance, *args, **kw):
                     instance_uuid=instance['uuid'],
                     state=instance['task_state'],
                     method=f.__name__)
+            if must_have_launched and not instance['launched_at']:
+                raise exception.InstanceInvalidState(
+                    attr=None,
+                    not_launched=True,
+                    instance_uuid=instance['uuid'],
+                    state=instance['vm_state'],
+                    method=f.__name__)
 
             return f(self, context, instance, *args, **kw)
         return inner
@@ -1305,7 +1314,8 @@ def _local_delete(self, context, instance, bdms):
     # NOTE(maoy): we allow delete to be called no matter what vm_state says.
     @wrap_check_policy
     @check_instance_lock
-    @check_instance_state(vm_state=None, task_state=None)
+    @check_instance_state(vm_state=None, task_state=None,
+                          must_have_launched=True)
     def soft_delete(self, context, instance):
         """Terminate an instance."""
         LOG.debug(_('Going to try to soft delete instance'),
@@ -1329,7 +1339,8 @@ def terminate(context, instance, bdms, reservations=None):
 
     @wrap_check_policy
     @check_instance_lock
-    @check_instance_state(vm_state=None, task_state=None)
+    @check_instance_state(vm_state=None, task_state=None,
+                          must_have_launched=False)
     def delete(self, context, instance):
         """Terminate an instance."""
         LOG.debug(_("Going to try to terminate instance"), instance=instance)
@@ -1369,7 +1380,8 @@ def restore(self, context, instance):
 
     @wrap_check_policy
     @check_instance_lock
-    @check_instance_state(vm_state=[vm_states.SOFT_DELETED])
+    @check_instance_state(vm_state=[vm_states.SOFT_DELETED],
+                          must_have_launched=False)
     def force_delete(self, context, instance):
         """Force delete a previously deleted (but not reclaimed) instance."""
         self._delete_instance(context, instance)
@@ -1790,7 +1802,8 @@ def _get_block_device_info(self, context, instance_uuid):
     @wrap_check_policy
     @check_instance_lock
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED,
-                                    vm_states.PAUSED, vm_states.SUSPENDED],
+                                    vm_states.PAUSED, vm_states.SUSPENDED,
+                                    vm_states.ERROR],
                           task_state=[None, task_states.REBOOTING,
                                       task_states.REBOOTING_HARD,
                                       task_states.RESUMING,
@@ -1826,7 +1839,8 @@ def reboot(self, context, instance, reboot_type):
 
     @wrap_check_policy
     @check_instance_lock
-    @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED],
+    @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED,
+                                    vm_states.ERROR],
                           task_state=[None])
     def rebuild(self, context, instance, image_href, admin_password, **kwargs):
         """Rebuild the given instance with the provided attributes."""
@@ -2224,7 +2238,8 @@ def resume(self, context, instance):
 
     @wrap_check_policy
     @check_instance_lock
-    @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
+    @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED,
+                                    vm_states.ERROR])
     def rescue(self, context, instance, rescue_password=None):
         """Rescue the given instance."""
 

nova/tests/api/ec2/test_cloud.py

@@ -46,6 +46,7 @@
 from nova.network import quantumv2
 from nova.openstack.common import log as logging
 from nova.openstack.common import rpc
+from nova.openstack.common import timeutils
 from nova import test
 from nova.tests.api.openstack.compute.contrib import (
     test_quantum_security_groups as test_quantum)
@@ -880,6 +881,7 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
                 'instance_type_id': 1,
                 'host': 'host1',
                 'vm_state': 'active',
+                'launched_at': timeutils.utcnow(),
                 'hostname': 'server-1111',
                 'created_at': datetime.datetime(2012, 5, 1, 1, 1, 1),
                 'system_metadata': sys_meta
@@ -891,6 +893,7 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
                 'instance_type_id': 1,
                 'host': 'host2',
                 'vm_state': 'active',
+                'launched_at': timeutils.utcnow(),
                 'hostname': 'server-1112',
                 'created_at': datetime.datetime(2012, 5, 1, 1, 1, 2),
                 'system_metadata': sys_meta
@@ -2442,6 +2445,7 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
                 'image_ref': image_uuid,
                 'instance_type_id': 1,
                 'vm_state': 'active',
+                'launched_at': timeutils.utcnow(),
                 'hostname': 'server-1111',
                 'created_at': datetime.datetime(2012, 5, 1, 1, 1, 1)
         }
@@ -2492,6 +2496,7 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
                 'image_ref': image_uuid,
                 'instance_type_id': 1,
                 'vm_state': 'active',
+                'launched_at': timeutils.utcnow(),
                 'hostname': 'server-1111',
                 'created_at': datetime.datetime(2012, 5, 1, 1, 1, 1)
         }
@@ -2501,6 +2506,7 @@ def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
                 'image_ref': image_uuid,
                 'instance_type_id': 1,
                 'vm_state': 'active',
+                'launched_at': timeutils.utcnow(),
                 'hostname': 'server-1112',
                 'created_at': datetime.datetime(2012, 5, 1, 1, 1, 2)
         }

nova/tests/api/openstack/compute/contrib/test_admin_actions.py

@@ -26,6 +26,7 @@
 from nova import context
 from nova import exception
 from nova.openstack.common import jsonutils
+from nova.openstack.common import timeutils
 from nova import test
 from nova.tests.api.openstack import fakes
 
@@ -41,6 +42,7 @@
              "tenant_id": 'fake_tenant_id',
              "created_at": datetime.datetime(2010, 10, 10, 12, 0, 0),
              "updated_at": datetime.datetime(2010, 11, 11, 11, 0, 0),
+             "launched_at": datetime.datetime(2010, 11, 11, 11, 0, 0),
              "security_groups": [{"id": 1, "name": "test"}],
              "progress": 0,
              "image_ref": 'http://foo.com/123',
@@ -61,7 +63,7 @@ def fake_compute_api_raises_invalid_state(*args, **kwargs):
 
 def fake_compute_api_get(self, context, instance_id):
     return {'id': 1, 'uuid': instance_id, 'vm_state': vm_states.ACTIVE,
-            'task_state': None}
+            'task_state': None, 'launched_at': timeutils.utcnow()}
 
 
 class AdminActionsTest(test.TestCase):

nova/tests/api/openstack/compute/test_server_metadata.py

@@ -26,6 +26,7 @@
 import nova.db
 from nova import exception
 from nova.openstack.common import jsonutils
+from nova.openstack.common import timeutils
 from nova import test
 from nova.tests.api.openstack import fakes
 
@@ -77,6 +78,7 @@ def return_server(context, server_id):
             'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
             'name': 'fake',
             'locked': False,
+            'launched_at': timeutils.utcnow(),
             'vm_state': vm_states.ACTIVE}
 
 
@@ -85,6 +87,7 @@ def return_server_by_uuid(context, server_uuid):
             'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
             'name': 'fake',
             'locked': False,
+            'launched_at': timeutils.utcnow(),
             'vm_state': vm_states.ACTIVE}