From e4e686588e7016f17ce9dfa544794f1bd09bd17f Mon Sep 17 00:00:00 2001 From: Jociele Padilha <45459238+jopadi@users.noreply.github.com> Date: Fri, 19 Jun 2026 12:12:58 +0200 Subject: [PATCH] Fix/logical backup job cleanup (#3111) * feat(logical-backup): add configurable job history limits and TTL Adds three new configuration options for logical backup cronjobs: - logical_backup_successful_jobs_history_limit (default: 3) - logical_backup_failed_jobs_history_limit (default: 3) - logical_backup_ttl_seconds_after_finished (default: 86400) These options control how many completed/failed backup jobs are retained by Kubernetes and when finished jobs are automatically deleted. This prevents accumulation of old backup jobs and pods in namespaces with many PostgreSQL clusters. Also updates the CronJob comparison logic to detect changes in these new fields and trigger reconciliation when needed. Closes zalando/postgres-operator#1092 * add added the 3 new fieldson crd * updated gen api --------- Co-authored-by: Jairo Llopis Co-authored-by: Felix Kunde --- .../crds/operatorconfigurations.yaml | 12 ++++++++ charts/postgres-operator/values.yaml | 6 ++++ docs/reference/operator_parameters.md | 9 ++++++ manifests/operatorconfiguration.crd.yaml | 12 ++++++++ pkg/apis/acid.zalan.do/v1/crds.go | 9 ++++++ .../v1/operator_configuration_type.go | 3 ++ .../acid.zalan.do/v1/zz_generated.deepcopy.go | 17 ++++++++++- pkg/cluster/cluster.go | 15 ++++++++++ pkg/cluster/cluster_test.go | 11 +++++++- pkg/cluster/k8sres.go | 28 ++++++++++++++++--- pkg/cluster/k8sres_test.go | 26 +++++++++++++++++ pkg/controller/operator_config.go | 3 ++ pkg/util/config/config.go | 3 ++ 13 files changed, 148 insertions(+), 6 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index 09356c476..1414366ff 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -726,6 +726,18 @@ spec: default: "30 00 * * *" logical_backup_cronjob_environment_secret: type: string + logical_backup_failed_jobs_history_limit: + type: integer + minimum: 0 + default: 3 + logical_backup_successful_jobs_history_limit: + type: integer + minimum: 0 + default: 3 + logical_backup_ttl_seconds_after_finished: + type: integer + minimum: 0 + default: 86400 debug: type: object properties: diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 82e9ac342..bb2831dd3 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -415,6 +415,12 @@ configLogicalBackup: logical_backup_schedule: "30 00 * * *" # secret to be used as reference for env variables in cronjob logical_backup_cronjob_environment_secret: "" + # number of successful backup jobs to keep in cronjob history + logical_backup_successful_jobs_history_limit: 3 + # number of failed backup jobs to keep in cronjob history + logical_backup_failed_jobs_history_limit: 3 + # TTL in seconds after which finished backup jobs are automatically deleted + logical_backup_ttl_seconds_after_finished: 86400 # automate creation of human users with teams API service configTeamsApi: diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 332742a16..d3d1fa742 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -904,6 +904,15 @@ grouped under the `logical_backup` key. * **logical_backup_cronjob_environment_secret** Reference to a Kubernetes secret, which keys will be added as environment variables to the cronjob. Default: "" +* **logical_backup_successful_jobs_history_limit** + number of successful backup jobs to keep in cronjob history. The default is `3`. + +* **logical_backup_failed_jobs_history_limit** + number of failed backup jobs to keep in cronjob history. The default is `3`. + +* **logical_backup_ttl_seconds_after_finished** + TTL in seconds after which finished backup jobs are automatically deleted. The default is `86400`. + The following environment variables can be passed to the logical backup cronjob via `logical_backup_cronjob_environment_secret` to control connectivity checks before the backup starts: diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 81bcd4381..fb009c459 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -712,6 +712,18 @@ spec: default: "30 00 * * *" logical_backup_cronjob_environment_secret: type: string + logical_backup_failed_jobs_history_limit: + type: integer + minimum: 0 + default: 3 + logical_backup_successful_jobs_history_limit: + type: integer + minimum: 0 + default: 3 + logical_backup_ttl_seconds_after_finished: + type: integer + minimum: 0 + default: 86400 debug: type: object properties: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 6b43d0c54..867f573b3 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -899,6 +899,15 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ "logical_backup_cronjob_environment_secret": { Type: "string", }, + "logical_backup_successful_jobs_history_limit": { + Type: "integer", + }, + "logical_backup_failed_jobs_history_limit": { + Type: "integer", + }, + "logical_backup_ttl_seconds_after_finished": { + Type: "integer", + }, }, }, "debug": { diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index 3f28effc8..5cec0ff33 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -252,6 +252,9 @@ type OperatorLogicalBackupConfiguration struct { MemoryRequest string `json:"logical_backup_memory_request,omitempty"` CPULimit string `json:"logical_backup_cpu_limit,omitempty"` MemoryLimit string `json:"logical_backup_memory_limit,omitempty"` + SuccessfulJobsHistoryLimit *int32 `json:"logical_backup_successful_jobs_history_limit,omitempty"` + FailedJobsHistoryLimit *int32 `json:"logical_backup_failed_jobs_history_limit,omitempty"` + TTLSecondsAfterFinished *int32 `json:"logical_backup_ttl_seconds_after_finished,omitempty"` } // PatroniConfiguration defines configuration for Patroni diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index ff83abec9..9005b0bbe 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -480,7 +480,7 @@ func (in *OperatorConfigurationData) DeepCopyInto(out *OperatorConfigurationData in.TeamsAPI.DeepCopyInto(&out.TeamsAPI) out.LoggingRESTAPI = in.LoggingRESTAPI out.Scalyr = in.Scalyr - out.LogicalBackup = in.LogicalBackup + in.LogicalBackup.DeepCopyInto(&out.LogicalBackup) in.ConnectionPooler.DeepCopyInto(&out.ConnectionPooler) in.Patroni.DeepCopyInto(&out.Patroni) return @@ -558,6 +558,21 @@ func (in *OperatorDebugConfiguration) DeepCopy() *OperatorDebugConfiguration { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OperatorLogicalBackupConfiguration) DeepCopyInto(out *OperatorLogicalBackupConfiguration) { *out = *in + if in.SuccessfulJobsHistoryLimit != nil { + in, out := &in.SuccessfulJobsHistoryLimit, &out.SuccessfulJobsHistoryLimit + *out = new(int32) + **out = **in + } + if in.FailedJobsHistoryLimit != nil { + in, out := &in.FailedJobsHistoryLimit, &out.FailedJobsHistoryLimit + *out = new(int32) + **out = **in + } + if in.TTLSecondsAfterFinished != nil { + in, out := &in.TTLSecondsAfterFinished, &out.TTLSecondsAfterFinished + *out = new(int32) + **out = **in + } return } diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 6d9e6150a..a1e9f3c4f 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -919,6 +919,21 @@ func (c *Cluster) compareLogicalBackupJob(cur, new *batchv1.CronJob) *compareLog reasons = append(reasons, fmt.Sprintf("logical backup container specs do not match: %v", strings.Join(contReasons, `', '`))) } + if !reflect.DeepEqual(cur.Spec.SuccessfulJobsHistoryLimit, new.Spec.SuccessfulJobsHistoryLimit) { + match = false + reasons = append(reasons, fmt.Sprintf("new job's successfulJobsHistoryLimit %v does not match the current one %v", new.Spec.SuccessfulJobsHistoryLimit, cur.Spec.SuccessfulJobsHistoryLimit)) + } + + if !reflect.DeepEqual(cur.Spec.FailedJobsHistoryLimit, new.Spec.FailedJobsHistoryLimit) { + match = false + reasons = append(reasons, fmt.Sprintf("new job's failedJobsHistoryLimit %v does not match the current one %v", new.Spec.FailedJobsHistoryLimit, cur.Spec.FailedJobsHistoryLimit)) + } + + if !reflect.DeepEqual(cur.Spec.JobTemplate.Spec.TTLSecondsAfterFinished, new.Spec.JobTemplate.Spec.TTLSecondsAfterFinished) { + match = false + reasons = append(reasons, fmt.Sprintf("new job's TTLSecondsAfterFinished %v does not match the current one %v", new.Spec.JobTemplate.Spec.TTLSecondsAfterFinished, cur.Spec.JobTemplate.Spec.TTLSecondsAfterFinished)) + } + return &compareLogicalBackupJobResult{match: match, reasons: reasons, deletedPodAnnotations: deletedPodAnnotations} } diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 5fdf1a220..95a445ff3 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -1567,12 +1567,21 @@ func TestCompareServices(t *testing.T) { } } +var ( + defaultSuccessfulJobsHistoryLimit = int32(3) + defaultFailedJobsHistoryLimit = int32(3) + defaultTTLSecondsAfterFinished = int32(86400) +) + func newCronJob(image, schedule string, vars []v1.EnvVar, mounts []v1.VolumeMount) *batchv1.CronJob { cron := &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: schedule, + Schedule: schedule, + SuccessfulJobsHistoryLimit: &defaultSuccessfulJobsHistoryLimit, + FailedJobsHistoryLimit: &defaultFailedJobsHistoryLimit, JobTemplate: batchv1.JobTemplateSpec{ Spec: batchv1.JobSpec{ + TTLSecondsAfterFinished: &defaultTTLSecondsAfterFinished, Template: v1.PodTemplateSpec{ Spec: v1.PodSpec{ Containers: []v1.Container{ diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 302797dc4..5b668c108 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -2452,7 +2452,13 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1.CronJob, error) { // configure a batch job jobSpec := batchv1.JobSpec{ - Template: *podTemplate, + Template: *podTemplate, + TTLSecondsAfterFinished: c.OpConfig.LogicalBackup.LogicalBackupTTLSecondsAfterFinished, + } + + if jobSpec.TTLSecondsAfterFinished == nil { + defaultTTL := int32(86400) + jobSpec.TTLSecondsAfterFinished = &defaultTTL } // configure a cron job @@ -2470,6 +2476,18 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1.CronJob, error) { schedule = c.OpConfig.LogicalBackupSchedule } + successfulJobsHistoryLimit := c.OpConfig.LogicalBackup.LogicalBackupSuccessfulJobsHistoryLimit + if successfulJobsHistoryLimit == nil { + defaultLimit := int32(3) + successfulJobsHistoryLimit = &defaultLimit + } + + failedJobsHistoryLimit := c.OpConfig.LogicalBackup.LogicalBackupFailedJobsHistoryLimit + if failedJobsHistoryLimit == nil { + defaultLimit := int32(3) + failedJobsHistoryLimit = &defaultLimit + } + cronJob := &batchv1.CronJob{ ObjectMeta: metav1.ObjectMeta{ Name: c.getLogicalBackupJobName(), @@ -2479,9 +2497,11 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1.CronJob, error) { OwnerReferences: c.ownerReferences(), }, Spec: batchv1.CronJobSpec{ - Schedule: schedule, - JobTemplate: jobTemplateSpec, - ConcurrencyPolicy: batchv1.ForbidConcurrent, + Schedule: schedule, + JobTemplate: jobTemplateSpec, + ConcurrencyPolicy: batchv1.ForbidConcurrent, + SuccessfulJobsHistoryLimit: successfulJobsHistoryLimit, + FailedJobsHistoryLimit: failedJobsHistoryLimit, }, } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index bf21b8645..8cbecb2fa 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -4229,6 +4229,32 @@ func TestGenerateLogicalBackupJob(t *testing.T) { if !reflect.DeepEqual(tt.expectedResources, clusterResources) { t.Errorf("%s - %s: expected resources %#v, got %#v", t.Name(), tt.subTest, tt.expectedResources, clusterResources) } + + expectedSuccessfulJobsHistoryLimit := int32(3) + if cluster.OpConfig.LogicalBackup.LogicalBackupSuccessfulJobsHistoryLimit != nil { + expectedSuccessfulJobsHistoryLimit = *cluster.OpConfig.LogicalBackup.LogicalBackupSuccessfulJobsHistoryLimit + } + if *cronJob.Spec.SuccessfulJobsHistoryLimit != expectedSuccessfulJobsHistoryLimit { + t.Errorf("%s - %s: expected successfulJobsHistoryLimit %d, got %d", t.Name(), tt.subTest, expectedSuccessfulJobsHistoryLimit, *cronJob.Spec.SuccessfulJobsHistoryLimit) + } + + expectedFailedJobsHistoryLimit := int32(3) + if cluster.OpConfig.LogicalBackup.LogicalBackupFailedJobsHistoryLimit != nil { + expectedFailedJobsHistoryLimit = *cluster.OpConfig.LogicalBackup.LogicalBackupFailedJobsHistoryLimit + } + if *cronJob.Spec.FailedJobsHistoryLimit != expectedFailedJobsHistoryLimit { + t.Errorf("%s - %s: expected failedJobsHistoryLimit %d, got %d", t.Name(), tt.subTest, expectedFailedJobsHistoryLimit, *cronJob.Spec.FailedJobsHistoryLimit) + } + + expectedTTL := int32(86400) + if cluster.OpConfig.LogicalBackup.LogicalBackupTTLSecondsAfterFinished != nil { + expectedTTL = *cluster.OpConfig.LogicalBackup.LogicalBackupTTLSecondsAfterFinished + } + if cronJob.Spec.JobTemplate.Spec.TTLSecondsAfterFinished == nil { + t.Errorf("%s - %s: expected TTLSecondsAfterFinished to be set", t.Name(), tt.subTest) + } else if *cronJob.Spec.JobTemplate.Spec.TTLSecondsAfterFinished != expectedTTL { + t.Errorf("%s - %s: expected TTLSecondsAfterFinished %d, got %d", t.Name(), tt.subTest, expectedTTL, *cronJob.Spec.JobTemplate.Spec.TTLSecondsAfterFinished) + } } } diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 9d752a76e..4514e7487 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -217,6 +217,9 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.LogicalBackupMemoryRequest = fromCRD.LogicalBackup.MemoryRequest result.LogicalBackupCPULimit = fromCRD.LogicalBackup.CPULimit result.LogicalBackupMemoryLimit = fromCRD.LogicalBackup.MemoryLimit + result.LogicalBackupSuccessfulJobsHistoryLimit = util.CoalesceInt32(fromCRD.LogicalBackup.SuccessfulJobsHistoryLimit, k8sutil.Int32ToPointer(3)) + result.LogicalBackupFailedJobsHistoryLimit = util.CoalesceInt32(fromCRD.LogicalBackup.FailedJobsHistoryLimit, k8sutil.Int32ToPointer(3)) + result.LogicalBackupTTLSecondsAfterFinished = fromCRD.LogicalBackup.TTLSecondsAfterFinished // debug config result.DebugLogging = *util.CoalesceBool(fromCRD.OperatorDebug.DebugLogging, util.True()) diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 9a18e0d25..06edac439 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -149,6 +149,9 @@ type LogicalBackup struct { LogicalBackupMemoryRequest string `name:"logical_backup_memory_request"` LogicalBackupCPULimit string `name:"logical_backup_cpu_limit"` LogicalBackupMemoryLimit string `name:"logical_backup_memory_limit"` + LogicalBackupSuccessfulJobsHistoryLimit *int32 `name:"logical_backup_successful_jobs_history_limit" default:"3"` + LogicalBackupFailedJobsHistoryLimit *int32 `name:"logical_backup_failed_jobs_history_limit" default:"3"` + LogicalBackupTTLSecondsAfterFinished *int32 `name:"logical_backup_ttl_seconds_after_finished" default:"86400"` } // Operator options for connection pooler