Repository: ansible/ansible
Pull request: postgresql_db - fix 40424 issue
##### SUMMARY
fix https://github.com/ansible/ansible/issues/40424

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request
 ##### ADDITIONAL INFORMATION
It's related to pg_dump bug that I found - it  return rc 0 if there are errors related with pg_hba.conf file.
```
-bash-4.2$ /usr/bin/pg_dump acme --port=5432 --username=postgres|/usr/bin/bzip2 > examplecom-2018-05-18T2334.sql.bz2
pg_dump: [archiver (db)] connection to database "acme" failed: FATAL:  pg_hba.conf rejects connection for host "[local]", user "postgres", database "acme", SSL off
-bash-4.2$ echo $?
0
```
So, we need to parse stderr to catch it.

In other cases, pg_dump returns non zero code as expected.

Problem statement:
Module postgresql_db (state: dump) exits with 0 return code even if dump error occured
<!---
Verify first that your issue/request is not already reported on GitHub.
THIS FORM WILL BE READ BY A MACHINE, COMPLETE ALL SECTIONS AS DESCRIBED.
Also test if the latest release, and devel branch are affected too.
ALWAYS add information AFTER (OUTSIDE) these html comments.
Otherwise it may end up being automatically closed by our bot. -->

##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
<!--- Insert, BELOW THIS COMMENT, the name of the module, plugin, task or feature.
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path-->
postgresql_db
##### ANSIBLE VERSION
<!--- Paste, BELOW THIS COMMENT, verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.2
  config file = /home/nbw/ansible/webmechanic/ansible.cfg
  configured module search path = [u'/home/nbw/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.6/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.6.6 (r266:84292, Aug 18 2016, 15:13:37) [GCC 4.4.7 20120313 (Red Hat 4.4.7-17)]
```

##### CONFIGURATION
<!--- If using Ansible 2.4 or above, paste, BELOW THIS COMMENT, the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).-->
ANSIBLE_PIPELINING(/home/nbw/ansible/ansible.cfg) = True
DEFAULT_HOST_LIST(/home/nbw/ansible/ansible.cfg) = [u'/home/nbw/ansible/inventory']
RETRY_FILES_ENABLED(/home/nbw/ansible/ansible.cfg) = False
##### OS / ENVIRONMENT
<!--- Mention, BELOW THIS COMMENT, the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.-->
CentOS release 6.9 (Final)
##### SUMMARY
<!--- Explain the problem briefly -->
postgresql_db module exits with "changed" status even if pg_dump failed 
##### STEPS TO REPRODUCE
<!--- For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used. -->

<!--- Paste example playbooks or commands between quotes below -->
```yaml
- postgresql_db:
    name: examplecom
    state: dump
    target: /var/preserve/examplecom-2018-05-18T2334.sql.bz2
```
Dump has zero size, but no fail. If run playbook with `-vvv`, we can see full command and error
```
"cmd": "/bin/pg_dump examplecom --host=pg --port=5432 --username=ansible -Fc |/bin/bzip2 > /var/preserve/examplecom-2018-05-18T2334.sql.bz2", "msg": "", "stderr": "pg_dump: [archiver (db)] connection to database \\"examplecom\\" failed: FATAL:  no pg_hba.conf entry for host \\"172.16.110.97\\", user \\"ansible\\", database \\"examplecom\\", SSL off\\n",
```
<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
Task failed with "msg": "pg_dump: [archiver (db)] connection to database \"examplecom\" failed: FATAL: ..."
##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->
Task completed successfully with status "changed"
<!--- Paste verbatim command output between quotes below -->

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/database/postgresql/postgresql_db.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/database/postgresql/postgresql_db.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @matburt
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
This should also fixes #39412
cc @Andersson007 @amenonsen @andytom @dagwieers @jbscalia @kostiantyn-nemchenko @nerzhul
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
cc @Dorn- @kustodian @sebasmannem
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: 40424_postgresql_db_fix: fix fedora29 rhel8 errors

Patch to review:
diff --git a/changelogs/fragments/40424_postgresql_db_not_failed_when_dump_err.yml b/changelogs/fragments/40424_postgresql_db_not_failed_when_dump_err.yml
new file mode 100644
index 00000000000000..9be407bf4ea2ad
--- /dev/null
+++ b/changelogs/fragments/40424_postgresql_db_not_failed_when_dump_err.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - postgresql_db - the module fails not always when pg_dump errors occured (https://github.com/ansible/ansible/issues/40424).
diff --git a/lib/ansible/modules/database/postgresql/postgresql_db.py b/lib/ansible/modules/database/postgresql/postgresql_db.py
index ab69d6e977f168..5cbbd2f416d6d0 100644
--- a/lib/ansible/modules/database/postgresql/postgresql_db.py
+++ b/lib/ansible/modules/database/postgresql/postgresql_db.py
@@ -495,6 +495,10 @@ def main():
                 rc, stdout, stderr, cmd = method(module, target, target_opts, db, **kw)
                 if rc != 0:
                     module.fail_json(msg=stderr, stdout=stdout, rc=rc, cmd=cmd)
+
+                elif stderr and ('FATAL' in str(stderr) or 'ERROR' in str(stderr)):
+                    module.fail_json(msg=stderr, stdout=stdout, rc=1, cmd=cmd)
+
                 else:
                     module.exit_json(changed=True, msg=stdout, stderr=stderr, rc=rc, cmd=cmd)
             except SQLParseError as e:


Code review comments:
diff_hunk: @@ -495,6 +495,10 @@ def main():
                 rc, stdout, stderr, cmd = method(module, target, target_opts, db, **kw)
                 if rc != 0:
                     module.fail_json(msg=stderr, stdout=stdout, rc=rc, cmd=cmd)
+
+                elif stderr and ('FATAL' in str(stderr) or 'ERROR' in str(stderr)):
+                    module.fail_json(msg=stderr, stdout=stdout, rc=1, cmd=cmd)
path: lib/ansible/modules/database/postgresql/postgresql_db.py
text: @user1:
Normally we have an `stdout` and an `stderr` return value. Personally I would prefer to have the `msg` value indicate that an error occurred, and keep the details in `stderr`. Since it may be multiline output I don't know if putting stderr in msg is a clever thing to do :-/

@author:
@user1 good job! Fixed. PTAL, please.
Looks really good
Dump done example:
```
    "msg": "Dump of acme has been done", 
    "rc": 0, 
    "stderr": "pg_dump: server version: 11.1; pg_dump version: 9.2.24\npg_dump: aborting because of server version mismatch\n", 
    "stderr_lines": [
        "pg_dump: server version: 11.1; pg_dump version: 9.2.24", 
        "pg_dump: aborting because of server version mismatch"
    ], 
    "stdout": "", 
    "stdout_lines": []
```
Dump failed example:
```
    "msg": "Dump of database acme failed", 
    "rc": 1, 
    "stderr": "pg_dump: [archiver (db)] connection to database \"acme\" failed: FATAL:  pg_hba.conf rejects connection for host \"[local]\", user \"postgres\", database \"acme\", SSL off\n", 
    "stderr_lines": [
        "pg_dump: [archiver (db)] connection to database \"acme\" failed: FATAL:  pg_hba.conf rejects connection for host \"[local]\", user \"postgres\", database \"acme\", SSL off"
    ], 
    "stdout": "", 
    "stdout_lines": []
```

@user1:
Hmmm, the "Dump of acme has been done" has not actually been done.

>        "pg_dump: aborting because of server version mismatch"

@author:
Oh;) , sorry, it was early morning... why it returnes 0... Ok 

@author:
Fixed, thank you very much
Dump done:
```
    "msg": "Dump of database acme has been done", 
    "rc": 0, 
    "stderr": "", 
    "stderr_lines": [], 
    "stdout": "", 
    "stdout_lines": []
```
Dump undone:
```
    "msg": "Dump of database acme failed", 
    "rc": 1, 
    "stderr": "pg_dump: [archiver (db)] connection to database \"acme\" failed: FATAL:  pg_hba.conf rejects connection for host \"[local]\", user \"postgres\", database \"acme\", SSL off\n", 
    "stderr_lines": [
        "pg_dump: [archiver (db)] connection to database \"acme\" failed: FATAL:  pg_hba.conf rejects connection for host \"[local]\", user \"postgres\", database \"acme\", SSL off"
    ], 
    "stdout": "", 
    "stdout_lines": []
```
And also undone
```
    "msg": "Dump of database acme failed", 
    "rc": 0, 
    "stderr": "pg_dump: server version: 11.1; pg_dump version: 9.2.24\npg_dump: aborting because of server version mismatch\n", 
    "stderr_lines": [
        "pg_dump: server version: 11.1; pg_dump version: 9.2.24", 
        "pg_dump: aborting because of server version mismatch"
    ], 
    "stdout": "", 
    "stdout_lines": []
```