Fix resume error (#160)

* catch apierror on stop tempo-ready * better comment * test * mp.setenv * mp.setenv * remove unused tmp_path
canonical · Aug 8, 2024 · a683f3a · a683f3a
1 parent f1a6048
commit a683f3a
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 11 deletions.
diff --git a/src/charm.py b/src/charm.py
@@ -34,6 +34,7 @@
 )
 from ops.main import main
 from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus
+from ops.pebble import APIError
 
 from tempo import Tempo
 
@@ -367,9 +368,21 @@ def _requested_receivers(self) -> Tuple[ReceiverProtocol, ...]:
 
     def _on_tempo_pebble_custom_notice(self, event: PebbleNoticeEvent):
         if event.notice.key == self.tempo.tempo_ready_notice_key:
+            # collect-unit-status should now report ready.
             logger.debug("pebble api reports ready")
-            # collect-unit-status should do the rest.
-            self.tempo.container.stop("tempo-ready")
+
+            try:
+                self.tempo.container.stop("tempo-ready")
+                # ops will fire APIError but ops.testing._TestingPebbleClient will fire RuntimeError.
+            except (APIError, RuntimeError):
+                # see https://matrix.to/#/!xzmWHtGpPfVCXKivIh:ubuntu.com/
+                #  $d42wOu61e5mqMhnDRUB6K8eV4kUAPQ_yhIQmqq5Q_cs?via=ubuntu.com&
+                #  via=matrix.org&via=matrix.debian.social
+                # issue: on sleep/resume, we get this event but there's no tempo-ready
+                # service in pebble (somehow?)
+                logger.debug(
+                    "`tempo-ready` service cannot be stopped at this time (probably doesn't exist)."
+                )
 
     def _on_tempo_pebble_ready(self, event: WorkloadEvent):
         if not self.tempo.container.can_connect():

diff --git a/tests/scenario/test_charm.py b/tests/scenario/test_charm.py
@@ -8,6 +8,7 @@
 from ops import pebble
 from scenario import Container, Mount, Relation, State
 from scenario.sequences import check_builtin_sequences
+from scenario.state import Notice, _BoundNotice
 
 from tempo import Tempo
 from tests.scenario.helpers import get_tempo_config
@@ -65,7 +66,7 @@ def test_tempo_restart_on_ingress_v2_changed(context, tmp_path, requested_protoc
     )
 
 
-def _tempo_mock_with_initial_config(tmp_path):
+def _tempo_mock_with_initial_config(tmp_path, tempo_ready_svc_exists: bool = True):
     tempo_config = tmp_path / "tempo.yaml"
     container = MagicMock()
     container.can_connect = lambda: True
@@ -75,19 +76,22 @@ def _tempo_mock_with_initial_config(tmp_path):
     )
     initial_config = Tempo(container).generate_config(["otlp_http"])
     tempo_config.write_text(yaml.safe_dump(initial_config))
+    layer_raw = {
+        "summary": "tempo layer",
+        "description": "foo",
+        "services": {
+            "tempo": {"startup": "enabled"},
+        },
+    }
+    if tempo_ready_svc_exists:
+        layer_raw["services"]["tempo-ready"] = {"startup": "disabled"}
+
     tempo = Container(
         "tempo",
         can_connect=True,
         layers={
             "tempo": pebble.Layer(
-                {
-                    "summary": "tempo layer",
-                    "description": "foo",
-                    "services": {
-                        "tempo": {"startup": "enabled"},
-                        "tempo-ready": {"startup": "disabled"},
-                    },
-                },
+                layer_raw,
             ),
         },
         service_status={
@@ -179,3 +183,22 @@ def test_tracing_storage_is_configured_to_s3_if_s3_relation_filled(
     new_config = get_tempo_config(tempo, context)
     expected_config = Tempo(container).generate_config(["otlp_http"], relation_data)
     assert new_config == expected_config
+
+
+def test_ready_check_on_resume(context, tmp_path, caplog, monkeypatch):
+    # GIVEN the charm has no tempo-ready service
+    container, tempo = _tempo_mock_with_initial_config(tmp_path, tempo_ready_svc_exists=False)
+
+    state = State(leader=True, containers=[tempo])
+
+    # WHEN we receive a custom-notice event
+    with caplog.at_level("DEBUG"):
+        monkeypatch.setenv("SCENARIO_SKIP_CONSISTENCY_CHECKS", "1")
+        # scenario doesn't play nice in this very edge case
+        context.run(_BoundNotice(Notice(Tempo.tempo_ready_notice_key), tempo).event, state)
+        monkeypatch.delenv("SCENARIO_SKIP_CONSISTENCY_CHECKS")
+
+    # THEN we get a debug-log but the charm doesn't error
+    assert "`tempo-ready` service cannot be stopped at this time (probably doesn't exist)." in {
+        r.message for r in caplog.records
+    }