Repository: skypilot-org/skypilot
Pull request: [k8s] Fix GPU count detection from autoscaler YAML
Closes #2608. 

Our resource detection from pod spec puts a minimum of 1 on CPU resource so that fractional CPU sized clusters can also be launched on k8s clusters. However, this was being incorrectly applied to GPU resources too, causing all clusters to be initialized with 1 GPU resource.

Tested (run the relevant ones):

- [x] Code formatting: `bash format.sh`
- [x] Any manual or new tests for this PR - `sky launch --cloud kubernetes -- ray status` now does not show GPUs.
- [x] k8s smoke tests: `pytest tests/test_smoke.py--kubernetes -k "not TestStorageWithCredentials"`


Problem statement:
[k8s] Ray cluster is incorrectly initialized with `gpu` resource
On a CPU-only kubernetes cluster, the ray cluster shows 1 GPU available, which is incorrect. Ssh into node and run `ray status`:
```
(base) sky@test-2ea4-ray-head:~$ ray status
======== Autoscaler status: 2023-09-25 23:08:16.304098 ========
Node status
---------------------------------------------------------------
Healthy:
 1 ray_head_default
Pending:
 (no pending nodes)
Recent failures:
 (no failures)

Resources
---------------------------------------------------------------
Usage:
 0.0/2.0 CPU
 0.0/1.0 GPU
 0B/2.00GiB memory
 0B/476.84MiB object_store_memory

Demands:
 (no resource demands)
```

Repro:

1. `sky local up` - this creates a CPU only cluster
2. `sky launch -c test --cloud kubernetes -y`
3. `ssh test`
4. `ray status`

Hints from issue discussion:

Commit message: fix gpu count detection in yaml

Patch to review:
diff --git a/sky/skylet/providers/kubernetes/config.py b/sky/skylet/providers/kubernetes/config.py
index 2ab466f03ed..f4c1623f1a2 100644
--- a/sky/skylet/providers/kubernetes/config.py
+++ b/sky/skylet/providers/kubernetes/config.py
@@ -128,11 +128,16 @@ def get_resource(container_resources: Dict[str, Any],
     # float('inf') means there's no limit set
     res_count = request if limit == float('inf') else limit
     # Convert to int since Ray autoscaler expects int.
-    # Cap the minimum resource to 1 because if resource count is set to 0,
-    # (e.g., when request=0.5), ray will not be able to schedule any tasks.
     # We also round up the resource count to the nearest integer to provide the
     # user at least the amount of resource they requested.
-    return max(1, math.ceil(res_count))
+    rounded_count = math.ceil(res_count)
+    if resource_name == 'cpu':
+        # For CPU, we set minimum count to 1 because if CPU count is set to 0,
+        # (e.g., when request=0.5), ray will not be able to schedule any tasks.
+        return max(1, rounded_count)
+    else:
+        # For GPU and memory, return the rounded count.
+        return rounded_count
 
 
 def _get_resource(container_resources: Dict[str, Any], resource_name: str,
@@ -144,7 +149,7 @@ def _get_resource(container_resources: Dict[str, Any], resource_name: str,
 
     Args:
         container_resources: Container's resource field.
-        resource_name: One of 'cpu', 'gpu' or memory.
+        resource_name: One of 'cpu', 'gpu' or 'memory'.
         field_name: One of 'requests' or 'limits'.
 
     Returns:
diff --git a/tests/test_smoke.py b/tests/test_smoke.py
index 8f8bd78487e..f00c6001f0b 100644
--- a/tests/test_smoke.py
+++ b/tests/test_smoke.py
@@ -2644,7 +2644,6 @@ def test_gcp_zero_quota_failover():
 
 
 # ------- Testing user ray cluster --------
-@pytest.mark.no_kubernetes  # Kubernetes does not support sky status -r yet.
 def test_user_ray_cluster(generic_cloud: str):
     name = _get_cluster_name()
     test = Test(


Code review comments:
diff_hunk: @@ -128,11 +128,16 @@ def get_resource(container_resources: Dict[str, Any],
     # float('inf') means there's no limit set
     res_count = request if limit == float('inf') else limit
     # Convert to int since Ray autoscaler expects int.
-    # Cap the minimum resource to 1 because if resource count is set to 0,
-    # (e.g., when request=0.5), ray will not be able to schedule any tasks.
     # We also round up the resource count to the nearest integer to provide the
     # user at least the amount of resource they requested.
-    return max(1, math.ceil(res_count))
+    rounded_count = math.ceil(res_count)
+    if resource_name == 'cpu':
+        # For CPU, we set minimum count to 1 because if CPU count is set to 0,
+        # (e.g., when request=0.5), ray will not be able to schedule any tasks.
path: sky/skylet/providers/kubernetes/config.py
text: @user1:
From the comment, `request=0.5`, what is the `request` referring to? If it's referring to the `request` variable used in this function, `rounded_count`(CPU count) will be set to 1, not 0 when `request=0.5`. If it's not the variable, I think we should elaborate a bit more in the comment to show that it's not referring to the variable :)

@author:
Good catch - clarified that it's referring to when skypilot task requests something like `--cpus 0.5`.