Skip to content

Conversation

@mastercactapus
Copy link

This pull request introduces a minor improvement to the Keys method in the Settings struct by ensuring the returned list of keys is sorted alphabetically.

This is to correct an issue we found where pods are re-created by the stateful set for (possibly) every reconcile due to the generated volume mounts getting re-ordered (e.g., using a secret for TLS key/cert as documented here) and possibly others.

The value of .Keys() is used in a few different places; notably by WalkKeysSafe, used by WalkSafe, and finally normalizeConfigurationFiles. Since Go map iterations are random otherwise, sorting them here means the WalkSafe method will "walk" in alphabetical order, making the generated mounts/volume order stable.

// normalizeConfigurationFiles normalizes .spec.configuration.files
func (n *Normalizer) normalizeConfigurationFiles(files *chi.Settings, scope any) *chi.Settings {
	if files == nil {
		return nil
	}
	files.Normalize(n.settingsNormalizerOptions(replacerFiles, scope))

	files.WalkSafe(func(key string, setting *chi.Setting) {
		subst.ReplaceSettingsFieldWithMountedFile(n.req, files, key)
	})

	return files
}
image

Thanks for taking the time to contribute to clickhouse-operator!

Please, read carefully instructions on how to make a Pull Request.

This will help a lot for maintainers to adopt your Pull Request.

Important items to consider before making a Pull Request

Please check items PR complies to:

  • All commits in the PR are squashed. More info
  • The PR is made into dedicated next-release branch, not into master branch1. More info
  • The PR is signed. More info

--

1 If you feel your PR does not affect any Go-code or any testable functionality (for example, PR contains docs only or supplementary materials), PR can be made into master branch, but it has to be confirmed by project's maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant