Skip to content

Commit

Permalink
Handling specific exception codes on RCI call.
Browse files Browse the repository at this point in the history
  • Loading branch information
BinBin He committed Dec 23, 2024
1 parent 41d593c commit 49c48be
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 0 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions ecs-agent/api/ecs/client/ecs_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"errors"
"fmt"
"net/http"
"os"
"strings"
"time"

Expand Down Expand Up @@ -231,6 +232,16 @@ func (client *ecsClient) registerContainerInstance(clusterRef string, containerI
registerRequest.ClientToken = &registrationToken
resp, err := client.standardClient.RegisterContainerInstance(&registerRequest)
if err != nil {

// On error codes InvalidParameterException and Client exception, there is no need to retry as
// retries will not resolve the issue. Exit 5 to terminally stop agent process to avoid a retry storm
// due to agent restart.
if utils.IsAWSErrorCodeEqual(err, ecsmodel.ErrCodeInvalidParameterException) ||
utils.IsAWSErrorCodeEqual(err, ecsmodel.ErrCodeAccessDeniedException) {
os.Exit(5)
}
// All other exceptions is deferred to agent retry per existing retry strategy
// including throttlingExceptions.
logger.Error("Unable to register as a container instance with ECS", logger.Fields{
field.Error: err,
})
Expand Down
56 changes: 56 additions & 0 deletions ecs-agent/api/ecs/client/ecs_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package ecsclient
import (
"errors"
"fmt"
"os"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -67,6 +68,7 @@ const (
)

var (
osExit = os.Exit
containerInstanceTags = []*ecsmodel.Tag{
{
Key: aws.String("my_key1"),
Expand Down Expand Up @@ -462,6 +464,60 @@ func TestRegisterContainerInstance(t *testing.T) {
}
}

func TestExceptionsRegisterContainerInstance(t *testing.T) {
// Setup.
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockEC2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl)
tester := setup(t, ctrl, mockEC2Metadata, nil)

expectedAttributes := map[string]string{
"ecs.os-type": tester.mockCfgAccessor.OSType(),
"ecs.os-family": tester.mockCfgAccessor.OSFamily(),
"my_custom_attribute": "Custom_Value1",
"my_other_custom_attribute": "Custom_Value2",
}
fakeCapabilities := []string{"capability1", "capability2"}

// On return of InvalidParameterException error.
expectedError := awserr.New(ecsmodel.ErrCodeInvalidParameterException, "Invalid parameter", nil)

gomock.InOrder(
mockEC2Metadata.EXPECT().GetDynamicData(ec2.InstanceIdentityDocumentResource).
Return("instanceIdentityDocument", nil),
mockEC2Metadata.EXPECT().GetDynamicData(ec2.InstanceIdentityDocumentSignatureResource).
Return("signature", nil),
tester.mockStandardClient.EXPECT().RegisterContainerInstance(gomock.Any()).
Do(func(req *ecsmodel.RegisterContainerInstanceInput) {
assert.Nil(t, req.Tags)
}).Return(nil, expectedError),
)

// Prevent the test from actually exiting
oldOsExit := osExit
defer func() { osExit = oldOsExit }()
var exitCode int
osExit = func(code int) {
exitCode = 5
panic("os.Exit called")
}

// Validate the expected errors are returned.
_, _, err := tester.client.RegisterContainerInstance("", nil, make([]*ecsmodel.Tag, 0),
"", nil, "")
assert.Error(t, err)
assert.Equal(t, expectedError, err)

// Check if os.Exit was called with the expected exit code
assert.Panics(t, func() {
_, _, _ = tester.client.RegisterContainerInstance("", nil, make([]*ecsmodel.Tag, 0),
"", nil, "")
})
assert.Equal(t, 5, exitCode)

}

func TestReRegisterContainerInstance(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
Expand Down
9 changes: 9 additions & 0 deletions ecs-agent/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"strconv"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"golang.org/x/exp/constraints"
)

Expand Down Expand Up @@ -68,3 +69,11 @@ func MaxNum[T constraints.Integer | constraints.Float](a, b T) T {
}
return b
}

// IsAWSErrorCodeEqual returns true if the err implements Error
// interface of awserr and it has the same error code as
// the passed in error code.
func IsAWSErrorCodeEqual(err error, code string) bool {
awsErr, ok := err.(awserr.Error)
return ok && awsErr.Code() == code
}
33 changes: 33 additions & 0 deletions ecs-agent/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@
package utils

import (
"errors"
"strconv"
"testing"

"github.com/aws/amazon-ecs-agent/ecs-agent/api/ecs/model/ecs"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -129,3 +132,33 @@ func testMaxNumFloat(t *testing.T) {
require.Equal(t, largerVal, MaxNum(largerVal, smallerVal))
require.Equal(t, largerVal, MaxNum(largerVal, largerVal))
}

func TestIsAWSErrorCodeEqual(t *testing.T) {
testcases := []struct {
name string
err error
res bool
}{
{
name: "Happy Path",
err: awserr.New(ecs.ErrCodeInvalidParameterException, "errMsg", errors.New("err")),
res: true,
},
{
name: "Wrong Error Code",
err: awserr.New("errCode", "errMsg", errors.New("err")),
res: false,
},
{
name: "Wrong Error Type",
err: errors.New("err"),
res: false,
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.res, IsAWSErrorCodeEqual(tc.err, ecs.ErrCodeInvalidParameterException))
})
}
}

0 comments on commit 49c48be

Please sign in to comment.