-
Notifications
You must be signed in to change notification settings - Fork 636
Look in more directories for upgrade binaries #4304
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
Conversation
cdbdad4 to
9ecc14d
Compare
| // - https://cwrap.org/nss_wrapper.html | ||
| `export LD_PRELOAD='libnss_wrapper.so' NSS_WRAPPER_GROUP NSS_WRAPPER_PASSWD`, | ||
|
|
||
| // Find directories that contain the desired Postgres executables. |
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.
Maybe describe what is going on here a little more.
My understanding is that the postgres.ShellPath stuff sets the path variable and the command -v postgres/initdb returns the full path to the binary
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've added some description.
That made me realize the PATH built into the image is also searched, so any ol' postgres might be found. I added an assertion* on the --version returned by something in old_bin and new_bin, and learned and doc'd that pg_upgrade does a similar check only recently-ish.
* same check as the postgres init container:
postgres-operator/internal/postgres/config.go
Lines 489 to 491 in e395e2a
| `results 'postgres version' "${postgres_version:=$(postgres --version ||:)}"`, | |
| `[[ "${postgres_version}" =~ ") ${expected_major_version}"($|[^0-9]) ]] ||`, | |
| `halt Expected PostgreSQL version "${expected_major_version}"`, |
|
With this change do we need the postgres bins on the |
This leaves the disk untouched when the upgrade image cannot support the requested upgrade. Issue: PGO-300 See: 406e069
9ecc14d to
4eae062
Compare
I find this Go code and resulting Bash easier to read. This also logs more about what is happening without changing the sequence of commands. The script included an unnecessary `|| exit`, so I moved the `set -eu` out of the Bash invocation into the script itself to make that behavior more obvious.
This makes major upgrades compatible with images from other distros. Issue: PGO-864
4eae062 to
be40084
Compare
|
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
Images for major upgrades must have binaries at Red Hat paths.
What is the new behavior (if this is a feature change)?
Major upgrades look for binaries in typical paths for multiple distro flavors: Alpine, Debian, Red Hat.
Other Information:
Issue: PGO-864