Merge pull request rook#13652 from travisn/cm-skip-reconcile

core: Skip reconcile if override configmap is empty

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) {