[IMP] runbot: call docker_run outside steps

When calling a step from a python step, it is impossible to alter some parameter, the only solution is to copy all step code.
With this change, python step are now able to override docker_run parameter of another step by modifying the dict returned by run_* steps
and assigning the result to "docker_params".

This will mainly be used to set the correct docker_image for migration pre and post test. This can also be used to alter a command,
like removing the pip install or adding extra pre/post operation on the command.

runbot/container.py

@@ -121,7 +121,7 @@ def docker_run(*args, **kwargs):
     return _docker_run(*args, **kwargs)
 
 
-def _docker_run(run_cmd, log_path, build_dir, container_name, image_tag=False, exposed_ports=None, cpu_limit=None, preexec_fn=None, ro_volumes=None, env_variables=None):
+def _docker_run(cmd=False, log_path=False, build_dir=False, container_name=False, image_tag=False, exposed_ports=None, cpu_limit=None, preexec_fn=None, ro_volumes=None, env_variables=None):
     """Run tests in a docker container
     :param run_cmd: command string to run in container
     :param log_path: path to the logfile that will contain odoo stdout and stderr
@@ -133,6 +133,8 @@ def _docker_run(run_cmd, log_path, build_dir, container_name, image_tag=False, e
     :params ro_volumes: dict of dest:source volumes to mount readonly in builddir
     :params env_variables: list of environment variables
     """
+    assert cmd and log_path and build_dir and container_name
+    run_cmd = cmd
     image_tag = image_tag or 'odoo:DockerDefault'
     container_name = sanitize_container_name(container_name)
     if isinstance(run_cmd, Command):

runbot/models/build.py

@@ -720,13 +720,13 @@ class BuildResult(models.Model):
                     build._log("run", message, level='ERROR')
                     build._kill(result='ko')
 
-    def _docker_run(self, *args, **kwargs):
+    def _docker_run(self, **kwargs):
         self.ensure_one()
         if 'image_tag' not in kwargs:
             kwargs.update({'image_tag': self.params_id.dockerfile_id.image_tag})
         if kwargs['image_tag'] != 'odoo:DockerDefault':
             self._log('Preparing', 'Using Dockerfile Tag %s' % kwargs['image_tag'])
-        docker_run(*args, **kwargs)
+        docker_run(**kwargs)
 
     def _path(self, *l, **kw):
         """Return the repo build path"""

runbot/models/build_config.py

@@ -8,7 +8,7 @@ import shlex
 import time
 from unidiff import PatchSet
 from ..common import now, grep, time2str, rfind, s2human, os, RunbotException
-from ..container import docker_run, docker_get_gateway_ip, Command
+from ..container import docker_get_gateway_ip, Command
 from odoo import models, fields, api
 from odoo.exceptions import UserError, ValidationError
 from odoo.tools.safe_eval import safe_eval, test_python_expr
@@ -238,12 +238,14 @@ class ConfigStep(models.Model):
         log_path = build._path('logs', '%s.txt' % self.name)
         build.write({'job_start': now(), 'job_end': False})  # state, ...
         build._log('run', 'Starting step **%s** from config **%s**' % (self.name, build.params_id.config_id.name), log_type='markdown', level='SEPARATOR')
-        return self._run_step(build, log_path)
+        self._run_step(build, log_path)
 
     def _run_step(self, build, log_path):
         build.log_counter = self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_maxlogs', 100)
         run_method = getattr(self, '_run_%s' % self.job_type)
-        return run_method(build, log_path)
+        docker_params = run_method(build, log_path)
+        if docker_params:
+            build._docker_run(**docker_params)
 
     def _run_create_build(self, build, log_path):
         count = 0
@@ -262,7 +264,6 @@ class ConfigStep(models.Model):
             'fields': fields,
             'models': models,
             'build': build,
-            'docker_run': docker_run,
             '_logger': _logger,
             'log_path': build._path('logs', '%s.txt' % self.name),
             'glob': glob.glob,
@@ -279,6 +280,7 @@ class ConfigStep(models.Model):
         eval_ctx = self.make_python_ctx(build)
         try:
             safe_eval(self.python_code.strip(), eval_ctx, mode="exec", nocopy=True)
+            return eval_ctx.get('docker_params')
         except ValueError as e:
             save_eval_value_error_re = r'<class \'odoo.addons.runbot.models.repo.RunbotException\'>: "(.*)" while evaluating\n.*'
             message = e.args[0]
@@ -348,9 +350,9 @@ class ConfigStep(models.Model):
         build_port = build.port
         self.env.cr.commit()  # commit before docker run to be 100% sure that db state is consistent with dockers
         self.invalidate_cache()
-        res = build._docker_run(cmd, log_path, build_path, docker_name, exposed_ports=[build_port, build_port + 1], ro_volumes=exports, env_variables=env_variables)
         self.env['runbot.runbot']._reload_nginx()
-        return res
+        return dict(cmd=cmd, log_path=log_path, build_dir=build_path, container_name=docker_name, exposed_ports=[build_port, build_port + 1], ro_volumes=exports, env_variables=env_variables)
+
 
     def _run_install_odoo(self, build, log_path):
         exports = build._checkout()
@@ -434,7 +436,7 @@ class ConfigStep(models.Model):
         max_timeout = int(self.env['ir.config_parameter'].get_param('runbot.runbot_timeout', default=10000))
         timeout = min(self.cpu_limit, max_timeout)
         env_variables = self.additionnal_env.split(';') if self.additionnal_env else []
-        return build._docker_run(cmd, log_path, build._path(), build._get_docker_name(), cpu_limit=timeout, ro_volumes=exports, env_variables=env_variables)
+        return dict(cmd=cmd, log_path=log_path, build_dir=build._path(), container_name=build._get_docker_name(), cpu_limit=timeout, ro_volumes=exports, env_variables=env_variables)
 
     def _upgrade_create_childs(self):
         pass
@@ -667,7 +669,7 @@ class ConfigStep(models.Model):
         exception_env = self.env['runbot.upgrade.exception']._generate()
         if exception_env:
             env_variables.append(exception_env)
-        build._docker_run(migrate_cmd, log_path, build._path(), build._get_docker_name(), cpu_limit=timeout, ro_volumes=exports, env_variables=env_variables, image_tag=target.params_id.dockerfile_id.image_tag)
+        return dict(cmd=migrate_cmd, log_path=log_path, build_dir=build._path(), container_name=build._get_docker_name(), cpu_limit=timeout, ro_volumes=exports, env_variables=env_variables, image_tag=target.params_id.dockerfile_id.image_tag)
 
     def _run_restore(self, build, log_path):
         # exports = build._checkout()
@@ -708,7 +710,7 @@ class ConfigStep(models.Model):
 
             ])
 
-        build._docker_run(cmd, log_path, build._path(), build._get_docker_name(), cpu_limit=self.cpu_limit)
+        return dict(cmd=cmd, log_path=log_path, build_dir=build._path(), container_name=build._get_docker_name(), cpu_limit=self.cpu_limit)
 
     def _reference_builds(self, bundle, trigger):
         upgrade_dumps_trigger_id = trigger.upgrade_dumps_trigger_id

runbot/tests/test_build_config_step.py

@@ -220,14 +220,19 @@ class TestBuildConfigStep(RunbotCase):
 
         self.patchers['docker_run'].side_effect = docker_run
 
-        config_step._run_install_odoo(self.parent_build, 'dev/null/logpath')
+        config_step._run_step(self.parent_build, 'dev/null/logpath')
 
         assert_db_name = 'custom_build'
         parent_build_params = self.parent_build.params_id.copy({'config_data': {'db_name': 'custom_build'}})
         parent_build = self.parent_build.copy({'params_id': parent_build_params.id})
-        config_step._run_install_odoo(parent_build, 'dev/null/logpath')
+        config_step._run_step(parent_build, 'dev/null/logpath')
 
-        config_step._run_run_odoo(parent_build, 'dev/null/logpath')
+        config_step = self.ConfigStep.create({
+            'name': 'run_test',
+            'job_type': 'run_odoo',
+            'custom_db_name': 'custom',
+        })
+        config_step._run_step(parent_build, 'dev/null/logpath')
 
         self.assertEqual(call_count, 3)
 
@@ -236,7 +241,7 @@ class TestBuildConfigStep(RunbotCase):
         """minimal test for python steps. Also test that `-d` in cmd creates a database"""
         test_code = """cmd = build._cmd()
 cmd += ['-d', 'test_database']
-docker_run(cmd)
+docker_params = dict(cmd=cmd)
         """
         config_step = self.ConfigStep.create({
             'name': 'default',
@@ -249,7 +254,7 @@ docker_run(cmd)
             self.assertIn('-d test_database', run_cmd)
 
         self.patchers['docker_run'].side_effect = docker_run
-        config_step._run_python(self.parent_build, 'dev/null/logpath')
+        config_step._run_step(self.parent_build, 'dev/null/logpath')
         self.patchers['docker_run'].assert_called_once()
         db = self.env['runbot.database'].search([('name', '=', 'test_database')])
         self.assertEqual(db.build_id, self.parent_build)
@@ -270,7 +275,7 @@ docker_run(cmd)
             call_count += 1
 
         self.patchers['docker_run'].side_effect = docker_run
-        config_step._run_install_odoo(self.parent_build, 'dev/null/logpath')
+        config_step._run_step(self.parent_build, 'dev/null/logpath')
 
         self.assertEqual(call_count, 1)