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

[Tests] Fix storage mount smoke test for k8s #4484

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Dec 18, 2024

When running the test with k8s, the private mount store will be set to Azure causing the issue with fuse3. We now explicitly set the store to s3 when it is on k8s.

For s3 another issue is that pip3 may not be available during the first file mount, so we change to uv pip instead.

To reproduce:
pytest tests/test_smoke.py::test_docker_storage_mounts[docker:ubuntu:18.04] --kubernetes
and adding the following to

skypilot/sky/task.py

Lines 941 to 946 in 83b2325

if storage_cloud is None:
storage_cloud_str = enabled_storage_clouds[0]
assert storage_cloud_str is not None, enabled_storage_clouds[0]
storage_region = None # Use default region in the Store class
else:
storage_cloud_str = str(storage_cloud)

        # TODO(zhwu): debugging only
        if storage_cloud is None:
            # TODO(zhwu): debugging only
            if 'azure' in enabled_storage_clouds:
                storage_cloud_str = 'azure'
            else:
                storage_cloud_str = enabled_storage_clouds[0]

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
    • pytest tests/test_smoke.py::test_docker_storage_mounts[docker:ubuntu:18.04]
    • pytest tests/test_smoke.py::test_docker_storage_mounts[docker:ubuntu:18.04] --kubernetes
  • Backward compatibility tests: conda deactivate; bash -i tests/backward_compatibility_tests.sh

@Michaelvll Michaelvll requested a review from cg505 December 19, 2024 03:25
@Michaelvll Michaelvll changed the title [Tests] Fix storage mount smoke test [Tests] Fix storage mount smoke test for k8s Dec 19, 2024
Comment on lines +335 to +336
private_mount_store = (None
if generic_cloud in ['aws', 'gcp'] else 's3')
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC this also forces s3 when testing on Azure, which would require AWS credentials even if we want to run just Azure tests. Should we retain the original behavior of excluding the private mount if Azure is specified?

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.

2 participants