Repository: skypilot-org/skypilot
Pull request: Fix rsync_exclude for gcp credential file_mounts
Closes #494. The `rsync_exclude` will treat the absolute path as the relative path, so the previous symlink problem observed in #483 still exists. I changed the `rsync_exclude` for the symlink in the credential to be a relative path. The solution is a bit hacky right now, but it may be good to fix it first and then refactor it later.

Tested:
- [x] `sky launch -c fm examples/using_file_mounts.yaml` (adding a private gcs link in the file_mounts)

Problem statement:
Multinode Sky Launch Bug
On most recent master, run multinode setup, e.g. `sky launch -c hello examples/env_check.yaml`.
Head node launches, but worker nodes failed to launch:

```
I 03-06 00:24:52 backend_utils.py:503] No cluster status.
I 03-06 00:24:52 backend_utils.py:503] The autoscaler failed with the following error:
I 03-06 00:24:52 backend_utils.py:503] Traceback (most recent call last):
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/monitor.py", line 434, in run
I 03-06 00:24:52 backend_utils.py:503]     self._initialize_autoscaler()
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/monitor.py", line 221, in _initialize_autoscaler
I 03-06 00:24:52 backend_utils.py:503]     prom_metrics=self.prom_metrics)
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/autoscaler.py", line 191, in __init__
I 03-06 00:24:52 backend_utils.py:503]     self.reset(errors_fatal=True)
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/autoscaler.py", line 920, in reset
I 03-06 00:24:52 backend_utils.py:503]     raise e
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/autoscaler.py", line 865, in reset
I 03-06 00:24:52 backend_utils.py:503]     generate_file_mounts_contents_hash=sync_continuously,
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/util.py", line 368, in hash_runtime_conf
I 03-06 00:24:52 backend_utils.py:503]     add_content_hashes(local_path)
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/util.py", line 357, in add_content_hashes
I 03-06 00:24:52 backend_utils.py:503]     add_hash_of_file(fpath)
I 03-06 00:24:52 backend_utils.py:503]   File "/home/ubuntu/.local/lib/python3.6/site-packages/ray/autoscaler/_private/util.py", line 341, in add_hash_of_file
I 03-06 00:24:52 backend_utils.py:503]     with open(fpath, "rb") as f:
I 03-06 00:24:52 backend_utils.py:503] FileNotFoundError: [Errno 2] No such file or directory: '/home/ubuntu/.config/gcloud/virtenv/bin/python'
I 03-06 00:24:52 backend_utils.py:503] 
I 03-06 00:24:52 backend_utils.py:503] Loaded cached provider configuration
I 03-06 00:24:52 backend_utils.py:503] If you experience issues with the cloud provider, try re-running the command with --no-config-cache.
I 03-06 00:24:52 backend_utils.py:503] Fetched IP: 34.228.226.109
```

Worker SSH PR is blocked by this.

Hints from issue discussion:
@Michaelvll Do you have a direction on what is causing this issue?
Tried latest f5f228368 on my laptop, didn't see this issue.  It sounds like the autoscaler cannot access / doesn't have read permissions to `/home/ubuntu/.config/gcloud/virtenv/bin/python`.  Check `ls -l`?

`cat ~/.sky/generated/hello.yml`:
```
...
file_mounts:
  ~/.sky/sky_wheels: /var/folders/8f/56gzvwkd3n3293xjlrztr6600000gp/T/sky_wheels_zongheng
  ~/.aws: ~/.aws
  ~/.config/gcloud: ~/.config/gcloud
  ~/.azure: ~/.azure

rsync_exclude:
- ~/.config/gcloud/virtenv
...
```
Please check that file on the head node with `ls -l` to see if it exists and is a symlink. I met this problem before. It was because that file is a symlink locally, and after file mounting the local file to the head, ray does not handle it correctly when transferring from head to worker. The problem should be already solved by #483, where I excluded the virtenv directory from the file mounts. Could you check the ray yaml to see if the folder is correctly excluded?
I can reproduce it right now. My `~/.config/gcloud/virtenv/bin/python` has been replaced to be non-symlink. Let me fix it.
@Michaelvll I tried `sky launch -c hello examples/env_check.yaml` on your mentioned PR #483 and the PR right before that one #487. PR #487 works while #483 does not and returns the same error.

@concretevitamin Did you try launching a new cluster from scratch. It works if the cluster is already luanched.
Regarding the symlink and `ls -l` it shows this:
```
lrwxrwxrwx 1 ubuntu ubuntu    7 Feb  9 22:40 python -> python3
lrwxrwxrwx 1 ubuntu ubuntu   61 Feb  9 22:40 python3 -> /Library/Frameworks/Python.framework/Versions/3.7/bin/python3
```
It is a nested symlink...

> @Michaelvll I tried `sky launch -c hello examples/env_check.yaml` on your mentioned PR #483 and the PR right before that one #487. PR #487 works while #483 does not and returns the same error.

It does not appear because we do not file_mounts the credential before #483. The reason for the current error is that the `rsync_exclude` section does not work for some reason right now. I am investigating that. If you want to see what is happening, you could log into the cluster and take a look at the `~/.config/gcloud/virtenv` folder. It is not excluded though we specified it in the ray yaml.

> @concretevitamin Did you try launching a new cluster from scratch. It works if the cluster is already luanched.
Regarding the symlink and ls -l it shows this:

The reason for @concretevitamin not having this issue is that there is no `virtenv` under his gcp credential.
I found the reason, `rsync_exclude` treat all path as relative path, so we cannot have an absolute path there: https://askubuntu.com/questions/349613/how-to-exclude-a-folder-from-rsync
@michaelzhiluo Could you try the #496 if it solves your problem?

Commit message: format

Patch to review:
diff --git a/sky/clouds/gcp.py b/sky/clouds/gcp.py
index 5aae7314d07..04b77abf5b7 100644
--- a/sky/clouds/gcp.py
+++ b/sky/clouds/gcp.py
@@ -256,6 +256,10 @@ def check_credentials(self) -> Tuple[bool, Optional[str]]:
         return True, None
 
     def get_credential_file_mounts(self) -> Tuple[Dict[str, str], List[str]]:
+        # Excluding the symlink to the python executable created by the gcp
+        # credential, which causes problem for ray up multiple nodes, tracked
+        # in #494, #496, #483.
+        # rsync_exclude only supports relative paths.
         return {
             '~/.config/gcloud': '~/.config/gcloud'
-        }, ['~/.config/gcloud/virtenv']
+        }, ['/virtenv/bin/python*']


Code review comments:
diff_hunk: @@ -256,6 +256,10 @@ def check_credentials(self) -> Tuple[bool, Optional[str]]:
         return True, None
 
     def get_credential_file_mounts(self) -> Tuple[Dict[str, str], List[str]]:
+        # Excluding the symlink to the python executable created by the gcp
+        # credential, which causes problem for ray up multiple nodes, tracked
+        # in #494, #496, #483.
+        # rsync_exclude only supports relative paths.
         return {
             '~/.config/gcloud': '~/.config/gcloud'
-        }, ['~/.config/gcloud/virtenv']
+        }, ['/virtenv/bin/python*']
path: sky/clouds/gcp.py
text: @user1:
What's the purpose of `~/.config/gcloud/virtenv` and when does it get created? Could we exclude this whole thing by `/virtenv` here?

@author:
It is unclear why the `virtenv` folder gets created. That may require some investigations of the gcloud credential. At least it seems that does not matter for downloading the private GCS store and launching the GCP cluster.  

Yes, we can exclude the whole folder here. One concern is that in the current design, the folder with the same name `virtenv` in other file_mounts of ray yaml (only the following) will also be excluded. It is fine for now but may affect the later new clouds to be added. Changed to `virtenv` and added a TODO for a future fix.
```
file_mounts:
  ~/.sky/sky_wheels: /var/folders/bx/cnr_jmsj4n79c9p0q4v68xvw0000gp/T/sky_wheels_zhwu
  ~/.aws: ~/.aws
  ~/.config/gcloud: ~/.config/gcloud
  ~/.azure: ~/.azure
```