From 63d1fb5cd61c90bd2855c6b836849dd4fc4d8290 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 18 Jul 2024 13:59:58 +0200 Subject: [PATCH] [IMP] runbot: store docker build output in a dedicated model When a docker fails to build, the output is logged in the chatter leading to a lot of noise and a not so readable output. Moreover, the output tries to format markdown and don't render line break correctly. This commit proposes to introduce a model to store this output, as well as some other info like the image identifier, build time, ... This will help to compare images versions between hosts and should be useful later to have multiple version of the same image with variant once the docker registry is introduced. --- README.md | 2 +- runbot/container.py | 10 ++-- runbot/models/__init__.py | 2 +- runbot/models/{dockerfile.py => docker.py} | 31 ++++++++++ runbot/models/host.py | 44 +++++++++++--- runbot/models/runbot.py | 2 +- runbot/security/ir.model.access.csv | 3 + runbot/templates/dockerfile.xml | 10 ++-- runbot/tests/test_dockerfile.py | 4 +- runbot/views/dockerfile_views.xml | 69 ++++++++++++++++++++-- runbot/views/menus.xml | 4 +- runbot_builder/builder.py | 2 +- runbot_builder/tools.py | 2 +- 13 files changed, 155 insertions(+), 30 deletions(-) rename runbot/models/{dockerfile.py => docker.py} (66%) diff --git a/README.md b/README.md index 72797379..feb695ac 100644 --- a/README.md +++ b/README.md @@ -309,7 +309,7 @@ Runbot is using a Dockerfile Odoo model to define the Dockerfile used for builds The model is using Odoo QWeb views as templates. -A new Dockerfile can be created as needed either by duplicating the default one and adapt parameters in the view. e.g.: changing the key `'from': 'ubuntu:bionic'` to `'from': 'debian:buster'` will create a new Dockerfile based on Debian instead of ubuntu. +A new Dockerfile can be created as needed either by duplicating the default one and adapt parameters in the view. e.g.: changing the key `'from': 'ubuntu:jammy'` to `'from': 'debian:buster'` will create a new Dockerfile based on Debian instead of ubuntu. Or by providing a plain Dockerfile in the template. Once the Dockerfile is created and the `to_build` field is checked, the Dockerfile will be built (pay attention that no other operations will occur during the build). diff --git a/runbot/container.py b/runbot/container.py index d0789297..6312230c 100644 --- a/runbot/container.py +++ b/runbot/container.py @@ -110,15 +110,17 @@ def _docker_build(build_dir, image_tag): """ docker_client = docker.from_env() try: - docker_client.images.build(path=build_dir, tag=image_tag, rm=True) + docker_image, result_stream = docker_client.images.build(path=build_dir, tag=image_tag, rm=True) + result_stream = list(result_stream) + msg = ''.join([r.get('stream', '') for r in result_stream]) + return docker_image, msg except docker.errors.APIError as e: - _logger.error('Build of image %s failed with this API error:', image_tag) + _logger.error('Build of image %s failed', image_tag) return (False, e.explanation) except docker.errors.BuildError as e: - _logger.error('Build of image %s failed with this BUILD error:', image_tag) + _logger.error('Build of image %s failed', image_tag) msg = f"{e.msg}\n{''.join(l.get('stream') or '' for l in e.build_log)}" return (False, msg) - return (True, None) def docker_run(*args, **kwargs): diff --git a/runbot/models/__init__.py b/runbot/models/__init__.py index 45c17f34..1da4e0e4 100644 --- a/runbot/models/__init__.py +++ b/runbot/models/__init__.py @@ -11,7 +11,7 @@ from . import codeowner from . import commit from . import custom_trigger from . import database -from . import dockerfile +from . import docker from . import host from . import ir_cron from . import ir_http diff --git a/runbot/models/dockerfile.py b/runbot/models/docker.py similarity index 66% rename from runbot/models/dockerfile.py rename to runbot/models/docker.py index c02ad3e0..aa4fd0d1 100644 --- a/runbot/models/dockerfile.py +++ b/runbot/models/docker.py @@ -22,6 +22,8 @@ class Dockerfile(models.Model): view_ids = fields.Many2many('ir.ui.view', compute='_compute_view_ids', groups="runbot.group_runbot_admin") project_ids = fields.One2many('runbot.project', 'dockerfile_id', string='Default for Projects') bundle_ids = fields.One2many('runbot.bundle', 'dockerfile_id', string='Used in Bundles') + build_results = fields.One2many('runbot.docker_build_result', 'dockerfile_id', string='Build results') + last_successful_result = fields.Many2one('runbot.docker_build_result', compute='_compute_last_successful_result') _sql_constraints = [('runbot_dockerfile_name_unique', 'unique(name)', 'A Dockerfile with this name already exists')] @@ -33,6 +35,10 @@ class Dockerfile(models.Model): copied_record.template_id.key = '%s (copy)' % copied_record.template_id.key return copied_record + def _compute_last_successful_result(self): + for record in self: + record.last_successful_result = next((result for result in record.build_results if result.result == 'success'), record.build_results.browse()) + @api.depends('template_id.arch_base') def _compute_dockerfile(self): for rec in self: @@ -53,3 +59,28 @@ class Dockerfile(models.Model): for rec in self: keys = re.findall(r' 5: + summary = line + break + record.summary = summary diff --git a/runbot/models/host.py b/runbot/models/host.py index 8766784d..df80f898 100644 --- a/runbot/models/host.py +++ b/runbot/models/host.py @@ -143,19 +143,45 @@ class Host(models.Model): USER {user} ENV COVERAGE_FILE /data/build/.coverage """ + content = dockerfile.dockerfile + docker_append with open(self.env['runbot.runbot']._path('docker', dockerfile.image_tag, 'Dockerfile'), 'w') as Dockerfile: - Dockerfile.write(dockerfile.dockerfile + docker_append) + Dockerfile.write(content) - docker_build_success, msg = docker_build(docker_build_path, dockerfile.image_tag) - if not docker_build_success: - dockerfile.to_build = False - dockerfile.message_post(body=f'Build failure:\n{msg}') - # self.env['runbot.runbot']._warning(f'Dockerfile build "{dockerfile.image_tag}" failed on host {self.name}') - else: - duration = time.time() - start + docker_build_identifier, msg = docker_build(docker_build_path, dockerfile.image_tag) + duration = time.time() - start + docker_build_result_values = {'dockerfile_id': dockerfile.id, 'output': msg, 'duration': duration, 'content': content, 'host_id': self.id} + duration = time.time() - start + if docker_build_identifier: + docker_build_result_values['result'] = 'success' + docker_build_result_values['identifier'] = docker_build_identifier.id if duration > 1: _logger.info('Dockerfile %s finished build in %s', dockerfile.image_tag, duration) - + else: + docker_build_result_values['result'] = 'error' + dockerfile.to_build = False + + should_save_result = not docker_build_identifier # always save in case of failure + if not should_save_result: + # check previous result anyway + previous_result = self.env['runbot.docker_build_result'].search([ + ('dockerfile_id', '=', dockerfile.id), + ('host_id', '=', self.id), + ], order='id desc', limit=1) + # identifier changed + if docker_build_identifier.id != previous_result.identifier: + should_save_result = True + if previous_result.output != docker_build_result_values['output']: # to discuss + should_save_result = True + if previous_result.content != docker_build_result_values['content']: # docker image changed + should_save_result = True + + if should_save_result: + result = self.env['runbot.docker_build_result'].create(docker_build_result_values) + if not docker_build_identifier: + message = f'Build failure, check results for more info ({result.summary})' + dockerfile.message_post(body=message) + _logger.error(message) + @ormcache() def _host_list(self): return {host.name: host.id for host in self.search([])} diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index 4c472aaf..3fa9d902 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -34,7 +34,7 @@ class Runbot(models.AbstractModel): def _root(self): """Return root directory of repository""" return os.path.abspath(os.sep.join([os.path.dirname(__file__), '../static'])) - + def _path(self, *path_parts): """Return the repo build path""" root = self.env['runbot.runbot']._root() diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index 67f6ec7e..b18bd0c4 100644 --- a/runbot/security/ir.model.access.csv +++ b/runbot/security/ir.model.access.csv @@ -128,6 +128,9 @@ access_runbot_upgrade_exception_admin,access_runbot_upgrade_exception_admin,runb access_runbot_dockerfile_user,access_runbot_dockerfile_user,runbot.model_runbot_dockerfile,runbot.group_user,1,0,0,0 access_runbot_dockerfile_admin,access_runbot_dockerfile_admin,runbot.model_runbot_dockerfile,runbot.group_runbot_admin,1,1,1,1 +access_runbot_docker_build_result_user,access_runbot_docker_build_result_user,runbot.model_runbot_docker_build_result,runbot.group_user,1,0,0,0 +access_runbot_docker_build_result_admin,access_runbot_docker_build_result_admin,runbot.model_runbot_docker_build_result,runbot.group_runbot_admin,1,1,1,1 + access_runbot_codeowner_admin,runbot_codeowner_admin,runbot.model_runbot_codeowner,runbot.group_runbot_admin,1,1,1,1 access_runbot_codeowner_user,runbot_codeowner_user,runbot.model_runbot_codeowner,group_user,1,0,0,0 diff --git a/runbot/templates/dockerfile.xml b/runbot/templates/dockerfile.xml index 8de28b6c..7311f746 100644 --- a/runbot/templates/dockerfile.xml +++ b/runbot/templates/dockerfile.xml @@ -56,7 +56,7 @@ RUN curl -sSL -o /tmp/wkhtml.deb \