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

Author: tjmoore4

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

@@ -11630,6 +11630,28 @@ spec:
                     format: int32
                     minimum: 3
                     type: integer
+                  log:
+                    description: Patroni log configuration settings.
+                    properties:
+                      fileSize:
+                        description: Patroni Log file size
+                        type: string
+                      logLevel:
+                        default: INFO
+                        description: |-
+                          Patroni log level
+                          https://docs.python.org/3.6/library/logging.html#levels
+                        enum:
+                        - CRITICAL
+                        - ERROR
+                        - WARNING
+                        - INFO
+                        - DEBUG
+                        - NOTSET
+                        type: string
+                    required:
+                    - fileSize
+                    type: object
                   port:
                     default: 8008
                     description: |-

internal/controller/postgrescluster/cluster.go

@@ -12,6 +12,7 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 
@@ -44,7 +45,7 @@ func (r *Reconciler) reconcileClusterConfigMap(
 
 	if err == nil {
 		err = patroni.ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters,
-			clusterConfigMap)
+			clusterConfigMap, r.patroniLogSize(ctx, cluster))
 	}
 	if err == nil {
 		err = errors.WithStack(r.apply(ctx, clusterConfigMap))
@@ -53,6 +54,32 @@ func (r *Reconciler) reconcileClusterConfigMap(
 	return clusterConfigMap, err
 }
 
+// patroniLogSize attemps to parse the defined log file size, if configured.
+// If a value is set, this enables volume based log storage and triggers the
+// relevant Patroni configuration. If the value given cannot be parsed, the log
+// file size defaults to 25M and a warning event is triggered.
+func (r *Reconciler) patroniLogSize(ctx context.Context, cluster *v1beta1.PostgresCluster) int64 {
+
+	if cluster.Spec.Patroni != nil {
+		if cluster.Spec.Patroni.Log != nil {
+			if cluster.Spec.Patroni.Log.FileSize != nil {
+
+				fileSize, err := resource.ParseQuantity(*cluster.Spec.Patroni.Log.FileSize)
+				sizeInBytes := fileSize.Value()
+
+				if err != nil {
+					r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "InvalidPatroniLogFileSize",
+						"Error when parsing spec.patroni.log.fileSize. File size will default to 25M.")
+
+					sizeInBytes = 25000000
+				}
+				return sizeInBytes
+			}
+		}
+	}
+	return 0
+}
+
 // +kubebuilder:rbac:groups="",resources="services",verbs={create,patch}
 
 // reconcileClusterPodService writes the Service that can provide stable DNS

internal/controller/postgrescluster/cluster_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/crunchydata/postgres-operator/internal/initialize"
 	"github.com/crunchydata/postgres-operator/internal/naming"
 	"github.com/crunchydata/postgres-operator/internal/testing/cmp"
+	"github.com/crunchydata/postgres-operator/internal/testing/events"
 	"github.com/crunchydata/postgres-operator/internal/testing/require"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
@@ -783,3 +784,73 @@ postgres-operator.crunchydata.com/role: replica
 		`))
 	})
 }
+
+func TestPatroniLogSize(t *testing.T) {
+	ctx := context.Background()
+
+	oneMeg := "1M"
+	badSize := "notASize"
+
+	cluster := v1beta1.PostgresCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "sometest",
+			Namespace: "test-namespace",
+		},
+		Spec: v1beta1.PostgresClusterSpec{}}
+
+	t.Run("Default", func(t *testing.T) {
+		recorder := events.NewRecorder(t, runtime.Scheme)
+		reconciler := &Reconciler{Recorder: recorder}
+
+		size := reconciler.patroniLogSize(ctx, &cluster)
+
+		assert.Equal(t, size, int64(0))
+		assert.Equal(t, len(recorder.Events), 0)
+	})
+
+	t.Run("NoSize", func(t *testing.T) {
+		recorder := events.NewRecorder(t, runtime.Scheme)
+		reconciler := &Reconciler{Recorder: recorder}
+
+		cluster.Spec.Patroni = &v1beta1.PatroniSpec{
+			Log: &v1beta1.PatroniLogConfig{}}
+
+		size := reconciler.patroniLogSize(ctx, &cluster)
+
+		assert.Equal(t, size, int64(0))
+		assert.Equal(t, len(recorder.Events), 0)
+	})
+
+	t.Run("ValidSize", func(t *testing.T) {
+		recorder := events.NewRecorder(t, runtime.Scheme)
+		reconciler := &Reconciler{Recorder: recorder}
+
+		cluster.Spec.Patroni = &v1beta1.PatroniSpec{
+			Log: &v1beta1.PatroniLogConfig{
+				FileSize: &oneMeg,
+			}}
+
+		size := reconciler.patroniLogSize(ctx, &cluster)
+
+		assert.Equal(t, size, int64(1000000))
+		assert.Equal(t, len(recorder.Events), 0)
+	})
+
+	t.Run("BadSize", func(t *testing.T) {
+		recorder := events.NewRecorder(t, runtime.Scheme)
+		reconciler := &Reconciler{Recorder: recorder}
+
+		cluster.Spec.Patroni = &v1beta1.PatroniSpec{
+			Log: &v1beta1.PatroniLogConfig{
+				FileSize: &badSize,
+			}}
+
+		size := reconciler.patroniLogSize(ctx, &cluster)
+
+		assert.Equal(t, size, int64(25000000))
+		assert.Equal(t, len(recorder.Events), 1)
+		assert.Equal(t, recorder.Events[0].Regarding.Name, cluster.Name)
+		assert.Equal(t, recorder.Events[0].Reason, "InvalidPatroniLogFileSize")
+		assert.Equal(t, recorder.Events[0].Note, "Error when parsing spec.patroni.log.fileSize. File size will default to 25M.")
+	})
+}

internal/naming/names.go

@@ -131,6 +131,10 @@ const (
 )
 
 const (
+	// PatroniPGDataLogPath is the Patroni default log path configuration used by the
+	// PostgreSQL instance.
+	PatroniPGDataLogPath = "/pgdata/patroni/log"
+
 	// PGBackRestRepoContainerName is the name assigned to the container used to run pgBackRest
 	PGBackRestRepoContainerName = "pgbackrest"
 

internal/patroni/config.go

@@ -43,7 +43,7 @@ func quoteShellWord(s string) string {
 // clusterYAML returns Patroni settings that apply to the entire cluster.
 func clusterYAML(
 	cluster *v1beta1.PostgresCluster,
-	pgHBAs postgres.HBAs, pgParameters postgres.Parameters,
+	pgHBAs postgres.HBAs, pgParameters postgres.Parameters, patroniLogFileSize int64,
 ) (string, error) {
 	root := map[string]any{
 		// The cluster identifier. This value cannot change during the cluster's
@@ -152,6 +152,20 @@ func clusterYAML(
 		},
 	}
 
+	// if a Patroni log file size is configured, configure volume file storage
+	if patroniLogFileSize != 0 {
+
+		root["log"] = map[string]any{
+			// Configure the Patroni log settings
+			// - https://patroni.readthedocs.io/en/latest/yaml_configuration.html#log
+			"dir":       naming.PatroniPGDataLogPath,
+			"file_num":  1,
+			"file_size": patroniLogFileSize,
+			"type":      "json",
+			"level":     cluster.Spec.Patroni.Log.LogLevel, // defaults to "INFO"
+		}
+	}
+
 	if !ClusterBootstrapped(cluster) {
 		// Patroni has not yet bootstrapped. Populate the "bootstrap.dcs" field to
 		// facilitate it. When Patroni is already bootstrapped, this field is ignored.

internal/patroni/config_test.go

@@ -32,7 +32,7 @@ func TestClusterYAML(t *testing.T) {
 		cluster.Namespace = "some-namespace"
 		cluster.Name = "cluster-name"
 
-		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
+		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 0)
 		assert.NilError(t, err)
 		assert.Equal(t, data, strings.TrimSpace(`
 # Generated by postgres-operator. DO NOT EDIT.
@@ -90,7 +90,7 @@ watchdog:
 		cluster.Name = "cluster-name"
 		cluster.Spec.PostgresVersion = 14
 
-		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
+		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 0)
 		assert.NilError(t, err)
 		assert.Equal(t, data, strings.TrimSpace(`
 # Generated by postgres-operator. DO NOT EDIT.
@@ -136,6 +136,77 @@ restapi:
   keyfile: null
   verify_client: optional
 scope: cluster-name-ha
+watchdog:
+  mode: "off"
+	`)+"\n")
+	})
+
+	t.Run("PatroniLogSizeConfigured", func(t *testing.T) {
+		cluster := new(v1beta1.PostgresCluster)
+		cluster.Default()
+		cluster.Namespace = "some-namespace"
+		cluster.Name = "cluster-name"
+		cluster.Spec.PostgresVersion = 14
+
+		fileSize := "1k"
+		logLevel := "DEBUG"
+		cluster.Spec.Patroni.Log = &v1beta1.PatroniLogConfig{
+			FileSize: &fileSize,
+			LogLevel: &logLevel,
+		}
+
+		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{}, 1000)
+		assert.NilError(t, err)
+		assert.Equal(t, data, strings.TrimSpace(`
+# Generated by postgres-operator. DO NOT EDIT.
+# Your changes will not be saved.
+bootstrap:
+  dcs:
+    loop_wait: 10
+    postgresql:
+      parameters: {}
+      pg_hba: []
+      use_pg_rewind: true
+      use_slots: false
+    ttl: 30
+ctl:
+  cacert: /etc/patroni/~postgres-operator/patroni.ca-roots
+  certfile: /etc/patroni/~postgres-operator/patroni.crt+key
+  insecure: false
+  keyfile: null
+kubernetes:
+  labels:
+    postgres-operator.crunchydata.com/cluster: cluster-name
+  namespace: some-namespace
+  role_label: postgres-operator.crunchydata.com/role
+  scope_label: postgres-operator.crunchydata.com/patroni
+  use_endpoints: true
+log:
+  dir: /pgdata/patroni/log
+  file_num: 1
+  file_size: 1000
+  level: DEBUG
+  type: json
+postgresql:
+  authentication:
+    replication:
+      sslcert: /tmp/replication/tls.crt
+      sslkey: /tmp/replication/tls.key
+      sslmode: verify-ca
+      sslrootcert: /tmp/replication/ca.crt
+      username: _crunchyrepl
+    rewind:
+      sslcert: /tmp/replication/tls.crt
+      sslkey: /tmp/replication/tls.key
+      sslmode: verify-ca
+      sslrootcert: /tmp/replication/ca.crt
+      username: _crunchyrepl
+restapi:
+  cafile: /etc/patroni/~postgres-operator/patroni.ca-roots
+  certfile: /etc/patroni/~postgres-operator/patroni.crt+key
+  keyfile: null
+  verify_client: optional
+scope: cluster-name-ha
 watchdog:
   mode: "off"
 	`)+"\n")

internal/patroni/reconcile.go

@@ -32,13 +32,14 @@ func ClusterConfigMap(ctx context.Context,
 	inHBAs postgres.HBAs,
 	inParameters postgres.Parameters,
 	outClusterConfigMap *corev1.ConfigMap,
+	patroniLogFileSize int64,
 ) error {
 	var err error
 
 	initialize.Map(&outClusterConfigMap.Data)
 
 	outClusterConfigMap.Data[configMapFileKey], err = clusterYAML(inCluster, inHBAs,
-		inParameters)
+		inParameters, patroniLogFileSize)
 
 	return err
 }

internal/patroni/reconcile_test.go

@@ -29,15 +29,15 @@ func TestClusterConfigMap(t *testing.T) {
 
 	cluster.Default()
 	config := new(corev1.ConfigMap)
-	assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config))
+	assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0))
 
 	// The output of clusterYAML should go into config.
-	data, _ := clusterYAML(cluster, pgHBAs, pgParameters)
+	data, _ := clusterYAML(cluster, pgHBAs, pgParameters, 0)
 	assert.DeepEqual(t, config.Data["patroni.yaml"], data)
 
 	// No change when called again.
 	before := config.DeepCopy()
-	assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config))
+	assert.NilError(t, ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, config, 0))
 	assert.DeepEqual(t, config, before)
 }
 

internal/postgres/config.go

@@ -291,9 +291,9 @@ chmod +x /tmp/pg_rewind_tde.sh
 `
 	}
 
-	args := []string{version, walDir, naming.PGBackRestPGDataLogPath}
+	args := []string{version, walDir, naming.PGBackRestPGDataLogPath, naming.PatroniPGDataLogPath}
 	script := strings.Join([]string{
-		`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3"`,
+		`declare -r expected_major_version="$1" pgwal_directory="$2" pgbrLog_directory="$3" patroniLog_directory="$4"`,
 
 		// Function to print the permissions of a file or directory and its parents.
 		bashPermissions,
@@ -369,6 +369,11 @@ chmod +x /tmp/pg_rewind_tde.sh
 		`install --directory --mode=0775 "${pgbrLog_directory}" ||`,
 		`halt "$(permissions "${pgbrLog_directory}" ||:)"`,
 
+		// Create the Patroni log directory.
+		`results 'Patroni log directory' "${patroniLog_directory}"`,
+		`install --directory --mode=0775 "${patroniLog_directory}" ||`,
+		`halt "$(permissions "${patroniLog_directory}" ||:)"`,
+
 		// Copy replication client certificate files
 		// from the /pgconf/tls/replication directory to the /tmp/replication directory in order
 		// to set proper file permissions. This is required because the group permission settings