Skip to content

Commit 3a69d47

Browse files
benjaminjbbenjaminjb
committed
Move the number generate out of the cm generate
1 parent d2aad56 commit 3a69d47

File tree

2 files changed

+53
-83
lines changed

2 files changed

+53
-83
lines changed

‎internal/controller/standalone_pgadmin/configmap.go‎

Lines changed: 43 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,38 @@ func configmap(ctx context.Context, pgadmin *v1beta1.PGAdmin,
6464

6565
// TODO(tjmoore4): Populate configuration details.
6666
initialize.Map(&configmap.Data)
67-
configSettings, err := generateConfig(ctx, pgadmin)
67+
var (
68+
logRetention bool
69+
maxBackupRetentionNumber = 1
70+
// One day in minutes for pgadmin rotation
71+
pgAdminRetentionPeriod = 24 * 60
72+
// Daily rotation for gunicorn rotation
73+
gunicornRetentionPeriod = "D"
74+
)
75+
// If OTel logs feature gate is enabled, we want to change the pgAdmin/gunicorn logging
76+
if feature.Enabled(ctx, feature.OpenTelemetryLogs) && pgadmin.Spec.Instrumentation != nil {
77+
logRetention = true
78+
79+
// If the user has set a retention period, we will use those values for log rotation,
80+
// which is otherwise managed by python.
81+
if pgadmin.Spec.Instrumentation.Logs != nil &&
82+
pgadmin.Spec.Instrumentation.Logs.RetentionPeriod != nil {
83+
84+
retentionNumber, period := collector.ParseDurationForLogrotate(pgadmin.Spec.Instrumentation.Logs.RetentionPeriod.AsDuration())
85+
// `LOG_ROTATION_MAX_LOG_FILES`` in pgadmin refers to the already rotated logs.
86+
// `backupCount` for gunicorn is similar.
87+
// Our retention unit is for total number of log files, so subtract 1 to account
88+
// for the currently-used log file.
89+
maxBackupRetentionNumber = retentionNumber - 1
90+
if period == "hourly" {
91+
// If the period is hourly, set the pgadmin
92+
// and gunicorn retention periods to hourly.
93+
pgAdminRetentionPeriod = 60
94+
gunicornRetentionPeriod = "H"
95+
}
96+
}
97+
}
98+
configSettings, err := generateConfig(pgadmin, logRetention, maxBackupRetentionNumber, pgAdminRetentionPeriod)
6899
if err == nil {
69100
configmap.Data[settingsConfigMapKey] = configSettings
70101
}
@@ -74,7 +105,8 @@ func configmap(ctx context.Context, pgadmin *v1beta1.PGAdmin,
74105
configmap.Data[settingsClusterMapKey] = clusterSettings
75106
}
76107

77-
gunicornSettings, gunicornLoggingSettings, err := generateGunicornConfig(ctx, pgadmin)
108+
gunicornSettings, gunicornLoggingSettings, err := generateGunicornConfig(pgadmin,
109+
logRetention, maxBackupRetentionNumber, gunicornRetentionPeriod)
78110
if err == nil {
79111
configmap.Data[gunicornConfigKey] = gunicornSettings
80112
configmap.Data[gunicornLoggingConfigKey] = gunicornLoggingSettings
@@ -84,7 +116,9 @@ func configmap(ctx context.Context, pgadmin *v1beta1.PGAdmin,
84116
}
85117

86118
// generateConfigs generates the config settings for the pgAdmin and gunicorn
87-
func generateConfig(ctx context.Context, pgadmin *v1beta1.PGAdmin) (string, error) {
119+
func generateConfig(pgadmin *v1beta1.PGAdmin,
120+
logRetention bool, maxBackupRetentionNumber, pgAdminRetentionPeriod int) (
121+
string, error) {
88122
settings := map[string]any{
89123
// Bind to all IPv4 addresses by default. "0.0.0.0" here represents INADDR_ANY.
90124
// - https://flask.palletsprojects.com/en/2.2.x/api/#flask.Flask.run
@@ -107,33 +141,7 @@ func generateConfig(ctx context.Context, pgadmin *v1beta1.PGAdmin) (string, erro
107141
settings["DATA_DIR"] = dataMountPath
108142
settings["LOG_FILE"] = LogFileAbsolutePath
109143

110-
// If OTel logs feature gate is enabled, we want to change the pgAdmin/gunicorn logging
111-
if feature.Enabled(ctx, feature.OpenTelemetryLogs) && pgadmin.Spec.Instrumentation != nil {
112-
113-
var (
114-
maxBackupRetentionNumber = 1
115-
// One day in minutes for pgadmin rotation
116-
pgAdminRetentionPeriod = 24 * 60
117-
)
118-
119-
// If the user has set a retention period, we will use those values for log rotation,
120-
// which is otherwise managed by python.
121-
if pgadmin.Spec.Instrumentation.Logs != nil &&
122-
pgadmin.Spec.Instrumentation.Logs.RetentionPeriod != nil {
123-
124-
retentionNumber, period := collector.ParseDurationForLogrotate(pgadmin.Spec.Instrumentation.Logs.RetentionPeriod.AsDuration())
125-
// `LOG_ROTATION_MAX_LOG_FILES`` in pgadmin refers to the already rotated logs.
126-
// `backupCount` for gunicorn is similar.
127-
// Our retention unit is for total number of log files, so subtract 1 to account
128-
// for the currently-used log file.
129-
maxBackupRetentionNumber = retentionNumber - 1
130-
if period == "hourly" {
131-
// If the period is hourly, set the pgadmin
132-
// and gunicorn retention periods to hourly.
133-
pgAdminRetentionPeriod = 60
134-
}
135-
}
136-
144+
if logRetention {
137145
settings["LOG_ROTATION_AGE"] = pgAdminRetentionPeriod
138146
settings["LOG_ROTATION_MAX_LOG_FILES"] = maxBackupRetentionNumber
139147
settings["JSON_LOGGER"] = true
@@ -229,9 +237,9 @@ func generateClusterConfig(
229237

230238
// generateGunicornConfig generates the config settings for the gunicorn server
231239
// - https://docs.gunicorn.org/en/latest/settings.html
232-
func generateGunicornConfig(ctx context.Context, pgadmin *v1beta1.PGAdmin) (
233-
string, string, error,
234-
) {
240+
func generateGunicornConfig(pgadmin *v1beta1.PGAdmin,
241+
logRetention bool, maxBackupRetentionNumber int, gunicornRetentionPeriod string,
242+
) (string, string, error) {
235243
settings := map[string]any{
236244
// Bind to all IPv4 addresses and set 25 threads by default.
237245
// - https://docs.gunicorn.org/en/latest/settings.html#bind
@@ -266,33 +274,8 @@ func generateGunicornConfig(ctx context.Context, pgadmin *v1beta1.PGAdmin) (
266274
// Gunicorn logging dict settings
267275
logSettings := map[string]any{}
268276

269-
// If OTel logs feature gate is enabled, we want to change the pgAdmin/gunicorn logging
270-
if feature.Enabled(ctx, feature.OpenTelemetryLogs) &&
271-
pgadmin.Spec.Instrumentation != nil {
272-
273-
var (
274-
maxBackupRetentionNumber = "1"
275-
// Daily rotation for gunicorn rotation
276-
gunicornRetentionPeriod = "D"
277-
)
278-
279-
// If the user has set a retention period, we will use those values for log rotation,
280-
// which is otherwise managed by python.
281-
if pgadmin.Spec.Instrumentation.Logs != nil &&
282-
pgadmin.Spec.Instrumentation.Logs.RetentionPeriod != nil {
283-
284-
retentionNumber, period := collector.ParseDurationForLogrotate(pgadmin.Spec.Instrumentation.Logs.RetentionPeriod.AsDuration())
285-
// `LOG_ROTATION_MAX_LOG_FILES`` in pgadmin refers to the already rotated logs.
286-
// `backupCount` for gunicorn is similar.
287-
// Our retention unit is for total number of log files, so subtract 1 to account
288-
// for the currently-used log file.
289-
maxBackupRetentionNumber = strconv.Itoa(retentionNumber - 1)
290-
if period == "hourly" {
291-
// If the period is hourly, set the pgadmin
292-
// and gunicorn retention periods to hourly.
293-
gunicornRetentionPeriod = "H"
294-
}
295-
}
277+
// If OTel logs feature gate is enabled, we want to change the gunicorn logging
278+
if logRetention {
296279

297280
// Gunicorn uses the Python logging package, which sets the following attributes:
298281
// https://docs.python.org/3/library/logging.html#logrecord-attributes.

‎internal/controller/standalone_pgadmin/configmap_test.go‎

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,17 @@ import (
1111

1212
"gotest.tools/v3/assert"
1313

14-
"github.com/crunchydata/postgres-operator/internal/feature"
1514
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
1615
"github.com/crunchydata/postgres-operator/internal/testing/require"
1716
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1817
)
1918

2019
func TestGenerateConfig(t *testing.T) {
2120
require.ParallelCapacity(t, 0)
22-
ctx := context.Background()
2321

2422
t.Run("Default", func(t *testing.T) {
2523
pgadmin := new(v1beta1.PGAdmin)
26-
result, err := generateConfig(ctx, pgadmin)
24+
result, err := generateConfig(pgadmin, false, 0, 0)
2725

2826
assert.NilError(t, err)
2927
assert.Equal(t, result, `{
@@ -43,7 +41,7 @@ func TestGenerateConfig(t *testing.T) {
4341
"SERVER_MODE": false,
4442
"UPGRADE_CHECK_ENABLED": true,
4543
}
46-
result, err := generateConfig(ctx, pgadmin)
44+
result, err := generateConfig(pgadmin, false, 0, 0)
4745

4846
assert.NilError(t, err)
4947
assert.Equal(t, result, `{
@@ -63,7 +61,7 @@ func TestGenerateConfig(t *testing.T) {
6361
"ALLOWED_HOSTS": []any{"225.0.0.0/8", "226.0.0.0/7", "228.0.0.0/6"},
6462
"DEFAULT_SERVER": "::",
6563
}
66-
result, err := generateConfig(ctx, pgadmin)
64+
result, err := generateConfig(pgadmin, false, 0, 0)
6765

6866
assert.NilError(t, err)
6967
assert.Equal(t, result, `{
@@ -83,18 +81,13 @@ func TestGenerateConfig(t *testing.T) {
8381
})
8482

8583
t.Run("OTel enabled", func(t *testing.T) {
86-
gate := feature.NewGate()
87-
assert.NilError(t, gate.SetFromMap(map[string]bool{
88-
feature.OpenTelemetryLogs: true,
89-
}))
90-
ctx := feature.NewContext(context.Background(), gate)
9184
pgadmin := new(v1beta1.PGAdmin)
9285
require.UnmarshalInto(t, &pgadmin.Spec, `{
9386
instrumentation: {
9487
logs: { retentionPeriod: 5h },
9588
},
9689
}`)
97-
result, err := generateConfig(ctx, pgadmin)
90+
result, err := generateConfig(pgadmin, true, 4, 60)
9891

9992
assert.NilError(t, err)
10093
assert.Equal(t, result, `{
@@ -257,7 +250,6 @@ namespace: some-ns
257250

258251
func TestGenerateGunicornConfig(t *testing.T) {
259252
require.ParallelCapacity(t, 0)
260-
ctx := context.Background()
261253

262254
t.Run("Default", func(t *testing.T) {
263255
pgAdmin := &v1beta1.PGAdmin{}
@@ -270,7 +262,7 @@ func TestGenerateGunicornConfig(t *testing.T) {
270262
"workers": 1
271263
}
272264
`
273-
actualString, logString, err := generateGunicornConfig(ctx, pgAdmin)
265+
actualString, logString, err := generateGunicornConfig(pgAdmin, false, 0, "H")
274266
assert.NilError(t, err)
275267
assert.Equal(t, actualString, expectedString)
276268
assert.Assert(t, strings.Contains(logString, "{}"))
@@ -293,7 +285,7 @@ func TestGenerateGunicornConfig(t *testing.T) {
293285
"workers": 1
294286
}
295287
`
296-
actualString, logString, err := generateGunicornConfig(ctx, pgAdmin)
288+
actualString, logString, err := generateGunicornConfig(pgAdmin, false, 0, "H")
297289
assert.NilError(t, err)
298290
assert.Equal(t, actualString, expectedString)
299291
assert.Assert(t, strings.Contains(logString, "{}"))
@@ -314,7 +306,7 @@ func TestGenerateGunicornConfig(t *testing.T) {
314306
"workers": 1
315307
}
316308
`
317-
actualString, logString, err := generateGunicornConfig(ctx, pgAdmin)
309+
actualString, logString, err := generateGunicornConfig(pgAdmin, false, 0, "H")
318310
assert.NilError(t, err)
319311
assert.Equal(t, actualString, expectedString)
320312
assert.Assert(t, strings.Contains(logString, "{}"))
@@ -334,18 +326,13 @@ func TestGenerateGunicornConfig(t *testing.T) {
334326
"workers": 1
335327
}
336328
`
337-
actualString, logString, err := generateGunicornConfig(ctx, pgAdmin)
329+
actualString, logString, err := generateGunicornConfig(pgAdmin, false, 0, "H")
338330
assert.NilError(t, err)
339331
assert.Equal(t, actualString, expectedString)
340332
assert.Assert(t, strings.Contains(logString, "{}"))
341333
})
342334

343335
t.Run("OTel enabled", func(t *testing.T) {
344-
gate := feature.NewGate()
345-
assert.NilError(t, gate.SetFromMap(map[string]bool{
346-
feature.OpenTelemetryLogs: true,
347-
}))
348-
ctx := feature.NewContext(context.Background(), gate)
349336
pgAdmin := &v1beta1.PGAdmin{}
350337
pgAdmin.Name = "test"
351338
pgAdmin.Namespace = "postgres-operator"
@@ -354,7 +341,7 @@ func TestGenerateGunicornConfig(t *testing.T) {
354341
logs: { retentionPeriod: 5h },
355342
},
356343
}`)
357-
actualString, logString, err := generateGunicornConfig(ctx, pgAdmin)
344+
actualString, logString, err := generateGunicornConfig(pgAdmin, true, 4, "H")
358345

359346
expectedString := `{
360347
"bind": "0.0.0.0:5050",
@@ -390,7 +377,7 @@ func TestGenerateGunicornConfig(t *testing.T) {
390377
"stream": "ext://sys.stdout"
391378
},
392379
"file": {
393-
"backupCount": "4",
380+
"backupCount": 4,
394381
"class": "logging.handlers.TimedRotatingFileHandler",
395382
"filename": "/var/lib/pgadmin/logs/gunicorn.log",
396383
"formatter": "json",

0 commit comments

Comments
 (0)