From 733be02577869695103beef3b4cd3e3f0952bb7d Mon Sep 17 00:00:00 2001
From: subhamkrai <srai@redhat.com>
Date: Mon, 7 Aug 2023 20:09:10 +0530
Subject: [PATCH] operator: log op. config cm setting when changed

earlier, we're always logging when `GetValue` method is
called to get operator config  values which somettime
spamming the logs for example `ROOK_WATCH_FOR_NODE_FAILURE`
keep logging every time 5 min.

Now, we'll store the settings in the map and log only when
map key value is changed.

Signed-off-by: subhamkrai <srai@redhat.com>
---
 pkg/operator/k8sutil/configmap.go      | 34 ++++++++++++++++++++------
 pkg/operator/k8sutil/configmap_test.go | 18 ++++++++++++++
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/pkg/operator/k8sutil/configmap.go b/pkg/operator/k8sutil/configmap.go
index cb6c83306..c7754510b 100644
--- a/pkg/operator/k8sutil/configmap.go
+++ b/pkg/operator/k8sutil/configmap.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"sync"
 	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -27,6 +28,8 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
+var loggedOperatorSettings = sync.Map{}
+
 // DeleteConfigMap deletes a ConfigMap.
 func DeleteConfigMap(ctx context.Context, clientset kubernetes.Interface, cmName, namespace string, opts *DeleteOptions) error {
 	k8sOpts := BaseKubernetesDeleteOptions()
@@ -62,13 +65,28 @@ func GetOperatorSetting(context context.Context, clientset kubernetes.Interface,
 }
 
 func GetValue(data map[string]string, settingName, defaultValue string) string {
-	if settingValue, ok := data[settingName]; ok {
-		logger.Infof("%s=%q (configmap)", settingName, settingValue)
-		return settingValue
-	} else if settingValue, ok := os.LookupEnv(settingName); ok {
-		logger.Infof("%s=%q (env var)", settingName, settingValue)
-		return settingValue
+	settingValue := defaultValue
+	settingSource := "default"
+
+	if val, ok := data[settingName]; ok {
+		settingValue = val
+		settingSource = "configmap"
+	} else if val, ok := os.LookupEnv(settingName); ok {
+		settingValue = val
+		settingSource = "env var"
 	}
-	logger.Infof("%s=%q (default)", settingName, defaultValue)
-	return defaultValue
+
+	if logChangedSettings(settingName, settingValue) {
+		logger.Infof("%s=%q (%s)", settingName, settingValue, settingSource)
+	}
+
+	return settingValue
+}
+
+func logChangedSettings(settingName, settingValue string) bool {
+	if val, ok := loggedOperatorSettings.Load(settingName); !ok || val != settingValue {
+		loggedOperatorSettings.Store(settingName, settingValue)
+		return true
+	}
+	return false
 }
diff --git a/pkg/operator/k8sutil/configmap_test.go b/pkg/operator/k8sutil/configmap_test.go
index 4048ee03e..b8db353f3 100644
--- a/pkg/operator/k8sutil/configmap_test.go
+++ b/pkg/operator/k8sutil/configmap_test.go
@@ -119,3 +119,21 @@ func TestGetOperatorSetting(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, envSettingValue, setting)
 }
+
+func TestLogChangedSettings(t *testing.T) {
+	key := "Node_Affinity"
+	value := "storage=rook, worker"
+
+	// When the key is not in the map, it should be added and return true for logging
+	ok := logChangedSettings(key, value)
+	assert.True(t, true, ok)
+
+	// When the key is already present in the map and value is same, it should return false for logging
+	ok = logChangedSettings(key, value)
+	assert.False(t, false, ok)
+
+	// When the key is already present in the map and value is updated, it should return true for logging
+	value = "storage=rook, test"
+	ok = logChangedSettings(key, value)
+	assert.True(t, true, ok)
+}