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 ability to adjust patroni log setting (#1203) #1857

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mhaley-tignis
Copy link

The operator produces its own SPILO_CONFIGURATION based on the information provided in the postgresql spec. This PR adds a patroni.log section to the spec to allow one to set log configuration for patroni. My end goal is to use this new feature to have patroni output to a log file within a container. I will then use another container in the same pod to scrape those process logs (promtail) and send them to our log storage (loki).

If you like these changes, I will go ahead and update the docs and example too.

Related: I have a PR up for spilo too so that we can configure logs via environment variables (something that this operator already supports) see zalando/spilo#719.

* Update postgresql CRD to include patroni log configuration
* Have operator include log config (if available) in the `SPILO_CONFIGURATION`
environment variable.
@mhaley-tignis
Copy link
Author

@FxKu picking someone at random to ping. Is there something more I should do to have someone review this PR? The e2e tests are failing because the autogenerate needs to run. I spent a couple hours trying to run it myself without any luck. The update-codegen.sh and the generate-groups.sh script run to completion without error, but they never actually spit anything out.

@mhaley-tignis
Copy link
Author

@CyberDem0n perhaps maybe you can take a look at this PR?

@mhaley-tignis
Copy link
Author

@sdudoladov @Jan-M @CyberDem0n @FxKu @jopadi hello? Can anyone lend me a hand? I would like to move this PR along so I don't need to continue to maintain my own fork of the library. Thank you!

@jehof
Copy link

jehof commented Jul 22, 2022

Is this MR obsolete with the addition of #1794 in release 1.8 of the operator? I may be wrong but now you should be able to define patroni configuration using Environment Variables and these should have a higher precedence.

@mhaley-tignis
Copy link
Author

Is this MR obsolete with the addition of #1794 in release 1.8 of the operator? I may be wrong but now you should be able to define patroni configuration using Environment Variables and these should have a higher precedence.

I was always able to define patroni config using environment variables, but they are ignored by the spilo startup scripts (see zalando/spilo#719).

The suggestion zalando/spilo#718 (comment) to set the SPILO_CONFIGURATION is not possible because this operator does not let you set that yourself. Thus, the PR to expand the options available to include the ability to set your own log configuration too.

I either need this PR or this zalando/spilo#719 one merged in order to be able to set patroni log settings.

@mhaley-tignis
Copy link
Author

One last attempt. @sdudoladov @Jan-M @CyberDem0n @FxKu @jopadi, is someone able to have a look here? Thanks in advance!

@JulesLalu
Copy link

JulesLalu commented Oct 17, 2023

I am interested in this feature too, it's a shame that no one has reviewed this PR since it was opened a year and a half ago..

@mhaley-tignis
Copy link
Author

@FxKu I noticed you made a release a few week ago. Any chance you have a few moments to review this PR?

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.

3 participants