refactor: use main event handler and defer it if Knative CRDs are not…

… found (#276)

* refactor: use main event handler and defer it if Knative CRDs are not found

The Knative Serving and Eventing charms have a strong dependency on knative-operator,
as it is the application that applies the KnativeServing and KnativeEventing CRDs and
has the business logic to reconcile such CRs. Because the Knative Serving and Eventing
charms depend entirely on the existence of those CRDs, this commit refactors the charms
logic to:
1. Use a main event handler that can be deferred in case the required CRDs are not present
2. Remove any operation for the on_install event, and use the main event handler for config_changed
as it is guaranteed that it'll be triggered right after on_install

Fixes #156

charms/knative-eventing/src/charm.py

@@ -12,13 +12,15 @@
 from pathlib import Path
 
 import yaml
-from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
+from charmed_kubeflow_chisme.exceptions import ErrorWithStatus, GenericCharmRuntimeError
 from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler as KRH  # noqa N813
 from charmed_kubeflow_chisme.lightkube.batch import delete_many
+from lightkube import Client
 from lightkube.core.exceptions import ApiError
+from lightkube.resources.apiextensions_v1 import CustomResourceDefinition
 from ops.charm import CharmBase
 from ops.main import main
-from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
+from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus
 
 from image_management import parse_image_config, remove_empty_images, update_images
 from lightkube_custom_resources.operator import KnativeEventing_v1beta1  # noqa F401
@@ -40,8 +42,7 @@ def __init__(self, *args):
         self._namespace = self.model.name
         self._resource_handler = None
 
-        self.framework.observe(self.on.install, self._on_install)
-        self.framework.observe(self.on.config_changed, self._on_config_changed)
+        self.framework.observe(self.on.config_changed, self._main)
         self.framework.observe(
             self.on["otel-collector"].relation_changed, self._on_otel_collector_relation_changed
         )
@@ -84,14 +85,25 @@ def _get_custom_images(self):
             )
         return custom_images
 
-    def _on_install(self, _):
+    def _main(self, event):
         if not self.model.config["namespace"]:
             self.model.unit.status = BlockedStatus("Config item `namespace` must be set")
             return
-        self._apply_and_set_status()
-
-    def _on_config_changed(self, _):
-        self._apply_and_set_status()
+        # Check the KnativeServing CRD is present; otherwise defer
+        lightkube_client = Client()
+        try:
+            lightkube_client.get(CustomResourceDefinition, "knativeservings.operator.knative.dev")
+            self._apply_and_set_status()
+        except ApiError as e:
+            if e.status.code == 404:
+                self.model.unit.status = WaitingStatus(
+                    "Waiting for knative-operator CRDs to be present."
+                )
+                event.defer()
+            else:
+                raise GenericCharmRuntimeError(
+                    f"Lightkube get CRD failed with error code: {e.status.code}"
+                ) from e
 
     def _on_otel_collector_relation_changed(self, _):
         """Event handler for on['otel-collector'].relation_changed."""

charms/knative-eventing/tests/unit/test_charm.py

@@ -5,10 +5,10 @@
 
 import pytest
 import yaml
-from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
+from charmed_kubeflow_chisme.exceptions import ErrorWithStatus, GenericCharmRuntimeError
 from charmed_kubeflow_chisme.lightkube.mocking import FakeApiError
 from lightkube import ApiError
-from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
+from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus
 
 from charm import CUSTOM_IMAGE_CONFIG_NAME, DEFAULT_IMAGES
 
@@ -42,30 +42,58 @@ def __init__(self, code=400):
 def test_events(harness, mocked_lightkube_client):
     # Test install and config_changed event handlers are called
     harness.begin()
-    harness.charm._on_install = MagicMock()
-    harness.charm._on_config_changed = MagicMock()
+    harness.charm._main = MagicMock()
     harness.charm._on_otel_collector_relation_changed = MagicMock()
 
-    harness.charm.on.install.emit()
-    harness.charm._on_install.assert_called_once()
-
     harness.charm.on.config_changed.emit()
-    harness.charm._on_config_changed.assert_called_once()
+    harness.charm._main.assert_called_once()
 
     rel_id = harness.add_relation("otel-collector", "app")
     harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
     harness.charm._on_otel_collector_relation_changed.assert_called_once()
 
 
-def test_on_install_active(harness, mocked_lightkube_client):
-    harness.begin()
+@patch("charm.Client")
+def test_active(lk_client, harness, mocked_lightkube_client):
+    harness.begin_with_initial_hooks()
     harness.update_config({"namespace": "test"})
+    rel_id = harness.add_relation("otel-collector", "app")
+    harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
     harness.charm.resource_handler.apply = MagicMock()
     harness.charm.resource_handler.apply.return_value = None
-    harness.charm.on.install.emit()
     assert harness.model.unit.status == ActiveStatus()
 
 
+@patch("charm.Client")
+def test_missing_knative_eventing_crd(lk_client, harness, mocker, mocked_lightkube_client):
+    harness.begin()
+
+    # Set the side effect of Client to a FakeApiError
+    lk_client.return_value.get.side_effect = _FakeApiError(code=404)
+
+    # Set the relation to otel-collector
+    rel_id = harness.add_relation("otel-collector", "app")
+    harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
+
+    harness.update_config({"namespace": "test"})
+    assert isinstance(harness.model.unit.status, WaitingStatus)
+
+
+@patch("charm.Client")
+def test_error_getting_knative_eventing_crd(lk_client, harness, mocker, mocked_lightkube_client):
+    harness.begin()
+
+    # Set the side effect of Client to a FakeApiError
+    lk_client.return_value.get.side_effect = _FakeApiError(code=403)
+
+    # Set the relation to otel-collector
+    rel_id = harness.add_relation("otel-collector", "app")
+    harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
+
+    with pytest.raises(GenericCharmRuntimeError):
+        harness.update_config({"namespace": "test"})
+
+
 @pytest.mark.parametrize(
     "apply_error, raised_exception",
     (

charms/knative-serving/src/charm.py

@@ -12,14 +12,16 @@
 from pathlib import Path
 
 import yaml
-from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
+from charmed_kubeflow_chisme.exceptions import ErrorWithStatus, GenericCharmRuntimeError
 from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler as KRH  # noqa N813
 from charmed_kubeflow_chisme.lightkube.batch import delete_many
 from charms.istio_pilot.v0.istio_gateway_info import GatewayProvider
+from lightkube import Client
 from lightkube.core.exceptions import ApiError
+from lightkube.resources.apiextensions_v1 import CustomResourceDefinition
 from ops.charm import CharmBase
 from ops.main import main
-from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
+from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus
 
 from image_management import parse_image_config, remove_empty_images, update_images
 from lightkube_custom_resources.operator import KnativeServing_v1beta1  # noqa F401
@@ -45,8 +47,7 @@ def __init__(self, *args):
         self._ingress_gateway_provider = GatewayProvider(self, relation_name="ingress-gateway")
         self._local_gateway_provider = GatewayProvider(self, relation_name="local-gateway")
 
-        self.framework.observe(self.on.install, self._on_install)
-        self.framework.observe(self.on.config_changed, self._on_config_changed)
+        self.framework.observe(self.on.config_changed, self._main)
         self.framework.observe(
             self.on["ingress-gateway"].relation_changed, self._on_ingress_gateway_relation_changed
         )
@@ -95,16 +96,29 @@ def _get_custom_images(self):
             )
         return custom_images
 
-    def _on_install(self, _):
+    def _main(self, event):
         if not self.model.config["namespace"]:
             self.model.unit.status = BlockedStatus("Config item `namespace` must be set")
             return
-        self._apply_and_set_status()
 
-    def _on_config_changed(self, _):
         self._send_ingress_gateway_data()
         self._send_local_gateway_data()
-        self._apply_and_set_status()
+
+        # Check the KnativeServing CRD is present; otherwise defer
+        lightkube_client = Client()
+        try:
+            lightkube_client.get(CustomResourceDefinition, "knativeservings.operator.knative.dev")
+            self._apply_and_set_status()
+        except ApiError as e:
+            if e.status.code == 404:
+                self.model.unit.status = WaitingStatus(
+                    "Waiting for knative-operator CRDs to be present."
+                )
+                event.defer()
+            else:
+                raise GenericCharmRuntimeError(
+                    f"Lightkube get CRD failed with error code: {e.status.code}"
+                ) from e
 
     def _on_ingress_gateway_relation_changed(self, _) -> None:
         self._send_ingress_gateway_data()

charms/knative-serving/tests/unit/test_charm.py

@@ -5,10 +5,10 @@
 
 import pytest
 import yaml
-from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
+from charmed_kubeflow_chisme.exceptions import ErrorWithStatus, GenericCharmRuntimeError
 from charmed_kubeflow_chisme.lightkube.mocking import FakeApiError
 from lightkube import ApiError
-from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
+from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus
 
 from charm import CUSTOM_IMAGE_CONFIG_NAME, DEFAULT_IMAGES
 
@@ -42,30 +42,58 @@ def __init__(self, code=400):
 def test_events(harness, mocked_lightkube_client):
     # Test install and config_changed event handlers are called
     harness.begin()
-    harness.charm._on_install = MagicMock()
-    harness.charm._on_config_changed = MagicMock()
+    harness.charm._main = MagicMock()
     harness.charm._on_otel_collector_relation_changed = MagicMock()
 
-    harness.charm.on.install.emit()
-    harness.charm._on_install.assert_called_once()
-
     harness.charm.on.config_changed.emit()
-    harness.charm._on_config_changed.assert_called_once()
+    harness.charm._main.assert_called_once()
 
     rel_id = harness.add_relation("otel-collector", "app")
     harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
     harness.charm._on_otel_collector_relation_changed.assert_called_once()
 
 
-def test_on_install_active(harness, mocked_lightkube_client):
-    harness.begin()
+@patch("charm.Client")
+def test_active(lk_client, harness, mocked_lightkube_client):
+    harness.begin_with_initial_hooks()
     harness.update_config({"namespace": "test"})
+    rel_id = harness.add_relation("otel-collector", "app")
+    harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
     harness.charm.resource_handler.apply = MagicMock()
     harness.charm.resource_handler.apply.return_value = None
-    harness.charm.on.install.emit()
     assert harness.model.unit.status == ActiveStatus()
 
 
+@patch("charm.Client")
+def test_missing_knative_serving_crd(lk_client, harness, mocker, mocked_lightkube_client):
+    harness.begin()
+
+    # Set the side effect of Client to a FakeApiError
+    lk_client.return_value.get.side_effect = _FakeApiError(code=404)
+
+    # Set the relation to otel-collector
+    rel_id = harness.add_relation("otel-collector", "app")
+    harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
+
+    harness.update_config({"namespace": "test"})
+    assert isinstance(harness.model.unit.status, WaitingStatus)
+
+
+@patch("charm.Client")
+def test_error_getting_knative_serving_crd(lk_client, harness, mocker, mocked_lightkube_client):
+    harness.begin()
+
+    # Set the side effect of Client to a FakeApiError
+    lk_client.return_value.get.side_effect = _FakeApiError(code=403)
+
+    # Set the relation to otel-collector
+    rel_id = harness.add_relation("otel-collector", "app")
+    harness.update_relation_data(rel_id, "app", {"some-key": "some-value"})
+
+    with pytest.raises(GenericCharmRuntimeError):
+        harness.update_config({"namespace": "test"})
+
+
 @pytest.mark.parametrize(
     "apply_error, raised_exception",
     (
@@ -120,6 +148,7 @@ def test_gateway_relation_data(
     harness.begin()
 
     # Update config values with test values
+    harness.charm._main = MagicMock()
     harness.update_config(charm_config)
 
     # Add one relation, send data, and assert the data is correct