Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add maintenance history for backupRepository CRs #8532

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/8532-Lyndon-Li
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #7810, add maintenance history for backupRepository CRs
31 changes: 29 additions & 2 deletions config/crd/v1/bases/velero.io_backuprepositories.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

@Lyndon-Li Lyndon-Li Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Breaking change]: LastMaintenanceTime should be used to record the completion time of a successful maintenance.
Previously, it is used to record the start time of a successful maintenance, which is not reasonable. E.g., I run a maintenance which takes more than 1 hour; since the default frequency for quick maintenance is 1 hour, the next maintenance will be launched immediately after the first one completes, but actually it is a wasting one as data has just been maintained.
Previously, the description in the CRD is ambiguous, it could mean the completion time or start time.

So in this PR, both the description and the value set to LastMaintenanceTime as changed.

succeeded.
format: date-time
nullable: true
type: string
Expand All @@ -104,6 +104,33 @@ spec:
- Ready
- NotReady
type: string
recentMaintenance:
description: RecentMaintenance is status of the recent repo maintenance.
items:
properties:
completeTimestamp:
Lyndon-Li marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
2 changes: 1 addition & 1 deletion config/crd/v1/crds/crds.go

Large diffs are not rendered by default.

35 changes: 34 additions & 1 deletion pkg/apis/velero/v1/backup_repository_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions pkg/apis/velero/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 25 additions & 7 deletions pkg/controller/backup_repository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@
)

const (
repoSyncPeriod = 5 * time.Minute
defaultMaintainFrequency = 7 * 24 * time.Hour
repoSyncPeriod = 5 * time.Minute
defaultMaintainFrequency = 7 * 24 * time.Hour
defaultMaintenanceStatusQueueLength = 3
)

type BackupRepoReconciler struct {
Expand Down Expand Up @@ -299,9 +300,9 @@
}

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
}
Expand All @@ -315,16 +316,33 @@
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())

Check warning on line 319 in pkg/controller/backup_repository_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_repository_controller.go#L319

Added line #L319 was not covered by tests
})
}

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)
}
Expand Down
177 changes: 177 additions & 0 deletions pkg/controller/backup_repository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
Loading