diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py index 39d2b8ea4..45b62cea6 100644 --- a/ironic_inspector/introspect.py +++ b/ironic_inspector/introspect.py @@ -63,7 +63,11 @@ def introspect(node_id, manage_boot=True, token=None): ironic=ironic) if manage_boot: - utils.executor().submit(_do_introspect, node_info, ironic) + 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) diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index e5beb0e65..054186e32 100644 --- a/ironic_inspector/main.py +++ b/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//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) diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 3c75509b0..7b0616939 100644 --- a/ironic_inspector/node_cache.py +++ b/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) diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index 60531a183..38d3d55e2 100644 --- a/ironic_inspector/test/functional.py +++ b/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) diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index d1377f303..b826f3b08 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/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): diff --git a/lower-constraints.txt b/lower-constraints.txt index e3d018260..4f1c22864 100644 --- a/lower-constraints.txt +++ b/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 diff --git a/releasenotes/notes/reply-with-content-type-644b741261c87c8c.yaml b/releasenotes/notes/reply-with-content-type-644b741261c87c8c.yaml new file mode 100644 index 000000000..347cc4e66 --- /dev/null +++ b/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. diff --git a/requirements.txt b/requirements.txt index 27b08bbd9..862e81790 100644 --- a/requirements.txt +++ b/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