Skip to content

Commit d9de5a3

Browse files
Bobbins228astefanutti
authored andcommitted
Fixed creating ingresses without admin access
1 parent 9c51eb7 commit d9de5a3

File tree

8 files changed

+90
-121
lines changed

8 files changed

+90
-121
lines changed

src/codeflare_sdk/cluster/cluster.py

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ def create_app_wrapper(self):
189189
local_interactive = self.config.local_interactive
190190
image_pull_secrets = self.config.image_pull_secrets
191191
dispatch_priority = self.config.dispatch_priority
192-
ingress_domain = self.config.ingress_domain
192+
domain_name = self.config.domain_name
193193
ingress_options = self.config.ingress_options
194194
return generate_appwrapper(
195195
name=name,
@@ -214,7 +214,7 @@ def create_app_wrapper(self):
214214
dispatch_priority=dispatch_priority,
215215
priority_val=priority_val,
216216
openshift_oauth=self.config.openshift_oauth,
217-
ingress_domain=ingress_domain,
217+
domain_name=domain_name,
218218
ingress_options=ingress_options,
219219
)
220220

@@ -468,7 +468,7 @@ def torchx_config(
468468
to_return["requirements"] = requirements
469469
return to_return
470470

471-
def from_k8_cluster_object(rc, mcad=True):
471+
def from_k8_cluster_object(rc, mcad=True, domain_name=None):
472472
machine_types = (
473473
rc["metadata"]["labels"]["orderedinstance"].split("_")
474474
if "orderedinstance" in rc["metadata"]["labels"]
@@ -508,6 +508,7 @@ def from_k8_cluster_object(rc, mcad=True):
508508
]["image"],
509509
local_interactive=local_interactive,
510510
mcad=mcad,
511+
domain_name=domain_name,
511512
)
512513
return Cluster(cluster_config)
513514

@@ -644,7 +645,10 @@ def get_cluster(cluster_name: str, namespace: str = "default"):
644645
for rc in rcs["items"]:
645646
if rc["metadata"]["name"] == cluster_name:
646647
mcad = _check_aw_exists(cluster_name, namespace)
647-
return Cluster.from_k8_cluster_object(rc, mcad=mcad)
648+
domain_name = _extract_domain_name(cluster_name, namespace)
649+
return Cluster.from_k8_cluster_object(
650+
rc, mcad=mcad, domain_name=domain_name
651+
)
648652
raise FileNotFoundError(
649653
f"Cluster {cluster_name} is not found in {namespace} namespace"
650654
)
@@ -663,14 +667,40 @@ def _check_aw_exists(name: str, namespace: str) -> bool:
663667
)
664668
except Exception as e: # pragma: no cover
665669
return _kube_api_error_handling(e, print_error=False)
666-
667670
for aw in aws["items"]:
668671
if aw["metadata"]["name"] == name:
669672
return True
670673
return False
671674

672675

673-
# Cant test this until get_current_namespace is fixed
676+
def _extract_domain_name(name: str, namespace: str) -> str:
677+
try:
678+
config_check()
679+
api_instance = client.CustomObjectsApi(api_config_handler())
680+
aws = api_instance.list_namespaced_custom_object(
681+
group="workload.codeflare.dev",
682+
version="v1beta1",
683+
namespace=namespace,
684+
plural="appwrappers",
685+
)
686+
except Exception as e: # pragma: no cover
687+
return _kube_api_error_handling(e, print_error=False)
688+
for aw in aws["items"]:
689+
if aw["metadata"]["name"] == name:
690+
host = aw["spec"]["resources"]["GenericItems"][1]["generictemplate"][
691+
"spec"
692+
]["rules"][0]["host"]
693+
694+
dot_index = host.find(".")
695+
if dot_index != -1:
696+
domain_name = host[dot_index + 1 :]
697+
return domain_name
698+
else:
699+
print("Host is not configured correctly.")
700+
return None
701+
702+
703+
# Cant test this until get_current_namespace is fixed and placed in this function over using `self`
674704
def _get_ingress_domain(self): # pragma: no cover
675705
try:
676706
config_check()

src/codeflare_sdk/cluster/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,4 @@ class ClusterConfiguration:
5454
dispatch_priority: str = None
5555
openshift_oauth: bool = False # NOTE: to use the user must have permission to create a RoleBinding for system:auth-delegator
5656
ingress_options: dict = field(default_factory=dict)
57-
ingress_domain: str = None
57+
domain_name: str = None

src/codeflare_sdk/utils/generate_yaml.py

Lines changed: 36 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
from base64 import b64encode
3030
from urllib3.util import parse_url
3131

32-
from kubernetes import client, config
33-
3432
from .kube_api_helpers import _get_api_host
3533

3634

@@ -56,26 +54,23 @@ def gen_dashboard_ingress_name(cluster_name):
5654
return f"ray-dashboard-{cluster_name}"
5755

5856

59-
# Check if the ingress api cluster resource exists
57+
# Check if the routes api exists
6058
def is_openshift_cluster():
6159
try:
6260
config_check()
63-
api_instance = client.CustomObjectsApi(api_config_handler())
64-
api_instance.get_cluster_custom_object(
65-
"config.openshift.io", "v1", "ingresses", "cluster"
66-
)
67-
68-
return True
69-
except client.ApiException as e: # pragma: no cover
70-
if e.status == 404 or e.status == 403:
71-
return False
61+
for api in client.ApisApi(api_config_handler()).get_api_versions().groups:
62+
for v in api.versions:
63+
if "route.openshift.io/v1" in v.group_version:
64+
return True
7265
else:
73-
print(f"Error detecting cluster type defaulting to Kubernetes: {e}")
7466
return False
67+
except client.ApiException as e: # pragma: no cover
68+
print(f"Error detecting cluster type defaulting to Kubernetes: {e}")
69+
return False
7570

7671

7772
def update_dashboard_ingress(
78-
ingress_item, cluster_name, namespace, ingress_options, ingress_domain
73+
ingress_item, cluster_name, namespace, ingress_options, domain_name
7974
): # pragma: no cover
8075
metadata = ingress_item.get("generictemplate", {}).get("metadata")
8176
spec = ingress_item.get("generictemplate", {}).get("spec")
@@ -123,34 +118,26 @@ def update_dashboard_ingress(
123118
"name"
124119
] = f"{cluster_name}-head-svc"
125120
else:
121+
if is_openshift_cluster():
122+
spec["ingressClassName"] = "openshift-default"
123+
else:
124+
spec["ingressClassName"] = "nginx"
125+
126126
metadata["name"] = f"ray-dashboard-{cluster_name}"
127127
metadata["namespace"] = namespace
128128
spec["rules"][0]["http"]["paths"][0]["backend"]["service"][
129129
"name"
130130
] = f"{cluster_name}-head-svc"
131-
if is_openshift_cluster():
132-
try:
133-
config_check()
134-
api_client = client.CustomObjectsApi(api_config_handler())
135-
ingress = api_client.get_cluster_custom_object(
136-
"config.openshift.io", "v1", "ingresses", "cluster"
137-
)
138-
del spec["ingressClassName"]
139-
except Exception as e: # pragma: no cover
140-
return _kube_api_error_handling(e)
141-
domain = ingress["spec"]["domain"]
142-
elif ingress_domain is None:
143-
raise ValueError(
144-
"ingress_domain is invalid. For Kubernetes Clusters please specify an ingress domain"
145-
)
131+
if domain_name is None:
132+
raise ValueError("domain_name is invalid. Please specify an ingress domain")
146133
else:
147-
domain = ingress_domain
134+
domain = domain_name
148135
del metadata["annotations"]
149136
spec["rules"][0]["host"] = f"ray-dashboard-{cluster_name}-{namespace}.{domain}"
150137

151138

152139
def update_rayclient_ingress(
153-
ingress_item, cluster_name, namespace, ingress_domain
140+
ingress_item, cluster_name, namespace, domain_name
154141
): # pragma: no cover
155142
metadata = ingress_item.get("generictemplate", {}).get("metadata")
156143
spec = ingress_item.get("generictemplate", {}).get("spec")
@@ -162,38 +149,27 @@ def update_rayclient_ingress(
162149
"name"
163150
] = f"{cluster_name}-head-svc"
164151

165-
if is_openshift_cluster():
166-
try:
167-
config_check()
168-
api_client = client.CustomObjectsApi(api_config_handler())
169-
ingress = api_client.get_cluster_custom_object(
170-
"config.openshift.io", "v1", "ingresses", "cluster"
171-
)
152+
if domain_name is not None:
153+
if is_openshift_cluster():
172154
ingressClassName = "openshift-default"
173155
annotations = {
174156
"nginx.ingress.kubernetes.io/rewrite-target": "/",
175157
"nginx.ingress.kubernetes.io/ssl-redirect": "true",
176158
"route.openshift.io/termination": "passthrough",
177159
}
178-
except Exception as e: # pragma: no cover
179-
return _kube_api_error_handling(e)
180-
domain = ingress["spec"]["domain"]
181-
elif ingress_domain is None:
182-
raise ValueError(
183-
"ingress_domain is invalid. For Kubernetes Clusters please specify an ingress domain"
184-
)
160+
else:
161+
ingressClassName = "nginx"
162+
annotations = {
163+
"nginx.ingress.kubernetes.io/rewrite-target": "/",
164+
"nginx.ingress.kubernetes.io/ssl-redirect": "true",
165+
"nginx.ingress.kubernetes.io/ssl-passthrough": "true",
166+
}
185167
else:
186-
domain = ingress_domain
187-
ingressClassName = "nginx"
188-
annotations = {
189-
"nginx.ingress.kubernetes.io/rewrite-target": "/",
190-
"nginx.ingress.kubernetes.io/ssl-redirect": "true",
191-
"nginx.ingress.kubernetes.io/ssl-passthrough": "true",
192-
}
168+
raise ValueError("domain_name is invalid. Please specify a domain")
193169

194170
metadata["annotations"] = annotations
195171
spec["ingressClassName"] = ingressClassName
196-
spec["rules"][0]["host"] = f"rayclient-{cluster_name}-{namespace}.{domain}"
172+
spec["rules"][0]["host"] = f"rayclient-{cluster_name}-{namespace}.{domain_name}"
197173

198174

199175
def update_names(yaml, item, appwrapper_name, cluster_name, namespace):
@@ -396,7 +372,7 @@ def update_ca_secret(ca_secret_item, cluster_name, namespace):
396372
data["ca.key"], data["ca.crt"] = generate_cert.generate_ca_cert(365)
397373

398374

399-
def enable_local_interactive(resources, cluster_name, namespace, ingress_domain):
375+
def enable_local_interactive(resources, cluster_name, namespace, domain_name):
400376
rayclient_ingress_item = resources["resources"].get("GenericItems")[2]
401377
ca_secret_item = resources["resources"].get("GenericItems")[3]
402378
item = resources["resources"].get("GenericItems")[0]
@@ -422,23 +398,12 @@ def enable_local_interactive(resources, cluster_name, namespace, ingress_domain)
422398

423399
command = command.replace("deployment-name", cluster_name)
424400

425-
if is_openshift_cluster():
426-
# We can try get the domain through checking ingresses.config.openshift.io
427-
try:
428-
config_check()
429-
api_client = client.CustomObjectsApi(api_config_handler())
430-
ingress = api_client.get_cluster_custom_object(
431-
"config.openshift.io", "v1", "ingresses", "cluster"
432-
)
433-
except Exception as e: # pragma: no cover
434-
return _kube_api_error_handling(e)
435-
domain = ingress["spec"]["domain"]
436-
elif ingress_domain is None:
401+
if domain_name is None:
437402
raise ValueError(
438-
"ingress_domain is invalid. For Kubernetes Clusters please specify an ingress domain"
403+
"domain_name is invalid. For Kubernetes Clusters please specify an ingress domain"
439404
)
440405
else:
441-
domain = ingress_domain
406+
domain = domain_name
442407

443408
command = command.replace("server-name", domain)
444409
update_rayclient_ingress(rayclient_ingress_item, cluster_name, namespace, domain)
@@ -618,7 +583,7 @@ def generate_appwrapper(
618583
dispatch_priority: str,
619584
priority_val: int,
620585
openshift_oauth: bool,
621-
ingress_domain: str,
586+
domain_name: str,
622587
ingress_options: dict,
623588
):
624589
user_yaml = read_template(template)
@@ -659,10 +624,10 @@ def generate_appwrapper(
659624
head_gpus,
660625
)
661626
update_dashboard_ingress(
662-
ingress_item, cluster_name, namespace, ingress_options, ingress_domain
627+
ingress_item, cluster_name, namespace, ingress_options, domain_name
663628
)
664629
if local_interactive:
665-
enable_local_interactive(resources, cluster_name, namespace, ingress_domain)
630+
enable_local_interactive(resources, cluster_name, namespace, domain_name)
666631
else:
667632
disable_raycluster_tls(resources["resources"])
668633

tests/test-case-no-mcad.yamls

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ metadata:
145145
name: ray-dashboard-unit-test-cluster-ray
146146
namespace: ns
147147
spec:
148+
ingressClassName: nginx
148149
rules:
149150
- host: ray-dashboard-unit-test-cluster-ray-ns.apps.cluster.awsroute.org
150151
http:

tests/test-case-prio.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ spec:
178178
name: ray-dashboard-prio-test-cluster
179179
namespace: ns
180180
spec:
181+
ingressClassName: nginx
181182
rules:
182183
- host: ray-dashboard-prio-test-cluster-ns.apps.cluster.awsroute.org
183184
http:

tests/test-case.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ spec:
175175
name: ray-dashboard-unit-test-cluster
176176
namespace: ns
177177
spec:
178+
ingressClassName: nginx
178179
rules:
179180
- host: ray-dashboard-unit-test-cluster-ns.apps.cluster.awsroute.org
180181
http:

0 commit comments

Comments
 (0)