[IMP] runbot: allow to choose a docker image identifier

When one wants to update a Dockerfile with significant changes without
impacting regular builds, he needs to create a test Dockerfile
beforehand.

With this commit, when a Dockerfile is changed, the newly built image is
tagged with a `.future` suffix, that way, the old image is still used
until the image identifier is changed to the future one.

That way, the `.future` image can be tested in separately before being
used in production.

runbot/__manifest__.py

@@ -6,7 +6,7 @@
     'author': "Odoo SA",
     'website': "http://runbot.odoo.com",
     'category': 'Website',
-    'version': '5.9',
+    'version': '5.10',
     'application': True,
     'depends': ['base', 'base_automation', 'website'],
     'data': [

runbot/container.py

@@ -137,6 +137,24 @@ def _docker_build(build_dir, image_tag):
         return dm.result
 
 
+def docker_tag(identifier_or_tag, new_tag):
+    return _docker_tag(identifier_or_tag, new_tag)
+
+def _docker_tag(identifier_or_tag, new_tag):
+    if not identifier_or_tag:
+        return
+    _logger.info('Tagging image %s to "%s"', identifier_or_tag, new_tag)
+    docker_client = docker.from_env()
+    repo, tag = new_tag.split(':')  # runbot DockerFile tags contains the repo part
+    try:
+        image = docker_client.images.get(identifier_or_tag)
+        image.tag(repo, tag)
+    except docker.errors.ImageNotFound:
+        _logger.warning('failed to find docker image with identifier %s', identifier_or_tag)
+    except docker.errors.APIError:
+        _logger.warning('failed to retag docker image with identifier %s', identifier_or_tag)
+
+
 def docker_push(image_tag, push_url='127.0.0.1:5001'):
     return _docker_push(image_tag, push_url)
 

runbot/data/dockerfile_data.xml

@@ -199,4 +199,15 @@ ENV PIP_BREAK_SYSTEM_PACKAGES=1</field>
 RUN python3 -m pip install --no-cache-dir -r /tmp/requirements.txt</field>
     </record>
 
+    <record model="ir.actions.server" id="action_sync_docker_identifiers">
+        <field name="name">Sync Identifiers</field>
+        <field name="model_id" ref="runbot.model_runbot_dockerfile" />
+        <field name="binding_model_id" ref="runbot.model_runbot_dockerfile" />
+        <field name="type">ir.actions.server</field>
+        <field name="state">code</field>
+        <field name="code">
+            records.action_sync_identifiers()
+        </field>
+    </record>
+
 </odoo>

runbot/migrations/18.0.5.10/post-migration.py

@@ -0,0 +1,14 @@
+import logging
+
+_logger = logging.getLogger(__name__)
+
+def migrate(cr, version):
+    cr.execute("""
+        UPDATE runbot_dockerfile
+           SET image_identifier = subq.identifier, image_future_identifier = subq.identifier
+          FROM (SELECT DISTINCT ON (dockerfile_id) dockerfile_id, identifier
+                  FROM runbot_docker_build_result
+                 WHERE result='success' order by dockerfile_id, create_date desc)
+            AS subq
+         WHERE runbot_dockerfile.id = subq.dockerfile_id;
+        """)

runbot/models/build.py

@@ -853,6 +853,8 @@ class BuildResult(models.Model):
             ro_volumes[f'/data/build/{dest}'] = source
         if 'image_tag' not in kwargs:
             kwargs.update({'image_tag': self.params_id.dockerfile_id.image_tag})
+        if self.params_id.config_data.get('docker_use_future') and not kwargs['image_tag'].endswith('.future'):
+            kwargs['image_tag'] += '.future'
         self._log('Preparing', 'Using Dockerfile Tag [%s](/runbot/dockerfile/tag/%s)', kwargs['image_tag'], kwargs['image_tag'], log_type='markdown')
         docker_registry_url = self.host_id._get_docker_registry_url()
         if docker_registry_url and self.host_id.use_remote_docker_registry:

runbot/models/docker.py

@@ -124,7 +124,10 @@ class Dockerfile(models.Model):
 
     name = fields.Char('Dockerfile name', required=True, help="Name of Dockerfile")
     active = fields.Boolean('Active', default=True, tracking=True)
+    image_identifier = fields.Char('Identifier')
+    image_future_identifier = fields.Char('Future Identifier')
     image_tag = fields.Char(compute='_compute_image_tag', store=True)
+    image_future_tag = fields.Char(compute='_compute_image_future_tag')
     template_id = fields.Many2one('ir.ui.view', string='Docker Template', domain=[('type', '=', 'qweb')], context={'default_type': 'qweb', 'default_arch_base': '<t></t>'})
     arch_base = fields.Text(related='template_id.arch_base', readonly=False, related_sudo=True)
     dockerfile = fields.Text(compute='_compute_dockerfile', tracking=True)
@@ -198,12 +201,22 @@ class Dockerfile(models.Model):
             if rec.name:
                 rec.image_tag = 'odoo:%s' % re.sub(r'[ /:\(\)\[\]]', '', rec.name)
 
+    @api.depends('image_tag')
+    def _compute_image_future_tag(self):
+        for rec in self:
+            rec.image_future_tag = f'{rec.image_tag}.future'
+
     @api.depends('template_id')
     def _compute_view_ids(self):
         for rec in self:
             keys = re.findall(r'<t.+t-call="(.+)".+', rec.arch_base or '')
             rec.view_ids = self.env['ir.ui.view'].search([('type', '=', 'qweb'), ('key', 'in', keys)]).ids
 
+    def action_sync_identifiers(self):
+        for dockerfile in self:
+            if dockerfile.image_future_identifier and dockerfile.image_future_identifier != dockerfile.image_identifier:
+                dockerfile.image_identifier = dockerfile.image_future_identifier
+
     def _template_to_layers(self):
 
         ##
@@ -332,7 +345,7 @@ class Dockerfile(models.Model):
 
         with open(self.env['runbot.runbot']._path('docker', self.image_tag, 'Dockerfile'), 'w') as Dockerfile:
             Dockerfile.write(content)
-        result = docker_build(docker_build_path, self.image_tag)
+        result = docker_build(docker_build_path, self.image_future_tag)
         duration = result['duration']
         msg = result['msg']
         success = image_id = result.get('image_id')
@@ -372,6 +385,7 @@ class Dockerfile(models.Model):
                 message = f'Build failure, check results for more info ({result.summary})'
                 self.message_post(body=message)
                 _logger.error(message)
+        return image_id
 
 
 class DockerBuildOutput(models.Model):

runbot/models/host.py

@@ -6,7 +6,7 @@ from docker.errors import ImageNotFound
 from odoo import models, fields, api
 from odoo.tools import config, ormcache
 from ..common import fqdn, local_pgadmin_cursor, os, list_local_dbs, local_pg_cursor
-from ..container import docker_push, docker_pull, docker_prune, docker_images, docker_remove
+from ..container import docker_push, docker_pull, docker_prune, docker_images, docker_remove, docker_tag
 
 _logger = logging.getLogger(__name__)
 
@@ -150,7 +150,7 @@ class Host(models.Model):
         # pull all images from the runbot docker registry
         is_registry = docker_registry_host == self
         all_docker_files = self.env['runbot.dockerfile'].search([])
-        all_tags = set(all_docker_files.mapped('image_tag'))
+        all_tags = set(all_docker_files.mapped('image_tag')) | set(all_docker_files.mapped('image_future_tag'))
         if docker_registry_url and self.use_remote_docker_registry and not is_registry:
             _logger.info('Pulling docker images...')
             total_duration = 0
@@ -166,18 +166,22 @@ class Host(models.Model):
         else:
             _logger.info('Building docker images...')
             for dockerfile in self.env['runbot.dockerfile'].search([('to_build', '=', True)]):
-                dockerfile._build(self)
+                identifier = dockerfile._build(self)
+                dockerfile.image_future_identifier = identifier or dockerfile.image_future_identifier
+                docker_tag(dockerfile.image_identifier, dockerfile.image_tag)
+                docker_tag(dockerfile.image_future_identifier, dockerfile.image_future_tag)
                 if is_registry:
-                    try:
+                    for tag in [dockerfile.image_tag, dockerfile.image_future_tag]:
-                        docker_push(dockerfile.image_tag)  # for now, always push locally
+                        try:
-                        if self.docker_registry_url:
+                            docker_push(tag)  # for now, always push locally
-                            docker_registry_url = self.docker_registry_url
+                            if self.docker_registry_url:
-                        else:
+                                docker_registry_url = self.docker_registry_url
-                            docker_registry_url = icp.get_param('runbot.docker_registry_url', default='').strip('/')
+                            else:
-                        if docker_registry_url:
+                                docker_registry_url = icp.get_param('runbot.docker_registry_url', default='').strip('/')
-                            docker_push(dockerfile.image_tag, docker_registry_url)
+                            if docker_registry_url:
-                    except ImageNotFound:
+                                docker_push(tag, docker_registry_url)
-                        _logger.warning("Image tag `%s` not found. Skipping push", dockerfile.image_tag)
+                        except ImageNotFound:
+                            _logger.warning("Image tag `%s` not found. Skipping push", dockerfile.image_future_tag)
 
         _logger.info('Cleaning docker images...')
         for image in docker_images():

runbot/models/res_config_settings.py

@@ -51,7 +51,8 @@ class ResConfigSettings(models.TransientModel):
     runbot_pending_warning = fields.Integer('Pending warning limit', default=5, config_parameter='runbot.pending.warning')
     runbot_pending_critical = fields.Integer('Pending critical limit', default=5, config_parameter='runbot.pending.critical')
 
-    runbot_docker_registry_host_id = fields.Many2one('runbot.host', 'Docker registry', help='Runbot host which handles Docker registry.', config_parameter='runbot.docker_registry_host_id')
+    runbot_docker_registry_host_id = fields.Many2one('runbot.host', 'Docker builder', help='Runbot host which handles Docker builds.', config_parameter='runbot.docker_registry_host_id')
+    runbot_docker_registry_url = fields.Char('Docker Registry url', help='Remote Registry Url', config_parameter='runbot.docker_registry_url')
     # TODO other icp
     # runbot.runbot_maxlogs 100
     # migration db

runbot/tests/common.py

@@ -181,6 +181,11 @@ class RunbotCase(TransactionCase):
         self.start_patcher('docker_run', 'odoo.addons.runbot.container._docker_run')
         self.start_patcher('docker_build', 'odoo.addons.runbot.container._docker_build')
         self.start_patcher('docker_push', 'odoo.addons.runbot.container._docker_push')
+        self.start_patcher('docker_prune', 'odoo.addons.runbot.container._docker_prune')
+        self.start_patcher('docker_pull', 'odoo.addons.runbot.container._docker_pull')
+        self.start_patcher('docker_tag', 'odoo.addons.runbot.container._docker_tag')
+        self.start_patcher('docker_images', 'odoo.addons.runbot.container._docker_images')
+        self.start_patcher('docker_remove', 'odoo.addons.runbot.container._docker_remove')
         self.start_patcher('docker_ps', 'odoo.addons.runbot.container._docker_ps', [])
         self.start_patcher('docker_stop', 'odoo.addons.runbot.container._docker_stop')
         self.start_patcher('docker_get_gateway_ip', 'odoo.addons.runbot.models.build_config.docker_get_gateway_ip', None)
@@ -196,7 +201,6 @@ class RunbotCase(TransactionCase):
         self.start_patcher('getmtime', 'odoo.addons.runbot.common.os.path.getmtime', datetime.datetime.now().timestamp())
         self.start_patcher('file_exist', 'odoo.tools.misc.os.path.exists', True)
 
-
         self.start_patcher('_get_py_version', 'odoo.addons.runbot.models.build.BuildResult._get_py_version', 3)
 
         def no_commit(*_args, **_kwargs):

runbot/tests/test_host.py

@@ -1,5 +1,7 @@
 import logging
 
+from unittest.mock import call
+
 from .common import RunbotCase
 
 from datetime import datetime, timedelta
@@ -111,3 +113,51 @@ class TestHost(RunbotCase):
         self.patchers['fetch_local_logs'].return_value = logs
         self.test_host._process_logs()
         self.patchers['host_local_pg_cursor'].assert_called()
+
+    def test_docker_builder_existing_image(self):
+        self.start_patcher('build_patcher', 'odoo.addons.runbot.models.docker.Dockerfile._build')
+
+        # deactivate DockerDefault to avoid test pollution
+        self.env.ref('runbot.docker_default').active = False
+
+        icp = self.env['ir.config_parameter']
+        icp.set_param('runbot.docker_registry_host_id', self.test_host.id)
+        icp.set_param('runbot.docker_registry_url', 'registryhost_nowhere')
+        dockerfile = self.env['runbot.dockerfile'].create({
+            'name': 'Docker Test',
+            'to_build': True,
+            'image_identifier': 'current',
+            'image_future_identifier': 'current'
+        })
+
+        self.assertEqual(dockerfile.image_tag, 'odoo:DockerTest')
+        self.assertEqual(dockerfile.image_future_tag, 'odoo:DockerTest.future')
+
+        self.patchers['build_patcher'].side_effect = lambda x: False  # simulate a build failure
+        self.test_host._docker_update_images()
+
+        self.assertEqual(dockerfile.image_future_identifier, 'current')
+
+        self.patchers['build_patcher'].side_effect = lambda x: 'future'  # now simulate a success
+        self.test_host._docker_update_images()
+
+        self.assertEqual(dockerfile.image_future_identifier, 'future')
+
+        expected_docker_tag_calls = [
+            call('current', 'odoo:DockerTest'),
+            call('future', 'odoo:DockerTest.future')
+        ]
+
+        self.patchers['docker_tag'].assert_has_calls(expected_docker_tag_calls)
+
+        expected_push_calls = [
+            call('odoo:DockerTest', '127.0.0.1:5001'),
+            call('odoo:DockerTest', 'registryhost_nowhere'),
+            call('odoo:DockerTest.future', '127.0.0.1:5001'),
+            call('odoo:DockerTest.future', 'registryhost_nowhere')
+        ]
+
+        self.patchers['docker_push'].assert_has_calls(expected_push_calls)
+        self.patchers['docker_pull'].assert_not_called()
+
+

runbot/views/dockerfile_views.xml

@@ -7,10 +7,13 @@
           <form string="Dockerfile">
             <sheet>
               <widget name="web_ribbon" title="Empty" bg_color="bg-warning" invisible="dockerfile != ''"/>
+              <widget name="web_ribbon" title="Newer identifier" bg_color="bg-danger" invisible="image_identifier == image_future_identifier"/>
               <widget name="web_ribbon" title="Archived" bg_color="bg-danger" invisible="active"/>
               <group>
                 <field name="active" invisible="1"/>
                 <field name="name"/>
+                <field name="image_identifier"/>
+                <field name="image_future_identifier"/>
                 <field name="image_tag"/>
                 <field name="to_build"/>
                 <field name="always_pull"/>
@@ -97,7 +100,7 @@
         <field name="name">runbot.dockerfile.list</field>
         <field name="model">runbot.dockerfile</field>
         <field name="arch" type="xml">
-          <list string="Dockerfile" decoration-danger="dockerfile == ''" decoration-warning="to_build == False">
+          <list string="Dockerfile" decoration-danger="dockerfile == '' or image_identifier != image_future_identifier" decoration-warning="to_build == False">
             <field name="name"/>
             <field name="image_tag"/>
             <field name="to_build" groups="!runbot.group_runbot_admin"/>

runbot/views/res_config_settings_views.xml

@@ -97,6 +97,9 @@
                     <setting>
                       <field name="runbot_docker_registry_host_id"/>
                     </setting>
+                    <setting class="col-lg-12">
+                      <field name="runbot_docker_registry_url"/>
+                    </setting>
                   </block>
                 </app>
               </xpath>