Repository: skypilot-org/skypilot
Pull request: Make sky exec submit jobs for inline commands; guard Azure disabled subscription error.
Tested
- rendered html
- `sky exec balsa -d --workdir=. <my command>`
- `sky gpunode` previously failed due to the unhandled Azure error; now it fails over to AWS.

Fixes #677.

Problem statement:
UX: Change the default behavior of `sky exec -- inline cmd` to submit job
The current default behavior of inline cmd without specifying the GPU requirements is the same as `ssh -c cmd`, and will not submit the task to the job queue. It is different from any other behaviors of `sky exec` and `sky launch`. It may cause confusion for the users, e.g. Daniel used the inline command to run his training job without specifying the GPU requirement, and found the ctrl-c did not kill the job entirely.

Proposal:
1. Submit all commands run with `sky exec` to the job queue.
2. For interactive usage (e.g. `gpustat` or `htop`), ask the user to `ssh` into the cluster.

Pros:
1. The behavior of all `sky exec` will be consistent without confusion.
2. We can make the user be used to the `sky exec` and `sky cancel` pipeline, without finding different behaviors when not specifying the `--gpus`. Also, we can force the user to utilize the `sky cancel` which will kill the processes without leaving defunct processes (compared to ctrl-c).
3. Asking the user to do any interactive usage by `ssh` into the cluster, so that we can let the user take the responsibility for those processes that are not killed entirely caused by ctrl-c (e.g. PyTorch DataParallel).

Cons:
1. Simple commands will have extra overheads if they are executed with `sky exec`. (It can be resolved by asking the user to ssh into the cluster and run those commands)

Hints from issue discussion:
The pros are very appealing!  A few thoughts:
1. Can we ask Daniel which behavior would he expect/prefer?
2. This  means interactive commands like `sky exec clus htop` will occupy the `sky queue` table.  Which may be ok, if we update docs to discourage this behavior (and use `ssh clus htop`, with some flags, etc.).
Looks like Daniel is confused by `sky exec` + inline command not being submitted as a job.  +1 to doing this change.

Related issue: `sky exec clus -d cmd` seems to not respect `-d` currently.

Commit message: YAPF

Patch to review:
diff --git a/docs/source/examples/iterative-dev-project.rst b/docs/source/examples/iterative-dev-project.rst
index 9d2722c9df4..62fde411847 100644
--- a/docs/source/examples/iterative-dev-project.rst
+++ b/docs/source/examples/iterative-dev-project.rst
@@ -32,9 +32,6 @@ To run a command or a script on the cluster, use :code:`sky exec`:
   $ # Workdir contents are synced to the cluster (~/sky_workdir/).
   $ sky exec dev -- python train.py
 
-  $ # Run a command.
-  $ sky exec dev -- gpustat -i
-
 Alternatively, the user can directly :code:`ssh` into the cluster's nodes and run commands:
 
 .. code-block:: console
diff --git a/docs/source/getting-started/quickstart.rst b/docs/source/getting-started/quickstart.rst
index b248f9cd689..9dd41cc1d75 100644
--- a/docs/source/getting-started/quickstart.rst
+++ b/docs/source/getting-started/quickstart.rst
@@ -103,9 +103,9 @@ Bash commands are also supported, such as:
 
 .. code-block:: console
 
-  $ sky exec mycluster -- gpustat -i
-  $ sky exec mycluster -- ls -l
-  $ sky exec mycluster htop
+  $ sky exec mycluster python train.py
+
+For interactive/monitoring commands, such as ``htop`` or ``gpustat -i``, use ``ssh`` instead (see below) to avoid job submission overheads.
 
 
 View all clusters
diff --git a/sky/backends/cloud_vm_ray_backend.py b/sky/backends/cloud_vm_ray_backend.py
index 05663be7828..688deaba3f7 100644
--- a/sky/backends/cloud_vm_ray_backend.py
+++ b/sky/backends/cloud_vm_ray_backend.py
@@ -542,8 +542,8 @@ def _update_blocklist_on_azure_error(self, region, zones, stdout, stderr):
         errors = [
             s.strip()
             for s in stdout_splits + stderr_splits
-            if ('Exception Details:' in s.strip() or
-                'InvalidTemplateDeployment' in s.strip())
+            if ('Exception Details:' in s.strip() or 'InvalidTemplateDeployment'
+                in s.strip() or '(ReadOnlyDisabledSubscription)' in s.strip())
         ]
         if not errors:
             logger.info('====== stdout ======')
@@ -558,7 +558,11 @@ def _update_blocklist_on_azure_error(self, region, zones, stdout, stderr):
         logger.warning(f'Got error(s) in {region.name}:')
         messages = '\n\t'.join(errors)
         logger.warning(f'{style.DIM}\t{messages}{style.RESET_ALL}')
-        self._blocked_regions.add(region.name)
+        if any('(ReadOnlyDisabledSubscription)' in s for s in errors):
+            for r in sky.Azure.regions():
+                self._blocked_regions.add(r.name)
+        else:
+            self._blocked_regions.add(region.name)
 
     def _update_blocklist_on_error(self, cloud, region, zones, stdout,
                                    stderr) -> None:
@@ -690,7 +694,7 @@ def _yield_region_zones(self, to_provision: 'resources_lib.Resources',
 
             assert cluster_status == global_user_state.ClusterStatus.INIT
             message = (f'Failed to launch cluster {cluster_name!r} '
-                       f'(previous status: {cluster_status.value})'
+                       f'(previous status: {cluster_status.value}) '
                        f'with the original resources: {to_provision}.')
             logger.error(message)
             # We attempted re-launching a previously INIT cluster with the
@@ -905,9 +909,9 @@ def _retry_region_zones(self,
             CloudVmRayBackend().teardown_no_lock(handle,
                                                  terminate=need_terminate)
 
-        message = ('Failed to acquire resources in all regions/zones'
-                   f' (requested {to_provision}).'
-                   ' Try changing resource requirements or use another cloud.')
+        message = ('Failed to acquire resources in all regions/zones of '
+                   f'{to_provision.cloud}. '
+                   'Try changing resource requirements or use another cloud.')
         logger.error(message)
         raise exceptions.ResourcesUnavailableError()
 
@@ -1062,7 +1066,7 @@ def provision_with_retries(
                 logger.warning(
                     f'\n{style.BRIGHT}Provision failed for {num_nodes}x '
                     f'{to_provision}. Trying other launchable resources '
-                    f'(if any)...{style.RESET_ALL}')
+                    f'(if any).{style.RESET_ALL}')
                 if not cluster_exists:
                     # Add failed resources to the blocklist, only when it
                     # is in fallback mode.
diff --git a/sky/cli.py b/sky/cli.py
index 4cfc53fcd23..66816f5b69c 100644
--- a/sky/cli.py
+++ b/sky/cli.py
@@ -637,7 +637,7 @@ def launch(
               '-d',
               default=False,
               is_flag=True,
-              help='If True, run setup first (blocking), '
+              help='If True, run workdir syncing first (blocking), '
               'then detach from the job\'s execution.')
 @click.option(
     '--workdir',
@@ -683,21 +683,6 @@ def exec(
     If ENTRYPOINT points to a valid YAML file, it is read in as the task
     specification. Otherwise, it is interpreted as a bash command.
 
-    \b
-    Execution and scheduling behavior:
-    \b
-    - If ENTRYPOINT is a YAML, or if it is a command with a resource demand
-      flag specified (`--gpus` or `--num-nodes`): it is treated as a proper
-      task that will undergo job queue scheduling, respecting its resource
-      requirement. It can be executed on any node of th cluster with enough
-      resources.
-    - Otherwise (if ENTRYPOINT is a command and no resource demand flag
-      specified), it is treated as an inline command, to be executed only on
-      the head node of the cluster. This is useful for monitoring commands
-      (e.g., gpustat, htop).
-
-    In both cases, the commands are run under the task's workdir (if specified).
-
     \b
     Actions performed by `sky exec`:
     \b
@@ -714,6 +699,18 @@ def exec(
     reflect those changes.  To ensure a cluster's setup is up to date, use `sky
     launch` instead.
 
+    \b
+    Execution and scheduling behavior:
+    \b
+    - The YAML task or the command will undergo job queue scheduling,
+      respecting any specified resource requirement. It can be executed on any
+      node of th cluster with enough resources.
+    - The YAML task or the command is run under the task's workdir (if
+      specified).
+    - The task/command is run non-interactively (without a pseudo-terminal or
+      pty), so interactive commands such as `htop` do not work. Use `ssh
+      my_cluster` instead.
+
     Typical workflow:
 
     .. code-block:: bash
@@ -732,21 +729,18 @@ def exec(
         # Do "sky launch" again if anything other than Task.run is modified:
         sky launch -c mycluster app.yaml
 
-    Advanced use cases:
-
     .. code-block:: bash
 
         # Pass in commands for execution
-        sky exec mycluster -- echo Hello World
+        sky exec mycluster -- python train.py
 
     """
     entrypoint = ' '.join(entrypoint)
     handle = global_user_state.get_handle_from_cluster_name(cluster)
     if handle is None:
-        raise click.BadParameter(f'Cluster \'{cluster}\' not found.  '
+        raise click.BadParameter(f'Cluster {cluster!r} not found. '
                                  'Use `sky launch` to provision first.')
     backend = backend_utils.get_backend_from_handle(handle)
-    resource_demand_specified = gpus is not None or num_nodes is not None
 
     with sky.Dag() as dag:
         if _check_yaml(entrypoint):
@@ -761,23 +755,6 @@ def exec(
             task = sky.Task(name='sky-cmd', run=entrypoint)
             task.set_resources({sky.Resources()})
 
-            if isinstance(backend, backends.CloudVmRayBackend):
-                # Run inline commands directly on head node if the resources are
-                # not set. User should take the responsibility to not overload
-                # the cluster.
-                if not resource_demand_specified:
-                    if workdir is not None:
-                        backend.sync_workdir(handle, workdir)
-                    backend.run_on_head(
-                        handle,
-                        entrypoint,
-                        stream_logs=True,
-                        # Allocate a pseudo-terminal to disable output buffering
-                        ssh_mode=backend_utils.SshMode.INTERACTIVE,
-                        under_remote_workdir=True,
-                        redirect_stdout_stderr=False)
-                    return
-
         # Override.
         if workdir is not None:
             task.workdir = workdir


Code review comments:
diff_hunk: @@ -103,9 +103,9 @@ Bash commands are also supported, such as:
 
 .. code-block:: console
 
-  $ sky exec mycluster -- gpustat -i
-  $ sky exec mycluster -- ls -l
-  $ sky exec mycluster htop
+  $ sky exec mycluster python train.py
path: docs/source/getting-started/quickstart.rst
text: @user1:
This will not allocate GPU for the training. Can we change this to 
```
sky exec mycluster python train_cpu.py
sky exec mycluster --gpus=V100:1 python train_gpu.py
```?

@author:
Done, good catch!

diff_hunk: @@ -732,21 +729,18 @@ def exec(
         # Do "sky launch" again if anything other than Task.run is modified:
         sky launch -c mycluster app.yaml
 
-    Advanced use cases:
-
     .. code-block:: bash
 
         # Pass in commands for execution
-        sky exec mycluster -- echo Hello World
+        sky exec mycluster -- python train.py
path: sky/cli.py
text: @user1:
How about `sky exec mycluster -- python train_cpu.py`?

@author:
Done, good catch!