[Storage] Azure bucket sub directory is ignored if the bucket previously exists #4706

Michaelvll · 2025-02-13T01:45:42Z

This PR resolves two issues:

Issue 1: When there is an existing Azure bucket in SkyPilot storage database, the sub path will be ignored. This affects the smoke test pytest tests/test_smoke.py::test_managed_jobs_storage --azure.

To reproduce:

set jobs.bucket: https://xxx.blob.core.windows.net/yyy that is previously created by SkyPilot in ~/.sky/config.yaml
sky jobs launch -n test-azure-upload test.yaml

# A managed spot example with storages.
#
# Runs a task that uses cloud buckets for uploading and accessing files.
#
# Usage:
#   sky jobs launch -c spot-storage examples/managed_job_with_storage.yaml
#   sky down spot-storage

file_mounts:
  # File mounts for folder
  /tmp/workdir: ~/tmp-workdir


  # File mounts for file
  ~/tmpfile: ~/tmpfile
  ~/a/b/c/tmpfile: ~/tmpfile

run: |
  set -ex
  cat ~/tmpfile
  cat ~/a/b/c/tmpfile
  
  # Write to a file in the mounted bucket
  echo "hello world!" > /output_path/output.txt

The bucket will be polluted with in the root directory, and the job above fails with:

Detailed Error

E 02-13 01:43:29 subprocess_utils.py:149] Cannot perform sync due to error: trying to sync between different resource types (either file <-> directory or directory <-> file) which is not allowed.sync must happen between source and destination of the same type, e.g. either file <-> file or directory <-> directory.To make sure target is handled as a directory, add a trailing '/' to the target.
E 02-13 01:43:29 subprocess_utils.py:149] 

I 02-13 01:43:29 recovery_strategy.py:339] Failed to launch a cluster with error: sky.exceptions.CommandError: Command mkdir -p ~/tmpfile && azcopy --version > /dev/null 2>&1 || (mkdir -p /usr/local/bin; curl -L https:/... failed with return code 1.
I 02-13 01:43:29 recovery_strategy.py:339] Failed to run command before rsync https://sky635611d9a69285a3.blob.core.windows.net/skypilot-filemounts-vscode-11d9a692-ahpbb7v3/job-7xgk48n3/tmp-files/file-0 -> ~/tmpfile. Ensure that the network is stable, then retry. mkdir -p ~/tmpfile && azcopy --version > /dev/null 2>&1 || (mkdir -p /usr/local/bin; curl -L https://aka.ms/downloadazcopy-v10-linux -o azcopy.tar.gz; sudo tar -xvzf azcopy.tar.gz --strip-components=1 -C /usr/local/bin --exclude=*.txt; sudo chmod +x /usr/local/bin/azcopy; rm azcopy.tar.gz) && azcopy sync 'https://sky635611d9a69285a3.blob.core.windows.net/skypilot-filemounts-vscode-11d9a692-ahpbb7v3/job-7xgk48n3/tmp-files/file-0?se=2025-02-13T02%3A43%3A27Z&sp=rcwl&sv=2025-01-05&sr=c&sig=mr59%2B5SlTa72/5c%2BkM0BY6eiIqCx/uQOs3dUB%2BFnV1g%3D' ~/tmpfile/ --recursive --delete-destination=false See logs in ~/sky_logs/sky-2025-02-13-01-39-47-646407/file_mounts.log)
I 02-13 01:43:29 recovery_strategy.py:342]   Traceback: Traceback (most recent call last):
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/jobs/recovery_strategy.py", line 286, in _launch
I 02-13 01:43:29 recovery_strategy.py:342]     sky.launch(
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/utils/common_utils.py", line 385, in _record
I 02-13 01:43:29 recovery_strategy.py:342]     return f(*args, **kwargs)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/utils/common_utils.py", line 385, in _record
I 02-13 01:43:29 recovery_strategy.py:342]     return f(*args, **kwargs)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/execution.py", line 529, in launch
I 02-13 01:43:29 recovery_strategy.py:342]     return _execute(
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/execution.py", line 329, in _execute
I 02-13 01:43:29 recovery_strategy.py:342]     backend.sync_file_mounts(handle, task.file_mounts,
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/utils/common_utils.py", line 385, in _record
I 02-13 01:43:29 recovery_strategy.py:342]     return f(*args, **kwargs)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/utils/common_utils.py", line 365, in _record
I 02-13 01:43:29 recovery_strategy.py:342]     return f(*args, **kwargs)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/backends/backend.py", line 101, in sync_file_mounts
I 02-13 01:43:29 recovery_strategy.py:342]     return self._sync_file_mounts(handle, all_file_mounts, storage_mounts)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/backends/cloud_vm_ray_backend.py", line 3207, in _sync_file_mounts
I 02-13 01:43:29 recovery_strategy.py:342]     self._execute_file_mounts(handle, all_file_mounts)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/backends/cloud_vm_ray_backend.py", line 4790, in _execute_file_mounts
I 02-13 01:43:29 recovery_strategy.py:342]     backend_utils.parallel_data_transfer_to_nodes(
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/backends/backend_utils.py", line 1477, in parallel_data_transfer_to_nodes
I 02-13 01:43:29 recovery_strategy.py:342]     subprocess_utils.run_in_parallel(_sync_node, runners, num_threads)
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/utils/subprocess_utils.py", line 122, in run_in_parallel
I 02-13 01:43:29 recovery_strategy.py:342]     return [func(args[0])]
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/backends/backend_utils.py", line 1455, in _sync_node
I 02-13 01:43:29 recovery_strategy.py:342]     subprocess_utils.handle_returncode(rc,
I 02-13 01:43:29 recovery_strategy.py:342]   File "/home/sky/skypilot-runtime/lib/python3.10/site-packages/sky/utils/subprocess_utils.py", line 156, in handle_returncode
I 02-13 01:43:29 recovery_strategy.py:342]     raise exceptions.CommandError(returncode, command, format_err_msg,
I 02-13 01:43:29 recovery_strategy.py:342] sky.exceptions.CommandError: Command mkdir -p ~/tmpfile && azcopy --version > /dev/null 2>&1 || (mkdir -p /usr/local/bin; curl -L https:/... failed with return code 1.
I 02-13 01:43:29 recovery_strategy.py:342] Failed to run command before rsync https://sky635611d9a69285a3.blob.core.windows.net/skypilot-filemounts-vscode-11d9a692-ahpbb7v3/job-7xgk48n3/tmp-files/file-0 -> ~/tmpfile. Ensure that the network is stable, then retry. mkdir -p ~/tmpfile && azcopy --version > /dev/null 2>&1 || (mkdir -p /usr/local/bin; curl -L https://aka.ms/downloadazcopy-v10-linux -o azcopy.tar.gz; sudo tar -xvzf azcopy.tar.gz --strip-components=1 -C /usr/local/bin --exclude=*.txt; sudo chmod +x /usr/local/bin/azcopy; rm azcopy.tar.gz) && azcopy sync 'https://sky635611d9a69285a3.blob.core.windows.net/skypilot-filemounts-vscode-11d9a692-ahpbb7v3/job-7xgk48n3/tmp-files/file-0?se=2025-02-13T02%3A43%3A27Z&sp=rcwl&sv=2025-01-05&sr=c&sig=mr59%2B5SlTa72/5c%2BkM0BY6eiIqCx/uQOs3dUB%2BFnV1g%3D' ~/tmpfile/ --recursive --delete-destination=false See logs in ~/sky_logs/sky-2025-02-13-01-39-47-646407/file_mounts.log

Issue 2: During the file mounts, the storage upload does not include the sub directory, which can be a bit confusing:
previously

  Excluded files to sync to cluster based on .gitignore.
✓ Storage synced: ./examples -> https://xxxx.blob.core.windows.net/skypilot-filemounts-vscode-yyyy/  View logs at: ~/sky_logs/sky-2025-02-13-01-38-17-371662/storage_sync.log
  Excluded files to sync to cluster based on .gitignore.
✓ Storage synced: ~/tmp-workdir -> https://xxxx.blob.core.windows.net/skypilot-filemounts-vscode-yyyy/  View logs at: ~/sky_logs/sky-2025-02-13-01-38-30-519142/storage_sync.log

Now

  Workdir: './examples' -> storage: 'skypilot-filemounts-vscode-11d9a692-ahpbb7v3'.
Verifying bucket and uploading from source for storage skypilot-filemounts-vscode-11d9a692-ahpbb7v3
  Excluded files to sync to cluster based on .gitignore.
✓ Storage synced: ~/tmp-workdir -> https://xxxx.blob.core.windows.net/skypilot-filemounts-vscode-yyyy/job-ct66lqv4/local-file-mounts/0/  View logs at: ~/sky_logs/sky-2025-02-13-01-35-59-046482/storage_sync.log
  Folder : '~/tmp-workdir' -> storage: 'skypilot-filemounts-vscode-11d9a692-ahpbb7v3'.
Verifying bucket and uploading from source for storage skypilot-filemounts-vscode-11d9a692-ahpbb7v3
  Excluded files to sync to cluster based on .gitignore.
✓ Storage synced: /tmp/skypilot-filemounts-files-ct66lqv4 -> https://xxxx.blob.core.windows.net/skypilot-filemounts-vscode-yyyy/job-ct66lqv4/tmp-files/  View logs at: ~/sky_logs/sky-2025-02-13-01-36-12-515049/storage_sync.log

Tested (run the relevant ones):

Code formatting: bash format.sh
Any manual or new tests for this PR (please specify below)
All smoke tests: pytest tests/test_smoke.py
Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
Backward compatibility tests: conda deactivate; bash -i tests/backward_compatibility_tests.sh

Michaelvll · 2025-02-13T01:47:47Z

sky/data/storage.py

+                       'sync_on_reconstruction', True),
+                    # Backward compatibility
+                    # TODO: remove the hasattr check after v0.11.0
+                    _bucket_sub_path=override_args.get(
+                        '_bucket_sub_path',
+                        metadata._bucket_sub_path  # pylint: disable=protected-access
+                    ) if hasattr(metadata, '_bucket_sub_path') else None)


This is the main part that resolves issue 1

zpoint

Could we update test_managed_jobs_intermediate_storage to also support azure so that we can include this test case?

Michaelvll · 2025-02-13T05:22:26Z

Could we update test_managed_jobs_intermediate_storage to also support azure so that we can include this test case?

There seems a bunch of places that require aws specific CLIs. Could you help add the test for azure in a future PR @zpoint?

zpoint · 2025-02-13T05:47:24Z

Sure

zpoint

Thanks @Michaelvll

zpoint · 2025-02-13T05:51:38Z

sky/data/storage.py

@@ -354,7 +354,8 @@ def from_metadata(cls, metadata: StoreMetadata, **override_args):
                                             metadata.is_sky_managed),
            sync_on_reconstruction=override_args.get('sync_on_reconstruction',
                                                     True),
-            # backward compatibility
+            # Backward compatibility
+            # TODO: remove the hasattr check after v0.11.0


Are we saying that backward compatibility is removed after v0.11.0?
For example, v0.6.0 won't be compatible with v0.11.0?

Yes, we have a convention that keep backward compatibilty for 3 major releases.

Michaelvll · 2025-02-13T19:40:32Z

/smoke-test --azure -k TestStorageWithCredentials

Michaelvll · 2025-02-13T20:03:18Z

/smoke-test --azure -k TestStorageWithCredentials

Michaelvll · 2025-02-13T22:44:45Z

/smoke-test --azure -k TestStorageWithCredentials

Michaelvll · 2025-02-13T22:45:48Z

/smoke-test --azure -k test_docker_storage_mounts

Michaelvll · 2025-02-13T22:54:49Z

cc'ing @romilbhardwaj for a quick look as well : )

Michaelvll added 3 commits February 13, 2025 01:18

Set sub path for azure storage

01c39d2

Full path for the ux

78cc291

fix variable reference

6ba026f

Michaelvll changed the title ~~[Storage] Fix Azure bucket sub directory ignored issue~~ [Storage] Azure bucket sub directory is ignored if the bucket previously exists Feb 13, 2025

Michaelvll commented Feb 13, 2025

View reviewed changes

Michaelvll mentioned this pull request Feb 13, 2025

[Core] SkyPilot Client-Server Architecture #4660

Merged

12 tasks

minor

da2bdd2

Michaelvll requested review from romilbhardwaj and zpoint February 13, 2025 02:03

Michaelvll added a commit that referenced this pull request Feb 13, 2025

Azure tests passed with #4706

65b6b5a

zpoint reviewed Feb 13, 2025

View reviewed changes

zpoint approved these changes Feb 13, 2025

View reviewed changes

enforce azure region for bucket

0cfb299

Delete storage even when failure happens

b6051fd

Michaelvll merged commit aa413c6 into master Feb 14, 2025
19 checks passed

Michaelvll deleted the azure-bucket-path-fix branch February 14, 2025 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Storage] Azure bucket sub directory is ignored if the bucket previously exists #4706

[Storage] Azure bucket sub directory is ignored if the bucket previously exists #4706

Michaelvll commented Feb 13, 2025 •

edited

Loading

Michaelvll Feb 13, 2025

zpoint left a comment

Michaelvll commented Feb 13, 2025

zpoint commented Feb 13, 2025

zpoint left a comment

zpoint Feb 13, 2025

Michaelvll Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

[Storage] Azure bucket sub directory is ignored if the bucket previously exists #4706

[Storage] Azure bucket sub directory is ignored if the bucket previously exists #4706

Conversation

Michaelvll commented Feb 13, 2025 • edited Loading

Michaelvll Feb 13, 2025

Choose a reason for hiding this comment

zpoint left a comment

Choose a reason for hiding this comment

Michaelvll commented Feb 13, 2025

zpoint commented Feb 13, 2025

zpoint left a comment

Choose a reason for hiding this comment

zpoint Feb 13, 2025

Choose a reason for hiding this comment

Michaelvll Feb 13, 2025

Choose a reason for hiding this comment

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025

Michaelvll commented Feb 13, 2025 •

edited

Loading