Repository: skypilot-org/skypilot
Pull request #6248 (2025-07-12 02:00:24)

take use_spot into account when comparing Resources
In `RetryingVmProvisioner`, we store a set of `_blocked_resources`, which gets added to each time a resource fails to be provisioned.

This set is used to filter resources that are indicated to be unavailable from previous attempts:
https://github.com/skypilot-org/skypilot/blob/784b2e329d9d58c00caaa94f53d437f6158f8c7d/sky/backends/cloud_vm_ray_backend.py#L1396-L1415

The issue lies in the `should_be_blocked_by()` call. It doesn't take into account the `use_spot` property of the resources.

This means, if let's say you have this resources config:
```yaml
resources:
  # this forces the optimizer to choose g6.4xlarge for both spot and on-demand
  cpus: 16+
  any_of:
    - infra: aws/us-west-2/us-west-2c
      use_spot: true
      accelerators: "L4:1"
    - infra: aws/us-west-2/us-west-2c
      accelerators: "L4:1"
```

and you failed to provision a `g6.4xlarge` spot instance in `aws/us-west-2/us-west-2c` due to AWS running out of capacity, then your on demand `g6.4xlarge` which you expected to fall back to, would be blocked, and we wouldn't try to provision it, although it could've been provisioned successfully.

This PR fixes it by taking into account `use_spot` when comparing the two resources in `should_be_blocked_by`.

I added unit tests in `test_resources.py`.

Tested (run the relevant ones):

- [x] Code formatting: install pre-commit (auto-check on commit) or `bash format.sh`
- [x] Any manual or new tests for this PR (please specify below)
- [ ] All smoke tests: `/smoke-test` (CI) or `pytest tests/test_smoke.py` (local)
- [x] 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 -->



Patch:
diff --git a/sky/dashboard/src/components/clusters.jsx b/sky/dashboard/src/components/clusters.jsx
index ae424f606..93d5be93f 100755
--- a/sky/dashboard/src/components/clusters.jsx
+++ b/sky/dashboard/src/components/clusters.jsx
@@ -354,7 +354,7 @@ export function Clusters() {
 
   return (
     <>
-      <div className="flex items-center justify-between mb-4 h-5">
+      <div className="flex items-center justify-between mb-2 h-5">
         <div className="text-base flex items-center">
           <Link
             href="/clusters"
@@ -363,7 +363,16 @@ export function Clusters() {
             Sky Clusters
           </Link>
 
-          <div className="flex items-center ml-6 space-x-3">
+          <FilterDropdown
+            propertyList={PROPERTY_OPTIONS}
+            valueList={optionValues}
+            setFilters={setFilters}
+            updateURLParams={updateURLParams}
+            placeholder="Filter clusters"
+          />
+        </div>
+        <div className="flex items-center space-x-4">
+          <div className="flex items-center">
             <label className="flex items-center cursor-pointer">
               <input
                 type="checkbox"
@@ -387,30 +396,22 @@ export function Clusters() {
               </span>
             </label>
           </div>
-
-          <FilterDropdown
-            propertyList={PROPERTY_OPTIONS}
-            valueList={optionValues}
-            setFilters={setFilters}
-            updateURLParams={updateURLParams}
-            placeholder="Filter clusters"
-          />
-        </div>
-        <div className="flex items-center">
-          {loading && (
-            <div className="flex items-center mr-2">
-              <CircularProgress size={15} className="mt-0" />
-              <span className="ml-2 text-gray-500">Loading...</span>
-            </div>
-          )}
-          <button
-            onClick={handleRefresh}
-            disabled={loading}
-            className="text-sky-blue hover:text-sky-blue-bright flex items-center"
-          >
-            <RotateCwIcon className="h-4 w-4 mr-1.5" />
-            {!isMobile && <span>Refresh</span>}
-          </button>
+          <div className="flex items-center">
+            {loading && (
+              <div className="flex items-center mr-2">
+                <CircularProgress size={15} className="mt-0" />
+                <span className="ml-2 text-gray-500">Loading...</span>
+              </div>
+            )}
+            <button
+              onClick={handleRefresh}
+              disabled={loading}
+              className="text-sky-blue hover:text-sky-blue-bright flex items-center"
+            >
+              <RotateCwIcon className="h-4 w-4 mr-1.5" />
+              {!isMobile && <span>Refresh</span>}
+            </button>
+          </div>
         </div>
       </div>
 
@@ -1129,15 +1130,15 @@ const FilterDropdown = ({
 
   return (
     <>
-      <div className="flex flex-row ml-4 mr-2 border border-gray-300 rounded-md">
-        <div className="border-r">
+      <div className="flex flex-row ml-4 mr-2 border border-gray-300 rounded-md overflow-hidden">
+        <div className="border-r border-gray-300">
           <Select
             onValueChange={(value) => setPropertValue(value)}
             value={propertyValue}
           >
             <SelectTrigger
               aria-label="Node"
-              className="focus:ring-0 focus:ring-offset-0 border-none w-36"
+              className="focus:ring-0 focus:ring-offset-0 border-none rounded-none w-32 h-8"
             >
               <SelectValue placeholder="Select Property" />
             </SelectTrigger>
@@ -1150,7 +1151,7 @@ const FilterDropdown = ({
             </SelectContent>
           </Select>
         </div>
-        <div className="relative">
+        <div className="relative flex-1">
           <input
             type="text"
             ref={inputRef}
@@ -1180,7 +1181,7 @@ const FilterDropdown = ({
                 inputRef.current.blur();
               }
             }}
-            className="h-10 w-32 sm:w-96 px-3 pr-8 text-sm rounded-md outline-none my-auto"
+            className="h-8 w-32 sm:w-96 px-3 pr-8 text-sm border-none rounded-none focus:ring-0 focus:outline-none"
             autoComplete="off"
           />
           {value && (
@@ -1265,7 +1266,7 @@ const Filters = ({ filters = [], setFilters, updateURLParams }) => {
 
   return (
     <>
-      <div className="flex items-center gap-4 p-2">
+      <div className="flex items-center gap-4 py-1 px-2">
         <div className="flex flex-wrap items-content gap-2">
           {filters.map((filter, _index) => (
             <FilterItem
@@ -1279,7 +1280,7 @@ const Filters = ({ filters = [], setFilters, updateURLParams }) => {
             <>
               <button
                 onClick={clearFilters}
-                className="rounded-full px-4 py-1 text-gray-700 bg-gray-200 hover:bg-gray-300"
+                className="rounded-full px-4 py-1 text-sm text-gray-700 bg-gray-200 hover:bg-gray-300"
               >
                 Clear filters
               </button>
@@ -1294,16 +1295,16 @@ const Filters = ({ filters = [], setFilters, updateURLParams }) => {
 const FilterItem = ({ filter, onRemove }) => {
   return (
     <>
-      <div className="flex items-center gap-1 text-blue-600 bg-blue-100 px-2 rounded-full">
-        <div className="flex items-center gap-1 px-2 py-1 rounded-l-lg">
+      <div className="flex items-center gap-1 text-blue-600 bg-blue-100 px-1 py-1 rounded-full text-sm">
+        <div className="flex items-center gap-1 px-2 rounded-l-lg">
           <span>{`${filter.property} `}</span>
-          <span>{filter.operator}</span>
-          <span className="font-bold">{` ${filter.value}`}</span>
+          <span>{`${filter.operator} `}</span>
+          <span>{` ${filter.value}`}</span>
         </div>
 
         <button
           onClick={() => onRemove()}
-          className="p-1 transform text-gray-400 hover:text-gray-600 bg-blue-500 hover:bg-blue-600 rounded-full flex flex-col items-center"
+          className="p-0.5 transform text-gray-400 hover:text-gray-600 bg-blue-500 hover:bg-blue-600 rounded-full flex flex-col items-center"
           title="Clear filter"
         >
           <svg
diff --git a/sky/dashboard/src/components/jobs.jsx b/sky/dashboard/src/components/jobs.jsx
index 0b44983d2..eeb8c4d61 100755
--- a/sky/dashboard/src/components/jobs.jsx
+++ b/sky/dashboard/src/components/jobs.jsx
@@ -853,7 +853,7 @@ export function ManagedJobsTable({
               <button
                 key={status}
                 onClick={() => handleStatusClick(status)}
-                className={`px-3 py-1 rounded-full flex items-center space-x-2 ${
+                className={`px-3 py-0.5 rounded-full flex items-center space-x-2 ${
                   isStatusHighlighted(status) ||
                   selectedStatuses.includes(status)
                     ? getBadgeStyle(status)
diff --git a/sky/resources.py b/sky/resources.py
index 69f3f6280..98a69c144 100644
--- a/sky/resources.py
+++ b/sky/resources.py
@@ -1730,6 +1730,8 @@ class Resources:
         if (blocked.accelerators is not None and
                 self.accelerators != blocked.accelerators):
             is_matched = False
+        if blocked.use_spot is not None and self.use_spot != blocked.use_spot:
+            is_matched = False
         return is_matched
 
     def is_empty(self) -> bool:


Test patch:
diff --git a/tests/unit_tests/test_resources.py b/tests/unit_tests/test_resources.py
index ea13d7c2a..7a3f92d70 100644
--- a/tests/unit_tests/test_resources.py
+++ b/tests/unit_tests/test_resources.py
@@ -1047,3 +1047,92 @@ def test_priority_to_yaml_and_load(resources_kwargs, expected_yaml_config):
     assert loaded_r.cpus == r.cpus
     if 'accelerators' in expected_yaml_config:
         assert loaded_r.accelerators == r.accelerators
+
+
+@pytest.mark.parametrize(
+    'r_kwargs, blocked_kwargs, expected',
+    [
+        # All fields match
+        ({
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.2xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+            'use_spot': True
+        }, {
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.2xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+            'use_spot': True
+        }, True),
+        # use_spot mismatch
+        ({
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.2xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+            'use_spot': True
+        }, {
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.2xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+            'use_spot': False
+        }, False),
+        # cloud mismatch
+        ({
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.2xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+        }, {
+            'cloud': clouds.GCP(),
+            'instance_type': 'g2-standard-4',
+            'region': 'us-east4',
+            'zone': 'us-east4-c',
+            'accelerators': {
+                'L4': 1
+            },
+        }, False),
+        # instance_type mismatch
+        ({
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.2xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+            'use_spot': True
+        }, {
+            'cloud': clouds.AWS(),
+            'instance_type': 'p3.8xlarge',
+            'region': 'us-west-2',
+            'zone': 'us-west-2a',
+            'accelerators': {
+                'V100': 1
+            },
+            'use_spot': True
+        }, False),
+    ])
+def test_should_be_blocked_by(r_kwargs, blocked_kwargs, expected):
+    """Test should_be_blocked_by method."""
+    r = Resources(**r_kwargs)
+    blocked = Resources(**blocked_kwargs)
+    assert r.should_be_blocked_by(blocked) == expected