From c08c15cd8c9d3965c0e2488fe2ba446a322b25f4 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Tue, 24 Jan 2023 20:37:24 +0200 Subject: [PATCH 1/4] Fix unit tests --- src/relations/db.py | 7 ++----- tests/unit/test_db.py | 39 +-------------------------------------- 2 files changed, 3 insertions(+), 43 deletions(-) diff --git a/src/relations/db.py b/src/relations/db.py index 7eae6b69b8..ad827496c9 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -250,11 +250,8 @@ def _on_relation_broken(self, event: RelationBrokenEvent) -> None: self.charm._has_blocked_status and self.charm.unit.status.message == EXTENSIONS_BLOCKING_MESSAGE ): - if "extensions" in event.relation.data.get( - event.app, {} - ) or "extensions" in event.relation.data.get(event.unit, {}): - if not self._check_for_blocking_relations(event.relation.id): - self.charm.unit.status = ActiveStatus() + if not self._check_for_blocking_relations(event.relation.id): + self.charm.unit.status = ActiveStatus() def update_endpoints(self, event: RelationChangedEvent = None) -> None: """Set the read/write and read-only endpoints.""" diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index 19cd0fea47..fb8d0b05cb 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -42,7 +42,6 @@ def setUp(self): # Define some relations. self.rel_id = self.harness.add_relation(RELATION_NAME, "application") self.harness.add_relation_unit(self.rel_id, "application/0") - self.harness.add_relation_unit(self.rel_id, self.unit) self.peer_rel_id = self.harness.add_relation(PEER, self.app) self.harness.add_relation_unit(self.peer_rel_id, self.unit) self.harness.update_relation_data( @@ -229,52 +228,16 @@ def test_on_relation_broken_extensions_unblock( postgresql_mock.delete_user = PropertyMock(return_value=None) self.harness.model.unit.status = BlockedStatus("extensions requested through relation") with self.harness.hooks_disabled(): - self.rel_id = self.harness.add_relation(RELATION_NAME, "application") self.harness.update_relation_data( self.rel_id, "application", - {"database": DATABASE, "extensions": ["test"]}, + {"database": DATABASE, "extensions": "test"}, ) # Break the relation before the database is ready. self.harness.remove_relation(self.rel_id) self.assertTrue(isinstance(self.harness.model.unit.status, ActiveStatus)) - @patch( - "charm.PostgresqlOperatorCharm.primary_endpoint", - new_callable=PropertyMock, - ) - @patch("charm.PostgresqlOperatorCharm._has_blocked_status", new_callable=PropertyMock) - @patch("charm.Patroni.member_started", new_callable=PropertyMock) - @patch("charm.DbProvides._on_relation_departed") - def test_on_relation_broken_extensions_keep_block( - self, _on_relation_departed, _member_started, _primary_endpoint, _has_blocked_status - ): - with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: - # Set some side effects to test multiple situations. - _has_blocked_status.return_value = True - _member_started.return_value = True - _primary_endpoint.return_value = {"1.1.1.1"} - postgresql_mock.delete_user = PropertyMock(return_value=None) - self.harness.model.unit.status = BlockedStatus("extensions requested through relation") - with self.harness.hooks_disabled(): - self.rel_id = self.harness.add_relation(RELATION_NAME, "application") - self.harness.update_relation_data( - self.rel_id, - "application", - {"database": DATABASE, "extensions": ["test"]}, - ) - second_rel_id = self.harness.add_relation(RELATION_NAME, "application2") - self.harness.update_relation_data( - second_rel_id, - "application2", - {"database": DATABASE, "extensions": ["test"]}, - ) - - # Break the relation before the database is ready. - self.harness.remove_relation(self.rel_id) - self.assertTrue(isinstance(self.harness.model.unit.status, BlockedStatus)) - @patch( "charm.DbProvides._get_state", side_effect="postgresql/0", From c45398e1397b334b351fd716c5bc974846760b2a Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Wed, 25 Jan 2023 11:29:51 +0200 Subject: [PATCH 2/4] Update postgresql_tls lib --- lib/charms/postgresql_k8s/v0/postgresql_tls.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/charms/postgresql_k8s/v0/postgresql_tls.py b/lib/charms/postgresql_k8s/v0/postgresql_tls.py index 46eba5aca6..63c3cf3bad 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql_tls.py +++ b/lib/charms/postgresql_k8s/v0/postgresql_tls.py @@ -125,7 +125,10 @@ def _on_tls_relation_broken(self, _) -> None: def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: """Enable TLS when TLS certificate available.""" - if event.certificate_signing_request.strip() != self.charm.get_secret(SCOPE, "csr").strip(): + if ( + event.certificate_signing_request.strip() + != str(self.charm.get_secret(SCOPE, "csr")).strip() + ): logger.error("An unknown certificate available.") return @@ -144,7 +147,7 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: def _on_certificate_expiring(self, event: CertificateExpiringEvent) -> None: """Request the new certificate when old certificate is expiring.""" - if event.certificate.strip() != self.charm.get_secret(SCOPE, "cert").strip(): + if event.certificate.strip() != str(self.charm.get_secret(SCOPE, "cert")).strip(): logger.error("An unknown certificate expiring.") return From d2e8f7febe2683d233ab640f9c931bb1857d1ffb Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Wed, 25 Jan 2023 13:42:16 +0200 Subject: [PATCH 3/4] Readding test_on_relation_broken_extensions_keep_block --- tests/unit/test_db.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index fb8d0b05cb..f31b3516cf 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -234,10 +234,41 @@ def test_on_relation_broken_extensions_unblock( {"database": DATABASE, "extensions": "test"}, ) - # Break the relation before the database is ready. + # Break the relation that blocked the charm. self.harness.remove_relation(self.rel_id) self.assertTrue(isinstance(self.harness.model.unit.status, ActiveStatus)) + @patch( + "charm.PostgresqlOperatorCharm.primary_endpoint", + new_callable=PropertyMock, + ) + @patch("charm.PostgresqlOperatorCharm._has_blocked_status", new_callable=PropertyMock) + @patch("charm.Patroni.member_started", new_callable=PropertyMock) + @patch("charm.DbProvides._on_relation_departed") + def test_on_relation_broken_extensions_keep_block( + self, _on_relation_departed, _member_started, _primary_endpoint, _has_blocked_status + ): + with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: + # Set some side effects to test multiple situations. + _has_blocked_status.return_value = True + _member_started.return_value = True + _primary_endpoint.return_value = {"1.1.1.1"} + postgresql_mock.delete_user = PropertyMock(return_value=None) + self.harness.model.unit.status = BlockedStatus("extensions requested through relation") + with self.harness.hooks_disabled(): + second_rel_id = self.harness.add_relation(RELATION_NAME, "application2") + self.harness.update_relation_data( + second_rel_id, + "application2", + {"database": DATABASE, "extensions": "test"}, + ) + + event = Mock() + event.relation.id = 1 + # Break one of the relations that block the charm. + self.harness.charm.legacy_db_relation._on_relation_broken(event) + self.assertTrue(isinstance(self.harness.model.unit.status, BlockedStatus)) + @patch( "charm.DbProvides._get_state", side_effect="postgresql/0", From f4bf81168a686e7b89d065fd6a4ed3fb6b26c7b7 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Wed, 25 Jan 2023 15:01:41 +0200 Subject: [PATCH 4/4] Code review improvements --- tests/unit/test_db.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index f31b3516cf..d9dfe72e2b 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -256,6 +256,12 @@ def test_on_relation_broken_extensions_keep_block( postgresql_mock.delete_user = PropertyMock(return_value=None) self.harness.model.unit.status = BlockedStatus("extensions requested through relation") with self.harness.hooks_disabled(): + first_rel_id = self.harness.add_relation(RELATION_NAME, "application1") + self.harness.update_relation_data( + first_rel_id, + "application1", + {"database": DATABASE, "extensions": "test"}, + ) second_rel_id = self.harness.add_relation(RELATION_NAME, "application2") self.harness.update_relation_data( second_rel_id, @@ -264,7 +270,7 @@ def test_on_relation_broken_extensions_keep_block( ) event = Mock() - event.relation.id = 1 + event.relation.id = first_rel_id # Break one of the relations that block the charm. self.harness.charm.legacy_db_relation._on_relation_broken(event) self.assertTrue(isinstance(self.harness.model.unit.status, BlockedStatus))