Skip to content

Commit 2a94e25

Browse files
cbandycbandy
committed
Simplify upgrade scripts with more variables
1 parent b3d57df commit 2a94e25

File tree

2 files changed

+97
-101
lines changed

2 files changed

+97
-101
lines changed

‎internal/controller/pgupgrade/jobs.go‎

Lines changed: 64 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ import (
2121
"github.com/crunchydata/postgres-operator/internal/feature"
2222
"github.com/crunchydata/postgres-operator/internal/initialize"
2323
"github.com/crunchydata/postgres-operator/internal/naming"
24+
"github.com/crunchydata/postgres-operator/internal/shell"
2425
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2526
)
2627

27-
// Upgrade job
28-
2928
// pgUpgradeJob returns the ObjectMeta for the pg_upgrade Job utilized to
3029
// upgrade from one major PostgreSQL version to another
3130
func pgUpgradeJob(upgrade *v1beta1.PGUpgrade) metav1.ObjectMeta {
@@ -48,19 +47,19 @@ func upgradeCommand(spec *v1beta1.PGUpgradeSettings, fetchKeyCommand string) []s
4847
oldVersion := spec.FromPostgresVersion
4948
newVersion := spec.ToPostgresVersion
5049

51-
// if the fetch key command is set for TDE, provide the value during initialization
52-
initdb := `/usr/pgsql-"${new_version}"/bin/initdb -k -D /pgdata/pg"${new_version}"`
50+
var argEncryptionKeyCommand string
5351
if fetchKeyCommand != "" {
54-
initdb += ` --encryption-key-command "` + fetchKeyCommand + `"`
52+
argEncryptionKeyCommand = ` --encryption-key-command=` + shell.QuoteWord(fetchKeyCommand)
5553
}
5654

5755
args := []string{fmt.Sprint(oldVersion), fmt.Sprint(newVersion)}
5856
script := strings.Join([]string{
5957
`declare -r data_volume='/pgdata' old_version="$1" new_version="$2"`,
60-
`printf 'Performing PostgreSQL upgrade from version "%s" to "%s" ...\n\n' "$@"`,
58+
`printf 'Performing PostgreSQL upgrade from version "%s" to "%s" ...\n' "$@"`,
59+
`section() { printf '\n\n%s\n' "$@"; }`,
6160

62-
// Note: Rather than import the nss_wrapper init container, as we do in
63-
// the main postgres-operator, this job does the required nss_wrapper
61+
// NOTE: Rather than import the nss_wrapper init container, as we do in
62+
// the PostgresCluster controller, this job does the required nss_wrapper
6463
// settings here.
6564

6665
// Create a copy of the system group definitions, but remove the "postgres"
@@ -81,49 +80,49 @@ func upgradeCommand(spec *v1beta1.PGUpgradeSettings, fetchKeyCommand string) []s
8180
// - https://cwrap.org/nss_wrapper.html
8281
`export LD_PRELOAD='libnss_wrapper.so' NSS_WRAPPER_GROUP NSS_WRAPPER_PASSWD`,
8382

83+
// Expect Postgres executables at the Red Hat paths.
84+
`old_data="${data_volume}/pg${old_version}"`,
85+
`new_data="${data_volume}/pg${new_version}"`,
86+
`old_bin="/usr/pgsql-${old_version}/bin"`,
87+
`new_bin="/usr/pgsql-${new_version}/bin"`,
88+
8489
// Below is the pg_upgrade script used to upgrade a PostgresCluster from
8590
// one major version to another. Additional information concerning the
8691
// steps used and command flag specifics can be found in the documentation:
8792
// - https://www.postgresql.org/docs/current/pgupgrade.html
8893

89-
// To begin, we first move to the mounted /pgdata directory and create a
90-
// new version directory which is then initialized with the initdb command.
91-
`cd /pgdata || exit`,
92-
`echo -e "Step 1: Making new pgdata directory...\n"`,
93-
`mkdir /pgdata/pg"${new_version}"`,
94-
`echo -e "Step 2: Initializing new pgdata directory...\n"`,
95-
initdb,
96-
97-
// Before running the upgrade check, which ensures the clusters are compatible,
98-
// proper permissions have to be set on the old pgdata directory and the
99-
// preload library settings must be copied over.
100-
`echo -e "\nStep 3: Setting the expected permissions on the old pgdata directory...\n"`,
101-
`chmod 750 /pgdata/pg"${old_version}"`,
102-
`echo -e "Step 4: Copying shared_preload_libraries setting to new postgresql.conf file...\n"`,
103-
`echo "shared_preload_libraries = '$(/usr/pgsql-"""${old_version}"""/bin/postgres -D \`,
104-
`/pgdata/pg"""${old_version}""" -C shared_preload_libraries)'" >> /pgdata/pg"${new_version}"/postgresql.conf`,
105-
106-
// Before the actual upgrade is run, we will run the upgrade --check to
107-
// verify everything before actually changing any data.
108-
`echo -e "Step 5: Running pg_upgrade check...\n"`,
109-
`time /usr/pgsql-"${new_version}"/bin/pg_upgrade --old-bindir /usr/pgsql-"${old_version}"/bin \`,
110-
`--new-bindir /usr/pgsql-"${new_version}"/bin --old-datadir /pgdata/pg"${old_version}"\`,
111-
` --new-datadir /pgdata/pg"${new_version}" --check` + argMethod + argJobs,
112-
113-
// Assuming the check completes successfully, the pg_upgrade command will
114-
// be run that actually prepares the upgraded pgdata directory.
115-
`echo -e "\nStep 6: Running pg_upgrade...\n"`,
116-
`time /usr/pgsql-"${new_version}"/bin/pg_upgrade --old-bindir /usr/pgsql-"${old_version}"/bin \`,
117-
`--new-bindir /usr/pgsql-"${new_version}"/bin --old-datadir /pgdata/pg"${old_version}" \`,
118-
`--new-datadir /pgdata/pg"${new_version}"` + argMethod + argJobs,
119-
120-
// Since we have cleared the Patroni cluster step by removing the EndPoints, we copy patroni.dynamic.json
121-
// from the old data dir to help retain PostgreSQL parameters you had set before.
122-
// - https://patroni.readthedocs.io/en/latest/existing_data.html#major-upgrade-of-postgresql-version
123-
`echo -e "\nStep 7: Copying patroni.dynamic.json...\n"`,
124-
`cp /pgdata/pg"${old_version}"/patroni.dynamic.json /pgdata/pg"${new_version}"`,
125-
126-
`echo -e "\npg_upgrade Job Complete!"`,
94+
// pg_upgrade writes its files in "${new_data}/pg_upgrade_output.d" since PostgreSQL v15.
95+
// Change to a writable working directory to be compatible with PostgreSQL v14 and earlier.
96+
//
97+
// https://www.postgresql.org/docs/release/15#id-1.11.6.20.5.11.3
98+
`cd "${data_volume}" || exit`,
99+
100+
`section 'Initializing new data directory...'`,
101+
`PGDATA="${new_data}" "${new_bin}/initdb" --data-checksums` + argEncryptionKeyCommand,
102+
103+
// Read the configured value then quote it; every single-quote U+0027 is replaced by two.
104+
//
105+
// https://www.postgresql.org/docs/current/config-setting.html
106+
// https://www.gnu.org/software/bash/manual/bash.html#ANSI_002dC-Quoting
107+
`section 'Copying shared_preload_libraries parameter...'`,
108+
`value=$(LC_ALL=C PGDATA="${old_data}" "${old_bin}/postgres" -C shared_preload_libraries)`,
109+
`echo >> "${new_data}/postgresql.conf" "shared_preload_libraries = '${value//$'\''/$'\'\''}'"`,
110+
111+
`section 'Checking for potential issues...'`,
112+
`"${new_bin}/pg_upgrade" --check` + argMethod + argJobs + ` \`,
113+
`--old-bindir="${old_bin}" --old-datadir="${old_data}" \`,
114+
`--new-bindir="${new_bin}" --new-datadir="${new_data}"`,
115+
116+
`section 'Performing upgrade...'`,
117+
`(set -x && time "${new_bin}/pg_upgrade"` + argMethod + argJobs + ` \`,
118+
`--old-bindir="${old_bin}" --old-datadir="${old_data}" \`,
119+
`--new-bindir="${new_bin}" --new-datadir="${new_data}")`,
120+
121+
// https://patroni.readthedocs.io/en/latest/existing_data.html#major-upgrade-of-postgresql-version
122+
`section 'Seeding Patroni settings...'`,
123+
`(set -x && cp "${old_data}/patroni.dynamic.json" "${new_data}")`,
124+
125+
`section 'Success!'`,
127126
}, "\n")
128127

129128
return append([]string{"bash", "-ceu", "--", script, "upgrade"}, args...)
@@ -232,35 +231,30 @@ func (r *PGUpgradeReconciler) generateUpgradeJob(
232231
// We currently target the `pgdata/pg{old_version}` and `pgdata/pg{old_version}_wal`
233232
// directories for removal.
234233
func removeDataCommand(upgrade *v1beta1.PGUpgrade) []string {
235-
oldVersion := fmt.Sprint(upgrade.Spec.FromPostgresVersion)
234+
oldVersion := upgrade.Spec.FromPostgresVersion
236235

237236
// Before removing the directories (both data and wal), we check that
238237
// the directory is not in use by running `pg_controldata` and making sure
239238
// the server state is "shut down in recovery"
240-
// TODO(benjaminjb): pg_controldata seems pretty stable, but might want to
241-
// experiment with a few more versions.
242-
args := []string{oldVersion}
239+
args := []string{fmt.Sprint(oldVersion)}
243240
script := strings.Join([]string{
244-
`declare -r old_version="$1"`,
245-
`printf 'Removing PostgreSQL data dir for pg%s...\n\n' "$@"`,
246-
`echo -e "Checking the directory exists and isn't being used...\n"`,
247-
`cd /pgdata || exit`,
248-
// The string `shut down in recovery` is the dbstate that postgres sets from
249-
// at least version 10 to 14 when a replica has been shut down.
250-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/pg_controldata/pg_controldata.c;h=f911f98d946d83f1191abf35239d9b4455c5f52a;hb=HEAD#l59
251-
// Note: `pg_controldata` is actually used by `pg_upgrade` before upgrading
252-
// to make sure that the server in question is shut down as a primary;
253-
// that aligns with our use here, where we're making sure that the server in question
254-
// was shut down as a replica.
255-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/pg_upgrade/controldata.c;h=41b8f69b8cbe4f40e6098ad84c2e8e987e24edaf;hb=HEAD#l122
256-
`if [ "$(/usr/pgsql-"${old_version}"/bin/pg_controldata /pgdata/pg"${old_version}" | grep -c "shut down in recovery")" -ne 1 ]; then echo -e "Directory in use, cannot remove..."; exit 1; fi`,
257-
`echo -e "Removing old pgdata directory...\n"`,
258-
// When deleting the wal directory, use `realpath` to resolve the symlink from
259-
// the pgdata directory. This is necessary because the wal directory can be
260-
// mounted at different places depending on if an external wal PVC is used,
261-
// i.e. `/pgdata/pg14_wal` vs `/pgwal/pg14_wal`
262-
`rm -rf /pgdata/pg"${old_version}" "$(realpath /pgdata/pg${old_version}/pg_wal)"`,
263-
`echo -e "Remove Data Job Complete!"`,
241+
`declare -r data_volume='/pgdata' old_version="$1"`,
242+
`printf 'Removing PostgreSQL %s data...\n\n' "$@"`,
243+
`delete() (set -x && rm -rf -- "$@")`,
244+
245+
`old_data="${data_volume}/pg${old_version}"`,
246+
`control=$(LC_ALL=C /usr/pgsql-${old_version}/bin/pg_controldata "${old_data}")`,
247+
`read -r state <<< "${control##*cluster state:}"`,
248+
249+
// We expect exactly one state for a replica that has been stopped.
250+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;hb=refs/tags/REL_10_0;f=src/bin/pg_controldata/pg_controldata.c#l55
251+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;hb=refs/tags/REL_17_0;f=src/bin/pg_controldata/pg_controldata.c#l58
252+
`[[ "${state}" == 'shut down in recovery' ]] || { printf >&2 'Unexpected state! %q\n' "${state}"; exit 1; }`,
253+
254+
// "rm" does not follow symbolic links.
255+
// Delete the old data directory after subdirectories that contain versioned data.
256+
`delete "${old_data}/pg_wal/"`,
257+
`delete "${old_data}" && echo 'Success!'`,
264258
}, "\n")
265259

266260
return append([]string{"bash", "-ceu", "--", script, "remove"}, args...)

‎internal/controller/pgupgrade/jobs_test.go‎

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -200,35 +200,36 @@ spec:
200200
- --
201201
- |-
202202
declare -r data_volume='/pgdata' old_version="$1" new_version="$2"
203-
printf 'Performing PostgreSQL upgrade from version "%s" to "%s" ...\n\n' "$@"
203+
printf 'Performing PostgreSQL upgrade from version "%s" to "%s" ...\n' "$@"
204+
section() { printf '\n\n%s\n' "$@"; }
204205
gid=$(id -G); NSS_WRAPPER_GROUP=$(mktemp)
205206
(sed "/^postgres:x:/ d; /^[^:]*:x:${gid%% *}:/ d" /etc/group
206207
echo "postgres:x:${gid%% *}:") > "${NSS_WRAPPER_GROUP}"
207208
uid=$(id -u); NSS_WRAPPER_PASSWD=$(mktemp)
208209
(sed "/^postgres:x:/ d; /^[^:]*:x:${uid}:/ d" /etc/passwd
209210
echo "postgres:x:${uid}:${gid%% *}::${data_volume}:") > "${NSS_WRAPPER_PASSWD}"
210211
export LD_PRELOAD='libnss_wrapper.so' NSS_WRAPPER_GROUP NSS_WRAPPER_PASSWD
211-
cd /pgdata || exit
212-
echo -e "Step 1: Making new pgdata directory...\n"
213-
mkdir /pgdata/pg"${new_version}"
214-
echo -e "Step 2: Initializing new pgdata directory...\n"
215-
/usr/pgsql-"${new_version}"/bin/initdb -k -D /pgdata/pg"${new_version}"
216-
echo -e "\nStep 3: Setting the expected permissions on the old pgdata directory...\n"
217-
chmod 750 /pgdata/pg"${old_version}"
218-
echo -e "Step 4: Copying shared_preload_libraries setting to new postgresql.conf file...\n"
219-
echo "shared_preload_libraries = '$(/usr/pgsql-"""${old_version}"""/bin/postgres -D \
220-
/pgdata/pg"""${old_version}""" -C shared_preload_libraries)'" >> /pgdata/pg"${new_version}"/postgresql.conf
221-
echo -e "Step 5: Running pg_upgrade check...\n"
222-
time /usr/pgsql-"${new_version}"/bin/pg_upgrade --old-bindir /usr/pgsql-"${old_version}"/bin \
223-
--new-bindir /usr/pgsql-"${new_version}"/bin --old-datadir /pgdata/pg"${old_version}"\
224-
--new-datadir /pgdata/pg"${new_version}" --check --link --jobs=1
225-
echo -e "\nStep 6: Running pg_upgrade...\n"
226-
time /usr/pgsql-"${new_version}"/bin/pg_upgrade --old-bindir /usr/pgsql-"${old_version}"/bin \
227-
--new-bindir /usr/pgsql-"${new_version}"/bin --old-datadir /pgdata/pg"${old_version}" \
228-
--new-datadir /pgdata/pg"${new_version}" --link --jobs=1
229-
echo -e "\nStep 7: Copying patroni.dynamic.json...\n"
230-
cp /pgdata/pg"${old_version}"/patroni.dynamic.json /pgdata/pg"${new_version}"
231-
echo -e "\npg_upgrade Job Complete!"
212+
old_data="${data_volume}/pg${old_version}"
213+
new_data="${data_volume}/pg${new_version}"
214+
old_bin="/usr/pgsql-${old_version}/bin"
215+
new_bin="/usr/pgsql-${new_version}/bin"
216+
cd "${data_volume}" || exit
217+
section 'Initializing new data directory...'
218+
PGDATA="${new_data}" "${new_bin}/initdb" --data-checksums
219+
section 'Copying shared_preload_libraries parameter...'
220+
value=$(LC_ALL=C PGDATA="${old_data}" "${old_bin}/postgres" -C shared_preload_libraries)
221+
echo >> "${new_data}/postgresql.conf" "shared_preload_libraries = '${value//$'\''/$'\'\''}'"
222+
section 'Checking for potential issues...'
223+
"${new_bin}/pg_upgrade" --check --link --jobs=1 \
224+
--old-bindir="${old_bin}" --old-datadir="${old_data}" \
225+
--new-bindir="${new_bin}" --new-datadir="${new_data}"
226+
section 'Performing upgrade...'
227+
(set -x && time "${new_bin}/pg_upgrade" --link --jobs=1 \
228+
--old-bindir="${old_bin}" --old-datadir="${old_data}" \
229+
--new-bindir="${new_bin}" --new-datadir="${new_data}")
230+
section 'Seeding Patroni settings...'
231+
(set -x && cp "${old_data}/patroni.dynamic.json" "${new_data}")
232+
section 'Success!'
232233
- upgrade
233234
- "19"
234235
- "25"
@@ -263,7 +264,7 @@ status: {}
263264

264265
tdeJob := reconciler.generateUpgradeJob(ctx, upgrade, startup, "echo testKey")
265266
assert.Assert(t, cmp.MarshalContains(tdeJob,
266-
`/usr/pgsql-"${new_version}"/bin/initdb -k -D /pgdata/pg"${new_version}" --encryption-key-command "echo testKey"`))
267+
`PGDATA="${new_data}" "${new_bin}/initdb" --data-checksums --encryption-key-command='echo testKey'`))
267268
}
268269

269270
func TestGenerateRemoveDataJob(t *testing.T) {
@@ -342,14 +343,15 @@ spec:
342343
- -ceu
343344
- --
344345
- |-
345-
declare -r old_version="$1"
346-
printf 'Removing PostgreSQL data dir for pg%s...\n\n' "$@"
347-
echo -e "Checking the directory exists and isn't being used...\n"
348-
cd /pgdata || exit
349-
if [ "$(/usr/pgsql-"${old_version}"/bin/pg_controldata /pgdata/pg"${old_version}" | grep -c "shut down in recovery")" -ne 1 ]; then echo -e "Directory in use, cannot remove..."; exit 1; fi
350-
echo -e "Removing old pgdata directory...\n"
351-
rm -rf /pgdata/pg"${old_version}" "$(realpath /pgdata/pg${old_version}/pg_wal)"
352-
echo -e "Remove Data Job Complete!"
346+
declare -r data_volume='/pgdata' old_version="$1"
347+
printf 'Removing PostgreSQL %s data...\n\n' "$@"
348+
delete() (set -x && rm -rf -- "$@")
349+
old_data="${data_volume}/pg${old_version}"
350+
control=$(LC_ALL=C /usr/pgsql-${old_version}/bin/pg_controldata "${old_data}")
351+
read -r state <<< "${control##*cluster state:}"
352+
[[ "${state}" == 'shut down in recovery' ]] || { printf >&2 'Unexpected state! %q\n' "${state}"; exit 1; }
353+
delete "${old_data}/pg_wal/"
354+
delete "${old_data}" && echo 'Success!'
353355
- remove
354356
- "19"
355357
image: img4

0 commit comments

Comments
 (0)