Skip to content

Rotate pgAdmin/gunicorn logs according to retentionPeriod #4101

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.

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

Merged
merged 9 commits into from
Mar 7, 2025
Merged
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions internal/collector/config.go
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 1 addition & 1 deletion internal/collector/config_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/instance.go
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/pgadmin.go
Original file line number Diff line number Diff line change
@@ -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))
}
4 changes: 2 additions & 2 deletions internal/controller/postgrescluster/pgbackrest.go
Original file line number Diff line number Diff line change
@@ -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))
}
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/pgbouncer.go
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/snapshots.go
Original file line number Diff line number Diff line change
@@ -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))
4 changes: 2 additions & 2 deletions internal/controller/postgrescluster/util.go
Original file line number Diff line number Diff line change
@@ -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",
130 changes: 123 additions & 7 deletions internal/controller/standalone_pgadmin/configmap.go
Original file line number Diff line number Diff line change
@@ -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,16 +105,19 @@ 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
}

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
Loading
Oops, something went wrong.