From 3437baedf646c7cd3da43440368edc194a880db8 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 3 May 2018 15:00:02 -0400 Subject: [PATCH] Fix being able to hard reboot a pausing instance The allowed task states for a hard reboot include 'pausing' but the instance.save(expected_task_states) doesn't include 'pausing', so if you try to hard reboot a pausing instance it will fail with UnexpectedTaskStateError. This makes the expected_task_states passed to Instance.save use the same list of allowed task states that we use in the check_instance_state decorator, and re-writes the unit test to use an actual database to verify the Instance.save() behavior and task state check in the DB API. While we're at it, the API reference is updated to indicate the allowed states for each type of reboot. Change-Id: I5a21350e48a637e581d269fb567bb96c1899e174 Closes-Bug: #1768927 --- api-ref/source/servers-actions.inc | 23 +++++++++++++++++++++ nova/compute/api.py | 10 +-------- nova/tests/unit/compute/test_compute.py | 12 +++++++++++ nova/tests/unit/compute/test_compute_api.py | 12 +---------- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/api-ref/source/servers-actions.inc b/api-ref/source/servers-actions.inc index 037c273645..23354bb8f1 100644 --- a/api-ref/source/servers-actions.inc +++ b/api-ref/source/servers-actions.inc @@ -423,6 +423,29 @@ Reboots a server. Specify the ``reboot`` action in the request body. +**Preconditions** + +The preconditions for rebooting a server depend on the type of reboot. + +You can only *SOFT* reboot a server when its status is ``ACTIVE``. + +You can only *HARD* reboot a server when its status is one of: + +* ``ACTIVE`` +* ``ERROR`` +* ``HARD_REBOOT`` +* ``PAUSED`` +* ``REBOOT`` +* ``SHUTOFF`` +* ``SUSPENDED`` + +If the server is locked, you must have administrator privileges +to reboot the server. + +**Asynchronous Postconditions** + +After you successfully reboot a server, its status changes to ``ACTIVE``. + Normal response codes: 202 Error response codes: unauthorized(401), forbidden(403), itemNotFound(404), diff --git a/nova/compute/api.py b/nova/compute/api.py index 2325a9d615..b0d4bf8cc8 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2963,15 +2963,7 @@ class API(base.Base): task_state=task_states.ALLOW_REBOOT) def _hard_reboot(self, context, instance): instance.task_state = task_states.REBOOTING_HARD - expected_task_state = [None, - task_states.REBOOTING, - task_states.REBOOT_PENDING, - task_states.REBOOT_STARTED, - task_states.REBOOTING_HARD, - task_states.RESUMING, - task_states.UNPAUSING, - task_states.SUSPENDING] - instance.save(expected_task_state = expected_task_state) + instance.save(expected_task_state=task_states.ALLOW_REBOOT) self._record_action_start(context, instance, instance_actions.REBOOT) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 4bf9ceb21a..edb5dccb3e 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -3239,6 +3239,18 @@ class ComputeTestCase(BaseTestCase, def test_reboot_fail_running(self): self._test_reboot(False, fail_reboot=True, fail_running=True) + def test_reboot_hard_pausing(self): + # We need an actual instance in the database for this test to make + # sure that expected_task_state works OK with Instance.save(). + instance = self._create_fake_instance_obj( + params={'task_state': task_states.PAUSING}) + with mock.patch.object(self.compute_api.compute_rpcapi, + 'reboot_instance') as rpc_reboot: + self.compute_api.reboot(self.context, instance, 'HARD') + rpc_reboot.assert_called_once_with( + self.context, instance=instance, block_device_info=None, + reboot_type='HARD') + def test_get_instance_block_device_info_source_image(self): bdms = block_device_obj.block_device_make_list(self.context, [fake_block_device.FakeDbBlockDeviceDict({ diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 6fa7e63b43..cc8a4bd54d 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -840,13 +840,7 @@ class _ComputeAPIUnitTestMixIn(object): expected_task_state = [None] if reboot_type == 'HARD': - expected_task_state.extend([task_states.REBOOTING, - task_states.REBOOT_PENDING, - task_states.REBOOT_STARTED, - task_states.REBOOTING_HARD, - task_states.RESUMING, - task_states.UNPAUSING, - task_states.SUSPENDING]) + expected_task_state = task_states.ALLOW_REBOOT if self.cell_type == 'api': rpcapi = self.compute_api.cells_rpcapi @@ -903,10 +897,6 @@ class _ComputeAPIUnitTestMixIn(object): self._test_reboot_type(vm_states.ACTIVE, 'HARD', task_state=task_states.RESUMING) - def test_reboot_hard_pausing(self): - self._test_reboot_type(vm_states.ACTIVE, - 'HARD', task_state=task_states.PAUSING) - def test_reboot_hard_unpausing(self): self._test_reboot_type(vm_states.ACTIVE, 'HARD', task_state=task_states.UNPAUSING)