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 <bugs-github@antipoul.fr>

2 files changed, 45 insertions, 30 deletions

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 0000000000..c7e35f1707
--- /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 ab69d6e977..89cd7c4ea1 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)
+    if not raw_connection:
+        try:
+            pgutils.ensure_libs(sslrootcert=module.params.get('ssl_rootcert'))
+            db_connection = psycopg2.connect(database=maintenance_db, **kw)
 
-    except pgutils.LibraryError as e:
-        module.fail_json(msg="unable to connect to database: {0}".format(to_native(e)), exception=traceback.format_exc())
+            # 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 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 pgutils.LibraryError as e:
+            module.fail_json(msg="unable to connect to database: {0}".format(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())
+        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())
 
-    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())
+            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)