Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit ef0301e

Browse files
committed
Add a Bash function to create Postgres tablespace and data directories
The commands for these were intentionally similar, and now they are the same and documented together. Issue: PGO-2558
1 parent c91dd31 commit ef0301e

File tree

2 files changed

+66
-86
lines changed

2 files changed

+66
-86
lines changed

‎internal/postgres/config.go‎

Lines changed: 58 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,38 @@ import (
2121
)
2222

2323
const (
24+
// bashDataDirectory is a Bash function that ensures a directory has the correct permissions for PostgreSQL data.
25+
//
26+
// Postgres requires its data directories be writable by only itself.
27+
// Pod "securityContext.fsGroup" sets g+w on directories for *some* storage providers.
28+
// Ensure the current user owns the directory, and remove group-write permission.
29+
//
30+
// - https://www.postgresql.org/docs/current/creating-cluster.html
31+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/postmaster/postmaster.c;hb=REL_10_0#l1522
32+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/utils/init/miscinit.c;hb=REL_11_0#l142
33+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/utils/init/miscinit.c;hb=REL_17_0#l386
34+
// - https://issue.k8s.io/93802#issuecomment-717646167
35+
//
36+
// During CREATE TABLESPACE, Postgres sets the permissions of a tablespace directory to match the data directory.
37+
//
38+
// - https://www.postgresql.org/docs/current/manage-ag-tablespaces.html
39+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/commands/tablespace.c;hb=REL_14_0#l600
40+
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/common/file_perm.c;hb=REL_14_0#l27
41+
//
42+
bashDataDirectory = `dataDirectory() {` +
43+
// When the directory does not exist, create it with the correct permissions.
44+
// When the directory has the correct owner, set the correct permissions.
45+
` if [[ ! -e "1ドル" || -O "1ドル" ]]; then install --directory --mode=0750 "1ドル";` +
46+
//
47+
// The directory exists but its owner is wrong.
48+
// When it is writable, the set-group-ID bit indicates that "fsGroup" probably ran on its contents making them safe to use.
49+
// In this case, we can make a new directory (owned by this user) and refill it.
50+
` elif [[ -w "1ドル" && -g "1ドル" ]]; then recreate "1ドル" '0750';` +
51+
//
52+
// The directory exists, its owner is wrong, and it is not writable.
53+
// This is probably fatal, but indicate failure to let the caller decide.
54+
` else false; fi; }`
55+
2456
// bashHalt is a Bash function that prints its arguments to stderr then
2557
// exits with a non-zero status. It uses the exit status of the prior
2658
// command if that was not zero.
@@ -327,47 +359,31 @@ func startupCommand(
327359
version := fmt.Sprint(cluster.Spec.PostgresVersion)
328360
walDir := WALDirectory(cluster, instance)
329361

330-
// If the user requests tablespaces, we want to make sure the directories exist with the
331-
// correct owner and permissions.
332-
tablespaceCmd := ""
333-
if feature.Enabled(ctx, feature.TablespaceVolumes) {
334-
// This command checks if a dir exists and if not, creates it;
335-
// if the dir does exist, then we `recreate` it to make sure the owner is correct;
336-
// if the dir exists with the wrong owner and is not writeable, we error.
337-
// This is the same behavior we use for the main PGDATA directory.
338-
// Note: Postgres requires the tablespace directory to be "an existing, empty directory
339-
// that is owned by the PostgreSQL operating system user."
340-
// - https://www.postgresql.org/docs/current/manage-ag-tablespaces.html
341-
// However, unlike the PGDATA directory, Postgres will _not_ error out
342-
// if the permissions are wrong on the tablespace directory.
343-
// Instead, when a tablespace is created in Postgres, Postgres will `chmod` the
344-
// tablespace directory to match permissions on the PGDATA directory (either 700 or 750).
345-
// Postgres setting the tablespace directory permissions:
346-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/commands/tablespace.c;hb=REL_14_0#l600
347-
// Postgres choosing directory permissions:
348-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/common/file_perm.c;hb=REL_14_0#l27
349-
// Note: This permission change seems to happen only when the tablespace is created in Postgres.
350-
// If the user manually `chmod`'ed the directory after the creation of the tablespace, Postgres
351-
// would not attempt to change the directory permissions.
352-
// Note: as noted below, we mount the tablespace directory to the mountpoint `/tablespaces/NAME`,
353-
// and so we add the subdirectory `data` in order to set the permissions.
354-
checkInstallRecreateCmd := strings.Join([]string{
355-
`if [[ ! -e "${tablespace_dir}" || -O "${tablespace_dir}" ]]; then`,
356-
`install --directory --mode=0750 "${tablespace_dir}"`,
357-
`elif [[ -w "${tablespace_dir}" && -g "${tablespace_dir}" ]]; then`,
358-
`recreate "${tablespace_dir}" '0750'`,
359-
`else (halt Permissions!); fi ||`,
360-
`halt "$(permissions "${tablespace_dir}" ||:)"`,
361-
}, "\n")
362+
mkdirs := make([]string, 0, 7+len(instance.TablespaceVolumes))
363+
mkdirs = append(mkdirs, `dataDirectory "${postgres_data_directory}" || halt "$(permissions "${postgres_data_directory}" ||:)"`)
362364

365+
// If the user requests tablespaces, we want to make sure the directories exist with the correct owner and permissions.
366+
//
367+
// The path for tablespaces volumes is /tablespaces/NAME/data -- the `data` directory is so we can arrange the permissions.
368+
if feature.Enabled(ctx, feature.TablespaceVolumes) {
363369
for _, tablespace := range instance.TablespaceVolumes {
364-
// The path for tablespaces volumes is /tablespaces/NAME/data
365-
// -- the `data` path is added so that we can arrange the permissions.
366-
tablespaceCmd = tablespaceCmd + "\ntablespace_dir=/tablespaces/" + tablespace.Name + "/data" + "\n" +
367-
checkInstallRecreateCmd
370+
dir := shell.QuoteWord("/tablespaces/" + tablespace.Name + "/data")
371+
mkdirs = append(mkdirs, `dataDirectory `+dir+` || halt "$(permissions `+dir+` ||:)"`)
368372
}
369373
}
370374

375+
// These directories are outside "data_directory" and can be created.
376+
mkdirs = append(mkdirs,
377+
`(`+shell.MakeDirectories(dataMountPath, LogDirectory())+`) ||`,
378+
`halt "$(permissions `+shell.QuoteWord(LogDirectory())+` ||:)"`,
379+
380+
`(`+shell.MakeDirectories(dataMountPath, naming.PatroniPGDataLogPath)+`) ||`,
381+
`halt "$(permissions `+shell.QuoteWord(naming.PatroniPGDataLogPath)+` ||:)"`,
382+
383+
`(`+shell.MakeDirectories(dataMountPath, naming.PGBackRestPGDataLogPath)+`) ||`,
384+
`halt "$(permissions `+shell.QuoteWord(naming.PGBackRestPGDataLogPath)+` ||:)"`,
385+
)
386+
371387
pg_rewind_override := ""
372388
if config.FetchKeyCommand(&cluster.Spec) != "" {
373389
// Quoting "EOF" disables parameter substitution during write.
@@ -384,6 +400,9 @@ chmod +x /tmp/pg_rewind_tde.sh
384400
script := strings.Join([]string{
385401
`declare -r expected_major_version="1ドル" pgwal_directory="2ドル"`,
386402

403+
// Function to create a Postgres data directory.
404+
bashDataDirectory,
405+
387406
// Function to print the permissions of a file or directory and its parents.
388407
bashPermissions,
389408

@@ -425,42 +444,10 @@ chmod +x /tmp/pg_rewind_tde.sh
425444

426445
// Determine if the data directory has been prepared for bootstrapping the cluster
427446
`bootstrap_dir="${postgres_data_directory}_bootstrap"`,
428-
`[[ -d "${bootstrap_dir}" ]] && results 'bootstrap directory' "${bootstrap_dir}"`,
429-
`[[ -d "${bootstrap_dir}" ]] && postgres_data_directory="${bootstrap_dir}"`,
430-
431-
// PostgreSQL requires its directory to be writable by only itself.
432-
// Pod "securityContext.fsGroup" sets g+w on directories for *some*
433-
// storage providers. Ensure the current user owns the directory, and
434-
// remove group-write permission.
435-
// - https://www.postgresql.org/docs/current/creating-cluster.html
436-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/postmaster/postmaster.c;hb=REL_10_0#l1522
437-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/utils/init/miscinit.c;hb=REL_11_0#l142
438-
// - https://git.postgresql.org/gitweb/?p=postgresql.git;f=src/backend/utils/init/miscinit.c;hb=REL_17_0#l386
439-
// - https://issue.k8s.io/93802#issuecomment-717646167
440-
//
441-
// When the directory does not exist, create it with the correct permissions.
442-
// When the directory has the correct owner, set the correct permissions.
443-
`if [[ ! -e "${postgres_data_directory}" || -O "${postgres_data_directory}" ]]; then`,
444-
`install --directory --mode=0750 "${postgres_data_directory}"`,
445-
//
446-
// The directory exists but its owner is wrong. When it is writable,
447-
// the set-group-ID bit indicates that "fsGroup" probably ran on its
448-
// contents making them safe to use. In this case, we can make a new
449-
// directory (owned by this user) and refill it.
450-
`elif [[ -w "${postgres_data_directory}" && -g "${postgres_data_directory}" ]]; then`,
451-
`recreate "${postgres_data_directory}" '0750'`,
452-
//
453-
// The directory exists, its owner is wrong, and it is not writable.
454-
`else (halt Permissions!); fi ||`,
455-
`halt "$(permissions "${postgres_data_directory}" ||:)"`,
447+
`[[ -d "${bootstrap_dir}" ]] && postgres_data_directory="${bootstrap_dir}" && results 'bootstrap directory' "${bootstrap_dir}"`,
456448

457-
// Create log directories.
458-
`(` + shell.MakeDirectories(dataMountPath, naming.PGBackRestPGDataLogPath) + `) ||`,
459-
`halt "$(permissions ` + naming.PGBackRestPGDataLogPath + ` ||:)"`,
460-
`(` + shell.MakeDirectories(dataMountPath, naming.PatroniPGDataLogPath) + `) ||`,
461-
`halt "$(permissions ` + naming.PatroniPGDataLogPath + ` ||:)"`,
462-
`(` + shell.MakeDirectories(dataMountPath, LogDirectory()) + `) ||`,
463-
`halt "$(permissions ` + LogDirectory() + ` ||:)"`,
449+
// Create directories for and related to the data directory.
450+
strings.Join(mkdirs, "\n"),
464451

465452
// Copy replication client certificate files
466453
// from the /pgconf/tls/replication directory to the /tmp/replication directory in order
@@ -478,7 +465,6 @@ chmod +x /tmp/pg_rewind_tde.sh
478465
// Add the pg_rewind wrapper script, if TDE is enabled.
479466
pg_rewind_override,
480467

481-
tablespaceCmd,
482468
// When the data directory is empty, there's nothing more to do.
483469
`[[ -f "${postgres_data_directory}/PG_VERSION" ]] || exit 0`,
484470

‎internal/postgres/reconcile_test.go‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ initContainers:
239239
- --
240240
- |-
241241
declare -r expected_major_version="1ドル" pgwal_directory="2ドル"
242+
dataDirectory() { if [[ ! -e "1ドル" || -O "1ドル" ]]; then install --directory --mode=0750 "1ドル"; elif [[ -w "1ドル" && -g "1ドル" ]]; then recreate "1ドル" '0750'; else false; fi; }
242243
permissions() { while [[ -n "1ドル" ]]; do set "${1%/*}" "$@"; done; shift; stat -Lc '%A %4u %4g %n' "$@"; }
243244
halt() { local rc=$?; >&2 echo "$@"; exit "${rc/#0/1}"; }
244245
results() { printf '::postgres-operator: %s::%s\n' "$@"; }
@@ -267,23 +268,16 @@ initContainers:
267268
[[ "${postgres_data_directory}" == "${PGDATA}" ]] ||
268269
halt Expected matching config and data directories
269270
bootstrap_dir="${postgres_data_directory}_bootstrap"
270-
[[ -d "${bootstrap_dir}" ]] && results 'bootstrap directory' "${bootstrap_dir}"
271-
[[ -d "${bootstrap_dir}" ]] && postgres_data_directory="${bootstrap_dir}"
272-
if [[ ! -e "${postgres_data_directory}" || -O "${postgres_data_directory}" ]]; then
273-
install --directory --mode=0750 "${postgres_data_directory}"
274-
elif [[ -w "${postgres_data_directory}" && -g "${postgres_data_directory}" ]]; then
275-
recreate "${postgres_data_directory}" '0750'
276-
else (halt Permissions!); fi ||
277-
halt "$(permissions "${postgres_data_directory}" ||:)"
278-
(mkdir -p '/pgdata/pgbackrest/log' && { chmod 0775 '/pgdata/pgbackrest/log' '/pgdata/pgbackrest' || :; }) ||
279-
halt "$(permissions /pgdata/pgbackrest/log ||:)"
280-
(mkdir -p '/pgdata/patroni/log' && { chmod 0775 '/pgdata/patroni/log' '/pgdata/patroni' || :; }) ||
281-
halt "$(permissions /pgdata/patroni/log ||:)"
271+
[[ -d "${bootstrap_dir}" ]] && postgres_data_directory="${bootstrap_dir}" && results 'bootstrap directory' "${bootstrap_dir}"
272+
dataDirectory "${postgres_data_directory}" || halt "$(permissions "${postgres_data_directory}" ||:)"
282273
(mkdir -p '/pgdata/logs/postgres' && { chmod 0775 '/pgdata/logs/postgres' '/pgdata/logs' || :; }) ||
283-
halt "$(permissions /pgdata/logs/postgres ||:)"
274+
halt "$(permissions '/pgdata/logs/postgres' ||:)"
275+
(mkdir -p '/pgdata/patroni/log' && { chmod 0775 '/pgdata/patroni/log' '/pgdata/patroni' || :; }) ||
276+
halt "$(permissions '/pgdata/patroni/log' ||:)"
277+
(mkdir -p '/pgdata/pgbackrest/log' && { chmod 0775 '/pgdata/pgbackrest/log' '/pgdata/pgbackrest' || :; }) ||
278+
halt "$(permissions '/pgdata/pgbackrest/log' ||:)"
284279
install -D --mode=0600 -t "/tmp/replication" "/pgconf/tls/replication"/{tls.crt,tls.key,ca.crt}
285280
286-
287281
[[ -f "${postgres_data_directory}/PG_VERSION" ]] || exit 0
288282
results 'data version' "${postgres_data_version:=$(< "${postgres_data_directory}/PG_VERSION")}"
289283
[[ "${postgres_data_version}" == "${expected_major_version}" ]] ||

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /