From f61839d81eb7ad78ce4ec87e9ffbfb8d4df80898 Mon Sep 17 00:00:00 2001 From: gatici Date: Wed, 13 Mar 2024 16:26:48 +0300 Subject: [PATCH] chore: Centralize Status Management Signed-off-by: gatici --- src/charm.py | 245 ++++++++++++++++++++++++++------------- tests/unit/test_charm.py | 44 ++++--- 2 files changed, 183 insertions(+), 106 deletions(-) diff --git a/src/charm.py b/src/charm.py index d099fb4..658663d 100755 --- a/src/charm.py +++ b/src/charm.py @@ -19,10 +19,17 @@ generate_private_key, ) from jinja2 import Environment, FileSystemLoader +from ops import ( + ActiveStatus, + BlockedStatus, + CollectStatusEvent, + ModelError, + RelationBrokenEvent, + WaitingStatus, +) from ops.charm import CharmBase 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__) @@ -47,8 +54,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) @@ -60,12 +65,13 @@ def __init__(self, *args): self._certificates = TLSCertificatesRequiresV3(self, "certificates") self._logging = LogForwarder(charm=self, relation_name=LOGGING_RELATION_NAME) self.framework.observe(self.on.update_status, self._configure_sdcore_pcf) + 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.on.database_relation_broken, self._configure_sdcore_pcf) self.framework.observe(self._database.on.database_created, self._configure_sdcore_pcf) self.framework.observe(self.on.fiveg_nrf_relation_joined, self._configure_sdcore_pcf) self.framework.observe(self._nrf_requires.on.nrf_available, self._configure_sdcore_pcf) - self.framework.observe(self._nrf_requires.on.nrf_broken, self._on_nrf_broken) + self.framework.observe(self._nrf_requires.on.nrf_broken, self._configure_sdcore_pcf) self.framework.observe(self.on.pcf_pebble_ready, self._configure_sdcore_pcf) self.framework.observe(self.on.certificates_relation_joined, self._configure_sdcore_pcf) self.framework.observe( @@ -78,66 +84,192 @@ def __init__(self, *args): self._certificates.on.certificate_expiring, self._on_certificate_expiring ) - def _configure_sdcore_pcf(self, event: EventBase) -> None: # noqa C901 - """Adds Pebble layer and manages Juju unit status. + def _on_collect_unit_status(self, event: CollectStatusEvent): # noqa C901 + """Check the unit status and set to Unit when CollectStatusEvent is fired. Args: - event (EventBase): Juju event. + event: CollectStatusEvent """ + 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 perform if we're removing the + # charm. + event.add_status(BlockedStatus("Scaling is not implemented for this charm")) + return + if not self._container.can_connect(): - self.unit.status = WaitingStatus("Waiting for container to be ready") + event.add_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" - ) + event.add_status(BlockedStatus(f"Waiting for {relation} relation")) return + if not self._database_is_available(): - self.unit.status = WaitingStatus( - f"Waiting for `{DATABASE_RELATION_NAME}` relation to be available" + event.add_status( + 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") + event.add_status(WaitingStatus("Waiting for NRF endpoint to be available")) return + if not self._storage_is_attached(): - self.unit.status = WaitingStatus("Waiting for the storage to be attached") + event.add_status(WaitingStatus("Waiting for the storage to be attached")) + return + + if not _get_pod_ip(): + event.add_status(WaitingStatus("Waiting for pod IP address to be available")) + return + + if self._csr_is_stored() and not self._get_current_provider_certificate(): + event.add_status(WaitingStatus("Waiting for certificates to be stored")) + return + + if not self._pcf_service_is_running(): + event.add_status(WaitingStatus("Waiting for PCF service to start")) return + + event.add_status(ActiveStatus()) + + def _pcf_service_is_running(self) -> bool: + """Check if the PCF service is running. + + Returns: + bool: Whether the PCF service is running. + """ + if not self._container.can_connect(): + return False + try: + service = self._container.get_service(self._service_name) + except ModelError: + return False + return service.is_running() + + def ready_to_configure(self) -> bool: + """Returns whether the preconditions are met to proceed with the configuration. + + Returns: + ready_to_configure: True if all conditions are met else False + """ + if not self._container.can_connect(): + return False + + for relation in [DATABASE_RELATION_NAME, NRF_RELATION_NAME, TLS_RELATION_NAME]: + if not self._relation_created(relation): + return False + + if not self._database_is_available(): + return False + + if not self._nrf_is_available(): + return False + + if not self._storage_is_attached(): + return False + if not _get_pod_ip(): - self.unit.status = WaitingStatus("Waiting for pod IP address to be available") + return False + + return True + + def _configure_sdcore_pcf(self, event: EventBase) -> None: + """Adds Pebble layer and manages Juju unit status. + + Args: + event (EventBase): Juju event. + """ + if not self.ready_to_configure(): return + if not self._private_key_is_stored(): self._generate_private_key() + if not self._csr_is_stored(): self._request_new_certificate() provider_certificate = self._get_current_provider_certificate() if not provider_certificate: - self.unit.status = WaitingStatus("Waiting for certificates to be stored") return - certificate_changed = self._update_certificate(provider_certificate=provider_certificate) - config_file_changed = self._update_config_file() - self._configure_pebble(restart=(config_file_changed or certificate_changed)) - self.unit.status = ActiveStatus() - def _on_nrf_broken(self, event: EventBase) -> None: - """Event handler for NRF relation broken. + if certificate_update_required := self._is_certificate_update_required( + provider_certificate + ): + self._store_certificate(certificate=provider_certificate) + + desired_config_file = self._generate_pcf_config_file() + if config_update_required := self._is_config_update_required(desired_config_file): + self._push_config_file(content=desired_config_file) + + should_restart = config_update_required or certificate_update_required + self._configure_pebble(restart=should_restart) + + def _is_certificate_update_required(self, provider_certificate) -> bool: + """Checks the provided certificate and existing certificate. + + Returns True if update is required. Args: - event (NRFBrokenEvent): Juju event + provider_certificate: str + Returns: + True if update is required else False """ - self.unit.status = BlockedStatus("Waiting for fiveg_nrf relation") + return self._get_existing_certificate() != provider_certificate - def _on_database_relation_broken(self, event: EventBase) -> None: - """Event handler for database relation broken. + def _get_existing_certificate(self) -> str: + """Returns the existing certificate if present else empty string.""" + return self._get_stored_certificate() if self._certificate_is_stored() else "" + + def _push_config_file( + self, + content: str, + ) -> None: + """Push the PCF config file to the container. Args: - event: Juju event + content (str): Content of the config file. """ - self.unit.status = BlockedStatus("Waiting for database relation") + self._container.push( + path=f"{BASE_CONFIG_PATH}/{CONFIG_FILE_NAME}", + source=content, + ) + logger.info("Pushed: %s to workload.", CONFIG_FILE_NAME) + + def _generate_pcf_config_file(self) -> str: + """Handles creation of the PCF config file based on a given template. + + Returns: + content (str): desired config file content + """ + return self._render_config_file( + database_name=DATABASE_NAME, + database_url=self._get_database_data()["uris"].split(",")[0], + nrf_url=self._nrf_requires.nrf_url, + pcf_sbi_port=PCF_SBI_PORT, + pod_ip=_get_pod_ip(), # type: ignore[arg-type] + scheme="https", + ) + + def _is_config_update_required(self, content: str) -> bool: + """Decides whether config update is required by checking existence and config content. + + Args: + content (str): desired config file content - def _on_certificates_relation_broken(self, event: EventBase) -> None: + Returns: + True if config update is required else False + """ + if not self._config_file_is_written() or not self._config_file_content_matches( + content=content + ): + return True + return False + + def _on_certificates_relation_broken(self, event: RelationBrokenEvent) -> None: """Deletes TLS related artifacts and reconfigures workload. Args: @@ -149,7 +281,6 @@ 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") def _get_current_provider_certificate(self) -> str | None: """Compares the current certificate request to what is in the interface. @@ -162,20 +293,6 @@ def _get_current_provider_certificate(self) -> str | None: return provider_certificate.certificate return None - def _update_certificate(self, provider_certificate) -> bool: - """Compares the provided certificate to what is stored. - - Returns True if the certificate was updated - """ - existing_certificate = ( - self._get_stored_certificate() if self._certificate_is_stored() else "" - ) - - if existing_certificate != provider_certificate: - self._store_certificate(certificate=provider_certificate) - return True - return False - def _on_certificate_expiring(self, event: CertificateExpiringEvent) -> None: """Requests new certificate. @@ -284,30 +401,6 @@ def _configure_pebble(self, restart: bool = False) -> None: return self._container.replan() - def _update_config_file(self) -> bool: - """Updates config file. - - Writes the config file if it does not exist or - the content does not match. - - Returns: - bool: True if config file was updated, False otherwise. - """ - content = self._render_config_file( - database_name=DATABASE_NAME, - database_url=self._get_database_data()["uris"].split(",")[0], - nrf_url=self._nrf_requires.nrf_url, - pcf_sbi_port=PCF_SBI_PORT, - pod_ip=_get_pod_ip(), # type: ignore[arg-type] - scheme="https", - ) - if not self._config_file_is_written() or not self._config_file_content_matches( - content=content - ): - self._write_config_file(content=content) - return True - return False - def _render_config_file( self, *, @@ -340,18 +433,6 @@ def _render_config_file( scheme=scheme, ) - def _write_config_file(self, content: str) -> None: - """Writes config file to workload. - - Args: - content (str): Config file content. - """ - self._container.push( - path=f"{BASE_CONFIG_PATH}/{CONFIG_FILE_NAME}", - source=content, - ) - logger.info("Pushed: %s to workload.", CONFIG_FILE_NAME) - def _config_file_is_written(self) -> bool: """Returns whether the config file was written to the workload container. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 2eaaa16..5d9be08 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -133,7 +133,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") ) @@ -144,10 +144,10 @@ def test_given_container_can_connect_and_database_relation_is_not_created_when_c self.harness.set_can_connect(container=self.container_name, val=True) 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"), ) def test_given_container_can_connect_and_fiveg_nrf_relation_is_not_created_when_configure_sdcore_pcf_then_status_is_blocked( # noqa: E501 @@ -157,10 +157,10 @@ def test_given_container_can_connect_and_fiveg_nrf_relation_is_not_created_when_ self._create_database_relation() 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 @@ -171,10 +171,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") @@ -196,9 +196,8 @@ def test_given_pcf_charm_in_active_state_when_nrf_relation_breaks_then_status_is relation_name=TLS_RELATION_NAME, remote_app="tls-certificates-operator" ) 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"), @@ -223,9 +222,8 @@ def test_given_pcf_charm_in_active_state_when_database_relation_breaks_then_stat relation_name=TLS_RELATION_NAME, remote_app="tls-certificates-operator" ) 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"), @@ -242,7 +240,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"), @@ -260,7 +258,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"), @@ -281,7 +279,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") ) @@ -305,7 +303,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") ) @@ -458,6 +456,10 @@ def test_given_config_file_is_written_when_configure_sdcore_pcf_is_called_then_s self.harness.add_storage(storage_name="certs", attach=True) self.harness.add_storage(storage_name="config", attach=True) root = self.harness.get_filesystem_root(self.container_name) + provider_certificate = Mock(ProviderCertificate) + provider_certificate.certificate = CERTIFICATE + provider_certificate.csr = CSR + patch_get_assigned_certificates.return_value = [provider_certificate] (root / "support/TLS/pcf.pem").write_text(CERTIFICATE) (root / "support/TLS/pcf.csr").write_text(CSR) (root / f"etc/pcf/{CONFIG_FILE_NAME}").write_text("super different config file content") @@ -468,14 +470,8 @@ def test_given_config_file_is_written_when_configure_sdcore_pcf_is_called_then_s self._create_nrf_relation() self._create_database_relation_and_populate_data() self._create_certificates_relation() - - provider_certificate = Mock(ProviderCertificate) - provider_certificate.certificate = CERTIFICATE - provider_certificate.csr = CSR - patch_get_assigned_certificates.return_value = [provider_certificate] - - self.harness.container_pebble_ready(self.container_name) - + self.harness.container_pebble_ready(container_name=self.container_name) + self.harness.evaluate_status() self.assertEqual(self.harness.model.unit.status, ActiveStatus()) @patch("ops.model.Container.restart", new=Mock) @@ -495,7 +491,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"),