openstack
/
nova
Code Issues Proposed changes

Avoid error 500 on shelve task_state race 

When shelving a server and saving its new SHELVING task state, we
expect the current task state to be None and do not handle any
UnexpectedTaskStateError exceptions that the database layer may throw
at us. In those cases, we return an error 500 to the user, when in
actuality an error 409 conflict would be more appropriate. This
patch makes the API layer handle UnexpectedTaskStateError exceptions
and return a 409 to the user.

Closes-bug: 1850694
Change-Id: Ie0b421cd5d3a5781c1dd09fab4ed013ece0f939d
This commit is contained in:
Artom Lifshitz 2019-10-30 14:03:49 -04:00 committed by Matt Riedemann
parent f33f8c2ba2
commit a423d8b277
2 changed files with 19 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/api/openstack/compute/shelve.py
View File

@ -48,7 +48,8 @@ class ShelveController(wsgi.Controller):
'project_id': instance.project_id})
try:
self.compute_api.shelve(context, instance)
except exception.InstanceIsLocked as e:
except (exception.InstanceIsLocked,
exception.UnexpectedTaskStateError) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except exception.InstanceInvalidState as state_error:
common.raise_http_conflict_for_instance_invalid_state(state_error,

17
nova/tests/unit/api/openstack/compute/test_shelve.py
View File

@ -21,6 +21,7 @@ import webob
from nova.api.openstack import api_version_request
from nova.api.openstack.compute import shelve as shelve_v21
from nova.compute import task_states
from nova.compute import vm_states
from nova import exception
from nova import policy
@ -46,6 +47,22 @@ class ShelvePolicyTestV21(test.NoDBTestCase):
self.assertRaises(webob.exc.HTTPConflict, self.controller._shelve,
self.req, uuidsentinel.fake, {})
@mock.patch('nova.api.openstack.common.get_instance')
@mock.patch('nova.objects.instance.Instance.save')
def test_shelve_task_state_race(self, mock_save, get_instance_mock):
instance = fake_instance.fake_instance_obj(
self.req.environ['nova.context'],
vm_state=vm_states.ACTIVE, task_state=None)
instance.launched_at = instance.created_at
get_instance_mock.return_value = instance
mock_save.side_effect = exception.UnexpectedTaskStateError(
instance_uuid=instance.uuid, expected=None,
actual=task_states.SHELVING)
ex = self.assertRaises(webob.exc.HTTPConflict, self.controller._shelve,
self.req, uuidsentinel.fake, body={'shelve': {}})
self.assertIn('Conflict updating instance', six.text_type(ex))
mock_save.assert_called_once_with(expected_task_state=[None])
@mock.patch('nova.api.openstack.common.get_instance')
def test_unshelve_locked_server(self, get_instance_mock):
get_instance_mock.return_value = (