Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Use the the original PostgreSQL configuration file #41
base: master
Are you sure you want to change the base?
Use the the original PostgreSQL configuration file #41
Changes from 5 commits
82efa2d
3637a0e
35930b0
0879c9f
9ee77fa
9790b98
743d5f3
dcaa4eb
8151508
5b1c41a
bc2b2ad
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 56 in defaults/main.yml
Check failure on line 56 in defaults/main.yml
Check failure on line 66 in defaults/main.yml
Check failure on line 66 in defaults/main.yml
Check failure on line 69 in defaults/main.yml
Check failure on line 69 in defaults/main.yml
Check failure on line 71 in defaults/main.yml
Check failure on line 71 in defaults/main.yml
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Check warning on line 79 in defaults/main.yml
jinja[spacing]
Check warning on line 79 in defaults/main.yml
jinja[spacing]
Check warning on line 3 in tasks/initialise.yml
Check warning on line 3 in tasks/initialise.yml
Check failure on line 26 in tasks/initialise.yml
Check failure on line 26 in tasks/initialise.yml
There was a problem hiding this comment.
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 theconfig_file_changed
logic should be removed?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
hereIt 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.Check warning on line 31 in tasks/initialise.yml
Check warning on line 31 in tasks/initialise.yml
Check warning on line 45 in tasks/initialise.yml
Check warning on line 45 in tasks/initialise.yml
Check failure on line 64 in tasks/initialise.yml
Check failure on line 64 in tasks/initialise.yml
Check failure on line 76 in tasks/initialise.yml
Check failure on line 76 in tasks/initialise.yml
Check failure on line 10 in tasks/org_config_file.yml
Check failure on line 10 in tasks/org_config_file.yml
Check failure on line 22 in tasks/org_config_file.yml
Check failure on line 22 in tasks/org_config_file.yml
Check warning on line 35 in tasks/org_config_file.yml
Check warning on line 35 in tasks/org_config_file.yml
Check failure on line 42 in tasks/org_config_file.yml
Check warning on line 42 in tasks/org_config_file.yml
jinja[spacing]
Check failure on line 42 in tasks/org_config_file.yml
Check warning on line 42 in tasks/org_config_file.yml
jinja[spacing]