Skip to content

Commit 69d7ce8

Browse files
committed
Allow pgbackrest in instance pod to log to an additional volumes.
Add maxlength to log path and optional tag to volumes.
1 parent 1312df1 commit 69d7ce8

File tree

11 files changed

+101
-11
lines changed

11 files changed

+101
-11
lines changed

config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,7 @@ spec:
14351435
running in Backup Job Pods.
14361436
properties:
14371437
path:
1438+
maxLength: 4096
14381439
type: string
14391440
type: object
14401441
priorityClassName:
@@ -1609,6 +1610,7 @@ spec:
16091610
running in postgres instance pods.
16101611
properties:
16111612
path:
1613+
maxLength: 4096
16121614
type: string
16131615
type: object
16141616
manual:
@@ -2584,6 +2586,7 @@ spec:
25842586
running in the repo host pod.
25852587
properties:
25862588
path:
2589+
maxLength: 4096
25872590
type: string
25882591
type: object
25892592
priorityClassName:
@@ -4610,6 +4613,8 @@ spec:
46104613
- repos
46114614
type: object
46124615
x-kubernetes-validations:
4616+
- message: log path is restricted to an existing additional volume
4617+
rule: '!has(self.log) || !has(self.log.path) || self.log.path.startsWith("/volumes/")'
46134618
- message: log path is restricted to an existing additional volume
46144619
rule: '!has(self.repoHost) || !has(self.repoHost.log) || !has(self.repoHost.log.path)
46154620
|| self.repoHost.volumes.additional.exists(x, self.repoHost.log.path.startsWith("/volumes/"+x.name))'
@@ -11309,6 +11314,7 @@ spec:
1130911314
type: object
1131011315
type: array
1131111316
volumes:
11317+
description: Volumes to be added to the instance set.
1131211318
properties:
1131311319
additional:
1131411320
description: Additional pre-existing volumes to add to the
@@ -20396,6 +20402,7 @@ spec:
2039620402
running in Backup Job Pods.
2039720403
properties:
2039820404
path:
20405+
maxLength: 4096
2039920406
type: string
2040020407
type: object
2040120408
priorityClassName:
@@ -20570,6 +20577,7 @@ spec:
2057020577
running in postgres instance pods.
2057120578
properties:
2057220579
path:
20580+
maxLength: 4096
2057320581
type: string
2057420582
type: object
2057520583
manual:
@@ -21545,6 +21553,7 @@ spec:
2154521553
running in the repo host pod.
2154621554
properties:
2154721555
path:
21556+
maxLength: 4096
2154821557
type: string
2154921558
type: object
2155021559
priorityClassName:
@@ -30263,6 +30272,7 @@ spec:
3026330272
type: object
3026430273
type: array
3026530274
volumes:
30275+
description: Volumes to be added to the instance set.
3026630276
properties:
3026730277
additional:
3026830278
description: Additional pre-existing volumes to add to the

internal/collector/postgres.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/crunchydata/postgres-operator/internal/naming"
1818
"github.com/crunchydata/postgres-operator/internal/postgres"
19+
"github.com/crunchydata/postgres-operator/internal/util"
1920
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2021
)
2122

@@ -247,8 +248,9 @@ func EnablePostgresLogging(
247248
}
248249

249250
// pgBackRest pipeline
251+
pgbackrestLogPath := util.GetPGBackRestLogPathForInstance(inCluster)
250252
outConfig.Extensions["file_storage/pgbackrest_logs"] = map[string]any{
251-
"directory": naming.PGBackRestPGDataLogPath + "/receiver",
253+
"directory": pgbackrestLogPath + "/receiver",
252254
"create_directory": false,
253255
"fsync": true,
254256
}
@@ -261,8 +263,8 @@ func EnablePostgresLogging(
261263
// a log record or two to the old file while rotation is occurring.
262264
// The collector knows not to create duplicate logs.
263265
"include": []string{
264-
naming.PGBackRestPGDataLogPath + "/*.log",
265-
naming.PGBackRestPGDataLogPath + "/*.log.1",
266+
pgbackrestLogPath + "/*.log",
267+
pgbackrestLogPath + "/*.log.1",
266268
},
267269
"storage": "file_storage/pgbackrest_logs",
268270

internal/controller/postgrescluster/instance.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,7 +1184,7 @@ func (r *Reconciler) reconcileInstance(
11841184
includeLogrotate := backupsSpecFound && pgbackrest.RepoHostVolumeDefined(cluster)
11851185
collector.AddToPod(ctx, cluster.Spec.Instrumentation, cluster.Spec.ImagePullPolicy, instanceConfigMap, &instance.Spec.Template,
11861186
[]corev1.VolumeMount{postgres.DataVolumeMount()}, pgPassword,
1187-
[]string{naming.PGBackRestPGDataLogPath}, includeLogrotate, true)
1187+
[]string{util.GetPGBackRestLogPathForInstance(cluster)}, includeLogrotate, true)
11881188
}
11891189

11901190
// Add postgres-exporter to the instance Pod spec
@@ -1412,7 +1412,7 @@ func (r *Reconciler) reconcileInstanceConfigMap(
14121412
collector.AddLogrotateConfigs(ctx, cluster.Spec.Instrumentation,
14131413
instanceConfigMap,
14141414
[]collector.LogrotateConfig{{
1415-
LogFiles: []string{naming.PGBackRestPGDataLogPath + "/*.log"},
1415+
LogFiles: []string{util.GetPGBackRestLogPathForInstance(cluster) + "/*.log"},
14161416
}})
14171417
}
14181418
}

internal/pgbackrest/config.go

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

@@ -70,8 +71,8 @@ const (
7071

7172
// CreatePGBackRestConfigMapIntent creates a configmap struct with pgBackRest pgbackrest.conf settings in the data field.
7273
// The keys within the data field correspond to the use of that configuration.
73-
// pgbackrest_job.conf is used by certain jobs, such as stanza create and backup
74-
// pgbackrest_primary.conf is used by the primary database pod
74+
// pgbackrest-server.conf is used by the pgBackRest TLS server
75+
// pgbackrest_instance.conf is used by the primary database pod
7576
// pgbackrest_repo.conf is used by the pgBackRest repository pod
7677
// pgbackrest_cloud.conf is used by cloud repo backup jobs
7778
func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1beta1.PostgresCluster,
@@ -111,6 +112,7 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet
111112
strconv.Itoa(postgresCluster.Spec.PostgresVersion),
112113
pgPort, postgresCluster.Spec.Backups.PGBackRest.Repos,
113114
postgresCluster.Spec.Backups.PGBackRest.Global,
115+
util.GetPGBackRestLogPathForInstance(postgresCluster),
114116
).String()
115117

116118
// As the cluster transitions from having a repository host to having none,
@@ -370,7 +372,7 @@ func populatePGInstanceConfigurationMap(
370372
serviceName, serviceNamespace, repoHostName, pgdataDir,
371373
fetchKeyCommand, postgresVersion string,
372374
pgPort int32, repos []v1beta1.PGBackRestRepo,
373-
globalConfig map[string]string,
375+
globalConfig map[string]string, pgbackrestLogPath string,
374376
) iniSectionSet {
375377

376378
// TODO(cbandy): pass a FQDN in already.
@@ -386,7 +388,7 @@ func populatePGInstanceConfigurationMap(
386388
// pgBackRest spool-path should always be co-located with the Postgres WAL path.
387389
global.Set("spool-path", "/pgdata/pgbackrest-spool")
388390
// pgBackRest will log to the pgData volume for commands run on the PostgreSQL instance
389-
global.Set("log-path", naming.PGBackRestPGDataLogPath)
391+
global.Set("log-path", pgbackrestLogPath)
390392

391393
for _, repo := range repos {
392394
global.Set(repo.Name+"-path", defaultRepo1Path+repo.Name)

internal/postgres/config.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/crunchydata/postgres-operator/internal/feature"
1919
"github.com/crunchydata/postgres-operator/internal/naming"
2020
"github.com/crunchydata/postgres-operator/internal/shell"
21+
"github.com/crunchydata/postgres-operator/internal/util"
2122
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2223
)
2324

@@ -406,8 +407,8 @@ func startupCommand(
406407
`(`+shell.MakeDirectories(dataMountPath, naming.PatroniPGDataLogPath)+`) ||`,
407408
`halt "$(permissions `+shell.QuoteWord(naming.PatroniPGDataLogPath)+` ||:)"`,
408409

409-
`(`+shell.MakeDirectories(dataMountPath, naming.PGBackRestPGDataLogPath)+`) ||`,
410-
`halt "$(permissions `+shell.QuoteWord(naming.PGBackRestPGDataLogPath)+` ||:)"`,
410+
`(`+shell.MakeDirectories(dataMountPath, util.GetPGBackRestLogPathForInstance(cluster))+`) ||`,
411+
`halt "$(permissions `+shell.QuoteWord(util.GetPGBackRestLogPathForInstance(cluster))+` ||:)"`,
411412
)
412413

413414
pg_rewind_override := ""

internal/util/pgbackrest.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2017 - 2025 Crunchy Data Solutions, Inc.
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package util
6+
7+
import (
8+
"path/filepath"
9+
10+
"github.com/crunchydata/postgres-operator/internal/naming"
11+
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
12+
)
13+
14+
// GetInstanceLogPath is responsible for determining the appropriate log path for pgbackrest
15+
// in instance pods. If the user has set a log path via the spec, use that. Otherwise, use
16+
// the default log path set in the naming package. Ensure trailing slashes are trimmed.
17+
//
18+
// This function assumes that the backups/pgbackrest spec is present in postgresCluster.
19+
func GetPGBackRestLogPathForInstance(postgresCluster *v1beta1.PostgresCluster) string {
20+
logPath := naming.PGBackRestPGDataLogPath
21+
if postgresCluster.Spec.Backups.PGBackRest.Log != nil &&
22+
postgresCluster.Spec.Backups.PGBackRest.Log.Path != "" {
23+
logPath = postgresCluster.Spec.Backups.PGBackRest.Log.Path
24+
}
25+
return filepath.Clean(logPath)
26+
}

internal/util/pgbackrest_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2021 - 2025 Crunchy Data Solutions, Inc.
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package util
6+
7+
import (
8+
"testing"
9+
10+
"gotest.tools/v3/assert"
11+
12+
"github.com/crunchydata/postgres-operator/internal/naming"
13+
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
14+
)
15+
16+
func TestGetPGBackRestLogPathForInstance(t *testing.T) {
17+
t.Run("NoSpecPath", func(t *testing.T) {
18+
postgrescluster := &v1beta1.PostgresCluster{
19+
Spec: v1beta1.PostgresClusterSpec{
20+
Backups: v1beta1.Backups{
21+
PGBackRest: v1beta1.PGBackRestArchive{},
22+
},
23+
},
24+
}
25+
assert.Equal(t, GetPGBackRestLogPathForInstance(postgrescluster), naming.PGBackRestPGDataLogPath)
26+
})
27+
28+
t.Run("SpecPathSet", func(t *testing.T) {
29+
postgrescluster := &v1beta1.PostgresCluster{
30+
Spec: v1beta1.PostgresClusterSpec{
31+
Backups: v1beta1.Backups{
32+
PGBackRest: v1beta1.PGBackRestArchive{
33+
Log: &v1beta1.LoggingConfiguration{
34+
Path: "/volumes/test/log",
35+
},
36+
},
37+
},
38+
},
39+
}
40+
assert.Equal(t, GetPGBackRestLogPathForInstance(postgrescluster), "/volumes/test/log")
41+
})
42+
}

pkg/apis/postgres-operator.crunchydata.com/v1/pgbackrest_types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
)
1010

1111
// PGBackRestArchive defines a pgBackRest archive configuration
12+
// +kubebuilder:validation:XValidation:rule=`!has(self.log) || !has(self.log.path) || self.log.path.startsWith("/volumes/")`,message=`log path is restricted to an existing additional volume`
1213
// +kubebuilder:validation:XValidation:rule=`!has(self.repoHost) || !has(self.repoHost.log) || !has(self.repoHost.log.path) || self.repoHost.volumes.additional.exists(x, self.repoHost.log.path.startsWith("/volumes/"+x.name))`,message=`log path is restricted to an existing additional volume`
1314
// +kubebuilder:validation:XValidation:rule=`!has(self.jobs) || !has(self.jobs.log) || !has(self.jobs.log.path) || self.jobs.volumes.additional.exists(x, self.jobs.log.path.startsWith("/volumes/"+x.name))`,message=`log path is restricted to an existing additional volume`
1415
type PGBackRestArchive struct {

pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,8 @@ type PostgresInstanceSetSpec struct {
530530
// +optional
531531
TablespaceVolumes []TablespaceVolume `json:"tablespaceVolumes,omitempty"`
532532

533+
// Volumes to be added to the instance set.
534+
// +optional
533535
Volumes *v1beta1.PostgresVolumesSpec `json:"volumes,omitempty"`
534536
}
535537

pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ type PostgresClusterSpec struct {
5959
// namespace as the cluster.
6060
// +optional
6161
DatabaseInitSQL *DatabaseInitSQL `json:"databaseInitSQL,omitempty"`
62+
6263
// Whether or not the PostgreSQL cluster should use the defined default
6364
// scheduling constraints. If the field is unset or false, the default
6465
// scheduling constraints will be used in addition to any custom constraints
@@ -526,6 +527,8 @@ type PostgresInstanceSetSpec struct {
526527
// +optional
527528
TablespaceVolumes []TablespaceVolume `json:"tablespaceVolumes,omitempty"`
528529

530+
// Volumes to be added to the instance set.
531+
// +optional
529532
Volumes *PostgresVolumesSpec `json:"volumes,omitempty"`
530533
}
531534

0 commit comments

Comments
 (0)