Skip to content

Commit 28ac2ad

Browse files
tjmoore4tjmoore4
committed
Configure Patroni Logs to be stored in a file
This commit allows for the configuration of the Postgres instance Patroni logs to go to a file on the 'pgdata' volume instead of to stdout. This file is located at '/pgdata/patroni/log/patroni.log'. Both the log size limit and log level are configurable; only the size limit setting is required. If not set, the default behavior of sending all logs to stdout is maintained. Changes to this configuration require a reload to take effect. - https://patroni.readthedocs.io/en/latest/yaml_configuration.html#log Issue: PGO-1701
1 parent c5cb440 commit 28ac2ad

File tree

12 files changed

+300
-13
lines changed

12 files changed

+300
-13
lines changed

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11630,6 +11630,30 @@ spec:
1163011630
format: int32
1163111631
minimum: 3
1163211632
type: integer
11633+
log:
11634+
description: Patroni log configuration settings.
11635+
properties:
11636+
fileSize:
11637+
description: |-
11638+
Patroni Log file size
11639+
https://pkg.go.dev/k8s.io/apimachinery/pkg/api/resource#Quantity
11640+
type: string
11641+
logLevel:
11642+
default: INFO
11643+
description: |-
11644+
Patroni log level
11645+
https://docs.python.org/3.6/library/logging.html#levels
11646+
enum:
11647+
- CRITICAL
11648+
- ERROR
11649+
- WARNING
11650+
- INFO
11651+
- DEBUG
11652+
- NOTSET
11653+
type: string
11654+
required:
11655+
- fileSize
11656+
type: object
1163311657
port:
1163411658
default: 8008
1163511659
description: |-

‎internal/controller/postgrescluster/cluster.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (r *Reconciler) reconcileClusterConfigMap(
4444

4545
if err == nil {
4646
err = patroni.ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters,
47-
clusterConfigMap)
47+
clusterConfigMap, r.patroniLogSize(ctx, cluster))
4848
}
4949
if err == nil {
5050
err = errors.WithStack(r.apply(ctx, clusterConfigMap))
@@ -53,6 +53,31 @@ func (r *Reconciler) reconcileClusterConfigMap(
5353
return clusterConfigMap, err
5454
}
5555

56+
// patroniLogSize attemps to parse the defined log file storage limit, if configured.
57+
// If a value is set, this enables volume based log storage and triggers the
58+
// relevant Patroni configuration. If the value given is less than 25M, the log
59+
// file size storage limit defaults to 25M and an event is triggered.
60+
func (r *Reconciler) patroniLogSize(ctx context.Context, cluster *v1beta1.PostgresCluster) int64 {
61+
62+
if cluster.Spec.Patroni != nil {
63+
if cluster.Spec.Patroni.Logging != nil {
64+
if cluster.Spec.Patroni.Logging.StorageLimit != nil {
65+
66+
sizeInBytes := cluster.Spec.Patroni.Logging.StorageLimit.Value()
67+
68+
if sizeInBytes < 25000000 {
69+
r.Recorder.Eventf(cluster, corev1.EventTypeNormal, "PatroniLogStorageLimitTooSmall",
70+
"Configured Patroni log storage limit is too small. File size will default to 25M.")
71+
72+
sizeInBytes = 25000000
73+
}
74+
return sizeInBytes
75+
}
76+
}
77+
}
78+
return 0
79+
}
80+
5681
// +kubebuilder:rbac:groups="",resources="services",verbs={create,patch}
5782

5883
// reconcileClusterPodService writes the Service that can provide stable DNS

‎internal/controller/postgrescluster/cluster_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
batchv1 "k8s.io/api/batch/v1"
1414
corev1 "k8s.io/api/core/v1"
1515
rbacv1 "k8s.io/api/rbac/v1"
16+
"k8s.io/apimachinery/pkg/api/resource"
1617
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1819
"k8s.io/client-go/tools/record"
@@ -23,6 +24,7 @@ import (
2324
"github.com/crunchydata/postgres-operator/internal/initialize"
2425
"github.com/crunchydata/postgres-operator/internal/naming"
2526
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
27+
"github.com/crunchydata/postgres-operator/internal/testing/events"
2628
"github.com/crunchydata/postgres-operator/internal/testing/require"
2729
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2830
)
@@ -783,3 +785,76 @@ postgres-operator.crunchydata.com/role: replica
783785
`))
784786
})
785787
}
788+
789+
func TestPatroniLogSize(t *testing.T) {
790+
ctx := context.Background()
791+
792+
oneHundredMeg, err := resource.ParseQuantity("100M")
793+
assert.NilError(t, err)
794+
795+
tooSmall, err := resource.ParseQuantity("1k")
796+
assert.NilError(t, err)
797+
798+
cluster := v1beta1.PostgresCluster{
799+
ObjectMeta: metav1.ObjectMeta{
800+
Name: "sometest",
801+
Namespace: "test-namespace",
802+
},
803+
Spec: v1beta1.PostgresClusterSpec{}}
804+
805+
t.Run("Default", func(t *testing.T) {
806+
recorder := events.NewRecorder(t, runtime.Scheme)
807+
reconciler := &Reconciler{Recorder: recorder}
808+
809+
size := reconciler.patroniLogSize(ctx, &cluster)
810+
811+
assert.Equal(t, size, int64(0))
812+
assert.Equal(t, len(recorder.Events), 0)
813+
})
814+
815+
t.Run("NoSize", func(t *testing.T) {
816+
recorder := events.NewRecorder(t, runtime.Scheme)
817+
reconciler := &Reconciler{Recorder: recorder}
818+
819+
cluster.Spec.Patroni = &v1beta1.PatroniSpec{
820+
Logging: &v1beta1.PatroniLogConfig{}}
821+
822+
size := reconciler.patroniLogSize(ctx, &cluster)
823+
824+
assert.Equal(t, size, int64(0))
825+
assert.Equal(t, len(recorder.Events), 0)
826+
})
827+
828+
t.Run("ValidSize", func(t *testing.T) {
829+
recorder := events.NewRecorder(t, runtime.Scheme)
830+
reconciler := &Reconciler{Recorder: recorder}
831+
832+
cluster.Spec.Patroni = &v1beta1.PatroniSpec{
833+
Logging: &v1beta1.PatroniLogConfig{
834+
StorageLimit: &oneHundredMeg,
835+
}}
836+
837+
size := reconciler.patroniLogSize(ctx, &cluster)
838+
839+
assert.Equal(t, size, int64(100000000))
840+
assert.Equal(t, len(recorder.Events), 0)
841+
})
842+
843+
t.Run("BadSize", func(t *testing.T) {
844+
recorder := events.NewRecorder(t, runtime.Scheme)
845+
reconciler := &Reconciler{Recorder: recorder}
846+
847+
cluster.Spec.Patroni = &v1beta1.PatroniSpec{
848+
Logging: &v1beta1.PatroniLogConfig{
849+
StorageLimit: &tooSmall,
850+
}}
851+
852+
size := reconciler.patroniLogSize(ctx, &cluster)
853+
854+
assert.Equal(t, size, int64(25000000))
855+
assert.Equal(t, len(recorder.Events), 1)
856+
assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name)
857+
assert.Equal(t, recorder.Events[0].Reason, "PatroniLogStorageLimitTooSmall")
858+
assert.Equal(t, recorder.Events[0].Note, "Configured Patroni log storage limit is too small. File size will default to 25M.")
859+
})
860+
}

‎internal/naming/names.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ const (
131131
)
132132

133133
const (
134+
// PatroniPGDataLogPath is the Patroni default log path configuration used by the
135+
// PostgreSQL instance.
136+
PatroniPGDataLogPath = "/pgdata/patroni/log"
137+
134138
// PGBackRestRepoContainerName is the name assigned to the container used to run pgBackRest
135139
PGBackRestRepoContainerName = "pgbackrest"
136140

‎internal/patroni/config.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func quoteShellWord(s string) string {
4343
// clusterYAML returns Patroni settings that apply to the entire cluster.
4444
func clusterYAML(
4545
cluster *v1beta1.PostgresCluster,
46-
pgHBAs postgres.HBAs, pgParameters postgres.Parameters,
46+
pgHBAs postgres.HBAs, pgParameters postgres.Parameters, patroniLogStorageLimit int64,
4747
) (string, error) {
4848
root := map[string]any{
4949
// The cluster identifier. This value cannot change during the cluster's
@@ -152,6 +152,29 @@ func clusterYAML(
152152
},
153153
}
154154

155+
// if a Patroni log file size is configured, configure volume file storage
156+
if patroniLogStorageLimit != 0 {
157+
158+
// Configure the Patroni log settings
159+
// - https://patroni.readthedocs.io/en/latest/yaml_configuration.html#log
160+
root["log"] = map[string]any{
161+
162+
"dir": naming.PatroniPGDataLogPath,
163+
"type": "json",
164+
165+
// defaults to "INFO"
166+
"level": cluster.Spec.Patroni.Logging.Level,
167+
168+
// There will only be two log files. Cannot set to 1 or the logs won't rotate.
169+
// - https://github.com/python/cpython/blob/3.11/Lib/logging/handlers.py#L134
170+
"file_num": 1,
171+
172+
// Since there are two log files, ensure the total space used is under
173+
// the configured limit.
174+
"file_size": patroniLogStorageLimit / 2,
175+
}
176+
}
177+
155178
if !ClusterBootstrapped(cluster) {
156179
// Patroni has not yet bootstrapped. Populate the "bootstrap.dcs" field to
157180
// facilitate it. When Patroni is already bootstrapped, this field is ignored.

‎internal/patroni/config_test.go

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"gotest.tools/v3/assert"
1515
corev1 "k8s.io/api/core/v1"
16+
"k8s.io/apimachinery/pkg/api/resource"
1617
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"sigs.k8s.io/yaml"
1819

@@ -32,7 +33,7 @@ func TestClusterYAML(t *testing.T) {
3233
cluster.Namespace = "some-namespace"
3334
cluster.Name = "cluster-name"
3435

35-
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
36+
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 0)
3637
assert.NilError(t, err)
3738
assert.Equal(t, data, strings.TrimSpace(`
3839
# Generated by postgres-operator. DO NOT EDIT.
@@ -90,7 +91,7 @@ watchdog:
9091
cluster.Name = "cluster-name"
9192
cluster.Spec.PostgresVersion = 14
9293

93-
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
94+
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 0)
9495
assert.NilError(t, err)
9596
assert.Equal(t, data, strings.TrimSpace(`
9697
# Generated by postgres-operator. DO NOT EDIT.
@@ -136,6 +137,79 @@ restapi:
136137
keyfile: null
137138
verify_client: optional
138139
scope: cluster-name-ha
140+
watchdog:
141+
mode: "off"
142+
`)+"\n")
143+
})
144+
145+
t.Run("PatroniLogSizeConfigured", func(t *testing.T) {
146+
cluster := new(v1beta1.PostgresCluster)
147+
cluster.Default()
148+
cluster.Namespace = "some-namespace"
149+
cluster.Name = "cluster-name"
150+
cluster.Spec.PostgresVersion = 14
151+
152+
fileSize, err := resource.ParseQuantity("1k")
153+
assert.NilError(t, err)
154+
155+
logLevel := "DEBUG"
156+
cluster.Spec.Patroni.Logging = &v1beta1.PatroniLogConfig{
157+
StorageLimit: &fileSize,
158+
Level: &logLevel,
159+
}
160+
161+
data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 1000)
162+
assert.NilError(t, err)
163+
assert.Equal(t, data, strings.TrimSpace(`
164+
# Generated by postgres-operator. DO NOT EDIT.
165+
# Your changes will not be saved.
166+
bootstrap:
167+
dcs:
168+
loop_wait: 10
169+
postgresql:
170+
parameters: {}
171+
pg_hba: []
172+
use_pg_rewind: true
173+
use_slots: false
174+
ttl: 30
175+
ctl:
176+
cacert: /etc/patroni/~postgres-operator/patroni.ca-roots
177+
certfile: /etc/patroni/~postgres-operator/patroni.crt+key
178+
insecure: false
179+
keyfile: null
180+
kubernetes:
181+
labels:
182+
postgres-operator.crunchydata.com/cluster: cluster-name
183+
namespace: some-namespace
184+
role_label: postgres-operator.crunchydata.com/role
185+
scope_label: postgres-operator.crunchydata.com/patroni
186+
use_endpoints: true
187+
log:
188+
dir: /pgdata/patroni/log
189+
file_num: 1
190+
file_size: 500
191+
level: DEBUG
192+
type: json
193+
postgresql:
194+
authentication:
195+
replication:
196+
sslcert: /tmp/replication/tls.crt
197+
sslkey: /tmp/replication/tls.key
198+
sslmode: verify-ca
199+
sslrootcert: /tmp/replication/ca.crt
200+
username: _crunchyrepl
201+
rewind:
202+
sslcert: /tmp/replication/tls.crt
203+
sslkey: /tmp/replication/tls.key
204+
sslmode: verify-ca
205+
sslrootcert: /tmp/replication/ca.crt
206+
username: _crunchyrepl
207+
restapi:
208+
cafile: /etc/patroni/~postgres-operator/patroni.ca-roots
209+
certfile: /etc/patroni/~postgres-operator/patroni.crt+key
210+
keyfile: null
211+
verify_client: optional
212+
scope: cluster-name-ha
139213
watchdog:
140214
mode: "off"
141215
`)+"\n")

‎internal/patroni/reconcile.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,14 @@ func ClusterConfigMap(ctx context.Context,
3232
inHBAs postgres.HBAs,
3333
inParameters postgres.Parameters,
3434
outClusterConfigMap *corev1.ConfigMap,
35+
patroniLogStorageLimit int64,
3536
) error {
3637
var err error
3738

3839
initialize.Map(&outClusterConfigMap.Data)
3940

4041
outClusterConfigMap.Data[configMapFileKey], err = clusterYAML(inCluster, inHBAs,
41-
inParameters)
42+
inParameters, patroniLogStorageLimit)
4243

4344
return err
4445
}

‎internal/patroni/reconcile_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,15 @@ func TestClusterConfigMap(t *testing.T) {
2929

3030
cluster.Default()
3131
config := new(corev1.ConfigMap)
32-
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config))
32+
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0))
3333

3434
// The output of clusterYAML should go into config.
35-
data, _ := clusterYAML(cluster, pgHBAs, pgParameters)
35+
data, _ := clusterYAML(cluster, pgHBAs, pgParameters, 0)
3636
assert.DeepEqual(t, config.Data["patroni.yaml"], data)
3737

3838
// No change when called again.
3939
before := config.DeepCopy()
40-
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config))
40+
assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0))
4141
assert.DeepEqual(t, config, before)
4242
}
4343

‎internal/postgres/config.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,9 @@ chmod +x /tmp/pg_rewind_tde.sh
291291
`
292292
}
293293

294-
args := []string{version, walDir, naming.PGBackRestPGDataLogPath}
294+
args := []string{version, walDir, naming.PGBackRestPGDataLogPath, naming.PatroniPGDataLogPath}
295295
script := strings.Join([]string{
296-
`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3"`,
296+
`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3" patroniLog_directory="$4"`,
297297

298298
// Function to print the permissions of a file or directory and its parents.
299299
bashPermissions,
@@ -369,6 +369,11 @@ chmod +x /tmp/pg_rewind_tde.sh
369369
`install --directory --mode=0775 "${pgbrLog_directory}" ||`,
370370
`halt "$(permissions "${pgbrLog_directory}" ||:)"`,
371371

372+
// Create the Patroni log directory.
373+
`results 'Patroni log directory' "${patroniLog_directory}"`,
374+
`install --directory --mode=0775 "${patroniLog_directory}" ||`,
375+
`halt "$(permissions "${patroniLog_directory}" ||:)"`,
376+
372377
// Copy replication client certificate files
373378
// from the /pgconf/tls/replication directory to the /tmp/replication directory in order
374379
// to set proper file permissions. This is required because the group permission settings

0 commit comments

Comments
 (0)