-
Notifications
You must be signed in to change notification settings - Fork 7k
[release] Add Azure VM launcher release test. #57921
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
Conversation
Signed-off-by: kevin <kevin@anyscale.com>
|
Could you paste a link of a success run? |
aslonnie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me know when this is ready to review.
|
|
||
| set -exo pipefail | ||
|
|
||
| pip3 install azure-cli-core==2.21.0 azure-core azure-identity azure-mgmt-compute azure-mgmt-network azure-mgmt-resource azure-common msrest msrestazure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use depset for this?
I thought many of the deps are already installed in the image?
| "--tenant", | ||
| tenant_id, | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Azure Auth Fails Without AWS Credentials
The azure_authenticate() function is called unconditionally for all Azure provider types, but it requires AWS credentials to access AWS Secrets Manager. This will fail when users try to run the script locally without AWS credentials, making the script difficult to use outside of CI. According to the PR discussion, this makes it "difficult to run the script from as someone who doesn't have access to the AWS bucket with the secrets in there." The authentication logic should be moved to a separate script or made optional to allow local development and testing.
| cluster_compute: azure/tests/azure_compute.yaml | ||
| run: | ||
| timeout: 2400 | ||
| script: bash release/azure_docker_login.sh && python -I launch_and_verify_cluster.py azure/tests/azure-cluster.yaml --num-expected-nodes 3 --retries 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add Azure VM launcher release test - Change region for the Azure cluster to be in `centralus` since `westus2` has trouble with availability. - Add helper function to authenticate with Azure using service principal in launch cluster script --------- Signed-off-by: kevin <kevin@anyscale.com>
- Add Azure VM launcher release test - Change region for the Azure cluster to be in `centralus` since `westus2` has trouble with availability. - Add helper function to authenticate with Azure using service principal in launch cluster script --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
centralussincewestus2has trouble with availability.