From 9f00e8682e07f3b94cb83fe1da669e712decd43a Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Fri, 10 Oct 2025 15:53:02 +0300 Subject: [PATCH 1/4] Check juju version before removing revision --- src/charm.py | 8 +++++++- tests/unit/test_charm.py | 26 +++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index e90658cca1..707a322a99 100755 --- a/src/charm.py +++ b/src/charm.py @@ -39,7 +39,7 @@ from charms.postgresql_k8s.v0.postgresql_tls import PostgreSQLTLS from charms.rolling_ops.v0.rollingops import RollingOpsManager, RunWithLock from charms.tempo_coordinator_k8s.v0.charm_tracing import trace_charm -from ops import main +from ops import JujuVersion, main from ops.charm import ( ActionEvent, HookEvent, @@ -442,6 +442,12 @@ def _on_secret_remove(self, event: SecretRemoveEvent) -> None: # https://github.com/juju/juju/issues/20794. This check is to avoid the # errors that would happen if we tried to remove the revision in that case # (in the revision removal, the label is present). + if self.model.juju_version < JujuVersion("3.6.11"): + logger.warning( + f"Skipping secret revision with label {event.secret.label} revision {event.revision} due to https://github.com/juju/juju/issues/20782" + ) + return + if event.secret.label is None: logger.debug("Secret with no label cannot be removed") return diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d1a524e226..1747cf08a2 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -15,7 +15,7 @@ PostgreSQLEnableDisableExtensionError, PostgreSQLUpdateUserPasswordError, ) -from ops import Unit +from ops import JujuVersion, Unit from ops.framework import EventBase from ops.model import ( ActiveStatus, @@ -2933,3 +2933,27 @@ def test_relations_user_databases_map(harness): "replication": "all", "rewind": "all", } + + +def test_on_secret_remove(harness, only_with_juju_secrets): + with ( + patch("ops.model.Model.juju_version", new_callable=PropertyMock) as _juju_version, + ): + event = Mock() + + # New juju + _juju_version.return_value = JujuVersion("3.6.11") + harness.charm._on_secret_remove(event) + event.remove_revision.assert_called_once_with() + event.reset_mock() + + # Old juju + _juju_version.return_value = JujuVersion("3.6.9") + harness.charm._on_secret_remove(event) + assert not event.remove_revision.called + event.reset_mock() + + # No secret + event.secret.label = None + harness.charm._on_secret_remove(event) + assert not event.remove_revision.called From e1ec2f5b61b8540a522968bc1c3005b07d3d2775 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Fri, 10 Oct 2025 15:56:22 +0300 Subject: [PATCH 2/4] Remove log context --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 707a322a99..550ca2356a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -444,7 +444,7 @@ def _on_secret_remove(self, event: SecretRemoveEvent) -> None: # (in the revision removal, the label is present). if self.model.juju_version < JujuVersion("3.6.11"): logger.warning( - f"Skipping secret revision with label {event.secret.label} revision {event.revision} due to https://github.com/juju/juju/issues/20782" + "Skipping secret revision removal due to https://github.com/juju/juju/issues/20782" ) return From 9b6db1dc93ba3f93350eb8d162cc75075dd912fd Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Fri, 10 Oct 2025 16:36:56 +0300 Subject: [PATCH 3/4] Move if block --- src/charm.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/charm.py b/src/charm.py index 550ca2356a..12671a0683 100755 --- a/src/charm.py +++ b/src/charm.py @@ -438,19 +438,20 @@ def get_hostname_by_unit(self, _) -> str: return self._unit_ip def _on_secret_remove(self, event: SecretRemoveEvent) -> None: - # A secret removal (entire removal, not just a revision removal) causes - # https://github.com/juju/juju/issues/20794. This check is to avoid the - # errors that would happen if we tried to remove the revision in that case - # (in the revision removal, the label is present). if self.model.juju_version < JujuVersion("3.6.11"): logger.warning( "Skipping secret revision removal due to https://github.com/juju/juju/issues/20782" ) return + # A secret removal (entire removal, not just a revision removal) causes + # https://github.com/juju/juju/issues/20794. This check is to avoid the + # errors that would happen if we tried to remove the revision in that case + # (in the revision removal, the label is present). if event.secret.label is None: logger.debug("Secret with no label cannot be removed") return + logger.debug(f"Removing secret with label {event.secret.label} revision {event.revision}") event.remove_revision() From 2dd794e5c2f81ca19eef5a54b5388d28f8d7e46d Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Mon, 13 Oct 2025 14:32:47 +0300 Subject: [PATCH 4/4] Add abort on fail for test_postgresql_parameters_change --- tests/integration/test_charm.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 217116f9dc..7bb321221f 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -183,6 +183,7 @@ async def test_postgresql_locales(ops_test: OpsTest) -> None: assert locales == SNAP_LOCALES +@pytest.mark.abort_on_fail async def test_postgresql_parameters_change(ops_test: OpsTest) -> None: """Test that's possible to change PostgreSQL parameters.""" await ops_test.model.applications[DATABASE_APP_NAME].set_config({