Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[k8s] Use images.csv catalog to get container image tags #2556

Merged
merged 6 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions sky/clouds/kubernetes.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from sky import sky_logging
from sky import status_lib
from sky.adaptors import kubernetes
from sky.clouds import service_catalog
from sky.utils import common_utils
from sky.utils import kubernetes_utils
from sky.utils import ux_utils
Expand Down Expand Up @@ -66,10 +67,8 @@ class Kubernetes(clouds.Cloud):
('Docker image is not supported in Kubernetes. ')
}

IMAGE_CPU = ('us-central1-docker.pkg.dev/'
'skypilot-375900/skypilotk8s/skypilot:latest')
IMAGE_GPU = ('us-central1-docker.pkg.dev/skypilot-375900/'
'skypilotk8s/skypilot-gpu:latest')
IMAGE_CPU = 'skypilot:cpu-ubuntu-2004'
IMAGE_GPU = 'skypilot:gpu-ubuntu-2004'

@classmethod
def _cloud_unsupported_features(
Expand Down Expand Up @@ -202,7 +201,14 @@ def make_deploy_resources_variables(
acc_type = k.accelerator_type if k.accelerator_type else None

# Select image based on whether we are using GPUs or not.
image = self.IMAGE_GPU if acc_count > 0 else self.IMAGE_CPU
image_id = self.IMAGE_GPU if acc_count > 0 else self.IMAGE_CPU
# Get the container image ID from the service catalog.
# TODO(romilb): Note that currently we do not support custom images,
# so the image_id should start with 'skypilot:'.
# In the future we may want to get image_id from the resources object.
assert image_id.startswith('skypilot:')
image_id = service_catalog.get_image_id_from_tag(image_id,
clouds='kubernetes')

k8s_acc_label_key = None
k8s_acc_label_value = None
Expand All @@ -224,7 +230,7 @@ def make_deploy_resources_variables(
'k8s_acc_label_key': k8s_acc_label_key,
'k8s_acc_label_value': k8s_acc_label_value,
# TODO(romilb): Allow user to specify custom images
'image_id': image,
'image_id': image_id,
}
return deploy_vars

Expand Down
4 changes: 1 addition & 3 deletions sky/clouds/service_catalog/README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Service Catalog

We support 3 major clouds: AWS, Azure and GCP.

This module provides information including the instance type offerings, their pricing and data transfer costs. It also provides functions to query these information, and to select the most suitable instance types based on resource requirements. Primarily used by the Clouds module.
This module provides information for clouds supported by SkyPilot, including the instance type offerings, their pricing and data transfer costs. It also provides functions to query these information, and to select the most suitable instance types based on resource requirements. Primarily used by the Clouds module.

- `data_fetchers/fetch_{aws,azure}.py`: each file is a standalone script that queries the cloud APIs to produce the pricing list files.
- `data_fetchers/fetch_gcp.py`: A script that generates the GCP catalog based by crawling GCP websites.
Expand Down
28 changes: 28 additions & 0 deletions sky/clouds/service_catalog/kubernetes_catalog.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
"""Kubernetes Catalog.

Kubernetes does not require a catalog of instances, but we need an image catalog
mapping SkyPilot image tags to corresponding container image tags.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mapping SkyPilot image tags to corresponding container image tags.
mapping SkyPilot image tags to corresponding container image ids.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, they are still container image tags (e.g., ubuntu:latest), since container image ids are hex values and are typically not used to specify images to use.

"""

from typing import Optional

from sky.clouds.service_catalog import common

_PULL_FREQUENCY_HOURS = 7

# We keep pull_frequency_hours so we can remotely update the default image paths
_image_df = common.read_catalog('kubernetes/images.csv',
pull_frequency_hours=_PULL_FREQUENCY_HOURS)

# TODO(romilb): Refactor implementation of common service catalog functions from
# clouds/kubernetes.py to kubernetes_catalog.py


def get_image_id_from_tag(tag: str, region: Optional[str]) -> Optional[str]:
"""Returns the image id from the tag."""
return common.get_image_id_from_tag_impl(_image_df, tag, region)


def is_image_tag_valid(tag: str, region: Optional[str]) -> bool:
"""Returns whether the image tag is valid."""
return common.is_image_tag_valid_impl(_image_df, tag, region)