From b8f373f690824f37dc533b0f2a77dba037f9ab68 Mon Sep 17 00:00:00 2001 From: Luca Corrieri Date: Thu, 9 Nov 2023 15:49:02 +0100 Subject: [PATCH] fix(controllers): reconcile on annotation update (#188) --- go.mod | 2 +- internal/controllers/terraformlayer/controller.go | 10 ++++++++-- .../controllers/terraformpullrequest/controller.go | 10 ++++++++-- internal/controllers/terraformrun/controller.go | 10 ++++++++-- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 8d76f220..23efd28f 100644 --- a/go.mod +++ b/go.mod @@ -71,7 +71,7 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.3 // indirect - github.com/google/go-cmp v0.5.9 // indirect + github.com/google/go-cmp v0.5.9 github.com/google/go-github/v50 v50.2.0 github.com/google/gofuzz v1.2.0 // indirect github.com/google/uuid v1.3.0 // indirect diff --git a/internal/controllers/terraformlayer/controller.go b/internal/controllers/terraformlayer/controller.go index 03b3140c..c0e7920c 100644 --- a/internal/controllers/terraformlayer/controller.go +++ b/internal/controllers/terraformlayer/controller.go @@ -20,6 +20,7 @@ import ( "context" "time" + "github.com/google/go-cmp/cmp" "github.com/padok-team/burrito/internal/burrito/config" "github.com/padok-team/burrito/internal/lock" "github.com/padok-team/burrito/internal/storage" @@ -139,8 +140,13 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { func ignorePredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - // Ignore updates to CR status in which case metadata.Generation does not change - return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() + // Update only if generation or annotations change, filter out anything else. + // We only need to check generation or annotations change here, because it is only + // updated on spec changes. On the other hand RevisionVersion + // changes also on status changes. We want to omit reconciliation + // for status updates. + return (e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()) || + cmp.Diff(e.ObjectOld.GetAnnotations(), e.ObjectNew.GetAnnotations()) != "" }, DeleteFunc: func(e event.DeleteEvent) bool { // Evaluates to false if the object has been confirmed deleted. diff --git a/internal/controllers/terraformpullrequest/controller.go b/internal/controllers/terraformpullrequest/controller.go index 4e76bf5d..a6f34e32 100644 --- a/internal/controllers/terraformpullrequest/controller.go +++ b/internal/controllers/terraformpullrequest/controller.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/google/go-cmp/cmp" "github.com/padok-team/burrito/internal/burrito/config" "github.com/padok-team/burrito/internal/controllers/terraformpullrequest/comment" "github.com/padok-team/burrito/internal/controllers/terraformpullrequest/github" @@ -116,8 +117,13 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { func ignorePredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - // Ignore updates to CR status in which case metadata.Generation does not change - return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() + // Update only if generation or annotations change, filter out anything else. + // We only need to check generation or annotations change here, because it is only + // updated on spec changes. On the other hand RevisionVersion + // changes also on status changes. We want to omit reconciliation + // for status updates. + return (e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()) || + cmp.Diff(e.ObjectOld.GetAnnotations(), e.ObjectNew.GetAnnotations()) != "" }, DeleteFunc: func(e event.DeleteEvent) bool { // Evaluates to false if the object has been confirmed deleted. diff --git a/internal/controllers/terraformrun/controller.go b/internal/controllers/terraformrun/controller.go index edbeda62..2394b9dd 100644 --- a/internal/controllers/terraformrun/controller.go +++ b/internal/controllers/terraformrun/controller.go @@ -21,6 +21,7 @@ import ( "math" "time" + "github.com/google/go-cmp/cmp" log "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -132,8 +133,13 @@ func getExponentialBackOffTime(DefaultRequeueAfter time.Duration, attempts int) func ignorePredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { - // Ignore updates to CR status in which case metadata.Generation does not change - return e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() + // Update only if generation or annotations change, filter out anything else. + // We only need to check generation or annotations change here, because it is only + // updated on spec changes. On the other hand RevisionVersion + // changes also on status changes. We want to omit reconciliation + // for status updates. + return (e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration()) || + cmp.Diff(e.ObjectOld.GetAnnotations(), e.ObjectNew.GetAnnotations()) != "" }, DeleteFunc: func(e event.DeleteEvent) bool { // Evaluates to false if the object has been confirmed deleted.