diff --git a/internal/controller/postgrescluster/cluster.go b/internal/controller/postgrescluster/cluster.go index 67544d621b..5cd515f5a6 100644 --- a/internal/controller/postgrescluster/cluster.go +++ b/internal/controller/postgrescluster/cluster.go @@ -15,6 +15,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/patroni" @@ -44,7 +45,7 @@ func (r *Reconciler) reconcileClusterConfigMap( if err == nil { err = patroni.ClusterConfigMap(ctx, cluster, pgHBAs, pgParameters, - clusterConfigMap, r.patroniLogSize(cluster)) + clusterConfigMap, r.patroniLogSize(ctx, cluster)) } if err == nil { err = errors.WithStack(r.apply(ctx, clusterConfigMap)) @@ -57,25 +58,25 @@ func (r *Reconciler) reconcileClusterConfigMap( // If a value is set, this enables volume based log storage and triggers the // relevant Patroni configuration. If the value given is less than 25M, the log // file size storage limit defaults to 25M and an event is triggered. -func (r *Reconciler) patroniLogSize(cluster *v1beta1.PostgresCluster) int64 { +// If a value is not set, but the OpenTelemetryLogs feature gate is enabled, the +// log file size storage limit will be set to 25M. +func (r *Reconciler) patroniLogSize(ctx context.Context, cluster *v1beta1.PostgresCluster) int64 { + if cluster.Spec.Patroni != nil && cluster.Spec.Patroni.Logging != nil && + cluster.Spec.Patroni.Logging.StorageLimit != nil { - if cluster.Spec.Patroni != nil { - if cluster.Spec.Patroni.Logging != nil { - if cluster.Spec.Patroni.Logging.StorageLimit != nil { + sizeInBytes := cluster.Spec.Patroni.Logging.StorageLimit.Value() - sizeInBytes := cluster.Spec.Patroni.Logging.StorageLimit.Value() + if sizeInBytes < 25000000 { + // TODO(validation): Eventually we should be able to remove this in favor of CEL validation. + // - https://kubernetes.io/docs/reference/using-api/cel/ + r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "PatroniLogStorageLimitTooSmall", + "Configured Patroni log storage limit is too small. File size will default to 25M.") - if sizeInBytes < 25000000 { - // TODO(validation): Eventually we should be able to remove this in favor of CEL validation. - // - https://kubernetes.io/docs/reference/using-api/cel/ - r.Recorder.Eventf(cluster, corev1.EventTypeWarning, "PatroniLogStorageLimitTooSmall", - "Configured Patroni log storage limit is too small. File size will default to 25M.") - - sizeInBytes = 25000000 - } - return sizeInBytes - } + sizeInBytes = 25000000 } + return sizeInBytes + } else if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + return 25000000 } return 0 } diff --git a/internal/controller/postgrescluster/cluster_test.go b/internal/controller/postgrescluster/cluster_test.go index e08d4e855c..6882cfa27b 100644 --- a/internal/controller/postgrescluster/cluster_test.go +++ b/internal/controller/postgrescluster/cluster_test.go @@ -21,6 +21,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/crunchydata/postgres-operator/internal/controller/runtime" + "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/testing/cmp" @@ -787,6 +788,7 @@ postgres-operator.crunchydata.com/role: replica } func TestPatroniLogSize(t *testing.T) { + ctx := context.Background() oneHundredMeg, err := resource.ParseQuantity("100M") assert.NilError(t, err) @@ -805,7 +807,7 @@ func TestPatroniLogSize(t *testing.T) { recorder := events.NewRecorder(t, runtime.Scheme) reconciler := &Reconciler{Recorder: recorder} - size := reconciler.patroniLogSize(&cluster) + size := reconciler.patroniLogSize(ctx, &cluster) assert.Equal(t, size, int64(0)) assert.Equal(t, len(recorder.Events), 0) @@ -818,7 +820,7 @@ func TestPatroniLogSize(t *testing.T) { cluster.Spec.Patroni = &v1beta1.PatroniSpec{ Logging: &v1beta1.PatroniLogConfig{}} - size := reconciler.patroniLogSize(&cluster) + size := reconciler.patroniLogSize(ctx, &cluster) assert.Equal(t, size, int64(0)) assert.Equal(t, len(recorder.Events), 0) @@ -833,7 +835,7 @@ func TestPatroniLogSize(t *testing.T) { StorageLimit: &oneHundredMeg, }} - size := reconciler.patroniLogSize(&cluster) + size := reconciler.patroniLogSize(ctx, &cluster) assert.Equal(t, size, int64(100000000)) assert.Equal(t, len(recorder.Events), 0) @@ -848,7 +850,7 @@ func TestPatroniLogSize(t *testing.T) { StorageLimit: &tooSmall, }} - size := reconciler.patroniLogSize(&cluster) + size := reconciler.patroniLogSize(ctx, &cluster) assert.Equal(t, size, int64(25000000)) assert.Equal(t, len(recorder.Events), 1) @@ -856,4 +858,43 @@ func TestPatroniLogSize(t *testing.T) { assert.Equal(t, recorder.Events[0].Reason, "PatroniLogStorageLimitTooSmall") assert.Equal(t, recorder.Events[0].Note, "Configured Patroni log storage limit is too small. File size will default to 25M.") }) + + t.Run("SizeUnsetOtelLogsEnabled", func(t *testing.T) { + gate := feature.NewGate() + assert.NilError(t, gate.SetFromMap(map[string]bool{ + feature.OpenTelemetryLogs: true, + })) + ctx := feature.NewContext(ctx, gate) + + recorder := events.NewRecorder(t, runtime.Scheme) + reconciler := &Reconciler{Recorder: recorder} + + cluster.Spec.Patroni = nil + + size := reconciler.patroniLogSize(ctx, &cluster) + + assert.Equal(t, size, int64(25000000)) + assert.Equal(t, len(recorder.Events), 0) + }) + + t.Run("SizeSetOtelLogsEnabled", func(t *testing.T) { + gate := feature.NewGate() + assert.NilError(t, gate.SetFromMap(map[string]bool{ + feature.OpenTelemetryLogs: true, + })) + ctx := feature.NewContext(ctx, gate) + + recorder := events.NewRecorder(t, runtime.Scheme) + reconciler := &Reconciler{Recorder: recorder} + + cluster.Spec.Patroni = &v1beta1.PatroniSpec{ + Logging: &v1beta1.PatroniLogConfig{ + StorageLimit: &oneHundredMeg, + }} + + size := reconciler.patroniLogSize(ctx, &cluster) + + assert.Equal(t, size, int64(100000000)) + assert.Equal(t, len(recorder.Events), 0) + }) } diff --git a/internal/patroni/config.go b/internal/patroni/config.go index 7e0b72f038..48c1ec399e 100644 --- a/internal/patroni/config.go +++ b/internal/patroni/config.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/yaml" "github.com/crunchydata/postgres-operator/internal/config" + "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/internal/shell" @@ -151,9 +152,17 @@ func clusterYAML( }, } - // if a Patroni log file size is configured, configure volume file storage + // If a Patroni log file size is configured (the user set it in the + // spec or the OpenTelemetryLogs feature gate is enabled), we need to + // configure volume file storage if patroniLogStorageLimit != 0 { + logLevel := initialize.Pointer("INFO") + if cluster.Spec.Patroni != nil && cluster.Spec.Patroni.Logging != nil && + cluster.Spec.Patroni.Logging.Level != nil { + logLevel = cluster.Spec.Patroni.Logging.Level + } + // Configure the Patroni log settings // - https://patroni.readthedocs.io/en/latest/yaml_configuration.html#log root["log"] = map[string]any{ @@ -162,7 +171,7 @@ func clusterYAML( "type": "json", // defaults to "INFO" - "level": cluster.Spec.Patroni.Logging.Level, + "level": logLevel, // Setting group read permissions so that the OTel filelog receiver can // read the log files.