From 3dc464ebb743c81adcb4c1e14311e4073153faff Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Wed, 3 Apr 2024 13:31:14 +0300 Subject: [PATCH 1/9] Update snap and renovate tweaks --- .github/renovate.json5 | 62 ++++++++++++++++++++++++++++++++++++++- .github/workflows/ci.yaml | 6 ++-- poetry.lock | 2 +- pyproject.toml | 3 +- src/constants.py | 2 +- 5 files changed, 68 insertions(+), 7 deletions(-) diff --git a/.github/renovate.json5 b/.github/renovate.json5 index f9c69332ab..ddc64b8123 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -1,7 +1,7 @@ { "$schema": "https://docs.renovatebot.com/renovate-schema.json", "extends": ["github>canonical/data-platform//renovate_presets/charm.json5"], - "reviewers": ["dragomirp", "marceloneppel", "taurus-forever"], + "reviewers": ["dragomirp", "lucasgameiroborges", "marceloneppel", "taurus-forever"], "packageRules": [ // Later rules override earlier rules { @@ -10,6 +10,66 @@ }, { "matchPackageNames": ["python"], "allowedVersions": "<3.11" + }, + { + "matchDepNames": ["Juju 2"], + "matchPackageNames": ["juju/juju"], + "allowedVersions": "<3.0.0", + "extractVersion": "^v(?.*)$", + "groupName": "Juju 2" + }, + { + "matchDepNames": ["Juju 3"], + "matchPackageNames": ["juju/juju"], + "allowedVersions": "<3.2.0", + "extractVersion": "^v(?.*)$", + "groupName": "Juju 3" + }, + { + "matchDepNames": ["libjuju 2"], + "matchPackageNames": ["juju"], + "matchManagers": ["regex"], + "matchDatasources": ["pypi"], + "versioning": "loose", + "allowedVersions": "<3", + "groupName": "Juju 2" + } + ], + "regexManagers": [ + { + "customType": "regex", + "fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"], + "matchStrings": [ + "(- agent: )(?.*?) +# renovate: latest juju 2" + ], + "depNameTemplate": "Juju 2", + "packageNameTemplate": "juju/juju", + "datasourceTemplate": "github-releases", + "versioningTemplate": "loose", + "extractVersionTemplate": "Juju release" + }, + { + "customType": "regex", + "fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"], + "matchStrings": [ + "(- agent: )(?.*?) +# renovate: latest juju 3" + ], + "depNameTemplate": "Juju 3", + "packageNameTemplate": "juju/juju", + "datasourceTemplate": "github-releases", + "versioningTemplate": "loose", + "extractVersionTemplate": "Juju release" + }, + { + "customType": "regex", + "fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"], + "matchStrings": [ + "(libjuju: )==(?.*?) +# renovate: latest libjuju 2" + ], + "depNameTemplate": "libjuju 2", + "packageNameTemplate": "juju", + "datasourceTemplate": "pypi", + "versioningTemplate": "loose" } ] } diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fd2a595900..8d9ed83ec1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -51,10 +51,10 @@ jobs: fail-fast: false matrix: juju: - - agent: 2.9.46 - libjuju: ^2 + - agent: 2.9.47 # renovate: latest juju 2 + libjuju: ==2.9.46.1 # renovate: latest libjuju 2 allure: false - - agent: 3.1.7 + - agent: 3.1.7 # renovate: latest juju 3 allure: true name: Integration test charm | ${{ matrix.juju.agent }} needs: diff --git a/poetry.lock b/poetry.lock index 4c45b5b62c..b6df28ac6c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2062,4 +2062,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "6a801b06e8e0b7f08f62700e06b6b390f5973986d8706a5170911b05fefd3c58" +content-hash = "153ca14f2bd513c50b698e4826591e462f690d92864122f555e72ac66e97da35" diff --git a/pyproject.toml b/pyproject.toml index 467dfb3ece..6274a18e2d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,7 +70,8 @@ pytest-github-secrets = {git = "https://github.com/canonical/data-platform-workf pytest-operator = "^0.34.0" pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", tag = "v13.0.0", subdirectory = "python/pytest_plugins/pytest_operator_cache"} pytest-operator-groups = {git = "https://github.com/canonical/data-platform-workflows", tag = "v13.0.0", subdirectory = "python/pytest_plugins/pytest_operator_groups"} -juju = "^3.3.0.0" +# renovate caret doesn't work: https://github.com/renovatebot/renovate/issues/26940 +juju = "<=3.4.0.0" boto3 = "*" tenacity = "*" landscape-api-py3 = "^0.9.0" diff --git a/src/constants.py b/src/constants.py index 3f0d79114c..49929ff037 100644 --- a/src/constants.py +++ b/src/constants.py @@ -35,7 +35,7 @@ SNAP_PACKAGES = [ ( POSTGRESQL_SNAP_NAME, - {"revision": {"aarch64": "105", "x86_64": "104"}, "channel": "14/stable"}, + {"revision": {"aarch64": "110", "x86_64": "111"}, "channel": "14/stable"}, ) ] From a48c740c8ec76ec44a9282610916d0efdfccc7cc Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Thu, 4 Apr 2024 17:58:11 +0300 Subject: [PATCH 2/9] Revert charm.py --- src/charm.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/charm.py b/src/charm.py index be196580c3..92407e9d83 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1501,11 +1501,7 @@ def update_config(self, is_creating_backup: bool = False) -> bool: "max_prepared_transactions": self.config.memory_max_prepared_transactions, }) - try: - self._handle_postgresql_restart_need(enable_tls) - except RetryError: - logger.warning("Early exit update_config: Cannot handle restrt need") - return False + self._handle_postgresql_restart_need(enable_tls) # Restart the monitoring service if the password was rotated cache = snap.SnapCache() From 0d085f078ddc55c240f01cb2b82e09902e9ef426 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Thu, 4 Apr 2024 20:11:09 +0300 Subject: [PATCH 3/9] Only check for blocked status --- tests/integration/test_backups.py | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py index 72dd34cd9b..c945f072ac 100644 --- a/tests/integration/test_backups.py +++ b/tests/integration/test_backups.py @@ -355,12 +355,9 @@ async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets) -> None # Wait for the restore to complete. async with ops_test.fast_forward(): - await wait_for_idle_on_blocked( - ops_test, - database_app_name, - 0, - S3_INTEGRATOR_APP_NAME, - ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE, + unit = ops_test.model.units.get(f"{database_app_name}/0") + await ops_test.model.block_until( + lambda: unit.workload_status_message == ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE ) # Check that the backup was correctly restored by having only the first created table. @@ -402,12 +399,9 @@ async def test_invalid_config_and_recovery_after_fixing_it( ) await action.wait() logger.info("waiting for the database charm to become blocked") - await wait_for_idle_on_blocked( - ops_test, - database_app_name, - 0, - S3_INTEGRATOR_APP_NAME, - FAILED_TO_ACCESS_CREATE_BUCKET_ERROR_MESSAGE, + unit = ops_test.model.units.get(f"{database_app_name}/0") + await ops_test.model.block_until( + lambda: unit.workload_status_message == FAILED_TO_ACCESS_CREATE_BUCKET_ERROR_MESSAGE ) # Provide valid backup configurations, but from another cluster repository. @@ -421,12 +415,9 @@ async def test_invalid_config_and_recovery_after_fixing_it( ) await action.wait() logger.info("waiting for the database charm to become blocked") - await wait_for_idle_on_blocked( - ops_test, - database_app_name, - 0, - S3_INTEGRATOR_APP_NAME, - ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE, + unit = ops_test.model.units.get(f"{database_app_name}/0") + await ops_test.model.block_until( + lambda: unit.workload_status_message == ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE ) # Provide valid backup configurations, with another path in the S3 bucket. From bdefebd7d2ae3ae4350b8efd91fe3fef6392025d Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Thu, 4 Apr 2024 23:07:02 +0300 Subject: [PATCH 4/9] Try larger ffwd period --- tests/integration/test_backups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py index 72dd34cd9b..059aad5303 100644 --- a/tests/integration/test_backups.py +++ b/tests/integration/test_backups.py @@ -354,7 +354,7 @@ async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets) -> None assert restore_status, "restore hasn't succeeded" # Wait for the restore to complete. - async with ops_test.fast_forward(): + async with ops_test.fast_forward(fast_interval="60s"): await wait_for_idle_on_blocked( ops_test, database_app_name, From 3033f6a8f0850c01902bec3f23675e54d3366b90 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Fri, 5 Apr 2024 13:34:43 +0300 Subject: [PATCH 5/9] Handle update config exceptions in push tls files --- src/charm.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 92407e9d83..9e4f198941 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1390,7 +1390,11 @@ def push_tls_files_to_workload(self) -> bool: if cert is not None: self._patroni.render_file(f"{PATRONI_CONF_PATH}/{TLS_CERT_FILE}", cert, 0o600) - return self.update_config() + try: + return self.update_config() + except Exception: + logger.exception("TLS files failed to push. Error in config update") + return False def _reboot_on_detached_storage(self, event: EventBase) -> None: """Reboot on detached storage. From a6dfc0cc987bcf1ccfa8590bc8696c6a988e65dd Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Fri, 5 Apr 2024 18:22:17 +0300 Subject: [PATCH 6/9] Use charm fixture instead of building it on the fly --- tests/integration/test_backups.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py index c945f072ac..aaad394aee 100644 --- a/tests/integration/test_backups.py +++ b/tests/integration/test_backups.py @@ -92,11 +92,8 @@ async def cloud_configs(ops_test: OpsTest, github_secrets) -> None: @pytest.mark.group(1) @pytest.mark.abort_on_fail -async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> None: +async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict], charm) -> None: """Build and deploy two units of PostgreSQL and then test the backup and restore actions.""" - # Build the PostgreSQL charm. - charm = await ops_test.build_charm(".") - # Deploy S3 Integrator and TLS Certificates Operator. await ops_test.model.deploy(S3_INTEGRATOR_APP_NAME) await ops_test.model.deploy(TLS_CERTIFICATES_APP_NAME, config=TLS_CONFIG, channel=TLS_CHANNEL) @@ -284,9 +281,8 @@ async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> No @pytest.mark.group(1) -async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets) -> None: +async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets, charm) -> None: """Test that is possible to restore a backup to another PostgreSQL cluster.""" - charm = await ops_test.build_charm(".") previous_database_app_name = f"{DATABASE_APP_NAME}-gcp" database_app_name = f"new-{DATABASE_APP_NAME}" await ops_test.model.deploy(charm, application_name=previous_database_app_name) From d5521d65aea2fb4629e3e64661bbf05fad63fb80 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Mon, 8 Apr 2024 18:58:52 +0300 Subject: [PATCH 7/9] Don't clean up per cloud app --- tests/integration/test_backups.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py index aaad394aee..741f1c6bea 100644 --- a/tests/integration/test_backups.py +++ b/tests/integration/test_backups.py @@ -274,8 +274,6 @@ async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict], charm assert backups, "backups not outputted" await ops_test.model.wait_for_idle(status="active", timeout=1000) - # Remove the database app. - await ops_test.model.remove_application(database_app_name, block_until_done=True) # Remove the TLS operator. await ops_test.model.remove_application(TLS_CERTIFICATES_APP_NAME, block_until_done=True) @@ -285,13 +283,11 @@ async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets, charm) """Test that is possible to restore a backup to another PostgreSQL cluster.""" previous_database_app_name = f"{DATABASE_APP_NAME}-gcp" database_app_name = f"new-{DATABASE_APP_NAME}" - await ops_test.model.deploy(charm, application_name=previous_database_app_name) await ops_test.model.deploy( charm, application_name=database_app_name, series=CHARM_SERIES, ) - await ops_test.model.relate(previous_database_app_name, S3_INTEGRATOR_APP_NAME) await ops_test.model.relate(database_app_name, S3_INTEGRATOR_APP_NAME) async with ops_test.fast_forward(): logger.info( From a3274390505b97b0128724abba3c68f0950ef528 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Mon, 8 Apr 2024 19:16:33 +0300 Subject: [PATCH 8/9] Check old juju 2 agent --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 23617cfa15..91d450a8b8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -51,7 +51,7 @@ jobs: fail-fast: false matrix: juju: - - agent: 2.9.47 # renovate: latest juju 2 + - agent: 2.9.46 # renovate: latest juju 2 libjuju: ==2.9.46.1 # renovate: latest libjuju 2 allure: false - agent: 3.1.7 # renovate: latest juju 3 From 7f45ebe42c30d190aba2c07643fcb9461338a92c Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Mon, 8 Apr 2024 20:10:14 +0300 Subject: [PATCH 9/9] Redeploy old charm --- tests/integration/test_backups.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py index 741f1c6bea..f63b714217 100644 --- a/tests/integration/test_backups.py +++ b/tests/integration/test_backups.py @@ -274,6 +274,9 @@ async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict], charm assert backups, "backups not outputted" await ops_test.model.wait_for_idle(status="active", timeout=1000) + # Remove the database app. + await ops_test.model.remove_application(database_app_name, block_until_done=True) + # Remove the TLS operator. await ops_test.model.remove_application(TLS_CERTIFICATES_APP_NAME, block_until_done=True) @@ -283,11 +286,13 @@ async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets, charm) """Test that is possible to restore a backup to another PostgreSQL cluster.""" previous_database_app_name = f"{DATABASE_APP_NAME}-gcp" database_app_name = f"new-{DATABASE_APP_NAME}" + await ops_test.model.deploy(charm, application_name=previous_database_app_name) await ops_test.model.deploy( charm, application_name=database_app_name, series=CHARM_SERIES, ) + await ops_test.model.relate(previous_database_app_name, S3_INTEGRATOR_APP_NAME) await ops_test.model.relate(database_app_name, S3_INTEGRATOR_APP_NAME) async with ops_test.fast_forward(): logger.info(