Skip to content

Commit

Permalink
Addressing review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: gatici <gulsum.atici@canonical.com>
  • Loading branch information
@gatici
gatici committed Nov 23, 2023
1 parent f444049 commit 53c6c12
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 22 deletions.
37 changes: 20 additions & 17 deletions src/charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,6 @@ class AMFOperatorCharm(CharmBase):

def __init__(self, *args):
super().__init__(*args)
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.
self.unit.status = BlockedStatus("Scaling is not implemented for this charm")
return
self._amf_container_name = self._amf_service_name = "amf"
self._amf_container = self.unit.get_container(self._amf_container_name)
self._nrf_requires = NRFRequires(charm=self, relation_name="fiveg-nrf")
Expand Down Expand Up @@ -134,16 +126,21 @@ def __init__(self, *args):
)

def _is_unit_in_non_active_status(self) -> Optional[StatusBase]: # noqa: C901
"""Check the possible conditions that Unit status is different than the Active.
Returns the Unit status that needs to be set if any condition is match, otherwise
returns None.
"""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
"""
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._amf_container.can_connect():
return MaintenanceStatus("Waiting for service to start")

Expand All @@ -169,6 +166,11 @@ def _is_unit_in_non_active_status(self) -> Optional[StatusBase]: # noqa: C901
if not _get_pod_ip():
return WaitingStatus("Waiting for pod IP address to be available")

try:
self._set_n2_information()
except ValueError:
return BlockedStatus("Waiting for MetalLB to be enabled")

return None

def _on_collect_unit_status(self, event: CollectStatusEvent):
Expand All @@ -177,9 +179,8 @@ def _on_collect_unit_status(self, event: CollectStatusEvent):
Args:
event: CollectStatusEvent
"""
if self._is_unit_in_non_active_status():
event.add_status(self._is_unit_in_non_active_status()) # type: ignore[arg-type]
return
if status := self._is_unit_in_non_active_status():
event.add_status(status)

def _on_install(self, event: InstallEvent) -> None:
client = Client()
Expand Down Expand Up @@ -235,19 +236,21 @@ def _configure_amf(self, event: EventBase) -> None: # noqa C901
"""
if self._is_unit_in_non_active_status():
# Unit Status is in Maintanence or Blocked or Waiting status
self.unit.status = self._is_unit_in_non_active_status() # type: ignore
event.defer()
return

if not self._certificate_is_stored():
self.unit.status = WaitingStatus("Waiting for certificates to be stored")
event.defer()
return

self._generate_config_file()
self._configure_amf_workload()
try:
self._set_n2_information()
except ValueError:
self.unit.status = BlockedStatus("Waiting for MetalLB to be enabled")
logger.info("Waiting for MetalLB to be enabled")
event.defer()
return
self.unit.status = ActiveStatus()

Expand Down
21 changes: 16 additions & 5 deletions tests/unit/test_charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def test_given_fiveg_nrf_relation_not_created_when_pebble_ready_then_status_is_b
self.harness.set_can_connect(container="amf", val=True)
self.harness.add_relation(relation_name="database", remote_app="mongodb")
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("Waiting for fiveg-nrf relation"),
Expand All @@ -73,6 +74,7 @@ def test_given_database_relation_not_created_when_pebble_ready_then_status_is_bl
self.harness.set_can_connect(container="amf", val=True)
self.harness.add_relation(relation_name="fiveg-nrf", remote_app="mongodb")
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("Waiting for database relation"),
Expand All @@ -85,6 +87,7 @@ def test_given_certificates_relation_not_created_when_pebble_ready_then_status_i
self.harness.add_relation(relation_name="fiveg-nrf", remote_app="mongodb")
self.harness.add_relation(relation_name="database", remote_app="mongodb")
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("Waiting for certificates relation"),
Expand All @@ -108,7 +111,7 @@ def test_given_amf_charm_in_active_state_when_nrf_relation_breaks_then_status_is
self.harness.container_pebble_ready("amf")

self.harness.remove_relation(nrf_relation_id)

self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("Waiting for fiveg-nrf relation"),
Expand Down Expand Up @@ -136,7 +139,7 @@ def test_given_amf_charm_in_active_state_when_database_relation_breaks_then_stat
self.harness.container_pebble_ready("amf")

self.harness.remove_relation(database_relation_id)

self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
BlockedStatus("Waiting for database relation"),
Expand All @@ -156,6 +159,7 @@ def test_given_relations_created_and_database_not_available_when_pebble_ready_th
relation_name="certificates", remote_app="tls-certificates-operator"
)
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
WaitingStatus("Waiting for the amf database to be available"),
Expand All @@ -177,6 +181,7 @@ def test_given_database_info_not_available_when_pebble_ready_then_status_is_wait
relation_name="certificates", remote_app="tls-certificates-operator"
)
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
WaitingStatus("Waiting for AMF database info to be available"),
Expand All @@ -200,6 +205,7 @@ def test_given_nrf_data_not_available_when_pebble_ready_then_status_is_waiting(
)
self._create_database_relation_and_populate_data()
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
WaitingStatus("Waiting for NRF data to be available"),
Expand All @@ -223,6 +229,7 @@ def test_given_storage_not_attached_when_pebble_ready_then_status_is_waiting(
)
self._create_database_relation_and_populate_data()
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
WaitingStatus("Waiting for storage to be attached"),
Expand Down Expand Up @@ -253,6 +260,7 @@ def test_given_certificates_not_stored_when_pebble_ready_then_status_is_waiting(
)
self._create_database_relation_and_populate_data()
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
WaitingStatus("Waiting for certificates to be stored"),
Expand Down Expand Up @@ -445,6 +453,7 @@ def test_relations_available_and_config_pushed_and_pebble_updated_when_pebble_re
)
self._create_database_relation_and_populate_data()
self.harness.container_pebble_ready("amf")
self.harness.evaluate_status()
self.assertEqual(
self.harness.model.unit.status,
ActiveStatus(),
Expand All @@ -468,7 +477,7 @@ def test_given_empty_ip_address_when_pebble_ready_then_status_is_waiting(
self._create_database_relation_and_populate_data()

self.harness.container_pebble_ready(container_name="amf")

self.harness.evaluate_status()
self.assertEqual(
self.harness.charm.unit.status,
WaitingStatus("Waiting for pod IP address to be available"),
Expand Down Expand Up @@ -661,6 +670,7 @@ def test_given_n2_information_and_service_is_running_and_metallb_service_is_not_
self.harness.container_pebble_ready("amf")
relation_id = self.harness.add_relation(relation_name="fiveg-n2", remote_app="n2-requirer")
self.harness.add_relation_unit(relation_id=relation_id, remote_unit_name="n2-requirer/0")
self.harness.evaluate_status()
self.assertEqual(
self.harness.charm.unit.status, BlockedStatus("Waiting for MetalLB to be enabled")
)
Expand Down Expand Up @@ -827,11 +837,12 @@ def test_given_certificates_are_stored_when_on_certificates_relation_broken_then
patch_check_output.return_value = b"1.1.1.1"
self.harness.set_can_connect(container="amf", val=True)
self.harness.add_relation(relation_name="fiveg-nrf", remote_app="mongodb")
self.harness.add_relation(
certificate_relation_id = self.harness.add_relation(
relation_name="certificates", remote_app="tls-certificates-operator"
)
self._create_database_relation_and_populate_data()
self.harness.charm._on_certificates_relation_broken(event=Mock())
self.harness.remove_relation(certificate_relation_id)
self.harness.evaluate_status()
self.assertEqual(
self.harness.charm.unit.status, BlockedStatus("Waiting for certificates relation")
)
Expand Down

0 comments on commit 53c6c12

Please sign in to comment.