core: skip reconcile if override configmap is empty

If the configmap rook-config-override is empty,
there is no need to trigger the reconcile to update
the ceph daemons. This configmap update is causing
unnecessary reconciles periodically in some clusters
even when it is empty.

Signed-off-by: travisn <tnielsen@redhat.com>

pkg/operator/ceph/controller/predicate.go

@@ -579,17 +579,17 @@ func WatchPredicateForNonCRDObject(owner runtime.Object, scheme *runtime.Scheme)
 				logger.Debugf("object %q matched on update", objectName)
 
 				// CONFIGMAP WHITELIST
-				// Only reconcile on rook-config-override CM changes
-				isCMTConfigOverride := isCMTConfigOverride(e.ObjectNew)
-				if isCMTConfigOverride {
-					logger.Debugf("do reconcile when the cm is %s", k8sutil.ConfigOverrideName)
+				// Only reconcile on rook-config-override CM changes if the configmap changed
+				shouldReconcileCM := shouldReconcileCM(e.ObjectOld, e.ObjectNew)
+				if shouldReconcileCM {
+					logger.Infof("reconcile due to updated configmap %s", k8sutil.ConfigOverrideName)
 					return true
 				}
 
 				// If the resource is a ConfigMap we don't reconcile
 				_, ok := e.ObjectNew.(*corev1.ConfigMap)
 				if ok {
-					logger.Debugf("do not reconcile on configmap that is not %q", k8sutil.ConfigOverrideName)
+					logger.Debugf("do not reconcile on configmap %q", objectName)
 					return false
 				}
 
@@ -747,15 +747,27 @@ func isDeployment(obj runtime.Object, appName string) bool {
 	return false
 }
 
-func isCMTConfigOverride(obj runtime.Object) bool {
+func shouldReconcileCM(objOld runtime.Object, objNew runtime.Object) bool {
 	// If not a ConfigMap, let's not reconcile
-	cm, ok := obj.(*corev1.ConfigMap)
+	cmNew, ok := objNew.(*corev1.ConfigMap)
 	if !ok {
 		return false
 	}
 
-	objectName := cm.GetName()
-	return objectName == k8sutil.ConfigOverrideName
+	// If not a ConfigMap, let's not reconcile
+	cmOld, ok := objOld.(*corev1.ConfigMap)
+	if !ok {
+		return false
+	}
+
+	objectName := cmNew.GetName()
+	if objectName != k8sutil.ConfigOverrideName {
+		return false
+	}
+	if !reflect.DeepEqual(cmNew.Data, cmOld.Data) {
+		return true
+	}
+	return false
 }
 
 func isCMToIgnoreOnDelete(obj runtime.Object) bool {

pkg/operator/ceph/controller/predicate_test.go

@@ -164,16 +164,39 @@ func TestIsCanary(t *testing.T) {
 
 func TestIsCMToIgnoreOnUpdate(t *testing.T) {
 	blockPool := &cephv1.CephBlockPool{}
-	assert.False(t, isCMTConfigOverride(blockPool))
+	reconcile := shouldReconcileCM(blockPool, blockPool)
+	assert.False(t, reconcile)
 
 	cm := &corev1.ConfigMap{}
-	assert.False(t, isCMTConfigOverride(cm))
+	reconcile = shouldReconcileCM(cm, cm)
+	assert.False(t, reconcile)
 
 	cm.Name = "rook-ceph-mon-endpoints"
-	assert.False(t, isCMTConfigOverride(cm))
+	reconcile = shouldReconcileCM(cm, cm)
+	assert.False(t, reconcile)
 
+	// Valid name, but cm completely empty
 	cm.Name = "rook-config-override"
-	assert.True(t, isCMTConfigOverride(cm))
+	oldCM := &corev1.ConfigMap{}
+	oldCM.Name = cm.Name
+	reconcile = shouldReconcileCM(oldCM, cm)
+	assert.False(t, reconcile)
+
+	// Both have empty config value
+	cm.Data = map[string]string{"config": ""}
+	oldCM.Data = map[string]string{"config": ""}
+	reconcile = shouldReconcileCM(oldCM, cm)
+	assert.False(t, reconcile)
+
+	// Something added to the CM
+	cm.Data = map[string]string{"config": "somevalue"}
+	reconcile = shouldReconcileCM(oldCM, cm)
+	assert.True(t, reconcile)
+
+	// A value changed in the CM
+	oldCM.Data = map[string]string{"config": "diffvalue"}
+	reconcile = shouldReconcileCM(oldCM, cm)
+	assert.True(t, reconcile)
 }
 
 func TestIsCMToIgnoreOnDelete(t *testing.T) {