Skip to content

Commit 9128747

Browse files
authored
Merge branch 'main' into benjb/pgbackrest-rotate
2 parents 7e3178c + bfd4160 commit 9128747

File tree

18 files changed

+247
-101
lines changed

18 files changed

+247
-101
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ require (
2929
k8s.io/component-base v0.31.0
3030
k8s.io/kube-openapi v0.0.0-20240521193020-835d969ad83a
3131
sigs.k8s.io/controller-runtime v0.19.3
32+
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd
3233
sigs.k8s.io/yaml v1.4.0
3334
)
3435

@@ -123,6 +124,5 @@ require (
123124
k8s.io/klog/v2 v2.130.1 // indirect
124125
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
125126
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
126-
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
127127
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
128128
)

internal/bridge/installation_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ import (
1818
corev1 "k8s.io/api/core/v1"
1919
corev1apply "k8s.io/client-go/applyconfigurations/core/v1"
2020
"sigs.k8s.io/controller-runtime/pkg/client"
21-
"sigs.k8s.io/yaml"
2221

2322
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
2423
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
24+
"github.com/crunchydata/postgres-operator/internal/testing/require"
2525
)
2626

2727
func TestExtractSecretContract(t *testing.T) {
@@ -136,7 +136,7 @@ func TestInstallationReconcile(t *testing.T) {
136136
assert.Assert(t, cmp.Contains(applies[0], `"kind":"Secret"`))
137137

138138
var decoded corev1.Secret
139-
assert.NilError(t, yaml.Unmarshal([]byte(applies[0]), &decoded))
139+
require.UnmarshalInto(t, &decoded, applies[0])
140140
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"id":"abc"`))
141141
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"secret":"xyz"`))
142142
})
@@ -230,7 +230,7 @@ func TestInstallationReconcile(t *testing.T) {
230230
assert.Assert(t, cmp.Contains(applies[0], `"kind":"Secret"`))
231231

232232
var decoded corev1.Secret
233-
assert.NilError(t, yaml.Unmarshal([]byte(applies[0]), &decoded))
233+
require.UnmarshalInto(t, &decoded, applies[0])
234234
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"id":"asdf"`))
235235
})
236236
}
@@ -326,7 +326,7 @@ func TestInstallationReconcile(t *testing.T) {
326326
assert.Assert(t, cmp.Contains(applies[0], `"kind":"Secret"`))
327327

328328
var decoded corev1.Secret
329-
assert.NilError(t, yaml.Unmarshal([]byte(applies[0]), &decoded))
329+
require.UnmarshalInto(t, &decoded, applies[0])
330330
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"id":"xyz"`))
331331
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"secret":"def"`))
332332
})
@@ -373,7 +373,7 @@ func TestInstallationReconcile(t *testing.T) {
373373
assert.Assert(t, cmp.Contains(applies[0], `"kind":"Secret"`))
374374

375375
var decoded corev1.Secret
376-
assert.NilError(t, yaml.Unmarshal([]byte(applies[0]), &decoded))
376+
require.UnmarshalInto(t, &decoded, applies[0])
377377
assert.Equal(t, len(decoded.Data["bridge-token"]), 0)
378378

379379
archived := string(decoded.Data["bridge-token--2020-10-28"])
@@ -463,7 +463,7 @@ func TestInstallationReconcile(t *testing.T) {
463463
assert.Assert(t, cmp.Contains(applies[0], `"kind":"Secret"`))
464464

465465
var decoded corev1.Secret
466-
assert.NilError(t, yaml.Unmarshal([]byte(applies[0]), &decoded))
466+
require.UnmarshalInto(t, &decoded, applies[0])
467467
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"id":"ddd"`))
468468
assert.Assert(t, cmp.Contains(string(decoded.Data["bridge-token"]), `"secret":"fresh"`))
469469
})

internal/collector/config.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ func (c *Config) ToYAML() (string, error) {
8888
func NewConfig(spec *v1beta1.InstrumentationSpec) *Config {
8989
config := &Config{
9090
Exporters: map[ComponentID]any{
91-
// TODO: Do we want a DebugExporter outside of development?
9291
// https://pkg.go.dev/go.opentelemetry.io/collector/exporter/debugexporter#section-readme
9392
DebugExporter: map[string]any{"verbosity": "detailed"},
9493
},

internal/collector/instance.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package collector
66

77
import (
88
"context"
9+
"fmt"
910

1011
corev1 "k8s.io/api/core/v1"
1112

@@ -17,6 +18,8 @@ import (
1718
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1819
)
1920

21+
const configDirectory = "/etc/otel-collector"
22+
2023
// AddToConfigMap populates the shared ConfigMap with fields needed to run the Collector.
2124
func AddToConfigMap(
2225
ctx context.Context,
@@ -52,7 +55,7 @@ func AddToPod(
5255
// Create volume and volume mount for otel collector config
5356
configVolumeMount := corev1.VolumeMount{
5457
Name: "collector-config",
55-
MountPath: "/etc/otel-collector",
58+
MountPath: configDirectory,
5659
ReadOnly: true,
5760
}
5861
configVolume := corev1.Volume{Name: configVolumeMount.Name}
@@ -102,7 +105,7 @@ func AddToPod(
102105
Value: sqlQueryPassword,
103106
},
104107
},
105-
108+
Resources: spec.Resources,
106109
SecurityContext: initialize.RestrictedSecurityContext(),
107110
VolumeMounts: append(volumeMounts, configVolumeMount),
108111
}
@@ -156,17 +159,35 @@ func startCommand(dir string, includeLogrotate bool) []string {
156159
`
157160

158161
if includeLogrotate {
159-
startScript = `
160-
/otelcol-contrib --config /etc/otel-collector/config.yaml &
162+
logrotateCommand = `logrotate -s /tmp/logrotate.status /etc/logrotate.d/logrotate.conf`
163+
}
164+
165+
var startScript = fmt.Sprintf(`
166+
OTEL_PIDFILE=/tmp/otel.pid
167+
168+
start_otel_collector() {
169+
echo "Starting OTel Collector"
170+
/otelcol-contrib --config %s/config.yaml &
171+
echo $! > $OTEL_PIDFILE
172+
}
173+
start_otel_collector
161174
162175
exec {fd}<> <(:||:)
163176
while read -r -t 5 -u "${fd}" ||:; do
164-
logrotate -s /tmp/logrotate.status /etc/logrotate.d/logrotate.conf
177+
%s
178+
if [[ "${directory}" -nt "/proc/self/fd/${fd}" ]] && kill -HUP $(head -1 ${OTEL_PIDFILE?});
179+
then
180+
echo "OTel configuration changed..."
181+
exec {fd}>&- && exec {fd}<> <(:||:)
182+
stat --format='Loaded configuration dated %%y' "${directory}"
183+
fi
184+
if [[ ! -e /proc/$(head -1 ${OTEL_PIDFILE?}) ]] ; then
185+
start_otel_collector
186+
fi
165187
done
166-
`
167-
}
188+
`, configDirectory, logrotateCommand)
168189

169-
wrapper := `monitor() {` + mkdirScript + startScript + `}; export -f monitor; exec -a "$0" bash -ceu monitor`
190+
wrapper := `monitor() {` + mkdirScript + startScript + `}; export directory="$1"; export -f monitor; exec -a "$0" bash -ceu monitor`
170191

171-
return []string{"bash", "-ceu", "--", wrapper, "collector"}
192+
return []string{"bash", "-ceu", "--", wrapper, "collector", configDirectory}
172193
}

internal/collector/pgadmin_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@ import (
1010

1111
"gotest.tools/v3/assert"
1212
corev1 "k8s.io/api/core/v1"
13-
"sigs.k8s.io/yaml"
1413

1514
"github.com/crunchydata/postgres-operator/internal/collector"
1615
pgadmin "github.com/crunchydata/postgres-operator/internal/controller/standalone_pgadmin"
1716
"github.com/crunchydata/postgres-operator/internal/feature"
1817
"github.com/crunchydata/postgres-operator/internal/initialize"
1918
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
19+
"github.com/crunchydata/postgres-operator/internal/testing/require"
2020
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2121
)
2222

@@ -125,7 +125,7 @@ collector.yaml: |
125125
ctx := feature.NewContext(context.Background(), gate)
126126

127127
var spec v1beta1.InstrumentationSpec
128-
assert.NilError(t, yaml.Unmarshal([]byte(`{
128+
require.UnmarshalInto(t, &spec, `{
129129
config: {
130130
exporters: {
131131
googlecloud: {
@@ -135,7 +135,7 @@ collector.yaml: |
135135
},
136136
},
137137
logs: { exporters: [googlecloud] },
138-
}`), &spec))
138+
}`)
139139

140140
configmap := new(corev1.ConfigMap)
141141
initialize.Map(&configmap.Data)

internal/collector/postgres_logs_transforms.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@
161161
# https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/attributes-registry/db.md
162162
- set(attributes["db.namespace"], body["dbname"]) where IsString(body["dbname"])
163163
- set(attributes["db.response.status_code"], body["state_code"]) where IsString(body["state_code"])
164-
# TODO(benjb): discuss db.query.summary, db.query.text
165164

166165
# Postgres is multiprocess so some client/backend details align here.
167166
#

internal/config/config_test.go

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
"testing"
1010

1111
"gotest.tools/v3/assert"
12-
"sigs.k8s.io/yaml"
1312

13+
"github.com/crunchydata/postgres-operator/internal/testing/require"
1414
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1515
)
1616

@@ -54,7 +54,7 @@ func TestFetchKeyCommand(t *testing.T) {
5454

5555
t.Run("blank", func(t *testing.T) {
5656
var spec1 v1beta1.PostgresClusterSpec
57-
assert.NilError(t, yaml.Unmarshal([]byte(`{
57+
require.UnmarshalInto(t, &spec1, `{
5858
patroni: {
5959
dynamicConfiguration: {
6060
postgresql: {
@@ -64,23 +64,23 @@ func TestFetchKeyCommand(t *testing.T) {
6464
},
6565
},
6666
},
67-
}`), &spec1))
67+
}`)
6868
assert.Equal(t, "", FetchKeyCommand(&spec1))
6969

7070
var spec2 v1beta1.PostgresClusterSpec
71-
assert.NilError(t, yaml.Unmarshal([]byte(`{
71+
require.UnmarshalInto(t, &spec2, `{
7272
config: {
7373
parameters: {
7474
encryption_key_command: "",
7575
},
7676
},
77-
}`), &spec2))
77+
}`)
7878
assert.Equal(t, "", FetchKeyCommand(&spec2))
7979
})
8080

8181
t.Run("exists", func(t *testing.T) {
8282
var spec1 v1beta1.PostgresClusterSpec
83-
assert.NilError(t, yaml.Unmarshal([]byte(`{
83+
require.UnmarshalInto(t, &spec1, `{
8484
patroni: {
8585
dynamicConfiguration: {
8686
postgresql: {
@@ -90,23 +90,23 @@ func TestFetchKeyCommand(t *testing.T) {
9090
},
9191
},
9292
},
93-
}`), &spec1))
93+
}`)
9494
assert.Equal(t, "echo mykey", FetchKeyCommand(&spec1))
9595

9696
var spec2 v1beta1.PostgresClusterSpec
97-
assert.NilError(t, yaml.Unmarshal([]byte(`{
97+
require.UnmarshalInto(t, &spec2, `{
9898
config: {
9999
parameters: {
100100
encryption_key_command: "cat somefile",
101101
},
102102
},
103-
}`), &spec2))
103+
}`)
104104
assert.Equal(t, "cat somefile", FetchKeyCommand(&spec2))
105105
})
106106

107107
t.Run("config.parameters takes precedence", func(t *testing.T) {
108108
var spec v1beta1.PostgresClusterSpec
109-
assert.NilError(t, yaml.Unmarshal([]byte(`{
109+
require.UnmarshalInto(t, &spec, `{
110110
config: {
111111
parameters: {
112112
encryption_key_command: "cat somefile",
@@ -121,7 +121,7 @@ func TestFetchKeyCommand(t *testing.T) {
121121
},
122122
},
123123
},
124-
}`), &spec))
124+
}`)
125125
assert.Equal(t, "cat somefile", FetchKeyCommand(&spec))
126126
})
127127
}
@@ -139,9 +139,9 @@ func TestPGAdminContainerImage(t *testing.T) {
139139
t.Setenv("RELATED_IMAGE_PGADMIN", "env-var-pgadmin")
140140
assert.Equal(t, PGAdminContainerImage(cluster), "env-var-pgadmin")
141141

142-
assert.NilError(t, yaml.Unmarshal([]byte(`{
142+
require.UnmarshalInto(t, &cluster.Spec, `{
143143
userInterface: { pgAdmin: { image: spec-image } },
144-
}`), &cluster.Spec))
144+
}`)
145145
assert.Equal(t, PGAdminContainerImage(cluster), "spec-image")
146146
}
147147

@@ -158,9 +158,9 @@ func TestPGBackRestContainerImage(t *testing.T) {
158158
t.Setenv("RELATED_IMAGE_PGBACKREST", "env-var-pgbackrest")
159159
assert.Equal(t, PGBackRestContainerImage(cluster), "env-var-pgbackrest")
160160

161-
assert.NilError(t, yaml.Unmarshal([]byte(`{
162-
backups: { pgBackRest: { image: spec-image } },
163-
}`), &cluster.Spec))
161+
require.UnmarshalInto(t, &cluster.Spec, `{
162+
backups: { pgbackrest: { image: spec-image } },
163+
}`)
164164
assert.Equal(t, PGBackRestContainerImage(cluster), "spec-image")
165165
}
166166

@@ -177,9 +177,9 @@ func TestPGBouncerContainerImage(t *testing.T) {
177177
t.Setenv("RELATED_IMAGE_PGBOUNCER", "env-var-pgbouncer")
178178
assert.Equal(t, PGBouncerContainerImage(cluster), "env-var-pgbouncer")
179179

180-
assert.NilError(t, yaml.Unmarshal([]byte(`{
180+
require.UnmarshalInto(t, &cluster.Spec, `{
181181
proxy: { pgBouncer: { image: spec-image } },
182-
}`), &cluster.Spec))
182+
}`)
183183
assert.Equal(t, PGBouncerContainerImage(cluster), "spec-image")
184184
}
185185

@@ -196,9 +196,9 @@ func TestPGExporterContainerImage(t *testing.T) {
196196
t.Setenv("RELATED_IMAGE_PGEXPORTER", "env-var-pgexporter")
197197
assert.Equal(t, PGExporterContainerImage(cluster), "env-var-pgexporter")
198198

199-
assert.NilError(t, yaml.Unmarshal([]byte(`{
200-
monitoring: { pgMonitor: { exporter: { image: spec-image } } },
201-
}`), &cluster.Spec))
199+
require.UnmarshalInto(t, &cluster.Spec, `{
200+
monitoring: { pgmonitor: { exporter: { image: spec-image } } },
201+
}`)
202202
assert.Equal(t, PGExporterContainerImage(cluster), "spec-image")
203203
}
204204

@@ -215,9 +215,9 @@ func TestStandalonePGAdminContainerImage(t *testing.T) {
215215
t.Setenv("RELATED_IMAGE_STANDALONE_PGADMIN", "env-var-pgadmin")
216216
assert.Equal(t, StandalonePGAdminContainerImage(pgadmin), "env-var-pgadmin")
217217

218-
assert.NilError(t, yaml.Unmarshal([]byte(`{
218+
require.UnmarshalInto(t, &pgadmin.Spec, `{
219219
image: spec-image
220-
}`), &pgadmin.Spec))
220+
}`)
221221
assert.Equal(t, StandalonePGAdminContainerImage(pgadmin), "spec-image")
222222
}
223223

internal/controller/pgupgrade/jobs_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/crunchydata/postgres-operator/internal/feature"
2020
"github.com/crunchydata/postgres-operator/internal/initialize"
2121
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
22+
"github.com/crunchydata/postgres-operator/internal/testing/require"
2223
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2324
)
2425

@@ -54,7 +55,7 @@ func TestLargestWholeCPU(t *testing.T) {
5455
} {
5556
t.Run(tt.Name, func(t *testing.T) {
5657
var resources corev1.ResourceRequirements
57-
assert.NilError(t, yaml.Unmarshal([]byte(tt.ResourcesYAML), &resources))
58+
require.UnmarshalInto(t, &resources, tt.ResourcesYAML)
5859
assert.Equal(t, tt.Result, largestWholeCPU(resources))
5960
})
6061
}
@@ -383,8 +384,7 @@ func TestPGUpgradeContainerImage(t *testing.T) {
383384
t.Setenv("RELATED_IMAGE_PGUPGRADE", "env-var-pgbackrest")
384385
assert.Equal(t, pgUpgradeContainerImage(upgrade), "env-var-pgbackrest")
385386

386-
assert.NilError(t, yaml.Unmarshal(
387-
[]byte(`{ image: spec-image }`), &upgrade.Spec))
387+
require.UnmarshalInto(t, &upgrade.Spec, `{ image: spec-image }`)
388388
assert.Equal(t, pgUpgradeContainerImage(upgrade), "spec-image")
389389
}
390390

0 commit comments

Comments
 (0)