ceph: convert the CephCluster controller to the controller-runtime

This is the final conversion to controller-runtime conversion. This time the
CephCluster CRD has been converted to use the controller-runtime
library.
The controller incorporates all the previous watchers too, so the Node
and hot-plug configmap are been watched too.
Only the operator setting configmap is not being watcher since it's not
related to the CephCluster CRD.
Not only the patch converts to controller-runtime but also tries to
re-organize the tree of the repo to actually make the code more
readable and have better functions/methods/tests separations.

Closes: https://github.com/rook/rook/issues/4939
Signed-off-by: Sébastien Han <seb@redhat.com>

pkg/operator/ceph/controller/controller_utils.go

@@ -18,12 +18,16 @@ package controller
 
 import (
 	"context"
+	"fmt"
+	"reflect"
 	"strings"
 	"time"
 
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/clusterd"
 	cephclient "github.com/rook/rook/pkg/daemon/ceph/client"
+	"github.com/rook/rook/pkg/operator/k8sutil"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -86,3 +90,25 @@ func IsReadyToReconcile(c client.Client, clustercontext *clusterd.Context, names
 	logger.Infof("%s: CephCluster %q found but skipping reconcile since Ceph health is %q", controllerName, cephCluster.Name, status.Health.Status)
 	return cephCluster.Spec, false, cephClusterExists, WaitForRequeueIfCephClusterNotReady
 }
+
+// ClusterOwnerRef represents the owner reference of the CephCluster CR
+func ClusterOwnerRef(clusterName, clusterID string) metav1.OwnerReference {
+	blockOwner := true
+	return metav1.OwnerReference{
+		APIVersion:         fmt.Sprintf("%s/%s", ClusterResource.Group, ClusterResource.Version),
+		Kind:               ClusterResource.Kind,
+		Name:               clusterName,
+		UID:                types.UID(clusterID),
+		BlockOwnerDeletion: &blockOwner,
+	}
+}
+
+// ClusterResource operator-kit Custom Resource Definition
+var ClusterResource = k8sutil.CustomResource{
+	Name:       "cephcluster",
+	Plural:     "cephclusters",
+	Group:      cephv1.CustomResourceGroup,
+	Version:    cephv1.Version,
+	Kind:       reflect.TypeOf(cephv1.CephCluster{}).Name(),
+	APIVersion: fmt.Sprintf("%s/%s", cephv1.CustomResourceGroup, cephv1.Version),
+}

pkg/operator/ceph/controller/handler.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2020 The Rook Authors. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+)
+
+// ObjectToCRMapper returns the list of a given object type metadata
+// It is used to trigger a reconcile object Kind A when watching object Kind B
+// So we reconcile Kind A instead of Kind B
+// For instance, we watch for CephCluster CR changes but want to reconcile CephFilesystem based on a Spec change
+func ObjectToCRMapper(c client.Client, ro runtime.Object, scheme *runtime.Scheme) (handler.Mapper, error) {
+	if _, ok := ro.(metav1.ListInterface); !ok {
+		return nil, errors.Errorf("expected a metav1.ListInterface, got %T instead", ro)
+	}
+
+	gvk, err := apiutil.GVKForObject(ro, scheme)
+	if err != nil {
+		return nil, err
+	}
+
+	return handler.ToRequestsFunc(func(o handler.MapObject) []ctrl.Request {
+		list := &unstructured.UnstructuredList{}
+		list.SetGroupVersionKind(gvk)
+		err := c.List(context.TODO(), list)
+		if err != nil {
+			return nil
+		}
+
+		results := []ctrl.Request{}
+		for _, obj := range list.Items {
+			results = append(results, ctrl.Request{
+				NamespacedName: client.ObjectKey{
+					Namespace: obj.GetNamespace(),
+					Name:      obj.GetName(),
+				},
+			})
+		}
+		return results
+
+	}), nil
+}

pkg/operator/ceph/controller/handler_test.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2020 The Rook Authors. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"reflect"
+	"testing"
+
+	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
+	"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+)
+
+func TestObjectToCRMapper(t *testing.T) {
+	fs := &cephv1.CephFilesystem{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		TypeMeta: metav1.TypeMeta{
+			Kind: reflect.TypeOf(cephv1.CephFilesystem{}).Name(),
+		},
+	}
+
+	// Objects to track in the fake client.
+	objects := []runtime.Object{
+		&cephv1.CephFilesystemList{},
+		fs,
+	}
+
+	// Register operator types with the runtime scheme.
+	s := scheme.Scheme
+	s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephFilesystemList{})
+	s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephFilesystem{})
+	s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephCluster{})
+
+	// Create a fake client to mock API calls.
+	cl := fake.NewFakeClientWithScheme(s, objects...)
+
+	// Fake reconcile request
+	fakeRequest := []ctrl.Request{
+		{NamespacedName: client.ObjectKey{Name: "my-pool", Namespace: "rook-ceph"}},
+	}
+
+	handlerFunc, err := ObjectToCRMapper(cl, objects[0], s)
+	assert.NoError(t, err)
+	assert.ElementsMatch(t, fakeRequest, handlerFunc.Map(handler.MapObject{Object: fs}))
+}

pkg/operator/ceph/controller/predicate.go

@@ -24,6 +24,10 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
+	"github.com/rook/rook/pkg/operator/ceph/config"
+	"github.com/rook/rook/pkg/operator/k8sutil"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -33,6 +37,9 @@ import (
 
 const (
 	cephVersionLabelKey = "ceph_version"
+	// Unfortunately this is a duplicate of the const EndpointConfigMapName in the mon package, but done to avoid import cycle
+	endpointConfigMapName   = "rook-ceph-mon-endpoints"
+	doNotReconcileLabelName = "do_not_reconcile"
 )
 
 // WatchControllerPredicate is a special update filter for update events
@@ -43,28 +50,33 @@ const (
 func WatchControllerPredicate() predicate.Funcs {
 	return predicate.Funcs{
 		CreateFunc: func(e event.CreateEvent) bool {
-			logger.Debug("create event from the parent object")
+			logger.Debug("create event from a CR")
 			return true
 		},
 		DeleteFunc: func(e event.DeleteEvent) bool {
-			logger.Debug("delete event from the parent object")
+			logger.Debug("delete event from a CR")
 			return true
 		},
 		UpdateFunc: func(e event.UpdateEvent) bool {
-			logger.Debug("update event from the parent object")
+			logger.Debug("update event from a CR")
 			// resource.Quantity has non-exportable fields, so we use its comparator method
 			resourceQtyComparer := cmp.Comparer(func(x, y resource.Quantity) bool { return x.Cmp(y) == 0 })
 
 			switch objOld := e.ObjectOld.(type) {
 			case *cephv1.CephObjectStore:
 				objNew := e.ObjectNew.(*cephv1.CephObjectStore)
-				logger.Debug("update event from the parent object CephObjectStore")
+				logger.Debug("update event on CephObjectStore CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
 				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
-				if diff != "" || objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
-					// Checking if diff is not empty so we don't print it when the CR gets deleted
-					if diff != "" {
-						logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
-					}
+				if diff != "" {
+					logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
+				} else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
+					logger.Debugf("CR %q is going be deleted", objNew.Name)
 					return true
 				} else if objOld.GetGeneration() != objNew.GetGeneration() {
 					logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name)
@@ -77,13 +89,18 @@ func WatchControllerPredicate() predicate.Funcs {
 
 			case *cephv1.CephObjectStoreUser:
 				objNew := e.ObjectNew.(*cephv1.CephObjectStoreUser)
-				logger.Debug("update event from the parent object CephObjectStoreUser")
+				logger.Debug("update event on CephObjectStoreUser CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
 				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
-				if diff != "" || objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
-					// Checking if diff is not empty so we don't print it when the CR gets deleted
-					if diff != "" {
-						logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
-					}
+				if diff != "" {
+					logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
+				} else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
+					logger.Debugf("CR %q is going be deleted", objNew.Name)
 					return true
 				} else if objOld.GetGeneration() != objNew.GetGeneration() {
 					logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name)
@@ -91,10 +108,15 @@ func WatchControllerPredicate() predicate.Funcs {
 
 			case *cephv1.CephBlockPool:
 				objNew := e.ObjectNew.(*cephv1.CephBlockPool)
-				logger.Debug("update event from the parent object CephBlockPool")
+				logger.Debug("update event on CephBlockPool CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
 				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
 				if diff != "" || objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
-					// Checking if diff is not empty so we don't print it when the CR gets deleted
 					if diff != "" {
 						logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
 					}
@@ -105,13 +127,18 @@ func WatchControllerPredicate() predicate.Funcs {
 
 			case *cephv1.CephFilesystem:
 				objNew := e.ObjectNew.(*cephv1.CephFilesystem)
-				logger.Debug("update event from the parent object CephFilesystem")
+				logger.Debug("update event on CephFilesystem CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
 				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
-				if diff != "" || objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
-					// Checking if diff is not empty so we don't print it when the CR gets deleted
-					if diff != "" {
-						logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
-					}
+				if diff != "" {
+					logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
+				} else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
+					logger.Debugf("CR %q is going be deleted", objNew.Name)
 					return true
 				} else if objOld.GetGeneration() != objNew.GetGeneration() {
 					logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name)
@@ -124,33 +151,73 @@ func WatchControllerPredicate() predicate.Funcs {
 
 			case *cephv1.CephNFS:
 				objNew := e.ObjectNew.(*cephv1.CephNFS)
-				logger.Debug("update event from the parent object CephNFS")
+				logger.Debug("update event on CephNFS CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
 				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
-				if diff != "" || objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
-					// Checking if diff is not empty so we don't print it when the CR gets deleted
-					if diff != "" {
-						logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
-					}
+				if diff != "" {
+					logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
+				} else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
+					logger.Debugf("CR %q is going be deleted", objNew.Name)
 					return true
 				} else if objOld.GetGeneration() != objNew.GetGeneration() {
 					logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name)
 				}
-
-			case *cephv1.CephRBDMirror:
-				objNew := e.ObjectNew.(*cephv1.CephRBDMirror)
-				logger.Debug("update event from the parent object CephRBDMirror")
-				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
-				if diff != "" || objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
-					// Checking if diff is not empty so we don't print it when the CR gets deleted
-					if diff != "" {
-						logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
-					}
+				// Handling upgrades
+				isUpgrade := isUpgrade(objOld.GetLabels(), objNew.GetLabels())
+				if isUpgrade {
 					return true
 				}
 
+			case *cephv1.CephRBDMirror:
+				objNew := e.ObjectNew.(*cephv1.CephRBDMirror)
+				logger.Debug("update event on CephRBDMirror CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
+				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
+				if diff != "" {
+					logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
+				} else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
+					logger.Debugf("CR %q is going be deleted", objNew.Name)
+					return true
+				} else if objOld.GetGeneration() != objNew.GetGeneration() {
+					logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name)
+				}
+				// Handling upgrades
+				isUpgrade := isUpgrade(objOld.GetLabels(), objNew.GetLabels())
+				if isUpgrade {
+					return true
+				}
+
+			case *cephv1.CephCluster:
+				objNew := e.ObjectNew.(*cephv1.CephCluster)
+				logger.Debug("update event on CephCluster CR")
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(objNew.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objNew.Name)
+					return false
+				}
+				diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer)
+				if diff != "" {
+					logger.Infof("CR has changed for %q. diff=%s", objNew.Name, diff)
+					return true
+				} else if objOld.GetDeletionTimestamp() != objNew.GetDeletionTimestamp() {
+					logger.Debugf("CR %q is going be deleted", objNew.Name)
+					return true
+				} else if objOld.GetGeneration() != objNew.GetGeneration() {
+					logger.Debugf("skipping resource %q update with unchanged spec", objNew.Name)
+				}
 			}
 
-			logger.Debug("wont update unknown object")
 			return false
 		},
 		GenericFunc: func(e event.GenericEvent) bool {
@@ -160,7 +227,7 @@ func WatchControllerPredicate() predicate.Funcs {
 }
 
 // objectChanged checks whether the object has been updated
-func objectChanged(oldObj, newObj runtime.Object) (bool, error) {
+func objectChanged(oldObj, newObj runtime.Object, objectName string) (bool, error) {
 	var doReconcile bool
 	old := oldObj.DeepCopyObject()
 	new := newObj.DeepCopyObject()
@@ -181,7 +248,7 @@ func objectChanged(oldObj, newObj runtime.Object) (bool, error) {
 		return doReconcile, nil
 	}
 
-	return isValidEvent(diff.Patch), nil
+	return isValidEvent(diff.Patch, objectName), nil
 }
 
 // WatchPredicateForNonCRDObject is a special filter for create events
@@ -199,35 +266,82 @@ func WatchPredicateForNonCRDObject(owner runtime.Object, scheme *runtime.Scheme)
 		CreateFunc: func(e event.CreateEvent) bool {
 			return false
 		},
+
 		DeleteFunc: func(e event.DeleteEvent) bool {
 			match, object, err := ownerMatcher.Match(e.Object)
 			if err != nil {
 				logger.Errorf("failed to check if object kind %q matched. %v", e.Object.GetObjectKind(), err)
 			}
+			objectName := object.GetName()
 			if match {
-				logger.Debugf("object %q matched on delete", object.GetName())
+				// If the resource is a CM, we might want to ignore it since some of them are ephemeral
+				isCMToIgnoreOnDelete := isCMToIgnoreOnDelete(e.Object)
+				if isCMToIgnoreOnDelete {
+					return false
+				}
+
+				// If the resource is a canary deployment we don't reconcile because it's ephemeral
+				isCanary := isCanary(e.Object)
+				if isCanary {
+					return false
+				}
+
+				logger.Infof("object %q matched on delete, reconciling", objectName)
 				return true
 			}
 
-			logger.Debugf("object %q did not match on delete", object.GetName())
+			logger.Debugf("object %q did not match on delete", objectName)
 			return false
 		},
+
 		UpdateFunc: func(e event.UpdateEvent) bool {
 			match, object, err := ownerMatcher.Match(e.ObjectNew)
 			if err != nil {
 				logger.Errorf("failed to check if object matched. %v", err)
 			}
+			objectName := object.GetName()
 			if match {
-				logger.Debugf("object %q matched on update", object.GetName())
-				objectChanged, err := objectChanged(e.ObjectOld, e.ObjectNew)
+				// If the labels "do_not_reconcile" is set on the object, let's not reconcile that request
+				isDoNotReconcile := isDoNotReconcile(object.GetLabels())
+				if isDoNotReconcile {
+					logger.Debugf("object %q matched on update but %q label is set, doing nothing", doNotReconcileLabelName, objectName)
+					return false
+				}
+
+				logger.Debugf("object %q matched on update", objectName)
+
+				// CONFIGMAP WHITELIST
+				// Only reconcile on rook-config-override CM changes
+				isCMTConfigOverride := isCMTConfigOverride(e.ObjectNew)
+				if !isCMTConfigOverride {
+					return false
+				}
+
+				// SECRETS BLACKLIST
+				// If the resource is a Secret, we might want to ignore it
+				// We want to reconcile Secrets in case their content gets altered
+				isSecretToIgnoreOnUpdate := isSecretToIgnoreOnUpdate(e.ObjectNew)
+				if isSecretToIgnoreOnUpdate {
+					return false
+				}
+
+				// If the resource is a deployment we don't reconcile
+				_, ok := e.ObjectNew.(*appsv1.Deployment)
+				if ok {
+					return false
+				}
+
+				// did the object change?
+				objectChanged, err := objectChanged(e.ObjectOld, e.ObjectNew, objectName)
 				if err != nil {
-					logger.Errorf("failed to check if object %q changed. %v", object.GetName(), err)
+					logger.Errorf("failed to check if object %q changed. %v", objectName, err)
 				}
 				return objectChanged
 			}
 
 			return false
 		},
+
 		GenericFunc: func(e event.GenericEvent) bool {
 			return false
 		},
@@ -237,29 +351,31 @@ func WatchPredicateForNonCRDObject(owner runtime.Object, scheme *runtime.Scheme)
 // isValidEvent analyses the diff between two objects events and determines
 // if we should reconcile that event or not
 // The goal is to avoid double-reconcile as much as possible
-func isValidEvent(patch []byte) bool {
+func isValidEvent(patch []byte, objectName string) bool {
 	patchString := string(patch)
 
-	// Seem a bit 'weak' but since we can't get a real struct of the object
-	// (unless we use the unstructured package, but that over complicates things)
-	// That's probably the most straightforward approach for now...
-	//
-	// The downscale only shows a "deletionTimestamp" which is not appropriate to catch
-	if strings.Contains(patchString, "Created new replica set") {
-		logger.Debug("don't reconcile on replicaset addition")
-		return false
-	}
-
-	// It looks like there is a diff
-	// if the status changed, we do nothing
 	var p map[string]interface{}
 	json.Unmarshal(patch, &p)
+	// don't reconcile on status update on an object (e.g. status "creating")
 	delete(p, "status")
+
+	// Do not reconcile on metadata change since managedFields are often updated by the server
+	delete(p, "metadata")
+
+	// If the patch is now empty, we don't reconcile, nothing changed
 	if len(p) == 0 {
 		return false
 	}
 
-	logger.Infof("will reconcile based on patch %s", patchString)
+	// Re-marshal to get the last diff
+	patch, err := json.Marshal(p)
+	if err != nil {
+		logger.Infof("controller will reconcile resource %q based on patch: %s", objectName, patchString)
+	}
+
+	// If after all the filtering there is still something in the patch, we reconcile
+	logger.Infof("controller will reconcile resource %q based on patch: %s", objectName, string(patch))
+
 	return true
 }
 
@@ -284,3 +400,82 @@ func isUpgrade(oldLabels, newLabels map[string]string) bool {
 
 	return false
 }
+
+func isCanary(obj runtime.Object) bool {
+	// If not a deployment, let's not reconcile
+	d, ok := obj.(*appsv1.Deployment)
+	if !ok {
+		return false
+	}
+
+	// Get the labels
+	labels := d.GetLabels()
+
+	labelVal, labelKeyExist := labels["mon_canary"]
+	if labelKeyExist && labelVal == "true" {
+		logger.Debugf("do not reconcile %q on monitor canary deployments", d.Name)
+		return true
+	}
+
+	return false
+}
+
+func isCMTConfigOverride(obj runtime.Object) bool {
+	// If not a ConfigMap, let's not reconcile
+	cm, ok := obj.(*corev1.ConfigMap)
+	if !ok {
+		return false
+	}
+
+	objectName := cm.GetName()
+	if objectName == k8sutil.ConfigOverrideName {
+		return true
+	}
+
+	return false
+}
+
+func isCMToIgnoreOnDelete(obj runtime.Object) bool {
+	// If not a ConfigMap, let's not reconcile
+	cm, ok := obj.(*corev1.ConfigMap)
+	if !ok {
+		return false
+	}
+
+	objectName := cm.GetName()
+	// is it the object the temporarily osd config map?
+	if strings.HasPrefix(objectName, "rook-ceph-osd-") && strings.HasSuffix(objectName, "-status") {
+		logger.Debugf("do not reconcile on %q config map changes", objectName)
+		return true
+	}
+
+	return false
+}
+
+func isSecretToIgnoreOnUpdate(obj runtime.Object) bool {
+	// If not a Secret, let's not reconcile
+	s, ok := obj.(*corev1.Secret)
+	if !ok {
+		return false
+	}
+
+	objectName := s.GetName()
+	switch objectName {
+	case config.StoreName:
+		logger.Debugf("do not reconcile on %q secret changes", objectName)
+		return true
+	}
+
+	return false
+}
+
+func isDoNotReconcile(labels map[string]string) bool {
+	value, ok := labels[doNotReconcileLabelName]
+
+	// Nothing exists
+	if ok && value == "true" {
+		return true
+	}
+
+	return false
+}

pkg/operator/ceph/controller/predicate_test.go

@@ -21,7 +21,10 @@ import (
 	"testing"
 
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
+	"github.com/rook/rook/pkg/operator/ceph/config"
 	"github.com/stretchr/testify/assert"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -65,13 +68,13 @@ func TestObjectChanged(t *testing.T) {
 	}
 
 	// Identical
-	changed, err := objectChanged(oldObject, newObject)
+	changed, err := objectChanged(oldObject, newObject, "foo")
 	assert.NoError(t, err)
 	assert.False(t, changed)
 
 	// Replica size changed
 	oldObject.Spec.Replicated.Size = newReplicas
-	changed, err = objectChanged(oldObject, newObject)
+	changed, err = objectChanged(oldObject, newObject, "foo")
 	assert.NoError(t, err)
 	assert.True(t, changed)
 }
@@ -102,3 +105,132 @@ func TestIsUpgrade(t *testing.T) {
 	b = isUpgrade(oldLabel, newLabel)
 	assert.True(t, b, fmt.Sprintf("%v,%v", oldLabel, newLabel))
 }
+
+func TestIsValidEvent(t *testing.T) {
+	obj := "rook-ceph-mon-a"
+	valid := []byte(`{
+		"metadata": {},
+		"spec": {},
+		"status": {
+		  "conditions": [
+			{
+			  "message": "ReplicaSet \"rook-ceph-mon-b-784fc58bf8\" is progressing.",
+			  "reason": "ReplicaSetUpdated",
+			  "type": "Progressing"
+			}
+		  ]
+		}
+	  }`)
+
+	b := isValidEvent(valid, obj)
+	assert.True(t, b)
+
+	valid = []byte(`{"foo": "bar"}`)
+	b = isValidEvent(valid, obj)
+	assert.True(t, b)
+
+	invalid := []byte(`{
+		"metadata": {},
+		"status": {},
+	  }`)
+	b = isValidEvent(invalid, obj)
+	assert.False(t, b)
+}
+
+func TestIsCanary(t *testing.T) {
+	dum := &cephv1.CephBlockPool{}
+
+	b := isCanary(dum)
+	assert.False(t, b)
+
+	d := &appsv1.Deployment{}
+	b = isCanary(d)
+	assert.False(t, b)
+
+	d.Labels = map[string]string{
+		"foo": "bar",
+	}
+	b = isCanary(d)
+	assert.False(t, b)
+
+	d.Labels["mon_canary"] = "true"
+	b = isCanary(d)
+	assert.True(t, b)
+}
+
+func TestIsCMToIgnoreOnUpdate(t *testing.T) {
+	dum := &cephv1.CephBlockPool{}
+
+	b := isCMTConfigOverride(dum)
+	assert.False(t, b)
+
+	cm := &corev1.ConfigMap{}
+	b = isCMTConfigOverride(cm)
+	assert.False(t, b)
+
+	cm.Name = "rook-ceph-mon-endpoints"
+	b = isCMTConfigOverride(cm)
+	assert.False(t, b)
+
+	cm.Name = "rook-config-override"
+	b = isCMTConfigOverride(cm)
+	assert.True(t, b)
+}
+
+func TestIsCMToIgnoreOnDelete(t *testing.T) {
+	dum := &cephv1.CephBlockPool{}
+
+	b := isCMToIgnoreOnDelete(dum)
+	assert.False(t, b)
+
+	cm := &corev1.ConfigMap{}
+	b = isCMToIgnoreOnDelete(cm)
+	assert.False(t, b)
+
+	cm.Name = "rook-ceph-mon-endpoints"
+	b = isCMToIgnoreOnDelete(cm)
+	assert.False(t, b)
+
+	cm.Name = "rook-ceph-osd-minikube-status"
+	b = isCMToIgnoreOnDelete(cm)
+	assert.True(t, b)
+}
+
+func TestIsSecretToIgnoreOnUpdate(t *testing.T) {
+	dum := &cephv1.CephBlockPool{}
+
+	b := isSecretToIgnoreOnUpdate(dum)
+	assert.False(t, b)
+
+	s := &corev1.Secret{}
+	b = isSecretToIgnoreOnUpdate(s)
+	assert.False(t, b)
+
+	s.Name = "foo"
+	b = isSecretToIgnoreOnUpdate(s)
+	assert.False(t, b)
+
+	s.Name = config.StoreName
+	b = isSecretToIgnoreOnUpdate(s)
+	assert.True(t, b)
+}
+
+func TestIsDoNotReconcile(t *testing.T) {
+	l := map[string]string{
+		"foo": "bar",
+	}
+
+	// value not present
+	b := isDoNotReconcile(l)
+	assert.False(t, b)
+
+	// good value wrong content
+	l["do_not_reconcile"] = "false"
+	b = isDoNotReconcile(l)
+	assert.False(t, b)
+
+	// good value and good content
+	l["do_not_reconcile"] = "true"
+	b = isDoNotReconcile(l)
+	assert.True(t, b)
+}