From 153d2d18a3f701ed99cc8e3e5ab5cda1420d4507 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Mon, 9 Dec 2024 11:55:30 +0100 Subject: [PATCH] made the endpoints optional --- src/cosl/coordinated_workers/coordinator.py | 8 +- src/cosl/interfaces/datasource_exchange.py | 24 ++++-- .../test_coordinator.py | 4 +- .../test_coordinator_status.py | 4 +- tests/test_datasource_exchange.py | 82 +++++++++++++++---- 5 files changed, 91 insertions(+), 31 deletions(-) diff --git a/src/cosl/coordinated_workers/coordinator.py b/src/cosl/coordinated_workers/coordinator.py index 640309e..582f44d 100644 --- a/src/cosl/coordinated_workers/coordinator.py +++ b/src/cosl/coordinated_workers/coordinator.py @@ -162,8 +162,8 @@ def _validate_container_name( "metrics": str, "charm-tracing": str, "workload-tracing": str, - "provide-datasource-exchange": str, - "require-datasource-exchange": str, + "send-datasource": Optional[str], + "receive-datasource": Optional[str], "s3": str, }, total=True, @@ -282,8 +282,8 @@ def __init__( self.s3_requirer = S3Requirer(self._charm, self._endpoints["s3"]) self.datasource_exchange = DatasourceExchange( self._charm, - provider_endpoint=self._endpoints["provide-datasource-exchange"], - requirer_endpoint=self._endpoints["require-datasource-exchange"], + provider_endpoint=self._endpoints.get("send-datasource", None), + requirer_endpoint=self._endpoints.get("receive-datasource", None), ) self._grafana_dashboards = GrafanaDashboardProvider( diff --git a/src/cosl/interfaces/datasource_exchange.py b/src/cosl/interfaces/datasource_exchange.py index 87b1547..80dc9dd 100644 --- a/src/cosl/interfaces/datasource_exchange.py +++ b/src/cosl/interfaces/datasource_exchange.py @@ -43,8 +43,6 @@ log = logging.getLogger("datasource_exchange") -DEFAULT_PROVIDE_ENDPOINT_NAME = "provide-ds-exchange" -DEFAULT_REQUIRE_ENDPOINT_NAME = "require-ds-exchange" DS_EXCHANGE_INTERFACE_NAME = "grafana_datasource_exchange" @@ -57,18 +55,23 @@ class DatasourceDict(TypedDict): type: str uid: str + grafana_uid: str class EndpointValidationError(ValueError): """Raised if an endpoint name is invalid.""" -def _validate_endpoints(charm: CharmBase, provider_endpoint: str, requirer_endpoint: str): +def _validate_endpoints( + charm: CharmBase, provider_endpoint: Optional[str], requirer_endpoint: Optional[str] +): meta = charm.meta for endpoint, source in ( (provider_endpoint, meta.provides), (requirer_endpoint, meta.requires), ): + if endpoint is None: + continue if endpoint not in source: raise EndpointValidationError(f"endpoint {endpoint!r} not declared in charm metadata") interface_name = source[endpoint].interface_name @@ -77,6 +80,11 @@ def _validate_endpoints(charm: CharmBase, provider_endpoint: str, requirer_endpo f"endpoint {endpoint} has unexpected interface {interface_name!r} " f"(should be {DS_EXCHANGE_INTERFACE_NAME})." ) + if not provider_endpoint and not requirer_endpoint: + raise EndpointValidationError( + "This charm should implement either a requirer or a provider (or both)" + "endpoint for `grafana-datasource-exchange`." + ) class DatasourceExchange: @@ -86,18 +94,16 @@ def __init__( self, charm: ops.CharmBase, *, - provider_endpoint: Optional[str] = None, - requirer_endpoint: Optional[str] = None, + provider_endpoint: Optional[str], + requirer_endpoint: Optional[str], ): self._charm = charm - provider_endpoint = provider_endpoint or DEFAULT_PROVIDE_ENDPOINT_NAME - requirer_endpoint = requirer_endpoint or DEFAULT_REQUIRE_ENDPOINT_NAME - _validate_endpoints(charm, provider_endpoint, requirer_endpoint) # gather all relations, provider or requirer all_relations = chain( - charm.model.relations[provider_endpoint], charm.model.relations[requirer_endpoint] + charm.model.relations.get(provider_endpoint, ()), + charm.model.relations.get(requirer_endpoint, ()), ) # filter out some common unhappy relation states diff --git a/tests/test_coordinated_workers/test_coordinator.py b/tests/test_coordinated_workers/test_coordinator.py index cd44789..b241c3c 100644 --- a/tests/test_coordinated_workers/test_coordinator.py +++ b/tests/test_coordinated_workers/test_coordinator.py @@ -123,8 +123,8 @@ def __init__(self, framework: ops.Framework): "charm-tracing": "my-charm-tracing", "workload-tracing": "my-workload-tracing", "s3": "my-s3", - "provide-datasource-exchange": "my-ds-exchange-provide", - "require-datasource-exchange": "my-ds-exchange-require", + "send-datasource": "my-ds-exchange-provide", + "receive-datasource": "my-ds-exchange-require", }, nginx_config=lambda coordinator: f"nginx configuration for {coordinator._charm.meta.name}", workers_config=lambda coordinator: f"workers configuration for {coordinator._charm.meta.name}", diff --git a/tests/test_coordinated_workers/test_coordinator_status.py b/tests/test_coordinated_workers/test_coordinator_status.py index 56ad0eb..300dafa 100644 --- a/tests/test_coordinated_workers/test_coordinator_status.py +++ b/tests/test_coordinated_workers/test_coordinator_status.py @@ -38,8 +38,8 @@ def __init__(self, framework: ops.Framework): "metrics": "metrics-endpoint", "charm-tracing": "self-charm-tracing", "workload-tracing": "self-workload-tracing", - "provide-datasource-exchange": "my-ds-exchange-provide", - "require-datasource-exchange": "my-ds-exchange-require", + "send-datasource": None, + "receive-datasource": "my-ds-exchange-require", }, nginx_config=lambda _: "nginx config", workers_config=lambda _: "worker config", diff --git a/tests/test_datasource_exchange.py b/tests/test_datasource_exchange.py index 7f6e223..45cc135 100644 --- a/tests/test_datasource_exchange.py +++ b/tests/test_datasource_exchange.py @@ -6,40 +6,95 @@ from scenario import Context, Relation, State from scenario.errors import UncaughtCharmError -from cosl.interfaces.datasource_exchange import DatasourceExchange, DSExchangeAppData +from cosl.interfaces.datasource_exchange import ( + DatasourceExchange, + DSExchangeAppData, + EndpointValidationError, +) @pytest.mark.parametrize( - "meta, invalid_reason", + "meta, declared", ( + ( + {}, + (None, None), + ), ( { "requires": {"boo": {"interface": "gibberish"}}, "provides": {"far": {"interface": "grafana_datasource_exchange"}}, }, - "unexpected interface 'gibberish'", + ("far", "boo"), ), ( { "requires": {"boo": {"interface": "grafana_datasource_exchange"}}, "provides": {"goo": {"interface": "grafana_datasource_exchange"}}, }, - "endpoint 'far' not declared", + ("far", "boo"), ), ), ) -def test_endpoint_validation(meta, invalid_reason): +def test_endpoint_validation_failure(meta, declared): + # GIVEN a charm with this metadata and declared provider/requirer endpoints + class BadCharm(CharmBase): def __init__(self, framework: Framework): super().__init__(framework) + prov, req = declared self.ds_exchange = DatasourceExchange( - self, provider_endpoint="far", requirer_endpoint="boo" + self, provider_endpoint=prov, requirer_endpoint=req ) - with pytest.raises(UncaughtCharmError, match=invalid_reason): + # WHEN any event is processed + with pytest.raises(UncaughtCharmError) as e: ctx = Context(BadCharm, meta={"name": "bob", **meta}) ctx.run(ctx.on.update_status(), State()) + # THEN we raise an EndpointValidationError + assert isinstance(e.value.__cause__, EndpointValidationError) + + +@pytest.mark.parametrize( + "meta, declared", + ( + ( + { + "requires": {"boo": {"interface": "grafana_datasource_exchange"}}, + "provides": {"far": {"interface": "grafana_datasource_exchange"}}, + }, + ("far", "boo"), + ), + ( + { + "provides": {"far": {"interface": "grafana_datasource_exchange"}}, + }, + ("far", None), + ), + ( + { + "requires": {"boo": {"interface": "grafana_datasource_exchange"}}, + }, + (None, "boo"), + ), + ), +) +def test_endpoint_validation_ok(meta, declared): + # GIVEN a charm with this metadata and declared provider/requirer endpoints + class BadCharm(CharmBase): + def __init__(self, framework: Framework): + super().__init__(framework) + prov, req = declared + self.ds_exchange = DatasourceExchange( + self, provider_endpoint=prov, requirer_endpoint=req + ) + + # WHEN any event is processed + ctx = Context(BadCharm, meta={"name": "bob", **meta}) + ctx.run(ctx.on.update_status(), State()) + # THEN no exception is raised + def test_ds_submit(): # GIVEN a charm with a single datasource_exchange relation @@ -47,15 +102,14 @@ class MyCharm(CharmBase): META = { "name": "robbie", "provides": {"foo": {"interface": "grafana_datasource_exchange"}}, - "requires": {"bar": {"interface": "grafana_datasource_exchange"}}, } def __init__(self, framework: Framework): super().__init__(framework) self.ds_exchange = DatasourceExchange( - self, provider_endpoint="foo", requirer_endpoint="bar" + self, provider_endpoint="foo", requirer_endpoint=None ) - self.ds_exchange.publish([{"type": "tempo", "uid": "123"}]) + self.ds_exchange.publish([{"type": "tempo", "uid": "123", "grafana_uid": "123123"}]) ctx = Context(MyCharm, meta=MyCharm.META) @@ -91,11 +145,11 @@ def __init__(self, framework: Framework): ctx = Context(MyCharm, meta=MyCharm.META) ds_requirer_in = [ - {"type": "c", "uid": "3"}, - {"type": "a", "uid": "1"}, - {"type": "b", "uid": "2"}, + {"type": "c", "uid": "3", "grafana_uid": "4"}, + {"type": "a", "uid": "1", "grafana_uid": "5"}, + {"type": "b", "uid": "2", "grafana_uid": "6"}, ] - ds_provider_in = [{"type": "d", "uid": "4"}] + ds_provider_in = [{"type": "d", "uid": "4", "grafana_uid": "7"}] dse_requirer_in = Relation( "foo",