[aws][autoscaler] fix regional default AMI's

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):

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

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()

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"))

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]},
     )