From d8f222c83f3a27a408d70cc7adbf4fcdf722c344 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Thu, 31 Jul 2025 14:50:08 -0700 Subject: [PATCH] Add ConfigMap support for keepLatestMaintenanceJobs Signed-off-by: Shubham Pampattiwar add changelog file Signed-off-by: Shubham Pampattiwar lint fix Signed-off-by: Shubham Pampattiwar --- .../unreleased/9135-shubham-pampattiwar | 1 + .../backup_repository_controller.go | 18 +- .../backup_repository_controller_test.go | 158 ++++++++++++++++++ pkg/repository/maintenance/maintenance.go | 33 ++++ .../maintenance/maintenance_test.go | 138 +++++++++++++++ 5 files changed, 345 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/9135-shubham-pampattiwar diff --git a/changelogs/unreleased/9135-shubham-pampattiwar b/changelogs/unreleased/9135-shubham-pampattiwar new file mode 100644 index 000000000..a489082a8 --- /dev/null +++ b/changelogs/unreleased/9135-shubham-pampattiwar @@ -0,0 +1 @@ +Add ConfigMap support for keepLatestMaintenanceJobs with CLI parameter fallback \ No newline at end of file diff --git a/pkg/controller/backup_repository_controller.go b/pkg/controller/backup_repository_controller.go index cdf0b71a3..52ccef6ae 100644 --- a/pkg/controller/backup_repository_controller.go +++ b/pkg/controller/backup_repository_controller.go @@ -275,7 +275,15 @@ func (r *BackupRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, errors.Wrap(err, "error check and run repo maintenance jobs") } - if err := maintenance.DeleteOldJobs(r.Client, req.Name, r.keepLatestMaintenanceJobs); err != nil { + // Get the configured number of maintenance jobs to keep from ConfigMap, fallback to CLI parameter + keepJobs := r.keepLatestMaintenanceJobs + if configuredKeep, err := maintenance.GetKeepLatestMaintenanceJobs(ctx, r.Client, log, r.namespace, r.repoMaintenanceConfig, backupRepo); err != nil { + log.WithError(err).Warn("Failed to get keepLatestMaintenanceJobs from ConfigMap, using CLI parameter value") + } else if configuredKeep > 0 { + keepJobs = configuredKeep + } + + if err := maintenance.DeleteOldJobs(r.Client, req.Name, keepJobs); err != nil { log.WithError(err).Warn("Failed to delete old maintenance jobs") } } @@ -397,8 +405,12 @@ func (r *BackupRepoReconciler) recallMaintenance(ctx context.Context, req *veler log.Warn("Updating backup repository because of unrecorded histories") return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) { - if lastMaintenanceTime.After(rr.Status.LastMaintenanceTime.Time) { - log.Warnf("Updating backup repository last maintenance time (%v) from history (%v)", rr.Status.LastMaintenanceTime.Time, lastMaintenanceTime.Time) + if lastMaintenanceTime != nil && (rr.Status.LastMaintenanceTime == nil || lastMaintenanceTime.After(rr.Status.LastMaintenanceTime.Time)) { + if rr.Status.LastMaintenanceTime != nil { + log.Warnf("Updating backup repository last maintenance time (%v) from history (%v)", rr.Status.LastMaintenanceTime.Time, lastMaintenanceTime.Time) + } else { + log.Warnf("Setting backup repository last maintenance time from history (%v)", lastMaintenanceTime.Time) + } rr.Status.LastMaintenanceTime = lastMaintenanceTime } diff --git a/pkg/controller/backup_repository_controller_test.go b/pkg/controller/backup_repository_controller_test.go index 8101d3e14..5b019ae58 100644 --- a/pkg/controller/backup_repository_controller_test.go +++ b/pkg/controller/backup_repository_controller_test.go @@ -1559,6 +1559,164 @@ func TestDeleteOldMaintenanceJob(t *testing.T) { } } +func TestDeleteOldMaintenanceJobWithConfigMap(t *testing.T) { + tests := []struct { + name string + repo *velerov1api.BackupRepository + serverKeepJobs int + expectedKeptJobs int + maintenanceJobs []batchv1api.Job + bsl *velerov1api.BackupStorageLocation + repoMaintenanceJob *corev1api.ConfigMap + }{ + { + name: "test with global config", + repo: &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "repo", + }, + Spec: velerov1api.BackupRepositorySpec{ + MaintenanceFrequency: metav1.Duration{Duration: testMaintenanceFrequency}, + BackupStorageLocation: "default", + VolumeNamespace: "test-ns", + RepositoryType: "restic", + }, + Status: velerov1api.BackupRepositoryStatus{ + Phase: velerov1api.BackupRepositoryPhaseReady, + }, + }, + serverKeepJobs: 3, + expectedKeptJobs: 5, + maintenanceJobs: []batchv1api.Job{ + *builder.ForJob("velero", "job-01").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-02").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-03").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-04").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-05").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-06").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + }, + bsl: builder.ForBackupStorageLocation("velero", "default").Result(), + repoMaintenanceJob: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "repo-maintenance-job-config", + }, + Data: map[string]string{ + "global": `{"keepLatestMaintenanceJobs": 5}`, + }, + }, + }, + { + name: "test with specific repo config overriding global", + repo: &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "repo", + }, + Spec: velerov1api.BackupRepositorySpec{ + MaintenanceFrequency: metav1.Duration{Duration: testMaintenanceFrequency}, + BackupStorageLocation: "default", + VolumeNamespace: "test-ns", + RepositoryType: "restic", + }, + Status: velerov1api.BackupRepositoryStatus{ + Phase: velerov1api.BackupRepositoryPhaseReady, + }, + }, + serverKeepJobs: 3, + expectedKeptJobs: 2, + maintenanceJobs: []batchv1api.Job{ + *builder.ForJob("velero", "job-01").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-02").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-03").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + }, + bsl: builder.ForBackupStorageLocation("velero", "default").Result(), + repoMaintenanceJob: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "repo-maintenance-job-config", + }, + Data: map[string]string{ + "global": `{"keepLatestMaintenanceJobs": 5}`, + "test-ns-default-restic": `{"keepLatestMaintenanceJobs": 2}`, + }, + }, + }, + { + name: "test fallback to CLI parameter when no ConfigMap", + repo: &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "repo", + }, + Spec: velerov1api.BackupRepositorySpec{ + MaintenanceFrequency: metav1.Duration{Duration: testMaintenanceFrequency}, + BackupStorageLocation: "default", + VolumeNamespace: "test-ns", + RepositoryType: "restic", + }, + Status: velerov1api.BackupRepositoryStatus{ + Phase: velerov1api.BackupRepositoryPhaseReady, + }, + }, + serverKeepJobs: 2, + expectedKeptJobs: 2, + maintenanceJobs: []batchv1api.Job{ + *builder.ForJob("velero", "job-01").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-02").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-03").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + *builder.ForJob("velero", "job-04").ObjectMeta(builder.WithLabels(repomaintenance.RepositoryNameLabel, "repo")).Succeeded(1).Result(), + }, + bsl: builder.ForBackupStorageLocation("velero", "default").Result(), + repoMaintenanceJob: nil, // No ConfigMap + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + objects := []runtime.Object{test.repo, test.bsl} + if test.repoMaintenanceJob != nil { + objects = append(objects, test.repoMaintenanceJob) + } + crClient := velerotest.NewFakeControllerRuntimeClient(t, objects...) + + for _, job := range test.maintenanceJobs { + require.NoError(t, crClient.Create(t.Context(), &job)) + } + + repoLocker := repository.NewRepoLocker() + mgr := repomanager.NewManager("", crClient, repoLocker, nil, nil, nil) + + repoMaintenanceConfigName := "" + if test.repoMaintenanceJob != nil { + repoMaintenanceConfigName = test.repoMaintenanceJob.Name + } + + reconciler := NewBackupRepoReconciler( + velerov1api.DefaultNamespace, + velerotest.NewLogger(), + crClient, + mgr, + time.Duration(0), + "", + test.serverKeepJobs, + repoMaintenanceConfigName, + kube.PodResources{}, + logrus.InfoLevel, + nil, + ) + + _, err := reconciler.Reconcile(t.Context(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.repo.Namespace, Name: "repo"}}) + require.NoError(t, err) + + jobList := new(batchv1api.JobList) + require.NoError(t, reconciler.Client.List(t.Context(), jobList, &client.ListOptions{Namespace: "velero"})) + assert.Len(t, jobList.Items, test.expectedKeptJobs, "Expected %d jobs to be kept, but got %d", test.expectedKeptJobs, len(jobList.Items)) + }) + } +} + func TestInitializeRepoWithRepositoryTypes(t *testing.T) { scheme := runtime.NewScheme() corev1api.AddToScheme(scheme) diff --git a/pkg/repository/maintenance/maintenance.go b/pkg/repository/maintenance/maintenance.go index 8c6dc2cc3..c887e8813 100644 --- a/pkg/repository/maintenance/maintenance.go +++ b/pkg/repository/maintenance/maintenance.go @@ -58,6 +58,9 @@ type JobConfigs struct { // PodResources is the config for the CPU and memory resources setting. PodResources *kube.PodResources `json:"podResources,omitempty"` + + // KeepLatestMaintenanceJobs is the number of latest maintenance jobs to keep for the repository. + KeepLatestMaintenanceJobs *int `json:"keepLatestMaintenanceJobs,omitempty"` } func GenerateJobName(repo string) string { @@ -271,11 +274,41 @@ func getJobConfig( if len(result.LoadAffinities) == 0 { result.LoadAffinities = globalResult.LoadAffinities } + + if result.KeepLatestMaintenanceJobs == nil && globalResult.KeepLatestMaintenanceJobs != nil { + result.KeepLatestMaintenanceJobs = globalResult.KeepLatestMaintenanceJobs + } } return result, nil } +// GetKeepLatestMaintenanceJobs returns the configured number of maintenance jobs to keep from the JobConfigs. +// If not configured in the ConfigMap, it returns 0 to indicate using the fallback value. +func GetKeepLatestMaintenanceJobs( + ctx context.Context, + client client.Client, + logger logrus.FieldLogger, + veleroNamespace string, + repoMaintenanceJobConfig string, + repo *velerov1api.BackupRepository, +) (int, error) { + if repoMaintenanceJobConfig == "" { + return 0, nil + } + + config, err := getJobConfig(ctx, client, logger, veleroNamespace, repoMaintenanceJobConfig, repo) + if err != nil { + return 0, err + } + + if config != nil && config.KeepLatestMaintenanceJobs != nil { + return *config.KeepLatestMaintenanceJobs, nil + } + + return 0, nil +} + // WaitJobComplete waits the completion of the specified maintenance job and return the BackupRepositoryMaintenanceStatus func WaitJobComplete(cli client.Client, ctx context.Context, jobName, ns string, logger logrus.FieldLogger) (velerov1api.BackupRepositoryMaintenanceStatus, error) { log := logger.WithField("job name", jobName) diff --git a/pkg/repository/maintenance/maintenance_test.go b/pkg/repository/maintenance/maintenance_test.go index 48fd58ed6..d1ffb0f7c 100644 --- a/pkg/repository/maintenance/maintenance_test.go +++ b/pkg/repository/maintenance/maintenance_test.go @@ -1162,3 +1162,141 @@ func TestBuildJob(t *testing.T) { }) } } + +func TestGetKeepLatestMaintenanceJobs(t *testing.T) { + tests := []struct { + name string + repoMaintenanceJobConfig string + configMap *corev1api.ConfigMap + repo *velerov1api.BackupRepository + expectedValue int + expectError bool + }{ + { + name: "no config map name provided", + repoMaintenanceJobConfig: "", + configMap: nil, + repo: mockBackupRepo(), + expectedValue: 0, + expectError: false, + }, + { + name: "config map not found", + repoMaintenanceJobConfig: "non-existent-config", + configMap: nil, + repo: mockBackupRepo(), + expectedValue: 0, + expectError: false, + }, + { + name: "config map with global keepLatestMaintenanceJobs", + repoMaintenanceJobConfig: "repo-job-config", + configMap: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "repo-job-config", + }, + Data: map[string]string{ + "global": `{"keepLatestMaintenanceJobs": 5}`, + }, + }, + repo: mockBackupRepo(), + expectedValue: 5, + expectError: false, + }, + { + name: "config map with specific repo keepLatestMaintenanceJobs overriding global", + repoMaintenanceJobConfig: "repo-job-config", + configMap: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "repo-job-config", + }, + Data: map[string]string{ + "global": `{"keepLatestMaintenanceJobs": 5}`, + "test-ns-default-kopia": `{"keepLatestMaintenanceJobs": 10}`, + }, + }, + repo: mockBackupRepo(), + expectedValue: 10, + expectError: false, + }, + { + name: "config map with no keepLatestMaintenanceJobs specified", + repoMaintenanceJobConfig: "repo-job-config", + configMap: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "repo-job-config", + }, + Data: map[string]string{ + "global": `{"podResources": {"cpuRequest": "100m"}}`, + }, + }, + repo: mockBackupRepo(), + expectedValue: 0, + expectError: false, + }, + { + name: "config map with invalid JSON", + repoMaintenanceJobConfig: "repo-job-config", + configMap: &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "repo-job-config", + }, + Data: map[string]string{ + "global": `{"keepLatestMaintenanceJobs": invalid}`, + }, + }, + repo: mockBackupRepo(), + expectedValue: 0, + expectError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + scheme := runtime.NewScheme() + corev1api.AddToScheme(scheme) + + var objects []runtime.Object + if test.configMap != nil { + objects = append(objects, test.configMap) + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objects...).Build() + logger := velerotest.NewLogger() + + result, err := GetKeepLatestMaintenanceJobs( + t.Context(), + client, + logger, + "velero", + test.repoMaintenanceJobConfig, + test.repo, + ) + + if test.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, test.expectedValue, result) + } + }) + } +} + +func mockBackupRepo() *velerov1api.BackupRepository { + return &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "velero", + Name: "test-repo", + }, + Spec: velerov1api.BackupRepositorySpec{ + VolumeNamespace: "test-ns", + BackupStorageLocation: "default", + RepositoryType: "kopia", + }, + } +}