Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Storage] gsutil rsync failing to sync source with dangling symlink #2165

Conversation

landscapepainter
Copy link
Collaborator

@landscapepainter landscapepainter commented Jul 1, 2023

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 to resolve dangling symlink issue. Also, as Sky Storage does not support symlinks, this is a suitable solution.

Tested (run the relevant ones):

  • Manually ran gsutil -m rsync -erx to sync src with dangling symlink to gcs
  • Relevant individual smoke tests: pytest tests/test_smoke.py::TestStorageWithCredentials
  • Tested with pytest tests/test_smoke.py::test_gcp_storage_mounts

@landscapepainter landscapepainter changed the title gsutil rsync failing to sync source with dangling symlink [Storage] gsutil rsync failing to sync source with dangling symlink Jul 1, 2023
@romilbhardwaj
Copy link
Collaborator

Thanks! As we talked, you've also tested against yaml in #2127.
Recommend also running:

  • pytest tests/test_smoke.py::test_gcp_storage_mounts

sky/data/storage.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@romilbhardwaj romilbhardwaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks @landscapepainter!

@landscapepainter landscapepainter merged commit ac9ba6a into skypilot-org:master Jul 6, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Storage] GCS syncing fails when the src directory contains courrupted symlink
2 participants