Skip to content

Commit

Permalink
Fixed get_cluster for ingress_options
Browse files Browse the repository at this point in the history
  • Loading branch information
Bobbins228 authored and openshift-merge-bot[bot] committed Jan 10, 2024
1 parent bece39c commit 0feab0f
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 26 deletions.
93 changes: 78 additions & 15 deletions src/codeflare_sdk/cluster/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ def torchx_config(
to_return["requirements"] = requirements
return to_return

def from_k8_cluster_object(rc, mcad=True, ingress_domain=None):
def from_k8_cluster_object(rc, mcad=True, ingress_domain=None, ingress_options={}):
machine_types = (
rc["metadata"]["labels"]["orderedinstance"].split("_")
if "orderedinstance" in rc["metadata"]["labels"]
Expand All @@ -502,6 +502,10 @@ def from_k8_cluster_object(rc, mcad=True, ingress_domain=None):
"volumeMounts"
in rc["spec"]["workerGroupSpecs"][0]["template"]["spec"]["containers"][0]
)
if local_interactive:
ingress_domain = get_ingress_domain_from_client(
rc["metadata"]["name"], rc["metadata"]["namespace"]
)
cluster_config = ClusterConfiguration(
name=rc["metadata"]["name"],
namespace=rc["metadata"]["namespace"],
Expand Down Expand Up @@ -533,6 +537,7 @@ def from_k8_cluster_object(rc, mcad=True, ingress_domain=None):
local_interactive=local_interactive,
mcad=mcad,
ingress_domain=ingress_domain,
ingress_options=ingress_options,
)
return Cluster(cluster_config)

Expand Down Expand Up @@ -692,27 +697,55 @@ def get_cluster(cluster_name: str, namespace: str = "default"):
api_instance = client.NetworkingV1Api(api_config_handler())
ingresses = api_instance.list_namespaced_ingress(namespace)
ingress_host = None
if mcad == True:
for ingress in ingresses.items:
# Search for ingress with AppWrapper name as the owner
if cluster_name == ingress.metadata.owner_references[0].name:
ingress_host = ingress.spec.rules[0].host
else:
for ingress in ingresses.items:
# Search for the ingress with the ingress-owner label
if ingress.metadata.labels["ingress-owner"] == cluster_name:
ingress_host = ingress.spec.rules[0].host
ingress_options = {}
for ingress in ingresses.items:
# Search for ingress with AppWrapper name as the owner
if (
"ingress-owner" in ingress.metadata.labels
and ingress.metadata.labels["ingress-owner"] == cluster_name
):
ingress_host = ingress.spec.rules[0].host
if (
"ingress-options" in ingress.metadata.labels
and ingress.metadata.labels["ingress-options"] == "true"
):
ingress_name = ingress.metadata.name
port = (
ingress.spec.rules[0]
.http.paths[0]
.backend.service.port.number
)
annotations = ingress.metadata.annotations
path = ingress.spec.rules[0].http.paths[0].path
ingress_class_name = ingress.spec.ingress_class_name
path_type = ingress.spec.rules[0].http.paths[0].path_type

ingress_options = {
"ingresses": [
{
"ingressName": ingress_name,
"port": port,
"annotations": annotations,
"ingressClassName": ingress_class_name,
"pathType": path_type,
"path": path,
"host": ingress_host,
}
]
}
except Exception as e:
return _kube_api_error_handling(e)

# We gather the ingress domain from the host
if ingress_host is not None:
if ingress_host is not None and ingress_options == {}:
ingress_domain = ingress_host.split(".", 1)[1]
else:
ingress_domain = None

return Cluster.from_k8_cluster_object(
rc, mcad=mcad, ingress_domain=ingress_domain
rc,
mcad=mcad,
ingress_domain=ingress_domain,
ingress_options=ingress_options,
)
raise FileNotFoundError(
f"Cluster {cluster_name} is not found in {namespace} namespace"
Expand Down Expand Up @@ -762,7 +795,10 @@ def _get_ingress_domain(self): # pragma: no cover
return _kube_api_error_handling(e)

for route in routes["items"]:
if route["spec"]["port"]["targetPort"] == "client":
if (
route["spec"]["port"]["targetPort"] == "client"
or route["spec"]["port"]["targetPort"] == 10001
):
domain = route["spec"]["host"]
else:
try:
Expand Down Expand Up @@ -949,3 +985,30 @@ def _copy_to_ray(cluster: Cluster) -> RayCluster:
if ray.status == CodeFlareClusterStatus.READY:
ray.status = RayClusterStatus.READY
return ray


def get_ingress_domain_from_client(cluster_name: str, namespace: str = "default"):
if is_openshift_cluster():
try:
config_check()
api_instance = client.CustomObjectsApi(api_config_handler())
route = api_instance.get_namespaced_custom_object(
group="route.openshift.io",
version="v1",
namespace=namespace,
plural="routes",
name=f"rayclient-{cluster_name}",
)
return route["spec"]["host"].split(".", 1)[1]
except Exception as e: # pragma no cover
return _kube_api_error_handling(e)
else:
try:
config_check()
api_instance = client.NetworkingV1Api(api_config_handler())
ingress = api_instance.read_namespaced_ingress(
f"rayclient-{cluster_name}", namespace
)
return ingress.spec.rules[0].host.split(".", 1)[1]
except Exception as e: # pragma no cover
return _kube_api_error_handling(e)
1 change: 1 addition & 0 deletions src/codeflare_sdk/templates/base-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ spec:
annotations:
annotations-example:annotations-example
labels:
ingress-options: "false"
ingress-owner: appwrapper-name
spec:
ingressClassName: nginx
Expand Down
26 changes: 21 additions & 5 deletions src/codeflare_sdk/utils/generate_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,25 +133,41 @@ def update_dashboard_ingress(
metadata["name"] = ingress_option["ingressName"]
metadata["namespace"] = namespace
metadata["labels"]["ingress-owner"] = cluster_name
if "annotations" not in ingress_option.keys():
metadata["labels"]["ingress-options"] = "true"
if (
"annotations" not in ingress_option.keys()
or ingress_option["annotations"] is None
):
del metadata["annotations"]
else:
metadata["annotations"] = ingress_option["annotations"]
if "path" not in ingress_option.keys():
if (
"path" not in ingress_option.keys()
or ingress_option["path"] is None
):
del spec["rules"][0]["http"]["paths"][0]["path"]
else:
spec["rules"][0]["http"]["paths"][0]["path"] = ingress_option[
"path"
]
if "pathType" not in ingress_option.keys():
if (
"pathType" not in ingress_option.keys()
or ingress_option["pathType"] is None
):
spec["rules"][0]["http"]["paths"][0][
"pathType"
] = "ImplementationSpecific"
if "host" not in ingress_option.keys():
if (
"host" not in ingress_option.keys()
or ingress_option["host"] is None
):
del spec["rules"][0]["host"]
else:
spec["rules"][0]["host"] = ingress_option["host"]
if "ingressClassName" not in ingress_option.keys():
if (
"ingressClassName" not in ingress_option.keys()
or ingress_option["ingressClassName"] is None
):
del spec["ingressClassName"]
else:
spec["ingressClassName"] = ingress_option["ingressClassName"]
Expand Down
1 change: 1 addition & 0 deletions tests/test-case-no-mcad.yamls
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
labels:
ingress-options: 'false'
ingress-owner: unit-test-cluster-ray
name: ray-dashboard-unit-test-cluster-ray
namespace: ns
Expand Down
1 change: 1 addition & 0 deletions tests/test-case-prio.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ spec:
kind: Ingress
metadata:
labels:
ingress-options: 'false'
ingress-owner: prio-test-cluster
name: ray-dashboard-prio-test-cluster
namespace: ns
Expand Down
1 change: 1 addition & 0 deletions tests/test-case.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ spec:
kind: Ingress
metadata:
labels:
ingress-options: 'false'
ingress-owner: unit-test-cluster
name: ray-dashboard-unit-test-cluster
namespace: ns
Expand Down
60 changes: 54 additions & 6 deletions tests/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
_app_wrapper_status,
_ray_cluster_status,
_get_ingress_domain,
get_ingress_domain_from_client,
)
from codeflare_sdk.cluster.auth import (
TokenAuthentication,
Expand Down Expand Up @@ -616,25 +617,27 @@ def ray_addr(self, *args):
return self._address


def ingress_retrieval(port, annotations=None):
def ingress_retrieval(port, annotations=None, cluster_name="unit-test-cluster"):
labels = {"ingress-owner": cluster_name, "ingress-options": "false"}
if port == 10001:
serviceName = "client"
else:
serviceName = "dashboard"
mock_ingress = client.V1Ingress(
metadata=client.V1ObjectMeta(
name=f"ray-{serviceName}-unit-test-cluster",
name=f"ray-{serviceName}-{cluster_name}",
annotations=annotations,
labels=labels,
owner_references=[
client.V1OwnerReference(
api_version="v1", kind="Ingress", name="quicktest", uid="unique-id"
api_version="v1", kind="Ingress", name=cluster_name, uid="unique-id"
)
],
),
spec=client.V1IngressSpec(
rules=[
client.V1IngressRule(
host=f"ray-{serviceName}-unit-test-cluster-ns.apps.cluster.awsroute.org",
host=f"ray-{serviceName}-{cluster_name}-ns.apps.cluster.awsroute.org",
http=client.V1HTTPIngressRuleValue(
paths=[
client.V1HTTPIngressPath(
Expand Down Expand Up @@ -1417,7 +1420,10 @@ def get_aw_obj(group, version, namespace, plural):
"apiVersion": "networking.k8s.io/v1",
"kind": "Ingress",
"metadata": {
"labels": {"ingress-owner": "appwrapper-name"},
"labels": {
"ingress-owner": "appwrapper-name",
"ingress-options": "false",
},
"name": "ray-dashboard-quicktest",
"namespace": "default",
},
Expand Down Expand Up @@ -1817,7 +1823,7 @@ def test_get_cluster(mocker):
)
mocker.patch(
"kubernetes.client.NetworkingV1Api.list_namespaced_ingress",
return_value=ingress_retrieval(port=8265),
return_value=ingress_retrieval(port=8265, cluster_name="quicktest"),
)
cluster = get_cluster("quicktest")
cluster_config = cluster.config
Expand All @@ -1837,6 +1843,48 @@ def test_get_cluster(mocker):
assert cluster_config.num_workers == 1


def test_get_ingress_domain_from_client(mocker):
mocker.patch("kubernetes.config.load_kube_config")
mocker.patch("kubernetes.client.ApisApi.get_api_versions")
mocker.patch(
"kubernetes.client.NetworkingV1Api.read_namespaced_ingress",
return_value=ingress_retrieval(
port=8265, cluster_name="unit-test-cluster"
).items[0],
)

ingress_domain = get_ingress_domain_from_client("unit-test-cluster", "ns")
assert ingress_domain == "apps.cluster.awsroute.org"

mocker.patch(
"codeflare_sdk.utils.generate_yaml.is_openshift_cluster", return_value=True
)
mocker.patch(
"kubernetes.client.CustomObjectsApi.get_namespaced_custom_object",
side_effect=route_retrieval,
)
ingress_domain = get_ingress_domain_from_client("unit-test-cluster", "ns")
assert ingress_domain == "apps.cluster.awsroute.org"


def route_retrieval(group, version, namespace, plural, name):
assert group == "route.openshift.io"
assert version == "v1"
assert namespace == "ns"
assert plural == "routes"
assert name == "ray-dashboard-unit-test-cluster"
return {
"items": [
{
"metadata": {"name": "ray-dashboard-unit-test-cluster"},
"spec": {
"host": "ray-dashboard-unit-test-cluster-ns.apps.cluster.awsroute.org"
},
}
]
}


def test_list_clusters(mocker, capsys):
mocker.patch("kubernetes.config.load_kube_config", return_value="ignore")
mocker.patch(
Expand Down

0 comments on commit 0feab0f

Please sign in to comment.