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

Author: Eric Fried

api-ref/source/os-server-external-events.inc

@@ -32,11 +32,15 @@ updated ``code`` and ``status`` indicating their level of success.
 Normal response codes: 200, 207
 
 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.
 
-Error response codes: badRequest(400), unauthorized(401), forbidden(403),
-itemNotFound(404)
+Error response codes: badRequest(400), unauthorized(401), forbidden(403)
+
+.. 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
 -------

nova/api/openstack/compute/server_external_events.py

@@ -13,14 +13,12 @@
 #    under the License.
 
 from oslo_log import log as logging
-import webob
 
 from nova.api.openstack.compute.schemas import server_external_events
 from nova.api.openstack import wsgi
 from nova.api import validation
 from nova.compute import api as compute
 from nova import context as nova_context
-from nova.i18n import _
 from nova import objects
 from nova.policies import server_external_events as see_policies
 
@@ -65,7 +63,7 @@ def _get_instances_all_cells(self, context, instance_uuids,
 
         return instances
 
-    @wsgi.expected_errors((403, 404))
+    @wsgi.expected_errors(403)
     @wsgi.response(200)
     @validation.schema(server_external_events.create, '2.0', '2.50')
     @validation.schema(server_external_events.create_v251, '2.51', '2.75')
@@ -146,9 +144,6 @@ def create(self, req, body):
         if accepted_events:
             self.compute_api.external_instance_event(
                 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
         # we have a general way to do this

nova/tests/unit/api/openstack/compute/test_server_external_events.py

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