chore: Use Ops.CollectStatus Event

Signed-off-by: gatici <gulsum.atici@canonical.com>

src/charm.py

@@ -23,10 +23,10 @@
 )
 from jinja2 import Environment, FileSystemLoader
 from lightkube.models.core_v1 import ServicePort
-from ops.charm import CharmBase
+from ops import ActiveStatus, BlockedStatus, StatusBase, WaitingStatus
+from ops.charm import CharmBase, CollectStatusEvent, RelationBrokenEvent
 from ops.framework import EventBase
 from ops.main import main
-from ops.model import ActiveStatus, BlockedStatus, WaitingStatus
 from ops.pebble import Layer
 
 logger = logging.getLogger(__name__)
@@ -50,8 +50,6 @@ class PCFOperatorCharm(CharmBase):
 
     def __init__(self, *args):
         super().__init__(*args)
-        if not self.unit.is_leader():
-            raise NotImplementedError("Scaling is not implemented for this charm")
         self._container_name = self._service_name = "pcf"
         self._container = self.unit.get_container(self._container_name)
 
@@ -64,6 +62,12 @@ def __init__(self, *args):
             ports=[ServicePort(name="sbi", port=PCF_SBI_PORT)],
         )
         self._certificates = TLSCertificatesRequiresV2(self, "certificates")
+        # Setting attributes to detect broken relations until
+        # https://github.com/canonical/operator/issues/940 is fixed
+        self._database_relation_breaking = False
+        self._nrf_relation_breaking = False
+        self._tls_relation_breaking = False
+        self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status)
         self.framework.observe(self.on.database_relation_joined, self._configure_sdcore_pcf)
         self.framework.observe(self.on.database_relation_broken, self._on_database_relation_broken)
         self.framework.observe(self._database.on.database_created, self._configure_sdcore_pcf)
@@ -87,60 +91,92 @@ def __init__(self, *args):
             self._certificates.on.certificate_expiring, self._on_certificate_expiring
         )
 
-    def _configure_sdcore_pcf(self, event: EventBase):
-        """Adds Pebble layer and manages Juju unit status.
+    def _is_unit_in_non_active_status(self) -> Optional[StatusBase]:  # noqa: C901
+        """Evaluate and return the unit's current status, or None if it should be active.
+
+        Returns:
+            StatusBase: MaintenanceStatus/BlockedStatus/WaitingStatus
+            None: If none of the conditionals match
 
-        Args:
-            event (EventBase): Juju event.
         """
+        if not self.unit.is_leader():
+            # NOTE: In cases where leader status is lost before the charm is
+            # finished processing all teardown events, this prevents teardown
+            # event code from running. Luckily, for this charm, none of the
+            # teardown code is necessary to preform if we're removing the
+            # charm.
+            return BlockedStatus("Scaling is not implemented for this charm")
+
         if not self._container.can_connect():
-            self.unit.status = WaitingStatus("Waiting for container to be ready")
-            return
-        for relation in [DATABASE_RELATION_NAME, NRF_RELATION_NAME, TLS_RELATION_NAME]:
-            if not self._relation_created(relation):
-                self.unit.status = BlockedStatus(
-                    f"Waiting for `{relation}` relation to be created"
-                )
-                return
+            return WaitingStatus("Waiting for container to be ready")
+
+        if not self.model.get_relation(NRF_RELATION_NAME) or self._nrf_relation_breaking:
+            return BlockedStatus(f"Waiting for {NRF_RELATION_NAME} relation")
+
+        if not self.model.get_relation(DATABASE_RELATION_NAME) or self._database_relation_breaking:
+            return BlockedStatus(f"Waiting for {DATABASE_RELATION_NAME} relation")
+
+        if not self.model.get_relation(TLS_RELATION_NAME) or self._tls_relation_breaking:
+            return BlockedStatus(f"Waiting for {TLS_RELATION_NAME} relation")
+
         if not self._database_is_available():
-            self.unit.status = WaitingStatus(
+            return WaitingStatus(
                 f"Waiting for `{DATABASE_RELATION_NAME}` relation to be available"
             )
-            return
+
         if not self._nrf_is_available():
-            self.unit.status = WaitingStatus("Waiting for NRF endpoint to be available")
-            return
+            return WaitingStatus("Waiting for NRF endpoint to be available")
+
         if not self._storage_is_attached():
-            self.unit.status = WaitingStatus("Waiting for the storage to be attached")
-            event.defer()
-            return
+            return WaitingStatus("Waiting for the storage to be attached")
+
         if not _get_pod_ip():
-            self.unit.status = WaitingStatus("Waiting for pod IP address to be available")
-            event.defer()
-            return
+            return WaitingStatus("Waiting for pod IP address to be available")
+
         if not self._certificate_is_stored():
-            self.unit.status = WaitingStatus("Waiting for certificates to be stored")
+            return WaitingStatus("Waiting for certificates to be stored")
+
+        return None
+
+    def _on_collect_unit_status(self, event: CollectStatusEvent):
+        """Check the unit status and set to Unit when CollectStatusEvent is fired.
+
+        Args:
+            event: CollectStatusEvent
+        """
+        if status := self._is_unit_in_non_active_status():
+            event.add_status(status)
+        else:
+            event.add_status(ActiveStatus())
+
+    def _configure_sdcore_pcf(self, event: EventBase):
+        """Adds Pebble layer and manages Juju unit status.
+
+        Args:
+            event (EventBase): Juju event.
+        """
+        if self._is_unit_in_non_active_status():
+            # Unit Status is in Maintanence or Blocked or Waiting status
             event.defer()
             return
         restart = self._update_config_file()
         self._configure_pebble(restart=restart)
-        self.unit.status = ActiveStatus()
 
-    def _on_nrf_broken(self, event: EventBase) -> None:
+    def _on_nrf_broken(self, event: RelationBrokenEvent) -> None:
         """Event handler for NRF relation broken.
 
         Args:
             event (NRFBrokenEvent): Juju event
         """
-        self.unit.status = BlockedStatus("Waiting for fiveg_nrf relation")
+        self._nrf_relation_breaking = True
 
-    def _on_database_relation_broken(self, event: EventBase) -> None:
+    def _on_database_relation_broken(self, event: RelationBrokenEvent) -> None:
         """Event handler for database relation broken.
 
         Args:
             event: Juju event
         """
-        self.unit.status = BlockedStatus("Waiting for database relation")
+        self._database_relation_breaking = True
 
     def _on_certificates_relation_created(self, event: EventBase) -> None:
         """Generates Private key.
@@ -153,7 +189,7 @@ def _on_certificates_relation_created(self, event: EventBase) -> None:
             return
         self._generate_private_key()
 
-    def _on_certificates_relation_broken(self, event: EventBase) -> None:
+    def _on_certificates_relation_broken(self, event: RelationBrokenEvent) -> None:
         """Deletes TLS related artifacts and reconfigures workload.
 
         Args:
@@ -165,7 +201,7 @@ def _on_certificates_relation_broken(self, event: EventBase) -> None:
         self._delete_private_key()
         self._delete_csr()
         self._delete_certificate()
-        self.unit.status = BlockedStatus("Waiting for certificates relation")
+        self._tls_relation_breaking = True
 
     def _on_certificates_relation_joined(self, event: EventBase) -> None:
         """Generates CSR and requests new certificate.
@@ -491,7 +527,10 @@ def _get_pod_ip() -> Optional[str]:
     Returns:
         str: The pod IP.
     """
-    ip_address = check_output(["unit-get", "private-address"])
+    try:
+        ip_address = check_output(["unit-get", "private-address"])
+    except FileNotFoundError:
+        return None
     return str(IPv4Address(ip_address.decode().strip())) if ip_address else None
 
 

tests/integration/test_integration.py

@@ -98,6 +98,9 @@ async def test_relate_and_wait_for_active_status(ops_test: OpsTest, build_and_de
 @pytest.mark.abort_on_fail
 async def test_remove_nrf_and_wait_for_blocked_status(ops_test: OpsTest, build_and_deploy):
     await ops_test.model.remove_application(NRF_APP_NAME, block_until_done=True)  # type: ignore[union-attr]  # noqa: E501
+    # Running config-changed hook with empty config to check whether _nrf_relation_breaking
+    # attribute will not be set to its default value
+    await ops_test.model.applications[APPLICATION_NAME].set_config({})  # type: ignore[union-attr]
     await ops_test.model.wait_for_idle(apps=[APPLICATION_NAME], status="blocked", timeout=60)  # type: ignore[union-attr]  # noqa: E501
 
 
@@ -122,6 +125,9 @@ async def test_restore_nrf_and_wait_for_active_status(ops_test: OpsTest, build_a
 async def test_remove_tls_and_wait_for_blocked_status(ops_test: OpsTest, build_and_deploy):
     assert ops_test.model
     await ops_test.model.remove_application(TLS_PROVIDER_NAME, block_until_done=True)
+    # Running config-changed hook with empty config to check whether _tls_relation_breaking
+    # attribute will not be set to its default value
+    await ops_test.model.applications[APPLICATION_NAME].set_config({})
     await ops_test.model.wait_for_idle(apps=[APPLICATION_NAME], status="blocked", timeout=60)
 
 
@@ -145,6 +151,9 @@ async def test_restore_tls_and_wait_for_active_status(ops_test: OpsTest, build_a
 async def test_remove_database_and_wait_for_blocked_status(ops_test: OpsTest, build_and_deploy):
     assert ops_test.model
     await ops_test.model.remove_application(DATABASE_APP_NAME, block_until_done=True)
+    # Running config-changed hook with empty config to check whether _database_relation_breaking
+    # attribute will not be set to its default value
+    await ops_test.model.applications[APPLICATION_NAME].set_config({})
     await ops_test.model.wait_for_idle(apps=[APPLICATION_NAME], status="blocked", timeout=60)
 
 

tests/unit/test_charm.py

@@ -114,7 +114,7 @@ def test_given_container_cant_connect_when_configure_sdcore_pcf_then_status_is_w
         self.harness.set_can_connect(container=self.container_name, val=False)
 
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status, WaitingStatus("Waiting for container to be ready")
         )
@@ -123,25 +123,32 @@ def test_given_container_can_connect_and_database_relation_is_not_created_when_c
         self,
     ):
         self.harness.set_can_connect(container=self.container_name, val=True)
-
+        self._create_nrf_relation()
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
-            BlockedStatus("Waiting for `database` relation to be created"),
+            BlockedStatus("Waiting for database relation"),
         )
 
+    @patch("charm.generate_private_key")
     def test_given_container_can_connect_and_fiveg_nrf_relation_is_not_created_when_configure_sdcore_pcf_then_status_is_blocked(  # noqa: E501
-        self,
+        self, patch_generate_private_key
     ):
         self.harness.set_can_connect(container=self.container_name, val=True)
         self._create_database_relation()
+        self.harness.add_storage(storage_name="certs", attach=True)
+        self.harness.add_storage(storage_name="config", attach=True)
+        private_key = b"whatever key content"
+        self.harness.set_can_connect(container=self.container_name, val=True)
+        patch_generate_private_key.return_value = private_key
 
+        self.harness.charm._on_certificates_relation_created(event=Mock)
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
-            BlockedStatus("Waiting for `fiveg_nrf` relation to be created"),
+            BlockedStatus("Waiting for fiveg_nrf relation"),
         )
 
     def test_given_container_can_connect_and_certificates_relation_is_not_created_when_configure_sdcore_pcf_then_status_is_blocked(  # noqa: E501
@@ -152,10 +159,10 @@ def test_given_container_can_connect_and_certificates_relation_is_not_created_wh
         self._create_nrf_relation()
 
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
-            BlockedStatus("Waiting for `certificates` relation to be created"),
+            BlockedStatus("Waiting for certificates relation"),
         )
 
     @patch("charm.check_output")
@@ -181,7 +188,7 @@ def test_given_pcf_charm_in_active_state_when_nrf_relation_breaks_then_status_is
         self.harness.container_pebble_ready(self.container_name)
 
         self.harness.remove_relation(nrf_relation_id)
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
             BlockedStatus("Waiting for fiveg_nrf relation"),
@@ -210,7 +217,7 @@ def test_given_pcf_charm_in_active_state_when_database_relation_breaks_then_stat
         self.harness.container_pebble_ready(self.container_name)
 
         self.harness.remove_relation(database_relation_id)
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
             BlockedStatus("Waiting for database relation"),
@@ -227,7 +234,7 @@ def test_given_container_can_connect_and_database_relation_is_not_available_when
             relation_name=TLS_RELATION_NAME, remote_app="tls-certificates-operator"
         )
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
             WaitingStatus("Waiting for `database` relation to be available"),
@@ -245,7 +252,7 @@ def test_given_container_can_connect_and_fiveg_nrf_relation_is_not_available_whe
         )
 
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
             WaitingStatus("Waiting for NRF endpoint to be available"),
@@ -266,7 +273,7 @@ def test_given_container_storage_is_not_attached_when_configure_sdcore_pcf_then_
         )
 
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status, WaitingStatus("Waiting for the storage to be attached")
         )
@@ -290,7 +297,7 @@ def test_given_certificate_is_not_stored_when_configure_sdcore_pcf_then_status_i
         patch_check_output.return_value = b"1.1.1.1"
 
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status, WaitingStatus("Waiting for certificates to be stored")
         )
@@ -448,7 +455,7 @@ def test_given_config_file_is_written_when_configure_sdcore_pcf_is_called_then_s
         )
 
         self.harness.charm._configure_sdcore_pcf(event=Mock())
-
+        self.harness.evaluate_status()
         self.assertEqual(self.harness.model.unit.status, ActiveStatus())
 
     @patch("ops.model.Container.restart", new=Mock)
@@ -469,7 +476,7 @@ def test_given_ip_not_available_when_configure_then_status_is_waiting(
             relation_name=TLS_RELATION_NAME, remote_app="tls-certificates-operator"
         )
         self.harness.container_pebble_ready(container_name=self.container_name)
-
+        self.harness.evaluate_status()
         self.assertEqual(
             self.harness.model.unit.status,
             WaitingStatus("Waiting for pod IP address to be available"),