From d784b77cb4e0b06c73ec980765df0d9bc1f101c4 Mon Sep 17 00:00:00 2001 From: Glandos Date: Mon, 11 Mar 2019 15:38:14 +0100 Subject: [PATCH] Remove dependency to psycopg2 with dump/restore (#53323) * Remove dependency to psycopg2 with dump/restore 'dump' and 'restore' state only need pg_dump and pg_restore. These tools don't use psycopg2 so this change tries to avoid the use of it in these cases. The db_exists test was replaced with an error detection when piping to compression program, using a FIFO file. This effectively reverts #39483, that was a fix for #39412. * Fix typo * Add changelog fragment * Add note for dump and restore not requiring psycopg2 * Fix YAML syntax * Update lib/ansible/modules/database/postgresql/postgresql_db.py Co-Authored-By: Glandos --- ...3323-no-psycopg2-for-dump-and-restore.yaml | 8 ++ .../database/postgresql/postgresql_db.py | 73 ++++++++++--------- 2 files changed, 48 insertions(+), 33 deletions(-) create mode 100644 changelogs/fragments/53323-no-psycopg2-for-dump-and-restore.yaml diff --git a/changelogs/fragments/53323-no-psycopg2-for-dump-and-restore.yaml b/changelogs/fragments/53323-no-psycopg2-for-dump-and-restore.yaml new file mode 100644 index 00000000000..c7e35f17073 --- /dev/null +++ b/changelogs/fragments/53323-no-psycopg2-for-dump-and-restore.yaml @@ -0,0 +1,8 @@ +bugfixes: + - States ``dump`` and ``restore`` only need pg_dump and pg_restore. These tools + don't use psycopg2 so this change tries to avoid the use of it in these + cases. Fixes https://github.com/ansible/ansible/issues/35906 + - Replace the fix for https://github.com/ansible/ansible/issues/39412 + made in https://github.com/ansible/ansible/pull/39483 when using a compression + program. This now uses a FIFO file to ensure failure detection of pg_dump. + The Windows compatibility is completely dropped in this case. diff --git a/lib/ansible/modules/database/postgresql/postgresql_db.py b/lib/ansible/modules/database/postgresql/postgresql_db.py index ab69d6e977f..89cd7c4ea18 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_db.py +++ b/lib/ansible/modules/database/postgresql/postgresql_db.py @@ -83,6 +83,8 @@ options: type: str default: postgres version_added: "2.5" +notes: +- State C(dump) and C(restore) don't require I(psycopg2) since version 2.8. author: "Ansible Core Team" extends_documentation_fragment: - postgres @@ -281,7 +283,12 @@ def db_dump(module, target, target_opts="", cmd += " {0} ".format(target_opts) if comp_prog_path: - cmd = '{0}|{1} > {2}'.format(cmd, comp_prog_path, pipes.quote(target)) + # Use a fifo to be notified of an error in pg_dump + # Using shell pipe has no way to return the code of the first command + # in a portable way. + fifo = os.path.join(module.tmpdir, 'pg_fifo') + os.mkfifo(fifo) + cmd = '{1} <{3} > {2} & {0} >{3}'.format(cmd, comp_prog_path, pipes.quote(target), fifo) else: cmd = '{0} > {1}'.format(cmd, pipes.quote(target)) @@ -397,9 +404,6 @@ def main(): supports_check_mode=True ) - if not HAS_PSYCOPG2: - module.fail_json(msg=missing_required_lib('psycopg2'), exception=PSYCOPG2_IMP_ERR) - db = module.params["db"] owner = module.params["owner"] template = module.params["template"] @@ -413,6 +417,11 @@ def main(): maintenance_db = module.params['maintenance_db'] session_role = module.params["session_role"] + raw_connection = state in ("dump", "restore") + + if not HAS_PSYCOPG2 and not raw_connection: + module.fail_json(msg=missing_required_lib('psycopg2'), exception=PSYCOPG2_IMP_ERR) + # To use defaults values, keyword arguments must be absent, so # check which values are empty and don't include in the **kw # dictionary @@ -437,34 +446,35 @@ def main(): target = "{0}/{1}.sql".format(os.getcwd(), db) target = os.path.expanduser(target) - try: - pgutils.ensure_libs(sslrootcert=module.params.get('ssl_rootcert')) - db_connection = psycopg2.connect(database=maintenance_db, **kw) - - # Enable autocommit so we can create databases - if psycopg2.__version__ >= '2.4.2': - db_connection.autocommit = True - else: - db_connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) - cursor = db_connection.cursor(cursor_factory=psycopg2.extras.DictCursor) - - except pgutils.LibraryError as e: - module.fail_json(msg="unable to connect to database: {0}".format(to_native(e)), exception=traceback.format_exc()) - - except TypeError as e: - if 'sslrootcert' in e.args[0]: - module.fail_json(msg='Postgresql server must be at least version 8.4 to support sslrootcert. Exception: {0}'.format(to_native(e)), - exception=traceback.format_exc()) - module.fail_json(msg="unable to connect to database: %s" % to_native(e), exception=traceback.format_exc()) - - except Exception as e: - module.fail_json(msg="unable to connect to database: %s" % to_native(e), exception=traceback.format_exc()) - - if session_role: + if not raw_connection: try: - cursor.execute('SET ROLE %s' % pg_quote_identifier(session_role, 'role')) + pgutils.ensure_libs(sslrootcert=module.params.get('ssl_rootcert')) + db_connection = psycopg2.connect(database=maintenance_db, **kw) + + # Enable autocommit so we can create databases + if psycopg2.__version__ >= '2.4.2': + db_connection.autocommit = True + else: + db_connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) + cursor = db_connection.cursor(cursor_factory=psycopg2.extras.DictCursor) + + except pgutils.LibraryError as e: + module.fail_json(msg="unable to connect to database: {0}".format(to_native(e)), exception=traceback.format_exc()) + + except TypeError as e: + if 'sslrootcert' in e.args[0]: + module.fail_json(msg='Postgresql server must be at least version 8.4 to support sslrootcert. Exception: {0}'.format(to_native(e)), + exception=traceback.format_exc()) + module.fail_json(msg="unable to connect to database: %s" % to_native(e), exception=traceback.format_exc()) + except Exception as e: - module.fail_json(msg="Could not switch role: %s" % to_native(e), exception=traceback.format_exc()) + module.fail_json(msg="unable to connect to database: %s" % to_native(e), exception=traceback.format_exc()) + + if session_role: + try: + cursor.execute('SET ROLE %s' % pg_quote_identifier(session_role, 'role')) + except Exception as e: + module.fail_json(msg="Could not switch role: %s" % to_native(e), exception=traceback.format_exc()) try: if module.check_mode: @@ -487,9 +497,6 @@ def main(): module.fail_json(msg=to_native(e), exception=traceback.format_exc()) elif state in ("dump", "restore"): - if not db_exists(cursor, db) and state == "dump": - module.fail_json( - msg="database \"{db}\" does not exist".format(db=db)) method = state == "dump" and db_dump or db_restore try: rc, stdout, stderr, cmd = method(module, target, target_opts, db, **kw)