Repository: skypilot-org/skypilot
Pull request: [k8s] Better GPU Label Formatter Support for CoreWeave
<!-- Describe the changes in this PR -->
Fixes #5628

so if I add the invalid node labels to a GKE cluster, then I can properly see that the coreweave gpu label formatter is being picked up. For obvious reasons, `show-gpus` fail (as the cluster doesn't actually have H100 gpus with that label).


<!-- 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: install pre-commit (auto-check on commit) or `bash format.sh`
- [ ] Any manual or new tests for this PR (please specify below)
- [ ] All smoke tests: `/smoke-test` (CI) or `pytest tests/test_smoke.py` (local)
- [ ] Relevant individual tests: `/smoke-test -k test_name` (CI) or `pytest tests/test_smoke.py::test_name` (local)
- [ ] Backward compatibility: `/quicktest-core` (CI) or `pytest tests/smoke_tests/test_backward_compat.py` (local)

<!-- CI commands (/-prefixed) can only be triggered by repo members -->


Problem statement:
[k8s] Handle invalid formats in GPU label formatter detection
A user had a Coreweave cluster with both GKE and CoreWeave labels
```
                    cloud.google.com/gke-accelerator=H100_NVLINK_80GB
                    gpu.nvidia.com/class=H100_NVLINK_80GB
                    gpu.nvidia.com/count=8
                    gpu.nvidia.com/model=H100_NVLINK_80GB
                    gpu.nvidia.com/vram=81
```

Since the GKE label value is in the wrong format (it typically has `-` as a separator and starts with `nvidia`), the user got:
```
$ sky show-gpus --cloud kubernetes
Invalid accelerator name in GKE cluster: H100_NVLINK_80GB
```

While it's very odd that the cluster comes with both labels, our label formatter should be robust to such setups. If a label format is detected but has invalid values, we should raise a warning and fall back to the next available label format.

Hints from issue discussion:
@romilbhardwaj @kyuds I was able to successfully launch a SkyPilot cluster on a CoreWeave Kubernetes cluster, but I had to do a few hacky things. I'm sure you can find a better solution for SkyPilot.

1. I ran into a similar invalid GKE cluster accelerator name error, so I put the `CoreWeaveLabelFormatter` at the start of the `LABEL_FORMATTER_REGISTRY`
2. I ran into an issue where after launching the cluster, launching would hang indefinitely. Checking the pod would yield an error saying "Failed to create pod sandbox: rpc error: code = Unknown desc = failed to create containerd task: no runtime for "containerd" is configured" The only runtime class in the cluster was one named `nvidia`, but used the `containerd` handler. I didn't want to break the cluster for anyone else, so I created a new runtime class called `nvidia-fixed` with the `nvidia` handler. I had to change the code in a couple places to set `...['runtimeClassName'] = 'nvidia-fixed'`.

After making those changes, launching worked, though there was a `414 Request-URI Too Large` error at the end that didn't seem to affect the actual launching, and I had to wait a bit before successfully SSHing into the cluster.
sorry was a bit backlogged. I have a preliminary pr with an attempt to address this issue, but I don't have a coreweave cluster to test the second point mentioned by @bradhilton ... In theory this should work though

Commit message: simple fix

Patch to review:
diff --git a/sky/provision/kubernetes/utils.py b/sky/provision/kubernetes/utils.py
index 0b792bfaad5..3dbfd18f6a1 100644
--- a/sky/provision/kubernetes/utils.py
+++ b/sky/provision/kubernetes/utils.py
@@ -460,6 +460,14 @@ def get_accelerator_from_label_value(cls, value: str) -> str:
             raise ValueError(
                 f'Invalid accelerator name in GKE cluster: {value}')
 
+    @classmethod
+    def validate_label_value(cls, value: str) -> Tuple[bool, str]:
+        try:
+            _ = cls.get_accelerator_from_label_value(value)
+            return True, ''
+        except ValueError as e:
+            return False, str(e)
+
 
 class GFDLabelFormatter(GPULabelFormatter):
     """GPU Feature Discovery label formatter
@@ -564,17 +572,29 @@ def detect_gpu_label_formatter(
         for label, value in node.metadata.labels.items():
             node_labels[node.metadata.name].append((label, value))
 
-    label_formatter = None
-
     # Check if the node labels contain any of the GPU label prefixes
     for lf in LABEL_FORMATTER_REGISTRY:
+        skip = False
         for _, label_list in node_labels.items():
-            for label, _ in label_list:
+            for label, value in label_list:
                 if lf.match_label_key(label):
-                    label_formatter = lf()
-                    return label_formatter, node_labels
+                    valid, reason = lf.validate_label_value(value)
+                    if valid:
+                        return lf(), node_labels
+                    else:
+                        logger.warning(f'Gpu label {label} matched for label '
+                                       f'formatter {lf.__class__.__name__}, '
+                                       f'but has invalid value {value}. '
+                                       f'Reason: {reason}. '
+                                       'Skipping...')
+                        skip = True
+                        break
+            if skip:
+                break
+        if skip:
+            continue
 
-    return label_formatter, node_labels
+    return None, node_labels
 
 
 class Autoscaler:


Code review comments:
diff_hunk: @@ -564,17 +572,29 @@ def detect_gpu_label_formatter(
         for label, value in node.metadata.labels.items():
             node_labels[node.metadata.name].append((label, value))
 
-    label_formatter = None
-
     # Check if the node labels contain any of the GPU label prefixes
     for lf in LABEL_FORMATTER_REGISTRY:
+        skip = False
         for _, label_list in node_labels.items():
-            for label, _ in label_list:
+            for label, value in label_list:
                 if lf.match_label_key(label):
-                    label_formatter = lf()
-                    return label_formatter, node_labels
+                    valid, reason = lf.validate_label_value(value)
+                    if valid:
+                        return lf(), node_labels
+                    else:
+                        logger.warning(f'Gpu label {label} matched for label '
path: sky/provision/kubernetes/utils.py
text: @user1:
```suggestion
                        logger.warning(f'GPU label {label} matched for label '
```

Edit: fix typo

@author:
this works? like not having the f-string?

@author:
changed Gpu to all caps