diff options
| -rw-r--r-- | nova/api/openstack/common.py | 3 | ||||
| -rw-r--r-- | nova/compute/api.py | 31 | ||||
| -rw-r--r-- | nova/tests/api/ec2/test_cloud.py | 6 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/contrib/test_admin_actions.py | 4 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/test_server_metadata.py | 3 | ||||
| -rw-r--r-- | nova/tests/compute/test_compute.py | 110 |
6 files changed, 143 insertions, 14 deletions
diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index bec919f4b..dd746e23d 100644 --- a/nova/api/openstack/common.py +++ b/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 diff --git a/nova/compute/api.py b/nova/compute/api.py index 1e24e8ce5..0a9b0e67b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -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.""" diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index 543bf4a62..22a6947f2 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -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) } diff --git a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py index 5e64bdf94..39eebbcd1 100644 --- a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py +++ b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py @@ -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): diff --git a/nova/tests/api/openstack/compute/test_server_metadata.py b/nova/tests/api/openstack/compute/test_server_metadata.py index fa25ad4a3..f0548ffa0 100644 --- a/nova/tests/api/openstack/compute/test_server_metadata.py +++ b/nova/tests/api/openstack/compute/test_server_metadata.py @@ -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} diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 928934f61..c887041f8 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -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() @@ -5722,6 +5726,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): @@ -5996,7 +6015,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) @@ -6027,6 +6046,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) @@ -6041,6 +6064,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': ''})) @@ -6187,7 +6238,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) @@ -6204,6 +6255,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) @@ -6213,7 +6267,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) @@ -6230,6 +6292,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) @@ -6239,6 +6304,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()) @@ -6275,7 +6344,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) @@ -6299,6 +6368,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') @@ -7738,7 +7834,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') @@ -8045,6 +8142,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'} @@ -8057,6 +8155,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'} @@ -8371,6 +8470,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 |