Merge pull request #8532 from Lyndon-Li/isolate-message-in-backup-repo

Add maintenance history for backupRepository CRs

changelogs/unreleased/8532-Lyndon-Li

@@ -0,0 +1 @@
+Fix issue #7810, add maintenance history for backupRepository CRs

config/crd/v1/bases/velero.io_backuprepositories.yaml

@@ -88,8 +88,8 @@ spec:
             description: BackupRepositoryStatus is the current status of a BackupRepository.
             properties:
               lastMaintenanceTime:
-                description: LastMaintenanceTime is the last time maintenance was
-                  run.
+                description: LastMaintenanceTime is the last time repo maintenance
+                  succeeded.
                 format: date-time
                 nullable: true
                 type: string
@@ -104,6 +104,33 @@ spec:
                 - Ready
                 - NotReady
                 type: string
+              recentMaintenance:
+                description: RecentMaintenance is status of the recent repo maintenance.
+                items:
+                  properties:
+                    completeTimestamp:
+                      description: CompleteTimestamp is the completion time of the
+                        repo maintenance.
+                      format: date-time
+                      nullable: true
+                      type: string
+                    message:
+                      description: Message is a message about the current status of
+                        the repo maintenance.
+                      type: string
+                    result:
+                      description: Result is the result of the repo maintenance.
+                      enum:
+                      - Succeeded
+                      - Failed
+                      type: string
+                    startTimestamp:
+                      description: StartTimestamp is the start time of the repo maintenance.
+                      format: date-time
+                      nullable: true
+                      type: string
+                  type: object
+                type: array
             type: object
         type: object
     served: true

pkg/apis/velero/v1/backup_repository_types.go

@@ -71,10 +71,43 @@ type BackupRepositoryStatus struct {
 	// +optional
 	Message string `json:"message,omitempty"`
 
-	// LastMaintenanceTime is the last time maintenance was run.
+	// LastMaintenanceTime is the last time repo maintenance succeeded.
 	// +optional
 	// +nullable
 	LastMaintenanceTime *metav1.Time `json:"lastMaintenanceTime,omitempty"`
+
+	// RecentMaintenance is status of the recent repo maintenance.
+	// +optional
+	RecentMaintenance []BackupRepositoryMaintenanceStatus `json:"recentMaintenance,omitempty"`
+}
+
+// BackupRepositoryMaintenanceResult represents the result of a repo maintenance.
+// +kubebuilder:validation:Enum=Succeeded;Failed
+type BackupRepositoryMaintenanceResult string
+
+const (
+	BackupRepositoryMaintenanceSucceeded BackupRepositoryMaintenanceResult = "Succeeded"
+	BackupRepositoryMaintenanceFailed    BackupRepositoryMaintenanceResult = "Failed"
+)
+
+type BackupRepositoryMaintenanceStatus struct {
+	// Result is the result of the repo maintenance.
+	// +optional
+	Result BackupRepositoryMaintenanceResult `json:"result,omitempty"`
+
+	// StartTimestamp is the start time of the repo maintenance.
+	// +optional
+	// +nullable
+	StartTimestamp *metav1.Time `json:"startTimestamp,omitempty"`
+
+	// CompleteTimestamp is the completion time of the repo maintenance.
+	// +optional
+	// +nullable
+	CompleteTimestamp *metav1.Time `json:"completeTimestamp,omitempty"`
+
+	// Message is a message about the current status of the repo maintenance.
+	// +optional
+	Message string `json:"message,omitempty"`
 }
 
 // TODO(2.0) After converting all resources to use the runtime-controller client,

pkg/apis/velero/v1/zz_generated.deepcopy.go

@@ -165,6 +165,29 @@ func (in *BackupRepositoryList) DeepCopyObject() runtime.Object {
 	return nil
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *BackupRepositoryMaintenanceStatus) DeepCopyInto(out *BackupRepositoryMaintenanceStatus) {
+	*out = *in
+	if in.StartTimestamp != nil {
+		in, out := &in.StartTimestamp, &out.StartTimestamp
+		*out = (*in).DeepCopy()
+	}
+	if in.CompleteTimestamp != nil {
+		in, out := &in.CompleteTimestamp, &out.CompleteTimestamp
+		*out = (*in).DeepCopy()
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupRepositoryMaintenanceStatus.
+func (in *BackupRepositoryMaintenanceStatus) DeepCopy() *BackupRepositoryMaintenanceStatus {
+	if in == nil {
+		return nil
+	}
+	out := new(BackupRepositoryMaintenanceStatus)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *BackupRepositorySpec) DeepCopyInto(out *BackupRepositorySpec) {
 	*out = *in
@@ -195,6 +218,13 @@ func (in *BackupRepositoryStatus) DeepCopyInto(out *BackupRepositoryStatus) {
 		in, out := &in.LastMaintenanceTime, &out.LastMaintenanceTime
 		*out = (*in).DeepCopy()
 	}
+	if in.RecentMaintenance != nil {
+		in, out := &in.RecentMaintenance, &out.RecentMaintenance
+		*out = make([]BackupRepositoryMaintenanceStatus, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupRepositoryStatus.

pkg/controller/backup_repository_controller.go

@@ -46,8 +46,9 @@ import (
 )
 
 const (
-	repoSyncPeriod           = 5 * time.Minute
-	defaultMaintainFrequency = 7 * 24 * time.Hour
+	repoSyncPeriod                      = 5 * time.Minute
+	defaultMaintainFrequency            = 7 * 24 * time.Hour
+	defaultMaintenanceStatusQueueLength = 3
 )
 
 type BackupRepoReconciler struct {
@@ -299,9 +300,9 @@ func ensureRepo(repo *velerov1api.BackupRepository, repoManager repomanager.Mana
 }
 
 func (r *BackupRepoReconciler) runMaintenanceIfDue(ctx context.Context, req *velerov1api.BackupRepository, log logrus.FieldLogger) error {
-	now := r.clock.Now()
+	startTime := r.clock.Now()
 
-	if !dueForMaintenance(req, now) {
+	if !dueForMaintenance(req, startTime) {
 		log.Debug("not due for maintenance")
 		return nil
 	}
@@ -315,16 +316,33 @@ func (r *BackupRepoReconciler) runMaintenanceIfDue(ctx context.Context, req *vel
 	if err := r.repositoryManager.PruneRepo(req); err != nil {
 		log.WithError(err).Warn("error pruning repository")
 		return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) {
-			rr.Status.Message = err.Error()
+			updateRepoMaintenanceHistory(rr, velerov1api.BackupRepositoryMaintenanceFailed, startTime, r.clock.Now(), err.Error())
 		})
 	}
 
 	return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) {
-		rr.Status.Message = ""
-		rr.Status.LastMaintenanceTime = &metav1.Time{Time: now}
+		completionTime := r.clock.Now()
+		rr.Status.LastMaintenanceTime = &metav1.Time{Time: completionTime}
+		updateRepoMaintenanceHistory(rr, velerov1api.BackupRepositoryMaintenanceSucceeded, startTime, completionTime, "")
 	})
 }
 
+func updateRepoMaintenanceHistory(repo *velerov1api.BackupRepository, result velerov1api.BackupRepositoryMaintenanceResult, startTime time.Time, completionTime time.Time, message string) {
+	latest := velerov1api.BackupRepositoryMaintenanceStatus{
+		Result:            result,
+		StartTimestamp:    &metav1.Time{Time: startTime},
+		CompleteTimestamp: &metav1.Time{Time: completionTime},
+		Message:           message,
+	}
+
+	startingPos := 0
+	if len(repo.Status.RecentMaintenance) >= defaultMaintenanceStatusQueueLength {
+		startingPos = len(repo.Status.RecentMaintenance) - defaultMaintenanceStatusQueueLength + 1
+	}
+
+	repo.Status.RecentMaintenance = append(repo.Status.RecentMaintenance[startingPos:], latest)
+}
+
 func dueForMaintenance(req *velerov1api.BackupRepository, now time.Time) bool {
 	return req.Status.LastMaintenanceTime == nil || req.Status.LastMaintenanceTime.Add(req.Spec.MaintenanceFrequency.Duration).Before(now)
 }

pkg/controller/backup_repository_controller_test.go

@@ -486,3 +486,180 @@ func TestGetBackupRepositoryConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateRepoMaintenanceHistory(t *testing.T) {
+	standardTime := time.Now()
+
+	backupRepoWithoutHistory := &velerov1api.BackupRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "repo",
+		},
+	}
+
+	backupRepoWithHistory := &velerov1api.BackupRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "repo",
+		},
+		Status: velerov1api.BackupRepositoryStatus{
+			RecentMaintenance: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 24)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 23)},
+					Message:           "fake-history-message-1",
+				},
+			},
+		},
+	}
+
+	backupRepoWithFullHistory := &velerov1api.BackupRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "repo",
+		},
+		Status: velerov1api.BackupRepositoryStatus{
+			RecentMaintenance: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 24)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 23)},
+					Message:           "fake-history-message-2",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 22)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 21)},
+					Message:           "fake-history-message-3",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 20)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 19)},
+					Message:           "fake-history-message-4",
+				},
+			},
+		},
+	}
+
+	backupRepoWithOverFullHistory := &velerov1api.BackupRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: velerov1api.DefaultNamespace,
+			Name:      "repo",
+		},
+		Status: velerov1api.BackupRepositoryStatus{
+			RecentMaintenance: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 24)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 23)},
+					Message:           "fake-history-message-5",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 22)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 21)},
+					Message:           "fake-history-message-6",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 20)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 19)},
+					Message:           "fake-history-message-7",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 18)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 17)},
+					Message:           "fake-history-message-8",
+				},
+			},
+		},
+	}
+
+	tests := []struct {
+		name            string
+		backupRepo      *velerov1api.BackupRepository
+		result          velerov1api.BackupRepositoryMaintenanceResult
+		expectedHistory []velerov1api.BackupRepositoryMaintenanceStatus
+	}{
+		{
+			name:       "empty history",
+			backupRepo: backupRepoWithoutHistory,
+			result:     velerov1api.BackupRepositoryMaintenanceSucceeded,
+			expectedHistory: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(time.Hour)},
+					Message:           "fake-message-0",
+				},
+			},
+		},
+		{
+			name:       "less than history queue length",
+			backupRepo: backupRepoWithHistory,
+			result:     velerov1api.BackupRepositoryMaintenanceSucceeded,
+			expectedHistory: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 24)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 23)},
+					Message:           "fake-history-message-1",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(time.Hour)},
+					Message:           "fake-message-0",
+				},
+			},
+		},
+		{
+			name:       "full history",
+			backupRepo: backupRepoWithFullHistory,
+			result:     velerov1api.BackupRepositoryMaintenanceFailed,
+			expectedHistory: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 22)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 21)},
+					Message:           "fake-history-message-3",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 20)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 19)},
+					Message:           "fake-history-message-4",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(time.Hour)},
+					Message:           "fake-message-0",
+				},
+			},
+		},
+		{
+			name:       "over full history",
+			backupRepo: backupRepoWithOverFullHistory,
+			result:     velerov1api.BackupRepositoryMaintenanceFailed,
+			expectedHistory: []velerov1api.BackupRepositoryMaintenanceStatus{
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 20)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 19)},
+					Message:           "fake-history-message-7",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime.Add(-time.Hour * 18)},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(-time.Hour * 17)},
+					Message:           "fake-history-message-8",
+				},
+				{
+					StartTimestamp:    &metav1.Time{Time: standardTime},
+					CompleteTimestamp: &metav1.Time{Time: standardTime.Add(time.Hour)},
+					Message:           "fake-message-0",
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			updateRepoMaintenanceHistory(test.backupRepo, test.result, standardTime, standardTime.Add(time.Hour), "fake-message-0")
+
+			for at := range test.backupRepo.Status.RecentMaintenance {
+				assert.Equal(t, test.expectedHistory[at].StartTimestamp.Time, test.backupRepo.Status.RecentMaintenance[at].StartTimestamp.Time)
+				assert.Equal(t, test.expectedHistory[at].CompleteTimestamp.Time, test.backupRepo.Status.RecentMaintenance[at].CompleteTimestamp.Time)
+				assert.Equal(t, test.expectedHistory[at].Message, test.backupRepo.Status.RecentMaintenance[at].Message)
+			}
+		})
+	}
+}