# HG changeset patch # User Vincent Hatakeyama <vincent.hatakeyama@xcg-consulting.fr> # Date 1599040043 -7200 # Wed Sep 02 11:47:23 2020 +0200 # Node ID 9889f48ee32d547a4c1ff0be78fa9553c43fbf2e # Parent 895bef67240dc059c97518205ad927596671c8cf ✨ use socket when starting PG diff --git a/NEWS.rst b/NEWS.rst --- a/NEWS.rst +++ b/NEWS.rst @@ -2,6 +2,13 @@ History ======= +5.0.0 +----- + +Use semver. + +docker_dev_start and do_tests: use socket when starting postgresql rather than a port that might be used on the host + 4.2.1 ----- diff --git a/odoo_scripts/do_tests.py b/odoo_scripts/do_tests.py --- a/odoo_scripts/do_tests.py +++ b/odoo_scripts/do_tests.py @@ -15,7 +15,11 @@ import docker # apt python3-docker (1.9) or pip3 install docker from psycopg2 import connect, OperationalError # apt python3-psycopg2 -from . import docker_dev_start +from .docker_dev_start import ( + docker_run_postgresql, + flake8, + main as docker_dev_start_main, +) from .config import Config if docker.__version__ > "2.5.0": @@ -25,9 +29,9 @@ _logger = logging.getLogger(__name__) -__version__ = "1.0.1" +__version__ = "1.0.2" __date__ = "2018-04-13" -__updated__ = "2020-06-30" +__updated__ = "2020-09-02" def main(argv=None): # IGNORE:C0111 @@ -265,7 +269,7 @@ args.append("--dbport") args.append(dbport) - docker_dev_start.flake8(odoo_type) + flake8(odoo_type) if nmspc.log_handler: for lh in nmspc.log_handler: args.append("--log-handler") @@ -278,20 +282,27 @@ if recreate_db: if start_postgresql: docker_client = docker_api(base_url="unix://var/run/docker.sock") - name, stop_method = docker_dev_start.docker_run_postgresql( + name, stop_method, socket_path = docker_run_postgresql( docker_client, project_name, postgresql_version, None ) + # system user used in the pg docker image + pg_user = "postgres" try: if start_postgresql: connect( user=odoo_db_user, password=odoo_db_password, database="postgres", - host="/tmp", + host=socket_path, + # need to be set to avoid using value from an ENV VAR + port=5432, ) except OperationalError: connection = connect( - user="pg", database="postgres", host="/tmp" + user=pg_user, + database="postgres", + host=socket_path, + post=5432, ) with connection.cursor() as cursor: # not injection safe but you are on your own machine @@ -304,7 +315,7 @@ connection.commit() connection.close() odoo_connection = connect( - user="pg", database="postgres", host="/tmp" + user=pg_user, database="postgres", host=socket_path, port=5432 ) odoo_connection.autocommit = True with odoo_connection.cursor() as cursor: @@ -314,7 +325,9 @@ cursor.execute( "CREATE DATABASE %s OWNER %s" % (dbname, odoo_db_user) ) - odoo_connection = connect(user="pg", database=dbname, host="/tmp") + odoo_connection = connect( + user=pg_user, database=dbname, host=socket_path, port=5432 + ) odoo_connection.autocommit = True with odoo_connection.cursor() as cursor: for extension in extensions: @@ -378,7 +391,7 @@ if install_log_level: install_args.append("--log-level") install_args.append(install_log_level) - result = docker_dev_start.main(install_args) + result = docker_dev_start_main(install_args) if result: return result @@ -395,7 +408,7 @@ if test_log_level: test_args.append("--log-level") test_args.append(test_log_level) - return docker_dev_start.main(test_args) + return docker_dev_start_main(test_args) else: raise NotImplementedError return 255 diff --git a/odoo_scripts/docker_dev_start.py b/odoo_scripts/docker_dev_start.py --- a/odoo_scripts/docker_dev_start.py +++ b/odoo_scripts/docker_dev_start.py @@ -14,7 +14,9 @@ import pwd from subprocess import call import sys -from typing import Dict +import tempfile +import time +from typing import Callable, Dict, Optional, Tuple import docker # apt python3-docker (1.9) or pip install docker from psycopg2 import connect, OperationalError # apt python3-psycopg2 @@ -31,9 +33,9 @@ _logger = logging.getLogger(__name__) -__version__ = "2.0.3" +__version__ = "2.0.4" __date__ = "2017-08-11" -__updated__ = "2020-06-26" +__updated__ = "2020-09-02" def which(program): @@ -543,11 +545,16 @@ _logger.debug("IP found %s", local_ip) arg.append("--db_host") - arg.append(local_ip) + if start_postgresql: + arg.append("/var/run/postgresql") + else: + arg.append(local_ip) + dbport = None if nmspc.dbport: + dbport = str(nmspc.dbport) arg.append("--db_port") - arg.append("%s" % nmspc.dbport) + arg.append(dbport) # auto detect local conf local_conf_path = nmspc.config @@ -585,10 +592,10 @@ _logger.info("No configuration file at: %s", local_conf_path) # default values if nothing else if not user: + # Odoo uses default if nothing is set (including in PGUSER env var) + _logger.info("No user defined, using odoo") user = "odoo" arg.append("--db_user %s" % user) - if not password: - password = "odoo" # data volume handling base_data_volume_name = "{}_data".format(project_name) @@ -675,94 +682,106 @@ options.append("py3o-fusion-server:{}".format(local_ip)) if start_postgresql and not odoo_help: - host_pg_port = nmspc.dbport if nmspc.dbport else 5433 - name, stop_postgresql = docker_run_postgresql( - docker_client, project_name, postgresql_version, host_pg_port + # Use socket rather than export port (avoid having to find a free port + # number) + name, stop_postgresql, socket_path = docker_run_postgresql( + docker_client, project_name, postgresql_version, host_pg_port=None ) - - arg.append("--db_port") - arg.append(str(host_pg_port)) + binds.append("{}:/var/run/postgresql".format(socket_path)) - # Check that connection can be done, try to create user if asked to - # TODO: handle the case where the database is still starting up - # TODO: (and remove the sleep) - try: - if local_ip or start_postgresql: - port = host_pg_port if start_postgresql else None - connect( - user=user, - password=password, + # Check that connection can be done, try to create user if asked to + # TODO: handle the case where the database is still starting up + # TODO: (and remove the sleep) + _logger.info("Testing postgres connection") + try: + if start_postgresql: + connect( + user=user, + password=password, + database="postgres", + host=socket_path, + port=5432, + ) + elif local_ip: + # TODO test this too + connect( + user=user, + password=password, + database="postgres", + host=local_ip, + port=dbport, + ) + else: + connect(user=user, password=password, database="postgres") + + except OperationalError as exception: + if nmspc.create_user: + _logger.debug("Cannot connect to database with user %s", user) + _logger.debug(exception) + _logger.info("Creating user %s", user) + if start_postgresql: + connection = connect( + user="postgres", database="postgres", - host=local_ip, - port=port, + host=socket_path, + port=5432, ) else: - connect(user=user, password=password, database="postgres") - - except OperationalError as exception: - if nmspc.create_user: - _logger.debug("Cannot connect to database with user %s", user) - _logger.debug(exception) - _logger.info("Creating user %s", user) - if start_postgresql: - connection = connect( - user="postgres", - database="postgres", - host=local_ip, - port=host_pg_port, - ) - else: - loginname = pwd.getpwuid(os.getuid())[0] - connection = connect(user=loginname, database="postgres") - with connection.cursor() as cursor: - # not injection safe but you are on your own machine - # with already full access to db - cursor.execute( - "CREATE ROLE %s LOGIN CREATEDB PASSWORD %%s" % user, - (password,), - ) - connection.commit() - connection.close() - else: - _logger.fatal("Cannot connect to database with user %s", user) - _logger.fatal(exception) - _logger.info( - "You can add the --create-user argument to create it" + loginname = pwd.getpwuid(os.getuid())[0] + # use socket to create user + # TODO test if removing more arguments works + connection = connect(user=loginname, database="postgres") + with connection.cursor() as cursor: + # not injection safe but you are on your own machine + # with already full access to db + cursor.execute( + "CREATE ROLE %s LOGIN CREATEDB PASSWORD %%s" % user, + (password,), ) - return 16 - # restore - if restore_filename: - restore_basename = os.path.basename(restore_filename) - _logger.info("Copying dump file in docker") - call( - [ - "docker", - "cp", - restore_filename, - "{}:/tmp/{}".format(name, restore_basename), - ] - ) - _logger.info("Creating database") - call(["docker", "exec", name, "createdb", "-U", user, database]) - _logger.info("Restoring database") - restore = call( - [ - "docker", - "exec", - name, - "pg_restore", - "-U", - user, - "-O", - "-d", - database, - "/tmp/{}".format(restore_basename), - ] - ) - if not restore: - return 15 - _logger.info("Removing dump file in docker") - call(["docker", "exec", name, "/tmp/{}".format(restore_basename)]) + connection.commit() + connection.close() + else: + _logger.fatal("Cannot connect to database with user %s", user) + _logger.fatal(exception) + _logger.info("You can add the --create-user argument to create it") + return 16 + + # --- restore --- + # TODO find out why odoo_help would stop us from restoring + if start_postgresql and not odoo_help and restore_filename: + restore_basename = os.path.basename(restore_filename) + _logger.info("Copying dump file in docker") + call( + [ + "docker", + "cp", + restore_filename, + "{}:/tmp/{}".format(name, restore_basename), + ] + ) + _logger.info("Creating database") + call(["docker", "exec", name, "createdb", "-U", user, database]) + _logger.info("Restoring database %s", database) + restore = call( + [ + "docker", + "exec", + name, + "pg_restore", + "-U", + user, + "-O", + "-d", + database, + "/tmp/{}".format(restore_basename), + ] + ) + if not restore: + return 15 + _logger.info("Removing dump file in docker") + call( + ["docker", "exec", name, "rm", "/tmp/{}".format(restore_basename)] + ) if not start_postgresql and not odoo_help and restore_filename: _logger.info("Creating database %s", database) @@ -961,28 +980,31 @@ def docker_run_postgresql( docker_client, - project_name, - postgresql_version, + project_name: str, + postgresql_version: str, host_pg_port=None, - stop_at_exit=True, -): + stop_at_exit: bool = True, +) -> Tuple[Optional[str], Callable, Optional[str]]: """ :param docker_client: :param project_name: :param postgresql_version: - :param host_pg_port: if None, put the socket in /tmp, else publish the port - :param stop_at_exit: - :return: + :param host_pg_port: if None, put the socket in a temp directory, + otherwise publish the port + :param stop_at_exit: True to stop the container at exit + :return: name of the container, method to call to stop it, socket directory + if any """ pg_repository = "postgres" version = "{}-alpine".format(postgresql_version) pg_image = "{}:{}".format(pg_repository, version) name = "postgresql-{}-{}".format(postgresql_version, project_name) - docker_client.pull(repository=pg_repository, tag=version) + # docker_client.pull(repository=pg_repository, tag=version) pg_data_volume_name = "postgresql_{}-{}".format( postgresql_version, project_name ) + path: str = None # volume = createVolume(docker_client, pg_data_volume_name) binds = ["{}:/var/lib/postgresql/data".format(pg_data_volume_name)] @@ -990,7 +1012,10 @@ if host_pg_port: port_bindings[5432] = host_pg_port else: - binds.append("/tmp:/var/run/postgresql") + path = tempfile.TemporaryDirectory( + prefix="odoo_scripts_postgres_socket-" + ).name + binds.append("{}:/var/run/postgresql".format(path)) host_config = docker_client.create_host_config( binds=binds, port_bindings=port_bindings ) @@ -999,7 +1024,7 @@ for container in docker_client.containers() ): _logger.debug("Postgresql Container already running") - return name, False + return name, False, None _logger.debug("Creating postgresql container") pg = docker_client.create_container( image=pg_image, host_config=host_config, name=name @@ -1017,15 +1042,15 @@ docker_client.remove_container(pg.get("Id")) except docker.errors.NotFound: _logger.info("Postgresql already stopped") + # crash because docker or pg changes the owner of the directory + # temp_directory.cleanup() if stop_at_exit: atexit.register(stop_postgresql) _logger.debug("Waiting for postgres to start") # give pg the time to start up - import time - time.sleep(5) - return name, stop_postgresql + return name, stop_postgresql, path def find_container(docker_client, name):