Skip to content

Commit

Permalink
fix(setting): check both setting and definition default for possible …
Browse files Browse the repository at this point in the history
…change.

Signed-off-by: James Munson <james.munson@suse.com>
  • Loading branch information
james-munson committed Mar 6, 2024
1 parent 5d12108 commit bd0c0c5
Showing 1 changed file with 23 additions and 25 deletions.
48 changes: 23 additions & 25 deletions datastore/longhorn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}

Expand Down

0 comments on commit bd0c0c5

Please sign in to comment.