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-2617 Basic profile support #229

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,11 @@ options:
default: false
type: boolean
description: Enable unaccent extension
profile:
description: |
Profile representing the scope of deployment, and used to tune resource allocation.
Allowed values are: “production” and “testing”.
Production will tune postgresql for maximum performance while testing will tune for
minimal running performance.
type: string
default: production
21 changes: 19 additions & 2 deletions lib/charms/postgresql_k8s/v0/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
Any charm using this library should import the `psycopg2` or `psycopg2-binary` dependency.
"""
import logging
from typing import List, Set, Tuple
from typing import List, Optional, Set, Tuple

import psycopg2
from psycopg2 import sql
Expand All @@ -32,7 +32,7 @@

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 12
LIBPATCH = 13

INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"

Expand Down Expand Up @@ -419,3 +419,20 @@ def update_user_password(self, username: str, password: str) -> None:
finally:
if connection is not None:
connection.close()

@staticmethod
def build_postgresql_parameters(
profile: str, available_memory: int
) -> Optional[dict[str, str]]:
"""Builds the PostgreSQL parameters.

Args:
profile: the profile to use.
available_memory: available memory to use in calculation

Returns:
Dictionary with the PostgreSQL parameters.
"""
logger.debug(f"Building PostgreSQL parameters for {profile=} and {available_memory=}")
if profile == "production":
return {"shared_buffers": "987MB", "effective_cache_size": "2958MB"}
1 change: 1 addition & 0 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,7 @@ def update_config(self, is_creating_backup: bool = False) -> bool:
backup_id=self.app_peer_data.get("restoring-backup"),
stanza=self.app_peer_data.get("stanza"),
restore_stanza=self.app_peer_data.get("restore-stanza"),
parameters=self.postgresql.build_postgresql_parameters(self.config["profile"], 0),
)
if not self._is_workload_running:
# If Patroni/PostgreSQL has not started yet and TLS relations was initialised,
Expand Down
5 changes: 3 additions & 2 deletions src/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ class UpdateSyncNodeCountError(Exception):
class Patroni:
"""This class handles the bootstrap of a PostgreSQL database through Patroni."""

pass

def __init__(
self,
unit_ip: str,
Expand Down Expand Up @@ -447,6 +445,7 @@ def render_patroni_yml_file(
stanza: str = None,
restore_stanza: Optional[str] = None,
backup_id: Optional[str] = None,
parameters: Optional[dict[str, str]] = None,
) -> None:
"""Render the Patroni configuration file.

Expand All @@ -457,6 +456,7 @@ def render_patroni_yml_file(
stanza: name of the stanza created by pgBackRest.
restore_stanza: name of the stanza used when restoring a backup.
backup_id: id of the backup that is being restored.
parameters: PostgreSQL parameters to be added to the postgresql.conf file.
"""
# Open the template patroni.yml file.
with open("templates/patroni.yml.j2", "r") as file:
Expand Down Expand Up @@ -486,6 +486,7 @@ def render_patroni_yml_file(
restore_stanza=restore_stanza,
version=self.get_postgresql_version().split(".")[0],
minority_count=self.planned_units // 2,
pg_parameters=parameters,
)
self.render_file(f"{PATRONI_CONF_PATH}/patroni.yaml", rendered, 0o600)

Expand Down
5 changes: 5 additions & 0 deletions templates/patroni.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ bootstrap:
{%- endif %}
archive_mode: on
wal_level: logical
{%- if pg_parameters %}
{%- for key, value in pg_parameters.items() %}
{{key}}: {{value}}
{%- endfor -%}
{% endif %}
Comment on lines +67 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we pass an empty dict as a default pg_parameters we can remove the outer if, but that can be changed later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm think I've tested this, but will recheck

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm think I've tested this, but will recheck

Not worth rerunning CI over it.


{%- if restoring_backup %}
method: pgbackrest
Expand Down
13 changes: 11 additions & 2 deletions tests/integration/ha_tests/test_replication.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ async def test_build_and_deploy(ops_test: OpsTest) -> None:
wait_for_apps = True
charm = await ops_test.build_charm(".")
async with ops_test.fast_forward():
await ops_test.model.deploy(charm, num_units=3, series=CHARM_SERIES)
await ops_test.model.deploy(
charm,
num_units=3,
series=CHARM_SERIES,
config={"profile": "testing"},
)
# Deploy the continuous writes application charm if it wasn't already deployed.
if not await app_name(ops_test, APPLICATION_NAME):
wait_for_apps = True
Expand Down Expand Up @@ -111,7 +116,11 @@ async def test_no_data_replicated_between_clusters(ops_test: OpsTest, continuous
charm = await ops_test.build_charm(".")
async with ops_test.fast_forward():
await ops_test.model.deploy(
charm, application_name=new_cluster_app, num_units=2, series=CHARM_SERIES
charm,
application_name=new_cluster_app,
num_units=2,
series=CHARM_SERIES,
config={"profile": "testing"},
)
await ops_test.model.wait_for_idle(apps=[new_cluster_app], status="active")

Expand Down
7 changes: 6 additions & 1 deletion tests/integration/ha_tests/test_restore_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,16 @@ async def test_build_and_deploy(ops_test: OpsTest) -> None:
num_units=3,
series=CHARM_SERIES,
storage={"pgdata": {"pool": "lxd-btrfs", "size": 2048}},
config={"profile": "testing"},
)

# Deploy the second cluster
await ops_test.model.deploy(
charm, application_name=SECOND_APPLICATION, num_units=1, series=CHARM_SERIES
charm,
application_name=SECOND_APPLICATION,
num_units=1,
series=CHARM_SERIES,
config={"profile": "testing"},
)

await ops_test.model.wait_for_idle(status="active", timeout=1000)
Expand Down
1 change: 1 addition & 0 deletions tests/integration/ha_tests/test_self_healing.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ async def test_build_and_deploy(ops_test: OpsTest) -> None:
num_units=3,
series=CHARM_SERIES,
storage={"pgdata": {"pool": "lxd-btrfs", "size": 2048}},
config={"profile": "testing"},
)
# Deploy the continuous writes application charm if it wasn't already deployed.
if not await app_name(ops_test, APPLICATION_NAME):
Expand Down
1 change: 1 addition & 0 deletions tests/integration/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ async def test_mailman3_core_db(ops_test: OpsTest, charm: str) -> None:
application_name=DATABASE_APP_NAME,
num_units=DATABASE_UNITS,
series=CHARM_SERIES,
config={"profile": "testing"},
)

# Wait until the PostgreSQL charm is successfully deployed.
Expand Down
1 change: 1 addition & 0 deletions tests/integration/test_password_rotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ async def test_deploy_active(ops_test: OpsTest):
application_name=APP_NAME,
num_units=3,
series=CHARM_SERIES,
config={"profile": "testing"},
)
await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000)

Expand Down
1 change: 1 addition & 0 deletions tests/integration/test_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ async def test_deploy_active(ops_test: OpsTest):
application_name=APP_NAME,
num_units=3,
series=CHARM_SERIES,
config={"profile": "testing"},
)
# No wait between deploying charms, since we can't guarantee users will wait. Furthermore,
# bundles don't wait between deploying charms.
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,7 @@ def test_update_config(
postgresql_mock.is_tls_enabled = PropertyMock(side_effect=[False, False, False, False])
_is_workload_running.side_effect = [True, True, False, True]
_member_started.side_effect = [True, True, False]
postgresql_mock.build_postgresql_parameters.return_value = {"test": "test"}

# Test without TLS files available.
self.harness.update_relation_data(
Expand All @@ -1067,6 +1068,7 @@ def test_update_config(
backup_id=None,
stanza=None,
restore_stanza=None,
parameters={"test": "test"},
)
_reload_patroni_configuration.assert_called_once()
_restart.assert_not_called()
Expand All @@ -1089,6 +1091,7 @@ def test_update_config(
backup_id=None,
stanza=None,
restore_stanza=None,
parameters={"test": "test"},
)
_reload_patroni_configuration.assert_called_once()
_restart.assert_called_once()
Expand Down