Repository: skypilot-org/skypilot
Pull request: [UX] Fail early from provisioning when rsync is uninstalled
<!-- Describe the changes in this PR -->
This PR closes #2113 

When `rsync` is not installed in the user's machine, `ray_up()` is reattempted for `_MAX_RAY_UP_RETRY` number of times without failing. As this is time consuming, this PR fails early in the provisioning step when `rsync` is not installed and gently hints the user to install `rsync`.

Considered failing from `need_ray_up()` as well, but at this point, `state.db` is already updated with `INIT` status of the cluster attempted to provisioned which isn't the most optimal scenario. Also, it takes time for single failure before reaching `need_ray_up()`.

<!-- 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: `bash format.sh`
- [x] manally tested with running `sky launch` after deleting `rsync` with and w/o `--dryrun` option

Problem statement:
[UX] Fail early when rsync is not installed locally
We should fail early and show hints about installing the rsync when it is not installed on user's laptop. Currently, when user try `sky launch`, it will just keep retrying, and the error message is hidden in the `provision.log` file.

Hints from issue discussion:
Had the same issue while testing with `sky launch` in a newly setup container environment where `rsync` is not installed.

Commit message: fail early for rsync uninstalled

Patch to review:
diff --git a/sky/backends/cloud_vm_ray_backend.py b/sky/backends/cloud_vm_ray_backend.py
index 96cbb5bff19..1f3f024692a 100644
--- a/sky/backends/cloud_vm_ray_backend.py
+++ b/sky/backends/cloud_vm_ray_backend.py
@@ -1336,6 +1336,22 @@ def _retry_zones(
         prev_cluster_status: Optional[status_lib.ClusterStatus],
     ):
         """The provision retry loop."""
+        # When rsync is not installed in the user's machine, Ray will
+        # silently retry to up the node for _MAX_RAY_UP_RETRY number of times
+        # This is time consuming so we fail early.
+        try:
+            subprocess.run('rsync --version',
+                           shell=True,
+                           check=True,
+                           stdout=subprocess.PIPE,
+                           stderr=subprocess.PIPE)
+        except subprocess.CalledProcessError as e:
+            with ux_utils.print_exception_no_traceback():
+                raise RuntimeError(
+                    'rsync is required for provisioning and it is not '
+                    'installed. Please run the following command:\n'
+                    '    $ sudo apt-get install rsync') from e
+
         style = colorama.Style
         fore = colorama.Fore
         # Get log_path name


Code review comments:
diff_hunk: @@ -1336,6 +1336,22 @@ def _retry_zones(
         prev_cluster_status: Optional[status_lib.ClusterStatus],
     ):
         """The provision retry loop."""
+        # When rsync is not installed in the user's machine, Ray will
+        # silently retry to up the node for _MAX_RAY_UP_RETRY number of times
+        # This is time consuming so we fail early.
+        try:
+            subprocess.run('rsync --version',
+                           shell=True,
+                           check=True,
+                           stdout=subprocess.PIPE,
+                           stderr=subprocess.PIPE)
+        except subprocess.CalledProcessError as e:
+            with ux_utils.print_exception_no_traceback():
+                raise RuntimeError(
+                    'rsync is required for provisioning and it is not '
+                    'installed. Please run the following command:\n'
+                    '    $ sudo apt-get install rsync') from e
path: sky/backends/cloud_vm_ray_backend.py
text: @user1:
`sudo apt-get install rsync` is system specific, i.e., only works for debian series of system. 
```suggestion
                    '`rsync` is required for provisioning and it is not '
                    'installed. For Debian/Ubuntu system, install it with:\n'
                    '  $ sudo apt install rsync') from e
```

diff_hunk: @@ -1336,6 +1336,22 @@ def _retry_zones(
         prev_cluster_status: Optional[status_lib.ClusterStatus],
     ):
         """The provision retry loop."""
+        # When rsync is not installed in the user's machine, Ray will
+        # silently retry to up the node for _MAX_RAY_UP_RETRY number of times
+        # This is time consuming so we fail early.
+        try:
+            subprocess.run('rsync --version',
+                           shell=True,
+                           check=True,
+                           stdout=subprocess.PIPE,
+                           stderr=subprocess.PIPE)
+        except subprocess.CalledProcessError as e:
+            with ux_utils.print_exception_no_traceback():
+                raise RuntimeError(
+                    'rsync is required for provisioning and it is not '
+                    'installed. Please run the following command:\n'
+                    '    $ sudo apt-get install rsync') from e
+
path: sky/backends/cloud_vm_ray_backend.py
text: @user1:
Can we move this to an upper level to maybe `_provision` instead?