Repository: skypilot-org/skypilot
Pull request: [Storage] gsutil rsync failing to sync source with dangling symlink
This PR closes #2147, which is initially raised by #2127

`aws s3 sync` is a command ran to sync Sky Storages set with `s3`/`r2` storages. When the command sees a dangling symlink, it simply ignores and continue to sync other contents. However, `gsutil rsync`, which is used to set `gcs`, does not support this and fails early. In order to have consistent behavior between `gcs` and `s3`/`r2` storages, I add an additional flag to the command to ignore all the symlinks in the source. Adding `-e` is the [solution gsutil team provides](https://github.com/GoogleCloudPlatform/gsutil/issues/222) to resolve dangling symlink issue. Also, as Sky Storage does not support symlinks, this is a suitable solution.


Tested (run the relevant ones):
- [x] Manually ran `gsutil -m rsync -erx` to sync src with dangling symlink to `gcs`
- [x] Relevant individual smoke tests: `pytest tests/test_smoke.py::TestStorageWithCredentials` 
- [x] Tested with `pytest tests/test_smoke.py::test_gcp_storage_mounts
`

Problem statement:
[Storage] GCS syncing fails when the src directory contains courrupted symlink
User reported that gcs sync errors out when the src directory contains dangling symlink.

The command we run to sync the contents from local directory to cloud storage for s3 is `aws s3 sync` and `gsutil rsync -r` for gcs. When the local directory being synced contains a dangling symlink(the file/dir it points to does not exist anymore), `gsutil rsync -r` errors out while `aws s3 sync` ignores this and move on and continues to sync other file/dir. 

Output for the commands when the dir contains dangling symlink:
`aws s3 sync`:
```
aws s3 sync . s3://doyoung-test
warning: Skipping file /path/to/dangling/symlink. File does not exist.
```

`gsutil rsync -r`:
```
gsutil rsync -r . gs://doyoung-test

WARNING: gsutil rsync uses hashes when modification time is not available at
both the source and destination. Your crcmod installation isn't using the
module's C extension, so checksumming will run very slowly. If this is your
first rsync since updating gsutil, this rsync can take significantly longer than
usual. For help installing the extension, please see "gsutil help crcmod".

Building synchronization state...
Caught non-retryable exception while listing file://.: [Errno 2] No such file or directory: '/path/to/dangling/symlink'
CommandException: Caught non-retryable exception - aborting rsync
```

We need to error-handle dangling symlink when syncing attempted to GCS. Perhaps, gently display an error message and move on to sync rest of the files in the directory rather than erroring out.

Hints from issue discussion:

Commit message: update_gsutil_rsync_cmd

Patch to review:
diff --git a/sky/data/storage.py b/sky/data/storage.py
index bb2c9ab2ebb..179b907f257 100644
--- a/sky/data/storage.py
+++ b/sky/data/storage.py
@@ -1507,13 +1507,13 @@ def batch_gsutil_rsync(self,
 
         def get_file_sync_command(base_dir_path, file_names):
             sync_format = '|'.join(file_names)
-            sync_command = (f'gsutil -m rsync -x \'^(?!{sync_format}$).*\' '
+            sync_command = (f'gsutil -m rsync -ex \'^(?!{sync_format}$).*\' '
                             f'{base_dir_path} gs://{self.name}')
             return sync_command
 
         def get_dir_sync_command(src_dir_path, dest_dir_name):
             # we exclude .git directory from the sync
-            sync_command = (f'gsutil -m rsync -r -x \'.git/*\' {src_dir_path} '
+            sync_command = (f'gsutil -m rsync -erx \'.git/*\' {src_dir_path} '
                             f'gs://{self.name}/{dest_dir_name}')
             return sync_command
 


Code review comments:
diff_hunk: @@ -1507,13 +1507,13 @@ def batch_gsutil_rsync(self,
 
         def get_file_sync_command(base_dir_path, file_names):
             sync_format = '|'.join(file_names)
-            sync_command = (f'gsutil -m rsync -x \'^(?!{sync_format}$).*\' '
+            sync_command = (f'gsutil -m rsync -ex \'^(?!{sync_format}$).*\' '
                             f'{base_dir_path} gs://{self.name}')
             return sync_command
 
         def get_dir_sync_command(src_dir_path, dest_dir_name):
             # we exclude .git directory from the sync
-            sync_command = (f'gsutil -m rsync -r -x \'.git/*\' {src_dir_path} '
+            sync_command = (f'gsutil -m rsync -erx \'.git/*\' {src_dir_path} '
path: sky/data/storage.py
text: @user1:
Would be nice to separate out these flags.