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

[Image] Check image size and existence #1508

Merged
merged 8 commits into from
Dec 19, 2022

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Dec 10, 2022

Describe the changes in this PR:

This PR adds the check for the image size and existence, so that any problematic image id will fail early. It should also fix #1506

Tested (run the relevant ones):

  • sky launch --cloud aws --region us-east-1 --image-id ami-nonexist
  • sky launch --cloud aws --region us-east-1 --image-id skypilot:non-exist
  • sky launch --cloud aws --image-id skypilot:non-exist
  • sky launch --cloud gcp --image-id project/nonexist
  • sky launch --cloud gcp --region us-east1 --image-id project/nonexist
  • sky launch --cloud aws --image-id ami-0729d913a335efca7 --region us-east-1 --disk-size 40
    ValueError: Image 'ami-0729d913a335efca7' is 45GB, which is larger than the specified disk_size: 40 GB. Please specify a larger disk_size.
  • sky launch --cloud gcp --image-id projects/deeplearning-platform-release/global/images/common-cu113-v20220701 --disk-size 40
    ValueError: Image 'projects/deeplearning-platform-release/global/images/common-cu113-v20220701' is 50.0GB, which is larger than the specified disk_size: 40 GB. Please specify a larger disk_size.
  • sky spot launch --cloud aws --image-id ami-0729d913a335efca7 --region us-east-1 --disk-size 40 echo hi
    ValueError: Image 'ami-0729d913a335efca7' is 45GB, which is larger than the specified disk_size: 40 GB. Please specify a larger disk_size.
  • All smoke tests: bash tests/run_smoke_tests.sh (except [Storage] Uploading list of files to bucket fails #1510, and test_spot due to the autostop of the controller)

@Michaelvll Michaelvll marked this pull request as ready for review December 10, 2022 23:17
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.

This is great @Michaelvll! Took a pass and left some comments. Haven't tested yet.

sky/backends/cloud_vm_ray_backend.py Outdated Show resolved Hide resolved
sky/adaptors/aws.py Outdated Show resolved Hide resolved
sky/clouds/aws.py Show resolved Hide resolved
sky/clouds/aws.py Outdated Show resolved Hide resolved
sky/clouds/gcp.py Show resolved Hide resolved
sky/clouds/gcp.py Outdated Show resolved Hide resolved
sky/clouds/aws.py Outdated Show resolved Hide resolved
sky/resources.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.

Tested with some simple manual tests - works great! Thanks for adding @Michaelvll!

@Michaelvll Michaelvll merged commit 309ecb0 into master Dec 19, 2022
@Michaelvll Michaelvll deleted the check-image-size-and-existence branch December 19, 2022 18:53
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.

disk_size issue when specify image_id in resource
2 participants