From 62e073d7bee04707002b1e7260a3ade8c0e759b5 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 26 Nov 2024 19:54:49 +0000 Subject: [PATCH 01/18] try using pg 16 rock --- .github/workflows/ci.yaml | 5 +++-- .github/workflows/release.yaml | 13 ++++++++---- CONTRIBUTING.md | 2 +- README.md | 6 +++--- charmcraft.yaml | 12 +++++------ lib/charms/postgresql_k8s/v0/postgresql.py | 19 +++++++++--------- metadata.yaml | 2 +- poetry.lock | 8 ++++---- pyproject.toml | 2 +- src/charm.py | 16 +++++++++++++-- src/dependency.json | 4 ++-- src/patroni.py | 2 +- templates/patroni.yml.j2 | 15 ++++++++++++++ terraform/variables.tf | 4 ++-- .../ha_tests/test_rollback_to_master_label.py | 2 +- tests/integration/ha_tests/test_smoke.py | 4 ++-- tests/integration/ha_tests/test_upgrade.py | 6 +++++- .../ha_tests/test_upgrade_from_stable.py | 7 +++++-- .../ha_tests/test_upgrade_to_primary_label.py | 4 +++- tests/integration/helpers.py | 4 ++-- tests/integration/test_db.py | 3 --- tests/integration/test_plugins.py | 6 ++++-- tests/integration/test_tls.py | 5 ++--- tests/unit/test_async_replication.py | 20 +++++++++++++++---- tests/unit/test_backups.py | 2 +- tests/unit/test_charm.py | 5 +++-- tests/unit/test_db.py | 2 +- tests/unit/test_patroni.py | 6 +++--- tests/unit/test_postgresql.py | 18 ++++++++--------- tests/unit/test_postgresql_provider.py | 2 +- 30 files changed, 128 insertions(+), 78 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2c5603839d..f6b1b2f0e4 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -45,9 +45,10 @@ jobs: build: name: Build charm - uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v23.0.5 + uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@lucas/fix-collect-bases with: - cache: true + cache: false + charmcraft-snap-channel: 3.x/stable integration-test: strategy: diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 5744f01eb6..8fad9431a2 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -5,7 +5,7 @@ name: Release to Charmhub on: push: branches: - - main + - 16/edge paths-ignore: - 'tests/**' - 'docs/**' @@ -14,6 +14,8 @@ on: - '.github/workflows/ci.yaml' - '.github/workflows/lib-check.yaml' - '.github/workflows/sync_docs.yaml' + # for testing purposes: + workflow_dispatch: jobs: ci-tests: @@ -42,16 +44,19 @@ jobs: build: name: Build charm - uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v23.0.5 + uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@lucas/fix-collect-bases + with: + cache: false + charmcraft-snap-channel: 3.x/stable release: name: Release charm needs: - ci-tests - build - uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v23.0.5 + uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@lucas/fix-collect-bases with: - channel: 14/edge + channel: 16/edge artifact-prefix: ${{ needs.build.outputs.artifact-prefix }} secrets: charmhub-token: ${{ secrets.CHARMHUB_TOKEN }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 30d41cfae8..b2cbbce897 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -65,7 +65,7 @@ juju model-config logging-config="=INFO;unit=DEBUG" microk8s enable rbac # Deploy the charm -juju deploy ./postgresql-k8s_ubuntu-22.04-amd64.charm --trust \ +juju deploy ./postgresql-k8s_ubuntu-24.04-amd64.charm --trust \ --resource postgresql-image=$(yq '(.resources.postgresql-image.upstream-source)' metadata.yaml) ``` diff --git a/README.md b/README.md index e5abc71343..3a5583dcf4 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Bootstrap a Kubernetes (e.g. [Multipass-based MicroK8s](https://discourse.charmh ```shell juju add-model postgresql-k8s -juju deploy postgresql-k8s --channel 14 --trust +juju deploy postgresql-k8s --channel 16/edge --trust ``` **Note:** the `--trust` flag is required because the charm and Patroni need to create some K8s resources. @@ -62,7 +62,7 @@ Adding a relation is accomplished with `juju relate` (or `juju integrate` for Ju ```shell # Deploy Charmed PostgreSQL cluster with 3 nodes -juju deploy postgresql-k8s -n 3 --trust --channel 14 +juju deploy postgresql-k8s -n 3 --trust --channel 16/edge # Deploy the relevant application charms juju deploy mycharm @@ -87,7 +87,7 @@ juju status --relations This charm supports legacy interface `pgsql` from the previous [PostgreSQL charm](https://launchpad.net/postgresql-charm): ```shell -juju deploy postgresql-k8s --trust --channel 14 +juju deploy postgresql-k8s --trust --channel 16/edge juju deploy finos-waltz-k8s --channel edge juju relate postgresql-k8s:db finos-waltz-k8s ``` diff --git a/charmcraft.yaml b/charmcraft.yaml index 38ca40857f..d560d0a9fc 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -1,11 +1,9 @@ type: charm -bases: - - name: ubuntu - channel: "22.04" - architectures: [amd64] - - name: ubuntu - channel: "22.04" - architectures: [arm64] +base: ubuntu@24.04 + +platforms: + amd64: + arm64: parts: charm: build-snaps: diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 4d8d6dc30c..1feaa72d05 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -36,7 +36,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 39 +LIBPATCH = 40 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles" @@ -311,7 +311,7 @@ def delete_user(self, user: str) -> None: logger.error(f"Failed to delete user: {e}") raise PostgreSQLDeleteUserError() - def enable_disable_extensions(self, extensions: Dict[str, bool], database: str = None) -> None: + def enable_disable_extensions(self, extensions: Dict[str, bool], database: str = None) -> None: # noqa: C901 """Enables or disables a PostgreSQL extension. Args: @@ -339,10 +339,12 @@ def enable_disable_extensions(self, extensions: Dict[str, bool], database: str = # Enable/disabled the extension in each database. for database in databases: - with self._connect_to_database( - database=database - ) as connection, connection.cursor() as cursor: + connection = self._connect_to_database(database=database) + connection.autocommit = True + with connection.cursor() as cursor: for extension, enable in ordered_extensions.items(): + if extension == "postgis": + cursor.execute("SET pgaudit.log = 'none';") cursor.execute( f"CREATE EXTENSION IF NOT EXISTS {extension};" if enable @@ -364,6 +366,7 @@ def _generate_database_privileges_statements( ) -> List[Composed]: """Generates a list of databases privileges statements.""" statements = [] + statements.append(sql.SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;")) if relations_accessing_this_database == 1: statements.append( sql.SQL( @@ -418,12 +421,10 @@ def _generate_database_privileges_statements( sql.SQL("GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA {} TO {};").format( schema, sql.Identifier(user) ), - sql.SQL("GRANT USAGE ON SCHEMA {} TO {};").format( - schema, sql.Identifier(user) - ), - sql.SQL("GRANT CREATE ON SCHEMA {} TO {};").format( + sql.SQL("GRANT USAGE, CREATE ON SCHEMA {} TO {};").format( schema, sql.Identifier(user) ), + sql.SQL("GRANT USAGE, CREATE ON SCHEMA {} TO admin;").format(schema), ]) return statements diff --git a/metadata.yaml b/metadata.yaml index cb3f49b6e4..c4025ba124 100644 --- a/metadata.yaml +++ b/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:3abdbc00413b065fbfea8c6a3aaad8e137790ebb3e7bf5e1f42e19cbc1861926 # renovate: oci-image tag: 14.13-22.04_edge + upstream-source: ghcr.io/canonical/charmed-postgresql:16.4-24.04_edge peers: database-peers: diff --git a/poetry.lock b/poetry.lock index 37b20482c7..3bcb4e3996 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.4 and should not be changed by hand. [[package]] name = "allure-pytest" @@ -1805,8 +1805,8 @@ pyyaml = "*" [package.source] type = "git" url = "https://github.com/canonical/data-platform-workflows" -reference = "v23.0.5" -resolved_reference = "e3f522c648375decee87fc0982c012e46ffb0b98" +reference = "lucas/fix-collect-bases" +resolved_reference = "c5e624e285315bac740d8c7cd9404ebc981a280d" subdirectory = "python/pytest_plugins/pytest_operator_cache" [[package]] @@ -2480,4 +2480,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "d2adca52ee53a109a292bc1237ce9152f800cc422843a29b7147483275c99070" +content-hash = "2476e9e604bce34ed14b1b4ff155613df2ab85b57c026b8df68cc375e19f391b" diff --git a/pyproject.toml b/pyproject.toml index 131b16c684..4aeb49ffba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,7 +66,7 @@ lightkube = "^0.15.5" pytest = "^8.3.3" pytest-github-secrets = {git = "https://github.com/canonical/data-platform-workflows", tag = "v23.0.5", subdirectory = "python/pytest_plugins/github_secrets"} pytest-operator = "^0.38.0" -pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", tag = "v23.0.5", subdirectory = "python/pytest_plugins/pytest_operator_cache"} +pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", branch = "lucas/fix-collect-bases", subdirectory = "python/pytest_plugins/pytest_operator_cache"} pytest-operator-groups = {git = "https://github.com/canonical/data-platform-workflows", tag = "v23.0.5", subdirectory = "python/pytest_plugins/pytest_operator_groups"} allure-pytest-collection-report = {git = "https://github.com/canonical/data-platform-workflows", tag = "v23.0.5", subdirectory = "python/pytest_plugins/allure_pytest_collection_report"} # renovate caret doesn't work: https://github.com/renovatebot/renovate/issues/26940 diff --git a/src/charm.py b/src/charm.py index 34c62fd57e..bc2cad7fa6 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1875,6 +1875,12 @@ def update_config(self, is_creating_backup: bool = False) -> bool: return True if not self._patroni.member_started: + if self.is_tls_enabled: + logger.debug( + "Early exit update_config: patroni not responding but TLS is enabled." + ) + self._handle_postgresql_restart_need(True) + return True logger.debug("Early exit update_config: Patroni not started yet") return False @@ -1936,8 +1942,14 @@ def _validate_config_options(self) -> None: def _handle_postgresql_restart_need(self): """Handle PostgreSQL restart need based on the TLS configuration and configuration changes.""" - restart_postgresql = self.is_tls_enabled != self.postgresql.is_tls_enabled() - self._patroni.reload_patroni_configuration() + if self._can_connect_to_postgresql: + restart_postgresql = self.is_tls_enabled != self.postgresql.is_tls_enabled() + else: + restart_postgresql = False + try: + self._patroni.reload_patroni_configuration() + except Exception as e: + logger.error(f"Reload patroni call failed! error: {e!s}") # Wait for some more time than the Patroni's loop_wait default value (10 seconds), # which tells how much time Patroni will wait before checking the configuration # file again to reload it. diff --git a/src/dependency.json b/src/dependency.json index fbe4dc6884..24a9c60013 100644 --- a/src/dependency.json +++ b/src/dependency.json @@ -8,7 +8,7 @@ "rock": { "dependencies": {}, "name": "charmed-postgresql", - "upgrade_supported": "^14", - "version": "14.11" + "upgrade_supported": "^16", + "version": "16.4" } } diff --git a/src/patroni.py b/src/patroni.py index 8cbb122a4d..7394b9c577 100644 --- a/src/patroni.py +++ b/src/patroni.py @@ -498,7 +498,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)) def reload_patroni_configuration(self) -> None: """Reloads the configuration after it was updated in the file.""" requests.post(f"{self._patroni_url}/reload", verify=self._verify, auth=self._patroni_auth) diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 96854216b1..716e197cde 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -155,11 +155,26 @@ postgresql: authentication: replication: password: {{ replication_password }} + {%- if enable_tls %} + sslrootcert: {{ conf_path }}/ca.pem + sslcert: {{ conf_path }}/cert.pem + sslkey: {{ conf_path }}/key.pem + {%- endif %} rewind: username: {{ rewind_user }} password: {{ rewind_password }} + {%- if enable_tls %} + sslrootcert: {{ conf_path }}/ca.pem + sslcert: {{ conf_path }}/cert.pem + sslkey: {{ conf_path }}/key.pem + {%- endif %} superuser: password: {{ superuser_password }} + {%- if enable_tls %} + sslrootcert: {{ conf_path }}/ca.pem + sslcert: {{ conf_path }}/cert.pem + sslkey: {{ conf_path }}/key.pem + {%- endif %} use_endpoints: true use_unix_socket: true {%- if is_no_sync_member or is_creating_backup %} diff --git a/terraform/variables.tf b/terraform/variables.tf index f69bd70d37..5a841c32b5 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -12,7 +12,7 @@ variable "app_name" { variable "channel" { description = "Charm channel to use when deploying" type = string - default = "14/stable" + default = "16/stable" } variable "revision" { @@ -24,7 +24,7 @@ variable "revision" { variable "base" { description = "Application base" type = string - default = "ubuntu@22.04" + default = "ubuntu@24.04" } variable "units" { diff --git a/tests/integration/ha_tests/test_rollback_to_master_label.py b/tests/integration/ha_tests/test_rollback_to_master_label.py index c76fc7a1f9..423806b17a 100644 --- a/tests/integration/ha_tests/test_rollback_to_master_label.py +++ b/tests/integration/ha_tests/test_rollback_to_master_label.py @@ -46,7 +46,7 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: ops_test.model.deploy( DATABASE_APP_NAME, num_units=3, - channel="14/stable", + channel="16/stable", revision=LABEL_REVISION, base=CHARM_BASE, trust=True, diff --git a/tests/integration/ha_tests/test_smoke.py b/tests/integration/ha_tests/test_smoke.py index 2509bcc521..5ae401cfe7 100644 --- a/tests/integration/ha_tests/test_smoke.py +++ b/tests/integration/ha_tests/test_smoke.py @@ -55,7 +55,7 @@ async def test_app_force_removal(ops_test: OpsTest): DATABASE_APP_NAME, application_name=DATABASE_APP_NAME, num_units=1, - channel="14/stable", + channel="16/stable", base=CHARM_BASE, trust=True, config={"profile": "testing"}, @@ -168,7 +168,7 @@ async def test_app_resources_conflicts(ops_test: OpsTest): DATABASE_APP_NAME, application_name=DUP_DATABASE_APP_NAME, num_units=1, - channel="14/stable", + channel="16/stable", base=CHARM_BASE, trust=True, config={"profile": "testing"}, diff --git a/tests/integration/ha_tests/test_upgrade.py b/tests/integration/ha_tests/test_upgrade.py index a8f9c2960b..8afe529a1d 100644 --- a/tests/integration/ha_tests/test_upgrade.py +++ b/tests/integration/ha_tests/test_upgrade.py @@ -35,6 +35,7 @@ @pytest.mark.group(1) +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_deploy_latest(ops_test: OpsTest) -> None: """Simple test to ensure that the PostgreSQL and application charms get deployed.""" @@ -42,7 +43,7 @@ async def test_deploy_latest(ops_test: OpsTest) -> None: ops_test.model.deploy( DATABASE_APP_NAME, num_units=3, - channel="14/edge", + channel="16/edge", trust=True, config={"profile": "testing"}, base=CHARM_BASE, @@ -66,6 +67,7 @@ async def test_deploy_latest(ops_test: OpsTest) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_pre_upgrade_check(ops_test: OpsTest) -> None: """Test that the pre-upgrade-check action runs successfully.""" @@ -93,6 +95,7 @@ async def test_pre_upgrade_check(ops_test: OpsTest) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_upgrade_from_edge(ops_test: OpsTest, continuous_writes) -> None: # Start an application that continuously writes data to the database. @@ -159,6 +162,7 @@ async def test_upgrade_from_edge(ops_test: OpsTest, continuous_writes) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_fail_and_rollback(ops_test, continuous_writes) -> None: # Start an application that continuously writes data to the database. diff --git a/tests/integration/ha_tests/test_upgrade_from_stable.py b/tests/integration/ha_tests/test_upgrade_from_stable.py index ac221930e1..0259384493 100644 --- a/tests/integration/ha_tests/test_upgrade_from_stable.py +++ b/tests/integration/ha_tests/test_upgrade_from_stable.py @@ -33,6 +33,7 @@ @pytest.mark.group(1) +@pytest.mark.unstable @markers.amd64_only # TODO: remove after arm64 stable release @pytest.mark.abort_on_fail async def test_deploy_stable(ops_test: OpsTest) -> None: @@ -41,7 +42,7 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: ops_test.model.deploy( DATABASE_APP_NAME, num_units=3, - channel="14/stable", + channel="16/stable", trust=True, base=CHARM_BASE, ), @@ -61,13 +62,14 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @markers.amd64_only # TODO: remove after arm64 stable release @pytest.mark.abort_on_fail async def test_pre_upgrade_check(ops_test: OpsTest) -> None: """Test that the pre-upgrade-check action runs successfully.""" application = ops_test.model.applications[DATABASE_APP_NAME] if "pre-upgrade-check" not in await application.get_actions(): - logger.info("skipping the test because the charm from 14/stable doesn't support upgrade") + logger.info("skipping the test because the charm from 16/stable doesn't support upgrade") return logger.info("Get leader unit") @@ -94,6 +96,7 @@ async def test_pre_upgrade_check(ops_test: OpsTest) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @markers.amd64_only # TODO: remove after arm64 stable release @pytest.mark.abort_on_fail async def test_upgrade_from_stable(ops_test: OpsTest, continuous_writes): diff --git a/tests/integration/ha_tests/test_upgrade_to_primary_label.py b/tests/integration/ha_tests/test_upgrade_to_primary_label.py index e8d3e42e64..5714e6761b 100644 --- a/tests/integration/ha_tests/test_upgrade_to_primary_label.py +++ b/tests/integration/ha_tests/test_upgrade_to_primary_label.py @@ -35,6 +35,7 @@ @pytest.mark.group(1) +@pytest.mark.unstable @markers.amd64_only # TODO: remove after arm64 stable release @pytest.mark.abort_on_fail async def test_deploy_stable(ops_test: OpsTest) -> None: @@ -49,7 +50,7 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: ops_test.model.deploy( DATABASE_APP_NAME, num_units=3, - channel="14/stable", + channel="16/stable", revision=(280 if architecture == "arm64" else 281), trust=True, **database_additional_params, @@ -74,6 +75,7 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @markers.amd64_only # TODO: remove after arm64 stable release async def test_upgrade(ops_test, continuous_writes) -> None: # Start an application that continuously writes data to the database. diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 9a0c4c5c0c..2affc12025 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -33,8 +33,8 @@ wait_fixed, ) -CHARM_BASE = "ubuntu@22.04" -CHARM_SERIES = "jammy" +CHARM_BASE = "ubuntu@24.04" +CHARM_SERIES = "noble" METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) DATABASE_APP_NAME = METADATA["name"] APPLICATION_NAME = "postgresql-test-app" diff --git a/tests/integration/test_db.py b/tests/integration/test_db.py index 658aa6b713..158c82b120 100644 --- a/tests/integration/test_db.py +++ b/tests/integration/test_db.py @@ -10,7 +10,6 @@ from . import markers from .helpers import ( APPLICATION_NAME, - CHARM_BASE, DATABASE_APP_NAME, build_and_deploy, check_database_creation, @@ -113,13 +112,11 @@ async def test_extensions_blocking(ops_test: OpsTest) -> None: await ops_test.model.deploy( APPLICATION_NAME, application_name=APPLICATION_NAME, - base=CHARM_BASE, channel="edge", ) await ops_test.model.deploy( APPLICATION_NAME, application_name=f"{APPLICATION_NAME}2", - base=CHARM_BASE, channel="edge", ) diff --git a/tests/integration/test_plugins.py b/tests/integration/test_plugins.py index 834a599ae2..d46fc252ae 100644 --- a/tests/integration/test_plugins.py +++ b/tests/integration/test_plugins.py @@ -62,8 +62,10 @@ HYPOPG_EXTENSION_STATEMENT = "CREATE TABLE hypopg_test (id integer, val text); SELECT hypopg_create_index('CREATE INDEX ON hypopg_test (id)');" IP4R_EXTENSION_STATEMENT = "CREATE TABLE ip4r_test (ip ip4);" JSONB_PLPERL_EXTENSION_STATEMENT = "CREATE OR REPLACE FUNCTION jsonb_plperl_test(val jsonb) RETURNS jsonb TRANSFORM FOR TYPE jsonb LANGUAGE plperl as $$ return $_[0]; $$;" -ORAFCE_EXTENSION_STATEMENT = "SELECT add_months(date '2005-05-31',1);" -PG_SIMILARITY_EXTENSION_STATEMENT = "SHOW pg_similarity.levenshtein_threshold;" +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');" +) PLPERL_EXTENSION_STATEMENT = "CREATE OR REPLACE FUNCTION plperl_test(name text) RETURNS text AS $$ return $_SHARED{$_[0]}; $$ LANGUAGE plperl;" PREFIX_EXTENSION_STATEMENT = "SELECT '123'::prefix_range @> '123456';" RDKIT_EXTENSION_STATEMENT = "SELECT is_valid_smiles('CCC');" diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index 5ca5d9f373..a2f1397009 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -13,7 +13,6 @@ change_patroni_setting, ) from .helpers import ( - CHARM_BASE, DATABASE_APP_NAME, build_and_deploy, check_database_creation, @@ -84,7 +83,7 @@ async def test_tls(ops_test: OpsTest) -> None: async with ops_test.fast_forward(): # Deploy TLS Certificates operator. await ops_test.model.deploy( - tls_certificates_app_name, config=tls_config, channel=tls_channel, base=CHARM_BASE + tls_certificates_app_name, config=tls_config, channel=tls_channel ) # Relate it to the PostgreSQL to enable TLS. await ops_test.model.relate(DATABASE_APP_NAME, tls_certificates_app_name) @@ -129,7 +128,7 @@ async def test_tls(ops_test: OpsTest) -> None: await run_command_on_unit( ops_test, replica, - 'su postgres -c "/usr/lib/postgresql/14/bin/pg_ctl -D /var/lib/postgresql/data/pgdata promote"', + 'su postgres -c "/usr/lib/postgresql/16/bin/pg_ctl -D /var/lib/postgresql/data/pgdata promote"', ) # Check that the replica was promoted. diff --git a/tests/unit/test_async_replication.py b/tests/unit/test_async_replication.py index b8bdde9e42..5edc3ea5e2 100644 --- a/tests/unit/test_async_replication.py +++ b/tests/unit/test_async_replication.py @@ -193,7 +193,11 @@ def test_on_async_relation_changed(harness, wait_for_standby): ) harness.set_can_connect("postgresql", True) harness.handle_exec("postgresql", [], result=0) - harness.add_relation(REPLICATION_OFFER_RELATION, harness.charm.app.name) + with patch( + "relations.async_replication.PostgreSQLAsyncReplication._get_unit_ip", + return_value="10.1.1.10", + ): + harness.add_relation(REPLICATION_OFFER_RELATION, harness.charm.app.name) assert harness.charm.async_replication.get_primary_cluster().name == harness.charm.app.name with ( @@ -216,6 +220,10 @@ def test_on_async_relation_changed(harness, wait_for_standby): "relations.async_replication.PostgreSQLAsyncReplication._wait_for_standby_leader", return_value=wait_for_standby, ), + patch( + "relations.async_replication.PostgreSQLAsyncReplication._get_unit_ip", + return_value="10.2.2.10", + ), ): _pebble.get_services.return_value = ["postgresql"] _patroni_member_started.return_value = True @@ -323,9 +331,13 @@ def test_on_secret_changed(harness, relation_name): secret_id = harness.add_model_secret("primary", {"operator-password": "old"}) peer_rel_id = harness.add_relation(PEER, "primary") - rel_id = harness.add_relation( - relation_name, harness.charm.app.name, unit_data={"unit-address": "10.1.1.10"} - ) + with patch( + "relations.async_replication.PostgreSQLAsyncReplication._get_unit_ip", + return_value="10.1.1.10", + ): + rel_id = harness.add_relation( + relation_name, harness.charm.app.name, unit_data={"unit-address": "10.1.1.10"} + ) secret_label = ( f"{PEER}.{harness.charm.app.name}.app" diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index d54cb39f81..a347dd0446 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -206,7 +206,7 @@ def test_can_use_s3_repository(harness): patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, patch( "charm.Patroni.rock_postgresql_version", - new_callable=PropertyMock(return_value="14.10"), + new_callable=PropertyMock(return_value="16.4"), ) as _rock_postgresql_version, patch("charm.PostgreSQLBackups._execute_command") as _execute_command, patch( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d5c1cf16e1..ce406d5b95 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -198,7 +198,7 @@ def test_on_postgresql_pebble_ready(harness): patch("charm.PostgresqlOperatorCharm._on_leader_elected"), patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, ): - _rock_postgresql_version.return_value = "14.7" + _rock_postgresql_version.return_value = "16.4" # Mock the primary endpoint ready property values. _primary_endpoint_ready.side_effect = [False, True, True] @@ -255,7 +255,7 @@ def test_on_postgresql_pebble_ready_no_connection(harness): ): mock_event = MagicMock() mock_event.workload = harness.model.unit.get_container(POSTGRESQL_CONTAINER) - _rock_postgresql_version.return_value = "14.7" + _rock_postgresql_version.return_value = "16.4" harness.charm._on_postgresql_pebble_ready(mock_event) @@ -1670,6 +1670,7 @@ def test_update_config(harness): harness.update_relation_data( rel_id, harness.charm.unit.name, {"tls": ""} ) # Mock some data in the relation to test that it doesn't change. + _is_tls_enabled.return_value = False harness.charm.update_config() _handle_postgresql_restart_need.assert_not_called() assert "tls" not in harness.get_relation_data(rel_id, harness.charm.unit.name) diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index ddcbec8390..9b631c4922 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -19,7 +19,7 @@ DATABASE = "test_database" RELATION_NAME = "db" -POSTGRESQL_VERSION = "14" +POSTGRESQL_VERSION = "16" @pytest.fixture(autouse=True) diff --git a/tests/unit/test_patroni.py b/tests/unit/test_patroni.py index 211b84fafb..cc1a9be573 100644 --- a/tests/unit/test_patroni.py +++ b/tests/unit/test_patroni.py @@ -202,7 +202,7 @@ def test_render_patroni_yml_file(harness, patroni): ) as _rock_postgresql_version, patch("charm.Patroni._render_file") as _render_file, ): - _rock_postgresql_version.return_value = "14.7" + _rock_postgresql_version.return_value = "16.4" # Get the expected content from a file. with open("templates/patroni.yml.j2") as file: @@ -217,7 +217,7 @@ def test_render_patroni_yml_file(harness, patroni): rewind_user=REWIND_USER, rewind_password=patroni._rewind_password, minority_count=patroni._members_count // 2, - version="14", + version="16", patroni_password=patroni._patroni_password, ) @@ -252,7 +252,7 @@ def test_render_patroni_yml_file(harness, patroni): rewind_user=REWIND_USER, rewind_password=patroni._rewind_password, minority_count=patroni._members_count // 2, - version="14", + version="16", patroni_password=patroni._patroni_password, ) assert expected_content_with_tls != expected_content diff --git a/tests/unit/test_postgresql.py b/tests/unit/test_postgresql.py index d08c60b6cb..14c8125766 100644 --- a/tests/unit/test_postgresql.py +++ b/tests/unit/test_postgresql.py @@ -163,6 +163,7 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 1, ["test_schema_1", "test_schema_2"], "test_user" ) == [ + SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), Composed([ SQL( "DO $$\nDECLARE r RECORD;\nBEGIN\n FOR r IN (SELECT statement FROM (SELECT 1 AS index,'ALTER TABLE '|| schemaname || '.\"' || tablename ||'\" OWNER TO " @@ -212,6 +213,7 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 2, ["test_schema_1", "test_schema_2"], "test_user" ) == [ + SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), Composed([ SQL("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA "), Identifier("test_schema_1"), @@ -234,18 +236,16 @@ def test_generate_database_privileges_statements(harness): SQL(";"), ]), Composed([ - SQL("GRANT USAGE ON SCHEMA "), + SQL("GRANT USAGE, CREATE ON SCHEMA "), Identifier("test_schema_1"), SQL(" TO "), Identifier("test_user"), SQL(";"), ]), Composed([ - SQL("GRANT CREATE ON SCHEMA "), + SQL("GRANT USAGE, CREATE ON SCHEMA "), Identifier("test_schema_1"), - SQL(" TO "), - Identifier("test_user"), - SQL(";"), + SQL(" TO admin;"), ]), Composed([ SQL("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA "), @@ -269,18 +269,16 @@ def test_generate_database_privileges_statements(harness): SQL(";"), ]), Composed([ - SQL("GRANT USAGE ON SCHEMA "), + SQL("GRANT USAGE, CREATE ON SCHEMA "), Identifier("test_schema_2"), SQL(" TO "), Identifier("test_user"), SQL(";"), ]), Composed([ - SQL("GRANT CREATE ON SCHEMA "), + SQL("GRANT USAGE, CREATE ON SCHEMA "), Identifier("test_schema_2"), - SQL(" TO "), - Identifier("test_user"), - SQL(";"), + SQL(" TO admin;"), ]), ] diff --git a/tests/unit/test_postgresql_provider.py b/tests/unit/test_postgresql_provider.py index e56b392387..95db7bda57 100644 --- a/tests/unit/test_postgresql_provider.py +++ b/tests/unit/test_postgresql_provider.py @@ -20,7 +20,7 @@ DATABASE = "test_database" EXTRA_USER_ROLES = "CREATEDB,CREATEROLE" RELATION_NAME = "database" -POSTGRESQL_VERSION = "14" +POSTGRESQL_VERSION = "16" @pytest.fixture(autouse=True) From ae2446c0510b40545710e18fef5751d53dad5eb8 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 26 Nov 2024 20:59:52 +0000 Subject: [PATCH 02/18] try fix integration tests --- tests/integration/ha_tests/test_async_replication.py | 9 ++------- tests/integration/ha_tests/test_replication.py | 2 -- .../ha_tests/test_rollback_to_master_label.py | 3 ++- tests/integration/ha_tests/test_self_healing.py | 2 -- tests/integration/ha_tests/test_upgrade.py | 1 - tests/integration/ha_tests/test_upgrade_from_stable.py | 1 - .../ha_tests/test_upgrade_to_primary_label.py | 1 - tests/integration/new_relations/test_new_relations.py | 7 ++----- .../new_relations/test_relations_coherence.py | 4 ++-- tests/integration/test_wrong_arch.py | 10 +++------- 10 files changed, 11 insertions(+), 29 deletions(-) diff --git a/tests/integration/ha_tests/test_async_replication.py b/tests/integration/ha_tests/test_async_replication.py index 3c5ea5ea09..91cd0b1bf7 100644 --- a/tests/integration/ha_tests/test_async_replication.py +++ b/tests/integration/ha_tests/test_async_replication.py @@ -18,7 +18,6 @@ from .. import architecture, markers from ..helpers import ( APPLICATION_NAME, - CHARM_BASE, DATABASE_APP_NAME, build_and_deploy, get_leader_unit, @@ -113,12 +112,8 @@ async def test_deploy_async_replication_setup( """Build and deploy two PostgreSQL cluster in two separate models to test async replication.""" await build_and_deploy(ops_test, CLUSTER_SIZE, wait_for_idle=False) await build_and_deploy(ops_test, CLUSTER_SIZE, wait_for_idle=False, model=second_model) - await ops_test.model.deploy( - APPLICATION_NAME, channel="latest/edge", num_units=1, base=CHARM_BASE - ) - await second_model.deploy( - APPLICATION_NAME, channel="latest/edge", num_units=1, base=CHARM_BASE - ) + await ops_test.model.deploy(APPLICATION_NAME, channel="latest/edge", num_units=1) + await second_model.deploy(APPLICATION_NAME, channel="latest/edge", num_units=1) async with ops_test.fast_forward(), fast_forward(second_model): await gather( diff --git a/tests/integration/ha_tests/test_replication.py b/tests/integration/ha_tests/test_replication.py index fcc88cd1d8..06aa34080a 100644 --- a/tests/integration/ha_tests/test_replication.py +++ b/tests/integration/ha_tests/test_replication.py @@ -9,7 +9,6 @@ from ..helpers import ( APPLICATION_NAME, - CHARM_BASE, app_name, build_and_deploy, db_connect, @@ -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, channel="edge", ) diff --git a/tests/integration/ha_tests/test_rollback_to_master_label.py b/tests/integration/ha_tests/test_rollback_to_master_label.py index 423806b17a..487f1d7113 100644 --- a/tests/integration/ha_tests/test_rollback_to_master_label.py +++ b/tests/integration/ha_tests/test_rollback_to_master_label.py @@ -37,6 +37,7 @@ @pytest.mark.group(1) +@pytest.mark.unstable @markers.juju3 @markers.amd64_only # TODO: remove after arm64 stable release @pytest.mark.abort_on_fail @@ -55,7 +56,6 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: APPLICATION_NAME, num_units=1, channel="latest/edge", - base=CHARM_BASE, ), ) logger.info("Wait for applications to become active") @@ -71,6 +71,7 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: @pytest.mark.group(1) +@pytest.mark.unstable @markers.juju3 @markers.amd64_only # TODO: remove after arm64 stable release async def test_fail_and_rollback(ops_test, continuous_writes) -> None: diff --git a/tests/integration/ha_tests/test_self_healing.py b/tests/integration/ha_tests/test_self_healing.py index 1afd64239c..7a94d50016 100644 --- a/tests/integration/ha_tests/test_self_healing.py +++ b/tests/integration/ha_tests/test_self_healing.py @@ -12,7 +12,6 @@ from .. import markers from ..helpers import ( APPLICATION_NAME, - CHARM_BASE, METADATA, app_name, build_and_deploy, @@ -69,7 +68,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, channel="edge", ) diff --git a/tests/integration/ha_tests/test_upgrade.py b/tests/integration/ha_tests/test_upgrade.py index 8afe529a1d..441bb1ec73 100644 --- a/tests/integration/ha_tests/test_upgrade.py +++ b/tests/integration/ha_tests/test_upgrade.py @@ -52,7 +52,6 @@ async def test_deploy_latest(ops_test: OpsTest) -> None: APPLICATION_NAME, num_units=1, channel="latest/edge", - base=CHARM_BASE, ), ) logger.info("Wait for applications to become active") diff --git a/tests/integration/ha_tests/test_upgrade_from_stable.py b/tests/integration/ha_tests/test_upgrade_from_stable.py index 0259384493..f0c3d0d260 100644 --- a/tests/integration/ha_tests/test_upgrade_from_stable.py +++ b/tests/integration/ha_tests/test_upgrade_from_stable.py @@ -50,7 +50,6 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: APPLICATION_NAME, num_units=1, channel="latest/edge", - base=CHARM_BASE, ), ) logger.info("Wait for applications to become active") diff --git a/tests/integration/ha_tests/test_upgrade_to_primary_label.py b/tests/integration/ha_tests/test_upgrade_to_primary_label.py index 5714e6761b..d80ee7f927 100644 --- a/tests/integration/ha_tests/test_upgrade_to_primary_label.py +++ b/tests/integration/ha_tests/test_upgrade_to_primary_label.py @@ -59,7 +59,6 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: APPLICATION_NAME, num_units=1, channel="latest/edge", - base=CHARM_BASE, ), ) logger.info("Wait for applications to become active") diff --git a/tests/integration/new_relations/test_new_relations.py b/tests/integration/new_relations/test_new_relations.py index 61bcbfdaa4..4214c538fd 100644 --- a/tests/integration/new_relations/test_new_relations.py +++ b/tests/integration/new_relations/test_new_relations.py @@ -56,7 +56,6 @@ async def test_database_relation_with_charm_libraries(ops_test: OpsTest, databas APPLICATION_APP_NAME, application_name=APPLICATION_APP_NAME, num_units=2, - base=CHARM_BASE, channel="edge", ), ops_test.model.deploy( @@ -196,7 +195,6 @@ async def test_two_applications_doesnt_share_the_same_relation_data(ops_test: Op await ops_test.model.deploy( APPLICATION_APP_NAME, application_name=another_application_app_name, - base=CHARM_BASE, channel="edge", ) await ops_test.model.wait_for_idle(apps=all_app_names, status="active") @@ -453,7 +451,7 @@ async def test_admin_role(ops_test: OpsTest): all_app_names = [DATA_INTEGRATOR_APP_NAME] all_app_names.extend(APP_NAMES) async with ops_test.fast_forward(): - await ops_test.model.deploy(DATA_INTEGRATOR_APP_NAME, base=CHARM_BASE) + await ops_test.model.deploy(DATA_INTEGRATOR_APP_NAME) await ops_test.model.wait_for_idle(apps=[DATA_INTEGRATOR_APP_NAME], status="blocked") await ops_test.model.applications[DATA_INTEGRATOR_APP_NAME].set_config({ "database-name": DATA_INTEGRATOR_APP_NAME.replace("-", "_"), @@ -544,7 +542,6 @@ async def test_invalid_extra_user_roles(ops_test: OpsTest): await ops_test.model.deploy( DATA_INTEGRATOR_APP_NAME, application_name=another_data_integrator_app_name, - base=CHARM_BASE, ) await ops_test.model.wait_for_idle( apps=[another_data_integrator_app_name], status="blocked" @@ -630,7 +627,7 @@ async def test_discourse(ops_test: OpsTest): await gather( ops_test.model.deploy(DISCOURSE_APP_NAME, application_name=DISCOURSE_APP_NAME), ops_test.model.deploy( - REDIS_APP_NAME, application_name=REDIS_APP_NAME, channel="latest/edge", base=CHARM_BASE + REDIS_APP_NAME, application_name=REDIS_APP_NAME, channel="latest/edge" ), ) diff --git a/tests/integration/new_relations/test_relations_coherence.py b/tests/integration/new_relations/test_relations_coherence.py index aa489473d1..4dc301bdd3 100644 --- a/tests/integration/new_relations/test_relations_coherence.py +++ b/tests/integration/new_relations/test_relations_coherence.py @@ -9,7 +9,7 @@ import pytest from pytest_operator.plugin import OpsTest -from ..helpers import CHARM_BASE, DATABASE_APP_NAME, build_and_deploy +from ..helpers import DATABASE_APP_NAME, build_and_deploy from .helpers import build_connection_string from .test_new_relations import DATA_INTEGRATOR_APP_NAME @@ -30,7 +30,7 @@ async def test_relations(ops_test: OpsTest, database_charm): await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active", timeout=3000) # Creating first time relation with user role - await ops_test.model.deploy(DATA_INTEGRATOR_APP_NAME, base=CHARM_BASE) + await ops_test.model.deploy(DATA_INTEGRATOR_APP_NAME) await ops_test.model.applications[DATA_INTEGRATOR_APP_NAME].set_config({ "database-name": DATA_INTEGRATOR_APP_NAME.replace("-", "_"), }) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index e05456e7a5..1a7469b9a7 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -20,16 +20,12 @@ async def fetch_charm( charm_path: typing.Union[str, os.PathLike], architecture: str, - bases_index: int, ) -> pathlib.Path: """Fetches packed charm from CI runner without checking for architecture.""" charm_path = pathlib.Path(charm_path) charmcraft_yaml = yaml.safe_load((charm_path / "charmcraft.yaml").read_text()) assert charmcraft_yaml["type"] == "charm" - base = charmcraft_yaml["bases"][bases_index] - build_on = base.get("build-on", [base])[0] - version = build_on["channel"] - packed_charms = list(charm_path.glob(f"*{version}-{architecture}.charm")) + packed_charms = list(charm_path.glob(f"{architecture}.charm")) return packed_charms[0].resolve(strict=True) @@ -37,7 +33,7 @@ async def fetch_charm( @markers.amd64_only async def test_arm_charm_on_amd_host(ops_test: OpsTest) -> None: """Tries deploying an arm64 charm on amd64 host.""" - charm = await fetch_charm(".", "arm64", 1) + charm = await fetch_charm(".", "arm64") resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } @@ -60,7 +56,7 @@ async def test_arm_charm_on_amd_host(ops_test: OpsTest) -> None: @markers.arm64_only async def test_amd_charm_on_arm_host(ops_test: OpsTest) -> None: """Tries deploying an amd64 charm on arm64 host.""" - charm = await fetch_charm(".", "amd64", 0) + charm = await fetch_charm(".", "amd64") resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } From 485fe58c69ccc81188d13c7edc55058748c00535 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 26 Nov 2024 21:07:37 +0000 Subject: [PATCH 03/18] remove argument on helper --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index bc2cad7fa6..59fa45115e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1879,7 +1879,7 @@ def update_config(self, is_creating_backup: bool = False) -> bool: logger.debug( "Early exit update_config: patroni not responding but TLS is enabled." ) - self._handle_postgresql_restart_need(True) + self._handle_postgresql_restart_need() return True logger.debug("Early exit update_config: Patroni not started yet") return False From df73c2118439b26784776919d8d82aaafe31dd05 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 26 Nov 2024 21:54:56 +0000 Subject: [PATCH 04/18] more fixes --- tests/integration/helpers.py | 6 ++---- tests/integration/relations/test_relations.py | 1 - tests/integration/test_wrong_arch.py | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 2affc12025..d5a9950e6a 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -841,10 +841,8 @@ async def backup_operations( ) -> None: """Basic set of operations for backup testing in different cloud providers.""" # Deploy S3 Integrator and TLS Certificates Operator. - await ops_test.model.deploy(s3_integrator_app_name, base=CHARM_BASE) - await ops_test.model.deploy( - tls_certificates_app_name, config=tls_config, channel=tls_channel, base=CHARM_BASE - ) + await ops_test.model.deploy(s3_integrator_app_name) + await ops_test.model.deploy(tls_certificates_app_name, config=tls_config, channel=tls_channel) # Deploy and relate PostgreSQL to S3 integrator (one database app for each cloud for now # as archivo_mode is disabled after restoring the backup) and to TLS Certificates Operator # (to be able to create backups from replicas). diff --git a/tests/integration/relations/test_relations.py b/tests/integration/relations/test_relations.py index 0d33cf38ee..2c70119530 100644 --- a/tests/integration/relations/test_relations.py +++ b/tests/integration/relations/test_relations.py @@ -34,7 +34,6 @@ async def test_deploy_charms(ops_test: OpsTest, database_charm): APPLICATION_APP_NAME, application_name=APPLICATION_APP_NAME, num_units=1, - base=CHARM_BASE, channel="edge", ), ops_test.model.deploy( diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index 1a7469b9a7..96605b645f 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -25,7 +25,7 @@ async def fetch_charm( charm_path = pathlib.Path(charm_path) charmcraft_yaml = yaml.safe_load((charm_path / "charmcraft.yaml").read_text()) assert charmcraft_yaml["type"] == "charm" - packed_charms = list(charm_path.glob(f"{architecture}.charm")) + packed_charms = list(charm_path.glob(f"*{architecture}.charm")) return packed_charms[0].resolve(strict=True) From 4b076b73b952688dc10c6c6e05329a1a6b04b644 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Mon, 3 Feb 2025 23:52:29 +0000 Subject: [PATCH 05/18] fix test issues --- .github/workflows/ci.yaml | 2 ++ lib/charms/postgresql_k8s/v0/postgresql.py | 12 +++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3d8bc7679f..2d02fb58a7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -50,6 +50,8 @@ jobs: build: name: Build charm uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v29.0.5 + with: + cache: false integration-test: strategy: diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index b46fe9b3bb..7c4eca6f77 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -35,7 +35,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 42 +LIBPATCH = 43 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles" @@ -317,7 +317,7 @@ def delete_user(self, user: str) -> None: logger.error(f"Failed to delete user: {e}") raise PostgreSQLDeleteUserError() from e - def enable_disable_extensions( + def enable_disable_extensions( # noqa: C901 self, extensions: Dict[str, bool], database: Optional[str] = None ) -> None: """Enables or disables a PostgreSQL extension. @@ -374,7 +374,7 @@ def _generate_database_privileges_statements( ) -> List[Composed]: """Generates a list of databases privileges statements.""" statements = [] - statements.append(sql.SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;")) + statements.append(SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;")) if relations_accessing_this_database == 1: statements.append( SQL( @@ -431,8 +431,10 @@ def _generate_database_privileges_statements( SQL("GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA {} TO {};").format( schema, Identifier(user) ), - SQL("GRANT USAGE ON SCHEMA {} TO {};").format(schema, Identifier(user)), - SQL("GRANT CREATE ON SCHEMA {} TO {};").format(schema, Identifier(user)), + SQL("GRANT USAGE, CREATE ON SCHEMA {} TO {};").format( + schema, Identifier(user) + ), + SQL("GRANT USAGE, CREATE ON SCHEMA {} TO admin;").format(schema), ]) return statements From 75560d9a7c7c1a09f59ae42dca87b4ad2729b68c Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 00:57:30 +0000 Subject: [PATCH 06/18] REVERT LATER: Adapt build_charm function --- tests/integration/conftest.py | 4 ++- .../ha_tests/test_rollback_to_master_label.py | 3 +- tests/integration/ha_tests/test_upgrade.py | 5 +-- .../ha_tests/test_upgrade_from_stable.py | 3 +- .../ha_tests/test_upgrade_to_primary_label.py | 3 +- tests/integration/helpers.py | 35 +++++++++++++++++-- tests/integration/test_charm.py | 7 ++-- 7 files changed, 49 insertions(+), 11 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index abacfd3269..151dd30486 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -4,9 +4,11 @@ import pytest from pytest_operator.plugin import OpsTest +from .helpers import build_charm + @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(".") return charm diff --git a/tests/integration/ha_tests/test_rollback_to_master_label.py b/tests/integration/ha_tests/test_rollback_to_master_label.py index e709a2bb63..96b5d7ecb3 100644 --- a/tests/integration/ha_tests/test_rollback_to_master_label.py +++ b/tests/integration/ha_tests/test_rollback_to_master_label.py @@ -18,6 +18,7 @@ CHARM_BASE, DATABASE_APP_NAME, METADATA, + build_charm, get_leader_unit, get_primary, get_unit_by_index, @@ -99,7 +100,7 @@ async def test_fail_and_rollback(ops_test, continuous_writes) -> None: primary_name = await get_primary(ops_test, DATABASE_APP_NAME) assert primary_name == f"{DATABASE_APP_NAME}/0" - local_charm = await ops_test.build_charm(".") + local_charm = await build_charm(".") filename = local_charm.split("/")[-1] if isinstance(local_charm, str) else local_charm.name fault_charm = Path("/tmp/", filename) shutil.copy(local_charm, fault_charm) diff --git a/tests/integration/ha_tests/test_upgrade.py b/tests/integration/ha_tests/test_upgrade.py index 3650e321b2..c9606461d3 100644 --- a/tests/integration/ha_tests/test_upgrade.py +++ b/tests/integration/ha_tests/test_upgrade.py @@ -17,6 +17,7 @@ CHARM_BASE, DATABASE_APP_NAME, METADATA, + build_charm, count_switchovers, get_leader_unit, get_primary, @@ -112,7 +113,7 @@ async def test_upgrade_from_edge(ops_test: OpsTest, continuous_writes) -> None: application = ops_test.model.applications[DATABASE_APP_NAME] logger.info("Build charm locally") - charm = await ops_test.build_charm(".") + charm = await build_charm(".") logger.info("Refresh the charm") await application.refresh(path=charm, resources=resources) @@ -186,7 +187,7 @@ async def test_fail_and_rollback(ops_test, continuous_writes) -> None: primary_name = await get_primary(ops_test, DATABASE_APP_NAME) assert primary_name == f"{DATABASE_APP_NAME}/0" - local_charm = await ops_test.build_charm(".") + local_charm = await build_charm(".") filename = local_charm.split("/")[-1] if isinstance(local_charm, str) else local_charm.name fault_charm = Path("/tmp/", filename) shutil.copy(local_charm, fault_charm) diff --git a/tests/integration/ha_tests/test_upgrade_from_stable.py b/tests/integration/ha_tests/test_upgrade_from_stable.py index f0c3d0d260..2b21ae0e7b 100644 --- a/tests/integration/ha_tests/test_upgrade_from_stable.py +++ b/tests/integration/ha_tests/test_upgrade_from_stable.py @@ -16,6 +16,7 @@ CHARM_BASE, DATABASE_APP_NAME, METADATA, + build_charm, count_switchovers, get_leader_unit, get_primary, @@ -116,7 +117,7 @@ async def test_upgrade_from_stable(ops_test: OpsTest, continuous_writes): actions = await application.get_actions() logger.info("Build charm locally") - charm = await ops_test.build_charm(".") + charm = await build_charm(".") logger.info("Refresh the charm") await application.refresh(path=charm, resources=resources) diff --git a/tests/integration/ha_tests/test_upgrade_to_primary_label.py b/tests/integration/ha_tests/test_upgrade_to_primary_label.py index eaf548fc46..952ba52b9f 100644 --- a/tests/integration/ha_tests/test_upgrade_to_primary_label.py +++ b/tests/integration/ha_tests/test_upgrade_to_primary_label.py @@ -17,6 +17,7 @@ CHARM_SERIES, DATABASE_APP_NAME, METADATA, + build_charm, get_leader_unit, get_primary, get_unit_by_index, @@ -101,7 +102,7 @@ async def test_upgrade(ops_test, continuous_writes) -> None: primary_name = await get_primary(ops_test, DATABASE_APP_NAME) assert primary_name == f"{DATABASE_APP_NAME}/0" - local_charm = await ops_test.build_charm(".") + local_charm = await build_charm(".") application = ops_test.model.applications[DATABASE_APP_NAME] resources = {"postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"]} diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index ee9bad14f3..d1fc94383a 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -7,7 +7,7 @@ from datetime import datetime from multiprocessing import ProcessError from pathlib import Path -from subprocess import check_call +from subprocess import check_call, run import botocore import psycopg2 @@ -70,6 +70,37 @@ async def app_name( return None +async def build_charm(charm_path) -> Path: + charm_path = Path(charm_path) + architecture = run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + # 24.04 pin is temporary solution while multi-base integration testing not supported by data-platform-workflows + packed_charms = list(charm_path.glob(f"*ubuntu@24.04-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + raise ValueError( + f"More than one matching .charm file found at {charm_path=} for {architecture=} and " + f"Ubuntu 24.04: {packed_charms}." + ) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and Ubuntu 24.04 at {charm_path=}" + ) + + async def build_and_deploy( ops_test: OpsTest, num_units: int, @@ -89,7 +120,7 @@ async def build_and_deploy( global charm if not charm: - charm = await ops_test.build_charm(".") + charm = await build_charm(".") resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 9e29ac16d0..e0d2d08ee9 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -18,6 +18,7 @@ METADATA, STORAGE_PATH, build_and_deploy, + build_charm, convert_records_to_dict, db_connect, get_application_units, @@ -379,7 +380,7 @@ async def test_application_removal(ops_test: OpsTest) -> None: @pytest.mark.group(1) async def test_redeploy_charm_same_model(ops_test: OpsTest): """Redeploy the charm in the same model to test that it works.""" - charm = await ops_test.build_charm(".") + charm = await build_charm(".") async with ops_test.fast_forward(): await ops_test.model.deploy( charm, @@ -423,7 +424,7 @@ async def test_redeploy_charm_same_model_after_forcing_removal(ops_test: OpsTest assert set(existing_resources) == set(expected_resources) # Check that the charm can be deployed again. - charm = await ops_test.build_charm(".") + charm = await build_charm(".") async with ops_test.fast_forward(): await ops_test.model.deploy( charm, @@ -453,7 +454,7 @@ async def test_storage_with_more_restrictive_permissions(ops_test: OpsTest): app_name = f"test-storage-{APP_NAME}" async with ops_test.fast_forward(): # Deploy and wait for the charm to get into the install hook (maintenance status). - charm = await ops_test.build_charm(".") + charm = await build_charm(".") async with ops_test.fast_forward(): await ops_test.model.deploy( charm, From 78b4df6ba38ad7c72271506af668ba81b04269ef Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 01:33:26 +0000 Subject: [PATCH 07/18] remove base=CHARM_BASE from deploy --- tests/integration/ha_tests/test_rollback_to_master_label.py | 2 -- tests/integration/ha_tests/test_smoke.py | 3 --- tests/integration/ha_tests/test_upgrade.py | 2 -- tests/integration/ha_tests/test_upgrade_from_stable.py | 2 -- tests/integration/helpers.py | 1 - tests/integration/new_relations/test_new_relations.py | 4 ---- tests/integration/relations/test_relations.py | 2 -- tests/integration/test_charm.py | 4 ---- tests/integration/test_trust.py | 2 -- tests/integration/test_wrong_arch.py | 4 +--- 10 files changed, 1 insertion(+), 25 deletions(-) diff --git a/tests/integration/ha_tests/test_rollback_to_master_label.py b/tests/integration/ha_tests/test_rollback_to_master_label.py index 96b5d7ecb3..80b33bd130 100644 --- a/tests/integration/ha_tests/test_rollback_to_master_label.py +++ b/tests/integration/ha_tests/test_rollback_to_master_label.py @@ -15,7 +15,6 @@ from ..architecture import architecture from ..helpers import ( APPLICATION_NAME, - CHARM_BASE, DATABASE_APP_NAME, METADATA, build_charm, @@ -51,7 +50,6 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: num_units=3, channel="16/stable", revision=LABEL_REVISION, - base=CHARM_BASE, trust=True, ), ops_test.model.deploy( diff --git a/tests/integration/ha_tests/test_smoke.py b/tests/integration/ha_tests/test_smoke.py index 5ae401cfe7..9bb2a623d3 100644 --- a/tests/integration/ha_tests/test_smoke.py +++ b/tests/integration/ha_tests/test_smoke.py @@ -12,7 +12,6 @@ from .. import markers from ..helpers import ( - CHARM_BASE, DATABASE_APP_NAME, scale_application, ) @@ -56,7 +55,6 @@ async def test_app_force_removal(ops_test: OpsTest): application_name=DATABASE_APP_NAME, num_units=1, channel="16/stable", - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ) @@ -169,7 +167,6 @@ async def test_app_resources_conflicts(ops_test: OpsTest): application_name=DUP_DATABASE_APP_NAME, num_units=1, channel="16/stable", - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ) diff --git a/tests/integration/ha_tests/test_upgrade.py b/tests/integration/ha_tests/test_upgrade.py index c9606461d3..8863562bdd 100644 --- a/tests/integration/ha_tests/test_upgrade.py +++ b/tests/integration/ha_tests/test_upgrade.py @@ -14,7 +14,6 @@ from ..helpers import ( APPLICATION_NAME, - CHARM_BASE, DATABASE_APP_NAME, METADATA, build_charm, @@ -47,7 +46,6 @@ async def test_deploy_latest(ops_test: OpsTest) -> None: channel="16/edge", trust=True, config={"profile": "testing"}, - base=CHARM_BASE, ), ops_test.model.deploy( APPLICATION_NAME, diff --git a/tests/integration/ha_tests/test_upgrade_from_stable.py b/tests/integration/ha_tests/test_upgrade_from_stable.py index 2b21ae0e7b..54adb435de 100644 --- a/tests/integration/ha_tests/test_upgrade_from_stable.py +++ b/tests/integration/ha_tests/test_upgrade_from_stable.py @@ -13,7 +13,6 @@ from .. import markers from ..helpers import ( APPLICATION_NAME, - CHARM_BASE, DATABASE_APP_NAME, METADATA, build_charm, @@ -45,7 +44,6 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: num_units=3, channel="16/stable", trust=True, - base=CHARM_BASE, ), ops_test.model.deploy( APPLICATION_NAME, diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index d1fc94383a..d98c77f0a8 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -131,7 +131,6 @@ async def build_and_deploy( application_name=database_app_name, trust=True, num_units=num_units, - base=CHARM_BASE, config={"profile": "testing"}, ), ) diff --git a/tests/integration/new_relations/test_new_relations.py b/tests/integration/new_relations/test_new_relations.py index 4214c538fd..0c24eb8442 100644 --- a/tests/integration/new_relations/test_new_relations.py +++ b/tests/integration/new_relations/test_new_relations.py @@ -16,7 +16,6 @@ from .. import markers from ..helpers import ( - CHARM_BASE, check_database_users_existence, scale_application, ) @@ -67,7 +66,6 @@ async def test_database_relation_with_charm_libraries(ops_test: OpsTest, databas }, application_name=DATABASE_APP_NAME, num_units=3, - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ), @@ -80,7 +78,6 @@ async def test_database_relation_with_charm_libraries(ops_test: OpsTest, databas }, application_name=ANOTHER_DATABASE_APP_NAME, num_units=3, - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ), @@ -606,7 +603,6 @@ async def test_database_deploy_clientapps(ops_test: OpsTest, database_charm): }, application_name=DATABASE_APP_NAME, num_units=3, - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ), diff --git a/tests/integration/relations/test_relations.py b/tests/integration/relations/test_relations.py index 2c70119530..4bbb867def 100644 --- a/tests/integration/relations/test_relations.py +++ b/tests/integration/relations/test_relations.py @@ -7,7 +7,6 @@ import pytest from pytest_operator.plugin import OpsTest -from ..helpers import CHARM_BASE from ..new_relations.test_new_relations import ( APPLICATION_APP_NAME, DATABASE_APP_METADATA, @@ -45,7 +44,6 @@ async def test_deploy_charms(ops_test: OpsTest, database_charm): }, application_name=APP_NAME, num_units=1, - base=CHARM_BASE, config={ "profile": "testing", "plugin_unaccent_enable": "True", diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index e0d2d08ee9..664b0fbf24 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -14,7 +14,6 @@ from tenacity import Retrying, stop_after_delay, wait_fixed from .helpers import ( - CHARM_BASE, METADATA, STORAGE_PATH, build_and_deploy, @@ -389,7 +388,6 @@ async def test_redeploy_charm_same_model(ops_test: OpsTest): }, application_name=APP_NAME, num_units=len(UNIT_IDS), - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ) @@ -433,7 +431,6 @@ async def test_redeploy_charm_same_model_after_forcing_removal(ops_test: OpsTest }, application_name=APP_NAME, num_units=len(UNIT_IDS), - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ) @@ -465,7 +462,6 @@ async def test_storage_with_more_restrictive_permissions(ops_test: OpsTest): }, application_name=app_name, num_units=1, - base=CHARM_BASE, trust=True, config={"profile": "testing"}, ) diff --git a/tests/integration/test_trust.py b/tests/integration/test_trust.py index 8de2491ed8..ca26e03770 100644 --- a/tests/integration/test_trust.py +++ b/tests/integration/test_trust.py @@ -10,7 +10,6 @@ from pytest_operator.plugin import OpsTest from .helpers import ( - CHARM_BASE, KUBECTL, METADATA, get_leader_unit, @@ -97,7 +96,6 @@ async def test_deploy_without_trust(ops_test: OpsTest, database_charm): application_name=APP_NAME, num_units=3, trust=False, - base=CHARM_BASE, ) await ops_test.model.block_until( diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index 4b9980ca3d..0261707bc6 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -11,7 +11,7 @@ from pytest_operator.plugin import OpsTest from . import markers -from .helpers import CHARM_BASE, DATABASE_APP_NAME, METADATA +from .helpers import DATABASE_APP_NAME, METADATA logger = logging.getLogger(__name__) @@ -40,7 +40,6 @@ async def test_arm_charm_on_amd_host(ops_test: OpsTest) -> None: application_name=DATABASE_APP_NAME, trust=True, num_units=1, - base=CHARM_BASE, config={"profile": "testing"}, ) @@ -63,7 +62,6 @@ async def test_amd_charm_on_arm_host(ops_test: OpsTest) -> None: application_name=DATABASE_APP_NAME, trust=True, num_units=1, - base=CHARM_BASE, config={"profile": "testing"}, ) From 415129684307849e741f80228eabfdcf73ce9076 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 12:57:57 +0000 Subject: [PATCH 08/18] remove juju 2.9 tests --- .github/workflows/ci.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2d02fb58a7..f6dac5843f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -58,9 +58,6 @@ jobs: 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 - agent: 3.6.2 # renovate: juju-agent-pin-minor allure_on_amd64: true architecture: From 08dade0a5842d540190a398540aab480be8740f8 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 13:25:41 +0000 Subject: [PATCH 09/18] grat permissions to public schema --- lib/charms/postgresql_k8s/v0/postgresql.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 7c4eca6f77..8b07943b6f 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -276,6 +276,7 @@ def create_user( cursor.execute( SQL("GRANT {} TO {};").format(Identifier(role), Identifier(user)) ) + cursor.execute(SQL("GRANT ALL ON SCHEMA public TO {};").format(Identifier(user))) except psycopg2.Error as e: logger.error(f"Failed to create user: {e}") raise PostgreSQLCreateUserError() from e @@ -374,7 +375,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 admin;")) + statements.append(SQL("GRANT ALL ON SCHEMA public TO admin;")) if relations_accessing_this_database == 1: statements.append( SQL( From 9110cef8b99831524da0301c49ae62851d9ebe66 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 13:34:12 +0000 Subject: [PATCH 10/18] fix linting and unit test --- .github/workflows/ci.yaml | 1 - tests/unit/test_postgresql.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f6dac5843f..39c053176d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -79,7 +79,6 @@ jobs: cloud: microk8s microk8s-snap-channel: 1.32-strict/stable # renovate: latest microk8s juju-agent-version: ${{ matrix.juju.agent }} - libjuju-version-constraint: ${{ matrix.juju.libjuju }} _beta_allure_report: ${{ matrix.juju.allure_on_amd64 && matrix.architecture == 'amd64' }} secrets: integration-test: | diff --git a/tests/unit/test_postgresql.py b/tests/unit/test_postgresql.py index afe50a1dea..bee0d6a947 100644 --- a/tests/unit/test_postgresql.py +++ b/tests/unit/test_postgresql.py @@ -163,7 +163,7 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 1, ["test_schema_1", "test_schema_2"], "test_user" ) == [ - SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), + SQL("GRANT ALL ON SCHEMA public TO admin;"), Composed([ SQL( "DO $$\nDECLARE r RECORD;\nBEGIN\n FOR r IN (SELECT statement FROM (SELECT 1 AS index,'ALTER TABLE '|| schemaname || '.\"' || tablename ||'\" OWNER TO " @@ -221,7 +221,7 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 2, ["test_schema_1", "test_schema_2"], "test_user" ) == [ - SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), + SQL("GRANT ALL ON SCHEMA public TO admin;"), Composed([ SQL("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA "), Identifier("test_schema_1"), From 560f1eb46f054a3839ab086daee2165b0dae2d41 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 15:13:49 +0000 Subject: [PATCH 11/18] try refactor the permissions change --- .github/workflows/ci.yaml | 2 ++ lib/charms/postgresql_k8s/v0/postgresql.py | 6 ++++-- tests/unit/test_postgresql.py | 14 ++++++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 39c053176d..972222e4ec 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -59,6 +59,7 @@ jobs: matrix: juju: - agent: 3.6.2 # renovate: juju-agent-pin-minor + libjuju: ==3.6.1.0 # renovate: latest libjuju 2 allure_on_amd64: true architecture: - amd64 @@ -79,6 +80,7 @@ jobs: cloud: microk8s microk8s-snap-channel: 1.32-strict/stable # renovate: latest microk8s juju-agent-version: ${{ matrix.juju.agent }} + libjuju-version-constraint: ${{ matrix.juju.libjuju }} _beta_allure_report: ${{ matrix.juju.allure_on_amd64 && matrix.architecture == 'amd64' }} secrets: integration-test: | diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 8b07943b6f..3340897b02 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -276,7 +276,6 @@ def create_user( cursor.execute( SQL("GRANT {} TO {};").format(Identifier(role), Identifier(user)) ) - cursor.execute(SQL("GRANT ALL ON SCHEMA public TO {};").format(Identifier(user))) except psycopg2.Error as e: logger.error(f"Failed to create user: {e}") raise PostgreSQLCreateUserError() from e @@ -375,7 +374,10 @@ def _generate_database_privileges_statements( ) -> List[Composed]: """Generates a list of databases privileges statements.""" statements = [] - statements.append(SQL("GRANT ALL ON SCHEMA public TO admin;")) + statements.append(SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;")) + statements.append( + SQL("GRANT USAGE, CREATE ON SCHEMA public TO {};").format(Identifier(user)) + ) if relations_accessing_this_database == 1: statements.append( SQL( diff --git a/tests/unit/test_postgresql.py b/tests/unit/test_postgresql.py index bee0d6a947..0fcd4afd09 100644 --- a/tests/unit/test_postgresql.py +++ b/tests/unit/test_postgresql.py @@ -163,7 +163,12 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 1, ["test_schema_1", "test_schema_2"], "test_user" ) == [ - SQL("GRANT ALL ON SCHEMA public TO admin;"), + SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), + Composed([ + SQL("GRANT USAGE, CREATE ON SCHEMA public TO "), + Identifier("test_user"), + SQL(";"), + ]), Composed([ SQL( "DO $$\nDECLARE r RECORD;\nBEGIN\n FOR r IN (SELECT statement FROM (SELECT 1 AS index,'ALTER TABLE '|| schemaname || '.\"' || tablename ||'\" OWNER TO " @@ -221,7 +226,12 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 2, ["test_schema_1", "test_schema_2"], "test_user" ) == [ - SQL("GRANT ALL ON SCHEMA public TO admin;"), + SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), + Composed([ + SQL("GRANT USAGE, CREATE ON SCHEMA public TO "), + Identifier("test_user"), + SQL(";"), + ]), Composed([ SQL("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA "), Identifier("test_schema_1"), From adafd4d39690a377b1a44a8dcee1b9f6ca072970 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 19:36:41 +0000 Subject: [PATCH 12/18] grant public access to public schemas --- lib/charms/postgresql_k8s/v0/postgresql.py | 5 +---- tests/integration/ha_tests/test_smoke.py | 2 ++ tests/unit/test_postgresql.py | 14 ++------------ 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 3340897b02..2fd8219bca 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -374,10 +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 admin;")) - statements.append( - SQL("GRANT USAGE, CREATE ON SCHEMA public TO {};").format(Identifier(user)) - ) + statements.append(SQL("GRANT USAGE, CREATE ON SCHEMA public TO PUBLIC;")) if relations_accessing_this_database == 1: statements.append( SQL( diff --git a/tests/integration/ha_tests/test_smoke.py b/tests/integration/ha_tests/test_smoke.py index 9bb2a623d3..75b702ed0f 100644 --- a/tests/integration/ha_tests/test_smoke.py +++ b/tests/integration/ha_tests/test_smoke.py @@ -44,6 +44,7 @@ @pytest.mark.group(1) @markers.amd64_only # TODO: remove after arm64 stable release +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_app_force_removal(ops_test: OpsTest): """Remove unit with force while storage is alive.""" @@ -157,6 +158,7 @@ async def test_app_garbage_ignorance(ops_test: OpsTest): @pytest.mark.group(1) @markers.amd64_only # TODO: remove after arm64 stable release +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_app_resources_conflicts(ops_test: OpsTest): """Test application deploy in dirty environment with garbage storage from another application.""" diff --git a/tests/unit/test_postgresql.py b/tests/unit/test_postgresql.py index 0fcd4afd09..2698b2154d 100644 --- a/tests/unit/test_postgresql.py +++ b/tests/unit/test_postgresql.py @@ -163,12 +163,7 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 1, ["test_schema_1", "test_schema_2"], "test_user" ) == [ - SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), - Composed([ - SQL("GRANT USAGE, CREATE ON SCHEMA public TO "), - Identifier("test_user"), - SQL(";"), - ]), + SQL("GRANT USAGE, CREATE ON SCHEMA public TO PUBLIC;"), Composed([ SQL( "DO $$\nDECLARE r RECORD;\nBEGIN\n FOR r IN (SELECT statement FROM (SELECT 1 AS index,'ALTER TABLE '|| schemaname || '.\"' || tablename ||'\" OWNER TO " @@ -226,12 +221,7 @@ def test_generate_database_privileges_statements(harness): assert harness.charm.postgresql._generate_database_privileges_statements( 2, ["test_schema_1", "test_schema_2"], "test_user" ) == [ - SQL("GRANT USAGE, CREATE ON SCHEMA public TO admin;"), - Composed([ - SQL("GRANT USAGE, CREATE ON SCHEMA public TO "), - Identifier("test_user"), - SQL(";"), - ]), + SQL("GRANT USAGE, CREATE ON SCHEMA public TO PUBLIC;"), Composed([ SQL("GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA "), Identifier("test_schema_1"), From c9cbf70330a5548783d33bedd68b9f47ee1e3796 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Tue, 4 Feb 2025 22:04:31 +0000 Subject: [PATCH 13/18] mark smoke test as unstable --- tests/integration/ha_tests/test_smoke.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/ha_tests/test_smoke.py b/tests/integration/ha_tests/test_smoke.py index 75b702ed0f..6373f11e92 100644 --- a/tests/integration/ha_tests/test_smoke.py +++ b/tests/integration/ha_tests/test_smoke.py @@ -105,6 +105,7 @@ async def test_app_force_removal(ops_test: OpsTest): @pytest.mark.group(1) @markers.amd64_only # TODO: remove after arm64 stable release +@pytest.mark.unstable @pytest.mark.abort_on_fail async def test_app_garbage_ignorance(ops_test: OpsTest): """Test charm deploy in dirty environment with garbage storage.""" From ba536a0bab50d2a57f6ef4248877367734ad3e1c Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Thu, 6 Feb 2025 12:58:58 +0000 Subject: [PATCH 14/18] test release charm to 16/edge --- .github/workflows/release.yaml | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 286bb97314..3499958008 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -18,34 +18,8 @@ on: workflow_dispatch: jobs: - ci-tests: - name: Tests - uses: ./.github/workflows/ci.yaml - secrets: inherit - permissions: - contents: write # Needed for Allure Report beta - - release-libraries: - name: Release libraries - needs: - - ci-tests - runs-on: ubuntu-latest - timeout-minutes: 60 - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Release charm libraries - uses: canonical/charming-actions/release-libraries@2.7.0 - with: - credentials: "${{ secrets.CHARMHUB_TOKEN }}" - github-token: "${{ secrets.GITHUB_TOKEN }}" - release: name: Release charm - needs: - - ci-tests uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v29.0.5 with: channel: 16/edge From efb6603f093216740646cbd949acb0594bfd632e Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Thu, 6 Feb 2025 13:03:38 +0000 Subject: [PATCH 15/18] fix test release workflow --- .github/workflows/release.yaml | 8 +++++++- metadata.yaml | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 3499958008..b7246f1487 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -18,12 +18,18 @@ on: workflow_dispatch: jobs: + build: + name: Build charm + uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v29.0.5 + with: + cache: false + release: name: Release charm uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v29.0.5 with: channel: 16/edge - artifact-prefix: ${{ needs.ci-tests.outputs.artifact-prefix }} + artifact-prefix: ${{ needs.build.outputs.artifact-prefix }} secrets: charmhub-token: ${{ secrets.CHARMHUB_TOKEN }} permissions: diff --git a/metadata.yaml b/metadata.yaml index c4025ba124..d6d82b6f04 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -28,7 +28,7 @@ resources: postgresql-image: type: oci-image description: OCI image for PostgreSQL - upstream-source: ghcr.io/canonical/charmed-postgresql:16.4-24.04_edge + upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:c7891aedd454a140cc800e3f9ce1d8958ce3376ead5efffa4f993c18a69260bf peers: database-peers: From d38b7688e6d282c03fe5425e6a28755beeb18da6 Mon Sep 17 00:00:00 2001 From: Lucas Borges Date: Thu, 6 Feb 2025 13:05:01 +0000 Subject: [PATCH 16/18] fix typo --- .github/workflows/release.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index b7246f1487..dbe15c0ef7 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -26,6 +26,8 @@ jobs: release: name: Release charm + needs: + - build uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v29.0.5 with: channel: 16/edge From f2d3bab0372213a1414f557b3e7dc93dc8ff83fa Mon Sep 17 00:00:00 2001 From: "lucas.gameiro-borges" Date: Mon, 10 Feb 2025 10:30:52 -0300 Subject: [PATCH 17/18] revert metadata.yaml --- metadata.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metadata.yaml b/metadata.yaml index d6d82b6f04..c4025ba124 100644 --- a/metadata.yaml +++ b/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:c7891aedd454a140cc800e3f9ce1d8958ce3376ead5efffa4f993c18a69260bf + upstream-source: ghcr.io/canonical/charmed-postgresql:16.4-24.04_edge peers: database-peers: From 66cf401b0d789899fabe8f62264b295e72b608f8 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Mon, 10 Feb 2025 19:18:39 -0300 Subject: [PATCH 18/18] nits --- .github/workflows/release.yaml | 34 +++++++++++++++++++++++++--------- src/dependency.json | 2 +- tests/unit/test_backups.py | 2 +- tests/unit/test_charm.py | 4 ++-- tests/unit/test_patroni.py | 2 +- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index dbe15c0ef7..aa1a8ce700 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -14,24 +14,40 @@ on: - '.github/workflows/ci.yaml' - '.github/workflows/lib-check.yaml' - '.github/workflows/sync_docs.yaml' - # for testing purposes: - workflow_dispatch: jobs: - build: - name: Build charm - uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v29.0.5 - with: - cache: false + ci-tests: + name: Tests + uses: ./.github/workflows/ci.yaml + secrets: inherit + permissions: + contents: write # Needed for Allure Report beta + + release-libraries: + name: Release libraries + needs: + - ci-tests + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Release charm libraries + uses: canonical/charming-actions/release-libraries@2.7.0 + with: + credentials: "${{ secrets.CHARMHUB_TOKEN }}" + github-token: "${{ secrets.GITHUB_TOKEN }}" release: name: Release charm needs: - - build + - ci-tests uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v29.0.5 with: channel: 16/edge - artifact-prefix: ${{ needs.build.outputs.artifact-prefix }} + artifact-prefix: ${{ needs.ci-tests.outputs.artifact-prefix }} secrets: charmhub-token: ${{ secrets.CHARMHUB_TOKEN }} permissions: diff --git a/src/dependency.json b/src/dependency.json index 24a9c60013..1f87a03a6d 100644 --- a/src/dependency.json +++ b/src/dependency.json @@ -9,6 +9,6 @@ "dependencies": {}, "name": "charmed-postgresql", "upgrade_supported": "^16", - "version": "16.4" + "version": "16.6" } } diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index c7258431cc..5d37458ebf 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -206,7 +206,7 @@ def test_can_use_s3_repository(harness): patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, patch( "charm.Patroni.rock_postgresql_version", - new_callable=PropertyMock(return_value="16.4"), + new_callable=PropertyMock(return_value="16.6"), ) as _rock_postgresql_version, patch("charm.PostgreSQLBackups._execute_command") as _execute_command, patch( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index c0e5144099..e1d2318c81 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -198,7 +198,7 @@ def test_on_postgresql_pebble_ready(harness): patch("charm.PostgresqlOperatorCharm._on_leader_elected"), patch("charm.PostgresqlOperatorCharm._create_pgdata") as _create_pgdata, ): - _rock_postgresql_version.return_value = "16.4" + _rock_postgresql_version.return_value = "16.6" # Mock the primary endpoint ready property values. _primary_endpoint_ready.side_effect = [False, True, True] @@ -255,7 +255,7 @@ def test_on_postgresql_pebble_ready_no_connection(harness): ): mock_event = MagicMock() mock_event.workload = harness.model.unit.get_container(POSTGRESQL_CONTAINER) - _rock_postgresql_version.return_value = "16.4" + _rock_postgresql_version.return_value = "16.6" harness.charm._on_postgresql_pebble_ready(mock_event) diff --git a/tests/unit/test_patroni.py b/tests/unit/test_patroni.py index 0ce48290dc..9be6ec6357 100644 --- a/tests/unit/test_patroni.py +++ b/tests/unit/test_patroni.py @@ -202,7 +202,7 @@ def test_render_patroni_yml_file(harness, patroni): ) as _rock_postgresql_version, patch("charm.Patroni._render_file") as _render_file, ): - _rock_postgresql_version.return_value = "16.4" + _rock_postgresql_version.return_value = "16.6" # Get the expected content from a file. with open("templates/patroni.yml.j2") as file: