From 5e45650014c942d0d7cc3ce8634d2936375bde25 Mon Sep 17 00:00:00 2001 From: Luca Corrieri Date: Sun, 5 Nov 2023 18:24:11 +0100 Subject: [PATCH] fix(controllers): reconcile on annotation update --- 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 e6c2820e..f27546b8 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" @@ -135,8 +136,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 44adbf3f..454675ad 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" @@ -117,8 +118,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 78a843a7..f511275a 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.