Merge "Respond so Apache HTTPd doesn't think the request failed"

ironic_inspector/introspect.py

@@ -63,7 +63,11 @@ def introspect(node_id, manage_boot=True, token=None):
                                                ironic=ironic)
 
     if manage_boot:
+        try:
             utils.executor().submit(_do_introspect, node_info, ironic)
+        except Exception as exc:
+            msg = _('Failed to submit introspection job: %s')
+            raise utils.Error(msg % exc, node_info=node)
     else:
         _do_introspect(node_info, ironic)
 

ironic_inspector/main.py

@@ -130,6 +130,31 @@ def error_response(exc, code=500):
     return res
 
 
+def _generate_empty_response(code):
+    """Change the content mime type to text/plain.
+
+    :param code: The HTTP response code as an integer.
+    :returns: An empty flask response object with the
+              requested return code.
+    """
+    # NOTE(TheJulia): Explicitly set a mime type and body on the
+    # response as some proxies view the lack of a mime type as a
+    # failure when the request was actually successful.
+    # Strictly speaking, 204s should have no body, where as 202's
+    # don't strictly require or expect content, but content can
+    # be included for user friendly response bodies.
+    if code == 204:
+        response = flask.make_response('', code)
+        response.mimetype = 'text/plain'
+    else:
+        # Send an empty dictionary to set a mimetype, and ultimately
+        # with this being a rest API we can, at some point, choose to
+        # convey some sort of status response back in the message
+        # body.
+        response = flask.make_response({}, code)
+    return response
+
+
 def convert_exceptions(func):
     @functools.wraps(func)
     def wrapper(*args, **kwargs):
@@ -333,7 +358,7 @@ def api_introspection(node_id):
         client.call({}, 'do_introspection', node_id=node_id,
                     manage_boot=manage_boot,
                     token=flask.request.headers.get('X-Auth-Token'))
-        return '', 202
+        return _generate_empty_response(202)
     else:
         node_info = node_cache.get_node(node_id)
         return flask.json.jsonify(generate_introspection_status(node_info))
@@ -358,7 +383,7 @@ def api_introspection_abort(node_id):
     client = get_client_compat()
     client.call({}, 'do_abort', node_id=node_id,
                 token=flask.request.headers.get('X-Auth-Token'))
-    return '', 202
+    return _generate_empty_response(202)
 
 
 @api('/v1/introspection/<node_id>/data', rule="introspection:data",
@@ -399,7 +424,7 @@ def api_introspection_reapply(node_id):
 
     client = get_client_compat()
     client.call({}, 'do_reapply', node_uuid=node_id, data=data)
-    return '', 202
+    return _generate_empty_response(202)
 
 
 def rule_repr(rule, short):
@@ -421,7 +446,7 @@ def api_rules():
         return flask.jsonify(rules=res)
     elif flask.request.method == 'DELETE':
         rules.delete_all()
-        return '', 204
+        return _generate_empty_response(204)
     else:
         body = flask.request.get_json(force=True)
         if body.get('uuid') and not uuidutils.is_uuid_like(body['uuid']):
@@ -452,7 +477,7 @@ def api_rule(uuid):
         return flask.jsonify(rule_repr(rule, short=False))
     else:
         rules.delete(uuid)
-        return '', 204
+        return _generate_empty_response(204)
 
 
 @_app.errorhandler(404)

ironic_inspector/node_cache.py

@@ -100,6 +100,8 @@ class NodeInfo(object):
         :returns: boolean value, whether lock was acquired successfully
         """
         if self._lock.is_locked():
+            LOG.debug('Attempting to acquire lock already held',
+                      node_info=self)
             return True
 
         LOG.debug('Attempting to acquire lock', node_info=self)

ironic_inspector/test/functional.py

@@ -629,7 +629,7 @@ class Test(Base):
 
         res = self.call_reapply(self.uuid)
         self.assertEqual(202, res.status_code)
-        self.assertEqual('', res.text)
+        self.assertEqual('{}\n', res.text)
         eventlet.greenthread.sleep(DEFAULT_SLEEP)
 
         status = self.call_get_status(self.uuid)
@@ -642,7 +642,7 @@ class Test(Base):
         # second reapply call
         res = self.call_reapply(self.uuid)
         self.assertEqual(202, res.status_code)
-        self.assertEqual('', res.text)
+        self.assertEqual('{}\n', res.text)
         eventlet.greenthread.sleep(DEFAULT_SLEEP)
 
         # Reapply with provided data
@@ -650,7 +650,7 @@ class Test(Base):
         new_data['inventory']['cpu']['count'] = 42
         res = self.call_reapply(self.uuid, data=new_data)
         self.assertEqual(202, res.status_code)
-        self.assertEqual('', res.text)
+        self.assertEqual('{}\n', res.text)
         eventlet.greenthread.sleep(DEFAULT_SLEEP)
 
         self.check_status(status, finished=True, state=istate.States.finished)

ironic_inspector/test/unit/test_main.py

@@ -75,6 +75,9 @@ class TestApiIntrospect(BaseAPITest):
                                                       node_id=self.uuid,
                                                       manage_boot=True,
                                                       token=None)
+        self.assertEqual('application/json',
+                         res.headers['content-type'])
+        self.assertEqual(b'{}\n', res.data)
 
     def test_intospect_failed(self):
         self.client_mock.call.side_effect = utils.Error("boom")
@@ -98,6 +101,9 @@ class TestApiIntrospect(BaseAPITest):
                                                       node_id=self.uuid,
                                                       manage_boot=False,
                                                       token=None)
+        self.assertEqual('application/json',
+                         res.headers['content-type'])
+        self.assertEqual(b'{}\n', res.data)
 
     def test_introspect_can_manage_boot_false(self):
         CONF.set_override('can_manage_boot', False)
@@ -196,7 +202,9 @@ class TestApiAbort(BaseAPITest):
                                                       node_id=self.uuid,
                                                       token='token')
         self.assertEqual(202, res.status_code)
-        self.assertEqual(b'', res.data)
+        self.assertEqual(b'{}\n', res.data)
+        self.assertEqual('application/json',
+                         res.headers['content-type'])
 
     def test_no_authentication(self):
 
@@ -206,7 +214,9 @@ class TestApiAbort(BaseAPITest):
                                                       node_id=self.uuid,
                                                       token=None)
         self.assertEqual(202, res.status_code)
-        self.assertEqual(b'', res.data)
+        self.assertEqual(b'{}\n', res.data)
+        self.assertEqual('application/json',
+                         res.headers['content-type'])
 
     def test_node_not_found(self):
         exc = utils.Error("Not Found.", code=404)
@@ -559,6 +569,8 @@ class TestApiRules(BaseAPITest):
         res = self.app.delete('/v1/rules/' + self.uuid)
         self.assertEqual(204, res.status_code)
         delete_mock.assert_called_once_with(self.uuid)
+        self.assertEqual('text/plain; charset=utf-8',
+                         res.headers['content-type'])
 
 
 class TestApiMisc(BaseAPITest):

lower-constraints.txt

@@ -23,7 +23,7 @@ eventlet==0.18.2
 extras==1.0.0
 fasteners==0.15
 fixtures==3.0.0
-Flask==1.0
+Flask==1.1.0
 future==0.18.2
 futurist==1.2.0
 gitdb==4.0.5

releasenotes/notes/reply-with-content-type-644b741261c87c8c.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes an issue which may occur with Apache httpd webservers acting as a
+    proxy where the server may report ``Bad Gateway``, however inspector
+    continues operating as if there was no problem. This was due to a
+    lack of a ``Content-Type`` header on HTTP 202 and 204 replies,
+    and lack of message body with HTTP 202 messages which Apache httpd
+    can error upon.

requirements.txt

@@ -5,7 +5,7 @@ automaton>=1.9.0 # Apache-2.0
 alembic>=0.9.6 # MIT
 construct>=2.9.39 # MIT
 eventlet!=0.18.3,!=0.20.1,>=0.18.2 # MIT
-Flask>=1.0 # BSD
+Flask>=1.1.0 # BSD
 futurist>=1.2.0 # Apache-2.0
 ironic-lib>=4.3.0 # Apache-2.0
 jsonpath-rw<2.0,>=1.2.0 # Apache-2.0