Filter out status updates from the reconcile loop

controllers/metal3.io/baremetalhost_controller.go

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"reflect"
 	"strconv"
 	"strings"
 	"time"
@@ -909,6 +910,30 @@ func hostHasFinalizer(host *metal3v1alpha1.BareMetalHost) bool {
 	return utils.StringInList(host.Finalizers, metal3v1alpha1.BareMetalHostFinalizer)
 }
 
+func (r *BareMetalHostReconciler) updateEventHandler(e event.UpdateEvent) bool {
+	_, oldOK := e.ObjectOld.(*metal3v1alpha1.BareMetalHost)
+	_, newOK := e.ObjectNew.(*metal3v1alpha1.BareMetalHost)
+	if !(oldOK && newOK) {
+		// The thing that changed wasn't a host, so we
+		// need to assume that we must update. This
+		// happens when, for example, an owned Secret
+		// changes.
+		return true
+	}
+
+	//If the update increased the resource Generation then let's process it
+	if e.MetaNew.GetGeneration() != e.MetaOld.GetGeneration() {
+		return true
+	}
+
+	//Discard updates that did not increase the resource Generation (such as on Status.LastUpdated), except for the finalizers or annotations
+	if reflect.DeepEqual(e.MetaNew.GetFinalizers(), e.MetaOld.GetFinalizers()) && reflect.DeepEqual(e.MetaNew.GetAnnotations(), e.MetaOld.GetAnnotations()) {
+		return false
+	}
+
+	return true
+}
+
 // SetupWithManager reigsters the reconciler to be run by the manager
 func (r *BareMetalHostReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
@@ -936,23 +961,7 @@ func (r *BareMetalHostReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&metal3v1alpha1.BareMetalHost{}).
 		WithEventFilter(
 			predicate.Funcs{
-				UpdateFunc: func(e event.UpdateEvent) bool {
-					oldHost, oldOK := e.ObjectOld.(*metal3v1alpha1.BareMetalHost)
-					newHost, newOK := e.ObjectNew.(*metal3v1alpha1.BareMetalHost)
-					if !(oldOK && newOK) {
-						// The thing that changed wasn't a host, so we
-						// need to assume that we must update. This
-						// happens when, for example, an owned Secret
-						// changes.
-						return true
-					}
-
-					if oldHost.Status.ErrorCount != newHost.Status.ErrorCount {
-						//skip reconcile loop
-						return false
-					}
-					return true
-				},
+				UpdateFunc: r.updateEventHandler,
 			}).
 		WithOptions(opts).
 		Owns(&corev1.Secret{}).

controllers/metal3.io/baremetalhost_controller_test.go

@@ -19,6 +19,7 @@ import (
 
 	ctrl "sigs.k8s.io/controller-runtime"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
@@ -1174,3 +1175,126 @@ func TestProvisionerIsReady(t *testing.T) {
 		},
 	)
 }
+
+func TestUpdateEventHandler(t *testing.T) {
+	cases := []struct {
+		name            string
+		event           event.UpdateEvent
+		expectedProcess bool
+	}{
+		{
+			name: "process-non-bmh-events",
+			event: event.UpdateEvent{
+				ObjectOld: &corev1.Secret{},
+				ObjectNew: &corev1.Secret{},
+			},
+			expectedProcess: true,
+		},
+		{
+			name: "process-generation-change",
+			event: event.UpdateEvent{
+				ObjectOld: &metal3v1alpha1.BareMetalHost{},
+				ObjectNew: &metal3v1alpha1.BareMetalHost{},
+				MetaOld:   &metav1.ObjectMeta{Generation: 0},
+				MetaNew:   &metav1.ObjectMeta{Generation: 1},
+			},
+
+			expectedProcess: true,
+		},
+		{
+			name: "skip-if-same-generation-finalizers-and-annotations",
+			event: event.UpdateEvent{
+				ObjectOld: &metal3v1alpha1.BareMetalHost{},
+				ObjectNew: &metal3v1alpha1.BareMetalHost{},
+				MetaOld: &metav1.ObjectMeta{
+					Generation: 0,
+					Finalizers: []string{metal3v1alpha1.BareMetalHostFinalizer},
+					Annotations: map[string]string{
+						metal3v1alpha1.PausedAnnotation: "true",
+					},
+				},
+				MetaNew: &metav1.ObjectMeta{
+					Generation: 0,
+					Finalizers: []string{metal3v1alpha1.BareMetalHostFinalizer},
+					Annotations: map[string]string{
+						metal3v1alpha1.PausedAnnotation: "true",
+					},
+				},
+			},
+
+			expectedProcess: false,
+		},
+		{
+			name: "process-same-generation-annotations-change",
+			event: event.UpdateEvent{
+				ObjectOld: &metal3v1alpha1.BareMetalHost{},
+				ObjectNew: &metal3v1alpha1.BareMetalHost{},
+				MetaOld: &metav1.ObjectMeta{
+					Generation:  0,
+					Finalizers:  []string{metal3v1alpha1.BareMetalHostFinalizer},
+					Annotations: map[string]string{},
+				},
+				MetaNew: &metav1.ObjectMeta{
+					Generation: 0,
+					Finalizers: []string{metal3v1alpha1.BareMetalHostFinalizer},
+					Annotations: map[string]string{
+						metal3v1alpha1.PausedAnnotation: "true",
+					},
+				},
+			},
+
+			expectedProcess: true,
+		},
+		{
+			name: "process-same-generation-finalizers-change",
+			event: event.UpdateEvent{
+				ObjectOld: &metal3v1alpha1.BareMetalHost{},
+				ObjectNew: &metal3v1alpha1.BareMetalHost{},
+				MetaOld: &metav1.ObjectMeta{
+					Generation: 0,
+					Finalizers: []string{},
+					Annotations: map[string]string{
+						metal3v1alpha1.PausedAnnotation: "true",
+					},
+				},
+				MetaNew: &metav1.ObjectMeta{
+					Generation: 0,
+					Finalizers: []string{metal3v1alpha1.BareMetalHostFinalizer},
+					Annotations: map[string]string{
+						metal3v1alpha1.PausedAnnotation: "true",
+					},
+				},
+			},
+
+			expectedProcess: true,
+		},
+		{
+			name: "process-same-generation-finalizers-and-annotation-change",
+			event: event.UpdateEvent{
+				ObjectOld: &metal3v1alpha1.BareMetalHost{},
+				ObjectNew: &metal3v1alpha1.BareMetalHost{},
+				MetaOld: &metav1.ObjectMeta{
+					Generation:  0,
+					Finalizers:  []string{},
+					Annotations: map[string]string{},
+				},
+				MetaNew: &metav1.ObjectMeta{
+					Generation: 0,
+					Finalizers: []string{metal3v1alpha1.BareMetalHostFinalizer},
+					Annotations: map[string]string{
+						metal3v1alpha1.PausedAnnotation: "true",
+					},
+				},
+			},
+
+			expectedProcess: true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			r := newTestReconciler()
+			assert.Equal(t, tc.expectedProcess, r.updateEventHandler(tc.event))
+		})
+	}
+}