fix(setting): check both setting and definition default for possible …

…change. Signed-off-by: James Munson <james.munson@suse.com>
longhorn · Mar 8, 2024 · 81254ac · 81254ac
1 parent aafa8dd
commit 81254ac
Showing 1 changed file with 23 additions and 25 deletions.
diff --git a/datastore/longhorn.go b/datastore/longhorn.go
@@ -54,7 +54,7 @@ func (s *DataStore) UpdateCustomizedSettings(defaultImages map[types.SettingName
 		return err
 	}
 
-	availableCustomizedDefaultSettings := s.filterCustomizedDefaultSettings(customizedDefaultSettings)
+	availableCustomizedDefaultSettings := s.filterCustomizedDefaultSettings(customizedDefaultSettings, defaultSettingCM.ResourceVersion)
 
 	if err := s.applyCustomizedDefaultSettingsToDefinitions(availableCustomizedDefaultSettings); err != nil {
 		return err
@@ -96,11 +96,11 @@ func (s *DataStore) createNonExistingSettingCRsWithDefaultSetting(configMapResou
 	return nil
 }
 
-func (s *DataStore) filterCustomizedDefaultSettings(customizedDefaultSettings map[string]string) map[string]string {
+func (s *DataStore) filterCustomizedDefaultSettings(customizedDefaultSettings map[string]string, defaultSettingCMResourceVersion string) map[string]string {
 	availableCustomizedDefaultSettings := make(map[string]string)
 
 	for name, value := range customizedDefaultSettings {
-		if !s.isSettingValueChanged(types.SettingName(name), value) {
+		if !s.shouldApplyCustomizedSettingValue(types.SettingName(name), value, defaultSettingCMResourceVersion) {
 			continue
 		}
 
@@ -114,8 +114,7 @@ func (s *DataStore) filterCustomizedDefaultSettings(customizedDefaultSettings ma
 	return availableCustomizedDefaultSettings
 }
 
-// isSettingValueChanged check if the customized default setting and value was changed
-func (s *DataStore) isSettingValueChanged(name types.SettingName, value string) bool {
+func (s *DataStore) shouldApplyCustomizedSettingValue(name types.SettingName, value string, defaultSettingCMResourceVersion string) bool {
 	setting, err := s.GetSettingExactRO(name)
 	if err != nil {
 		if !ErrorIsNotFound(err) {
@@ -124,10 +123,20 @@ func (s *DataStore) isSettingValueChanged(name types.SettingName, value string)
 		}
 		return true
 	}
-	if setting.Value == value {
-		return false
+
+	configMapResourceVersion := ""
+	if setting.Annotations != nil {
+		configMapResourceVersion = setting.Annotations[types.GetLonghornLabelKey(types.ConfigMapResourceVersionKey)]
 	}
-	return true
+
+	// Also check the setting definition.
+	definition, ok := types.GetSettingDefinition(name)
+	if !ok {
+		logrus.Errorf("customized default setting %v is not defined", name)
+		return true
+	}
+
+	return (setting.Value != value || configMapResourceVersion != defaultSettingCMResourceVersion || definition.Default != value)
 }
 
 func (s *DataStore) syncSettingsWithDefaultImages(defaultImages map[types.SettingName]string) error {
@@ -197,16 +206,6 @@ func (s *DataStore) applyCustomizedDefaultSettingsToDefinitions(customizedDefaul
 
 func (s *DataStore) syncSettingCRsWithCustomizedDefaultSettings(customizedDefaultSettings map[string]string, defaultSettingCMResourceVersion string) error {
 	for _, sName := range types.SettingNameList {
-		configMapResourceVersion := ""
-		if s, err := s.GetSettingExactRO(sName); err != nil {
-			if !apierrors.IsNotFound(err) {
-				return err
-			}
-		} else {
-			if s.Annotations != nil {
-				configMapResourceVersion = s.Annotations[types.GetLonghornLabelKey(types.ConfigMapResourceVersionKey)]
-			}
-		}
 
 		definition, ok := types.GetSettingDefinition(sName)
 		if !ok {
@@ -218,13 +217,12 @@ func (s *DataStore) syncSettingCRsWithCustomizedDefaultSettings(customizedDefaul
 			continue
 		}
 
-		if configMapResourceVersion != defaultSettingCMResourceVersion {
-			if definition.Required && value == "" {
-				continue
-			}
-			if err := s.createOrUpdateSetting(sName, value, defaultSettingCMResourceVersion); err != nil {
-				return err
-			}
+		if definition.Required && value == "" {
+			continue
+		}
+
+		if err := s.createOrUpdateSetting(sName, value, defaultSettingCMResourceVersion); err != nil {
+			return err
 		}
 	}