Add a field specifying when to delete log files

The Kubernetes "duration" format is similar to RFC 3339 and time.Duration
but also handles days and weeks.

Issue: PGO-2197

Author: cbandy

config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml

@@ -1949,6 +1949,18 @@ spec:
                         items:
                           type: string
                         type: array
+                      retentionPeriod:
+                        description: |-
+                          How long to retain log files locally. An RFC 3339 duration or a number
+                          and unit: `3d`, `4 weeks`, `12 hr`, etc.
+                        format: duration
+                        maxLength: 20
+                        minLength: 1
+                        pattern: ^(PT)?( *[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$
+                        type: string
+                        x-kubernetes-validations:
+                        - message: must be at least one hour
+                          rule: duration("1h") <= self && self <= duration("8760h")
                     type: object
                   resources:
                     description: Resources holds the resource requirements for the

config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

@@ -11482,6 +11482,18 @@ spec:
                         items:
                           type: string
                         type: array
+                      retentionPeriod:
+                        description: |-
+                          How long to retain log files locally. An RFC 3339 duration or a number
+                          and unit: `3d`, `4 weeks`, `12 hr`, etc.
+                        format: duration
+                        maxLength: 20
+                        minLength: 1
+                        pattern: ^(PT)?( *[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$
+                        type: string
+                        x-kubernetes-validations:
+                        - message: must be at least one hour
+                          rule: duration("1h") <= self && self <= duration("8760h")
                     type: object
                   resources:
                     description: Resources holds the resource requirements for the

go.mod

@@ -27,6 +27,7 @@ require (
 	k8s.io/apimachinery v0.31.0
 	k8s.io/client-go v0.31.0
 	k8s.io/component-base v0.31.0
+	k8s.io/kube-openapi v0.0.0-20240521193020-835d969ad83a
 	sigs.k8s.io/controller-runtime v0.19.3
 	sigs.k8s.io/yaml v1.4.0
 )
@@ -120,7 +121,6 @@ require (
 	k8s.io/apiextensions-apiserver v0.31.0 // indirect
 	k8s.io/apiserver v0.31.0 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
-	k8s.io/kube-openapi v0.0.0-20240521193020-835d969ad83a // indirect
 	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

internal/testing/validation/pgadmin_test.go

@@ -0,0 +1,95 @@
+// Copyright 2021 - 2025 Crunchy Data Solutions, Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package validation
+
+import (
+	"context"
+	"testing"
+
+	"gotest.tools/v3/assert"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/yaml"
+
+	"github.com/crunchydata/postgres-operator/internal/controller/runtime"
+	"github.com/crunchydata/postgres-operator/internal/testing/require"
+	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
+)
+
+func TestPGAdminInstrumentation(t *testing.T) {
+	ctx := context.Background()
+	cc := require.Kubernetes(t)
+	t.Parallel()
+
+	namespace := require.Namespace(t, cc)
+	base := v1beta1.NewPGAdmin()
+	base.Namespace = namespace.Name
+	base.Name = "pgadmin-instrumentation"
+
+	assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll),
+		"expected this base to be valid")
+
+	t.Run("LogsRetentionPeriod", func(t *testing.T) {
+		pgadmin := base.DeepCopy()
+		assert.NilError(t, yaml.UnmarshalStrict([]byte(`{
+			instrumentation: {
+				logs: { retentionPeriod: 5m },
+			},
+		}`), &pgadmin.Spec))
+
+		err := cc.Create(ctx, pgadmin, client.DryRunAll)
+		assert.Assert(t, apierrors.IsInvalid(err))
+		assert.ErrorContains(t, err, "retentionPeriod")
+		assert.ErrorContains(t, err, "hour|day|week")
+		assert.ErrorContains(t, err, "one hour")
+
+		//nolint:errorlint // This is a test, and a panic is unlikely.
+		status := err.(apierrors.APIStatus).Status()
+		assert.Assert(t, status.Details != nil)
+		assert.Equal(t, len(status.Details.Causes), 2)
+
+		for _, cause := range status.Details.Causes {
+			assert.Equal(t, cause.Field, "spec.instrumentation.logs.retentionPeriod")
+		}
+
+		t.Run("Valid", func(t *testing.T) {
+			for _, tt := range []string{
+				"28 weeks",
+				"90 DAY",
+				"1 hr",
+				"PT1D2H",
+				"1 week 2 days",
+			} {
+				u, err := runtime.ToUnstructuredObject(pgadmin)
+				assert.NilError(t, err)
+				assert.NilError(t, unstructured.SetNestedField(u.Object,
+					tt, "spec", "instrumentation", "logs", "retentionPeriod"))
+
+				assert.NilError(t, cc.Create(ctx, u, client.DryRunAll), tt)
+			}
+		})
+
+		t.Run("Invalid", func(t *testing.T) {
+			for _, tt := range []string{
+				// Amount too small
+				"0 days",
+				"0",
+
+				// Text too long
+				"2 weeks 3 days 4 hours",
+			} {
+				u, err := runtime.ToUnstructuredObject(pgadmin)
+				assert.NilError(t, err)
+				assert.NilError(t, unstructured.SetNestedField(u.Object,
+					tt, "spec", "instrumentation", "logs", "retentionPeriod"))
+
+				err = cc.Create(ctx, u, client.DryRunAll)
+				assert.Assert(t, apierrors.IsInvalid(err), tt)
+				assert.ErrorContains(t, err, "retentionPeriod")
+			}
+		})
+	})
+}

internal/testing/validation/postgrescluster_test.go

@@ -28,7 +28,7 @@ func TestPostgresUserOptions(t *testing.T) {
 	base := v1beta1.NewPostgresCluster()
 
 	// Start with a bunch of required fields.
-	assert.NilError(t, yaml.Unmarshal([]byte(`{
+	assert.NilError(t, yaml.UnmarshalStrict([]byte(`{
 		postgresVersion: 16,
 		backups: {
 			pgbackrest: {

pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go

@@ -52,4 +52,21 @@ type InstrumentationLogsSpec struct {
 	// the logs pipeline.
 	// +optional
 	Exporters []string `json:"exporters,omitempty"`
+
+	// How long to retain log files locally. An RFC 3339 duration or a number
+	// and unit: `3d`, `4 weeks`, `12 hr`, etc.
+	// ---
+	// Kubernetes ensures the value is in the "duration" format, but go ahead
+	// and loosely validate the format to show some acceptable units.
+	// +kubebuilder:validation:Pattern=`^(PT)?( *[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$`
+	//
+	// `controller-gen` needs to know "Type=string" to allow a "Pattern".
+	// +kubebuilder:validation:Type=string
+	//
+	// Set a max length to keep rule costs low.
+	// +kubebuilder:validation:MaxLength=20
+	// +kubebuilder:validation:XValidation:rule=`duration("1h") <= self && self <= duration("8760h")`,message="must be at least one hour"
+	//
+	// +optional
+	RetentionPeriod *Duration `json:"retentionPeriod,omitempty"`
 }

pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go

@@ -5,10 +5,89 @@
 package v1beta1
 
 import (
+	"encoding/json"
+
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/kube-openapi/pkg/validation/strfmt"
 )
 
+// ---
+// Duration represents a string accepted by the Kubernetes API in the "duration"
+// [format]. This format extends the "duration" [defined by OpenAPI] by allowing
+// some whitespace and more units:
+//
+//   - nanoseconds: ns, nano, nanos
+//   - microseconds: us, µs, micro, micros
+//   - milliseconds: ms, milli, millis
+//   - seconds: s, sec, secs
+//   - minutes: m, min, mins
+//   - hours: h, hr, hour, hours
+//   - days: d, day, days
+//   - weeks: w, wk, week, weeks
+//
+// An empty amount is represented as "0" with no unit.
+// One day is always 24 hours and one week is always 7 days (168 hours).
+//
+// +kubebuilder:validation:Format=duration
+// +kubebuilder:validation:MinLength=1
+// +kubebuilder:validation:Type=string
+//
+// During CEL validation, a value of this type is a "google.protobuf.Duration".
+// It is safe to pass the value to `duration()` but not necessary.
+//
+// - https://docs.k8s.io/reference/using-api/cel/#type-system-integration
+// - https://github.com/google/cel-spec/blob/-/doc/langdef.md#types-and-conversions
+//
+// [defined by OpenAPI]: https://spec.openapis.org/registry/format/duration.html
+// [format]: https://spec.openapis.org/oas/latest.html#data-type-format
+type Duration struct {
+	parsed metav1.Duration
+	string
+}
+
+// NewDuration creates a duration from the Kubernetes "duration" format in s.
+func NewDuration(s string) (*Duration, error) {
+	td, err := strfmt.ParseDuration(s)
+
+	// The unkeyed fields here helpfully raise warnings from the compiler
+	// if [metav1.Duration] changes shape in the future.
+	type unkeyed metav1.Duration
+	umd := unkeyed{td}
+
+	return &Duration{metav1.Duration(umd), s}, err
+}
+
+// AsDuration returns d as a [metav1.Duration].
+func (d *Duration) AsDuration() metav1.Duration {
+	return d.parsed
+}
+
+// MarshalJSON implements [encoding/json.Marshaler].
+func (d Duration) MarshalJSON() ([]byte, error) {
+	if d.parsed.Duration == 0 {
+		return json.Marshal("0")
+	}
+
+	return json.Marshal(d.string)
+}
+
+// UnmarshalJSON implements [encoding/json.Unmarshaler].
+func (d *Duration) UnmarshalJSON(data []byte) error {
+	var next *Duration
+	var str string
+
+	err := json.Unmarshal(data, &str)
+	if err == nil {
+		next, err = NewDuration(str)
+	}
+	if err == nil {
+		*d = *next
+	}
+	return err
+}
+
 // SchemalessObject is a map compatible with JSON object.
 //
 // Use with the following markers: