Validate instance type before CloudFormation

ecs-cli/modules/cli/cluster/cluster_app.go

@@ -71,6 +71,11 @@ const (
 	ParameterKeySpotPrice                = "SpotPrice"
 )
 
+const (
+	invalidInstanceTypeFmt     = "instance type %s not found in list of supported instance types %s"
+	instanceTypeUnsupportedFmt = "instance type %s not supported in region %s: %w"
+)
+
 var flagNamesToStackParameterKeys map[string]string
 var requiredParameters []string = []string{ParameterKeyCluster}
 
@@ -314,6 +319,24 @@ func createCluster(context *cli.Context, awsClients *AWSClients, commandConfig *
 	}
 
 	if launchType == config.LaunchTypeEC2 {
+		instanceType, err := getInstanceType(cfnParams)
+		if err != nil {
+			return err
+		}
+		supportedInstanceTypes, err := awsClients.EC2Client.DescribeInstanceTypeOfferings(commandConfig.Region())
+		if err != nil {
+			return fmt.Errorf("describe instance type offerings: %w", err)
+		}
+
+		if err = validateInstanceType(instanceType, supportedInstanceTypes); err != nil {
+			// if we detect the default value is unsupported then we'll suggest to the user overriding the value with the appropriate flag
+			if instanceType == cloudformation.DefaultECSInstanceType {
+				logrus.Warnf("Default instance type %s not supported in region %s. Override the default instance type with the --%s flag and provide a supported value.",
+					instanceType, commandConfig.Region(), flags.InstanceTypeFlag)
+			}
+			return fmt.Errorf(instanceTypeUnsupportedFmt, instanceType, commandConfig.Region(), err)
+		}
+
 		// Check if image id was supplied, else populate
 		_, err = cfnParams.GetParameter(ParameterKeyAmiId)
 		if err == cloudformation.ParameterNotFoundError {
@@ -345,11 +368,7 @@ func createCluster(context *cli.Context, awsClients *AWSClients, commandConfig *
 		}
 	}
 	// Create cfn stack
-	instanceTypes, err := awsClients.EC2Client.DescribeInstanceTypeOfferings(commandConfig.Region())
-	if err != nil {
-		return errors.Wrapf(err, "No instance type found in region %s", commandConfig.Region())
-	}
-	template, err := cloudformation.GetClusterTemplate(tags, stackName, instanceTypes)
+	template, err := cloudformation.GetClusterTemplate(tags, stackName)
 	if err != nil {
 		return errors.Wrapf(err, "Error building cloudformation template")
 	}
@@ -387,21 +406,38 @@ func canEnableContainerInstanceTagging(client ecsclient.ECSClient) (bool, error)
 	return false, nil
 }
 
-func retrieveInstanceType(cfnParams *cloudformation.CfnStackParams) (string, error) {
+func getInstanceType(cfnParams *cloudformation.CfnStackParams) (string, error) {
 	param, err := cfnParams.GetParameter(ParameterKeyInstanceType)
-
 	if err == cloudformation.ParameterNotFoundError {
 		logrus.Infof("Defaulting instance type to %s", cloudformation.DefaultECSInstanceType)
+
+		cfnParams.Add(ParameterKeyInstanceType, cloudformation.DefaultECSInstanceType)
+
 		return cloudformation.DefaultECSInstanceType, nil
-	}
-	if err != nil {
+	} else if err != nil {
 		return "", err
 	}
+
 	return aws.StringValue(param.ParameterValue), nil
 }
 
+func validateInstanceType(instanceType string, supportedInstanceTypes []string) error {
+	found := false
+	for _, it := range supportedInstanceTypes {
+		if it == instanceType {
+			found = true
+			break
+		}
+	}
+	if !found {
+		return fmt.Errorf(invalidInstanceTypeFmt, instanceType, supportedInstanceTypes)
+	}
+
+	return nil
+}
+
 func populateAMIID(cfnParams *cloudformation.CfnStackParams, client amimetadata.Client) error {
-	instanceType, err := retrieveInstanceType(cfnParams)
+	instanceType, err := getInstanceType(cfnParams)
 	if err != nil {
 		return err
 	}

ecs-cli/modules/cli/cluster/cluster_app_test.go

@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"errors"
 	"flag"
+	"fmt"
 	"os"
 	"testing"
 
@@ -1024,7 +1025,7 @@ func TestClusterUpARM64(t *testing.T) {
 	)
 
 	gomock.InOrder(
-		mockEC2.EXPECT().DescribeInstanceTypeOfferings("us-west-1").Return([]string{"t2.micro"}, nil),
+		mockEC2.EXPECT().DescribeInstanceTypeOfferings("us-west-1").Return([]string{"t2.micro", "a1.medium"}, nil),
 	)
 
 	flagSet := flag.NewFlagSet("ecs-cli-up", 0)
@@ -1041,6 +1042,58 @@ func TestClusterUpARM64(t *testing.T) {
 	assert.NoError(t, err, "Unexpected error bringing up cluster")
 }
 
+func TestClusterUpWithUnsupportedInstanceType(t *testing.T) {
+	defer os.Clearenv()
+	mockECS, mockCloudformation, mockSSM, mockEC2 := setupTest(t)
+	awsClients := &AWSClients{mockECS, mockCloudformation, mockSSM, mockEC2}
+
+	instanceType := "a1.medium"
+	region := "us-west-1"
+	supportedInstanceTypes := []string{"t2.micro"}
+
+	invalidInstanceTypeErr := fmt.Errorf(invalidInstanceTypeFmt, instanceType, supportedInstanceTypes)
+	expectedError := fmt.Errorf(instanceTypeUnsupportedFmt,
+		instanceType, region, invalidInstanceTypeErr)
+
+	gomock.InOrder(
+		mockECS.EXPECT().CreateCluster(clusterName, gomock.Any()).Return(clusterName, nil),
+	)
+
+	gomock.InOrder(
+		mockSSM.EXPECT().GetRecommendedECSLinuxAMI(instanceType).Return(amiMetadata(armAMIID), nil),
+	)
+
+	gomock.InOrder(
+		mockCloudformation.EXPECT().ValidateStackExists(stackName).Return(errors.New("error")),
+		mockCloudformation.EXPECT().CreateStack(gomock.Any(), stackName, true, gomock.Any(), gomock.Any()).Do(func(v, w, x, y, z interface{}) {
+			capabilityIAM := x.(bool)
+			cfnParams := y.(*cloudformation.CfnStackParams)
+			amiIDParam, err := cfnParams.GetParameter(ParameterKeyAmiId)
+			assert.NoError(t, err, "Unexpected error getting cfn parameter")
+			assert.Equal(t, armAMIID, aws.StringValue(amiIDParam.ParameterValue), "Expected ami ID to be set to recommended for arm64")
+			assert.True(t, capabilityIAM, "Expected capability capabilityIAM to be true")
+		}).Return("", nil),
+		mockCloudformation.EXPECT().WaitUntilCreateComplete(stackName).Return(nil),
+	)
+
+	gomock.InOrder(
+		mockEC2.EXPECT().DescribeInstanceTypeOfferings(region).Return(supportedInstanceTypes, nil),
+	)
+
+	flagSet := flag.NewFlagSet("ecs-cli-up", 0)
+	flagSet.Bool(flags.CapabilityIAMFlag, true, "")
+	flagSet.String(flags.KeypairNameFlag, "default", "")
+	flagSet.String(flags.InstanceTypeFlag, instanceType, "")
+
+	context := cli.NewContext(nil, flagSet, nil)
+	rdwr := newMockReadWriter()
+	commandConfig, err := newCommandConfig(context, rdwr)
+	assert.NoError(t, err, "Unexpected error creating CommandConfig")
+
+	err = createCluster(context, awsClients, commandConfig)
+	assert.Equal(t, err, expectedError)
+}
+
 func TestClusterUpWithTags(t *testing.T) {
 	defer os.Clearenv()
 	mockECS, mockCloudformation, mockSSM, mockEC2 := setupTest(t)

ecs-cli/modules/clients/aws/cloudformation/cluster_template.go

@@ -21,7 +21,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ecs"
 )
 
-func GetClusterTemplate(tags []*ecs.Tag, stackName string, instanceTypes []string) (string, error) {
+func GetClusterTemplate(tags []*ecs.Tag, stackName string) (string, error) {
 	tagJSON, err := json.Marshal(tags)
 	if err != nil {
 		return "", err
@@ -33,12 +33,7 @@ func GetClusterTemplate(tags []*ecs.Tag, stackName string, instanceTypes []strin
 		return "", err
 	}
 
-	instanceTypesJSON, err := json.Marshal(instanceTypes)
-	if err != nil {
-		return "", err
-	}
-
-	return fmt.Sprintf(clusterTemplate, string(tagJSON), string(asgTagJSON), string(instanceTypesJSON)), nil
+	return fmt.Sprintf(clusterTemplate, string(tagJSON), string(asgTagJSON)), nil
 }
 
 // Autoscaling CFN tags have an additional field that determines if they are
@@ -115,9 +110,7 @@ var clusterTemplate = `
     "EcsInstanceType": {
       "Type": "String",
       "Description": "ECS EC2 instance type",
-      "Default": "` + DefaultECSInstanceType + `",
-      "AllowedValues": %[3]s,
-      "ConstraintDescription": "must be a valid EC2 instance type."
+      "Default": ""
     },
     "SpotPrice": {
       "Type": "Number",