ANXS / postgresql

Fairly full featured Ansible role for Postgresql.
http://anxs.io/
MIT License
848 stars 573 forks source link

feat: Support PostgreSQL v13 #504

Closed gclough closed 3 years ago

gclough commented 3 years ago

Added support for PostgreSQL v13

robustq commented 3 years ago

@gclough On my local, it seems that the synchronous_standby_names is producing invalid output for the postgresql.conf added for PG13. Ref: https://github.com/ANXS/postgresql/pull/504/files#diff-af3f9fc95b5e2ef58744b0d05ed18d67128e49097c7f491eaadf99856ff07059R307

With the current template, this error occurs:

LOG:  invalid value for parameter "synchronous_standby_names": " (*)"

And indeed, in the conf file, we see:

synchronous_standby_names = ' (*)'      # standby servers that provide sync rep

Reverting to the syntax used by the postgresql.conf-12.j2 file resolved the issue:

synchronous_standby_names = '{% if postgresql_synchronous_standby_names != [] %}{% if postgresql_synchronous_standby_choose_sync != "" and postgresql_synchronous_standby_num_sync != "" %}{{ postgresql_synchronous_standby_choose_sync }} {% endif %}{% if postgresql_synchronous_standby_num_sync != "" %}{{ postgresql_synchronous_standby_num_sync }} ({% endif %}"{{ postgresql_synchronous_standby_names | join('\",\"') }}"{% if postgresql_synchronous_standby_num_sync != "" %}){% endif %}{% endif %}'   # standby servers that provide sync rep
robustq commented 3 years ago

@gclough Thank you again for your work on this - another update is that the password_encryption settings have changed per #460

password_encryption = {{ 'on' if postgresql_password_encryption else 'off' }}       # md5 or scram-sha-256

to

password_encryption = {{ postgresql_password_encryption }}     # md5 or scram-sha-256
egmont1227 commented 3 years ago

@gclough Thank you again for your work on this - another update is that the password_encryption settings have changed per #460

And this got fixes for default parameters as well, see #507

maglub commented 3 years ago

The debian8 test fails:

TASK [ANXS.postgresql : PostgreSQL | Add PostgreSQL repository | apt] **********
changed: [postgresql-10]
changed: [postgresql-9.6]
changed: [postgresql-11]
changed: [postgresql-12]
changed: [postgresql-9.5]
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: , E:Some index files failed to download. They have been ignored, or old ones used instead.
fatal: [postgresql-13]: FAILED! => {"changed": false, "failed": true, "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_NTs_WJ/ansible_module_apt_repository.py\", line 556, in <module>\n    main()\n  File \"/tmp/ansible_NTs_WJ/ansible_module_apt_repository.py\", line 544, in main\n    cache.update()\n  File \"/usr/lib/python2.7/dist-packages/apt/cache.py\", line 462, in update\n    raise FetchFailedException(e)\napt.cache.FetchFailedException: W:Failed to fetch http://apt.postgresql.org/pub/repos/apt/dists/jessie-pgdg/InRelease  Unable to find expected entry '13/binary-amd64/Packages' in Release file (Wrong sources.list entry or malformed file)\n, E:Some index files failed to download. They have been ignored, or old ones used instead.\n", "module_stdout": "", "msg": "MODULE FAILURE", "rc": 0}
gclough commented 3 years ago

The debian8 test fails:

Yes... PostgreSQL v13 isn't supported on Debian 8, so I need to put some login in the pipeline to not test that configuration.

RCM7 commented 3 years ago

Tested this branch with Debian buster and it worked well 👌

gclough commented 3 years ago

@gclough Thank you again for your work on this - another update is that the password_encryption settings have changed per #460

password_encryption = {{ 'on' if postgresql_password_encryption else 'off' }}     # md5 or scram-sha-256

to

password_encryption = {{ postgresql_password_encryption }}     # md5 or scram-sha-256

Fixed with the next commit.

gclough commented 3 years ago

@gclough Thank you again for your work on this - another update is that the password_encryption settings have changed per #460

And this got fixes for default parameters as well, see #507

Fixed in the next commit.

gclough commented 3 years ago
synchronous_standby_names = '{% if postgresql_synchronous_standby_names != [] %}{% if postgresql_synchronous_standby_choose_sync != "" and postgresql_synchronous_standby_num_sync != "" %}{{ postgresql_synchronous_standby_choose_sync }} {% endif %}{% if postgresql_synchronous_standby_num_sync != "" %}{{ postgresql_synchronous_standby_num_sync }} ({% endif %}"{{ postgresql_synchronous_standby_names | join('\",\"') }}"{% if postgresql_synchronous_standby_num_sync != "" %}){% endif %}{% endif %}' # standby servers that provide sync rep

Sorry about that... not sure what happened there. I've reverted it to use the v12 syntax, which is the same format in v13 so there's no need to change it.

gclough commented 3 years ago

Tested this branch with Debian buster and it worked well 👌

Added to Docker tests and readme

gclough commented 3 years ago

The debian8 test fails:

TASK [ANXS.postgresql : PostgreSQL | Add PostgreSQL repository | apt] **********
changed: [postgresql-10]
changed: [postgresql-9.6]
changed: [postgresql-11]
changed: [postgresql-12]
changed: [postgresql-9.5]
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: , E:Some index files failed to download. They have been ignored, or old ones used instead.
fatal: [postgresql-13]: FAILED! => {"changed": false, "failed": true, "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_NTs_WJ/ansible_module_apt_repository.py\", line 556, in <module>\n    main()\n  File \"/tmp/ansible_NTs_WJ/ansible_module_apt_repository.py\", line 544, in main\n    cache.update()\n  File \"/usr/lib/python2.7/dist-packages/apt/cache.py\", line 462, in update\n    raise FetchFailedException(e)\napt.cache.FetchFailedException: W:Failed to fetch http://apt.postgresql.org/pub/repos/apt/dists/jessie-pgdg/InRelease  Unable to find expected entry '13/binary-amd64/Packages' in Release file (Wrong sources.list entry or malformed file)\n, E:Some index files failed to download. They have been ignored, or old ones used instead.\n", "module_stdout": "", "msg": "MODULE FAILURE", "rc": 0}

Correct... PostgreSQL v13 doesn't support Debian 8. I've added Debian 9 and 10 to the Travis tests.

gclough commented 3 years ago

@robustq , @egmont1227 , @maglub , and @RCM7 . Could you spare some time to check this again now that I've included all of the comments?

I've stripped it right back to just CentOS v7, and Debian v9. We can do subsequent Merge Requests to fix later versions, and add back Ubuntu. I figured it was best to get us back to at least "something" working, then build from there.

image

gclough commented 3 years ago

I plan to merge this on Monday 15/Mar, unless I hear of any objections? We need a functional base to build from, so that we can add back other OS support, etc.