diff --git a/operator/handlers.py b/operator/handlers.py index 6529433..177cdb8 100644 --- a/operator/handlers.py +++ b/operator/handlers.py @@ -386,6 +386,41 @@ def build_ingress_body( return ingress_body +def build_status( + *, + url: str, + image: str, + service_account: str, + memory_request: str, + memory_limit: str, + project_claim_name: str, + project_id: str, +) -> Dict[str, Any]: + """Build the value kopf writes to the custom resource's status. + + The ``create`` handler is registered with ``id="viz"``, so kopf nests this + return value under ``status.viz``. The CRD carries the annotation + ``data-manager.informaticsmatters.com/application-url-location: viz.url``, + and the Data Manager extracts the application URL from the ``status``- + relative path it names, i.e. ``custom_resource['status']['viz']['url']``. + + For those to agree, ``url`` MUST be a *top-level* key here (giving + ``status.viz.url``). Nesting it as ``{"viz": {"url": ...}}`` would put it at + ``status.viz.viz.url``, where the Data Manager never looks, so it would + never see the URL within its grace period. + """ + return { + "url": url, + "image": image, + "serviceAccountName": service_account, + "resources": { + "requests": {"memory": memory_request}, + "limits": {"memory": memory_limit}, + }, + "project": {"claimName": project_claim_name, "id": project_id}, + } + + def _create_ignoring_conflict(create_call: Any, description: str) -> None: """Create a Kubernetes object, tolerating a 409/Conflict. @@ -579,13 +614,16 @@ def create(spec: Dict[str, Any], name: str, namespace: str, **_: Any) -> Dict[st url = f"https://{ingress_domain}{ingress_path}" logging.info("Done %s (namespace=%s url=%s)", name, namespace, url) - return { - "viz": {"url": url}, - "image": image, - "serviceAccountName": service_account, - "resources": { - "requests": {"memory": memory_request}, - "limits": {"memory": memory_limit}, - }, - "project": {"claimName": project_claim_name, "id": project_id}, - } + # The returned dict is nested by kopf under 'status.viz' (the handler id). + # 'url' is a top-level key so the URL lands at 'status.viz.url', matching + # the CRD's 'application-url-location: viz.url' annotation that the Data + # Manager reads. See build_status for the full rationale. + return build_status( + url=url, + image=image, + service_account=service_account, + memory_request=memory_request, + memory_limit=memory_limit, + project_claim_name=project_claim_name, + project_id=project_id, + ) diff --git a/tests/test_handlers.py b/tests/test_handlers.py index b9ff2f3..6aa5c84 100644 --- a/tests/test_handlers.py +++ b/tests/test_handlers.py @@ -341,3 +341,59 @@ def test_default_ingress_class_is_overridden_by_svo_ingress_class( ) -> None: monkeypatch.setenv("SVO_INGRESS_CLASS", "traefik") assert handlers._get_default_ingress_class() == "traefik" + + +# --- status / application URL location -------------------------------------- + +# The CRD carries the annotation +# 'data-manager.informaticsmatters.com/application-url-location: viz.url'. +# The Data Manager reads the URL from the 'status'-relative path it names, i.e. +# custom_resource['status']['viz']['url']. The create handler has id='viz', so +# kopf nests its return value under 'status.viz'; therefore 'url' MUST be a +# top-level key of the returned dict for the two to line up. +APPLICATION_URL_LOCATION = "viz.url" + + +def _example_status(**overrides: Any) -> Dict[str, Any]: + kwargs: Dict[str, Any] = { + "url": "https://example.com/viz-abcdef", + "image": "ghcr.io/informaticsmatters/squonk2-viz-app:0.1.4", + "service_account": "default", + "memory_request": "256Mi", + "memory_limit": "1Gi", + "project_claim_name": "claim-1", + "project_id": "project-000", + } + kwargs.update(overrides) + return handlers.build_status(**kwargs) + + +def test_build_status_places_url_at_the_annotated_location() -> None: + # Reproduces the reporting bug: the URL must be reachable at the path named + # by the CRD's 'application-url-location' annotation once kopf has nested + # the handler's return value under 'status.' ('status.viz'). + handler_id = "viz" + status = {handler_id: _example_status(url="https://example.com/viz-abcdef")} + + cursor: Any = status + for key in APPLICATION_URL_LOCATION.split("."): + cursor = cursor[key] + assert cursor == "https://example.com/viz-abcdef" + + +def test_build_status_does_not_double_nest_under_viz() -> None: + # Guard against the regression where the URL was returned as + # {'viz': {'url': ...}}, landing at status.viz.viz.url and so invisible to + # the Data Manager (which only looks at status.viz.url). + status = _example_status() + assert "viz" not in status + assert status["url"] == "https://example.com/viz-abcdef" + + +def test_build_status_reports_instance_metadata() -> None: + status = _example_status() + assert status["image"] == "ghcr.io/informaticsmatters/squonk2-viz-app:0.1.4" + assert status["serviceAccountName"] == "default" + assert status["resources"]["requests"]["memory"] == "256Mi" + assert status["resources"]["limits"]["memory"] == "1Gi" + assert status["project"] == {"claimName": "claim-1", "id": "project-000"}