From 8ff170dc95bf3101fe38a2624e941bfa3b7c1138 Mon Sep 17 00:00:00 2001 From: "Leandro I. Costantino" Date: Mon, 19 May 2014 19:58:47 -0300 Subject: [PATCH] VM in rescue state must have a restricted set of actions Right now it is possible to pause, suspend and stop a VM in state RESCUED, so after the state is changed, it's not possible to trigger unrescue anymore since the original state is lost. This patch remove vm_states.RESCUED as valid state from stop, pause and suspend actions. The vm_states devref is also updated to reflect this change including the current reboot flow.( vm_states.RESCUED cannot be rebooted as per today code) DocImpact Closes-Bug: #1319182 Co-Authored-By: Cyril Roelandt Change-Id: I531dea5a5499bf93c24bea37850d562134dee281 --- doc/source/devref/vmstates.rst | 7 ++-- nova/compute/api.py | 7 ++-- nova/tests/compute/test_compute_api.py | 46 ++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/doc/source/devref/vmstates.rst b/doc/source/devref/vmstates.rst index 80075124fd5a..4ab800ec6988 100644 --- a/doc/source/devref/vmstates.rst +++ b/doc/source/devref/vmstates.rst @@ -88,6 +88,7 @@ task states for various commands issued by the user: rescue -> error active -> rescue stopped -> rescue + error -> rescue unrescue [shape="rectangle"] unrescue -> active @@ -139,7 +140,9 @@ task states for various commands issued by the user: reboot -> error active -> reboot stopped -> reboot - rescued -> reboot + paused -> reboot + suspended -> reboot + error -> reboot live_migrate [shape="rectangle"] live_migrate -> active @@ -159,4 +162,4 @@ The following diagram shows the sequence of VM states, task states, and power states when a new VM instance is created. -.. image:: /images/run_instance_walkthrough.png \ No newline at end of file +.. image:: /images/run_instance_walkthrough.png diff --git a/nova/compute/api.py b/nova/compute/api.py index 95da15e8ad4d..55496439eb51 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1751,8 +1751,7 @@ class API(base.Base): @check_instance_lock @check_instance_host @check_instance_cell - @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED, - vm_states.ERROR]) + @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.ERROR]) def stop(self, context, instance, do_cast=True): """Stop an instance.""" self.force_stop(context, instance, do_cast) @@ -2509,7 +2508,7 @@ class API(base.Base): @wrap_check_policy @check_instance_lock @check_instance_cell - @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED]) + @check_instance_state(vm_state=[vm_states.ACTIVE]) def pause(self, context, instance): """Pause the given instance.""" instance.task_state = task_states.PAUSING @@ -2536,7 +2535,7 @@ class API(base.Base): @wrap_check_policy @check_instance_lock @check_instance_cell - @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED]) + @check_instance_state(vm_state=[vm_states.ACTIVE]) def suspend(self, context, instance): """Suspend the given instance.""" instance.task_state = task_states.SUSPENDING diff --git a/nova/tests/compute/test_compute_api.py b/nova/tests/compute/test_compute_api.py index 5a3105f65e64..2c8cf9a50318 100644 --- a/nova/tests/compute/test_compute_api.py +++ b/nova/tests/compute/test_compute_api.py @@ -67,6 +67,16 @@ class _ComputeAPIUnitTestMixIn(object): self.context = context.RequestContext(self.user_id, self.project_id) + def _get_vm_states(self, exclude_states=None): + vm_state = set([vm_states.ACTIVE, vm_states.BUILDING, vm_states.PAUSED, + vm_states.SUSPENDED, vm_states.RESCUED, vm_states.STOPPED, + vm_states.RESIZED, vm_states.SOFT_DELETED, + vm_states.DELETED, vm_states.ERROR, vm_states.SHELVED, + vm_states.SHELVED_OFFLOADED]) + if not exclude_states: + exclude_states = set() + return vm_state - exclude_states + def _create_flavor(self, params=None): flavor = {'id': 1, 'flavorid': 1, @@ -204,6 +214,19 @@ class _ComputeAPIUnitTestMixIn(object): self.assertEqual(task_states.SUSPENDING, instance.task_state) + def _test_suspend_fails(self, vm_state): + params = dict(vm_state=vm_state) + instance = self._create_instance_obj(params=params) + self.assertIsNone(instance.task_state) + self.assertRaises(exception.InstanceInvalidState, + self.compute_api.suspend, + self.context, instance) + + def test_suspend_fails_invalid_states(self): + invalid_vm_states = self._get_vm_states(set([vm_states.ACTIVE])) + for state in invalid_vm_states: + self._test_suspend_fails(state) + def test_resume(self): # Ensure instance can be resumed (if suspended). instance = self._create_instance_obj( @@ -309,13 +332,19 @@ class _ComputeAPIUnitTestMixIn(object): def test_stop_stopped_instance_with_bypass(self): self._test_stop(vm_states.STOPPED, force=True) - def test_stop_invalid_state(self): - params = dict(vm_state=vm_states.PAUSED) + def _test_stop_invalid_state(self, vm_state): + params = dict(vm_state=vm_state) instance = self._create_instance_obj(params=params) self.assertRaises(exception.InstanceInvalidState, self.compute_api.stop, self.context, instance) + def test_stop_fails_invalid_states(self): + invalid_vm_states = self._get_vm_states(set([vm_states.ACTIVE, + vm_states.ERROR])) + for state in invalid_vm_states: + self._test_stop_invalid_state(state) + def test_stop_a_stopped_inst(self): params = {'vm_state': vm_states.STOPPED} instance = self._create_instance_obj(params=params) @@ -1203,6 +1232,19 @@ class _ComputeAPIUnitTestMixIn(object): self.assertEqual(task_states.PAUSING, instance.task_state) + def _test_pause_fails(self, vm_state): + params = dict(vm_state=vm_state) + instance = self._create_instance_obj(params=params) + self.assertIsNone(instance.task_state) + self.assertRaises(exception.InstanceInvalidState, + self.compute_api.pause, + self.context, instance) + + def test_pause_fails_invalid_states(self): + invalid_vm_states = self._get_vm_states(set([vm_states.ACTIVE])) + for state in invalid_vm_states: + self._test_pause_fails(state) + def test_unpause(self): # Ensure instance can be unpaused. params = dict(vm_state=vm_states.PAUSED)