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

Relative cluster yaml #1176

Merged
merged 6 commits into from
Sep 29, 2022
Merged

Relative cluster yaml #1176

merged 6 commits into from
Sep 29, 2022

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Sep 28, 2022

Closes #1138.

Since this is also requested by @lhqing to share the ~/.sky folder across different devices, we may want to have this PR to unblock the issue. This PR replaces the path for the cluster yaml to the relative path as suggested by @concretevitamin in #1138 (comment)

Tested:

  • sky start existing-cluster; sky stop existing-cluster; sky down existing-cluster
  • sky launch --cloud gcp ''; sky launch -c test-zhwu --cloud gcp ''; scp ~/.sky test-zhwu:~/.sky; ssh test-zhwu; sky down sky-ea57-zhwu -y; sky status

@Michaelvll Michaelvll marked this pull request as ready for review September 28, 2022 04:09
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.

lgtm, thanks for fixing! :)

If you haven't already, can you also verify if this enables the container use case as mentioned in this comment?

sky/backends/cloud_vm_ray_backend.py Show resolved Hide resolved
sky/backends/cloud_vm_ray_backend.py Show resolved Hide resolved
@@ -13,13 +13,23 @@

from sky import sky_logging

_USER_HASH_FILE = os.path.expanduser('~/.sky/user_hash')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a good change - in the future this also allows us to move the user to a new machine while preserving their hash

Copy link
Collaborator Author

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Thank you for the review @romilbhardwaj! Just tested it with the docker case:

  1. sudo docker run -it --rm -v $HOME/.sky:/root/.sky:rw -v $HOME/.config:/root/.config:rw -v $HOME/.aws:/root/.aws:ro continuumio/miniconda /bin/bash
  2. sky cpunode -c docker-test --cloud gcp in the docker
  3. sky down docker-test on the host machine.

sky/backends/cloud_vm_ray_backend.py Show resolved Hide resolved
sky/backends/cloud_vm_ray_backend.py Show resolved Hide resolved
@Michaelvll Michaelvll merged commit bd0d224 into master Sep 29, 2022
@Michaelvll Michaelvll deleted the relative-cluster-yaml branch September 29, 2022 06:15
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.

Shared sky status
2 participants