Skip to content

Commit 951fa40

Browse files
committed
Ensure Postgres and Patroni log directories are writable
The `install` command only sets permissions on the final directory.
1 parent e6ea78b commit 951fa40

File tree

6 files changed

+22
-25
lines changed

6 files changed

+22
-25
lines changed

internal/collector/patroni.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ func EnablePatroniLogging(ctx context.Context,
2121

2222
// Keep track of what log records and files have been processed.
2323
// Use a subdirectory of the logs directory to stay within the same failure domain.
24+
// TODO(log-rotation): Create this directory during Collector startup.
2425
//
2526
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/extension/storage/filestorage#readme
2627
outConfig.Extensions["file_storage/patroni_logs"] = map[string]any{

internal/collector/pgbackrest.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func NewConfigForPgBackrestRepoHostPod(
4747

4848
// Keep track of what log records and files have been processed.
4949
// Use a subdirectory of the logs directory to stay within the same failure domain.
50+
// TODO(log-rotation): Create this directory during Collector startup.
5051
config.Extensions["file_storage/pgbackrest_logs"] = map[string]any{
5152
"directory": directory + "/receiver",
5253
"create_directory": true,

internal/collector/pgbouncer.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ func EnablePgBouncerLogging(ctx context.Context,
5050

5151
// Keep track of what log records and files have been processed.
5252
// Use a subdirectory of the logs directory to stay within the same failure domain.
53+
// TODO(log-rotation): Create this directory during Collector startup.
5354
//
5455
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/extension/storage/filestorage#readme
5556
outConfig.Extensions["file_storage/pgbouncer_logs"] = map[string]any{

internal/collector/postgres.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ func EnablePostgresLogging(
110110

111111
// Keep track of what log records and files have been processed.
112112
// Use a subdirectory of the logs directory to stay within the same failure domain.
113+
// TODO(log-rotation): Create this directory during Collector startup.
113114
//
114115
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/extension/storage/filestorage#readme
115116
outConfig.Extensions["file_storage/postgres_logs"] = map[string]any{
@@ -215,6 +216,7 @@ func EnablePostgresLogging(
215216
}
216217

217218
// pgBackRest pipeline
219+
// TODO(log-rotation): Create this directory during Collector startup.
218220
outConfig.Extensions["file_storage/pgbackrest_logs"] = map[string]any{
219221
"directory": naming.PGBackRestPGDataLogPath + "/receiver",
220222
"create_directory": true,

internal/postgres/config.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/crunchydata/postgres-operator/internal/config"
1515
"github.com/crunchydata/postgres-operator/internal/feature"
1616
"github.com/crunchydata/postgres-operator/internal/naming"
17+
"github.com/crunchydata/postgres-operator/internal/shell"
1718
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1819
)
1920

@@ -297,9 +298,9 @@ chmod +x /tmp/pg_rewind_tde.sh
297298
`
298299
}
299300

300-
args := []string{version, walDir, naming.PGBackRestPGDataLogPath, naming.PatroniPGDataLogPath}
301+
args := []string{version, walDir}
301302
script := strings.Join([]string{
302-
`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3" patroniLog_directory="$4"`,
303+
`declare -r expected_major_version="$1" pgwal_directory="$2"`,
303304

304305
// Function to print the permissions of a file or directory and its parents.
305306
bashPermissions,
@@ -370,17 +371,12 @@ chmod +x /tmp/pg_rewind_tde.sh
370371
`else (halt Permissions!); fi ||`,
371372
`halt "$(permissions "${postgres_data_directory}" ||:)"`,
372373

373-
// Create the pgBackRest log directory.
374-
`results 'pgBackRest log directory' "${pgbrLog_directory}"`,
375-
`install --directory --mode=0775 "${pgbrLog_directory}" ||`,
376-
`halt "$(permissions "${pgbrLog_directory}" ||:)"`,
377-
378-
// Create the Patroni log directory.
379-
`results 'Patroni log directory' "${patroniLog_directory}"`,
380-
`install --directory --mode=0775 "${patroniLog_directory}" ||`,
381-
`halt "$(permissions "${patroniLog_directory}" ||:)"`,
382-
383-
`install --directory --mode=0775 ` + LogDirectory() + ` ||`,
374+
// Create log directories.
375+
`(` + shell.MakeDirectories(0o775, dataMountPath, naming.PGBackRestPGDataLogPath) + `) ||`,
376+
`halt "$(permissions ` + naming.PGBackRestPGDataLogPath + ` ||:)"`,
377+
`(` + shell.MakeDirectories(0o775, dataMountPath, naming.PatroniPGDataLogPath) + `) ||`,
378+
`halt "$(permissions ` + naming.PatroniPGDataLogPath + ` ||:)"`,
379+
`(` + shell.MakeDirectories(0o775, dataMountPath, LogDirectory()) + `) ||`,
384380
`halt "$(permissions ` + LogDirectory() + ` ||:)"`,
385381

386382
// Copy replication client certificate files

internal/postgres/reconcile_test.go

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ initContainers:
230230
- -ceu
231231
- --
232232
- |-
233-
declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3" patroniLog_directory="$4"
233+
declare -r expected_major_version="$1" pgwal_directory="$2"
234234
permissions() { while [[ -n "$1" ]]; do set "${1%/*}" "$@"; done; shift; stat -Lc '%A %4u %4g %n' "$@"; }
235235
halt() { local rc=$?; >&2 echo "$@"; exit "${rc/#0/1}"; }
236236
results() { printf '::postgres-operator: %s::%s\n' "$@"; }
@@ -267,13 +267,11 @@ initContainers:
267267
recreate "${postgres_data_directory}" '0700'
268268
else (halt Permissions!); fi ||
269269
halt "$(permissions "${postgres_data_directory}" ||:)"
270-
results 'pgBackRest log directory' "${pgbrLog_directory}"
271-
install --directory --mode=0775 "${pgbrLog_directory}" ||
272-
halt "$(permissions "${pgbrLog_directory}" ||:)"
273-
results 'Patroni log directory' "${patroniLog_directory}"
274-
install --directory --mode=0775 "${patroniLog_directory}" ||
275-
halt "$(permissions "${patroniLog_directory}" ||:)"
276-
install --directory --mode=0775 /pgdata/logs/postgres ||
270+
(mkdir -p '/pgdata/pgbackrest/log' && chmod 0775 '/pgdata/pgbackrest/log' '/pgdata/pgbackrest') ||
271+
halt "$(permissions /pgdata/pgbackrest/log ||:)"
272+
(mkdir -p '/pgdata/patroni/log' && chmod 0775 '/pgdata/patroni/log' '/pgdata/patroni') ||
273+
halt "$(permissions /pgdata/patroni/log ||:)"
274+
(mkdir -p '/pgdata/logs/postgres' && chmod 0775 '/pgdata/logs/postgres' '/pgdata/logs') ||
277275
halt "$(permissions /pgdata/logs/postgres ||:)"
278276
install -D --mode=0600 -t "/tmp/replication" "/pgconf/tls/replication"/{tls.crt,tls.key,ca.crt}
279277
@@ -290,8 +288,6 @@ initContainers:
290288
- startup
291289
- "11"
292290
- /pgdata/pg11_wal
293-
- /pgdata/pgbackrest/log
294-
- /pgdata/patroni/log
295291
env:
296292
- name: PGDATA
297293
value: /pgdata/pg11
@@ -479,7 +475,7 @@ volumes:
479475

480476
// Startup moves WAL files to data volume.
481477
assert.DeepEqual(t, pod.InitContainers[0].Command[4:],
482-
[]string{"startup", "11", "/pgdata/pg11_wal", "/pgdata/pgbackrest/log", "/pgdata/patroni/log"})
478+
[]string{"startup", "11", "/pgdata/pg11_wal"})
483479
})
484480

485481
t.Run("WithAdditionalConfigFiles", func(t *testing.T) {
@@ -709,7 +705,7 @@ volumes:
709705

710706
// Startup moves WAL files to WAL volume.
711707
assert.DeepEqual(t, pod.InitContainers[0].Command[4:],
712-
[]string{"startup", "11", "/pgwal/pg11_wal", "/pgdata/pgbackrest/log", "/pgdata/patroni/log"})
708+
[]string{"startup", "11", "/pgwal/pg11_wal"})
713709
})
714710
}
715711

0 commit comments

Comments
 (0)