Nix os-server-external-events 404 condition

The POST /os-server-external-events API had the following confusing
behavior:

With multiple events in the payload, if *some* (but not all) were
dropped, the HTTP response was 207, with per-event 4xx error codes in
the payload. But if *all* of the events were dropped, the overall HTTP
response was 404 with no payload. Thus, especially for consumers sending
only one event at a time, it was impossible to distinguish e.g. "you
tried to send an event for a nonexistent instance" from "the instance
you specified hasn't landed on a host yet".

This fix gets rid of that sweeping 404 condition, so if *any* subset of
the events are dropped (including *all* of them), the HTTP response will
always be 207, and the payload will always contain granular per-event
error codes.

This effectively means the API can no longer return 404, ever.

Closes-Bug: #1855752
Change-Id: Ibad1b51e2cf50d00102295039b6e82bc00bec058
This commit is contained in:
Eric Fried 2019-12-09 09:58:53 -06:00
parent e937c5c6c4
commit e6f7425444
3 changed files with 24 additions and 18 deletions

View File

@ -32,11 +32,15 @@ updated ``code`` and ``status`` indicating their level of success.
Normal response codes: 200, 207 Normal response codes: 200, 207
A 200 will be returned if all events succeeded, 207 will be returned A 200 will be returned if all events succeeded, 207 will be returned
if some events could not be processed. The ``code`` attribute for the if any events could not be processed. The ``code`` attribute for the
event will explain further what went wrong. event will explain further what went wrong.
Error response codes: badRequest(400), unauthorized(401), forbidden(403), Error response codes: badRequest(400), unauthorized(401), forbidden(403)
itemNotFound(404)
.. note:: Prior to the fix for `bug 1855752`_, error response code 404 may be
erroneously returned when all events failed.
.. _bug 1855752: https://bugs.launchpad.net/nova/+bug/1855752
Request Request
------- -------

View File

@ -13,14 +13,12 @@
# under the License. # under the License.
from oslo_log import log as logging from oslo_log import log as logging
import webob
from nova.api.openstack.compute.schemas import server_external_events from nova.api.openstack.compute.schemas import server_external_events
from nova.api.openstack import wsgi from nova.api.openstack import wsgi
from nova.api import validation from nova.api import validation
from nova.compute import api as compute from nova.compute import api as compute
from nova import context as nova_context from nova import context as nova_context
from nova.i18n import _
from nova import objects from nova import objects
from nova.policies import server_external_events as see_policies from nova.policies import server_external_events as see_policies
@ -65,7 +63,7 @@ class ServerExternalEventsController(wsgi.Controller):
return instances return instances
@wsgi.expected_errors((403, 404)) @wsgi.expected_errors(403)
@wsgi.response(200) @wsgi.response(200)
@validation.schema(server_external_events.create, '2.0', '2.50') @validation.schema(server_external_events.create, '2.0', '2.50')
@validation.schema(server_external_events.create_v251, '2.51', '2.75') @validation.schema(server_external_events.create_v251, '2.51', '2.75')
@ -146,9 +144,6 @@ class ServerExternalEventsController(wsgi.Controller):
if accepted_events: if accepted_events:
self.compute_api.external_instance_event( self.compute_api.external_instance_event(
context, accepted_instances, accepted_events) context, accepted_instances, accepted_events)
else:
msg = _('No instances found for any event')
raise webob.exc.HTTPNotFound(explanation=msg)
# FIXME(cyeoh): This needs some infrastructure support so that # FIXME(cyeoh): This needs some infrastructure support so that
# we have a general way to do this # we have a general way to do this

View File

@ -16,7 +16,6 @@ import fixtures as fx
import mock import mock
from oslo_utils.fixture import uuidsentinel as uuids from oslo_utils.fixture import uuidsentinel as uuids
import six import six
import webob
from nova.api.openstack.compute import server_external_events \ from nova.api.openstack.compute import server_external_events \
as server_external_events_v21 as server_external_events_v21
@ -108,12 +107,17 @@ class ServerExternalEventsTestV21(test.NoDBTestCase):
result = response.obj result = response.obj
code = response._code code = response._code
self.assertEqual(1, api_method.call_count) if expected_events:
call = api_method.call_args_list[0] self.assertEqual(1, api_method.call_count)
args = call[0] call = api_method.call_args_list[0]
args = call[0]
call_instances = args[1] call_instances = args[1]
call_events = args[2] call_events = args[2]
else:
self.assertEqual(0, api_method.call_count)
call_instances = []
call_events = []
self.assertEqual(set(expected_uuids), self.assertEqual(set(expected_uuids),
set([instance.uuid for instance in call_instances])) set([instance.uuid for instance in call_instances]))
@ -157,11 +161,14 @@ class ServerExternalEventsTestV21(test.NoDBTestCase):
self.assertEqual(207, code) self.assertEqual(207, code)
def test_create_no_good_instances(self): def test_create_no_good_instances(self):
"""Always 207 with granular codes even if all fail; see bug 1855752."""
body = self.default_body body = self.default_body
body['events'][0]['server_uuid'] = MISSING_UUID body['events'][0]['server_uuid'] = MISSING_UUID
body['events'][1]['server_uuid'] = MISSING_UUID body['events'][1]['server_uuid'] = fake_instance_uuids[-1]
self.assertRaises(webob.exc.HTTPNotFound, result, code = self._assert_call(body, [], [])
self.api.create, self.req, body=body) self.assertEqual(404, result['events'][0]['code'])
self.assertEqual(422, result['events'][1]['code'])
self.assertEqual(207, code)
def test_create_bad_status(self): def test_create_bad_status(self):
body = self.default_body body = self.default_body