Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support PostgreSQL v13 #504

Merged
merged 7 commits into from
Mar 15, 2021
Merged

feat: Support PostgreSQL v13 #504

merged 7 commits into from
Mar 15, 2021

Conversation

gclough
Copy link
Collaborator

@gclough gclough commented Feb 10, 2021

Added support for PostgreSQL v13

@gclough gclough mentioned this pull request Feb 10, 2021
@robustq
Copy link

robustq commented Feb 21, 2021

@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
Copy link

robustq commented Feb 22, 2021

@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
Copy link
Contributor

egmont1227 commented Feb 24, 2021

@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
Copy link
Collaborator

maglub commented Feb 25, 2021

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 gclough mentioned this pull request Mar 3, 2021
@gclough
Copy link
Collaborator Author

gclough commented Mar 3, 2021

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
Copy link
Collaborator

RCM7 commented Mar 3, 2021

Tested this branch with Debian buster and it worked well 👌

@gclough
Copy link
Collaborator Author

gclough commented Mar 10, 2021

@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 gclough closed this Mar 10, 2021
@gclough
Copy link
Collaborator Author

gclough commented Mar 10, 2021

@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 gclough reopened this Mar 10, 2021
@gclough
Copy link
Collaborator Author

gclough commented Mar 10, 2021

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
Copy link
Collaborator Author

gclough commented Mar 10, 2021

Tested this branch with Debian buster and it worked well 👌

Added to Docker tests and readme

@gclough
Copy link
Collaborator Author

gclough commented Mar 10, 2021

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
Copy link
Collaborator Author

gclough commented Mar 10, 2021

@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
Copy link
Collaborator Author

gclough commented Mar 12, 2021

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.

@gclough gclough mentioned this pull request Mar 12, 2021
@gclough gclough merged commit 9662278 into ANXS:master Mar 15, 2021
@gclough gclough deleted the postgresql_13 branch March 15, 2021 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants