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

pg_upgrade.yml: Add ‘string’ filter for postgresql version variables #777

Merged
merged 11 commits into from
Oct 3, 2024

Conversation

ruslanloman
Copy link
Contributor

Description

Ansible pg_upgrade.yml pre_check is failed when postgresql_version is int, because
regex_replace ansible filter expects a string or pattern.

Initially postgresql_version was a string, but changed in this PR to int

ansible-playbook pg_upgrade.yml -e "pg_old_version=15 pg_new_version=16"

TASK [upgrade : Make sure that the old and new data and config directories do not match] ***********************************************
fatal: [192.168.40.135]: FAILED! => {"msg": "The conditional check '(pg_old_datadir == pg_new_datadir) or (pg_old_confdir == pg_new_confdir)' failed. The error was: An unhandled exception occurred while templating '{{ postgresql_data_dir | regex_replace('(/$)', '') | regex_replace(postgresql_version, pg_old_version) }}'. Error was a <class 'ansible.errors.AnsibleError'>, original message: Unexpected templating type error occurred on ({{ postgresql_data_dir | regex_replace('(/$)', '') | regex_replace(postgresql_version, pg_old_version) }}): first argument must be string or compiled pattern. first argument must be string or compiled pattern\n\nThe error appears to be in '/home/loman/postgresql_cluster/automation/roles/upgrade/tasks/pre_checks.yml': line 14, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n# Stop, if the directories of the old and new versions are the same\n- name: \"Make sure that the old and new data and config directories do not match\"\n  ^ here\n"}

@vitabaks
Copy link
Owner

vitabaks commented Oct 2, 2024

Thanks! @ruslanloman

It is better to add a string filter to the appropriate parts of the code than to require users to specify the version as a string. This will be a more flexible and more reliable option.

example

{{ postgresql_data_dir | string | regex_replace('(/$)', '') | regex_replace(postgresql_version | string, pg_old_version | string) }}

To avoid this error, we need to make sure that all arguments passed to regex_replace are strings. To do this, we can use the string filter, which explicitly converts the value to a string.

@ruslanloman ruslanloman closed this Oct 2, 2024
@vitabaks
Copy link
Owner

vitabaks commented Oct 2, 2024

@ruslanloman Are you planning a new PR or can I do it myself?

@ruslanloman
Copy link
Contributor Author

ruslanloman commented Oct 2, 2024

Hi @vitabaks,

Thank you for your comment. Yes, I'm preparing an update for PR. I agree about this part "require users to specify the version as a string". I found out that we can use jinja2 replace filter instead of ansible regex_replace filter. I've updated code, but checking that deployment and upgrade works as expected.

automation/vars/main.yml Outdated Show resolved Hide resolved
@ruslanloman
Copy link
Contributor Author

@vitabaks Updated. My local deployment/upgrade test passed.

@vitabaks
Copy link
Owner

vitabaks commented Oct 2, 2024

recommended to use a string filter to avoid possible errors, example:

replace(postgresql_version | string, pg_old_version | string)

… int

  * regex_replace ansible filter expects a string or pattern
    otherwise it fails with the error.
    "first argument must be string or compiled pattern"
@ruslanloman
Copy link
Contributor Author

recommended to use a string filter to avoid possible errors, example:

replace(postgresql_version | string, pg_old_version | string)

Updated. Thanks

@vitabaks vitabaks changed the title Ansible pg_upgrade.yml pre_check is failed when postgresql_version is int pg_upgrade.yml: Add ‘string’ filter for postgresql version variables Oct 3, 2024
@vitabaks vitabaks merged commit 9ce1754 into vitabaks:master Oct 3, 2024
15 checks passed
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.

2 participants