Skip to content
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

Allow configuration of global_sidecars / sidecar_docker_images #781

Closed
frittentheke opened this issue Jan 6, 2020 · 4 comments
Closed

Comments

@frittentheke
Copy link
Contributor

It's awesome to be able to "simply" inject sidecars globally via sidecar_docker_images of the operator configuration (#331).

Unfortunately there is no way to globally configure those sidecars yet ...

  1. While there is the pod_environment_configmap setting, the referenced config map has to exist in the namespace the postgresql cluster resides in (https://postgres-operator.readthedocs.io/en/latest/reference/operator_parameters/#kubernetes-resources)
  2. The variables in this config map are only propagated to the postgresql container (not actually the whole pod / all containers) as the name of the variable would suggest: Environment set by pod_environment_configmap not used in restore_script.sh #550

Certainly there are many use cases for globally applied sidecars, but mostly it's about applying common monitoring / logshipping, like discussed here #264.

I just ran into the same issue of being able to globally add the sidecars, but being unable to apply some common environment variables to configure those (see comment #264 (comment))

Easiest would likely be to allow configuration of global sidecar containers just like individual postgresql custom resources (https://postgres-operator.readthedocs.io/en/latest/reference/cluster_manifest/#sidecar-definitions).

While fully exposing all PodSpec settings (#479) does work as well, it is harder to validate against the postgresql CRD.

@FxKu
Copy link
Member

FxKu commented Jan 7, 2020

I think propagating the content of the pod_environment_configmap to all sidecars should be relatively easy to implement. Have nothing against this idea. As you said, the name already sounds like it would be available to all containers. Feel free to open PR.

@frittentheke
Copy link
Contributor Author

frittentheke commented Jan 7, 2020

@FxKu thanks for looking into this.

While propagation of pod_environment_configmap to all containers makes sense in itself, this still means such a configmap has to exist in each namespace to "solve " the issue of configuring the global sidecars. I'd rather have this configuration central in the operator configuration, next to where the sidecars are defined. Do you have any thoughts on that?

@FxKu
Copy link
Member

FxKu commented Jan 9, 2020

I wonder if the type of PodEnvironmentConfigMap could be changed to spec.NamespacedName like we have it elsewhere. Then we simply extract the namespace from the given parameter or use default, if not set. Here's the codeline.

@FxKu
Copy link
Member

FxKu commented May 4, 2020

Can be closed now with #890 merged.

@FxKu FxKu closed this as completed May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants