From 064546441ff9d14b7ebd69538f16b76e6b25ef58 Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Fri, 8 Nov 2019 12:16:57 +0100 Subject: [PATCH] [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 | 58 +++++++++++++++++++++++++++++++---- runbot/models/build.py | 27 +++++++++------- runbot/models/build_config.py | 6 ++-- runbot/tests/__init__.py | 2 +- runbot/tests/test_build.py | 2 +- runbot/tests/test_command.py | 32 +++++++++++++++++++ 6 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 runbot/tests/test_command.py diff --git a/runbot/container.py b/runbot/container.py index bc27bfea..b3ab86a4 100644 --- a/runbot/container.py +++ b/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: - docker_command.extend(['--volume=%s:/home/odoo/.odoorc:ro' % final_rc]) + rc_content = cmd_object.get_config(starting_config=open(final_rc, 'r').read() if final_rc else '') + 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): diff --git a/runbot/models/build.py b/runbot/models/build.py index 09b54bbb..dc8c8d58 100644 --- a/runbot/models/build.py +++ b/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 - cmd += ['-r %s' % pwd.getpwuid(os.getuid()).pw_name] - return Command(pres, cmd, []) + return command def _github_status_notify_all(self, status): """Notify each repo with a status""" diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 397e218c..4aaf6dc3 100644 --- a/runbot/models/build_config.py +++ b/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': diff --git a/runbot/tests/__init__.py b/runbot/tests/__init__.py index bbbba335..90852177 100644 --- a/runbot/tests/__init__.py +++ b/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 diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index c701ebdb..cf934aeb 100644 --- a/runbot/tests/test_build.py +++ b/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') diff --git a/runbot/tests/test_command.py b/runbot/tests/test_command.py new file mode 100644 index 00000000..4d34272b --- /dev/null +++ b/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)