From 67b6db5331bfbc89206ba42af82b7fa743802348 Mon Sep 17 00:00:00 2001 From: gatici Date: Fri, 24 Nov 2023 16:58:28 +0300 Subject: [PATCH] chore: Use Ops.CollectStatus Event Signed-off-by: gatici --- src/charm.py | 109 +++++++++++++++++--------- tests/integration/test_integration.py | 9 +++ tests/unit/test_charm.py | 41 ++++++---- 3 files changed, 107 insertions(+), 52 deletions(-) diff --git a/src/charm.py b/src/charm.py index 782cc10..bd34145 100755 --- a/src/charm.py +++ b/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 diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index c61cd6e..e522d11 100644 --- a/tests/integration/test_integration.py +++ b/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) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 8eb57dc..d87f0ec 100644 --- a/tests/unit/test_charm.py +++ b/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"),