Skip to content

Commit c98d13f

Browse files
guptaakacopybara-github
authored andcommitted
Deprecate proxy_server_image and auto-detect compatible proxy server image from Pathways service
PiperOrigin-RevId: 974690013
1 parent 8203925 commit c98d13f

8 files changed

Lines changed: 741 additions & 359 deletions

File tree

pathwaysutils/experimental/shared_pathways_service/gke_utils.py

Lines changed: 112 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
"""GKE utils for deploying and managing the Pathways proxy."""
22

3-
import json
3+
import functools
44
import logging
55
import re
66
import socket
77
import subprocess
88
import time
99
import urllib.parse
1010

11+
from kubernetes import client
12+
from kubernetes import config as k8s_config
1113
import portpicker
1214

1315
_logger = logging.getLogger(__name__)
@@ -477,6 +479,19 @@ def is_local_port_free(port: int) -> bool:
477479
return portpicker.is_port_free(port)
478480

479481

482+
@functools.lru_cache(maxsize=1)
483+
def _get_k8s_core_api() -> client.CoreV1Api:
484+
"""Initializes and returns the Kubernetes CoreV1Api client."""
485+
try:
486+
k8s_config.load_kube_config()
487+
except Exception: # pylint: disable=broad-except
488+
try:
489+
k8s_config.load_incluster_config()
490+
except Exception as e:
491+
raise RuntimeError("Failed to load Kubernetes configuration") from e
492+
return client.CoreV1Api()
493+
494+
480495
def get_worker_sidecar_image(
481496
pathways_service: str, namespace: str = "default"
482497
) -> str | None:
@@ -485,49 +500,109 @@ def get_worker_sidecar_image(
485500
_validate_k8s_name(namespace)
486501

487502
# Try to extract the jobset name from the Pathways service hostname.
488-
jobset_name = None
489-
if "-pathways-head" in pathways_head_hostname:
490-
jobset_name = pathways_head_hostname.split("-pathways-head")[0]
503+
jobset_name = pathways_head_hostname.split("-pathways-head")[0]
491504

492-
command = ["kubectl", "get", "pods", "-n", namespace, "-o", "json"]
493505
try:
494-
result = subprocess.run(
495-
command,
496-
check=True,
497-
capture_output=True,
498-
text=True,
506+
v1 = _get_k8s_core_api()
507+
pod_list = v1.list_namespaced_pod(
508+
namespace=namespace,
509+
label_selector=f"jobset.sigs.k8s.io/jobset-name={jobset_name}",
499510
)
500-
except subprocess.CalledProcessError as e:
501-
_logger.exception("Failed to get pods. kubectl output:\n%r", e.stderr)
511+
except Exception as e: # pylint: disable=broad-except
512+
_logger.exception("Failed to list pods for sidecar image: %r", e)
502513
return None
503514

515+
for pod in pod_list.items:
516+
spec = pod.spec
517+
if not spec:
518+
continue
519+
containers = (spec.init_containers or []) + (spec.containers or [])
520+
for container in containers:
521+
if container.name == "colocated-python-sidecar":
522+
if container.image:
523+
return container.image
524+
525+
return None
526+
527+
528+
def get_server_image(
529+
pathways_service: str, namespace: str = "default"
530+
) -> str:
531+
"""Gets the server image used by the Pathways service."""
532+
pathways_head_hostname = pathways_service.split(":")[0]
533+
_validate_k8s_name(namespace)
534+
535+
# Try to extract the jobset name from the Pathways service hostname.
536+
if "-pathways-head" not in pathways_head_hostname:
537+
raise ValueError(
538+
"Failed to extract jobset name from Pathways service hostname:"
539+
f" {pathways_head_hostname}"
540+
)
541+
jobset_name = pathways_head_hostname.split("-pathways-head")[0]
542+
504543
try:
505-
pods_data = json.loads(result.stdout)
506-
except json.JSONDecodeError as e:
507-
_logger.exception("Failed to parse kubectl get pods output: %r", e)
508-
return None
544+
v1 = _get_k8s_core_api()
545+
pod_list = v1.list_namespaced_pod(
546+
namespace=namespace,
547+
label_selector=f"jobset.sigs.k8s.io/jobset-name={jobset_name}",
548+
)
549+
except client.rest.ApiException as e:
550+
_logger.exception("Failed to list pods: %r", e)
551+
raise
552+
except Exception as e:
553+
_logger.exception("Failed to get pods: %r", e)
554+
raise
509555

510-
items = pods_data.get("items", [])
511-
512-
# Look for pods belonging to the jobset and having the sidecar
513-
# container/initContainer.
514-
if jobset_name:
515-
for pod in items:
516-
metadata = pod.get("metadata", {})
517-
labels = metadata.get("labels", {})
518-
pod_jobset_name = labels.get("jobset.sigs.k8s.io/jobset-name")
519-
pod_name = metadata.get("name", "")
520-
521-
if pod_jobset_name == jobset_name or pod_name.startswith(jobset_name):
522-
spec = pod.get("spec", {})
523-
for container in spec.get("initContainers", []) + spec.get(
524-
"containers", []
525-
):
526-
if container.get("name") == "colocated-python-sidecar":
527-
image = container.get("image")
528-
if image:
529-
return image
556+
for pod in pod_list.items:
557+
spec = pod.spec
558+
if not spec:
559+
continue
560+
containers = (spec.containers or []) + (spec.init_containers or [])
561+
for container in containers:
562+
if container.name in ("pathways-rm", "pathways-worker"):
563+
if container.image:
564+
return container.image
565+
566+
raise RuntimeError(
567+
"Failed to get server image for Pathways service:"
568+
f" {pathways_service} in namespace: {namespace}"
569+
)
570+
571+
572+
def get_compatible_proxy_server_image(server_image: str) -> str:
573+
"""Converts a Pathways server image to its compatible proxy server image."""
574+
if not server_image:
575+
return server_image
576+
577+
# Extract tag or digest if present.
578+
if "@" in server_image:
579+
repo, tag_or_digest = server_image.split("@", 1)
580+
sep = "@"
581+
else:
582+
last_slash = server_image.rfind("/")
583+
if ":" in server_image[last_slash + 1:]:
584+
repo, tag_or_digest = server_image.rsplit(":", 1)
585+
sep = ":"
586+
else:
587+
repo = server_image
588+
tag_or_digest = None
589+
sep = ""
590+
591+
prefix, sep_slash, last_component = repo.rpartition("/")
592+
new_last_component = last_component.replace("server", "proxy_server")
593+
594+
new_repo = f"{prefix}{sep_slash}{new_last_component}"
595+
if tag_or_digest is not None:
596+
return f"{new_repo}{sep}{tag_or_digest}"
597+
return new_repo
598+
599+
600+
def get_proxy_server_image(
601+
pathways_service: str, namespace: str = "default"
602+
) -> str:
603+
"""Gets the compatible proxy server image for the given Pathways service."""
604+
server_image = get_server_image(pathways_service, namespace=namespace)
605+
return get_compatible_proxy_server_image(server_image)
530606

531-
return None
532607

533608

pathwaysutils/experimental/shared_pathways_service/isc_pathways.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import threading
1313
import time
1414
from typing import Any
15+
import warnings
1516

1617
import jax
1718
import jax.extend.backend as jax_backend
@@ -455,7 +456,7 @@ def connect(
455456
pathways_service: str,
456457
expected_tpu_instances: Mapping[str, int],
457458
proxy_job_name: str | None = None,
458-
proxy_server_image: str = DEFAULT_PROXY_IMAGE,
459+
proxy_server_image: str | None = None,
459460
proxy_options: Sequence[str] | None = None,
460461
collect_service_metrics: bool = False,
461462
) -> Iterator["_ISCPathways"]:
@@ -471,8 +472,10 @@ def connect(
471472
of instances. For example: {"tpuv6e:2x2": 2}
472473
proxy_job_name: The name to use for the deployed proxy. If not provided, a
473474
random name will be generated.
474-
proxy_server_image: The proxy server image to use. If not provided, a
475-
default will be used.
475+
proxy_server_image: (Deprecated) The proxy server image to use. If not
476+
provided, it will be auto-detected from the Pathways service. If the given
477+
proxy image is incompatible with the Pathways service, it will be
478+
replaced with the compatible proxy image.
476479
proxy_options: Configuration options for the Pathways proxy. If not
477480
provided, no extra options will be used.
478481
collect_service_metrics: Whether to collect usage metrics for Shared
@@ -481,15 +484,36 @@ def connect(
481484
Yields:
482485
The Pathways manager.
483486
"""
487+
if proxy_server_image is not None:
488+
warnings.warn(
489+
"`proxy_server_image` is deprecated and will be removed in a future"
490+
" release. The proxy server image is automatically detected from the"
491+
" Pathways service.",
492+
DeprecationWarning,
493+
stacklevel=2,
494+
)
484495
_logger.info("Validating Pathways service and TPU instances...")
485496
validators.validate_pathways_service(pathways_service)
486497
validators.validate_tpu_instances(expected_tpu_instances)
487-
validators.validate_proxy_server_image(proxy_server_image)
488498
validators.validate_proxy_options(proxy_options)
489499
gke_utils.fetch_cluster_credentials(
490500
cluster_name=cluster, project_id=project, location=region
491501
)
492502

503+
compatible_proxy_image = gke_utils.get_proxy_server_image(pathways_service)
504+
_logger.info(
505+
"Auto-detected compatible proxy server image: %s", compatible_proxy_image
506+
)
507+
if proxy_server_image and proxy_server_image != compatible_proxy_image:
508+
_logger.warning(
509+
"The provided proxy image '%s' is incompatible with the service"
510+
" '%s'. Replacing it with the compatible proxy image '%s'.",
511+
proxy_server_image,
512+
pathways_service,
513+
compatible_proxy_image,
514+
)
515+
proxy_server_image = compatible_proxy_image
516+
493517
proxy_options_obj = ProxyOptions.from_list(proxy_options)
494518
if proxy_options_obj.sidecar:
495519
sidecar_image = gke_utils.get_worker_sidecar_image(

pathwaysutils/experimental/shared_pathways_service/run_connect_example.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
flags.DEFINE_string(
3737
"proxy_server_image",
3838
None,
39-
"The proxy server image to use. If not provided, a default will be used.",
39+
"Deprecated: The proxy server image to use. If not provided, it will be"
40+
" auto-detected from the Pathways service.",
4041
)
4142
flags.DEFINE_list(
4243
"proxy_options",
@@ -73,8 +74,7 @@ def main(argv: Sequence[str]) -> None:
7374
pathways_service=FLAGS.pathways_service,
7475
expected_tpu_instances={FLAGS.tpu_type: FLAGS.tpu_count},
7576
proxy_job_name=FLAGS.proxy_job_name,
76-
proxy_server_image=FLAGS.proxy_server_image
77-
or isc_pathways.DEFAULT_PROXY_IMAGE,
77+
proxy_server_image=FLAGS.proxy_server_image,
7878
proxy_options=FLAGS.proxy_options,
7979
collect_service_metrics=FLAGS.collect_service_metrics,
8080
):

pathwaysutils/experimental/shared_pathways_service/run_workload.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import shlex
2323
import subprocess
2424
from typing import Any, ContextManager
25+
import warnings
2526

2627
from absl import app
2728
from absl import flags
@@ -55,7 +56,8 @@
5556
_PROXY_SERVER_IMAGE = flags.DEFINE_string(
5657
"proxy_server_image",
5758
"",
58-
"The proxy server image to use. If not provided, a default will be used.",
59+
"Deprecated: The proxy server image to use. If not provided, it will be"
60+
" auto-detected from the Pathways service.",
5961
)
6062
_PROXY_OPTIONS = flags.DEFINE_list(
6163
"proxy_options",
@@ -103,7 +105,8 @@ def run_command(
103105
tpu_type: The TPU machine type and topology.
104106
tpu_count: The number of TPU slices.
105107
command: The command to run on TPUs.
106-
proxy_server_image: The proxy server image to use.
108+
proxy_server_image: (Deprecated) The proxy server image to use. If not
109+
provided, it will be auto-detected from the Pathways service.
107110
proxy_options: Configuration options for the Pathways proxy.
108111
collect_service_metrics: Whether to collect usage metrics for Shared
109112
Pathways Service. Defaults to False.
@@ -113,6 +116,14 @@ def run_command(
113116
Raises:
114117
subprocess.CalledProcessError: If the workload command fails.
115118
"""
119+
if proxy_server_image:
120+
warnings.warn(
121+
"`proxy_server_image` is deprecated and will be removed in a future"
122+
" release. The proxy server image is automatically detected from the"
123+
" Pathways service.",
124+
DeprecationWarning,
125+
stacklevel=2,
126+
)
116127
logging.info("Connecting to Shared Pathways Service...")
117128
with connect_fn(
118129
cluster=cluster,
@@ -122,9 +133,7 @@ def run_command(
122133
pathways_service=pathways_service,
123134
expected_tpu_instances={tpu_type: tpu_count},
124135
proxy_server_image=(
125-
proxy_server_image
126-
if proxy_server_image
127-
else isc_pathways.DEFAULT_PROXY_IMAGE
136+
proxy_server_image if proxy_server_image else None
128137
),
129138
proxy_options=proxy_options,
130139
collect_service_metrics=collect_service_metrics,

pathwaysutils/experimental/shared_pathways_service/validators.py

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,22 +113,6 @@ def validate_tpu_instances(expected_tpu_instances: Mapping[Any, Any]) -> None:
113113
_validate_tpu_supported(inst)
114114

115115

116-
def validate_proxy_server_image(proxy_server_image: str) -> None:
117-
"""Validates the proxy server image format."""
118-
if not proxy_server_image or not proxy_server_image.strip():
119-
raise ValueError("Proxy server image cannot be empty.")
120-
if "/" not in proxy_server_image:
121-
raise ValueError(
122-
f"Proxy server image '{proxy_server_image}' must contain '/', "
123-
"separating the registry or namespace from the final image name."
124-
)
125-
if ":" not in proxy_server_image and "@" not in proxy_server_image:
126-
raise ValueError(
127-
f"Proxy server image '{proxy_server_image}' must contain a tag with ':'"
128-
" or a digest with '@'."
129-
)
130-
131-
132116
def validate_xla_flags(xla_flags: Iterable[str] | None) -> None:
133117
"""Validates that all XLA flags start with '--xla_'."""
134118
if not xla_flags:

0 commit comments

Comments
 (0)