diff --git a/python/ray/autoscaler/_private/aws/config.py b/python/ray/autoscaler/_private/aws/config.py index fe9fc0c59965..da66dc763066 100644 --- a/python/ray/autoscaler/_private/aws/config.py +++ b/python/ray/autoscaler/_private/aws/config.py @@ -590,16 +590,20 @@ def _check_ami(config): region = config["provider"]["region"] default_ami = DEFAULT_AMI.get(region) - if not default_ami: - # If we do not provide a default AMI for the given region, noop. - return for key, node_type in config["available_node_types"].items(): node_config = node_type["node_config"] node_ami = node_config.get("ImageId", "").lower() if node_ami in ["", "latest_dlami"]: - node_config["ImageId"] = default_ami - ami_src_info[key] = "dlami" + if not default_ami: + cli_logger.abort( + f"Node type `{key}` has no ImageId in its node_config " + f"and no default AMI is available for the region `{region}`. " + "ImageId will need to be set manually in your cluster config." + ) + else: + node_config["ImageId"] = default_ami + ami_src_info[key] = "dlami" def _upsert_security_groups(config, node_types): diff --git a/python/ray/autoscaler/aws/defaults.yaml b/python/ray/autoscaler/aws/defaults.yaml index 0eaa79f62466..2d3bd7b40ff4 100644 --- a/python/ray/autoscaler/aws/defaults.yaml +++ b/python/ray/autoscaler/aws/defaults.yaml @@ -56,7 +56,6 @@ available_node_types: # http://boto3.readthedocs.io/en/latest/reference/services/ec2.html#EC2.ServiceResource.create_instances node_config: InstanceType: m5.large - ImageId: ami-0a2363a9cff180a64 # Deep Learning AMI (Ubuntu) Version 30 # You can provision additional disk space with a conf as follows BlockDeviceMappings: - DeviceName: /dev/sda1 @@ -79,7 +78,6 @@ available_node_types: # http://boto3.readthedocs.io/en/latest/reference/services/ec2.html#EC2.ServiceResource.create_instances node_config: InstanceType: m5.large - ImageId: ami-0a2363a9cff180a64 # Deep Learning AMI (Ubuntu) Version 30 # Run workers on spot by default. Comment this out to use on-demand. InstanceMarketOptions: MarketType: spot diff --git a/python/ray/tests/aws/conftest.py b/python/ray/tests/aws/conftest.py index 68ab582dca8f..e2c9c946e77d 100644 --- a/python/ray/tests/aws/conftest.py +++ b/python/ray/tests/aws/conftest.py @@ -7,16 +7,18 @@ @pytest.fixture() -def iam_client_stub(): - resource = resource_cache("iam", "us-west-2") +def iam_client_stub(request): + region = getattr(request, "param", "us-west-2") + resource = resource_cache("iam", region) with Stubber(resource.meta.client) as stubber: yield stubber stubber.assert_no_pending_responses() @pytest.fixture() -def ec2_client_stub(): - resource = resource_cache("ec2", "us-west-2") +def ec2_client_stub(request): + region = getattr(request, "param", "us-west-2") + resource = resource_cache("ec2", region) with Stubber(resource.meta.client) as stubber: yield stubber stubber.assert_no_pending_responses() diff --git a/python/ray/tests/aws/test_autoscaler_aws.py b/python/ray/tests/aws/test_autoscaler_aws.py index 7a22d3b7aa8c..c64eb84d9156 100644 --- a/python/ray/tests/aws/test_autoscaler_aws.py +++ b/python/ray/tests/aws/test_autoscaler_aws.py @@ -223,10 +223,24 @@ def test_subnet_given_head_and_worker_sg(iam_client_stub, ec2_client_stub): ec2_client_stub.assert_no_pending_responses() -def test_fills_out_amis_and_iam(iam_client_stub, ec2_client_stub): +# Parametrize across multiple regions, since default AMI is different in each +@pytest.mark.parametrize( + "iam_client_stub,ec2_client_stub,region", + [3 * (region,) for region in DEFAULT_AMI], + indirect=["iam_client_stub", "ec2_client_stub"], +) +def test_fills_out_amis_and_iam(iam_client_stub, ec2_client_stub, region): + # Set up expected key pair for specific region + region_key_pair = DEFAULT_KEY_PAIR.copy() + region_key_pair["KeyName"] = DEFAULT_KEY_PAIR["KeyName"].replace( + "us-west-2", region + ) + # Setup stubs to mock out boto3 stubs.configure_iam_role_default(iam_client_stub) - stubs.configure_key_pair_default(ec2_client_stub) + stubs.configure_key_pair_default( + ec2_client_stub, region=region, expected_key_pair=region_key_pair + ) stubs.describe_a_security_group(ec2_client_stub, DEFAULT_SG) stubs.configure_subnet_default(ec2_client_stub) @@ -243,6 +257,8 @@ def test_fills_out_amis_and_iam(iam_client_stub, ec2_client_stub): head_node_config["SecurityGroupIds"] = ["sg-1234abcd"] worker_node_config["SecurityGroupIds"] = ["sg-1234abcd"] + config["provider"]["region"] = region + defaults_filled = bootstrap_aws(config) ami = DEFAULT_AMI.get(defaults_filled.get("provider", {}).get("region")) diff --git a/python/ray/tests/aws/utils/stubs.py b/python/ray/tests/aws/utils/stubs.py index afe77fdce256..32877167e5b1 100644 --- a/python/ray/tests/aws/utils/stubs.py +++ b/python/ray/tests/aws/utils/stubs.py @@ -2,7 +2,6 @@ import copy from ray.tests.aws.utils import helpers -from ray.tests.aws.utils.mocks import mock_path_exists_key_pair from ray.tests.aws.utils.constants import ( DEFAULT_INSTANCE_PROFILE, DEFAULT_KEY_PAIR, @@ -11,6 +10,7 @@ DEFAULT_LT, TWENTY_SUBNETS_IN_DIFFERENT_AZS, ) +from ray.autoscaler._private.aws.config import key_pair from unittest import mock @@ -27,17 +27,24 @@ def configure_iam_role_default(iam_client_stub): ) -def configure_key_pair_default(ec2_client_stub): +def configure_key_pair_default( + ec2_client_stub, region="us-west-2", expected_key_pair=DEFAULT_KEY_PAIR +): patcher = mock.patch("os.path.exists") + + def mock_path_exists_key_pair(path): + _, key_path = key_pair(0, region, expected_key_pair["KeyName"]) + return path == key_path + os_path_exists_mock = patcher.start() os_path_exists_mock.side_effect = mock_path_exists_key_pair ec2_client_stub.add_response( "describe_key_pairs", expected_params={ - "Filters": [{"Name": "key-name", "Values": [DEFAULT_KEY_PAIR["KeyName"]]}] + "Filters": [{"Name": "key-name", "Values": [expected_key_pair["KeyName"]]}] }, - service_response={"KeyPairs": [DEFAULT_KEY_PAIR]}, + service_response={"KeyPairs": [expected_key_pair]}, )