From f449c38c8cf56ef25e2f944b84f3bdc4319deba0 Mon Sep 17 00:00:00 2001
From: Chris Bandy <chris.bandy@crunchydata.com>
Date: Wed, 30 Jul 2025 13:57:15 -0500
Subject: [PATCH 1/3] Expand relative paths in shell.MakeDirectories

This function needed defined behavior for relative paths. For example,
the default log directory for Postgres is just "log" which it interprets
relative to the data directory.

Issue: PGO-2558
---
 internal/shell/paths.go      | 17 ++++++++++++++---
 internal/shell/paths_test.go | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/internal/shell/paths.go b/internal/shell/paths.go
index 701144694..5b4cafe30 100644
--- a/internal/shell/paths.go
+++ b/internal/shell/paths.go
@@ -36,6 +36,8 @@ func CleanFileName(path string) string {
 // exists. It creates every directory leading to path from (but not including)
 // base and sets their permissions for Kubernetes, regardless of umask.
 //
+// Relative paths are expanded relative to base.
+//
 // See:
 //   - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/chmod.html
 //   - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/mkdir.html
@@ -47,8 +49,17 @@ func MakeDirectories(base string, paths ...string) string {
 		return `test -d ` + QuoteWord(base)
 	}
 
-	allPaths := slices.Clone(paths)
-	for _, p := range paths {
+	// Expand each path relative to the base path.
+	expandedPaths := slices.Clone(paths)
+	for i, p := range expandedPaths {
+		if !filepath.IsAbs(p) {
+			expandedPaths[i] = filepath.Join(base, p)
+		}
+	}
+
+	// Gather parent directories of each path.
+	allPaths := slices.Clone(expandedPaths)
+	for _, p := range expandedPaths {
 		if r, err := filepath.Rel(base, p); err == nil && filepath.IsLocal(r) {
 			// The result of [filepath.Rel] is a shorter representation of the full path; skip it.
 			r = filepath.Dir(r)
@@ -72,7 +83,7 @@ func MakeDirectories(base string, paths ...string) string {
 
 	return `` +
 		// Create all the paths and any missing parents.
-		`mkdir -p ` + strings.Join(QuoteWords(paths...), " ") +
+		`mkdir -p ` + strings.Join(QuoteWords(expandedPaths...), " ") +
 
 		// Try to set the permissions of every path and each parent.
 		// This swallows the exit status of `chmod` because not all filesystems
diff --git a/internal/shell/paths_test.go b/internal/shell/paths_test.go
index b5adb69b1..f3b9aea82 100644
--- a/internal/shell/paths_test.go
+++ b/internal/shell/paths_test.go
@@ -94,6 +94,38 @@ func TestMakeDirectories(t *testing.T) {
 		})
 	})
 
+	t.Run("Relative", func(t *testing.T) {
+		assert.Equal(t,
+			MakeDirectories("/x", "one", "two/three"),
+			`mkdir -p '/x/one' '/x/two/three' && { chmod 0775 '/x/one' '/x/two/three' '/x/two' || :; }`,
+			"expected paths relative to base")
+
+		assert.Equal(t,
+			MakeDirectories("/x/y/z", "../one", "./two", "../../../../three"),
+			`mkdir -p '/x/y/one' '/x/y/z/two' '/three' && { chmod 0775 '/x/y/one' '/x/y/z/two' '/three' || :; }`,
+			"expected paths relative to base")
+
+		script := MakeDirectories("x/y", "../one", "./two", "../../../../three")
+		assert.Equal(t, script,
+			`mkdir -p 'x/one' 'x/y/two' '../../three' && { chmod 0775 'x/one' 'x/y/two' '../../three' || :; }`,
+			"expected paths relative to base")
+
+		// Calling `mkdir -p '../..'` works, but run it by ShellCheck as a precaution.
+		t.Run("ShellCheckPOSIX", func(t *testing.T) {
+			shellcheck := require.ShellCheck(t)
+
+			dir := t.TempDir()
+			file := filepath.Join(dir, "script.sh")
+			assert.NilError(t, os.WriteFile(file, []byte(script), 0o600))
+
+			// Expect ShellCheck for "sh" to be happy.
+			// - https://www.shellcheck.net/wiki/SC2148
+			cmd := exec.CommandContext(t.Context(), shellcheck, "--enable=all", "--shell=sh", file)
+			output, err := cmd.CombinedOutput()
+			assert.NilError(t, err, "%q\n%s", cmd.Args, output)
+		})
+	})
+
 	t.Run("Unrelated", func(t *testing.T) {
 		assert.Equal(t,
 			MakeDirectories("/one", "/two/three/four"),

From f5dd3489b989096c757da6a46475bca6c929ba60 Mon Sep 17 00:00:00 2001
From: Chris Bandy <chris.bandy@crunchydata.com>
Date: Wed, 30 Jul 2025 15:55:59 -0500
Subject: [PATCH 2/3] Set correct permissions on the specified log directory

Controllers ignored the specified "log_directory" when preparing
directories for Postgres. This parameter can be specified when the
OpenTelemetryLogs gate is disabled.

This does not change what happens when the OpenTelemetryLogs gate is
enabled. In that case, controllers override the spec with their own
value and prepare that directory.

Issue: PGO-2558
---
 internal/collector/postgres.go                | 96 +++++++++++--------
 internal/collector/postgres_test.go           | 12 ++-
 .../controller/postgrescluster/controller.go  |  2 +-
 .../controller/postgrescluster/instance.go    |  9 +-
 .../controller/postgrescluster/postgres.go    |  2 +
 internal/postgres/config.go                   | 20 ++--
 internal/postgres/config_test.go              | 13 ++-
 internal/postgres/parameters.go               | 11 +++
 internal/postgres/parameters_test.go          |  2 +
 internal/postgres/reconcile.go                |  3 +-
 internal/postgres/reconcile_test.go           | 24 ++---
 11 files changed, 125 insertions(+), 69 deletions(-)

diff --git a/internal/collector/postgres.go b/internal/collector/postgres.go
index 892748c0a..6dc2eddcd 100644
--- a/internal/collector/postgres.go
+++ b/internal/collector/postgres.go
@@ -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)
 
@@ -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
@@ -76,8 +127,8 @@ 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 {
@@ -85,42 +136,9 @@ func EnablePostgresLogging(
 	}
 
 	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.
@@ -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{
diff --git a/internal/collector/postgres_test.go b/internal/collector/postgres_test.go
index 89f5f5225..a1a221bd6 100644
--- a/internal/collector/postgres_test.go
+++ b/internal/collector/postgres_test.go
@@ -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)
@@ -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)
diff --git a/internal/controller/postgrescluster/controller.go b/internal/controller/postgrescluster/controller.go
index bbe141c0b..c3264d0a3 100644
--- a/internal/controller/postgrescluster/controller.go
+++ b/internal/controller/postgrescluster/controller.go
@@ -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,
 		)
 	}
 
diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go
index 0c91ca715..46a18be71 100644
--- a/internal/controller/postgrescluster/instance.go
+++ b/internal/controller/postgrescluster/instance.go
@@ -593,6 +593,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.
@@ -630,7 +631,7 @@ func (r *Reconciler) reconcileInstanceSets(
 			patroniLeaderService, primaryCertificate,
 			findAvailableInstanceNames(*set, instances, clusterVolumes),
 			numInstancePods, clusterVolumes, exporterQueriesConfig, exporterWebConfig,
-			backupsSpecFound, otelConfig,
+			backupsSpecFound, otelConfig, pgParameters,
 		)
 
 		if err == nil {
@@ -1066,6 +1067,7 @@ func (r *Reconciler) scaleUpInstances(
 	exporterQueriesConfig, exporterWebConfig *corev1.ConfigMap,
 	backupsSpecFound bool,
 	otelConfig *collector.Config,
+	pgParameters *postgres.ParameterSet,
 ) ([]*appsv1.StatefulSet, error) {
 	log := logging.FromContext(ctx)
 
@@ -1112,7 +1114,7 @@ func (r *Reconciler) scaleUpInstances(
 			rootCA, clusterPodService, instanceServiceAccount,
 			patroniLeaderService, primaryCertificate, instances[i],
 			numInstancePods, clusterVolumes, exporterQueriesConfig, exporterWebConfig,
-			backupsSpecFound, otelConfig,
+			backupsSpecFound, otelConfig, pgParameters,
 		)
 	}
 	if err == nil {
@@ -1144,6 +1146,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)
@@ -1187,7 +1190,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 {
diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go
index 8922e5f73..489b9ad06 100644
--- a/internal/controller/postgrescluster/postgres.go
+++ b/internal/controller/postgrescluster/postgres.go
@@ -26,6 +26,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"
@@ -130,6 +131,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)
diff --git a/internal/postgres/config.go b/internal/postgres/config.go
index 174aee34b..4d909f050 100644
--- a/internal/postgres/config.go
+++ b/internal/postgres/config.go
@@ -89,13 +89,13 @@ func ConfigDirectory(cluster *v1beta1.PostgresCluster) string {
 // DataDirectory returns the absolute path to the "data_directory" of cluster.
 // - https://www.postgresql.org/docs/current/runtime-config-file-locations.html
 func DataDirectory(cluster *v1beta1.PostgresCluster) string {
-	return fmt.Sprintf("%s/pg%d", dataMountPath, cluster.Spec.PostgresVersion)
+	return fmt.Sprintf("%s/pg%d", DataStorage(cluster), cluster.Spec.PostgresVersion)
 }
 
-// LogDirectory returns the absolute path to the "log_directory" of cluster.
-// - https://www.postgresql.org/docs/current/runtime-config-logging.html
-func LogDirectory() string {
-	return fmt.Sprintf("%s/logs/postgres", dataMountPath)
+// DataStorage returns the absolute path to the disk where cluster stores its data.
+// Use [DataDirectory] for the exact directory that Postgres uses.
+func DataStorage(cluster *v1beta1.PostgresCluster) string {
+	return dataMountPath
 }
 
 // LogRotation returns parameters that rotate log files while keeping a minimum amount.
@@ -310,9 +310,12 @@ done
 // PostgreSQL.
 func startupCommand(
 	ctx context.Context,
-	cluster *v1beta1.PostgresCluster, instance *v1beta1.PostgresInstanceSetSpec,
+	cluster *v1beta1.PostgresCluster,
+	instance *v1beta1.PostgresInstanceSetSpec,
+	parameters *ParameterSet,
 ) []string {
 	version := fmt.Sprint(cluster.Spec.PostgresVersion)
+	logDir := parameters.Value("log_directory")
 	walDir := WALDirectory(cluster, instance)
 
 	// If the user requests tablespaces, we want to make sure the directories exist with the
@@ -447,8 +450,9 @@ chmod +x /tmp/pg_rewind_tde.sh
 		`halt "$(permissions ` + naming.PGBackRestPGDataLogPath + ` ||:)"`,
 		`(` + shell.MakeDirectories(dataMountPath, naming.PatroniPGDataLogPath) + `) ||`,
 		`halt "$(permissions ` + naming.PatroniPGDataLogPath + ` ||:)"`,
-		`(` + shell.MakeDirectories(dataMountPath, LogDirectory()) + `) ||`,
-		`halt "$(permissions ` + LogDirectory() + ` ||:)"`,
+		`(` + shell.MakeDirectories(DataDirectory(cluster), logDir) + `) ||`,
+		// FIXME: This error prints the wrong directory when logDir is relative (the default).
+		`halt "$(permissions ` + logDir + ` ||:)"`,
 
 		// Copy replication client certificate files
 		// from the /pgconf/tls/replication directory to the /tmp/replication directory in order
diff --git a/internal/postgres/config_test.go b/internal/postgres/config_test.go
index 762bd8a0b..32490df26 100644
--- a/internal/postgres/config_test.go
+++ b/internal/postgres/config_test.go
@@ -40,6 +40,13 @@ func TestDataDirectory(t *testing.T) {
 	assert.Equal(t, DataDirectory(cluster), "/pgdata/pg12")
 }
 
+func TestDataStorage(t *testing.T) {
+	cluster := new(v1beta1.PostgresCluster)
+	cluster.Spec.PostgresVersion = rand.IntN(20)
+
+	assert.Equal(t, DataStorage(cluster), "/pgdata")
+}
+
 func TestLogRotation(t *testing.T) {
 	t.Parallel()
 
@@ -543,8 +550,10 @@ func TestStartupCommand(t *testing.T) {
 	cluster.Spec.PostgresVersion = 13
 	instance := new(v1beta1.PostgresInstanceSetSpec)
 
+	parameters := NewParameters().Default
+
 	ctx := context.Background()
-	command := startupCommand(ctx, cluster, instance)
+	command := startupCommand(ctx, cluster, instance, parameters)
 
 	// Expect a bash command with an inline script.
 	assert.DeepEqual(t, command[:3], []string{"bash", "-ceu", "--"})
@@ -579,7 +588,7 @@ func TestStartupCommand(t *testing.T) {
 				},
 			},
 		}
-		command := startupCommand(ctx, cluster, instance)
+		command := startupCommand(ctx, cluster, instance, parameters)
 		assert.Assert(t, len(command) > 3)
 		assert.Assert(t, strings.Contains(command[3], `cat << "EOF" > /tmp/pg_rewind_tde.sh
 #!/bin/sh
diff --git a/internal/postgres/parameters.go b/internal/postgres/parameters.go
index 6fb7b0d2f..3590816b4 100644
--- a/internal/postgres/parameters.go
+++ b/internal/postgres/parameters.go
@@ -48,6 +48,17 @@ func NewParameters() Parameters {
 	// - https://www.postgresql.org/docs/current/auth-password.html
 	parameters.Default.Add("password_encryption", "scram-sha-256")
 
+	// Explicitly use Postgres' default log directory. This value is relative to the "data_directory" parameter.
+	//
+	// Controllers look at this parameter to grant group-write S_IWGRP on the directory.
+	// Postgres does not grant this permission on the directories it creates.
+	//
+	// TODO(logs): A better default would be *outside* the data directory.
+	// This parameter needs to be configurable and documented before the default can change.
+	//
+	// PostgreSQL must be reloaded when changing this parameter.
+	parameters.Default.Add("log_directory", "log")
+
 	// Pod "securityContext.fsGroup" ensures processes and filesystems agree on a GID;
 	// use the same permissions for group and owner.
 	// This allows every process in the pod to read Postgres log files.
diff --git a/internal/postgres/parameters_test.go b/internal/postgres/parameters_test.go
index ad8c6e90c..54095750f 100644
--- a/internal/postgres/parameters_test.go
+++ b/internal/postgres/parameters_test.go
@@ -28,6 +28,8 @@ func TestNewParameters(t *testing.T) {
 	assert.DeepEqual(t, parameters.Default.AsMap(), map[string]string{
 		"jit": "off",
 
+		"log_directory": "log",
+
 		"password_encryption": "scram-sha-256",
 	})
 }
diff --git a/internal/postgres/reconcile.go b/internal/postgres/reconcile.go
index 5041140b0..157b56292 100644
--- a/internal/postgres/reconcile.go
+++ b/internal/postgres/reconcile.go
@@ -68,6 +68,7 @@ func InstancePod(ctx context.Context,
 	inClusterCertificates, inClientCertificates *corev1.SecretProjection,
 	inDataVolume, inWALVolume *corev1.PersistentVolumeClaim,
 	inTablespaceVolumes []*corev1.PersistentVolumeClaim,
+	inParameters *ParameterSet,
 	outInstancePod *corev1.PodTemplateSpec,
 ) {
 	certVolumeMount := corev1.VolumeMount{
@@ -201,7 +202,7 @@ func InstancePod(ctx context.Context,
 	startup := corev1.Container{
 		Name: naming.ContainerPostgresStartup,
 
-		Command: startupCommand(ctx, inCluster, inInstanceSpec),
+		Command: startupCommand(ctx, inCluster, inInstanceSpec, inParameters),
 		Env:     Environment(inCluster),
 
 		Image:           container.Image,
diff --git a/internal/postgres/reconcile_test.go b/internal/postgres/reconcile_test.go
index 61a85d5cd..fcb3fef54 100644
--- a/internal/postgres/reconcile_test.go
+++ b/internal/postgres/reconcile_test.go
@@ -69,6 +69,8 @@ func TestInstancePod(t *testing.T) {
 	cluster.Spec.ImagePullPolicy = corev1.PullAlways
 	cluster.Spec.PostgresVersion = 11
 
+	parameters := NewParameters().Default
+
 	dataVolume := new(corev1.PersistentVolumeClaim)
 	dataVolume.Name = "datavol"
 
@@ -117,7 +119,7 @@ func TestInstancePod(t *testing.T) {
 	// without WAL volume nor WAL volume spec
 	pod := new(corev1.PodTemplateSpec)
 	InstancePod(ctx, cluster, instance,
-		serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, pod)
+		serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, parameters, pod)
 
 	assert.Assert(t, cmp.MarshalMatches(pod.Spec, `
 containers:
@@ -272,8 +274,8 @@ initContainers:
     halt "$(permissions /pgdata/pgbackrest/log ||:)"
     (mkdir -p '/pgdata/patroni/log' && { chmod 0775 '/pgdata/patroni/log' '/pgdata/patroni' || :; }) ||
     halt "$(permissions /pgdata/patroni/log ||:)"
-    (mkdir -p '/pgdata/logs/postgres' && { chmod 0775 '/pgdata/logs/postgres' '/pgdata/logs' || :; }) ||
-    halt "$(permissions /pgdata/logs/postgres ||:)"
+    (mkdir -p '/pgdata/pg11/log' && { chmod 0775 '/pgdata/pg11/log' || :; }) ||
+    halt "$(permissions log ||:)"
     install -D --mode=0600 -t "/tmp/replication" "/pgconf/tls/replication"/{tls.crt,tls.key,ca.crt}
 
 
@@ -385,7 +387,7 @@ volumes:
 
 		pod := new(corev1.PodTemplateSpec)
 		InstancePod(ctx, cluster, instance,
-			serverSecretProjection, clientSecretProjection, dataVolume, walVolume, nil, pod)
+			serverSecretProjection, clientSecretProjection, dataVolume, walVolume, nil, parameters, pod)
 
 		assert.Assert(t, len(pod.Spec.Containers) > 0)
 		assert.Assert(t, len(pod.Spec.InitContainers) > 0)
@@ -486,7 +488,7 @@ volumes:
 
 		pod := new(corev1.PodTemplateSpec)
 		InstancePod(ctx, clusterWithConfig, instance,
-			serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, pod)
+			serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, parameters, pod)
 
 		assert.Assert(t, len(pod.Spec.Containers) > 0)
 		assert.Assert(t, len(pod.Spec.InitContainers) > 0)
@@ -523,7 +525,7 @@ volumes:
 
 		t.Run("SidecarNotEnabled", func(t *testing.T) {
 			InstancePod(ctx, cluster, sidecarInstance,
-				serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, pod)
+				serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, parameters, pod)
 
 			assert.Equal(t, len(pod.Spec.Containers), 2, "expected 2 containers in Pod")
 		})
@@ -536,7 +538,7 @@ volumes:
 			ctx := feature.NewContext(ctx, gate)
 
 			InstancePod(ctx, cluster, sidecarInstance,
-				serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, pod)
+				serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, parameters, pod)
 
 			assert.Equal(t, len(pod.Spec.Containers), 3, "expected 3 containers in Pod")
 
@@ -573,7 +575,7 @@ volumes:
 		tablespaceVolumes := []*corev1.PersistentVolumeClaim{tablespaceVolume1, tablespaceVolume2}
 
 		InstancePod(ctx, cluster, instance,
-			serverSecretProjection, clientSecretProjection, dataVolume, nil, tablespaceVolumes, pod)
+			serverSecretProjection, clientSecretProjection, dataVolume, nil, tablespaceVolumes, parameters, pod)
 
 		assert.Assert(t, cmp.MarshalMatches(pod.Spec.Containers[0].VolumeMounts, `
 - mountPath: /pgconf/tls
@@ -611,7 +613,7 @@ volumes:
 
 		pod := new(corev1.PodTemplateSpec)
 		InstancePod(ctx, cluster, instance,
-			serverSecretProjection, clientSecretProjection, dataVolume, walVolume, nil, pod)
+			serverSecretProjection, clientSecretProjection, dataVolume, walVolume, nil, parameters, pod)
 
 		assert.Assert(t, len(pod.Spec.Containers) > 0)
 		assert.Assert(t, len(pod.Spec.InitContainers) > 0)
@@ -713,7 +715,7 @@ volumes:
 
 		pod := new(corev1.PodTemplateSpec)
 		InstancePod(ctx, cluster, instance,
-			serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, pod)
+			serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, parameters, pod)
 
 		assert.Assert(t, len(pod.Spec.Containers) > 0)
 		assert.Assert(t, cmp.MarshalContains(pod.Spec.Containers[0].VolumeMounts, `
@@ -743,7 +745,7 @@ volumes:
 			annotated.Labels = map[string]string{"gg": "asdf"}
 
 			InstancePod(ctx, cluster, instance,
-				serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, annotated)
+				serverSecretProjection, clientSecretProjection, dataVolume, nil, nil, parameters, annotated)
 
 			assert.Assert(t, cmp.MarshalContains(annotated.Spec.Volumes, `
 - ephemeral:

From 354a00db46bc2b48df61333a37a7a0c9e8534d6a Mon Sep 17 00:00:00 2001
From: Chris Bandy <chris.bandy@crunchydata.com>
Date: Fri, 1 Aug 2025 10:55:56 -0500
Subject: [PATCH 3/3] FIXUP: mkdir conditionally

---
 internal/postgres/config.go         | 31 ++++++++++++++++++++---------
 internal/postgres/reconcile_test.go | 12 +++++------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/internal/postgres/config.go b/internal/postgres/config.go
index 4d909f050..583314d3e 100644
--- a/internal/postgres/config.go
+++ b/internal/postgres/config.go
@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 	"math"
+	"path"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
@@ -315,9 +316,28 @@ func startupCommand(
 	parameters *ParameterSet,
 ) []string {
 	version := fmt.Sprint(cluster.Spec.PostgresVersion)
-	logDir := parameters.Value("log_directory")
+	dataDir := DataDirectory(cluster)
 	walDir := WALDirectory(cluster, instance)
 
+	// TODO: describe this?
+	mkdirs := make([]string, 0, 6)
+	mkdir := func(b, p string) {
+		if path.IsAbs(p) {
+			p = path.Clean(p)
+		} else {
+			p = path.Join(dataDir, p)
+		}
+
+		// Create directories unless they are in the empty Postgres data directory.
+		mkdirs = append(mkdirs,
+			`[[ `+shell.QuoteWord(p)+` != "${postgres_data_directory}"* || -f "${postgres_data_directory}/PG_VERSION" ]] &&`,
+			`{ (`+shell.MakeDirectories(b, p)+`) || halt "$(permissions `+shell.QuoteWord(p)+` ||:)"; }`,
+		)
+	}
+	mkdir(dataMountPath, naming.PGBackRestPGDataLogPath)
+	mkdir(dataMountPath, naming.PatroniPGDataLogPath)
+	mkdir(dataDir, parameters.Value("log_directory"))
+
 	// If the user requests tablespaces, we want to make sure the directories exist with the
 	// correct owner and permissions.
 	tablespaceCmd := ""
@@ -445,14 +465,7 @@ chmod +x /tmp/pg_rewind_tde.sh
 		`else (halt Permissions!); fi ||`,
 		`halt "$(permissions "${postgres_data_directory}" ||:)"`,
 
-		// Create log directories.
-		`(` + shell.MakeDirectories(dataMountPath, naming.PGBackRestPGDataLogPath) + `) ||`,
-		`halt "$(permissions ` + naming.PGBackRestPGDataLogPath + ` ||:)"`,
-		`(` + shell.MakeDirectories(dataMountPath, naming.PatroniPGDataLogPath) + `) ||`,
-		`halt "$(permissions ` + naming.PatroniPGDataLogPath + ` ||:)"`,
-		`(` + shell.MakeDirectories(DataDirectory(cluster), logDir) + `) ||`,
-		// FIXME: This error prints the wrong directory when logDir is relative (the default).
-		`halt "$(permissions ` + logDir + ` ||:)"`,
+		strings.Join(mkdirs, "\n"),
 
 		// Copy replication client certificate files
 		// from the /pgconf/tls/replication directory to the /tmp/replication directory in order
diff --git a/internal/postgres/reconcile_test.go b/internal/postgres/reconcile_test.go
index fcb3fef54..ad24e1989 100644
--- a/internal/postgres/reconcile_test.go
+++ b/internal/postgres/reconcile_test.go
@@ -270,12 +270,12 @@ initContainers:
     recreate "${postgres_data_directory}" '0750'
     else (halt Permissions!); fi ||
     halt "$(permissions "${postgres_data_directory}" ||:)"
-    (mkdir -p '/pgdata/pgbackrest/log' && { chmod 0775 '/pgdata/pgbackrest/log' '/pgdata/pgbackrest' || :; }) ||
-    halt "$(permissions /pgdata/pgbackrest/log ||:)"
-    (mkdir -p '/pgdata/patroni/log' && { chmod 0775 '/pgdata/patroni/log' '/pgdata/patroni' || :; }) ||
-    halt "$(permissions /pgdata/patroni/log ||:)"
-    (mkdir -p '/pgdata/pg11/log' && { chmod 0775 '/pgdata/pg11/log' || :; }) ||
-    halt "$(permissions log ||:)"
+    [[ '/pgdata/pgbackrest/log' != "${postgres_data_directory}"* || -f "${postgres_data_directory}/PG_VERSION" ]] &&
+    { (mkdir -p '/pgdata/pgbackrest/log' && { chmod 0775 '/pgdata/pgbackrest/log' '/pgdata/pgbackrest' || :; }) || halt "$(permissions '/pgdata/pgbackrest/log' ||:)"; }
+    [[ '/pgdata/patroni/log' != "${postgres_data_directory}"* || -f "${postgres_data_directory}/PG_VERSION" ]] &&
+    { (mkdir -p '/pgdata/patroni/log' && { chmod 0775 '/pgdata/patroni/log' '/pgdata/patroni' || :; }) || halt "$(permissions '/pgdata/patroni/log' ||:)"; }
+    [[ '/pgdata/pg11/log' != "${postgres_data_directory}"* || -f "${postgres_data_directory}/PG_VERSION" ]] &&
+    { (mkdir -p '/pgdata/pg11/log' && { chmod 0775 '/pgdata/pg11/log' || :; }) || halt "$(permissions '/pgdata/pg11/log' ||:)"; }
     install -D --mode=0600 -t "/tmp/replication" "/pgconf/tls/replication"/{tls.crt,tls.key,ca.crt}