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

[UX] Support checking user specified cloud(s) with sky check from CLI #3229

Merged
merged 14 commits into from
May 16, 2024
57 changes: 41 additions & 16 deletions sky/check.py
romilbhardwaj marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
"""Credential checks: check cloud credentials and enable clouds."""
import traceback
from typing import Dict, Iterable, List, Optional, Tuple
from typing import Any, Dict, Iterable, List, Optional, Tuple

import click
import colorama
import rich

from sky import clouds
from sky import clouds as sky_clouds
from sky import exceptions
from sky import global_user_state
from sky.adaptors import cloudflare
from sky.utils import ux_utils


# TODO(zhwu): add check for a single cloud to improve performance
def check(quiet: bool = False, verbose: bool = False) -> None:
def check(
dtran24 marked this conversation as resolved.
Show resolved Hide resolved
romilbhardwaj marked this conversation as resolved.
Show resolved Hide resolved
quiet: bool = False,
verbose: bool = False,
clouds: Optional[Tuple[str]] = None,
romilbhardwaj marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
echo = (lambda *_args, **_kwargs: None) if quiet else click.echo
echo('Checking credentials to enable clouds for SkyPilot.')

enabled_clouds = []
dtran24 marked this conversation as resolved.
Show resolved Hide resolved
disabled_clouds = []

def check_one_cloud(cloud_tuple: Tuple[str, clouds.Cloud]) -> None:
def check_one_cloud(cloud_tuple: Tuple[str, Any]) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note - Any was required for cloudflare support, which is not a cloud but a module.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about Union[sky_clouds.Cloud, ModuleType]? (from types import ModuleType)

cloud_repr, cloud = cloud_tuple
echo(f' Checking {cloud_repr}...', nl=False)
try:
Expand All @@ -43,22 +46,43 @@ def check_one_cloud(cloud_tuple: Tuple[str, clouds.Cloud]) -> None:
if reason is not None:
echo(f' Hint: {reason}')
else:
disabled_clouds.append(cloud_repr)
echo(f' Reason: {reason}')

clouds_to_check = [
(repr(cloud), cloud) for cloud in clouds.CLOUD_REGISTRY.values()
]
clouds_to_check.append(('Cloudflare, for R2 object store', cloudflare))
if clouds is not None:
clouds_to_check: List[Tuple[str, Any]] = []
for cloud in clouds:
if cloud.lower() == 'cloudflare':
clouds_to_check.append(
('Cloudflare, for R2 object store', cloudflare))
else:
clouds_to_check.append(
(cloud.lower(), sky_clouds.CLOUD_REGISTRY.from_str(cloud)))
else:
clouds_to_check = [(repr(cloud), cloud)
for cloud in sky_clouds.CLOUD_REGISTRY.values()]
clouds_to_check.append(('Cloudflare, for R2 object store', cloudflare))

for cloud_tuple in sorted(clouds_to_check):
check_one_cloud(cloud_tuple)

# Cloudflare is not a real cloud in clouds.CLOUD_REGISTRY, and should not be
# inserted into the DB (otherwise `sky launch` and other code would error
# out when it's trying to look it up in the registry).
# Cloudflare is not a real cloud in sky_clouds.CLOUD_REGISTRY, and should
# not be inserted into the DB (otherwise `sky launch` and other code would
# error out when it's trying to look it up in the registry).
enabled_clouds = [
cloud for cloud in enabled_clouds if not cloud.startswith('Cloudflare')
]
disabled_clouds = [
cloud for cloud in disabled_clouds if not cloud.startswith('Cloudflare')
]
previously_enabled_clouds = [
repr(cloud) for cloud in global_user_state.get_cached_enabled_clouds()
]
for cloud in previously_enabled_clouds:
is_not_duplicate = cloud not in enabled_clouds
is_not_disabled = cloud not in disabled_clouds
if is_not_duplicate and is_not_disabled:
enabled_clouds.append(cloud)
romilbhardwaj marked this conversation as resolved.
Show resolved Hide resolved
global_user_state.set_enabled_clouds(enabled_clouds)

if len(enabled_clouds) == 0:
Expand Down Expand Up @@ -88,7 +112,7 @@ def check_one_cloud(cloud_tuple: Tuple[str, clouds.Cloud]) -> None:


def get_cached_enabled_clouds_or_refresh(
raise_if_no_cloud_access: bool = False) -> List[clouds.Cloud]:
raise_if_no_cloud_access: bool = False) -> List[sky_clouds.Cloud]:
"""Returns cached enabled clouds and if no cloud is enabled, refresh.

This function will perform a refresh if no public cloud is enabled.
Expand Down Expand Up @@ -120,7 +144,8 @@ def get_cached_enabled_clouds_or_refresh(


def get_cloud_credential_file_mounts(
excluded_clouds: Optional[Iterable[clouds.Cloud]]) -> Dict[str, str]:
excluded_clouds: Optional[Iterable[sky_clouds.Cloud]]
) -> Dict[str, str]:
"""Returns the files necessary to access all enabled clouds.

Returns a dictionary that will be added to a task's file mounts
Expand All @@ -130,7 +155,7 @@ def get_cloud_credential_file_mounts(
file_mounts = {}
for cloud in enabled_clouds:
if (excluded_clouds is not None and
clouds.cloud_in_iterable(cloud, excluded_clouds)):
sky_clouds.cloud_in_iterable(cloud, excluded_clouds)):
continue
cloud_file_mounts = cloud.get_credential_file_mounts()
file_mounts.update(cloud_file_mounts)
Expand Down
16 changes: 10 additions & 6 deletions sky/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import sky
from sky import backends
from sky import check as sky_check
from sky import clouds
from sky import clouds as sky_clouds
from sky import core
from sky import exceptions
from sky import global_user_state
Expand Down Expand Up @@ -479,7 +479,7 @@ def _parse_override_params(
if cloud.lower() == 'none':
override_params['cloud'] = None
else:
override_params['cloud'] = clouds.CLOUD_REGISTRY.from_str(cloud)
override_params['cloud'] = sky_clouds.CLOUD_REGISTRY.from_str(cloud)
if region is not None:
if region.lower() == 'none':
override_params['region'] = None
Expand Down Expand Up @@ -2863,23 +2863,27 @@ def _down_or_stop(name: str):


@cli.command()
@click.argument('clouds', required=False, type=str, nargs=-1)
@click.option('--verbose',
'-v',
is_flag=True,
default=False,
help='Show the activated account for each cloud.')
@usage_lib.entrypoint
def check(verbose: bool):
def check(clouds: Tuple[str], verbose: bool):
"""Check which clouds are available to use.

This checks access credentials for all clouds supported by SkyPilot. If a
cloud is detected to be inaccessible, the reason and correction steps will
be shown.

If CLOUDS are specified, checks credentials for only those clouds.

The enabled clouds are cached and form the "search space" to be considered
dtran24 marked this conversation as resolved.
Show resolved Hide resolved
for each task.
"""
romilbhardwaj marked this conversation as resolved.
Show resolved Hide resolved
sky_check.check(verbose=verbose)
clouds_arg = clouds if len(clouds) > 0 else None
sky_check.check(verbose=verbose, clouds=clouds_arg)


@cli.command()
Expand Down Expand Up @@ -2958,7 +2962,7 @@ def show_gpus(
'--all-regions and --region flags cannot be used simultaneously.')

# This will validate 'cloud' and raise if not found.
cloud_obj = clouds.CLOUD_REGISTRY.from_str(cloud)
cloud_obj = sky_clouds.CLOUD_REGISTRY.from_str(cloud)
service_catalog.validate_region_zone(region, None, clouds=cloud)
show_all = all
if show_all and accelerator_str is not None:
Expand All @@ -2978,7 +2982,7 @@ def _output():
name, quantity = None, None

# Kubernetes specific bools
cloud_is_kubernetes = isinstance(cloud_obj, clouds.Kubernetes)
cloud_is_kubernetes = isinstance(cloud_obj, sky_clouds.Kubernetes)
kubernetes_autoscaling = kubernetes_utils.get_autoscaler_type(
) is not None

Expand Down
Loading