[IMP] runbot: use a config file to simplify args

When starting an odoo instance with Docker, a very long command line is
computed and appears in the logs.

With this commit, an .odoorc configuration file is written ind the build
dir and mounted in the Docker container.

Previously, the runbot .odoorc/.openerprc file was mounted to share some
parameters. Now, if that file exsists, its content is merged with build
.odoorc.

runbot/container.py

@@ -9,7 +9,9 @@ When testing this file:
     The second parameter is the exposed port
 """
 import argparse
+import configparser
 import datetime
+import io
 import json
 import logging
 import os
@@ -32,11 +34,20 @@ ENV COVERAGE_FILE /data/build/.coverage
 
 
 class Command():
-    def __init__(self, pres, cmd, posts, finals=None):
+    def __init__(self, pres, cmd, posts, finals=None, config_tuples=None):
+        """ Command object that represent commands to run in Docker container
+        :param pres: list of pre-commands
+        :param cmd: list of main command only run if the pres commands succeed (&&)
+        :param posts: list of post commands posts only run if the cmd command succedd (&&)
+        :param finals: list of finals commands always executed
+        :param config_tuples: list of key,value tuples to write in config file
+        returns a string of the full command line to run
+        """
         self.pres = pres or []
         self.cmd = cmd
         self.posts = posts or []
         self.finals = finals or []
+        self.config_tuples = config_tuples or []
 
     def __getattr__(self, name):
         return getattr(self.cmd, name)
@@ -47,6 +58,12 @@ class Command():
     def __add__(self, l):
         return Command(self.pres, self.cmd + l, self.posts, self.finals)
 
+    def __str__(self):
+        return ' '.join(self)
+
+    def __repr__(self):
+        return self.build().replace('&& ', '&&\n').replace('|| ', '||\n\t').replace(';', ';\n')
+
     def build(self):
         cmd_chain = []
         cmd_chain += [' '.join(pre) for pre in self.pres if pre]
@@ -56,6 +73,24 @@ class Command():
         cmd_chain += [' '.join(final) for final in self.finals if final]
         return ' ; '.join(cmd_chain)
 
+    def add_config_tuple(self, option, value):
+        self.config_tuples.append((option, value))
+
+    def get_config(self, starting_config=''):
+        """ returns a config file content based on config tuples and
+            and eventually update the starting config
+        """
+        config = configparser.ConfigParser()
+        config.read_string(starting_config)
+        if self.config_tuples and not config.has_section('options'):
+            config.add_section('options')
+        for option, value in self.config_tuples:
+            config.set('options', option, value)
+        res = io.StringIO()
+        config.write(res)
+        res.seek(0)
+        return res.read()
+
 
 def docker_build(log_path, build_dir):
     """Build the docker image
@@ -85,10 +120,15 @@ def docker_run(run_cmd, log_path, build_dir, container_name, exposed_ports=None,
     :params ro_volumes: dict of dest:source volumes to mount readonly in builddir
     :params env_variables: list of environment variables
     """
+    if isinstance(run_cmd, Command):
+        cmd_object = run_cmd
+        run_cmd = cmd_object.build()
+    else:
+        cmd_object = Command([], run_cmd.split(' '), [])
     _logger.debug('Docker run command: %s', run_cmd)
     logs = open(log_path, 'w')
     run_cmd = 'cd /data/build && %s' % run_cmd
-    logs.write("Docker command:\n%s\n=================================================\n" % run_cmd.replace('&& ', '&&\n').replace('|| ', '||\n\t'))
+    logs.write("Docker command:\n%s\n=================================================\n" % cmd_object)
     # create start script
     docker_command = [
         'docker', 'run', '--rm',
@@ -110,8 +150,12 @@ def docker_run(run_cmd, log_path, build_dir, container_name, exposed_ports=None,
     serverrc_path = os.path.expanduser('~/.openerp_serverrc')
     odoorc_path = os.path.expanduser('~/.odoorc')
     final_rc = odoorc_path if os.path.exists(odoorc_path) else serverrc_path if os.path.exists(serverrc_path) else None
-    if final_rc:
+    rc_content = cmd_object.get_config(starting_config=open(final_rc, 'r').read() if final_rc else '')
-        docker_command.extend(['--volume=%s:/home/odoo/.odoorc:ro' % final_rc])
+    rc_path = os.path.join(build_dir, '.odoorc')
+    with open(rc_path, 'w') as rc_file:
+        rc_file.write(rc_content)
+    docker_command.extend(['--volume=%s:/home/odoo/.odoorc:ro' % rc_path])
+
     if exposed_ports:
         for dp, hp in enumerate(exposed_ports, start=8069):
             docker_command.extend(['-p', '127.0.0.1:%s:%s' % (hp, dp)])
@@ -204,8 +248,10 @@ def tests(args):
     elif args.flamegraph:
         flame_log = '/data/build/logs/flame.log'
         python_params = ['-m', 'flamegraph', '-o', flame_log]
-    odoo_cmd = ['python%s' % py_version ] + python_params + ['/data/build/odoo-bin', '-d %s' % args.db_name, '--addons-path=/data/build/addons', '--data-dir', '/data/build/datadir', '-r %s' % os.getlogin(), '-i', args.odoo_modules,  '--test-enable', '--stop-after-init', '--max-cron-threads=0']
+    odoo_cmd = ['python%s' % py_version ] + python_params + ['/data/build/odoo-bin', '-d %s' % args.db_name, '--addons-path=/data/build/addons', '-i', args.odoo_modules,  '--test-enable', '--stop-after-init', '--max-cron-threads=0']
     cmd = Command(pres, odoo_cmd, posts)
+    cmd.add_config_tuple('data-dir', '/data/build/datadir')
+    cmd.add_config_tuple('db_user', '%s' % os.getlogin())
 
     if args.dump:
         os.makedirs(os.path.join(args.build_dir, 'logs', args.db_name), exist_ok=True)
@@ -235,7 +281,7 @@ def tests(args):
     # Test full testing
     logfile = os.path.join(args.build_dir, 'logs', 'logs-full-test.txt')
     container_name = 'odoo-container-test-%s' % datetime.datetime.now().microsecond
-    docker_run(cmd.build(), logfile, args.build_dir, container_name)
+    docker_run(cmd, logfile, args.build_dir, container_name)
     time.sleep(1)  # give time for the container to start
 
     while docker_is_running(container_name):

runbot/models/build.py

@@ -903,33 +903,38 @@ class runbot_build(models.Model):
         cmd = ['python%s' % py_version] + python_params + [os.path.join(server_dir, server_file), '--addons-path', ",".join(addons_paths)]
         # options
         config_path = build._server("tools/config.py")
-        if local_only:
-            if grep(config_path, "--http-interface"):
-                cmd.append("--http-interface=127.0.0.1")
-            elif grep(config_path, "--xmlrpc-interface"):
-                cmd.append("--xmlrpc-interface=127.0.0.1")
         if grep(config_path, "no-xmlrpcs"):  # move that to configs ?
             cmd.append("--no-xmlrpcs")
         if grep(config_path, "no-netrpc"):
             cmd.append("--no-netrpc")
+
+        command = Command(pres, cmd, [])
+
+        # use the username of the runbot host to connect to the databases
+        command.add_config_tuple('db_user', '%s' % pwd.getpwuid(os.getuid()).pw_name)
+
+        if local_only:
+            if grep(config_path, "--http-interface"):
+                command.add_config_tuple("http-interface", "127.0.0.1")
+            elif grep(config_path, "--xmlrpc-interface"):
+                command.add_config_tuple("xmlrpc-interface", "127.0.0.1")
+
         if grep(config_path, "log-db"):
             logdb_uri = self.env['ir.config_parameter'].get_param('runbot.runbot_logdb_uri')
             logdb = self.env.cr.dbname
             if logdb_uri and grep(build._server('sql_db.py'), 'allow_uri'):
                 logdb = '%s' % logdb_uri
-            cmd += ["--log-db=%s" % logdb]
+            command.add_config_tuple("log-db", "%s" % logdb)
             if grep(build._server('tools/config.py'), 'log-db-level'):
-                cmd += ["--log-db-level", '25']
+                command.add_config_tuple("log-db-level", '25')
 
         if grep(config_path, "data-dir"):
             datadir = build._path('datadir')
             if not os.path.exists(datadir):
                 os.mkdir(datadir)
-            cmd += ["--data-dir", '/data/build/datadir']
+            command.add_config_tuple("data-dir", '/data/build/datadir')
 
-        # use the username of the runbot host to connect to the databases
+        return command
-        cmd += ['-r %s' % pwd.getpwuid(os.getuid()).pw_name]
-        return Command(pres, cmd, [])
 
     def _github_status_notify_all(self, status):
         """Notify each repo with a status"""

runbot/models/build_config.py

@@ -287,7 +287,7 @@ 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 = docker_run(cmd.build(), log_path, build_path, docker_name, exposed_ports=[build_port, build_port + 1], ro_volumes=exports)
+        res = docker_run(cmd, log_path, build_path, docker_name, exposed_ports=[build_port, build_port + 1], ro_volumes=exports)
         build.repo_id._reload_nginx()
         return res
 
@@ -341,7 +341,7 @@ class ConfigStep(models.Model):
                     cmd.extend(['--test-tags', test_tags])
 
         if grep(config_path, "--screenshots"):
-            cmd += ['--screenshots', '/data/build/tests']
+            cmd.add_config_tuple('screenshots', '/data/build/tests')
 
         cmd.append('--stop-after-init')  # install job should always finish
         if '--log-level' not in extra_params:
@@ -369,7 +369,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 docker_run(cmd.build(), log_path, build._path(), build._get_docker_name(), cpu_limit=timeout, ro_volumes=exports, env_variables=env_variables)
+        return docker_run(cmd, log_path, build._path(), build._get_docker_name(), cpu_limit=timeout, ro_volumes=exports, env_variables=env_variables)
 
     def log_end(self, build):
         if self.job_type == 'create_build':

runbot/tests/__init__.py

@@ -7,4 +7,4 @@ from . import test_schedule
 from . import test_cron
 from . import test_build_config_step
 from . import test_event
-
+from . import test_command

runbot/tests/test_build.py

@@ -113,7 +113,7 @@ class Test_Build(common.TransactionCase):
             'port': '1234',
         })
         cmd = build._cmd(py_version=3)
-        self.assertIn('--log-db=%s' % uri, cmd)
+        self.assertIn('log-db = %s' % uri, cmd.get_config())
 
     @patch('odoo.addons.runbot.models.build.os.path.isdir')
     @patch('odoo.addons.runbot.models.build.os.path.isfile')

runbot/tests/test_command.py

@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+from odoo.tests import common
+from ..container import Command
+
+
+CONFIG = """[options]
+foo = bar
+"""
+
+
+class Test_Command(common.TransactionCase):
+
+    def test_command(self):
+        pres = ['pip3', 'install', 'foo']
+        posts = ['python3', '-m', 'coverage', 'html']
+        finals = ['pgdump bar']
+        cmd = Command([pres], ['python3', 'odoo-bin'], [posts], finals=[finals])
+        self.assertEqual(str(cmd), 'python3 odoo-bin')
+
+        expected = 'pip3 install foo && python3 odoo-bin && python3 -m coverage html ; pgdump bar'
+        self.assertEqual(cmd.build(), expected)
+
+        cmd = Command([pres], ['python3', 'odoo-bin'], [posts])
+        cmd.add_config_tuple('a', 'b')
+        cmd.add_config_tuple('x', 'y')
+
+        content = cmd.get_config(starting_config=CONFIG)
+
+        self.assertIn('[options]', content)
+        self.assertIn('foo = bar', content)
+        self.assertIn('a = b', content)
+        self.assertIn('x = y', content)