[DPE-5833] Create new PostgreSQL version 16 charm #788

lucasgameiroborges · 2024-11-26T19:56:08Z

Part of the 4-PR bundle to merge PostgreSQL 16. See doc for further info: https://docs.google.com/document/d/1K4xMxKDOiHfdCTc_hxMh6bW1DCZ6HC6wa6g8tAJAtno/edit?tab=t.0

codecov · 2024-11-26T20:54:58Z

Codecov Report

Attention: Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Project coverage is 75.53%. Comparing base (ab80bae) to head (66cf401).

Files with missing lines	Patch %	Lines
src/charm.py	40.00%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           16/edge     #788      +/-   ##
===========================================
- Coverage    75.53%   75.53%   -0.01%     
===========================================
  Files           12       12              
  Lines         3115     3123       +8     
  Branches       466      468       +2     
===========================================
+ Hits          2353     2359       +6     
  Misses         621      621              
- Partials       141      143       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lucasgameiroborges · 2025-02-11T14:55:30Z

.github/workflows/ci.yaml


  integration-test:
    strategy:
      fail-fast: false
      matrix:
        juju:
-          - agent: 2.9.51  # renovate: juju-agent-pin-minor
-            libjuju: ==2.9.49.1  # renovate: latest libjuju 2
-            allure_on_amd64: false


no more juju 2

lucasgameiroborges · 2025-02-11T14:57:17Z

lib/charms/postgresql_k8s/v0/postgresql.py

                    for extension, enable in ordered_extensions.items():
+                        if extension == "postgis":
+                            cursor.execute("SET pgaudit.log = 'none';")


necessary for postgis to work

Interesting. So, postgis and pgaudit don't work together?

lucasgameiroborges · 2025-02-11T14:57:49Z

templates/patroni.yml.j2

+      sslrootcert: {{ conf_path }}/ca.pem
+      sslcert: {{ conf_path }}/cert.pem
+      sslkey: {{ conf_path }}/key.pem
+      {%- endif %}


no idea why this wasn't an issue before.

lucasgameiroborges · 2025-02-11T14:58:21Z

tests/integration/conftest.py


 @pytest.fixture(scope="module")
 async def database_charm(ops_test: OpsTest):
    """Build the database charm."""
-    charm = await ops_test.build_charm(".")
+    charm = await build_charm(".")


workaround for multi-base integration testing (currently hardcoded to jammy)

lucasgameiroborges · 2025-02-11T14:59:23Z

tests/integration/ha_tests/test_replication.py

@@ -43,7 +42,6 @@ async def test_build_and_deploy(ops_test: OpsTest) -> None:
            await ops_test.model.deploy(
                APPLICATION_NAME,
                application_name=APPLICATION_NAME,
-                base=CHARM_BASE,


this parameter still doesn't play nice with noble base.

lucasgameiroborges · 2025-02-11T15:00:36Z

lib/charms/postgresql_k8s/v0/postgresql.py

@@ -372,6 +374,7 @@ def _generate_database_privileges_statements(
    ) -> List[Composed]:
        """Generates a list of databases privileges statements."""
        statements = []
+        statements.append(SQL("GRANT USAGE, CREATE ON SCHEMA public TO PUBLIC;"))


Here and below are the specific workarounds mentioned in the doc for the new restrictive permissions added on PG 15/16.

marceloneppel

Great work, Lucas! Thanks for all those updates to the code to match what's needed for PG 16.

marceloneppel · 2025-02-13T14:59:22Z

tests/integration/ha_tests/test_upgrade_to_primary_label.py

@@ -17,6 +17,7 @@
    CHARM_SERIES,
    DATABASE_APP_NAME,
    METADATA,
+    build_charm,


This test file and tests/integration/ha_tests/test_rollback_to_master_label.py can be removed as we don't have a previous version without the primary label for PG 16.

marceloneppel · 2025-02-13T15:01:50Z

tests/integration/test_plugins.py

+ORAFCE_EXTENSION_STATEMENT = "SELECT oracle.add_months(date '2005-05-31',1);"
+PG_SIMILARITY_EXTENSION_STATEMENT = (
+    "SET pg_similarity.levenshtein_threshold = 0.7; SELECT 'aaa', 'aab', lev('aaa','aab');"
+)


Do these statements also work on PG 14? If so, I think we can create a ticket to remember to backport it later.

marceloneppel · 2025-02-13T15:08:21Z

lib/charms/postgresql_k8s/v0/postgresql.py

                    for extension, enable in ordered_extensions.items():
+                        if extension == "postgis":
+                            cursor.execute("SET pgaudit.log = 'none';")


Interesting. So, postgis and pgaudit don't work together?

marceloneppel · 2025-02-13T15:09:37Z

src/patroni.py

@@ -534,7 +534,7 @@ def render_patroni_yml_file(
        )
        self._render_file(f"{self._storage_path}/patroni.yml", rendered, 0o644)

-    @retry(stop=stop_after_attempt(10), wait=wait_exponential(multiplier=1, min=2, max=30))
+    @retry(stop=stop_after_attempt(20), wait=wait_exponential(multiplier=1, min=2, max=30))


Did you have a link to a failing test that motivated this change? Did it happen multiple times?

marceloneppel · 2025-02-13T18:08:44Z

lib/charms/postgresql_k8s/v0/postgresql.py

+                    SQL("GRANT USAGE, CREATE ON SCHEMA {} TO {};").format(
+                        schema, Identifier(user)
+                    ),
+                    SQL("GRANT USAGE, CREATE ON SCHEMA {} TO admin;").format(schema),


I checked the PostgreSQL 15 release notes and changed my mind about the approach here. Now, I think that it's important that we don't grant those permissions to users on the public schema to avoid CVE-2018-1058.

marceloneppel · 2025-02-13T18:11:48Z

metadata.yaml

@@ -28,7 +28,7 @@ resources:
  postgresql-image:
    type: oci-image
    description: OCI image for PostgreSQL
-    upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:7e41d7f60e45ee2f5463aa9aafcd3c35121423423ee08c26a174b99ad0235b7e # renovate: oci-image tag: 14.15-22.04_edge
+    upstream-source: ghcr.io/canonical/charmed-postgresql:16.4-24.04_edge


I'd prefer to keep the SHA256 digest here because it'll prevent us from publishing the charm with a newer and not yet tested version of the OCI image.

try using pg 16 rock

62e073d

github-actions bot added the Libraries: Out of sync label Nov 26, 2024

lucasgameiroborges and others added 18 commits November 26, 2024 20:59

try fix integration tests

ae2446c

remove argument on helper

485fe58

more fixes

df73c21

Merge remote-tracking branch 'origin/16/edge' into test-pg-16

1a899bb

fix test issues

4b076b7

REVERT LATER: Adapt build_charm function

75560d9

remove base=CHARM_BASE from deploy

78b4df6

remove juju 2.9 tests

4151296

grat permissions to public schema

08dade0

fix linting and unit test

9110cef

try refactor the permissions change

560f1eb

grant public access to public schemas

adafd4d

mark smoke test as unstable

c9cbf70

test release charm to 16/edge

ba536a0

fix test release workflow

efb6603

fix typo

d38b768

revert metadata.yaml

f2d3bab

nits

66cf401

lucasgameiroborges commented Feb 11, 2025

View reviewed changes

lucasgameiroborges requested review from dragomirp, taurus-forever and marceloneppel February 11, 2025 15:00

lucasgameiroborges marked this pull request as ready for review February 11, 2025 15:01

marceloneppel approved these changes Feb 13, 2025

View reviewed changes

marceloneppel self-requested a review February 13, 2025 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DPE-5833] Create new PostgreSQL version 16 charm #788

[DPE-5833] Create new PostgreSQL version 16 charm #788

lucasgameiroborges commented Nov 26, 2024 •

edited

Loading

codecov bot commented Nov 26, 2024 •

edited

Loading

lucasgameiroborges Feb 11, 2025

lucasgameiroborges Feb 11, 2025

marceloneppel Feb 13, 2025

lucasgameiroborges Feb 11, 2025

lucasgameiroborges Feb 11, 2025

lucasgameiroborges Feb 11, 2025

lucasgameiroborges Feb 11, 2025

marceloneppel left a comment

marceloneppel Feb 13, 2025

marceloneppel Feb 13, 2025

marceloneppel Feb 13, 2025

marceloneppel Feb 13, 2025

marceloneppel Feb 13, 2025

marceloneppel Feb 13, 2025

[DPE-5833] Create new PostgreSQL version 16 charm #788

Are you sure you want to change the base?

[DPE-5833] Create new PostgreSQL version 16 charm #788

Conversation

lucasgameiroborges commented Nov 26, 2024 • edited Loading

codecov bot commented Nov 26, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marceloneppel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasgameiroborges commented Nov 26, 2024 •

edited

Loading

codecov bot commented Nov 26, 2024 •

edited

Loading