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.

Sign up for GitHub

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

Jump to bottom

Use the the original PostgreSQL configuration file #41

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Use the the original PostgreSQL configuration file #41

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
82efa2d
use the proginal postgresql configuration file
khaledk2 May 23, 2024
3637a0e
Allow updating the config, add test case and restore listen_addresses
khaledk2 May 25, 2024
35930b0
Fix re-run the role without having the org file
khaledk2 Jun 11, 2024
0879c9f
fix syntax error
khaledk2 Jun 11, 2024
9ee77fa
Clean up the code and set_facts
khaledk2 Jun 11, 2024
9790b98
remove redundant check
khaledk2 Jun 12, 2024
743d5f3
Improve tasks name
khaledk2 Jun 14, 2024
dcaa4eb
clean up the configuration file and copy to org in case of org is not…
khaledk2 Jun 27, 2024
8151508
Fix check mode orginal file issue
khaledk2 Jul 7, 2024
5b1c41a
clean up the code
khaledk2 Jul 7, 2024
bc2b2ad
Update task name to address Petr's comment
khaledk2 Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
(postgresql_package_version | length > 0) |
ternary('-' + postgresql_package_version, '')
}}
conf_sample_file: "/usr/pgsql-{{ postgresql_version }}/share/postgresql.conf.sample"

# Attributes are parsed and used to set facts at tasks/debian.yml.
# Debian variation, following the same principles of postgresql_dist_redhat
Expand All @@ -62,4 +63,17 @@
conf_postgresql_src: postgresql-conf.j2
datadir: /var/lib/postgresql/{{ postgresql_version }}/main
basename: postgresql-{{ postgresql_version }}
conf_sample_file: "/usr/share/postgresql/{{ postgresql_version }}/postgresql.conf.sample"
service: postgresql

# conf_sample_file: sample configuration file, it should be copied during the installation by the installation package
sbesson marked this conversation as resolved.
Show resolved Hide resolved

# the following two variables are used to download link ofr the source to get the sample configuration file
# It will be used in case of the sample file is not find locally for any reason
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to have a hard fail in case conf_sample_file does not exist?

Are you aware of any scenario where this file could legitimately be expected to be missing?

Copy link
Contributor Author

@khaledk2 khaledk2 Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On our RHEL 9 test machine, there are 4 PostgreSQL versions have been installed (13, 14, 15, 16). Only version 16 folder has the sample file and the others do not have this file. I am not sure how these versions have been installed but I think the file should be there. I am not sure why it is missing. So, I have added the logic of downloading and extracting the file to account for similar cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting (and slightly concerning). Note all of the PSQL sample configuration file checks seem to be currently skipped in the Molecule tests on both Rocky Linux. I don't know if that's expected or indicates another issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will check for the sample file if the original file does not exist and the configuration file has been edited before, i.e. in case of installing the same PostgreSQL version with a previous role version.

versions_source_download:
13: "13.15"
14: "14.12"
15: "15.7"
16: "16.3"

source_download_linnk: "https://ftp.postgresql.org/pub/source/v{{ versions_source_download [postgresql_version] }}/postgresql-{{ versions_source_download [postgresql_version] }}.tar.gz"

Check warning on line 79 in defaults/main.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9)

jinja[spacing] 

Jinja2 spacing could be improved: https://ftp.postgresql.org/pub/source/v{{ versions_source_download \[postgresql_version] }}/postgresql-{{ versions_source_download \[postgresql_version] }}.tar.gz -> https://ftp.postgresql.org/pub/source/v{{ versions_source_download\[postgresql_version] }}/postgresql-{{ versions_source_download\[postgresql_version] }}.tar.gz

Check warning on line 79 in defaults/main.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204)

jinja[spacing] 

Jinja2 spacing could be improved: https://ftp.postgresql.org/pub/source/v{{ versions_source_download \[postgresql_version] }}/postgresql-{{ versions_source_download \[postgresql_version] }}.tar.gz -> https://ftp.postgresql.org/pub/source/v{{ versions_source_download\[postgresql_version] }}/postgresql-{{ versions_source_download\[postgresql_version] }}.tar.gz
4 changes: 2 additions & 2 deletions molecule/resources/tests/test_default.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ def test_server_listen(host):
def test_backup_config_exist(host):
version = get_version(host)
if host.system_info.distribution == 'rocky':
config_backup = '/var/lib/pgsql/{version}/data/postgresql.conf.backup'
config_backup = '/var/lib/pgsql/{version}/data/postgresql.conf.org'
else:
config_backup = '/etc/postgresql/{version}/main/postgresql.conf.backup'
config_backup = '/etc/postgresql/{version}/main/postgresql.conf.org'
with host.sudo():
backup_file = config_backup.format(version=version)
assert host.file(backup_file).is_file
Expand Down
67 changes: 52 additions & 15 deletions tasks/initialise.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
# tasks file for ome.postgresql
##Ansible managed

Check warning on line 3 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

3:3 [comments] missing starting space in comment

Check warning on line 3 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

3:3 [comments] missing starting space in comment
sbesson marked this conversation as resolved.
Show resolved Hide resolved

- block:
- name: postgres | set permissions on data directory
Expand All @@ -22,20 +23,19 @@
PGSETUP_INITDB_OPTIONS: >-
--encoding=UTF8 --locale=en_US.UTF-8 --auth-host=md5

- name: postgres | Check for presence of "Modified by ome postgresql ansible role" in file

Check failure on line 26 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

26:81 [line-length] line too long (94 > 80 characters)

Check failure on line 26 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

26:81 [line-length] line too long (94 > 80 characters)
ansible.builtin.lineinfile:
path: "{{ postgresql_dist_confdir }}/postgresql.conf"
line: "#Modified by ome postgresql ansible role"
line: "#Ansible managed"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the new logic is to always recreate a configuration file from the combination of the original PSQL configuration file and the role variables (in the same spirit as the previous template task), my impression is this check is redundant and the config_file_changed logic should be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modified file check should work for the fresh installation, as the original configure file will be copied from the configuration file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous RedHat/Rocky template had # {{ ansible_managed }} so this will mark this file as not changed. Is that the expectation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed the template to include #{{ ansible_managed }}.

It will check for it, i.e. looking for "#Ansible managed" in the file; if it is not found it will mark it as an alerted configuration file and start looking for the org file (postgresql.conf.org) that may exist in the current folder, if not it will invoke the tasks inside org_config_file.yml to determine the sample file.
If the configuration file does not have the "#Ansible managed" it will assume the file is the original configuration file then copy it to postgresql.conf.org and use its contents in the configuration template.

The configuration file does not have "#Ansible managed" on test122b; it will assume it is the original configuration file and copy it to org.
I think because you are in check mode, the file is not copied so it failed when trying to get the file contents.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking the example of test122b here

[sbesson@test122b-database ~]$  sudo tail -n 20 /var/lib/pgsql/16/data/postgresql.conf


#------------------------------------------------------------------------------
# CUSTOMIZED OPTIONS
#------------------------------------------------------------------------------

# Add settings for extensions here

listen_addresses = '*'

shared_buffers = 8002MB


listen_addresses = '*'

effective_cache_size = 23880MB

shared_buffers = 7960MB

[sbesson@test122b-database ~]$ sudo head -n 2 /var/lib/pgsql/16/data/postgresql.conf
#Modified by ome postgresql ansible role
# -----------------------------

It does not have the #Ansible managed marker but it is also not the original configuration.
In that case, copying it as postgresql.conf.org is incorrect and also means that every invocation of the role will append the options at the bottom of the configuration file.

state: absent
check_mode: yes

Check warning on line 31 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

31:19 [truthy] truthy value should be one of [false, true]

Check warning on line 31 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

31:19 [truthy] truthy value should be one of [false, true]
changed_when: false
register: config_file_changed
sbesson marked this conversation as resolved.
Show resolved Hide resolved

- name: postgres | Check that the postgresql.conf.backup file exists
ansible.builtin.stat:
path: "{{ postgresql_dist_confdir }}/postgresql.conf.backup"
register: backup_result

path: "{{ postgresql_dist_confdir }}/postgresql.conf.org"
register: org_file

# read the default postgresql configuration file
- name: postgres | get the postgres conf file contents
Expand All @@ -44,34 +44,70 @@
src: "{{ postgresql_dist_confdir }}/postgresql.conf"
register: postgres_config_file_contents_o
when:
- not backup_result.stat.exists
- not org_file.stat.exists

- name: Print the org_file.stat.exists
ansible.builtin.debug:
msg:

Check failure on line 51 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

51:8 [indentation] wrong indentation: expected 8 but found 7

Check failure on line 51 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

51:8 [indentation] wrong indentation: expected 8 but found 7
- System 1 {{ org_file.stat.exists }} and {{ config_file_changed.found }}

Check failure on line 52 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

52:8 [indentation] wrong indentation: expected 9 but found 7

Check failure on line 52 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

52:8 [indentation] wrong indentation: expected 9 but found 7
- "{{ postgresql_dist_confdir }}/postgresql.conf.org"
- "{{ postgresql_dist_redhat.conf_sample_file }}"

- name: postgres | get the orginal configuration file
become_user: "{{ postgresql_become_user }}"
import_tasks: org_config_file.yml
when:
- not org_file.stat.exists
- config_file_changed.found

- name: postgres | get the orginal configuration file
become_user: "{{ postgresql_become_user }}"
ansible.builtin.slurp:
src: "{{ postgresql_dist_confdir }}/postgresql.conf.org"
register: postgres_config_file_contents_c
when:
- not org_file.stat.exists
- config_file_changed.found


# read the default postgresql configuration file
- name: postgres | get the postgres conf file contents from backup
- name: postgres | get the postgres conf file contents from org
become_user: "{{ postgresql_become_user }}"
ansible.builtin.slurp:
src: "{{ postgresql_dist_confdir }}/postgresql.conf.backup"
src: "{{ postgresql_dist_confdir }}/postgresql.conf.org"
register: postgres_config_file_contents_b
when: backup_result.stat.exists
when: org_file.stat.exists


- set_fact: postgres_config_file_contents={{ postgres_config_file_contents_o }}

Check failure on line 82 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

82:81 [line-length] line too long (83 > 80 characters)

Check failure on line 82 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

82:81 [line-length] line too long (83 > 80 characters)
when:
- not backup_result.stat.exists
- not org_file.stat.exists

- set_fact: postgres_config_file_contents={{ postgres_config_file_contents_b }}

Check failure on line 86 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

86:81 [line-length] line too long (83 > 80 characters)

Check failure on line 86 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

86:81 [line-length] line too long (83 > 80 characters)
when:
- backup_result.stat.exists
- org_file.stat.exists

- name: postgres | Copy a postgresql.conf to postgresql.conf.backup
- set_fact: postgres_config_file_contents={{ postgres_config_file_contents_c }}

Check failure on line 90 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

90:81 [line-length] line too long (83 > 80 characters)

Check failure on line 90 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

90:81 [line-length] line too long (83 > 80 characters)
when:
- not org_file.stat.exists
- config_file_changed.found

- name: postgres | Copy a postgresql.conf to postgresql.conf.org
become_user: "{{ postgresql_become_user }}"
ansible.builtin.copy:
src: "{{ postgresql_dist_confdir }}/postgresql.conf"
dest: "{{ postgresql_dist_confdir }}/postgresql.conf.backup"
dest: "{{ postgresql_dist_confdir }}/postgresql.conf.org"
remote_src: yes

Check warning on line 100 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

100:21 [truthy] truthy value should be one of [false, true]

Check warning on line 100 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

100:21 [truthy] truthy value should be one of [false, true]
when:
- not backup_result.stat.exists
- not org_file.stat.exists
- config_file_changed.found | default(0) == 0

- name: Print the org_file.stat.exists
ansible.builtin.debug:
msg: System {{ org_file.stat.exists }} and {{ config_file_changed.found }}

Check failure on line 107 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

107:81 [line-length] line too long (82 > 80 characters)

Check failure on line 107 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

107:81 [line-length] line too long (82 > 80 characters)
# when:

Check warning on line 108 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

108:7 [comments-indentation] comment not indented like content

Check warning on line 108 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

108:7 [comments-indentation] comment not indented like content
# - (not org_file.stat.exists and config_file_changed.found) or config_file_changed.found or org_file.stat.exists

- name: postgres | copy postgresql config file
become_user: "{{ postgresql_become_user }}"
ansible.builtin.template:
Expand All @@ -82,8 +118,9 @@
owner: "{{ postgresql_become_user }}"
notify:
- restart postgresql
when:
- config_file_changed.found | default(0) == 0 or backup_result.stat.exists
when: (not org_file.stat.exists and config_file_changed.found) or not config_file_changed.found or org_file.stat.exists
sbesson marked this conversation as resolved.
Show resolved Hide resolved
# - not org_file.stat.exists

Check warning on line 122 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

122:9 [comments-indentation] comment not indented like content

Check warning on line 122 in tasks/initialise.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

122:9 [comments-indentation] comment not indented like content
# - config_file_changed.found

- name: postgres | configure client authorisation
become_user: "{{ postgresql_become_user }}"
Expand Down
45 changes: 45 additions & 0 deletions tasks/org_config_file.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
# get the original configuration file

- name: postgres | Check that the postgresql.conf.sample file exists (ubuntu)
ansible.builtin.stat:
path: "{{ postgresql_dist_debian.conf_sample_file }}"
register: conf_sample_file
when: ansible_os_family | lower == 'debian'

- name: postgres | postgres | Check that the postgresql.conf.sample file exists (redhat)

Check failure on line 10 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

10:81 [line-length] line too long (88 > 80 characters)

Check failure on line 10 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

10:81 [line-length] line too long (88 > 80 characters)
sbesson marked this conversation as resolved.
Show resolved Hide resolved
ansible.builtin.stat:
path: "{{ postgresql_dist_redhat.conf_sample_file }}"
register: conf_sample_file
when: ansible_os_family | lower == 'redhat'

- name: postgres | copy the sample file to org
become_user: "{{ postgresql_become_user }}"
ansible.builtin.copy:
src: "{{ postgresql_dist_redhat.conf_sample_file }}"
dest: "{{ postgresql_dist_confdir }}/postgresql.conf.org"
when:
- conf_sample_file.stat.exists

Check failure on line 22 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

22:7 [hyphens] too many spaces after hyphen

Check failure on line 22 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

22:7 [hyphens] too many spaces after hyphen

- name: postgres | download the source code
ansible.builtin.get_url:
url: "{{ source_download_linnk }}"
dest: /tmp/postgresql.tar.gz
when:
- not conf_sample_file.stat.exists

- name: postgres | extract the files
ansible.builtin.unarchive:
src: /tmp/postgresql.tar.gz
dest: /tmp
remote_src: yes

Check warning on line 35 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

35:17 [truthy] truthy value should be one of [false, true]

Check warning on line 35 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

35:17 [truthy] truthy value should be one of [false, true]
when:
- not conf_sample_file.stat.exists

- name: postgres | copy the file to org file
become_user: "{{ postgresql_become_user }}"
ansible.builtin.copy:
src: "/tmp/postgresql-{{ versions_source_download [postgresql_version] }}/src/backend/utils/misc/postgresql.conf.sample"

Check failure on line 42 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9) 

42:81 [line-length] line too long (124 > 80 characters)

Check warning on line 42 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (rockylinux9)

jinja[spacing] 

Jinja2 spacing could be improved: /tmp/postgresql-{{ versions_source_download \[postgresql_version] }}/src/backend/utils/misc/postgresql.conf.sample -> /tmp/postgresql-{{ versions_source_download\[postgresql_version] }}/src/backend/utils/misc/postgresql.conf.sample

Check failure on line 42 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204) 

42:81 [line-length] line too long (124 > 80 characters)

Check warning on line 42 in tasks/org_config_file.yml

View workflow job for this annotation

GitHub Actions / Test (ubuntu2204)

jinja[spacing] 

Jinja2 spacing could be improved: /tmp/postgresql-{{ versions_source_download \[postgresql_version] }}/src/backend/utils/misc/postgresql.conf.sample -> /tmp/postgresql-{{ versions_source_download\[postgresql_version] }}/src/backend/utils/misc/postgresql.conf.sample
dest: "{{ postgresql_dist_confdir }}/postgresql.conf.org"
when:
- not conf_sample_file.stat.exists
2 changes: 1 addition & 1 deletion templates/postgresql-conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#Modified by ome postgresql ansible role
#{{ ansible_managed }}
{{ postgres_config_file_contents['content'] | b64decode | regex_replace('}$', '') }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above. If we are using this as the marker of managed file, we'll need to use the same whitespace.

Note also that postgresql-conf-10-ubuntu.j2 did not include this comment so the logic will not work with PSQL installations on Ubuntu with earlier versions of the role


listen_addresses = {{ postgresql_server_listen }}
Expand Down
Loading