openstack
/
nova
Code Issues Proposed changes

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
This commit is contained in:
Phil Day 2013-06-07 15:18:03 +01:00
parent d147af21db
commit 99c51e3423
6 changed files with 143 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/api/openstack/common.py
View File

@ -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

31
nova/compute/api.py
View File

@ -110,10 +110,12 @@ RO_SECURITY_GROUPS = ['default']
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 check_instance_state(vm_state=None, task_state=(None,)):
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 @@ class API(base.Base):
# 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 @@ class API(base.Base):
@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 @@ class API(base.Base):
@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 @@ class API(base.Base):
@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 @@ class API(base.Base):
@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 @@ class API(base.Base):
@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."""

6
nova/tests/api/ec2/test_cloud.py
View File

@ -46,6 +46,7 @@ from nova.network import api as network_api
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 @@ class CloudTestCase(test.TestCase):
'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 @@ class CloudTestCase(test.TestCase):
'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 @@ class CloudTestCase(test.TestCase):
'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 @@ class CloudTestCase(test.TestCase):
'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 @@ class CloudTestCase(test.TestCase):
'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)
}

4
nova/tests/api/openstack/compute/contrib/test_admin_actions.py
View File

@ -26,6 +26,7 @@ from nova.conductor import api as conductor_api
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 @@ INSTANCE = {
"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):

3
nova/tests/api/openstack/compute/test_server_metadata.py
View File

@ -26,6 +26,7 @@ from nova.compute import vm_states
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}

110
nova/tests/compute/test_compute.py
View File

@ -259,6 +259,9 @@ class BaseTestCase(test.TestCase):
inst['os_type'] = 'Linux'
inst['system_metadata'] = make_fake_sys_meta()
inst['locked'] = False
inst['created_at'] = timeutils.utcnow()
inst['updated_at'] = timeutils.utcnow()
inst['launched_at'] = timeutils.utcnow()
inst.update(params)
_create_service_entries(self.context.elevated(),
{'fake_zone': [inst['host']]})
@ -1248,6 +1251,7 @@ class ComputeTestCase(BaseTestCase):
def test_run_terminate_timestamps(self):
# Make sure timestamps are set for launched and destroyed.
instance = jsonutils.to_primitive(self._create_fake_instance())
instance['launched_at'] = None
self.assertEqual(instance['launched_at'], None)
self.assertEqual(instance['deleted_at'], None)
launch = timeutils.utcnow()
@ -5710,6 +5714,21 @@ class ComputeAPITestCase(BaseTestCase):
db.instance_destroy(self.context, instance['uuid'])
def test_delete_if_not_launched(self):
instance, instance_uuid = self._run_instance(params={
'host': CONF.host})
db.instance_update(self.context, instance['uuid'],
{"vm_state": vm_states.ERROR,
"launched_at": None})
self.compute_api.delete(self.context, instance)
instance = db.instance_get_by_uuid(self.context, instance_uuid)
self.assertEqual(instance['task_state'], task_states.DELETING)
db.instance_destroy(self.context, instance['uuid'])
def test_delete_in_resizing(self):
def fake_quotas_reserve(context, expire=None, project_id=None,
**deltas):
@ -5984,7 +6003,7 @@ class ComputeAPITestCase(BaseTestCase):
db.instance_destroy(self.context, instance['uuid'])
def test_rebuild(self):
def _test_rebuild(self, vm_state):
instance = jsonutils.to_primitive(self._create_fake_instance())
instance_uuid = instance['uuid']
self.compute.run_instance(self.context, instance=instance)
@ -6015,6 +6034,10 @@ class ComputeAPITestCase(BaseTestCase):
image_ref = instance["image_ref"] + '-new_image_ref'
password = "new_password"
db.instance_update(self.context, instance['uuid'],
{"vm_state": vm_state})
self.compute_api.rebuild(self.context, instance, image_ref, password)
self.assertEqual(info['image_ref'], image_ref)
@ -6029,6 +6052,34 @@ class ComputeAPITestCase(BaseTestCase):
'preserved': 'preserve this!'})
db.instance_destroy(self.context, instance['uuid'])
def test_rebuild(self):
# Test we can rebuild an instance in the Error State
self._test_rebuild(vm_state=vm_states.ACTIVE)
def test_rebuild_in_error_state(self):
# Test we can rebuild an instance in the Error State
self._test_rebuild(vm_state=vm_states.ERROR)
def test_rebuild_in_error_not_launched(self):
instance = jsonutils.to_primitive(
self._create_fake_instance(params={'image_ref': ''}))
instance_uuid = instance['uuid']
self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
self.compute.run_instance(self.context, instance=instance)
db.instance_update(self.context, instance['uuid'],
{"vm_state": vm_states.ERROR,
"launched_at": None})
instance = db.instance_get_by_uuid(self.context, instance['uuid'])
self.assertRaises(exception.InstanceInvalidState,
self.compute_api.rebuild,
self.context,
instance,
instance['image_ref'],
"new password")
def test_rebuild_no_image(self):
instance = jsonutils.to_primitive(
self._create_fake_instance(params={'image_ref': ''}))
@ -6175,7 +6226,7 @@ class ComputeAPITestCase(BaseTestCase):
self.stubs.Set(nova.virt.fake.FakeDriver, 'legacy_nwinfo',
lambda x: False)
def test_reboot_soft(self):
def _test_reboot_soft(self, vm_state):
# Ensure instance can be soft rebooted.
instance = jsonutils.to_primitive(self._create_fake_instance())
self.compute.run_instance(self.context, instance=instance)
@ -6192,6 +6243,9 @@ class ComputeAPITestCase(BaseTestCase):
inst_ref = db.instance_get_by_uuid(self.context, instance['uuid'])
self.assertEqual(inst_ref['task_state'], None)
db.instance_update(self.context, instance['uuid'],
{"vm_state": vm_state})
reboot_type = "SOFT"
self._stub_out_reboot(device_name)
self.compute_api.reboot(self.context, inst_ref, reboot_type)
@ -6201,7 +6255,15 @@ class ComputeAPITestCase(BaseTestCase):
db.instance_destroy(self.context, inst_ref['uuid'])
def test_reboot_hard(self):
def test_soft_reboot(self):
# Ensure instance can be rebooted while in error state.
self._test_reboot_soft(vm_state=vm_states.ACTIVE)
def test_soft_reboot_of_instance_in_error(self):
# Ensure instance can be rebooted while in error state.
self._test_reboot_soft(vm_state=vm_states.ERROR)
def test_reboot_hard(self, vm_state=vm_states.ACTIVE):
# Ensure instance can be hard rebooted.
instance = jsonutils.to_primitive(self._create_fake_instance())
self.compute.run_instance(self.context, instance=instance)
@ -6218,6 +6280,9 @@ class ComputeAPITestCase(BaseTestCase):
inst_ref = db.instance_get_by_uuid(self.context, instance['uuid'])
self.assertEqual(inst_ref['task_state'], None)
db.instance_update(self.context, instance['uuid'],
{"vm_state": vm_state})
reboot_type = "HARD"
self._stub_out_reboot(device_name)
self.compute_api.reboot(self.context, inst_ref, reboot_type)
@ -6227,6 +6292,10 @@ class ComputeAPITestCase(BaseTestCase):
db.instance_destroy(self.context, inst_ref['uuid'])
def test_hard_reboot_of_instance_in_error(self):
# Ensure instance can be rebooted while in error state.
self.test_reboot_hard(vm_state=vm_states.ERROR)
def test_hard_reboot_of_soft_rebooting_instance(self):
# Ensure instance can be hard rebooted while soft rebooting.
instance = jsonutils.to_primitive(self._create_fake_instance())
@ -6263,7 +6332,7 @@ class ComputeAPITestCase(BaseTestCase):
inst_ref,
reboot_type)
def test_soft_reboot_of_rescued_instance(self):
def test_reboot_of_rescued_instance(self):
# Ensure instance can't be rebooted while in rescued state.
instance = jsonutils.to_primitive(self._create_fake_instance())
self.compute.run_instance(self.context, instance=instance)
@ -6287,6 +6356,33 @@ class ComputeAPITestCase(BaseTestCase):
inst_ref,
'HARD')
def test_reboot_of_instance_in_error_not_launched(self):
# Ensure instance can be not rebooted while in error states
# if they have never been booted at least once.
instance = jsonutils.to_primitive(self._create_fake_instance())
self.compute.run_instance(self.context, instance=instance)
inst_ref = db.instance_get_by_uuid(self.context, instance['uuid'])
self.assertEqual(inst_ref['task_state'], None)
db.instance_update(self.context, instance['uuid'],
{"vm_state": vm_states.ERROR,
"launched_at": None})
inst_ref = db.instance_get_by_uuid(self.context, inst_ref['uuid'])
self.assertRaises(exception.InstanceInvalidState,
self.compute_api.reboot,
self.context,
inst_ref,
'SOFT')
self.assertRaises(exception.InstanceInvalidState,
self.compute_api.reboot,
self.context,
inst_ref,
'HARD')
def test_hostname_create(self):
# Ensure instance hostname is set during creation.
inst_type = flavors.get_flavor_by_name('m1.tiny')
@ -7689,7 +7785,8 @@ class ComputeAPITestCase(BaseTestCase):
self.assertRaises(exception.InvalidDevicePath,
self.compute_api.attach_volume,
self.context,
{'locked': False, 'vm_state': vm_states.ACTIVE},
{'locked': False, 'vm_state': vm_states.ACTIVE,
'launched_at': timeutils.utcnow()},
None,
'/invalid')
@ -7996,6 +8093,7 @@ class ComputeAPITestCase(BaseTestCase):
# Ensure exception is raised while detaching an un-attached volume
instance = {'uuid': 'uuid1',
'locked': False,
'launched_at': timeutils.utcnow(),
'vm_state': vm_states.ACTIVE}
volume = {'id': 1, 'attach_status': 'detached'}
@ -8008,6 +8106,7 @@ class ComputeAPITestCase(BaseTestCase):
# instance doesn't match.
instance = {'uuid': 'uuid1',
'locked': False,
'launched_at': timeutils.utcnow(),
'vm_state': vm_states.ACTIVE}
volume = {'id': 1, 'attach_status': 'in-use',
'instance_uuid': 'uuid2'}
@ -8322,6 +8421,7 @@ class ComputeAPITestCase(BaseTestCase):
def test_fail_evacuate_from_non_existing_host(self):
inst = {}
inst['vm_state'] = vm_states.ACTIVE
inst['launched_at'] = timeutils.utcnow()
inst['image_ref'] = FAKE_IMAGE_REF
inst['reservation_id'] = 'r-fakeres'
inst['user_id'] = self.user_id