From da5eeb8a669b6068d35686f8bc13c51192244a1b Mon Sep 17 00:00:00 2001
From: Xavier-Do <xdo@odoo.com>
Date: Mon, 24 Mar 2025 09:33:01 +0100
Subject: [PATCH] [IMP] runbot: add in_error on dockerfile

---
 runbot/container.py               |  5 +++--
 runbot/models/docker.py           | 14 +++++++++++++-
 runbot/models/host.py             | 24 ++++++++++++++++--------
 runbot/views/dockerfile_views.xml |  5 ++++-
 4 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/runbot/container.py b/runbot/container.py
index c95729b1..5e43c5f8 100644
--- a/runbot/container.py
+++ b/runbot/container.py
@@ -151,12 +151,13 @@ def 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)
+        if new_tag not in image.tags:
+            _logger.info('Tagging image %s to "%s"', identifier_or_tag, new_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:
diff --git a/runbot/models/docker.py b/runbot/models/docker.py
index d2c2a1fd..0e17ca08 100644
--- a/runbot/models/docker.py
+++ b/runbot/models/docker.py
@@ -126,12 +126,14 @@ class Dockerfile(models.Model):
     active = fields.Boolean('Active', default=True, tracking=True)
     image_identifier = fields.Char('Identifier', tracking=True)
     image_future_identifier = fields.Char('Future Identifier', tracking=True)
+    image_previous_identifier = fields.Char('Previous Identifier', tracking=True)
     image_tag = fields.Char(compute='_compute_image_tag', store=True)
     image_future_tag = fields.Char(compute='_compute_image_helper_tags')
     image_previous_tag = fields.Char(compute='_compute_image_helper_tags')
     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)
+    in_error = fields.Boolean('In error', help='The last build failed.', default=False)
     to_build = fields.Boolean('To Build', help='Build Dockerfile. Check this when the Dockerfile is ready.', default=False)
     always_pull = fields.Boolean('Always pull', help='Always Pull on the hosts, not only at the use time', default=False, tracking=True, copy=False)
     version_ids = fields.One2many('runbot.version', 'dockerfile_id', string='Versions')
@@ -196,6 +198,10 @@ class Dockerfile(models.Model):
 
             rec.dockerfile = content
 
+    @api.onchange('dockerfile')
+    def onchange_dockerfile(self):
+        self.in_error = False
+
     @api.depends('name')
     def _compute_image_tag(self):
         for rec in self:
@@ -214,6 +220,12 @@ class Dockerfile(models.Model):
             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 write(self, values):
+        if 'image_identifier' in values and not 'image_previous_identifier' in values and self.image_identifier != values['image_identifier']:
+            self.ensure_one()
+            values['image_previous_identifier'] = self.image_identifier
+        return super().write(values)
+
     def action_sync_identifiers(self):
         for dockerfile in self:
             if dockerfile.image_future_identifier and dockerfile.image_future_identifier != dockerfile.image_identifier:
@@ -357,7 +369,7 @@ class Dockerfile(models.Model):
             docker_build_result_values['identifier'] = image_id
         else:
             docker_build_result_values['result'] = 'error'
-            self.to_build = False
+            self.in_error = True
 
         should_save_result = not success  # always save in case of failure
         if not should_save_result:
diff --git a/runbot/models/host.py b/runbot/models/host.py
index a187bcbc..568f443b 100644
--- a/runbot/models/host.py
+++ b/runbot/models/host.py
@@ -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')) | set(all_docker_files.mapped('image_future_tag'))
+        all_tags = set(all_docker_files.mapped('image_tag'))
         if docker_registry_url and self.use_remote_docker_registry and not is_registry:
             _logger.info('Pulling docker images...')
             total_duration = 0
@@ -166,13 +166,17 @@ class Host(models.Model):
         else:
             _logger.info('Building docker images...')
             for dockerfile in self.env['runbot.dockerfile'].search([('to_build', '=', True)]):
-                docker_tag(dockerfile.image_identifier, dockerfile.image_previous_tag)
-                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)
+                future_identifier = None
+                if not dockerfile.in_error:
+                    future_identifier = dockerfile._build(self)
+                    if future_identifier:
+                        docker_tag(future_identifier, dockerfile.image_future_tag)
                 if is_registry:
-                    for tag in [dockerfile.image_previous_tag, dockerfile.image_tag, dockerfile.image_future_tag]:
+                    if future_identifier:
+                        dockerfile.image_future_identifier = future_identifier
+                    docker_tag(dockerfile.image_previous_identifier, dockerfile.image_previous_tag)
+                    docker_tag(dockerfile.image_identifier, dockerfile.image_tag)
+                    for tag in [dockerfile.image_tag, dockerfile.image_future_tag]:
                         try:
                             docker_push(tag)  # for now, always push locally
                             if self.docker_registry_url:
@@ -182,11 +186,15 @@ class Host(models.Model):
                             if docker_registry_url:
                                 docker_push(tag, docker_registry_url)
                         except ImageNotFound:
-                            _logger.warning("Image tag `%s` not found. Skipping push", dockerfile.image_future_tag)
+                            _logger.warning("Image tag `%s` not found. Skipping push", tag)
+                else:
+                    if future_identifier:
+                        docker_tag(future_identifier, dockerfile.image_tag) # for a setup without registry
 
         _logger.info('Cleaning docker images...')
         for image in docker_images():
             for tag in image.tags:
+                tag = tag.removesuffix('.future').removesuffix('.previous')
                 if tag.startswith('odoo:') and tag not in all_tags:  # what about odoo:latest
                     _logger.info(f"Removing tag '{tag}' since it doesn't exist anymore")
                     docker_remove(tag)
diff --git a/runbot/views/dockerfile_views.xml b/runbot/views/dockerfile_views.xml
index 26ae45fa..0ece8ae8 100644
--- a/runbot/views/dockerfile_views.xml
+++ b/runbot/views/dockerfile_views.xml
@@ -16,6 +16,7 @@
                 <field name="image_future_identifier"/>
                 <field name="image_tag"/>
                 <field name="to_build"/>
+                <field name="in_error"/>
                 <field name="always_pull"/>
                 <field name="version_ids" widget="many2many_tags"/>
                 <field name="project_ids" widget="many2many_tags"/>
@@ -100,11 +101,13 @@
         <field name="name">runbot.dockerfile.list</field>
         <field name="model">runbot.dockerfile</field>
         <field name="arch" type="xml">
-          <list string="Dockerfile" decoration-danger="dockerfile == '' or image_identifier != image_future_identifier" decoration-warning="to_build == False">
+          <list string="Dockerfile" decoration-danger="dockerfile == '' or image_identifier != image_future_identifier" decoration-warning="in_error == True">
             <field name="name"/>
             <field name="image_tag"/>
             <field name="to_build" groups="!runbot.group_runbot_admin"/>
             <field name="to_build" widget="boolean_toggle" groups="runbot.group_runbot_admin"/>
+            <field name="in_error" groups="!runbot.group_runbot_admin"/>
+            <field name="in_error" widget="boolean_toggle" groups="runbot.group_runbot_admin"/>
             <field name="always_pull" groups="!runbot.group_runbot_admin"/>
             <field name="always_pull" widget="boolean_toggle" groups="runbot.group_runbot_admin"/>
             <field name="version_ids" widget="many2many_tags"/>