diff --git a/internal/collector/config.go b/internal/collector/config.go index d288380aea..8b77177a7a 100644 --- a/internal/collector/config.go +++ b/internal/collector/config.go @@ -153,7 +153,7 @@ func AddLogrotateConfigs(ctx context.Context, spec *v1beta1.InstrumentationSpec, func generateLogrotateConfig( config LogrotateConfig, retentionPeriod metav1.Duration, ) string { - number, interval := parseDurationForLogrotate(retentionPeriod) + number, interval := ParseDurationForLogrotate(retentionPeriod) return fmt.Sprintf( logrotateConfigFormatString, @@ -164,12 +164,12 @@ func generateLogrotateConfig( ) } -// parseDurationForLogrotate takes a retention period and returns the rotate +// ParseDurationForLogrotate takes a retention period and returns the rotate // number and interval string that should be used in the logrotate config. // If the retentionPeriod is less than 24 hours, the function will return the // number of hours and "hourly"; otherwise, we will round up to the nearest day // and return the day count and "daily" -func parseDurationForLogrotate(retentionPeriod metav1.Duration) (int, string) { +func ParseDurationForLogrotate(retentionPeriod metav1.Duration) (int, string) { hours := math.Ceil(retentionPeriod.Hours()) if hours < 24 { return int(hours), "hourly" diff --git a/internal/collector/config_test.go b/internal/collector/config_test.go index c621a14aad..cc37a9b8f7 100644 --- a/internal/collector/config_test.go +++ b/internal/collector/config_test.go @@ -197,7 +197,7 @@ func TestParseDurationForLogrotate(t *testing.T) { t.Run(tt.retentionPeriod, func(t *testing.T) { duration, err := v1beta1.NewDuration(tt.retentionPeriod) assert.NilError(t, err) - number, interval := parseDurationForLogrotate(duration.AsDuration()) + number, interval := ParseDurationForLogrotate(duration.AsDuration()) assert.Equal(t, tt.number, number) assert.Equal(t, tt.interval, interval) }) diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 6d6509eafb..5c9786459d 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1242,7 +1242,7 @@ func (r *Reconciler) reconcileInstance( // add an emptyDir volume to the PodTemplateSpec and an associated '/tmp' volume mount to // all containers included within that spec if err == nil { - addTMPEmptyDir(&instance.Spec.Template) + AddTMPEmptyDir(&instance.Spec.Template) } // mount shared memory to the Postgres instance diff --git a/internal/controller/postgrescluster/pgadmin.go b/internal/controller/postgrescluster/pgadmin.go index 40874aa1be..87d385becd 100644 --- a/internal/controller/postgrescluster/pgadmin.go +++ b/internal/controller/postgrescluster/pgadmin.go @@ -365,7 +365,7 @@ func (r *Reconciler) reconcilePGAdminStatefulSet( // add an emptyDir volume to the PodTemplateSpec and an associated '/tmp' // volume mount to all containers included within that spec - addTMPEmptyDir(&sts.Spec.Template) + AddTMPEmptyDir(&sts.Spec.Template) return errors.WithStack(r.apply(ctx, sts)) } diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index 3645871bd5..54068193af 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -720,7 +720,7 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster postgresCluster.Spec.ImagePullPolicy, &repo.Spec.Template) - addTMPEmptyDir(&repo.Spec.Template) + AddTMPEmptyDir(&repo.Spec.Template) // set ownership references if err := r.setControllerReference(postgresCluster, repo); err != nil { @@ -1272,7 +1272,7 @@ func (r *Reconciler) reconcileRestoreJob(ctx context.Context, cluster.Spec.ImagePullPolicy, &restoreJob.Spec.Template) - addTMPEmptyDir(&restoreJob.Spec.Template) + AddTMPEmptyDir(&restoreJob.Spec.Template) return errors.WithStack(r.apply(ctx, restoreJob)) } diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index 2b1dcae779..d5a935bbf3 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -476,7 +476,7 @@ func (r *Reconciler) generatePGBouncerDeployment( } // Add tmp directory and volume for log files - addTMPEmptyDir(&deploy.Spec.Template) + AddTMPEmptyDir(&deploy.Spec.Template) return deploy, true, err } diff --git a/internal/controller/postgrescluster/snapshots.go b/internal/controller/postgrescluster/snapshots.go index fa168ebdf4..c639408df2 100644 --- a/internal/controller/postgrescluster/snapshots.go +++ b/internal/controller/postgrescluster/snapshots.go @@ -394,7 +394,7 @@ func (r *Reconciler) dedicatedSnapshotVolumeRestore(ctx context.Context, cluster.Spec.ImagePullPolicy, &restoreJob.Spec.Template) - addTMPEmptyDir(&restoreJob.Spec.Template) + AddTMPEmptyDir(&restoreJob.Spec.Template) restoreJob.Annotations[naming.PGBackRestBackupJobCompletion] = backupJob.Status.CompletionTime.Format(time.RFC3339) return errors.WithStack(r.apply(ctx, restoreJob)) diff --git a/internal/controller/postgrescluster/util.go b/internal/controller/postgrescluster/util.go index bb5b3e085a..a1ba6ce087 100644 --- a/internal/controller/postgrescluster/util.go +++ b/internal/controller/postgrescluster/util.go @@ -134,13 +134,13 @@ func addDevSHM(template *corev1.PodTemplateSpec) { } } -// addTMPEmptyDir adds a "tmp" EmptyDir volume to the provided Pod template, while then also adding a +// AddTMPEmptyDir adds a "tmp" EmptyDir volume to the provided Pod template, while then also adding a // volume mount at /tmp for all containers defined within the Pod template // The '/tmp' directory is currently utilized for the following: // - As the pgBackRest lock directory (this is the default lock location for pgBackRest) // - The location where the replication client certificates can be loaded with the proper // permissions set -func addTMPEmptyDir(template *corev1.PodTemplateSpec) { +func AddTMPEmptyDir(template *corev1.PodTemplateSpec) { template.Spec.Volumes = append(template.Spec.Volumes, corev1.Volume{ Name: "tmp", diff --git a/internal/controller/standalone_pgadmin/configmap.go b/internal/controller/standalone_pgadmin/configmap.go index 8382bbb2ca..72a95b14db 100644 --- a/internal/controller/standalone_pgadmin/configmap.go +++ b/internal/controller/standalone_pgadmin/configmap.go @@ -19,6 +19,7 @@ import ( "github.com/pkg/errors" "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/naming" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" @@ -32,7 +33,7 @@ func (r *PGAdminReconciler) reconcilePGAdminConfigMap( ctx context.Context, pgadmin *v1beta1.PGAdmin, clusters map[string][]*v1beta1.PostgresCluster, ) (*corev1.ConfigMap, error) { - configmap, err := configmap(pgadmin, clusters) + configmap, err := configmap(ctx, pgadmin, clusters) if err != nil { return configmap, err } @@ -50,7 +51,7 @@ func (r *PGAdminReconciler) reconcilePGAdminConfigMap( } // configmap returns a v1.ConfigMap for pgAdmin. -func configmap(pgadmin *v1beta1.PGAdmin, +func configmap(ctx context.Context, pgadmin *v1beta1.PGAdmin, clusters map[string][]*v1beta1.PostgresCluster, ) (*corev1.ConfigMap, error) { configmap := &corev1.ConfigMap{ObjectMeta: naming.StandalonePGAdmin(pgadmin)} @@ -63,7 +64,38 @@ func configmap(pgadmin *v1beta1.PGAdmin, // TODO(tjmoore4): Populate configuration details. initialize.Map(&configmap.Data) - configSettings, err := generateConfig(pgadmin) + var ( + logRetention bool + maxBackupRetentionNumber = 1 + // One day in minutes for pgadmin rotation + pgAdminRetentionPeriod = 24 * 60 + // Daily rotation for gunicorn rotation + gunicornRetentionPeriod = "D" + ) + // If OTel logs feature gate is enabled, we want to change the pgAdmin/gunicorn logging + if feature.Enabled(ctx, feature.OpenTelemetryLogs) && pgadmin.Spec.Instrumentation != nil { + logRetention = true + + // If the user has set a retention period, we will use those values for log rotation, + // which is otherwise managed by python. + if pgadmin.Spec.Instrumentation.Logs != nil && + pgadmin.Spec.Instrumentation.Logs.RetentionPeriod != nil { + + retentionNumber, period := collector.ParseDurationForLogrotate(pgadmin.Spec.Instrumentation.Logs.RetentionPeriod.AsDuration()) + // `LOG_ROTATION_MAX_LOG_FILES`` in pgadmin refers to the already rotated logs. + // `backupCount` for gunicorn is similar. + // Our retention unit is for total number of log files, so subtract 1 to account + // for the currently-used log file. + maxBackupRetentionNumber = retentionNumber - 1 + if period == "hourly" { + // If the period is hourly, set the pgadmin + // and gunicorn retention periods to hourly. + pgAdminRetentionPeriod = 60 + gunicornRetentionPeriod = "H" + } + } + } + configSettings, err := generateConfig(pgadmin, logRetention, maxBackupRetentionNumber, pgAdminRetentionPeriod) if err == nil { configmap.Data[settingsConfigMapKey] = configSettings } @@ -73,7 +105,8 @@ func configmap(pgadmin *v1beta1.PGAdmin, configmap.Data[settingsClusterMapKey] = clusterSettings } - gunicornSettings, err := generateGunicornConfig(pgadmin) + gunicornSettings, err := generateGunicornConfig(pgadmin, + logRetention, maxBackupRetentionNumber, gunicornRetentionPeriod) if err == nil { configmap.Data[gunicornConfigKey] = gunicornSettings } @@ -81,8 +114,10 @@ func configmap(pgadmin *v1beta1.PGAdmin, return configmap, err } -// generateConfig generates the config settings for the pgAdmin -func generateConfig(pgadmin *v1beta1.PGAdmin) (string, error) { +// generateConfigs generates the config settings for the pgAdmin and gunicorn +func generateConfig(pgadmin *v1beta1.PGAdmin, + logRetention bool, maxBackupRetentionNumber, pgAdminRetentionPeriod int) ( + string, error) { settings := map[string]any{ // Bind to all IPv4 addresses by default. "0.0.0.0" here represents INADDR_ANY. // - https://flask.palletsprojects.com/en/2.2.x/api/#flask.Flask.run @@ -102,6 +137,22 @@ func generateConfig(pgadmin *v1beta1.PGAdmin) (string, error) { settings["UPGRADE_CHECK_ENABLED"] = false settings["UPGRADE_CHECK_URL"] = "" settings["UPGRADE_CHECK_KEY"] = "" + settings["DATA_DIR"] = dataMountPath + settings["LOG_FILE"] = LogFileAbsolutePath + + if logRetention { + settings["LOG_ROTATION_AGE"] = pgAdminRetentionPeriod + settings["LOG_ROTATION_MAX_LOG_FILES"] = maxBackupRetentionNumber + settings["JSON_LOGGER"] = true + settings["CONSOLE_LOG_LEVEL"] = "WARNING" + settings["FILE_LOG_LEVEL"] = "INFO" + settings["FILE_LOG_FORMAT_JSON"] = map[string]string{ + "time": "created", + "name": "name", + "level": "levelname", + "message": "message", + } + } // To avoid spurious reconciles, the following value must not change when // the spec does not change. [json.Encoder] and [json.Marshal] do this by @@ -185,7 +236,9 @@ func generateClusterConfig( // generateGunicornConfig generates the config settings for the gunicorn server // - https://docs.gunicorn.org/en/latest/settings.html -func generateGunicornConfig(pgadmin *v1beta1.PGAdmin) (string, error) { +func generateGunicornConfig(pgadmin *v1beta1.PGAdmin, + logRetention bool, maxBackupRetentionNumber int, gunicornRetentionPeriod string, +) (string, error) { settings := map[string]any{ // Bind to all IPv4 addresses and set 25 threads by default. // - https://docs.gunicorn.org/en/latest/settings.html#bind @@ -202,6 +255,69 @@ func generateGunicornConfig(pgadmin *v1beta1.PGAdmin) (string, error) { // Write mandatory settings over any specified ones. // - https://docs.gunicorn.org/en/latest/settings.html#workers settings["workers"] = 1 + // Gunicorn logging dict settings + logSettings := map[string]any{} + + // If OTel logs feature gate is enabled, we want to change the gunicorn logging + if logRetention { + + // 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/ + // We override the gunicorn defaults (using `logconfig_dict`) to set our own file handler. + // - https://docs.gunicorn.org/en/stable/settings.html#logconfig-dict + // - https://github.com/benoitc/gunicorn/blob/23.0.0/gunicorn/glogging.py#L47 + logSettings = map[string]any{ + + "loggers": map[string]any{ + "gunicorn.access": map[string]any{ + "handlers": []string{"file"}, + "level": "INFO", + "propagate": true, + "qualname": "gunicorn.access", + }, + "gunicorn.error": map[string]any{ + "handlers": []string{"file"}, + "level": "INFO", + "propagate": true, + "qualname": "gunicorn.error", + }, + }, + "handlers": map[string]any{ + "file": map[string]any{ + "class": "logging.handlers.TimedRotatingFileHandler", + "filename": GunicornLogFileAbsolutePath, + "backupCount": maxBackupRetentionNumber, + "interval": 1, + "when": gunicornRetentionPeriod, + "formatter": "json", + }, + "console": map[string]any{ + "class": "logging.StreamHandler", + "formatter": "generic", + "stream": "ext://sys.stdout", + }, + }, + "formatters": map[string]any{ + "generic": map[string]any{ + "class": "logging.Formatter", + "datefmt": "[%Y-%m-%d %H:%M:%S %z]", + "format": "%(asctime)s [%(process)d] [%(levelname)s] %(message)s", + }, + "json": map[string]any{ + "class": "jsonformatter.JsonFormatter", + "separators": []string{",", ":"}, + "format": map[string]string{ + "time": "created", + "name": "name", + "level": "levelname", + "message": "message", + }, + }, + }, + } + } + settings["logconfig_dict"] = logSettings // To avoid spurious reconciles, the following value must not change when // the spec does not change. [json.Encoder] and [json.Marshal] do this by diff --git a/internal/controller/standalone_pgadmin/configmap_test.go b/internal/controller/standalone_pgadmin/configmap_test.go index b2a93ac2de..a23ee08d18 100644 --- a/internal/controller/standalone_pgadmin/configmap_test.go +++ b/internal/controller/standalone_pgadmin/configmap_test.go @@ -5,6 +5,7 @@ package standalone_pgadmin import ( + "context" "testing" "gotest.tools/v3/assert" @@ -19,11 +20,13 @@ func TestGenerateConfig(t *testing.T) { t.Run("Default", func(t *testing.T) { pgadmin := new(v1beta1.PGAdmin) - result, err := generateConfig(pgadmin) + result, err := generateConfig(pgadmin, false, 0, 0) assert.NilError(t, err) assert.Equal(t, result, `{ + "DATA_DIR": "/var/lib/pgadmin", "DEFAULT_SERVER": "0.0.0.0", + "LOG_FILE": "/var/lib/pgadmin/logs/pgadmin.log", "SERVER_MODE": true, "UPGRADE_CHECK_ENABLED": false, "UPGRADE_CHECK_KEY": "", @@ -37,11 +40,13 @@ func TestGenerateConfig(t *testing.T) { "SERVER_MODE": false, "UPGRADE_CHECK_ENABLED": true, } - result, err := generateConfig(pgadmin) + result, err := generateConfig(pgadmin, false, 0, 0) assert.NilError(t, err) assert.Equal(t, result, `{ + "DATA_DIR": "/var/lib/pgadmin", "DEFAULT_SERVER": "0.0.0.0", + "LOG_FILE": "/var/lib/pgadmin/logs/pgadmin.log", "SERVER_MODE": true, "UPGRADE_CHECK_ENABLED": false, "UPGRADE_CHECK_KEY": "", @@ -55,7 +60,7 @@ func TestGenerateConfig(t *testing.T) { "ALLOWED_HOSTS": []any{"225.0.0.0/8", "226.0.0.0/7", "228.0.0.0/6"}, "DEFAULT_SERVER": "::", } - result, err := generateConfig(pgadmin) + result, err := generateConfig(pgadmin, false, 0, 0) assert.NilError(t, err) assert.Equal(t, result, `{ @@ -64,7 +69,41 @@ func TestGenerateConfig(t *testing.T) { "226.0.0.0/7", "228.0.0.0/6" ], + "DATA_DIR": "/var/lib/pgadmin", "DEFAULT_SERVER": "::", + "LOG_FILE": "/var/lib/pgadmin/logs/pgadmin.log", + "SERVER_MODE": true, + "UPGRADE_CHECK_ENABLED": false, + "UPGRADE_CHECK_KEY": "", + "UPGRADE_CHECK_URL": "" +}`+"\n") + }) + + t.Run("OTel enabled", func(t *testing.T) { + pgadmin := new(v1beta1.PGAdmin) + require.UnmarshalInto(t, &pgadmin.Spec, `{ + instrumentation: { + logs: { retentionPeriod: 5h }, + }, + }`) + result, err := generateConfig(pgadmin, true, 4, 60) + + assert.NilError(t, err) + assert.Equal(t, result, `{ + "CONSOLE_LOG_LEVEL": "WARNING", + "DATA_DIR": "/var/lib/pgadmin", + "DEFAULT_SERVER": "0.0.0.0", + "FILE_LOG_FORMAT_JSON": { + "level": "levelname", + "message": "message", + "name": "name", + "time": "created" + }, + "FILE_LOG_LEVEL": "INFO", + "JSON_LOGGER": true, + "LOG_FILE": "/var/lib/pgadmin/logs/pgadmin.log", + "LOG_ROTATION_AGE": 60, + "LOG_ROTATION_MAX_LOG_FILES": 4, "SERVER_MODE": true, "UPGRADE_CHECK_ENABLED": false, "UPGRADE_CHECK_KEY": "", @@ -161,10 +200,11 @@ func TestGeneratePGAdminConfigMap(t *testing.T) { pgadmin.Namespace = "some-ns" pgadmin.Name = "pg1" clusters := map[string][]*v1beta1.PostgresCluster{} + ctx := context.Background() t.Run("Data,ObjectMeta,TypeMeta", func(t *testing.T) { pgadmin := pgadmin.DeepCopy() - configmap, err := configmap(pgadmin, clusters) + configmap, err := configmap(ctx, pgadmin, clusters) assert.NilError(t, err) assert.Assert(t, cmp.MarshalMatches(configmap.TypeMeta, ` @@ -190,7 +230,7 @@ namespace: some-ns Labels: map[string]string{"c": "v3", "d": "v4"}, } - configmap, err := configmap(pgadmin, clusters) + configmap, err := configmap(ctx, pgadmin, clusters) assert.NilError(t, err) // Annotations present in the metadata. @@ -217,11 +257,12 @@ func TestGenerateGunicornConfig(t *testing.T) { expectedString := `{ "bind": "0.0.0.0:5050", + "logconfig_dict": {}, "threads": 25, "workers": 1 } ` - actualString, err := generateGunicornConfig(pgAdmin) + actualString, err := generateGunicornConfig(pgAdmin, false, 0, "H") assert.NilError(t, err) assert.Equal(t, actualString, expectedString) }) @@ -239,11 +280,12 @@ func TestGenerateGunicornConfig(t *testing.T) { "bind": "0.0.0.0:5050", "certfile": "/path/to/certfile", "keyfile": "/path/to/keyfile", + "logconfig_dict": {}, "threads": 25, "workers": 1 } ` - actualString, err := generateGunicornConfig(pgAdmin) + actualString, err := generateGunicornConfig(pgAdmin, false, 0, "H") assert.NilError(t, err) assert.Equal(t, actualString, expectedString) }) @@ -259,11 +301,12 @@ func TestGenerateGunicornConfig(t *testing.T) { expectedString := `{ "bind": "127.0.0.1:5051", + "logconfig_dict": {}, "threads": 30, "workers": 1 } ` - actualString, err := generateGunicornConfig(pgAdmin) + actualString, err := generateGunicornConfig(pgAdmin, false, 0, "H") assert.NilError(t, err) assert.Equal(t, actualString, expectedString) }) @@ -278,11 +321,89 @@ func TestGenerateGunicornConfig(t *testing.T) { expectedString := `{ "bind": "0.0.0.0:5050", + "logconfig_dict": {}, + "threads": 25, + "workers": 1 +} +` + actualString, err := generateGunicornConfig(pgAdmin, false, 0, "H") + assert.NilError(t, err) + assert.Equal(t, actualString, expectedString) + }) + + t.Run("OTel enabled", func(t *testing.T) { + pgAdmin := &v1beta1.PGAdmin{} + pgAdmin.Name = "test" + pgAdmin.Namespace = "postgres-operator" + require.UnmarshalInto(t, &pgAdmin.Spec, `{ + instrumentation: { + logs: { retentionPeriod: 5h }, + }, + }`) + actualString, err := generateGunicornConfig(pgAdmin, true, 4, "H") + + expectedString := `{ + "bind": "0.0.0.0:5050", + "logconfig_dict": { + "formatters": { + "generic": { + "class": "logging.Formatter", + "datefmt": "[%Y-%m-%d %H:%M:%S %z]", + "format": "%(asctime)s [%(process)d] [%(levelname)s] %(message)s" + }, + "json": { + "class": "jsonformatter.JsonFormatter", + "format": { + "level": "levelname", + "message": "message", + "name": "name", + "time": "created" + }, + "separators": [ + ",", + ":" + ] + } + }, + "handlers": { + "console": { + "class": "logging.StreamHandler", + "formatter": "generic", + "stream": "ext://sys.stdout" + }, + "file": { + "backupCount": 4, + "class": "logging.handlers.TimedRotatingFileHandler", + "filename": "/var/lib/pgadmin/logs/gunicorn.log", + "formatter": "json", + "interval": 1, + "when": "H" + } + }, + "loggers": { + "gunicorn.access": { + "handlers": [ + "file" + ], + "level": "INFO", + "propagate": true, + "qualname": "gunicorn.access" + }, + "gunicorn.error": { + "handlers": [ + "file" + ], + "level": "INFO", + "propagate": true, + "qualname": "gunicorn.error" + } + } + }, "threads": 25, "workers": 1 } ` - actualString, err := generateGunicornConfig(pgAdmin) + assert.NilError(t, err) assert.Equal(t, actualString, expectedString) }) diff --git a/internal/controller/standalone_pgadmin/pod.go b/internal/controller/standalone_pgadmin/pod.go index 7590a3a3cc..aa6fdde481 100644 --- a/internal/controller/standalone_pgadmin/pod.go +++ b/internal/controller/standalone_pgadmin/pod.go @@ -7,7 +7,6 @@ package standalone_pgadmin import ( "context" "fmt" - "path" "strings" corev1 "k8s.io/api/core/v1" @@ -84,15 +83,6 @@ func pod( }, } - // create a temp volume for restart pid/other/debugging use - // TODO: discuss tmp vol vs. persistent vol - tmpVolume := corev1.Volume{Name: "tmp"} - tmpVolume.VolumeSource = corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{ - Medium: corev1.StorageMediumMemory, - }, - } - // pgadmin container container := corev1.Container{ Name: naming.ContainerPGAdmin, @@ -140,10 +130,6 @@ func pod( MountPath: scriptMountPath, ReadOnly: true, }, - { - Name: tmpVolume.Name, - MountPath: "/tmp", - }, }, } @@ -192,7 +178,6 @@ func pod( configVolume, dataVolume, scriptVolume, - tmpVolume, } outPod.Containers = []corev1.Container{container} outPod.InitContainers = []corev1.Container{startup} @@ -277,7 +262,8 @@ func startupScript(pgadmin *v1beta1.PGAdmin) []string { // startCommands (v8 image includes Gunicorn) var startCommandV7 = "pgadmin4 &" - var startCommandV8 = "gunicorn -c /etc/pgadmin/gunicorn_config.py --chdir $PGADMIN_DIR pgAdmin4:app &" + var startCommandV8 = "gunicorn -c /etc/pgadmin/gunicorn_config.py" + + " --chdir $PGADMIN_DIR pgAdmin4:app &" // This script sets up, starts pgadmin, and runs the appropriate `loadServerCommand` to register the discovered servers. // pgAdmin is hosted by Gunicorn and uses a config file. @@ -325,10 +311,15 @@ loadServerCommand // descriptor and uses the timeout of the builtin `read` to wait. That same // descriptor gets closed and reopened to use the builtin `[ -nt` to check mtimes. // - https://unix.stackexchange.com/a/407383 + // In order to get gunicorn to reload the logging config + // we need to send a KILL rather than a HUP signal. + // - https://github.com/benoitc/gunicorn/issues/3353 + // Right now the config file is on the same configMap as the cluster file + // so if the mtime changes for any of those files, it will change for all. var reloadScript = ` exec {fd}<> <(:||:) while read -r -t 5 -u "${fd}" ||:; do - if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand + if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -KILL $(head -1 ${PGADMIN4_PIDFILE?}); then exec {fd}>&- && exec {fd}<> <(:||:) stat --format='Loaded shared servers dated %y' "${cluster_file}" @@ -375,10 +366,10 @@ func startupCommand() []string { // configDatabaseURIPath is the path for mounting the database URI connection string configDatabaseURIPathAbsolutePath = configMountPath + "/" + configDatabaseURIPath - // The constants set in configSystem will not be overridden through + // The values set in configSystem will not be overridden through // spec.config.settings. configSystem = ` -import glob, json, re, os, logging +import glob, json, re, os DEFAULT_BINARY_PATHS = {'pg': sorted([''] + glob.glob('/usr/pgsql-*/bin')).pop()} with open('` + configMountPath + `/` + configFilePath + `') as _f: _conf, _data = re.compile(r'[A-Z_0-9]+'), json.load(_f) @@ -390,18 +381,8 @@ if os.path.isfile('` + ldapPasswordAbsolutePath + `'): if os.path.isfile('` + configDatabaseURIPathAbsolutePath + `'): with open('` + configDatabaseURIPathAbsolutePath + `') as _f: CONFIG_DATABASE_URI = _f.read() - -DATA_DIR = '` + dataMountPath + `' -LOG_FILE = '` + LogFileAbsolutePath + `' -LOG_ROTATION_AGE = 24 * 60 # minutes -LOG_ROTATION_SIZE = 5 # MiB -LOG_ROTATION_MAX_LOG_FILES = 1 - -JSON_LOGGER = True -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 // after all other config files. // - https://docs.gunicorn.org/en/latest/configure.html#configuration-file @@ -412,37 +393,13 @@ FILE_LOG_FORMAT_JSON = {'time': 'created', 'name': 'name', 'level': 'levelname', // // 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: - // https://docs.python.org/3/library/logging.html#logrecord-attributes. - // JsonFormatter is used to format the log: https://pypi.org/project/jsonformatter/ gunicornConfig = ` -import json, re, collections, copy, gunicorn, gunicorn.glogging +import json, re, gunicorn +gunicorn.SERVER_SOFTWARE = 'Python' 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': '` + GunicornLogFileAbsolutePath + `', - 'backupCount': 1, 'maxBytes': 2 << 20, # MiB - 'formatter': 'json', -} -logconfig_dict['formatters']['json'] = { - 'class': 'jsonformatter.JsonFormatter', - 'separators': (',', ':'), - 'format': collections.OrderedDict([ - ('time', 'created'), - ('name', 'name'), - ('level', 'levelname'), - ('message', 'message'), - ]) -} ` ) @@ -453,10 +410,8 @@ logconfig_dict['formatters']['json'] = { // - https://issue.k8s.io/121294 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")), + // Create the logs directory with g+rwx to ensure pgAdmin can write to it as well. + shell.MakeDirectories(0o775, dataMountPath, LogDirectoryAbsolutePath), // Write the system and server configurations. `echo "$1" > ` + scriptMountPath + `/config_system.py`, diff --git a/internal/controller/standalone_pgadmin/pod_test.go b/internal/controller/standalone_pgadmin/pod_test.go index ce3ad076d2..501624bc1f 100644 --- a/internal/controller/standalone_pgadmin/pod_test.go +++ b/internal/controller/standalone_pgadmin/pod_test.go @@ -74,7 +74,7 @@ containers: exec {fd}<> <(:||:) while read -r -t 5 -u "${fd}" ||:; do - if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand + if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -KILL $(head -1 ${PGADMIN4_PIDFILE?}); then exec {fd}>&- && exec {fd}<> <(:||:) stat --format='Loaded shared servers dated %y' "${cluster_file}" @@ -130,8 +130,6 @@ containers: - mountPath: /etc/pgadmin name: pgadmin-config-system readOnly: true - - mountPath: /tmp - name: tmp initContainers: - command: - bash @@ -139,12 +137,12 @@ initContainers: - -- - |- mkdir -p '/etc/pgadmin/conf.d' && chmod 0775 '/etc/pgadmin/conf.d' - mkdir -p '/var/lib/pgadmin/logs/receiver' && chmod 0775 '/var/lib/pgadmin/logs/receiver' '/var/lib/pgadmin/logs' + mkdir -p '/var/lib/pgadmin/logs' && chmod 0775 '/var/lib/pgadmin/logs' echo "$1" > /etc/pgadmin/config_system.py echo "$2" > /etc/pgadmin/gunicorn_config.py - startup - | - import glob, json, re, os, logging + import glob, json, re, os DEFAULT_BINARY_PATHS = {'pg': sorted([''] + glob.glob('/usr/pgsql-*/bin')).pop()} with open('/etc/pgadmin/conf.d/~postgres-operator/pgadmin-settings.json') as _f: _conf, _data = re.compile(r'[A-Z_0-9]+'), json.load(_f) @@ -156,44 +154,13 @@ initContainers: if os.path.isfile('/etc/pgadmin/conf.d/~postgres-operator/config-database-uri'): with open('/etc/pgadmin/conf.d/~postgres-operator/config-database-uri') as _f: CONFIG_DATABASE_URI = _f.read() - - DATA_DIR = '/var/lib/pgadmin' - LOG_FILE = '/var/lib/pgadmin/logs/pgadmin.log' - LOG_ROTATION_AGE = 24 * 60 # minutes - LOG_ROTATION_SIZE = 5 # MiB - LOG_ROTATION_MAX_LOG_FILES = 1 - - JSON_LOGGER = True - CONSOLE_LOG_LEVEL = logging.WARNING - FILE_LOG_LEVEL = logging.INFO - FILE_LOG_FORMAT_JSON = {'time': 'created', 'name': 'name', 'level': 'levelname', 'message': 'message'} - | - import json, re, collections, copy, gunicorn, gunicorn.glogging + import json, re, gunicorn + gunicorn.SERVER_SOFTWARE = 'Python' with open('/etc/pgadmin/conf.d/~postgres-operator/gunicorn-config.json') 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', - 'backupCount': 1, 'maxBytes': 2 << 20, # MiB - 'formatter': 'json', - } - logconfig_dict['formatters']['json'] = { - 'class': 'jsonformatter.JsonFormatter', - 'separators': (',', ':'), - 'format': collections.OrderedDict([ - ('time', 'created'), - ('name', 'name'), - ('level', 'levelname'), - ('message', 'message'), - ]) - } name: pgadmin-startup resources: {} securityContext: @@ -230,9 +197,6 @@ volumes: medium: Memory sizeLimit: 32Ki name: pgadmin-config-system -- emptyDir: - medium: Memory - name: tmp `)) // No change when called again. @@ -247,6 +211,13 @@ volumes: pgadmin.Spec.Resources.Requests = corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("100m"), } + retentionPeriod, err := v1beta1.NewDuration("12 hours") + assert.NilError(t, err) + pgadmin.Spec.Instrumentation = &v1beta1.InstrumentationSpec{ + Logs: &v1beta1.InstrumentationLogsSpec{ + RetentionPeriod: retentionPeriod, + }, + } call() @@ -289,7 +260,7 @@ containers: exec {fd}<> <(:||:) while read -r -t 5 -u "${fd}" ||:; do - if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand + if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -KILL $(head -1 ${PGADMIN4_PIDFILE?}); then exec {fd}>&- && exec {fd}<> <(:||:) stat --format='Loaded shared servers dated %y' "${cluster_file}" @@ -349,8 +320,6 @@ containers: - mountPath: /etc/pgadmin name: pgadmin-config-system readOnly: true - - mountPath: /tmp - name: tmp initContainers: - command: - bash @@ -358,12 +327,12 @@ initContainers: - -- - |- mkdir -p '/etc/pgadmin/conf.d' && chmod 0775 '/etc/pgadmin/conf.d' - mkdir -p '/var/lib/pgadmin/logs/receiver' && chmod 0775 '/var/lib/pgadmin/logs/receiver' '/var/lib/pgadmin/logs' + mkdir -p '/var/lib/pgadmin/logs' && chmod 0775 '/var/lib/pgadmin/logs' echo "$1" > /etc/pgadmin/config_system.py echo "$2" > /etc/pgadmin/gunicorn_config.py - startup - | - import glob, json, re, os, logging + import glob, json, re, os DEFAULT_BINARY_PATHS = {'pg': sorted([''] + glob.glob('/usr/pgsql-*/bin')).pop()} with open('/etc/pgadmin/conf.d/~postgres-operator/pgadmin-settings.json') as _f: _conf, _data = re.compile(r'[A-Z_0-9]+'), json.load(_f) @@ -375,44 +344,13 @@ initContainers: if os.path.isfile('/etc/pgadmin/conf.d/~postgres-operator/config-database-uri'): with open('/etc/pgadmin/conf.d/~postgres-operator/config-database-uri') as _f: CONFIG_DATABASE_URI = _f.read() - - DATA_DIR = '/var/lib/pgadmin' - LOG_FILE = '/var/lib/pgadmin/logs/pgadmin.log' - LOG_ROTATION_AGE = 24 * 60 # minutes - LOG_ROTATION_SIZE = 5 # MiB - LOG_ROTATION_MAX_LOG_FILES = 1 - - JSON_LOGGER = True - CONSOLE_LOG_LEVEL = logging.WARNING - FILE_LOG_LEVEL = logging.INFO - FILE_LOG_FORMAT_JSON = {'time': 'created', 'name': 'name', 'level': 'levelname', 'message': 'message'} - | - import json, re, collections, copy, gunicorn, gunicorn.glogging + import json, re, gunicorn + gunicorn.SERVER_SOFTWARE = 'Python' with open('/etc/pgadmin/conf.d/~postgres-operator/gunicorn-config.json') 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', - 'backupCount': 1, 'maxBytes': 2 << 20, # MiB - 'formatter': 'json', - } - logconfig_dict['formatters']['json'] = { - 'class': 'jsonformatter.JsonFormatter', - 'separators': (',', ':'), - 'format': collections.OrderedDict([ - ('time', 'created'), - ('name', 'name'), - ('level', 'levelname'), - ('message', 'message'), - ]) - } image: new-image imagePullPolicy: Always name: pgadmin-startup @@ -453,9 +391,6 @@ volumes: medium: Memory sizeLimit: 32Ki name: pgadmin-config-system -- emptyDir: - medium: Memory - name: tmp `)) }) } diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go index 2c9a17595d..c75668defc 100644 --- a/internal/controller/standalone_pgadmin/statefulset.go +++ b/internal/controller/standalone_pgadmin/statefulset.go @@ -16,6 +16,7 @@ import ( "github.com/pkg/errors" "github.com/crunchydata/postgres-operator/internal/collector" + "github.com/crunchydata/postgres-operator/internal/controller/postgrescluster" "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" @@ -121,8 +122,9 @@ func statefulset( pod(pgadmin, configmap, &sts.Spec.Template.Spec, dataVolume) - if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if pgadmin.Spec.Instrumentation != nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) { // Logs for gunicorn and pgadmin write to /var/lib/pgadmin/logs + // so the collector needs access to that that path. dataVolumeMount := corev1.VolumeMount{ Name: "pgadmin-data", MountPath: "/var/lib/pgadmin", @@ -132,8 +134,10 @@ func statefulset( } collector.AddToPod(ctx, pgadmin.Spec.Instrumentation, pgadmin.Spec.ImagePullPolicy, - configmap, &sts.Spec.Template.Spec, volumeMounts, "", []string{}, false) + configmap, &sts.Spec.Template.Spec, volumeMounts, "", []string{LogDirectoryAbsolutePath}, false) } + postgrescluster.AddTMPEmptyDir(&sts.Spec.Template) + return sts }