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

[DPE-1781] Configuration support #239

Merged
merged 13 commits into from
Oct 24, 2023
Merged

Conversation

marceloneppel
Copy link
Member

@marceloneppel marceloneppel commented Oct 5, 2023

Issue

There is no way to provide custom configurations to the workload (PostgreSQL database).

Solution

Add to patroni.yaml the requested configurations with their default values.

Expose the allowed configurations through charm config options (and add validators based on the allowed values).

max_connections and max_prepared_transactions can be changed only through the Patroni API, so they are changed in the charm code differently from the other configurations.

Also, added a call to time.sleep(10) to give some time to Patroni to reload the configurations before it's possible to know which ones need a database restart. Previously, sometimes a change in shared_buffers wouldn't trigger a restart.

@marceloneppel marceloneppel changed the title (WIP) [DPE-1781] Configuration support [DPE-1781] Configuration support Oct 18, 2023
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #239 (c35ecb6) into main (1a1540a) will decrease coverage by 0.50%.
The diff coverage is 73.49%.

❗ Current head c35ecb6 differs from pull request most recent head 44d9d4f. Consider uploading reports for the commit 44d9d4f to get more accurate results

@@            Coverage Diff             @@
##             main     #239      +/-   ##
==========================================
- Coverage   79.41%   78.92%   -0.50%     
==========================================
  Files          10       10              
  Lines        1987     2149     +162     
  Branches      327      349      +22     
==========================================
+ Hits         1578     1696     +118     
- Misses        356      383      +27     
- Partials       53       70      +17     
Files Coverage Δ
src/constants.py 100.00% <100.00%> (ø)
src/cluster.py 75.47% <75.00%> (-0.11%) ⬇️
src/charm.py 72.41% <62.50%> (-0.28%) ⬇️
src/config.py 75.55% <74.48%> (-4.45%) ⬇️

Copy link
Contributor

@taurus-forever taurus-forever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

JFMI, why don't we set defaults in config.yaml?

@marceloneppel
Copy link
Member Author

LGTM.

JFMI, why don't we set defaults in config.yaml?

No specific reason. I think it's a good idea to put them. I'll update the code.

@marceloneppel
Copy link
Member Author

LGTM.

JFMI, why don't we set defaults in config.yaml?

I added the default values and the feedback from the K8S charm PR to efc7c8f.

@marceloneppel marceloneppel merged commit f747ab7 into main Oct 24, 2023
32 checks passed
@marceloneppel marceloneppel deleted the dpe-1781-configuration-support branch October 24, 2023 11:43
BON4 pushed a commit to BON4/postgresql-operator that referenced this pull request Apr 23, 2024
* Add missing parameters and add configuration support

* Use fixed snap

* Fix parameters validation

* Add validations

* Update library

* Revert changes on charm.py

* Add configuration logic and tests

* PR feedback (including the feedback from canonical/postgresql-k8s-operator#281)

* Fix TLS tests

* Increase timeout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants