-
Notifications
You must be signed in to change notification settings - Fork 1k
refactor spilo env var generation #1848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
} | ||
|
||
if len(memberOf) > 1 { | ||
if len(memberOf) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bugfix. When there's only one DB owner it should also work
|
||
result = append(result, v1.EnvVar{Name: "CLONE_METHOD", Value: "CLONE_WITH_WALE"}) | ||
result = append(result, v1.EnvVar{Name: "CLONE_TARGET_TIME", Value: description.EndTimestamp}) | ||
result = append(result, v1.EnvVar{Name: "CLONE_WAL_BUCKET_SCOPE_PREFIX", Value: ""}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only append it once. When S3WalPath is set, prefix must be empty, hence moved to else block above
Value: c.OpConfig.GCPCredentials, | ||
}, | ||
} | ||
result = append(result, envs...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot do similar to what we do in cloning. There users have the choice to overwrite the WAL location with S3WalPath but can stick to operator config to generate the path. However, in standby there's only one way possible: Explicit mention of WAL path. Therefore, we should not use c.OpConfig.GCPCredentials
here but must rely on the user to define it. This way, GCPCredentials
from config could also be overridden which is otherwise not possible.
customPodEnvVarsList := append(configMapEnvVarsList, secretEnvVarsList...) | ||
sort.Slice(customPodEnvVarsList, | ||
func(i, j int) bool { return customPodEnvVarsList[i].Name < customPodEnvVarsList[j].Name }) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved inside of generateSpiloPodEnvVars
. Plus, if fetching configMap or secret fails there will be a warning but no error returned
👍 |
1 similar comment
👍 |
👍 |
1 similar comment
👍 |
Do everything related to env var of DB pods inside
GenerateSpiloPodEnvVars
and extend unit tests. Plus, I shuffled around unit tests in k8sres_test so that tests covering the same topics are next to each other.incl. fix for setting cron_admin when there's only one owner to grant it to