Skip to content

Set correct permissions on the specified log directory #4226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Set correct permissions on the specified log directory #4226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef0301e
Add a Bash function to create Postgres tablespace and data directories
cbandy Aug 15, 2025
d4a953a
Expand relative paths in shell.MakeDirectories
cbandy Jul 30, 2025
9642615
Set correct permissions on the specified log directory
cbandy Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions internal/collector/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package collector
import (
"context"
"fmt"
"path"

corev1 "k8s.io/api/core/v1"

Expand Down Expand Up @@ -185,7 +184,7 @@ func startCommand(logDirectories []string, includeLogrotate bool) []string {
if len(logDirectories) != 0 {
for _, logDir := range logDirectories {
mkdirScript = mkdirScript + `
` + shell.MakeDirectories(logDir, path.Join(logDir, "receiver"))
` + shell.MakeDirectories(logDir, "receiver")
}
}

Expand Down
96 changes: 57 additions & 39 deletions internal/collector/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,60 @@ import (
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
)

func PostgreSQLParameters(ctx context.Context,
inCluster *v1beta1.PostgresCluster,
outParameters *postgres.Parameters,
) {
version := inCluster.Spec.PostgresVersion

if OpenTelemetryLogsEnabled(ctx, inCluster) {
var spec *v1beta1.InstrumentationLogsSpec
if inCluster != nil && inCluster.Spec.Instrumentation != nil {
spec = inCluster.Spec.Instrumentation.Logs
}

// Retain logs for a short time unless specified.
retention := metav1.Duration{Duration: 24 * time.Hour}
if spec != nil && spec.RetentionPeriod != nil {
retention = spec.RetentionPeriod.AsDuration()
}

// Rotate log files according to retention and name them for the OpenTelemetry Collector.
//
// The ".log" suffix is replaced by ".csv" for CSV log files, and
// the ".log" suffix is replaced by ".json" for JSON log files.
//
// https://www.postgresql.org/docs/current/runtime-config-logging.html
for k, v := range postgres.LogRotation(retention, "postgresql-", ".log") {
outParameters.Mandatory.Add(k, v)
}

// Enable logging to file. Postgres uses a "logging collector" to safely write concurrent messages.
// NOTE: That collector is designed to not lose messages. When it is overloaded, other Postgres processes block.
//
// https://www.postgresql.org/docs/current/runtime-config-logging.html
outParameters.Mandatory.Add("logging_collector", "on")

// PostgreSQL v8.3 adds support for CSV logging, and
// PostgreSQL v15 adds support for JSON logging.
// The latter is preferred because newlines are escaped as "\n", U+005C + U+006E.
if version >= 15 {
outParameters.Mandatory.Add("log_destination", "jsonlog")
} else {
outParameters.Mandatory.Add("log_destination", "csvlog")
}

// Log in a timezone the OpenTelemetry Collector understands.
outParameters.Mandatory.Add("log_timezone", "UTC")

// TODO(logs): Remove this call and do it in [postgres.NewParameters] regardless of the gate.
outParameters.Mandatory.Add("log_directory", fmt.Sprintf("%s/logs/postgres", postgres.DataStorage(inCluster)))
}
}

func NewConfigForPostgresPod(ctx context.Context,
inCluster *v1beta1.PostgresCluster,
outParameters *postgres.ParameterSet,
inParameters *postgres.ParameterSet,
) *Config {
config := NewConfig(inCluster.Spec.Instrumentation)

Expand All @@ -30,7 +81,7 @@ func NewConfigForPostgresPod(ctx context.Context,
EnablePatroniMetrics(ctx, inCluster, config)

// Logging
EnablePostgresLogging(ctx, inCluster, config, outParameters)
EnablePostgresLogging(ctx, inCluster, inParameters, config)
EnablePatroniLogging(ctx, inCluster, config)

return config
Expand Down Expand Up @@ -76,51 +127,18 @@ func postgresCSVNames(version int) string {
func EnablePostgresLogging(
ctx context.Context,
inCluster *v1beta1.PostgresCluster,
inParameters *postgres.ParameterSet,
outConfig *Config,
outParameters *postgres.ParameterSet,
) {
var spec *v1beta1.InstrumentationLogsSpec
if inCluster != nil && inCluster.Spec.Instrumentation != nil {
spec = inCluster.Spec.Instrumentation.Logs
}

if OpenTelemetryLogsEnabled(ctx, inCluster) {
directory := postgres.LogDirectory()
directory := inParameters.Value("log_directory")
version := inCluster.Spec.PostgresVersion

// https://www.postgresql.org/docs/current/runtime-config-logging.html
outParameters.Add("logging_collector", "on")
outParameters.Add("log_directory", directory)

// PostgreSQL v8.3 adds support for CSV logging, and
// PostgreSQL v15 adds support for JSON logging. The latter is preferred
// because newlines are escaped as "\n", U+005C + U+006E.
if version < 15 {
outParameters.Add("log_destination", "csvlog")
} else {
outParameters.Add("log_destination", "jsonlog")
}

// If retentionPeriod is set in the spec, use that value; otherwise, we want
// to use a reasonably short duration. Defaulting to 1 day.
retentionPeriod := metav1.Duration{Duration: 24 * time.Hour}
if spec != nil && spec.RetentionPeriod != nil {
retentionPeriod = spec.RetentionPeriod.AsDuration()
}

// Rotate log files according to retention.
//
// The ".log" suffix is replaced by ".csv" for CSV log files, and
// the ".log" suffix is replaced by ".json" for JSON log files.
//
// https://www.postgresql.org/docs/current/runtime-config-logging.html
for k, v := range postgres.LogRotation(retentionPeriod, "postgresql-", ".log") {
outParameters.Add(k, v)
}

// Log in a timezone that the OpenTelemetry Collector will understand.
outParameters.Add("log_timezone", "UTC")

// Keep track of what log records and files have been processed.
// Use a subdirectory of the logs directory to stay within the same failure domain.
// TODO(log-rotation): Create this directory during Collector startup.
Expand All @@ -145,8 +163,8 @@ func EnablePostgresLogging(
// The 2nd through 5th fields are optional, so match through to the 7th field.
// This should do a decent job of not matching the middle of some SQL statement.
//
// The number of fields has changed over the years, but the first few
// are always formatted the same way.
// The number of fields has changed over the years, but the first few are always formatted the same way.
// [PostgreSQLParameters] ensures the timezone is UTC.
//
// NOTE: This regexp is invoked in multi-line mode. https://go.dev/s/re2syntax
"multiline": map[string]string{
Expand Down
12 changes: 8 additions & 4 deletions internal/collector/postgres_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ func TestEnablePostgresLogging(t *testing.T) {
}`)

config := NewConfig(nil)
params := postgres.NewParameterSet()
params := postgres.NewParameters()

EnablePostgresLogging(ctx, cluster, config, params)
// NOTE: This call is necessary only because the default "log_directory" is not absolute.
PostgreSQLParameters(ctx, cluster, &params)
EnablePostgresLogging(ctx, cluster, params.Mandatory, config)

result, err := config.ToYAML()
assert.NilError(t, err)
Expand Down Expand Up @@ -293,9 +295,11 @@ service:
cluster.Spec.Instrumentation = testInstrumentationSpec()

config := NewConfig(cluster.Spec.Instrumentation)
params := postgres.NewParameterSet()
params := postgres.NewParameters()

EnablePostgresLogging(ctx, cluster, config, params)
// NOTE: This call is necessary only because the default "log_directory" is not absolute.
PostgreSQLParameters(ctx, cluster, &params)
EnablePostgresLogging(ctx, cluster, params.Mandatory, config)

result, err := config.ToYAML()
assert.NilError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (r *Reconciler) Reconcile(
ctx, cluster, clusterConfigMap, clusterReplicationSecret, rootCA,
clusterPodService, instanceServiceAccount, instances, patroniLeaderService,
primaryCertificate, clusterVolumes, exporterQueriesConfig, exporterWebConfig,
backupsSpecFound, otelConfig,
backupsSpecFound, otelConfig, pgParameters,
)
}

Expand Down
9 changes: 6 additions & 3 deletions internal/controller/postgrescluster/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ func (r *Reconciler) reconcileInstanceSets(
exporterQueriesConfig, exporterWebConfig *corev1.ConfigMap,
backupsSpecFound bool,
otelConfig *collector.Config,
pgParameters *postgres.ParameterSet,
) error {

// Go through the observed instances and check if a primary has been determined.
Expand Down Expand Up @@ -571,7 +572,7 @@ func (r *Reconciler) reconcileInstanceSets(
patroniLeaderService, primaryCertificate,
findAvailableInstanceNames(*set, instances, clusterVolumes),
numInstancePods, clusterVolumes, exporterQueriesConfig, exporterWebConfig,
backupsSpecFound, otelConfig,
backupsSpecFound, otelConfig, pgParameters,
)

if err == nil {
Expand Down Expand Up @@ -1007,6 +1008,7 @@ func (r *Reconciler) scaleUpInstances(
exporterQueriesConfig, exporterWebConfig *corev1.ConfigMap,
backupsSpecFound bool,
otelConfig *collector.Config,
pgParameters *postgres.ParameterSet,
) ([]*appsv1.StatefulSet, error) {
log := logging.FromContext(ctx)

Expand Down Expand Up @@ -1053,7 +1055,7 @@ func (r *Reconciler) scaleUpInstances(
rootCA, clusterPodService, instanceServiceAccount,
patroniLeaderService, primaryCertificate, instances[i],
numInstancePods, clusterVolumes, exporterQueriesConfig, exporterWebConfig,
backupsSpecFound, otelConfig,
backupsSpecFound, otelConfig, pgParameters,
)
}
if err == nil {
Expand Down Expand Up @@ -1085,6 +1087,7 @@ func (r *Reconciler) reconcileInstance(
exporterQueriesConfig, exporterWebConfig *corev1.ConfigMap,
backupsSpecFound bool,
otelConfig *collector.Config,
pgParameters *postgres.ParameterSet,
) error {
log := logging.FromContext(ctx).WithValues("instance", instance.Name)
ctx = logging.NewContext(ctx, log)
Expand Down Expand Up @@ -1128,7 +1131,7 @@ func (r *Reconciler) reconcileInstance(
postgres.InstancePod(
ctx, cluster, spec,
primaryCertificate, replicationCertSecretProjection(clusterReplicationSecret),
postgresDataVolume, postgresWALVolume, tablespaceVolumes,
postgresDataVolume, postgresWALVolume, tablespaceVolumes, pgParameters,
&instance.Spec.Template)

if backupsSpecFound {
Expand Down
2 changes: 2 additions & 0 deletions internal/controller/postgrescluster/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/crunchydata/postgres-operator/internal/collector"
"github.com/crunchydata/postgres-operator/internal/feature"
"github.com/crunchydata/postgres-operator/internal/initialize"
"github.com/crunchydata/postgres-operator/internal/logging"
Expand Down Expand Up @@ -129,6 +130,7 @@ func (*Reconciler) generatePostgresParameters(
ctx context.Context, cluster *v1beta1.PostgresCluster, backupsSpecFound bool,
) *postgres.ParameterSet {
builtin := postgres.NewParameters()
collector.PostgreSQLParameters(ctx, cluster, &builtin)
pgaudit.PostgreSQLParameters(&builtin)
pgbackrest.PostgreSQLParameters(cluster, &builtin, backupsSpecFound)
pgmonitor.PostgreSQLParameters(ctx, cluster, &builtin)
Expand Down
Loading
Loading