From 13035b58c3345e71510f4c43557309c750fe7881 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Tue, 6 Jan 2026 16:45:44 +0100 Subject: [PATCH] [IMP] runbot: add a cache system fro dockerfiles The `ADD` directive used for remote resources sometimes fails when the resource is not available. In order to avoid that kind of failure, this commit adds a kind of cache of those resources. In order to do that, the `ADD http...` directives are converted into `COPY` and the distant resource is downloaded in a cache directory when a `# CACHE xxxx` comment is found just before the `ADD`. Where `xxxx`is the cache validity duration in seconds. --- runbot/models/docker.py | 39 +++++++++++++- runbot/tests/test_dockerfile.py | 96 ++++++++++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 3 deletions(-) diff --git a/runbot/models/docker.py b/runbot/models/docker.py index cd4816ad9..f7f117f2d 100644 --- a/runbot/models/docker.py +++ b/runbot/models/docker.py @@ -2,8 +2,13 @@ import logging import os import re +import time +from pathlib import Path + import docker -from odoo import api, fields, models, exceptions +import requests + +from odoo import api, exceptions, fields, models from ..container import docker_build from ..fields import JsonDictField @@ -330,11 +335,41 @@ def _get_docker_metadata(self, image_id): return {'error': str(e)} return metadata + def _get_cached_content(self): + self.ensure_one() + cache_dir = Path(self.env['runbot.runbot']._path('docker', 'cache')) + cache_dir.mkdir(exist_ok=True) + cache_re = re.compile(r'^#\s?CACHE\s(?P\d{1,4})$') + add_re = re.compile(r'^ADD\s(?Phttp.*)$') + lines = self.dockerfile.split('\n') + for i, line in enumerate(lines): + if cache_match := cache_re.match(line): + if add_match := add_re.match(lines[i + 1]): + cache_duration = int(cache_match.group('duration')) + filename = re.sub(r'[^a-zA-Z0-9]', '_', add_match.group('url'))[:255] + lines[i + 1] = f'COPY cache/{filename}' + cache_file_path = cache_dir / filename + if not cache_file_path.exists() or time.time() - cache_file_path.lstat().st_mtime > cache_duration: + try: + with requests.get(add_match.group('url'), stream=True) as response: + response.raise_for_status() + with cache_file_path.open('wb') as cache_file: + for chunk in response.iter_content(chunk_size=8192): + cache_file.write(chunk) + except (requests.exceptions.HTTPError, requests.exceptions.RequestException): + if cache_file_path.exists(): + cache_file_path.touch() # to avoid spamming in case of failures + raise + return '\n'.join(lines) + def _build(self, host=None): tag_dir = re.sub(r'[^\w]', '_', self.image_tag) docker_build_path = self.env['runbot.runbot']._path('docker', tag_dir) os.makedirs(docker_build_path, exist_ok=True) - content = self.dockerfile + symlink_to_cache = Path(docker_build_path) / 'cache' + if not symlink_to_cache.exists(): + symlink_to_cache.symlink_to('../cache') + content = self._get_cached_content() with open(self.env['runbot.runbot']._path('docker', tag_dir, 'Dockerfile'), 'w') as Dockerfile: Dockerfile.write(content) result = docker_build(docker_build_path, self.image_future_tag, self.pull_on_build) diff --git a/runbot/tests/test_dockerfile.py b/runbot/tests/test_dockerfile.py index 213310872..4d7f0e431 100644 --- a/runbot/tests/test_dockerfile.py +++ b/runbot/tests/test_dockerfile.py @@ -3,10 +3,12 @@ import logging import os import re +import time from psycopg2.errors import UniqueViolation +from requests.exceptions import HTTPError from odoo import Command, exceptions -from unittest.mock import patch, mock_open +from unittest.mock import patch, mock_open, MagicMock from odoo.tests.common import tagged, HttpCase, mute_logger from .common import RunbotCase @@ -149,3 +151,95 @@ def test_dockerfile_variant_unique(self): 'name': 'Documentation2', 'parent_id': default_dockerfile.id, }) + + def test_dockerfile_cache_add(self): + dockerfile = self.env['runbot.dockerfile'].create({ + 'name': 'TestsAddCache', + 'to_build': True, + 'layer_ids': [ + Command.create({ + 'name': 'CacheAddTest', + 'layer_type': 'raw', + 'content': 'some useless content', + }), + ], + }) + + self.start_patcher('docker_username', 'odoo.addons.runbot.models.docker.USERNAME', new='TestUser') + + expected_content = """# CacheAddTest +some useless content + +USER TestUser +""" + + content = dockerfile._get_cached_content() + self.assertEqual(content, expected_content, 'Dockerfile without "ADD" should be left unchanged') + + raw_layer = """FROM ubuntu:noble +ADD https://nowhere.example.org/nothing.txt +""" + + expected_content = """# CacheAddTest +FROM ubuntu:noble +ADD https://nowhere.example.org/nothing.txt + + +USER TestUser +""" + dockerfile.layer_ids[0].content = raw_layer + content = dockerfile._get_cached_content() + self.assertEqual(content, expected_content, 'Dockerfile without "#CACHE" directive should be left unchanged') + + # Here we start the useful cache tests + raw_layer = """FROM ubuntu:noble +# CACHE 60 +ADD https://nowhere.example.org/nothing.txt +""" + + expected_content = """# CacheAddTest +FROM ubuntu:noble +# CACHE 60 +COPY cache/https___nowhere_example_org_nothing_txt + + +USER TestUser +""" + mock_response = MagicMock() + mock_response.iter_content.return_value = [b'small file content'] + self.start_patcher('docker_requests_get', 'odoo.addons.runbot.models.docker.requests.get', return_value=mock_response) + + # 1 - The cache file does not exists yet + self.start_patcher('docker_path_exists', 'odoo.addons.runbot.models.docker.Path.exists', return_value=False) + dockerfile.layer_ids[0].content = raw_layer + with patch('odoo.addons.runbot.models.docker.Path.open', mock_open()) as cache_file_mock: + content = dockerfile._get_cached_content() + cache_file_mock.assert_called_once_with('wb') + self.assertEqual(content, expected_content, 'Dockerfile with "#CACHE" should change the ADD directive to COPY') + + # 2 - The cache file exists but the cache duration is expired + self.patchers['docker_path_exists'].return_value = True + self.start_patcher('docker_path_lstat', 'odoo.addons.runbot.models.docker.Path.lstat') + self.patchers['docker_path_lstat'].return_value.st_mtime = time.time() - 100 + with patch('odoo.addons.runbot.models.docker.Path.open', mock_open()) as cache_file_mock: + content = dockerfile._get_cached_content() + cache_file_mock.assert_called_once_with('wb') + self.assertEqual(content, expected_content, 'Dockerfile with "#CACHE" should change the ADD directive to COPY') + + # 3 - The cache file exists but the cache duration is not expired + self.start_patcher('docker_path_touch', 'odoo.addons.runbot.models.docker.Path.touch', return_value=True) + self.patchers['docker_path_lstat'].return_value.st_mtime = time.time() - 2 + with patch('odoo.addons.runbot.models.docker.Path.open', mock_open()) as cache_file_mock: + content = dockerfile._get_cached_content() + cache_file_mock.assert_not_called() + self.assertEqual(content, expected_content, 'Dockerfile with "#CACHE" should change the ADD directive to COPY') + self.patchers['docker_path_touch'].assert_not_called() + + # 4 - The cache file does not exists yet but the there is an error while downloading + self.patchers['docker_path_exists'].return_value = False + self.patchers['docker_requests_get'].side_effect = HTTPError + + dockerfile.layer_ids[0].content = raw_layer + with patch('odoo.addons.runbot.models.docker.Path.open', mock_open()) as cache_file_mock: + with self.assertRaises(HTTPError, msg='HTTPError Exception should be reraised during cache download'): + content = dockerfile._get_cached_content()