Skip to content

Commit 16d2ccc

Browse files
dsessler7dsessler7
committed
Adjust pgbouncer log rotation to use retentionPeriod API. Make logrotate config code more component-agnostic. Other adjustments.
1 parent c35c5f8 commit 16d2ccc

File tree

12 files changed

+194
-214
lines changed

12 files changed

+194
-214
lines changed

‎internal/collector/config.go‎

Lines changed: 42 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
package collector
66

77
import (
8+
"context"
89
_ "embed"
9-
"errors"
1010
"fmt"
11-
"regexp"
11+
"math"
1212

13+
corev1 "k8s.io/api/core/v1"
1314
"k8s.io/apimachinery/pkg/util/sets"
1415
"sigs.k8s.io/yaml"
1516

@@ -114,62 +115,52 @@ func NewConfig(spec *v1beta1.InstrumentationSpec) *Config {
114115
return config
115116
}
116117

117-
func generateLogrotateConfig(logFilePath string, retentionPeriod string, postrotateScript string) (string, error) {
118-
retentionPeriodMap, err := parseRetentionPeriodForLogrotate(retentionPeriod)
119-
if err != nil {
120-
return "", err
118+
// AddLogrotateConfig generates a logrotate configuration and adds it to the
119+
// provided configmap
120+
func AddLogrotateConfig(ctx context.Context, spec *v1beta1.InstrumentationSpec,
121+
outInstanceConfigMap *corev1.ConfigMap, logFilePath, postrotateScript string,
122+
) {
123+
var logrotateConfig string
124+
if outInstanceConfigMap.Data == nil {
125+
outInstanceConfigMap.Data = make(map[string]string)
121126
}
122127

128+
if spec != nil && spec.Logs != nil && spec.Logs.RetentionPeriod != nil {
129+
logrotateConfig = generateLogrotateConfig(logFilePath, spec.Logs.RetentionPeriod,
130+
postrotateScript)
131+
}
132+
133+
outInstanceConfigMap.Data["logrotate.conf"] = logrotateConfig
134+
}
135+
136+
// generateLogrotateConfig generates a configuration string for logrotate based
137+
// on the provided full log file path, retention period, and postrotate script
138+
func generateLogrotateConfig(logFilePath string, retentionPeriod *v1beta1.Duration,
139+
postrotateScript string,
140+
) string {
141+
number, interval := parseDurationForLogrotate(retentionPeriod)
142+
123143
return fmt.Sprintf(
124144
logrotateConfigFormatString,
125145
logFilePath,
126-
retentionPeriodMap["number"],
127-
retentionPeriodMap["interval"],
146+
number,
147+
interval,
128148
postrotateScript,
129-
), err
149+
)
130150
}
131151

132-
func parseRetentionPeriodForLogrotate(retentionPeriod string) (map[string]string, error) {
133-
// logrotate does not have an "hourly" interval, but if an interval is not
134-
// set it will rotate whenever logrotate is called, so set an empty string
135-
// in the config file for hourly
136-
unitIntervalMap := map[string]string{
137-
"h": "",
138-
"hr": "",
139-
"hour": "",
140-
"d": "daily",
141-
"day": "daily",
142-
"w": "weekly",
143-
"wk": "weekly",
144-
"week": "weekly",
145-
}
146-
147-
// Define duration regex and capture matches
148-
durationMatcher := regexp.MustCompile(`(?P<number>\d+)\s*(?P<interval>[A-Za-zµ]+)`)
149-
matches := durationMatcher.FindStringSubmatch(retentionPeriod)
150-
151-
// If three matches were not found (whole match and two submatch captures),
152-
// the retentionPeriod format must be invalid. Return an error.
153-
if len(matches) < 3 {
154-
return nil, errors.New("invalid retentionPeriod; must be number of hours, days, or weeks")
155-
}
156-
157-
// Create a map with "number" and "interval" keys
158-
retentionPeriodMap := make(map[string]string)
159-
for i, name := range durationMatcher.SubexpNames() {
160-
if i > 0 && i <= len(matches) {
161-
retentionPeriodMap[name] = matches[i]
162-
}
152+
// FIXME: If the rotate count is 0, logrotate will not keep any archives, it will
153+
// essentially just empty out the working file during rotation, but it will still
154+
// rotate on the interval specified (hourly, daily, weekly, etc). If the user
155+
// sets the retentionPeriod to 0, we currently default to hourly regardless of
156+
// what unit the user provides. E.g. If the user sets the retentionPeriod to "0w"
157+
// they might expect that it will rotate once a week, but not keep any archives,
158+
// but we will actually rotate every hour... Is there a way to get the unit
159+
// provided in the Duration??
160+
func parseDurationForLogrotate(retentionPeriod *v1beta1.Duration) (int, string) {
161+
hours := math.Round(retentionPeriod.Duration.Hours())
162+
if hours < 24 {
163+
return int(hours), "hourly"
163164
}
164-
165-
// If the duration unit provided is found in the unitIntervalMap, set the
166-
// "interval" value to the "interval" string expected by logrotate; otherwise,
167-
// return an error
168-
if _, exists := unitIntervalMap[retentionPeriodMap["interval"]]; exists {
169-
retentionPeriodMap["interval"] = unitIntervalMap[retentionPeriodMap["interval"]]
170-
} else {
171-
return nil, fmt.Errorf("invalid retentionPeriod; %s is not a valid unit", retentionPeriodMap["interval"])
172-
}
173-
174-
return retentionPeriodMap, nil
165+
return int(math.Round(hours / 24)), "daily"
175166
}

‎internal/collector/config_test.go‎

Lines changed: 64 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"testing"
99

1010
"gotest.tools/v3/assert"
11+
12+
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1113
)
1214

1315
func TestConfigToYAML(t *testing.T) {
@@ -62,110 +64,67 @@ service:
6264
})
6365
}
6466

65-
func TestParseRetentionPeriodForLogrotate(t *testing.T) {
66-
t.Run("SuccessfulParse", func(t *testing.T) {
67-
for _, tt := range []struct {
68-
retentionPeriod string
69-
retentionPeriodMap map[string]string
70-
}{
71-
{
72-
retentionPeriod: "12h",
73-
retentionPeriodMap: map[string]string{
74-
"number": "12",
75-
"interval": "",
76-
},
77-
},
78-
{
79-
retentionPeriod: "24hr",
80-
retentionPeriodMap: map[string]string{
81-
"number": "24",
82-
"interval": "",
83-
},
84-
},
85-
{
86-
retentionPeriod: "36hour",
87-
retentionPeriodMap: map[string]string{
88-
"number": "36",
89-
"interval": "",
90-
},
91-
},
92-
{
93-
retentionPeriod: "3d",
94-
retentionPeriodMap: map[string]string{
95-
"number": "3",
96-
"interval": "daily",
97-
},
98-
},
99-
{
100-
retentionPeriod: "365day",
101-
retentionPeriodMap: map[string]string{
102-
"number": "365",
103-
"interval": "daily",
104-
},
105-
},
106-
{
107-
retentionPeriod: "1w",
108-
retentionPeriodMap: map[string]string{
109-
"number": "1",
110-
"interval": "weekly",
111-
},
112-
},
113-
{
114-
retentionPeriod: "4wk",
115-
retentionPeriodMap: map[string]string{
116-
"number": "4",
117-
"interval": "weekly",
118-
},
119-
},
120-
{
121-
retentionPeriod: "52week",
122-
retentionPeriodMap: map[string]string{
123-
"number": "52",
124-
"interval": "weekly",
125-
},
126-
},
127-
} {
128-
t.Run(tt.retentionPeriod, func(t *testing.T) {
129-
rpm, err := parseRetentionPeriodForLogrotate(tt.retentionPeriod)
130-
assert.NilError(t, err)
131-
assert.Equal(t, tt.retentionPeriodMap["number"], rpm["number"])
132-
})
133-
}
134-
})
67+
// TODO: write this test after rebasing on new retention API changes.
68+
// func TestGenerateLogrotateConfig(t *testing.T) {
13569

136-
t.Run("UnsuccessfulParse", func(t *testing.T) {
137-
for _, tt := range []struct {
138-
retentionPeriod string
139-
errMessage string
140-
}{
141-
{
142-
retentionPeriod: "",
143-
errMessage: "invalid retentionPeriod; must be number of hours, days, or weeks",
144-
},
145-
{
146-
retentionPeriod: "asdf",
147-
errMessage: "invalid retentionPeriod; must be number of hours, days, or weeks",
148-
},
149-
{
150-
retentionPeriod: "1234",
151-
errMessage: "invalid retentionPeriod; must be number of hours, days, or weeks",
152-
},
153-
{
154-
retentionPeriod: "d2",
155-
errMessage: "invalid retentionPeriod; must be number of hours, days, or weeks",
156-
},
157-
{
158-
retentionPeriod: "1000z",
159-
errMessage: "invalid retentionPeriod; z is not a valid unit",
160-
},
161-
} {
162-
t.Run(tt.retentionPeriod, func(t *testing.T) {
163-
rpm, err := parseRetentionPeriodForLogrotate(tt.retentionPeriod)
164-
assert.Assert(t, rpm == nil)
165-
assert.Assert(t, err != nil)
166-
assert.ErrorContains(t, err, tt.errMessage)
167-
// assert.Equal(t, tt.retentionPeriodMap["number"], rpm["number"])
168-
})
169-
}
170-
})
70+
// }
71+
72+
// FIXME: This test is currently broken. Fix after rebasing on new
73+
// retention API changes.
74+
func TestParseDurationForLogrotate(t *testing.T) {
75+
for _, tt := range []struct {
76+
retentionPeriod string
77+
number int
78+
interval string
79+
}{
80+
{
81+
retentionPeriod: "12h",
82+
number: 12,
83+
interval: "hourly",
84+
},
85+
{
86+
retentionPeriod: "24hr",
87+
number: 1,
88+
interval: "daily",
89+
},
90+
{
91+
retentionPeriod: "36hour",
92+
number: 2,
93+
interval: "daily",
94+
},
95+
{
96+
retentionPeriod: "3d",
97+
number: 3,
98+
interval: "daily",
99+
},
100+
{
101+
retentionPeriod: "365day",
102+
number: 365,
103+
interval: "daily",
104+
},
105+
{
106+
retentionPeriod: "1w",
107+
number: 7,
108+
interval: "daily",
109+
},
110+
{
111+
retentionPeriod: "4wk",
112+
number: 28,
113+
interval: "daily",
114+
},
115+
{
116+
retentionPeriod: "52week",
117+
number: 364,
118+
interval: "daily",
119+
},
120+
} {
121+
t.Run(tt.retentionPeriod, func(t *testing.T) {
122+
var duration *v1beta1.Duration
123+
err := duration.UnmarshalJSON([]byte(tt.retentionPeriod))
124+
assert.NilError(t, err)
125+
number, interval := parseDurationForLogrotate(duration)
126+
assert.Equal(t, tt.number, number)
127+
assert.Equal(t, tt.interval, interval)
128+
})
129+
}
171130
}

0 commit comments

Comments
 (0)