Store pgAdmin log file positions in the logs directory

This prevents log records from being emitted multiple times.

Author: cbandy

internal/collector/pgadmin.go

@@ -17,24 +17,20 @@ func EnablePgAdminLogging(ctx context.Context, configmap *corev1.ConfigMap) erro
 	if !feature.Enabled(ctx, feature.OpenTelemetryLogs) {
 		return nil
 	}
+
 	otelConfig := NewConfig()
-	otelConfig.Extensions["file_storage/pgadmin"] = map[string]any{
-		"directory":        "/var/log/pgadmin/receiver",
-		"create_directory": true,
-		"fsync":            true,
-	}
-	otelConfig.Extensions["file_storage/gunicorn"] = map[string]any{
-		"directory":        "/var/log/gunicorn" + "/receiver",
-		"create_directory": true,
+	otelConfig.Extensions["file_storage/pgadmin_data_logs"] = map[string]any{
+		"directory":        "/var/lib/pgadmin/logs/receiver",
+		"create_directory": false,
 		"fsync":            true,
 	}
 	otelConfig.Receivers["filelog/pgadmin"] = map[string]any{
 		"include": []string{"/var/lib/pgadmin/logs/pgadmin.log"},
-		"storage": "file_storage/pgadmin",
+		"storage": "file_storage/pgadmin_data_logs",
 	}
 	otelConfig.Receivers["filelog/gunicorn"] = map[string]any{
 		"include": []string{"/var/lib/pgadmin/logs/gunicorn.log"},
-		"storage": "file_storage/gunicorn",
+		"storage": "file_storage/pgadmin_data_logs",
 	}
 
 	otelConfig.Processors["resource/pgadmin"] = map[string]any{
@@ -71,7 +67,7 @@ func EnablePgAdminLogging(ctx context.Context, configmap *corev1.ConfigMap) erro
 	}
 
 	otelConfig.Pipelines["logs/pgadmin"] = Pipeline{
-		Extensions: []ComponentID{"file_storage/pgadmin"},
+		Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
 		Receivers:  []ComponentID{"filelog/pgadmin"},
 		Processors: []ComponentID{
 			"resource/pgadmin",
@@ -83,7 +79,7 @@ func EnablePgAdminLogging(ctx context.Context, configmap *corev1.ConfigMap) erro
 	}
 
 	otelConfig.Pipelines["logs/gunicorn"] = Pipeline{
-		Extensions: []ComponentID{"file_storage/gunicorn"},
+		Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
 		Receivers:  []ComponentID{"filelog/gunicorn"},
 		Processors: []ComponentID{
 			"resource/pgadmin",
@@ -95,9 +91,8 @@ func EnablePgAdminLogging(ctx context.Context, configmap *corev1.ConfigMap) erro
 	}
 
 	otelYAML, err := otelConfig.ToYAML()
-	if err != nil {
-		return err
+	if err == nil {
+		configmap.Data["collector.yaml"] = otelYAML
 	}
-	configmap.Data["collector.yaml"] = otelYAML
-	return nil
+	return err
 }

internal/collector/pgadmin_test.go

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
-package collector
+package collector_test
 
 import (
 	"context"
@@ -11,11 +11,11 @@ import (
 	"gotest.tools/v3/assert"
 	corev1 "k8s.io/api/core/v1"
 
+	"github.com/crunchydata/postgres-operator/internal/collector"
+	pgadmin "github.com/crunchydata/postgres-operator/internal/controller/standalone_pgadmin"
 	"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"
-	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
 func TestEnablePgAdminLogging(t *testing.T) {
@@ -27,10 +27,9 @@ func TestEnablePgAdminLogging(t *testing.T) {
 
 		ctx := feature.NewContext(context.Background(), gate)
 
-		pgadmin := new(v1beta1.PGAdmin)
-		configmap := &corev1.ConfigMap{ObjectMeta: naming.StandalonePGAdmin(pgadmin)}
+		configmap := new(corev1.ConfigMap)
 		initialize.Map(&configmap.Data)
-		err := EnablePgAdminLogging(ctx, configmap)
+		err := collector.EnablePgAdminLogging(ctx, configmap)
 		assert.NilError(t, err)
 
 		assert.Assert(t, cmp.MarshalMatches(configmap.Data, `
@@ -41,13 +40,9 @@ collector.yaml: |
     debug:
       verbosity: detailed
   extensions:
-    file_storage/gunicorn:
-      create_directory: true
-      directory: /var/log/gunicorn/receiver
-      fsync: true
-    file_storage/pgadmin:
-      create_directory: true
-      directory: /var/log/pgadmin/receiver
+    file_storage/pgadmin_data_logs:
+      create_directory: false
+      directory: `+pgadmin.LogDirectoryAbsolutePath+`/receiver
       fsync: true
   processors:
     batch/1s:
@@ -83,16 +78,15 @@ collector.yaml: |
   receivers:
     filelog/gunicorn:
       include:
-      - /var/lib/pgadmin/logs/gunicorn.log
-      storage: file_storage/gunicorn
+      - `+pgadmin.GunicornLogFileAbsolutePath+`
+      storage: file_storage/pgadmin_data_logs
     filelog/pgadmin:
       include:
-      - /var/lib/pgadmin/logs/pgadmin.log
-      storage: file_storage/pgadmin
+      - `+pgadmin.LogFileAbsolutePath+`
+      storage: file_storage/pgadmin_data_logs
   service:
     extensions:
-    - file_storage/gunicorn
-    - file_storage/pgadmin
+    - file_storage/pgadmin_data_logs
     pipelines:
       logs/gunicorn:
         exporters:

internal/controller/standalone_pgadmin/pod.go

@@ -7,6 +7,7 @@ package standalone_pgadmin
 import (
 	"context"
 	"fmt"
+	"path"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
@@ -17,6 +18,7 @@ import (
 	"github.com/crunchydata/postgres-operator/internal/initialize"
 	"github.com/crunchydata/postgres-operator/internal/kubernetes"
 	"github.com/crunchydata/postgres-operator/internal/naming"
+	"github.com/crunchydata/postgres-operator/internal/shell"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
@@ -28,8 +30,17 @@ const (
 	ldapFilePath           = "~postgres-operator/ldap-bind-password"
 	gunicornConfigFilePath = "~postgres-operator/" + gunicornConfigKey
 
-	// Nothing should be mounted to this location except the script our initContainer writes
+	// scriptMountPath is where to mount a temporary directory that is only
+	// writable during Pod initialization.
+	//
+	// NOTE: No ConfigMap nor Secret should ever be mounted here because they
+	// could be used to inject code through "config_system.py".
 	scriptMountPath = "/etc/pgadmin"
+
+	dataMountPath               = "/var/lib/pgadmin"
+	LogDirectoryAbsolutePath    = dataMountPath + "/logs"
+	GunicornLogFileAbsolutePath = LogDirectoryAbsolutePath + "/gunicorn.log"
+	LogFileAbsolutePath         = LogDirectoryAbsolutePath + "/pgadmin.log"
 )
 
 // pod populates a PodSpec with the container and volumes needed to run pgAdmin.
@@ -39,54 +50,28 @@ func pod(
 	outPod *corev1.PodSpec,
 	pgAdminVolume *corev1.PersistentVolumeClaim,
 ) {
-	const (
-		// config and data volume names
-		configVolumeName      = "pgadmin-config"
-		dataVolumeName        = "pgadmin-data"
-		pgAdminLogVolumeName  = "pgadmin-log"
-		gunicornLogVolumeName = "gunicorn-log"
-		scriptVolumeName      = "pgadmin-config-system"
-		tempVolumeName        = "tmp"
-	)
-
 	// create the projected volume of config maps for use in
 	// 1. dynamic server discovery
 	// 2. adding the config variables during pgAdmin startup
-	configVolume := corev1.Volume{Name: configVolumeName}
+	configVolume := corev1.Volume{Name: "pgadmin-config"}
 	configVolume.VolumeSource = corev1.VolumeSource{
 		Projected: &corev1.ProjectedVolumeSource{
 			Sources: podConfigFiles(inConfigMap, *inPGAdmin),
 		},
 	}
 
 	// create the data volume for the persistent database
-	dataVolume := corev1.Volume{Name: dataVolumeName}
+	dataVolume := corev1.Volume{Name: "pgadmin-data"}
 	dataVolume.VolumeSource = corev1.VolumeSource{
 		PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
 			ClaimName: pgAdminVolume.Name,
 			ReadOnly:  false,
 		},
 	}
 
-	// create the temp volume for logs
-	pgAdminLogVolume := corev1.Volume{Name: pgAdminLogVolumeName}
-	pgAdminLogVolume.VolumeSource = corev1.VolumeSource{
-		EmptyDir: &corev1.EmptyDirVolumeSource{
-			Medium: corev1.StorageMediumMemory,
-		},
-	}
-
-	// create the temp volume for gunicorn logs
-	gunicornLogVolume := corev1.Volume{Name: gunicornLogVolumeName}
-	gunicornLogVolume.VolumeSource = corev1.VolumeSource{
-		EmptyDir: &corev1.EmptyDirVolumeSource{
-			Medium: corev1.StorageMediumMemory,
-		},
-	}
-
 	// Volume used to write a custom config_system.py file in the initContainer
 	// which then loads the configs found in the `configVolume`
-	scriptVolume := corev1.Volume{Name: scriptVolumeName}
+	scriptVolume := corev1.Volume{Name: "pgadmin-config-system"}
 	scriptVolume.VolumeSource = corev1.VolumeSource{
 		EmptyDir: &corev1.EmptyDirVolumeSource{
 			Medium: corev1.StorageMediumMemory,
@@ -101,7 +86,7 @@ func pod(
 
 	// create a temp volume for restart pid/other/debugging use
 	// TODO: discuss tmp vol vs. persistent vol
-	tmpVolume := corev1.Volume{Name: tempVolumeName}
+	tmpVolume := corev1.Volume{Name: "tmp"}
 	tmpVolume.VolumeSource = corev1.VolumeSource{
 		EmptyDir: &corev1.EmptyDirVolumeSource{
 			Medium: corev1.StorageMediumMemory,
@@ -142,29 +127,21 @@ func pod(
 		},
 		VolumeMounts: []corev1.VolumeMount{
 			{
-				Name:      configVolumeName,
+				Name:      configVolume.Name,
 				MountPath: configMountPath,
 				ReadOnly:  true,
 			},
 			{
-				Name:      dataVolumeName,
-				MountPath: "/var/lib/pgadmin",
-			},
-			{
-				Name:      gunicornLogVolumeName,
-				MountPath: "/var/log/gunicorn",
-			},
-			{
-				Name:      pgAdminLogVolumeName,
-				MountPath: "/var/log/pgadmin",
+				Name:      dataVolume.Name,
+				MountPath: dataMountPath,
 			},
 			{
-				Name:      scriptVolumeName,
+				Name:      scriptVolume.Name,
 				MountPath: scriptMountPath,
 				ReadOnly:  true,
 			},
 			{
-				Name:      tempVolumeName,
+				Name:      tmpVolume.Name,
 				MountPath: "/tmp",
 			},
 		},
@@ -199,19 +176,21 @@ func pod(
 		VolumeMounts: []corev1.VolumeMount{
 			// Volume to write a custom `config_system.py` file to.
 			{
-				Name:      scriptVolumeName,
+				Name:      scriptVolume.Name,
 				MountPath: scriptMountPath,
 				ReadOnly:  false,
 			},
+			{
+				Name:      dataVolume.Name,
+				MountPath: dataMountPath,
+			},
 		},
 	}
 
 	// add volumes and containers
 	outPod.Volumes = []corev1.Volume{
 		configVolume,
 		dataVolume,
-		pgAdminLogVolume,
-		gunicornLogVolume,
 		scriptVolume,
 		tmpVolume,
 	}
@@ -426,8 +405,8 @@ if os.path.isfile('` + configDatabaseURIPathAbsolutePath + `'):
     with open('` + configDatabaseURIPathAbsolutePath + `') as _f:
         CONFIG_DATABASE_URI = _f.read()
 
-DATA_DIR = '/var/lib/pgadmin'
-LOG_FILE = '/var/lib/pgadmin/logs/pgadmin.log'
+DATA_DIR = '` + dataMountPath + `'
+LOG_FILE = '` + LogFileAbsolutePath + `'
 LOG_ROTATION_AGE = 24 * 60 # minutes
 LOG_ROTATION_SIZE = 5 # MiB
 LOG_ROTATION_MAX_LOG_FILES = 1
@@ -437,18 +416,18 @@ CONSOLE_LOG_LEVEL = logging.WARNING
 FILE_LOG_LEVEL = logging.INFO
 FILE_LOG_FORMAT_JSON = {'time': 'created', 'name': 'name', 'level': 'levelname', 'message': 'message'}
 `
-		// gunicorn reads from the `/etc/pgadmin/gunicorn_config.py` file during startup
+		// Gunicorn reads from the `/etc/pgadmin/gunicorn_config.py` file during startup
 		// after all other config files.
 		// - https://docs.gunicorn.org/en/latest/configure.html#configuration-file
 		//
 		// This command writes a script in `/etc/pgadmin/gunicorn_config.py` that reads
 		// from the `gunicorn-config.json` file and sets those variables globally.
-		// That way those values are available as settings when gunicorn starts.
+		// That way those values are available as settings when Gunicorn starts.
 		//
-		// Note: All gunicorn settings are lowercase with underscores, so ignore
+		// Note: All Gunicorn settings are lowercase with underscores, so ignore
 		// any keys/names that are not.
 		//
-		// gunicorn uses the Python logging package, which sets the following attributes:
+		// Gunicorn uses the Python logging package, which sets the following attributes:
 		// https://docs.python.org/3/library/logging.html#logrecord-attributes.
 		// JsonFormatter is used to format the log: https://pypi.org/project/jsonformatter/
 		gunicornConfig = `
@@ -457,13 +436,14 @@ with open('` + configMountPath + `/` + gunicornConfigFilePath + `') as _f:
     _conf, _data = re.compile(r'[a-z_]+'), json.load(_f)
     if type(_data) is dict:
         globals().update({k: v for k, v in _data.items() if _conf.fullmatch(k)})
+
 gunicorn.SERVER_SOFTWARE = 'Python'
 logconfig_dict = copy.deepcopy(gunicorn.glogging.CONFIG_DEFAULTS)
 logconfig_dict['loggers']['gunicorn.access']['handlers'] = ['file']
 logconfig_dict['loggers']['gunicorn.error']['handlers'] = ['file']
 logconfig_dict['handlers']['file'] = {
   'class': 'logging.handlers.RotatingFileHandler',
-  'filename': '/var/lib/pgadmin/logs/gunicorn.log',
+  'filename': '` + GunicornLogFileAbsolutePath + `',
   'backupCount': 1, 'maxBytes': 2 << 20, # MiB
   'formatter': 'json',
 }
@@ -483,9 +463,15 @@ logconfig_dict['formatters']['json'] = {
 	args := []string{strings.TrimLeft(configSystem, "\n"), strings.TrimLeft(gunicornConfig, "\n")}
 
 	script := strings.Join([]string{
-		// Use the initContainer to create this path to avoid the error noted here:
+		// Create the config directory so Kubernetes can mount it later.
 		// - https://issue.k8s.io/121294
-		`mkdir -p ` + configMountPath,
+		shell.MakeDirectories(0o775, scriptMountPath, configMountPath),
+
+		// Create the logs directory with g+rwx so the OTel Collector can
+		// write to it as well.
+		// TODO(log-rotation): Move the last segment into the Collector startup.
+		shell.MakeDirectories(0o775, dataMountPath, path.Join(LogDirectoryAbsolutePath, "receiver")),
+
 		// Write the system and server configurations.
 		`echo "$1" > ` + scriptMountPath + `/config_system.py`,
 		`echo "$2" > ` + scriptMountPath + `/gunicorn_config.py`,