From e9d1729dcd4eadbc53be0f603e899e3f13095b58 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Fri, 2 Feb 2024 13:33:51 -0500 Subject: [PATCH 1/2] Add BackupRepositories invalidation on BSL Create Simplify comments Signed-off-by: Tiger Kaovilai --- changelogs/unreleased/7380-kaovilai | 1 + .../backup_repository_controller.go | 15 ++++++++++++--- pkg/util/kube/event_handler.go | 6 ++---- pkg/util/kube/predicate.go | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/7380-kaovilai diff --git a/changelogs/unreleased/7380-kaovilai b/changelogs/unreleased/7380-kaovilai new file mode 100644 index 0000000000..a95fa4db2d --- /dev/null +++ b/changelogs/unreleased/7380-kaovilai @@ -0,0 +1 @@ +BackupRepositories associated with a BSL are invalidated when BSL is (re-)created. \ No newline at end of file diff --git a/pkg/controller/backup_repository_controller.go b/pkg/controller/backup_repository_controller.go index 70918c8619..968c0348cd 100644 --- a/pkg/controller/backup_repository_controller.go +++ b/pkg/controller/backup_repository_controller.go @@ -76,7 +76,7 @@ func (r *BackupRepoReconciler) SetupWithManager(mgr ctrl.Manager) error { For(&velerov1api.BackupRepository{}). Watches(s, nil). Watches(&source.Kind{Type: &velerov1api.BackupStorageLocation{}}, kube.EnqueueRequestsFromMapUpdateFunc(r.invalidateBackupReposForBSL), - builder.WithPredicates(kube.NewUpdateEventPredicate(r.needInvalidBackupRepo))). + builder.WithPredicates(kube.NewCreateOrUpdateEventPredicate(r.needInvalidBackupRepo))). Complete(r) } @@ -90,13 +90,13 @@ func (r *BackupRepoReconciler) invalidateBackupReposForBSL(bslObj client.Object) }).AsSelector(), } if err := r.List(context.TODO(), list, options); err != nil { - r.logger.WithField("BSL", bsl.Name).WithError(err).Error("unable to list BackupRepositorys") + r.logger.WithField("BSL", bsl.Name).WithError(err).Error("unable to list BackupRepositories") return []reconcile.Request{} } for i := range list.Items { r.logger.WithField("BSL", bsl.Name).Infof("Invalidating Backup Repository %s", list.Items[i].Name) - if err := r.patchBackupRepository(context.Background(), &list.Items[i], repoNotReady("re-establish on BSL change")); err != nil { + if err := r.patchBackupRepository(context.Background(), &list.Items[i], repoNotReady("re-establish on BSL change or create")); err != nil { r.logger.WithField("BSL", bsl.Name).WithError(err).Errorf("fail to patch BackupRepository %s", list.Items[i].Name) } } @@ -104,7 +104,16 @@ func (r *BackupRepoReconciler) invalidateBackupReposForBSL(bslObj client.Object) return []reconcile.Request{} } +// needInvalidBackupRepo returns true if the BSL's storage type, bucket, prefix, CACert, or config has changed or if BSL was created. func (r *BackupRepoReconciler) needInvalidBackupRepo(oldObj client.Object, newObj client.Object) bool { + if oldObj == nil { + // BSL was created because oldBSL is nil + // Return true so invalidateBackupReposForBSL is triggered + // BSL creationTimestamp should be newer than the BackupRepository's creationTimestamp if any. + // BSL was created after the BackupRepository, so we need to re-establish the BackupRepository on BSL creation. + return true + } + oldBSL := oldObj.(*velerov1api.BackupStorageLocation) newBSL := newObj.(*velerov1api.BackupStorageLocation) diff --git a/pkg/util/kube/event_handler.go b/pkg/util/kube/event_handler.go index 471a41c81e..e07d173449 100644 --- a/pkg/util/kube/event_handler.go +++ b/pkg/util/kube/event_handler.go @@ -26,10 +26,8 @@ import ( type MapUpdateFunc func(client.Object) []reconcile.Request -// EnqueueRequestsFromMapUpdateFunc is for the same purpose with EnqueueRequestsFromMapFunc. -// Merely, it is more friendly to updating the mapped objects in the MapUpdateFunc, because -// on Update event, MapUpdateFunc is called for only once with the new object, so if MapUpdateFunc -// does some update to the mapped objects, the update is done for once +// EnqueueRequestsFromMapUpdateFunc has the same purpose with handler.EnqueueRequestsFromMapFunc. +// MapUpdateFunc is simpler on Update event because mapAndEnqueue is called once with the new object. EnqueueRequestsFromMapFunc is called twice with the old and new object. func EnqueueRequestsFromMapUpdateFunc(fn MapUpdateFunc) handler.EventHandler { return &enqueueRequestsFromMapFunc{ toRequests: fn, diff --git a/pkg/util/kube/predicate.go b/pkg/util/kube/predicate.go index 9ac9d9894e..637c928195 100644 --- a/pkg/util/kube/predicate.go +++ b/pkg/util/kube/predicate.go @@ -93,6 +93,25 @@ func NewUpdateEventPredicate(f func(client.Object, client.Object) bool) predicat } } +// NewCreateOrUpdateEventPredicate creates a new Predicate that checks the create or update events with the provided func +// and ignore others +func NewCreateOrUpdateEventPredicate(f func(client.Object, client.Object) bool) predicate.Predicate { + return predicate.Funcs{ + UpdateFunc: func(event event.UpdateEvent) bool { + return f(event.ObjectOld, event.ObjectNew) + }, + CreateFunc: func(event event.CreateEvent) bool { + return f(nil, event.Object) + }, + DeleteFunc: func(event event.DeleteEvent) bool { + return false + }, + GenericFunc: func(event event.GenericEvent) bool { + return false + }, + } +} + // FalsePredicate always returns false for all kinds of events type FalsePredicate struct{} From 423d63da43cfb6b47308953a93256a2d07484835 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Mon, 5 Feb 2024 21:09:26 -0500 Subject: [PATCH 2/2] Simplify Signed-off-by: Tiger Kaovilai --- .../backup_repository_controller.go | 16 ++++++---------- pkg/util/kube/predicate.go | 19 ------------------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/pkg/controller/backup_repository_controller.go b/pkg/controller/backup_repository_controller.go index 968c0348cd..a84a583a21 100644 --- a/pkg/controller/backup_repository_controller.go +++ b/pkg/controller/backup_repository_controller.go @@ -76,7 +76,11 @@ func (r *BackupRepoReconciler) SetupWithManager(mgr ctrl.Manager) error { For(&velerov1api.BackupRepository{}). Watches(s, nil). Watches(&source.Kind{Type: &velerov1api.BackupStorageLocation{}}, kube.EnqueueRequestsFromMapUpdateFunc(r.invalidateBackupReposForBSL), - builder.WithPredicates(kube.NewCreateOrUpdateEventPredicate(r.needInvalidBackupRepo))). + 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 }))). Complete(r) } @@ -104,16 +108,8 @@ func (r *BackupRepoReconciler) invalidateBackupReposForBSL(bslObj client.Object) return []reconcile.Request{} } -// needInvalidBackupRepo returns true if the BSL's storage type, bucket, prefix, CACert, or config has changed or if BSL was created. +// needInvalidBackupRepo returns true if the BSL's storage type, bucket, prefix, CACert, or config has changed func (r *BackupRepoReconciler) needInvalidBackupRepo(oldObj client.Object, newObj client.Object) bool { - if oldObj == nil { - // BSL was created because oldBSL is nil - // Return true so invalidateBackupReposForBSL is triggered - // BSL creationTimestamp should be newer than the BackupRepository's creationTimestamp if any. - // BSL was created after the BackupRepository, so we need to re-establish the BackupRepository on BSL creation. - return true - } - oldBSL := oldObj.(*velerov1api.BackupStorageLocation) newBSL := newObj.(*velerov1api.BackupStorageLocation) diff --git a/pkg/util/kube/predicate.go b/pkg/util/kube/predicate.go index 637c928195..9ac9d9894e 100644 --- a/pkg/util/kube/predicate.go +++ b/pkg/util/kube/predicate.go @@ -93,25 +93,6 @@ func NewUpdateEventPredicate(f func(client.Object, client.Object) bool) predicat } } -// NewCreateOrUpdateEventPredicate creates a new Predicate that checks the create or update events with the provided func -// and ignore others -func NewCreateOrUpdateEventPredicate(f func(client.Object, client.Object) bool) predicate.Predicate { - return predicate.Funcs{ - UpdateFunc: func(event event.UpdateEvent) bool { - return f(event.ObjectOld, event.ObjectNew) - }, - CreateFunc: func(event event.CreateEvent) bool { - return f(nil, event.Object) - }, - DeleteFunc: func(event event.DeleteEvent) bool { - return false - }, - GenericFunc: func(event event.GenericEvent) bool { - return false - }, - } -} - // FalsePredicate always returns false for all kinds of events type FalsePredicate struct{}