Repository: skypilot-org/skypilot
Pull request #2753 (2023-11-03 02:23:10)

Docs: add a hint to customizing spot controller.
<!-- Describe the changes in this PR -->

This has come up quite a few times so adding a hint.

<!-- 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):

- [ ] Code formatting: `bash format.sh`
- [x] Any manual or new tests for this PR (please specify below): rendered locally
- [ ] 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: `bash tests/backward_comaptibility_tests.sh`



Patch:
diff --git a/docs/source/examples/spot-jobs.rst b/docs/source/examples/spot-jobs.rst
index c62246f97..83e01fa40 100644
--- a/docs/source/examples/spot-jobs.rst
+++ b/docs/source/examples/spot-jobs.rst
@@ -275,26 +275,34 @@ you can still tear it down manually with
 Customizing spot controller resources
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-You may customize the resources of the spot controller for the following reasons:
+You may want to customize the resources of the spot controller for several reasons:
 
-1. Enforcing the spot controller to run on a specific location. (Default: cheapest location)
-2. Changing the maximum number of spot jobs that can be run concurrently. (Default: 16)
-3. Changing the disk_size of the spot controller to store more logs. (Default: 50GB)
+1. Use a lower-cost controller (if you have a low number of concurrent spot jobs).
+2. Enforcing the spot controller to run on a specific location. (Default: cheapest location)
+3. Changing the maximum number of spot jobs that can be run concurrently, which is 2x the vCPUs of the controller. (Default: 16)
+4. Changing the disk_size of the spot controller to store more logs. (Default: 50GB)
 
 To achieve the above, you can specify custom configs in :code:`~/.sky/config.yaml` with the following fields:
 
 .. code-block:: yaml
 
   spot:
+    # NOTE: these settings only take effect for a new spot controller, not if
+    # you have an existing one.
     controller:
       resources:
-        # All configs below are optional
-        # 1. Specify the location of the spot controller.
+        # All configs below are optional.
+        # Specify the location of the spot controller.
         cloud: gcp
         region: us-central1
-        # 2. Specify the maximum number of spot jobs that can be run concurrently.
+        # Specify the maximum number of spot jobs that can be run concurrently.
         cpus: 4+  # number of vCPUs, max concurrent spot jobs = 2 * cpus
-        # 3. Specify the disk_size of the spot controller.
+        # Specify the disk_size in GB of the spot controller.
         disk_size: 100
 
 The :code:`resources` field has the same spec as a normal SkyPilot job; see `here <https://skypilot.readthedocs.io/en/latest/reference/yaml-spec.html>`__.
+
+.. note::
+  These settings will not take effect if you have an existing controller (either
+  stopped or live).  For them to take effect, tear down the existing controller
+  first, which requires all in-progress spot jobs to finish or be canceled.
diff --git a/sky/utils/schemas.py b/sky/utils/schemas.py
index 0267b7a26..20453c49e 100644
--- a/sky/utils/schemas.py
+++ b/sky/utils/schemas.py
@@ -323,7 +323,14 @@ def get_config_schema():
                             'type': 'object',
                             'required': [],
                             'additionalProperties': {
-                                'type': 'string',
+                                'anyOf': [
+                                    {
+                                        'type': 'string'
+                                    },
+                                    {
+                                        'type': 'null'
+                                    },
+                                ]
                             }
                         }]
                     },


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index b4053523d..52ff17676 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -58,6 +58,31 @@ def test_empty_config(monkeypatch, tmp_path) -> None:
     _check_empty_config()
 
 
+def test_valid_null_proxy_config(monkeypatch, tmp_path) -> None:
+    """Test that the config is not loaded if the config file is empty."""
+    with open(tmp_path / 'valid.yaml', 'w') as f:
+        f.write(f"""\
+        aws:
+            instance_tags:
+                mytag: myvalue
+            ssh_proxy_command:
+                eu-west-1: null
+                us-east-1: null
+            use_internal_ips: true
+            vpc_name: abc
+
+        spot:
+            controller:
+                resources:
+                    disk_size: 256
+        """)
+    monkeypatch.setattr(skypilot_config, 'CONFIG_PATH', tmp_path / 'valid.yaml')
+    _reload_config()
+    proxy_config = skypilot_config.get_nested(
+        ('aws', 'ssh_proxy_command', 'eu-west-1'), 'default')
+    assert proxy_config is None, proxy_config
+
+
 def test_invalid_field_config(monkeypatch, tmp_path) -> None:
     """Test that the config is not loaded if the config file contains unknown field."""
     config_path = tmp_path / 'invalid.yaml'