Add ConfigMap support for keepLatestMaintenanceJobs

Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com> add changelog file Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com> lint fix Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
2026-01-05 04:55:22 +00:00 · 2025-07-31 14:50:08 -07:00
parent 1535afb45e
commit d8f222c83f
5 changed files with 345 additions and 3 deletions
--- a/changelogs/unreleased/9135-shubham-pampattiwar
+++ b/changelogs/unreleased/9135-shubham-pampattiwar
@@ -0,0 +1 @@
+Add ConfigMap support for keepLatestMaintenanceJobs with CLI parameter fallback
--- 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
 		}

--- 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)
--- 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)
--- 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",
+		},
+	}
+}
				`@@ -0,0 +1 @@`
				`Add ConfigMap support for keepLatestMaintenanceJobs with CLI parameter fallback`