Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions python/ray/autoscaler/azure/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@ filegroup(
srcs = ["defaults.yaml"],
visibility = ["//visibility:public"],
)

filegroup(
name = "test_configs",
data = glob(["tests/*.yaml"]),
visibility = ["//release:__pkg__"],
)
6 changes: 3 additions & 3 deletions python/ray/autoscaler/azure/tests/azure-cluster.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# Unique identifier for the head node and workers of this cluster.
cluster_name: nightly-cpu-minimal
cluster_name: nightly-cpu-minimal-centralus
max_workers: 2
idle_timeout_minutes: 5

# Cloud-provider specific configuration.
provider:
type: azure
# https://azure.microsoft.com/en-us/global-infrastructure/locations
location: westus2
resource_group: ray-nightly-cpu-minimal
location: centralus
resource_group: ray-nightly-cpu-minimal-centralus
cache_stopped_nodes: False

auth:
Expand Down
15 changes: 15 additions & 0 deletions python/ray/autoscaler/azure/tests/azure_compute.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# This test launches an Azure VM cluster from an AWS instance.
# The test script runs on AWS while the actual cluster is created in Azure.
cloud_id: {{env["ANYSCALE_CLOUD_ID"]}}
region: us-west-2

head_node_type:
name: head_node
instance_type: t3.large

worker_node_types:
- name: worker_node
instance_type: t3.large
min_workers: 0
max_workers: 0
use_spot: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Azure Tests Fail Due to Incorrect AWS Configuration

The azure_compute.yaml file, intended for Azure cluster launcher tests, incorrectly specifies AWS configuration. It uses AWS region us-west-2 and instance type t3.large, which causes Azure tests to fail.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@khluu - maybe consider adding a comment saying this we run launch_and_verify_cluster.py from an a AWS VM but the ray cluster gets created in azure?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

68 changes: 61 additions & 7 deletions python/ray/autoscaler/launch_and_verify_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<cluster_configuration_file_path>
"""
import argparse
import json
import os
import re
import subprocess
Expand Down Expand Up @@ -143,6 +144,51 @@ def override_docker_image(config_yaml, docker_image):
config_yaml["docker"] = docker_config


def azure_authenticate():
"""Get Azure service principal credentials from AWS Secrets Manager and authenticate."""
print("======================================")
print("Getting Azure credentials from AWS Secrets Manager...")

# Initialize AWS Secrets Manager client
secrets_client = boto3.client("secretsmanager", region_name="us-west-2")

# Get service principal credentials
secret_response = secrets_client.get_secret_value(
SecretId="azure-service-principal-oss-release"
)
secret = secret_response["SecretString"]

client_id = json.loads(secret)["client_id"]
tenant_id = json.loads(secret)["tenant_id"]

# Get certificate
cert_response = secrets_client.get_secret_value(
SecretId="azure-service-principal-certificate"
)
cert = cert_response["SecretString"]

# Write certificate to temp file
tmp_dir = tempfile.mkdtemp()
cert_path = os.path.join(tmp_dir, "azure_cert.pem")
with open(cert_path, "w") as f:
f.write(cert)

# Login to Azure
subprocess.check_call(
[
"az",
"login",
"--service-principal",
"--username",
client_id,
"--certificate",
cert_path,
"--tenant",
tenant_id,
]
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Temporary Files Not Cleaned Up

The azure_authenticate function creates a temporary directory and certificate file but doesn't clean them up. This results in a resource leak, with temporary files accumulating on the filesystem.

Fix in Cursor Fix in Web

Copy link
Copy Markdown

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.

Fix in Cursor Fix in Web



def download_ssh_key_aws():
"""Download the ssh key from the S3 bucket to the local machine."""
print("======================================")
Expand Down Expand Up @@ -208,10 +254,13 @@ def cleanup_cluster(config_yaml, cluster_config):
num_tries = 3
for i in range(num_tries):
try:
env = os.environ.copy()
env.pop("PYTHONPATH", None)
subprocess.run(
["ray", "down", "-v", "-y", str(cluster_config)],
check=True,
capture_output=True,
env=env,
)
cleanup_security_groups(config_yaml)
# Final success
Expand Down Expand Up @@ -321,7 +370,11 @@ def cleanup_security_groups(config):


def run_ray_commands(
config_yaml, cluster_config, retries, no_config_cache, num_expected_nodes=1
config_yaml,
cluster_config,
retries,
no_config_cache,
num_expected_nodes=1,
):
"""
Run the necessary Ray commands to start a cluster, verify Ray is running, and clean
Expand All @@ -333,18 +386,19 @@ def run_ray_commands(
no_config_cache: Whether to pass the --no-config-cache flag to the ray CLI
commands.
"""

provider_type = config_yaml.get("provider", {}).get("type")
if provider_type == "azure":
azure_authenticate()
Comment thread
marosset marked this conversation as resolved.
print("======================================")
print("Starting new cluster...")
cmd = ["ray", "up", "-v", "-y"]
if no_config_cache:
cmd.append("--no-config-cache")
cmd.append(str(cluster_config))

print(" ".join(cmd))

env = os.environ.copy()
env.pop("PYTHONPATH", None)
try:
subprocess.run(cmd, check=True, capture_output=True)
subprocess.run(cmd, check=True, capture_output=True, env=env)
except subprocess.CalledProcessError as e:
print(e.output)
# print stdout and stderr
Expand All @@ -371,7 +425,7 @@ def run_ray_commands(
]
if no_config_cache:
cmd.append("--no-config-cache")
subprocess.run(cmd, check=True)
subprocess.run(cmd, check=True, env=env)
success = True
break
except subprocess.CalledProcessError:
Expand Down
1 change: 1 addition & 0 deletions release/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ py_test(
) + [
"ray_release/tests/test_collection_data.yaml",
"//python/ray/autoscaler/aws:test_configs",
"//python/ray/autoscaler/azure:test_configs",
"//python/ray/autoscaler/gcp:test_configs",
],
exec_compatible_with = ["//:hermetic_python"],
Expand Down
6 changes: 6 additions & 0 deletions release/ray_release/byod/byod_azure_cluster_launcher.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash
# This script is used to build an extra layer to run release tests on Azure.

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

Copy link
Copy Markdown
Contributor

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?

21 changes: 21 additions & 0 deletions release/release_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3476,6 +3476,27 @@
prepare: bash prepare.sh
script: python run_gcs_ft_on_k8s.py

- name: azure_cluster_launcher
group: cluster-launcher-test
working_dir: ../python/ray/autoscaler/
frequency: nightly
team: clusters
cluster:
byod:
post_build_script: byod_azure_cluster_launcher.sh
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Incorrect Script Path in Azure Cluster Test

The azure_cluster_launcher test's script path release/azure_docker_login.sh is relative to the working_dir (../python/ray/autoscaler/). This path is incorrect, and the script won't be found.

Fix in Cursor Fix in Web


variations:
- __suffix__: v1
run:
script: RAY_UP_enable_autoscaler_v2=0 python -I launch_and_verify_cluster.py azure/tests/azure-cluster.yaml --num-expected-nodes 3 --retries 10
- __suffix__: v2
run:
script: RAY_UP_enable_autoscaler_v2=1 python -I launch_and_verify_cluster.py azure/tests/azure-cluster.yaml --num-expected-nodes 3 --retries 10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Azure Cluster Tests Missing Docker Login

The azure_cluster_launcher test variations (v1, v2) omit the bash release/azure_docker_login.sh command, which is present in the main test's run.script. This will likely cause authentication failures for the variations.

Fix in Cursor Fix in Web


- name: aws_cluster_launcher
group: cluster-launcher-test
working_dir: ../python/ray/autoscaler/
Expand Down