-
Notifications
You must be signed in to change notification settings - Fork 217
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
Upgrade flow for images RHEL/CentOS 8+ #562
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -297,7 +297,7 @@ function wait_for_postgresql_master() { | |
run_pgupgrade () | ||
( | ||
# Remove .pid file if the file persists after ugly shut down | ||
if [ -f "$PGDATA/postmaster.pid" ] && ! pgrep -f "postgres" > /dev/null; then | ||
if [ -f "$PGDATA/postmaster.pid" ] && ! pg_isready > /dev/null; then | ||
rm -rf "$PGDATA/postmaster.pid" | ||
fi | ||
|
||
|
@@ -311,8 +311,17 @@ run_pgupgrade () | |
old_collection=rh-postgresql$old_raw_version | ||
fi | ||
|
||
old_pgengine=/opt/rh/$old_collection/root/usr/bin | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be changed only for RHEL8+ images. The general change would cause problems in rhel7/centos7 containers. Condition needed in distgen source files. |
||
new_pgengine=/opt/rh/rh-postgresql${new_raw_version}/root/usr/bin | ||
# Backward compatibility for RHEL/CentOS 7 | ||
source /etc/os-release | ||
|
||
if [[ $VERSION_ID -lt 8 ]]; then | ||
old_pgengine=/opt/rh/$old_collection/root/usr/bin | ||
new_pgengine=/opt/rh/rh-postgresql${new_raw_version}/root/usr/bin | ||
else | ||
old_pgengine=/usr/lib64/pgsql/postgresql-$old_raw_version/bin | ||
new_pgengine=/usr/bin | ||
fi | ||
|
||
PGDATA_new="${PGDATA}-new" | ||
|
||
printf >&2 "\n========== \$PGDATA upgrade: %s -> %s ==========\n\n" \ | ||
|
@@ -350,9 +359,9 @@ run_pgupgrade () | |
# boot up data directory with old postgres once again to make sure | ||
# it was shut down properly, otherwise the upgrade process fails | ||
info_msg "Starting old postgresql once again for a clean shutdown..." | ||
"${old_pgengine}/pg_ctl" start -w --timeout 86400 -o "-h ''" | ||
"${old_pgengine}/pg_ctl" start -w --timeout 86400 -o "-h 127.0.0.1''" | ||
info_msg "Waiting for postgresql to be ready for shutdown again..." | ||
"${old_pgengine}/pg_isready" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you removing this safety procedure? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On my testing it returned exit code 2 - I did manual checks the socket was up and running but the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after some searching, It seems we start the server as local only -- which mean it doesn't create a TCP port instead it creates a Linux socket. the 'pg_isready' tries the port and fails There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
"${old_pgengine}/pg_isready" -h 127.0.0.1 | ||
info_msg "Shutting down old postgresql cleanly..." | ||
"${old_pgengine}/pg_ctl" stop | ||
|
||
|
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.
Why are you removing the condition? We do not want to remove
.pid
file of the running processThere 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.
So I removed it because I had some issues when I was testing the upgrade. I will look into it deeper.
we should check if the PID is up first
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.
so this is from a running container I added a stop before the line
seem that the service is not running, also there isn't any open socket
I'm assuming the it is partially match
run-postgresql
how about
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.
changed
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.
makes sense +1