From e67f87dd80f09b91d717c78120e906e548be9073 Mon Sep 17 00:00:00 2001 From: ChristianZaccaria Date: Thu, 1 Feb 2024 18:21:20 +0000 Subject: [PATCH 1/2] Fix cluster.status() for Routes --- src/codeflare_sdk/cluster/cluster.py | 62 +++++++++++++++++++--------- tests/unit_test.py | 1 + 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/codeflare_sdk/cluster/cluster.py b/src/codeflare_sdk/cluster/cluster.py index 607e59f3..aff3ea6b 100644 --- a/src/codeflare_sdk/cluster/cluster.py +++ b/src/codeflare_sdk/cluster/cluster.py @@ -902,25 +902,47 @@ def _map_to_ray_cluster(rc) -> Optional[RayCluster]: status = RayClusterStatus(rc["status"]["state"].lower()) else: status = RayClusterStatus.UNKNOWN - try: - config_check() - api_instance = client.NetworkingV1Api(api_config_handler()) - ingresses = api_instance.list_namespaced_ingress(rc["metadata"]["namespace"]) - except Exception as e: # pragma no cover - return _kube_api_error_handling(e) - ray_ingress = None - for ingress in ingresses.items: - annotations = ingress.metadata.annotations - protocol = "http" - if ( - ingress.metadata.name == f"ray-dashboard-{rc['metadata']['name']}" - or ingress.metadata.name.startswith(f"{rc['metadata']['name']}-ingress") - ): - if annotations == None: - protocol = "http" - elif "route.openshift.io/termination" in annotations: - protocol = "https" - ray_ingress = f"{protocol}://{ingress.spec.rules[0].host}" + config_check() + dashboard_url = None + if is_openshift_cluster(): + try: + api_instance = client.CustomObjectsApi(api_config_handler()) + routes = api_instance.list_namespaced_custom_object( + group="route.openshift.io", + version="v1", + namespace=rc["metadata"]["namespace"], + plural="routes", + ) + except Exception as e: # pragma: no cover + return _kube_api_error_handling(e) + + for route in routes["items"]: + rc_name = rc["metadata"]["name"] + if route["metadata"]["name"] == f"ray-dashboard-{rc_name}" or route[ + "metadata" + ]["name"].startswith(f"{rc_name}-ingress"): + protocol = "https" if route["spec"].get("tls") else "http" + dashboard_url = f"{protocol}://{route['spec']['host']}" + else: + try: + api_instance = client.NetworkingV1Api(api_config_handler()) + ingresses = api_instance.list_namespaced_ingress( + rc["metadata"]["namespace"] + ) + except Exception as e: # pragma no cover + return _kube_api_error_handling(e) + for ingress in ingresses.items: + annotations = ingress.metadata.annotations + protocol = "http" + if ( + ingress.metadata.name == f"ray-dashboard-{rc['metadata']['name']}" + or ingress.metadata.name.startswith(f"{rc['metadata']['name']}-ingress") + ): + if annotations == None: + protocol = "http" + elif "route.openshift.io/termination" in annotations: + protocol = "https" + dashboard_url = f"{protocol}://{ingress.spec.rules[0].host}" return RayCluster( name=rc["metadata"]["name"], @@ -947,7 +969,7 @@ def _map_to_ray_cluster(rc) -> Optional[RayCluster]: head_gpu=rc["spec"]["headGroupSpec"]["template"]["spec"]["containers"][0][ "resources" ]["limits"]["nvidia.com/gpu"], - dashboard=ray_ingress, + dashboard=dashboard_url, ) diff --git a/tests/unit_test.py b/tests/unit_test.py index e0a3c385..035789cf 100644 --- a/tests/unit_test.py +++ b/tests/unit_test.py @@ -1903,6 +1903,7 @@ def route_retrieval(group, version, namespace, plural, name): def test_list_clusters(mocker, capsys): mocker.patch("kubernetes.config.load_kube_config", return_value="ignore") + mocker.patch("kubernetes.client.ApisApi.get_api_versions") mocker.patch( "kubernetes.client.CustomObjectsApi.list_namespaced_custom_object", side_effect=get_obj_none, From 5c2d2e1ea6f97595181b0348b11a0083405d217f Mon Sep 17 00:00:00 2001 From: ChristianZaccaria Date: Tue, 6 Feb 2024 14:45:15 +0000 Subject: [PATCH 2/2] Add unit tests for rc routes --- tests/unit_test.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/unit_test.py b/tests/unit_test.py index 035789cf..fc9ecde2 100644 --- a/tests/unit_test.py +++ b/tests/unit_test.py @@ -32,6 +32,7 @@ from codeflare_sdk.cluster.cluster import ( Cluster, ClusterConfiguration, + _map_to_ray_cluster, list_all_clusters, list_all_queued, _copy_to_ray, @@ -1901,6 +1902,53 @@ def route_retrieval(group, version, namespace, plural, name): } +def test_map_to_ray_cluster(mocker): + mocker.patch("kubernetes.config.load_kube_config") + + mocker.patch( + "codeflare_sdk.cluster.cluster.is_openshift_cluster", return_value=True + ) + + mock_api_client = mocker.MagicMock(spec=client.ApiClient) + mocker.patch( + "codeflare_sdk.cluster.auth.api_config_handler", return_value=mock_api_client + ) + + mock_routes = { + "items": [ + { + "apiVersion": "route.openshift.io/v1", + "kind": "Route", + "metadata": { + "name": "ray-dashboard-quicktest", + "namespace": "ns", + }, + "spec": {"host": "ray-dashboard-quicktest"}, + }, + ] + } + + def custom_side_effect(group, version, namespace, plural, **kwargs): + if plural == "routes": + return mock_routes + elif plural == "rayclusters": + return get_ray_obj("ray.io", "v1", "ns", "rayclusters") + + mocker.patch( + "kubernetes.client.CustomObjectsApi.list_namespaced_custom_object", + side_effect=custom_side_effect, + ) + + rc = get_ray_obj("ray.io", "v1", "ns", "rayclusters")["items"][0] + rc_name = rc["metadata"]["name"] + rc_dashboard = f"http://ray-dashboard-{rc_name}" + + result = _map_to_ray_cluster(rc) + + assert result is not None + assert result.dashboard == rc_dashboard + + def test_list_clusters(mocker, capsys): mocker.patch("kubernetes.config.load_kube_config", return_value="ignore") mocker.patch("kubernetes.client.ApisApi.get_api_versions")