PR#694: Add new 'module-build-logs' endpoint which returns the build log when available. - fm-orchestrator

fm-orchestrator

#694 Add new 'module-build-logs' endpoint which returns the build log when available.

Closed 6 years ago by mprahl. Opened 6 years ago by jkaluza.

jkaluza/fm-orchestrator module-build-logs into master

Add new 'module-build-logs' endpoint which returns the build log when available.

Jan Kaluza • 6 years ago

bd161cf

module_build_service/views.py

file modified

+25 -1

		`@@ -26,12 +26,13 @@`
		`This is the implementation of the orchestrator's public RESTful API.`
		`"""`

		`+ import os`
		`import json`
		`import module_build_service.auth`
		`from flask import request, jsonify`
		`from flask.views import MethodView`

		`- from module_build_service import app, conf, log`
		`+ from module_build_service import app, conf, log, build_logs`
		`from module_build_service import models, db`
		`from module_build_service.utils import (`
		`pagination_metadata, filter_module_builds, filter_component_builds,`
		`@@ -60,6 +61,12 @@`
		`'methods': ['GET', 'PATCH'],`
		`}`
		`},`
		`+ 'build_log': {`
		`+ 'url': '/module-build-service/1/module-build-logs/<int:id>',`
		`+ 'options': {`
		`+ 'methods': ['GET'],`
		`+ }`
		`+ },`
		`'component_builds_list': {`
		`'url': '/module-build-service/1/component-builds/',`
		`'options': {`
		`@@ -76,6 +83,17 @@`
		`}`


		`+ class BuildLogAPI(MethodView):`
		`+`
		`+ def get(self, id):`
		`+ path = build_logs.path(id)`
		`+ if not os.path.exists(path):`
		`+ raise NotFound('Module build log not found')`
		`+`
		`+ with open(path, "r") as logfile:`
		`+ return logfile.read(), 200`
fivaldi commented 6 years ago I am thinking of streaming approach instead of just `.read()`. What do you think? See: http://flask.pocoo.org/docs/0.12/patterns/streaming/
		`+`
		`+`
		`class ComponentBuildAPI(MethodView):`

		`def get(self, id):`
		`@@ -291,6 +309,7 @@`
		`""" Registers version 1 of MBS API. """`
		`module_view = ModuleBuildAPI.as_view('module_builds')`
		`component_view = ComponentBuildAPI.as_view('component_builds')`
		`+ build_log_view = BuildLogAPI.as_view('module_build_logs')`
		`for key, val in api_v1.items():`
		`if key.startswith('component_build'):`
		`app.add_url_rule(val['url'],`
		`@@ -302,6 +321,11 @@`
		`endpoint=key,`
		`view_func=module_view,`
		`**val['options'])`
		`+ elif key.startswith('build_log'):`
		`+ app.add_url_rule(val['url'],`
		`+ endpoint=key,`
		`+ view_func=build_log_view,`
		`+ **val['options'])`
		`else:`
		`raise NotImplementedError("Unhandled api key.")`

tests/test_views/test_views.py

file modified

+21 -1

		`@@ -28,7 +28,7 @@`
		`import modulemd as _modulemd`
		`import module_build_service.scm`

		`- from mock import patch, PropertyMock, MagicMock`
		`+ from mock import patch, PropertyMock, MagicMock, mock_open`
		`from shutil import copyfile`
		`from os import path, mkdir`
		`from os.path import dirname`
		`@@ -928,3 +928,23 @@`
		`allow_custom_scmurls.return_value = True`
		`res2 = submit('git://some.custom.url.org/modules/testmodule.git?#68931c9')`
		`self.assertEquals(res2.status_code, 201)`
		`+`
		`+ @patch('module_build_service.views.open',`
		`+ mock_open(read_data="Log file example"), create=True)`
		`+ @patch('os.path.exists', return_value=True)`
		`+ def test_get_build_log(self, mocked_exists):`
		`+ res = self.client.get('/module-build-service/1/module-build-logs/1')`
		`+ self.assertEqual(res.data, "Log file example")`
		`+`
		`+ @patch('module_build_service.views.open',`
		`+ mock_open(read_data="Log file example"), create=True)`
		`+ @patch('os.path.exists', return_value=False)`
		`+ def test_get_build_log_log_removed(self, mocked_exists):`
		`+ res = self.client.get('/module-build-service/1/module-build-logs/1')`
		`+ data = json.loads(res.data)`
		`+ self.assertEquals(data['status'], 404)`
		`+ self.assertEquals(data['message'], "Module build log not found")`
		`+`
		`+ def test_get_build_log_no_id(self):`
		`+ res = self.client.get('/module-build-service/1/module-build-logs')`
		`+ self.assertEquals(res.status_code, 404)`

jkaluza commented 6 years ago

MBS is generating build logs to upload them to Koji when module build is successfully done. In case the module build fails, there is no way to include the build log to Koji - Koji content generator cannot generate failed build.

This new "module-build-logs" endpoint allows people to access the build log of any module build - even the failed one. This is especially useful for MBS admins, because they don't have to login the machine to check the journalctl or other logging backend and search there for all log messages related to particular build. Instead, they can just query MBS to get the build log relevant to the module build.

Since the failed build logs are not by default stored permanently, they can be removed after some time by cron or other service (Although we are not doing that right now). Therefore this does not have to return the log in all cases, but there is very big change that for some time after the module build fail, the log will be available via MBS API.

ralph commented 6 years ago

Hm... will this work in the prod environ? There the frontend and backend are split between two different VMs which I don't think have shared storage.

A local test would work, though - because the two processes are on the same machine.

ralph commented 6 years ago

We could introduce shared storage between them if that's the only way, @jkaluza.

I just did it for the ODCS node, so I know how now. :)

jkaluza commented 6 years ago

I'm thinking if we need that feature or not. I would myself like it, since I would have per-build-id MBS logs without the need to ssh backend and parsing the journalctl -f myself to find out what line belogs to what build id. If you think this is good idea, lets do it.

ralph commented 6 years ago

It is. Let's do it.

fivaldi commented on line 41 of module_build_service/views.py 6 years ago

I am thinking of streaming approach instead of just .read(). What do you think? See: http://flask.pocoo.org/docs/0.12/patterns/streaming/

ralph commented 6 years ago

I'm not sure how well that will work through our proxy layer (in Fedora).

We have:

proxy01 runs apache with reverseproxy turned on. Requests get their tls terminated here. reverseproxy points the request at haproxy.
proxy01 runs haproxy. receives reverseproxy requests from apache. haproxy points the request to the app node.
mbs-frontend01 runs apache. receives connection from haproxy. hosts the wsgi-app which would do the .read() or the streaming response.

In my opinion, we should/could implement both. Having a simple .read() is safe. Adding a streaming approach for a /watchlogs endpoint could be a nice addition, but again I'd keep both just to be safe in case streaming /watchlogs falls over in the prod environment.

Edited 6 years ago by ralph

fivaldi commented 6 years ago

My original thought was just protecting the app from (possibly) memory-hungry .read() operation. I consulted this with @lbalhar and we agreed that the generator approach should be the safest solution.

:thumbsup: for @ralph's /watchlogs enhancement which is great idea (in my opinion) as another PR/issue. This could re-use the generator approach (if implemented in this PR). It would just tail the logfile, streaming its contents.