Repository: skypilot-org/skypilot
Pull request: [UX] Auto-exclude unavailable kubernetes contexts
<!-- Describe the changes in this PR -->

close https://github.com/skypilot-org/skypilot/issues/2807

- ~~Excludes unavailable kubernetes contexts at runtime~~ (**UPDATED**: see https://github.com/skypilot-org/skypilot/pull/4692#discussion_r1952092329);
  - skip unavailable kubernetes contexts, the context will be re-considered in next visit;
  - behavior on permanent error would be optimized in https://github.com/skypilot-org/skypilot/issues/2543, out of the scope of this PR
- Refine `sky check` message for multi-kubernetes setup to show hint/reason of each context and guide user to exclude stale kubernetes cluster from `allowed_contexts` config;

@romilbhardwaj @Michaelvll please kindly take a look.

UX:

- The default context is unavailable:

```bash
$ sky launch
Excluding Kubernetes context kind-kind: Timed out when trying to get node info from Kubernetes cluster. Please check if the cluster is healthy and retry. To debug, run: kubectl get nodes --context kind-kind
All Kubernetes contexts have reported as unavailable. Retry if it is a transient error, or run sky check to refresh Kubernetes availability if permanent.
Considered resources (1 node):
------------------------------------------------------------------------------------------
 CLOUD   INSTANCE      vCPUs   Mem(GB)   ACCELERATORS   REGION/ZONE   COST ($)   CHOSEN
------------------------------------------------------------------------------------------
 AWS     m6i.2xlarge   8       32        -              us-east-1     0.38          ✔
------------------------------------------------------------------------------------------
```

> The following test results were produced with config:
> ```
> allowed_clouds:
>    - aws
>    - kubernetes
> kubernetes:
>   allowed_contexts:
>   - kind-kind
>   - kind-test
> ```

- Some of `allowed_contexts` are unavailable:

```bash
$ sky launch
Excluding Kubernetes context kind-kind: Timed out when trying to get node info from Kubernetes cluster. Please check if the cluster is healthy and retry. To debug, run: kubectl get nodes --context kind-kind
Considered resources (1 node):
-----------------------------------------------------------------------------------------------
 CLOUD        INSTANCE      vCPUs   Mem(GB)   ACCELERATORS   REGION/ZONE   COST ($)   CHOSEN
-----------------------------------------------------------------------------------------------
 Kubernetes   2CPU--2GB     2       2         -              kind-test     0.00          ✔
 AWS          m6i.2xlarge   8       32        -              us-east-1     0.38
-----------------------------------------------------------------------------------------------
```

- All `allowed_contexts` are unavailable:

```bash
$ sky launch
Excluding Kubernetes context kind-kind: Timed out when trying to get node info from Kubernetes cluster. Please check if the cluster is healthy and retry. To debug, run: kubectl get nodes --context kind-kind
Excluding Kubernetes context kind-test: Timed out when trying to get node info from Kubernetes cluster. Please check if the cluster is healthy and retry. To debug, run: kubectl get nodes --context kind-test
All Kubernetes contexts have reported as unavailable. Retry if it is a transient error, or run sky check to refresh Kubernetes availability if permanent.
Considered resources (1 node):
------------------------------------------------------------------------------------------
 CLOUD   INSTANCE      vCPUs   Mem(GB)   ACCELERATORS   REGION/ZONE   COST ($)   CHOSEN
------------------------------------------------------------------------------------------
 AWS     m6i.2xlarge   8       32        -              us-east-1     0.38          ✔
------------------------------------------------------------------------------------------
```

- `sky check`:

<img width="1479" alt="image" src="https://github.com/user-attachments/assets/61ada0d2-9c97-49ea-8492-9d08fdb9c6e4" />

<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

Tested (run the relevant ones):

- [x] Code formatting: `bash format.sh`
- [x] Any manual or new tests for this PR (see above)
- [ ] All smoke tests: `pytest tests/test_smoke.py` 
- [ ] Relevant individual smoke tests: `pytest tests/test_smoke.py::test_fill_in_the_name` 
- [ ] Backward compatibility tests: `conda deactivate; bash -i tests/backward_compatibility_tests.sh`


Problem statement:
[k8s][ux] Auto-exclude stale Kubernetes cloud
I often terminate a Kubernetes cluster externally using the cloud console/cli (e.g., `gcloud container clusters delete <cluster-name> --region us-central1-c`), but I forget to run sky check to update the list of enabled clouds.

As a result, the next `sky launch` fails:
```
sky.exceptions.ResourcesUnavailableError: Timed out when trying to get node info from Kubernetes cluster. Please check if the cluster is healthy and retry.
```

We should consider printing a warning and continuing by either:
1) Excluding `Kubernetes` from the list of clouds considered by the optimizer
2) Removing `Kubernetes` from the list of enabled clouds stored in global user state.

1 is less aggressive and doesn't require user to re-run `sky check` in case it is a transient failure.

Hints from issue discussion:
This is also related to #3013 
Going to self-assign and work on this!
This issue is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days.
This issue is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days.
This issue was closed because it has been stalled for 10 days with no activity.
I'm running into this after having renewed my k8s cert in my kube config. I can see the pods as unhealthy, there might have been another isuse. 

However, I'm unable to start new clusters on said k8s due this error.


### Update

It seems like the error is actually swallowing a real error - in my case `BAD_BASE64_DECODE` - which in my case I can only see when executing the `purge` command
Thanks for the report @chris-aeviator - that sounds bad. Can you share the full output log and the commands you ran so I can reproduce it? 
Nvm @chris-aeviator, I can reproduce this. Looks like a recent regression from #4443. Being fixed in https://github.com/skypilot-org/skypilot/pull/4514 - can you give that branch a try and see if it fixes your issue too? 
Looking into this.

Commit message: address review comments

Signed-off-by: Aylei <rayingecho@gmail.com>

Patch to review:
diff --git a/sky/clouds/kubernetes.py b/sky/clouds/kubernetes.py
index 3bab7687f48..7c44e79f1b2 100644
--- a/sky/clouds/kubernetes.py
+++ b/sky/clouds/kubernetes.py
@@ -3,9 +3,10 @@
 import os
 import re
 import typing
-from typing import Dict, Iterator, List, Optional, Tuple, Union
+from typing import Dict, Iterator, List, Optional, Set, Tuple, Union
 
 from sky import clouds
+from sky import exceptions
 from sky import sky_logging
 from sky import skypilot_config
 from sky.adaptors import kubernetes
@@ -77,6 +78,11 @@ class Kubernetes(clouds.Cloud):
     PROVISIONER_VERSION = clouds.ProvisionerVersion.SKYPILOT
     STATUS_VERSION = clouds.StatusVersion.SKYPILOT
 
+    _INDENT_PREFIX = ' ' * 4
+
+    # Set of contexts that has logged as temporarily unavailable
+    logged_unavailable_contexts: Set[str] = set()
+
     @property
     def ssh_key_secret_field_name(self):
         # Use a fresh user hash to avoid conflicts in the secret object naming.
@@ -89,6 +95,8 @@ def ssh_key_secret_field_name(self):
     def _unsupported_features_for_resources(
         cls, resources: 'resources_lib.Resources'
     ) -> Dict[clouds.CloudImplementationFeatures, str]:
+        # TODO(aylei): features need to be regional (per context) to make
+        # multi-kubernetes selection/failover work.
         unsupported_features = cls._CLOUD_UNSUPPORTED_FEATURES.copy()
         context = resources.region
         if context is None:
@@ -105,10 +113,13 @@ def _unsupported_features_for_resources(
             unsupported_features[
                 clouds.CloudImplementationFeatures.AUTO_TERMINATE] = message
         # Allow spot instances if supported by the cluster
-        spot_label_key, _ = kubernetes_utils.get_spot_label(context)
-        if spot_label_key is not None:
-            unsupported_features.pop(
-                clouds.CloudImplementationFeatures.SPOT_INSTANCE, None)
+        try:
+            spot_label_key, _ = kubernetes_utils.get_spot_label(context)
+            if spot_label_key is not None:
+                unsupported_features.pop(
+                    clouds.CloudImplementationFeatures.SPOT_INSTANCE, None)
+        except exceptions.ResourcesUnavailableError as e:
+            cls._log_unavailable_context(context, str(e))
         return unsupported_features
 
     @classmethod
@@ -169,6 +180,38 @@ def existing_allowed_contexts(cls) -> List[str]:
         cls._log_skipped_contexts_once(tuple(skipped_contexts))
         return existing_contexts
 
+    @classmethod
+    def _log_unavailable_context(cls,
+                                 context: str,
+                                 reason: Optional[str] = None,
+                                 silent: bool = False) -> None:
+        """log a Kubernetes context as unavailable.
+
+        Args:
+            context: The Kubernetes context to mark as unavailable.
+            reason: Optional reason for marking the context as unavailable.
+            silent: Whether to suppress the log message.
+        """
+        # Skip if this context has already been logged as unavailable
+        if context in cls.logged_unavailable_contexts:
+            return
+
+        cls.logged_unavailable_contexts.add(context)
+        if not silent:
+            msg = f'Excluding Kubernetes context {context}'
+            if reason is not None:
+                msg += f': {reason}'
+            logger.info(msg)
+
+            # Check if all existing allowed contexts are now unavailable
+            existing_contexts = cls.existing_allowed_contexts()
+            if existing_contexts and all(ctx in cls.logged_unavailable_contexts
+                                         for ctx in existing_contexts):
+                logger.warning(
+                    'All Kubernetes contexts have reported as unavailable. '
+                    'Retry if it is a transient error, or run sky check to '
+                    'refresh Kubernetes availability if permanent.')
+
     @classmethod
     def regions_with_offering(cls, instance_type: Optional[str],
                               accelerators: Optional[Dict[str, int]],
@@ -197,8 +240,12 @@ def regions_with_offering(cls, instance_type: Optional[str],
             # provision_timeout, after which failover will be triggered.
             for r in regions:
                 context = r.name
-                fits, reason = kubernetes_utils.check_instance_fits(
-                    context, instance_type)
+                try:
+                    fits, reason = kubernetes_utils.check_instance_fits(
+                        context, instance_type)
+                except exceptions.ResourcesUnavailableError as e:
+                    cls._log_unavailable_context(context, str(e))
+                    continue
                 if fits:
                     regions_to_return.append(r)
                 else:
@@ -606,18 +653,52 @@ def check_credentials(cls) -> Tuple[bool, Optional[str]]:
                     'Check if you have a valid kubeconfig file' +
                     check_skypilot_config_msg)
         reasons = []
+        hints = []
+        success = False
         for context in existing_allowed_contexts:
             try:
                 check_result = kubernetes_utils.check_credentials(context)
                 if check_result[0]:
-                    return check_result
-                reasons.append(f'{context}: {check_result[1]}')
+                    success = True
+                    hints.append(f'Context {context}: {check_result[1]}')
+                else:
+                    reasons.append(f'Context {context}: {check_result[1]}')
             except Exception as e:  # pylint: disable=broad-except
                 return (False, f'Credential check failed for {context}: '
                         f'{common_utils.format_exception(e)}')
+        if success:
+            return (True, cls._format_credential_check_results(hints, reasons))
         return (False, 'Failed to find available context with working '
                 'credentials. Details:\n' + '\n'.join(reasons))
 
+    @classmethod
+    def _format_credential_check_results(cls, hints: List[str],
+                                         reasons: List[str]) -> str:
+        """Format credential check results with hints and reasons.
+
+        Args:
+            hints: List of successful context check messages.
+            reasons: List of failed context check reasons.
+
+        Returns:
+            A formatted string containing hints and by failure reasons.
+        """
+        message_parts = []
+        if len(hints) == 1 and not reasons:
+            return hints[0]
+        if hints:
+            message_parts.append(f'\n{cls._INDENT_PREFIX}  ' +
+                                 f'\n{cls._INDENT_PREFIX}  '.join(hints))
+        if reasons:
+            if hints:
+                message_parts.append('\n')
+            message_parts.append(
+                f'\n{cls._INDENT_PREFIX}Unavailable contexts (remove from '
+                '"allowed_contexts" config if permanently unavailable): '
+                f'\n{cls._INDENT_PREFIX}  ' +
+                f'\n{cls._INDENT_PREFIX}  '.join(reasons))
+        return ''.join(message_parts)
+
     def get_credential_file_mounts(self) -> Dict[str, str]:
         if os.path.exists(os.path.expanduser(CREDENTIAL_PATH)):
             # Upload kubeconfig to the default path to avoid having to set
diff --git a/sky/provision/kubernetes/utils.py b/sky/provision/kubernetes/utils.py
index 594e3752b41..62edc9f3863 100644
--- a/sky/provision/kubernetes/utils.py
+++ b/sky/provision/kubernetes/utils.py
@@ -1,6 +1,7 @@
 """Kubernetes utilities for SkyPilot."""
 import dataclasses
 import functools
+import inspect
 import json
 import math
 import os
@@ -113,7 +114,8 @@
 
 def _retry_on_error(max_retries=DEFAULT_MAX_RETRIES,
                     retry_interval=DEFAULT_RETRY_INTERVAL_SECONDS,
-                    resource_type: Optional[str] = None):
+                    resource_type: Optional[str] = None,
+                    context_param: str = 'context'):
     """Decorator to retry Kubernetes API calls on transient failures.
 
     Args:
@@ -121,6 +123,8 @@ def _retry_on_error(max_retries=DEFAULT_MAX_RETRIES,
         retry_interval: Initial seconds to wait between retries
         resource_type: Type of resource being accessed (e.g. 'node', 'pod').
             Used to provide more specific error messages.
+        context_param: Name of the parameter that contains the Kubernetes
+            context. Empty means no context parameter. Defaults to 'context'.
     """
 
     def decorator(func):
@@ -131,6 +135,17 @@ def wrapper(*args, **kwargs):
             backoff = common_utils.Backoff(initial_backoff=retry_interval,
                                            max_backoff_factor=3)
 
+            # try to capture the kubernetes context from parameters
+            if context_param:
+                context = kwargs.get(context_param)
+                if context is None and len(args) > 0:
+                    sig = inspect.signature(func)
+                    param_names = list(sig.parameters.keys())
+                    if context_param in param_names:
+                        context_idx = param_names.index(context_param)
+                        if context_idx < len(args):
+                            context = args[context_idx]
+
             for attempt in range(max_retries):
                 try:
                     return func(*args, **kwargs)
@@ -156,6 +171,8 @@ def wrapper(*args, **kwargs):
                 if resource_type else ''
             debug_cmd = f' To debug, run: kubectl get {resource_type}s' \
                 if resource_type else ''
+            if context:
+                debug_cmd += f' --context {context}'
 
             if isinstance(last_exception, kubernetes.max_retry_error()):
                 error_msg = f'Timed out{resource_msg} from Kubernetes cluster.'
@@ -615,9 +632,6 @@ def check_instance_fits(context: Optional[str],
         Optional[str]: Error message if the instance does not fit.
     """
 
-    # TODO(zhwu): this should check the node for specific context, instead
-    # of the default context to make failover fully functional.
-
     def check_cpu_mem_fits(candidate_instance_type: 'KubernetesInstanceType',
                            node_list: List[Any]) -> Tuple[bool, Optional[str]]:
         """Checks if the instance fits on the cluster based on CPU and memory.


Code review comments:
diff_hunk: @@ -131,6 +135,17 @@ def wrapper(*args, **kwargs):
             backoff = common_utils.Backoff(initial_backoff=retry_interval,
                                            max_backoff_factor=3)
 
+            # try to capture the kubernetes context from parameters
+            if context_param:
+                context = kwargs.get(context_param)
+                if context is None and len(args) > 0:
+                    sig = inspect.signature(func)
path: sky/provision/kubernetes/utils.py
text: @user1:
This is quite hacky. How about we just enforce `context` to be appear in kwargs, and all wrapped function to take `context` as kwargs? `def wrapped_function(myarg1, *, context):`

@author:
Sounds good, is there any sound solution to enforce `context` to be appear in kwargs?

@user1:
How about we just `assert` it in the decorator function, which will only be triggerred when the function is being called, but it might be fine.

diff_hunk: @@ -169,6 +180,38 @@ def existing_allowed_contexts(cls) -> List[str]:
         cls._log_skipped_contexts_once(tuple(skipped_contexts))
         return existing_contexts
 
+    @classmethod
+    def _log_unavailable_context(cls,
+                                 context: str,
+                                 reason: Optional[str] = None,
+                                 silent: bool = False) -> None:
path: sky/clouds/kubernetes.py
text: @user1:
Seems `slient` is not used. Should we avoid it to keep the API simple?

@author:
good catch

diff_hunk: @@ -169,6 +180,38 @@ def existing_allowed_contexts(cls) -> List[str]:
         cls._log_skipped_contexts_once(tuple(skipped_contexts))
         return existing_contexts
 
+    @classmethod
+    def _log_unavailable_context(cls,
+                                 context: str,
+                                 reason: Optional[str] = None,
+                                 silent: bool = False) -> None:
+        """log a Kubernetes context as unavailable.
path: sky/clouds/kubernetes.py
text: @user1:
```suggestion
        """Logs a Kubernetes context as unavailable.
```