Skip to content

Commit

Permalink
Don't run maintenance on the ReadOnly BackupRepositories.
Browse files Browse the repository at this point in the history 
Signed-off-by: Xun Jiang/Bruce Jiang <[email protected]>
  • Loading branch information
@blackpiglet
blackpiglet committed Feb 13, 2025
1 parent 79707aa commit f1a19cb
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 22 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8681-blackpiglet
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Don't run maintenance on the ReadOnly BackupRepositories.
51 changes: 38 additions & 13 deletions pkg/controller/backup_repository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,27 @@ func NewBackupRepoReconciler(namespace string, logger logrus.FieldLogger, client
}

func (r *BackupRepoReconciler) SetupWithManager(mgr ctrl.Manager) error {
s := kube.NewPeriodicalEnqueueSource(r.logger.WithField("controller", constant.ControllerBackupRepo), mgr.GetClient(), &velerov1api.BackupRepositoryList{}, repoSyncPeriod, kube.PeriodicalEnqueueSourceOption{})
s := kube.NewPeriodicalEnqueueSource(
r.logger.WithField("controller", constant.ControllerBackupRepo),
mgr.GetClient(),
&velerov1api.BackupRepositoryList{},
repoSyncPeriod,
kube.PeriodicalEnqueueSourceOption{},
)

return ctrl.NewControllerManagedBy(mgr).
For(&velerov1api.BackupRepository{}, builder.WithPredicates(kube.SpecChangePredicate{})).
WatchesRawSource(s).
Watches(&velerov1api.BackupStorageLocation{}, kube.EnqueueRequestsFromMapUpdateFunc(r.invalidateBackupReposForBSL),
Watches(
&velerov1api.BackupStorageLocation{},
kube.EnqueueRequestsFromMapUpdateFunc(r.invalidateBackupReposForBSL),
builder.WithPredicates(
// When BSL updates, check if the backup repositories need to be invalidated
kube.NewUpdateEventPredicate(r.needInvalidBackupRepo),
// When BSL is created, invalidate any backup repositories that reference it
kube.NewCreateEventPredicate(func(client.Object) bool { return true }))).
kube.NewCreateEventPredicate(func(client.Object) bool { return true }),
),
).
Complete(r)
}

Expand Down Expand Up @@ -194,8 +204,14 @@ func (r *BackupRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, err
}

bsl, bslErr := r.getBSL(ctx, backupRepo)
if bslErr != nil {
log.WithError(bslErr).Error("Fail to get BSL for BackupRepository. Skip reconciling.")
return ctrl.Result{}, nil
}

if backupRepo.Status.Phase == "" || backupRepo.Status.Phase == velerov1api.BackupRepositoryPhaseNew {
if err := r.initializeRepo(ctx, backupRepo, log); err != nil {
if err := r.initializeRepo(ctx, backupRepo, bsl, log); err != nil {
log.WithError(err).Error("error initialize repository")
return ctrl.Result{}, errors.WithStack(err)
}
Expand All @@ -213,14 +229,19 @@ func (r *BackupRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request)

switch backupRepo.Status.Phase {
case velerov1api.BackupRepositoryPhaseNotReady:
ready, err := r.checkNotReadyRepo(ctx, backupRepo, log)
ready, err := r.checkNotReadyRepo(ctx, backupRepo, bsl, log)
if err != nil {
return ctrl.Result{}, err
} else if !ready {
return ctrl.Result{}, nil
}
fallthrough
case velerov1api.BackupRepositoryPhaseReady:
if bsl.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly {
log.Debugf("Skip running maintenance for BackupRepository, because its BSL is in the ReadOnly mode.")
return ctrl.Result{}, nil
}

if err := r.recallMaintenance(ctx, backupRepo, log); err != nil {
return ctrl.Result{}, errors.Wrap(err, "error handling incomplete repo maintenance jobs")
}
Expand All @@ -237,29 +258,33 @@ func (r *BackupRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}

func (r *BackupRepoReconciler) getIdentiferByBSL(ctx context.Context, req *velerov1api.BackupRepository) (string, error) {
loc := &velerov1api.BackupStorageLocation{}
func (r *BackupRepoReconciler) getBSL(ctx context.Context, req *velerov1api.BackupRepository) (*velerov1api.BackupStorageLocation, error) {
loc := new(velerov1api.BackupStorageLocation)

if err := r.Get(ctx, client.ObjectKey{
Namespace: req.Namespace,
Name: req.Spec.BackupStorageLocation,
}, loc); err != nil {
return "", errors.Wrapf(err, "error to get BSL %s", req.Spec.BackupStorageLocation)
return nil, err
}

repoIdentifier, err := repoconfig.GetRepoIdentifier(loc, req.Spec.VolumeNamespace)
return loc, nil
}

func (r *BackupRepoReconciler) getIdentifierByBSL(bsl velerov1api.BackupStorageLocation, req *velerov1api.BackupRepository) (string, error) {
repoIdentifier, err := repoconfig.GetRepoIdentifier(&bsl, req.Spec.VolumeNamespace)
if err != nil {
return "", errors.Wrapf(err, "error to get identifier for repo %s", req.Name)
}

return repoIdentifier, nil
}

func (r *BackupRepoReconciler) initializeRepo(ctx context.Context, req *velerov1api.BackupRepository, log logrus.FieldLogger) error {
func (r *BackupRepoReconciler) initializeRepo(ctx context.Context, req *velerov1api.BackupRepository, bsl *velerov1api.BackupStorageLocation, log logrus.FieldLogger) error {
log.WithField("repoConfig", r.backupRepoConfig).Info("Initializing backup repository")

// confirm the repo's BackupStorageLocation is valid
repoIdentifier, err := r.getIdentiferByBSL(ctx, req)
repoIdentifier, err := r.getIdentifierByBSL(*bsl, req)
if err != nil {
return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) {
rr.Status.Message = err.Error()
Expand Down Expand Up @@ -475,10 +500,10 @@ func dueForMaintenance(req *velerov1api.BackupRepository, now time.Time) bool {
return req.Status.LastMaintenanceTime == nil || req.Status.LastMaintenanceTime.Add(req.Spec.MaintenanceFrequency.Duration).Before(now)
}

func (r *BackupRepoReconciler) checkNotReadyRepo(ctx context.Context, req *velerov1api.BackupRepository, log logrus.FieldLogger) (bool, error) {
func (r *BackupRepoReconciler) checkNotReadyRepo(ctx context.Context, req *velerov1api.BackupRepository, bsl *velerov1api.BackupStorageLocation, log logrus.FieldLogger) (bool, error) {
log.Info("Checking backup repository for readiness")

repoIdentifier, err := r.getIdentiferByBSL(ctx, req)
repoIdentifier, err := r.getIdentifierByBSL(*bsl, req)
if err != nil {
return false, r.patchBackupRepository(ctx, req, repoNotReady(err.Error()))
}
Expand Down
14 changes: 5 additions & 9 deletions pkg/controller/backup_repository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestCheckNotReadyRepo(t *testing.T) {
reconciler := mockBackupRepoReconciler(t, "PrepareRepo", rr, nil)
err := reconciler.Client.Create(context.TODO(), rr)
assert.NoError(t, err)
locations := &velerov1api.BackupStorageLocation{
location := velerov1api.BackupStorageLocation{
Spec: velerov1api.BackupStorageLocationSpec{
Config: map[string]string{"resticRepoPrefix": "s3:test.amazonaws.com/bucket/restic"},
},
Expand All @@ -110,9 +110,7 @@ func TestCheckNotReadyRepo(t *testing.T) {
},
}

err = reconciler.Client.Create(context.TODO(), locations)
assert.NoError(t, err)
_, err = reconciler.checkNotReadyRepo(context.TODO(), rr, reconciler.logger)
_, err = reconciler.checkNotReadyRepo(context.TODO(), rr, &location, reconciler.logger)
assert.NoError(t, err)
assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase)
assert.Equal(t, "s3:test.amazonaws.com/bucket/restic/volume-ns-1", rr.Spec.ResticIdentifier)
Expand Down Expand Up @@ -404,7 +402,7 @@ func TestInitializeRepo(t *testing.T) {
reconciler := mockBackupRepoReconciler(t, "PrepareRepo", rr, nil)
err := reconciler.Client.Create(context.TODO(), rr)
assert.NoError(t, err)
locations := &velerov1api.BackupStorageLocation{
location := velerov1api.BackupStorageLocation{
Spec: velerov1api.BackupStorageLocationSpec{
Config: map[string]string{"resticRepoPrefix": "s3:test.amazonaws.com/bucket/restic"},
},
Expand All @@ -414,9 +412,7 @@ func TestInitializeRepo(t *testing.T) {
},
}

err = reconciler.Client.Create(context.TODO(), locations)
assert.NoError(t, err)
err = reconciler.initializeRepo(context.TODO(), rr, reconciler.logger)
err = reconciler.initializeRepo(context.TODO(), rr, &location, reconciler.logger)
assert.NoError(t, err)
assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase)
}
Expand Down Expand Up @@ -475,7 +471,7 @@ func TestBackupRepoReconcile(t *testing.T) {
reconciler := mockBackupRepoReconciler(t, "", test.repo, nil)
err := reconciler.Client.Create(context.TODO(), test.repo)
assert.NoError(t, err)
_, err = reconciler.Reconcile(context.TODO(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.repo.Namespace, Name: test.repo.Name}})
_, err = reconciler.Reconcile(context.TODO(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.repo.Namespace, Name: "repo"}})
if test.expectNil {
assert.NoError(t, err)
} else {
Expand Down

0 comments on commit f1a19cb

Please sign in to comment.