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

Support start and stop Azure instances #316

Merged
merged 5 commits into from
Feb 15, 2022
Merged

Support start and stop Azure instances #316

merged 5 commits into from
Feb 15, 2022

Conversation

suquark
Copy link
Collaborator

@suquark suquark commented Feb 14, 2022

Update azure-cli to 2.30.0. Update smoke tests.

Closes #148

  • Unit tests
  • Smoke tests

@suquark suquark changed the title Support start and stop Azure instances [WIP] Support start and stop Azure instances Feb 14, 2022
@suquark suquark added the blocked PR blocked by other issues label Feb 14, 2022
@suquark suquark removed the blocked PR blocked by other issues label Feb 15, 2022
@suquark suquark changed the title [WIP] Support start and stop Azure instances Support start and stop Azure instances Feb 15, 2022
@suquark suquark added the do not merge do not merge this PR now label Feb 15, 2022
@suquark
Copy link
Collaborator Author

suquark commented Feb 15, 2022

Note: ready for review. The test failure is related to new azure-cli versions, will be fixed later

@suquark suquark removed the do not merge do not merge this PR now label Feb 15, 2022
@suquark
Copy link
Collaborator Author

suquark commented Feb 15, 2022

Ready for review.

Copy link
Member

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

Looks good. @Michaelvll should know more about the versioning fixes.

prototype/sky/backends/backend_utils.py Show resolved Hide resolved
prototype/sky/cloud_adaptors/azure.py Outdated Show resolved Hide resolved
@@ -197,10 +197,11 @@ def check_credentials(self) -> Tuple[bool, Optional[str]]:
# This file is required because it will be synced to remote VMs for
# `az` to access private storage buckets.
# `az account show` does not guarantee this file exists.
if not os.path.isfile(os.path.expanduser('~/.azure/accessTokens.json')):
azure_token_cache_file = '~/.azure/msal_token_cache.json'
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between:

  • ~/.azure/accessTokens.json
  • ~/.azure/msal_token_cache.json (sounds like a cache)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

~/.azure/accessTokens.json is the cache for older azure-cli, ~/.azure/msal_token_cache.json is the cache for the new version

Copy link
Collaborator

@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.

Upgrading the azure-cli sounds good to me. With this, I think we can upgrade the remote ray version safely without upgrading the local ray version.

@suquark suquark merged commit addcaac into master Feb 15, 2022
@suquark suquark deleted the azure_start_stop branch February 15, 2022 19:25
@Michaelvll Michaelvll mentioned this pull request Feb 15, 2022
gmittal pushed a commit that referenced this pull request Mar 15, 2022
* azure external node provider

* allow stopping Azure

* update token check
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.

GCP/Azure do not support sky stop
3 participants