Allow to override clone and standby description

[farodin91]

Fixes #1902 unable to override scope prefix

[farodin91]

@FxKu Would you like to review?

[farodin91]

@sdudoladov Would you like to review?

[FxKu]

@farodin91 the clone and standby description would be overridable if you move line 910 - 916 before line 939. Then cluster-specfic env var as well as environment configmap or secret come first. No need to add another overridableEnvs list.

You would have to adjust unit test though, because order of variables and overriding behavior changes. The new order must be reflected in the docs, too (2. and 3. becoming 5. and 6.).

[farodin91]

@FxKu I updated the behavior.

[farodin91]

@FxKu Do you have time to look into it?

[FxKu]

@farodin91 yes. LGTM. Will merge next week when I‘m back from vacation.

[FxKu]

@farodin91 can you rebase the PR, please? #2045 introduced some minor changes to env var generation. Thank you :)

[farodin91]

@FxKu Rebased.

[FxKu]

@farodin91 Thanks! I gave this a second thought and now having doubts that this is right approach. Global config (like from pod_environment_configmap/secret) should not override local config (like clone and standby sections). Especially for CloneDescription we already have fields like S3AccessKeyId where one would expect that it overrides CLONE_AWS_ACCESS_KEY_ID from a global config map.

In that regard, having spec.Env before generateCloneEnvironment etc. is fine. This would also allow you to override the CLONE_WAL_BUCKET_SCOPE_PREFIX. I know, doing this in every manifest is not what you aim for.

Why do you want to set the prefix in the first place? Note, that it has to be empty when wal path is set (manifest or config). This was the behavior ever since except for version 1.8.0 and 1.8.1 where it was forgotten by mistake. Is it empty in your case? That's the only case I see now, where we should not set it to "". Are there more fields you want to override from what is set in generateCloneEnvironment?

[farodin91]

For me, it just CLONE_WAL_BUCKET_SCOPE_PREFIX. I could built a code section to set empty if not set, at the end.

We use the prefix for the cluster name.

[FxKu]

with #2159 merged I would close this PR now. You can now override CLONE_WAL_BUCKET_SCOPE_PREFIX per cluster in via manifest Env section.

In order to do it globally, we would have to move only the _PREFIX setting from generate[Clone|Standby]Environment about after fetching variables from secret / config map. Feel free to reopen if you want to continue working on it.

Thanks again for bringing it up.