Skip to content

Commit

Permalink
Detect Graviton 2 instance as arm64 Fix #1113 (#1116)
Browse files Browse the repository at this point in the history
* Detect Graviton 2 instance as arm64 Fix #1113

- The original PR #695 only handles a1, which is the only arm instance
  type at that time. Now there are more instance types like c6g etc.
- Tweak the unit test a bit so instance types using same ami are merged into
  one test case.

* Add more generic instance types in AMI unit test
  • Loading branch information
pingleig authored Dec 10, 2020
1 parent bbd651c commit 4c40431
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
7 changes: 6 additions & 1 deletion ecs-cli/modules/clients/aws/amimetadata/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,13 @@ func (c *metadataClient) parameterValueFor(ssmParamName string) (*AMIMetadata, e
return metadata, err
}

// See: https://aws.amazon.com/ec2/instance-types/
// a1 is the first generation of graviton processors.
// t4g, m6g, c6g, r6g are using graviton 2.
// The d suffix is for disk optimized and applies to all except a1 and t4g, e.g. m6gd.medium.
// Invalid instance type like t4gd.nano will trigger validation error in API so we don't do validation here.
func isARM64Instance(instanceType string) bool {
r := regexp.MustCompile("a1\\.(medium|\\d*x?large)")
r := regexp.MustCompile("(a1|.\\dgd?)\\.(medium|\\d*x?large|metal)")
if r.MatchString(instanceType) {
return true
}
Expand Down
47 changes: 19 additions & 28 deletions ecs-cli/modules/clients/aws/amimetadata/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ type Configurer func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSM

func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
tests := []struct {
instanceType string
instanceTypes []string
configureMock Configurer
expectedErr error
}{
{
// validate that we use the ARM64 optimized AMI for Arm instances
"a1.medium",
[]string{"a1.medium", "m6g.medium", "c6gd.16xlarge", "m6g.metal"},
func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
assert.Equal(t, amazonLinux2ARM64RecommendedParameterName, *input.Name)
Expand All @@ -32,18 +32,7 @@ func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
},
{
// validate that we use GPU optimized AMI for GPU instances
"p2.large",
func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
assert.Equal(t, amazonLinux2X86GPURecommendedParameterName, *input.Name)
}).Return(emptySSMParameterOutput(), nil)
return ssmClient
},
nil,
},
{
// validate that we use GPU optimized AMI for GPU instances
"g4dn.xlarge",
[]string{"p2.large", "g4dn.xlarge"},
func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
assert.Equal(t, amazonLinux2X86GPURecommendedParameterName, *input.Name)
Expand All @@ -54,7 +43,7 @@ func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
},
{
// validate that we use the generic AMI for other instances
"t2.micro",
[]string{"t2.micro", "m5ad.large", "c4.large", "i3.2xlarge"},
func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
assert.Equal(t, amazonLinux2X86RecommendedParameterName, *input.Name)
Expand All @@ -65,7 +54,7 @@ func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
},
{
// validate that we throw an error if the AMI is not available in a region
"t2.micro",
[]string{"t2.micro"},
func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
assert.Equal(t, amazonLinux2X86RecommendedParameterName, *input.Name)
Expand All @@ -79,7 +68,7 @@ func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
},
{
// validate that we throw unexpected errors
"t2.micro",
[]string{"t2.micro"},
func(ssmClient *mock_ssmiface.MockSSMAPI) *mock_ssmiface.MockSSMAPI {
ssmClient.EXPECT().GetParameter(gomock.Any()).Do(func(input *ssm.GetParameterInput) {
assert.Equal(t, amazonLinux2X86RecommendedParameterName, *input.Name)
Expand All @@ -91,19 +80,21 @@ func TestMetadataClient_GetRecommendedECSLinuxAMI(t *testing.T) {
}

for _, test := range tests {
m := newMockSSMAPI(t)
test.configureMock(m)
for _, instanceType := range test.instanceTypes {
m := newMockSSMAPI(t)
test.configureMock(m)

c := metadataClient{
m,
"us-east-1",
}
_, actualErr := c.GetRecommendedECSLinuxAMI(test.instanceType)
c := metadataClient{
m,
"us-east-1",
}
_, actualErr := c.GetRecommendedECSLinuxAMI(instanceType)

if test.expectedErr == nil {
assert.NoError(t, actualErr)
} else {
assert.EqualError(t, actualErr, test.expectedErr.Error())
if test.expectedErr == nil {
assert.NoError(t, actualErr)
} else {
assert.EqualError(t, actualErr, test.expectedErr.Error())
}
}
}
}
Expand Down

0 comments on commit 4c40431

Please sign in to comment.