Skip to content

Commit

Permalink
update more error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinnywang
tinnywang committed Dec 5, 2024
1 parent ff2fcb2 commit 9eee623
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 42 deletions.
6 changes: 4 additions & 2 deletions agent/app/agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
smithy "github.com/aws/smithy-go"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1059,8 +1060,9 @@ func TestReregisterContainerInstanceInstanceTypeChanged(t *testing.T) {
mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil),
client.EXPECT().RegisterContainerInstance(containerInstanceARN, gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any()).Return("", "", awserr.New("",
apierrors.InstanceTypeChangedErrorMessage, errors.New(""))),
gomock.Any(), gomock.Any()).Return("", "", &smithy.GenericAPIError{
Message: apierrors.InstanceTypeChangedErrorMessage,
}),
)

cfg := getTestConfig()
Expand Down
2 changes: 1 addition & 1 deletion agent/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/aws/aws-sdk-go-v2/config v1.28.1
github.com/aws/aws-sdk-go-v2/credentials v1.17.42
github.com/aws/aws-sdk-go-v2/service/ecs v1.47.3
github.com/aws/smithy-go v1.22.0
github.com/awslabs/go-config-generator-for-fluentd-and-fluentbit v0.0.0-20210308162251-8959c62cb8f9
github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575
github.com/container-storage-interface/spec v1.8.0
Expand Down Expand Up @@ -52,7 +53,6 @@ require (
github.com/aws/aws-sdk-go-v2/service/sso v1.24.3 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.3 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.32.3 // indirect
github.com/aws/smithy-go v1.22.0 // indirect
github.com/cilium/ebpf v0.16.0 // indirect
github.com/containerd/containerd v1.7.24 // indirect
github.com/containerd/log v0.1.0 // indirect
Expand Down
76 changes: 51 additions & 25 deletions agent/handlers/task_server_setup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,14 @@ import (
v2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
v4 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v4/state"
mock_execwrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/execwrapper/mocks"
smithy "github.com/aws/smithy-go"

awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/aws-sdk-go-v2/service/ecs"
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/request"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/gorilla/mux"
Expand Down Expand Up @@ -3287,11 +3289,17 @@ func TestGetTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeServerException, ecsErrMessage, nil),
http.StatusInternalServerError,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusInternalServerError,
},
},
Err: &ecstypes.ServerException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3311,11 +3319,17 @@ func TestGetTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeAccessDeniedException, ecsErrMessage, nil),
http.StatusBadRequest,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusBadRequest,
},
},
Err: &ecstypes.AccessDeniedException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusBadRequest,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3335,7 +3349,7 @@ func TestGetTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.New(apierrors.ErrCodeInvalidParameterException, ecsErrMessage, nil)),
&ecstypes.InvalidParameterException{Message: &ecsErrMessage}),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand All @@ -3352,7 +3366,7 @@ func TestGetTaskProtection(t *testing.T) {
setStateExpectations: happyStateExpectations,
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil, awserr.New(request.CanceledErrorCode, "request cancelled", nil)),
nil, &smithy.CanceledError{}),
expectedStatusCode: http.StatusGatewayTimeout,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand Down Expand Up @@ -3561,11 +3575,17 @@ func TestUpdateTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeServerException, ecsErrMessage, nil),
http.StatusInternalServerError,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusInternalServerError,
},
},
Err: &ecstypes.ServerException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3583,11 +3603,17 @@ func TestUpdateTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.NewRequestFailure(
awserr.New(apierrors.ErrCodeAccessDeniedException, ecsErrMessage, nil),
http.StatusBadRequest,
ecsRequestID,
),
&awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusBadRequest,
},
},
Err: &ecstypes.AccessDeniedException{Message: &ecsErrMessage},
},
RequestID: ecsRequestID,
},
),
expectedStatusCode: http.StatusBadRequest,
expectedResponseBody: tptypes.TaskProtectionResponse{
Expand All @@ -3605,7 +3631,7 @@ func TestUpdateTaskProtection(t *testing.T) {
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil,
awserr.New(apierrors.ErrCodeInvalidParameterException, ecsErrMessage, nil)),
&ecstypes.InvalidParameterException{Message: &ecsErrMessage}),
expectedStatusCode: http.StatusInternalServerError,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand All @@ -3620,7 +3646,7 @@ func TestUpdateTaskProtection(t *testing.T) {
setStateExpectations: happyStateExpectations,
setCredentialsManagerExpectations: happyCredentialsManagerExpectations,
setTaskProtectionClientFactoryExpectations: taskProtectionClientFactoryExpectations(
nil, awserr.New(request.CanceledErrorCode, "request cancelled", nil)),
nil, &smithy.CanceledError{}),
expectedStatusCode: http.StatusGatewayTimeout,
expectedResponseBody: tptypes.TaskProtectionResponse{
Error: &tptypes.ErrorResponse{
Expand Down
20 changes: 17 additions & 3 deletions agent/handlers/v2/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
package v2

import (
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/smithy-go"
"github.com/cihub/seelog"
"github.com/pkg/errors"

apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
Expand All @@ -25,9 +29,7 @@ import (
tmdsresponse "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/response"
"github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/utils"
tmdsv2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
"github.com/aws/aws-sdk-go/aws"
"github.com/cihub/seelog"
"github.com/pkg/errors"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
)

// Agent versions >= 1.2.0: Null, zero, and CPU values of 1
Expand Down Expand Up @@ -280,5 +282,17 @@ func newErrorResponse(err error, field, resourceARN string) *tmdsv2.ErrorRespons
}
}

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
errResp.ErrorCode = apiErr.ErrorCode()
errResp.ErrorMessage = apiErr.ErrorMessage()
}

var re *awshttp.ResponseError
if errors.As(err, &re) {
errResp.StatusCode = re.HTTPStatusCode()
errResp.RequestId = re.RequestID
}

return errResp
}
38 changes: 29 additions & 9 deletions agent/handlers/v2/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v2
import (
"encoding/json"
"fmt"
"net/http"
"testing"
"time"

Expand All @@ -30,12 +31,12 @@ import (
apitaskstatus "github.com/aws/amazon-ecs-agent/ecs-agent/api/task/status"
ni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
tmdsv2 "github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v2"
"github.com/aws/aws-sdk-go-v2/aws"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
ecstypes "github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -679,33 +680,52 @@ func TestTaskResponseWithV4TagsError(t *testing.T) {
},
}

errCode := "ThrottlingException"
errMessage := "Rate exceeded"
errStatusCode := 400
containerTagsRequestId := "cef9da77-aee7-431d-84d5-f92b2d342c51"
taskTagsRequestId := "45dbbc67-0c60-4248-855e-14fdf4c11870"
containerTagsErr := awserr.NewRequestFailure(awserr.Error(awserr.New(errCode, errMessage, errors.New(""))), errStatusCode, containerTagsRequestId)
taskTagsError := awserr.NewRequestFailure(awserr.Error(awserr.New(errCode, errMessage, errors.New(""))), errStatusCode, taskTagsRequestId)
containerTagsErr := &awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: errStatusCode,
},
},
Err: &ecstypes.LimitExceededException{Message: &errMessage},
},
RequestID: containerTagsRequestId,
}
taskTagsErr := &awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: errStatusCode,
},
},
Err: &ecstypes.LimitExceededException{Message: &errMessage},
},
RequestID: taskTagsRequestId,
}

gomock.InOrder(
state.EXPECT().TaskByArn(taskARN).Return(task, true),
state.EXPECT().ContainerMapByArn(taskARN).Return(containerNameToDockerContainer, true),
ecsClient.EXPECT().GetResourceTags(containerInstanceArn).Return(nil, containerTagsErr),
ecsClient.EXPECT().GetResourceTags(taskARN).Return(nil, taskTagsError),
ecsClient.EXPECT().GetResourceTags(taskARN).Return(nil, taskTagsErr),
)

taskWithTagsResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, availabilityZone, containerInstanceArn, true, true)
assert.NoError(t, err)
_, err = json.Marshal(taskWithTagsResponse)
assert.NoError(t, err)
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorField, "ContainerInstanceTags")
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorCode, errCode)
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorCode, (&ecstypes.LimitExceededException{}).ErrorCode())
assert.Equal(t, taskWithTagsResponse.Errors[0].ErrorMessage, errMessage)
assert.Equal(t, taskWithTagsResponse.Errors[0].StatusCode, errStatusCode)
assert.Equal(t, taskWithTagsResponse.Errors[0].RequestId, containerTagsRequestId)
assert.Equal(t, taskWithTagsResponse.Errors[0].ResourceARN, containerInstanceArn)
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorField, "TaskTags")
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorCode, errCode)
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorCode, (&ecstypes.LimitExceededException{}).ErrorCode())
assert.Equal(t, taskWithTagsResponse.Errors[1].ErrorMessage, errMessage)
assert.Equal(t, taskWithTagsResponse.Errors[1].StatusCode, errStatusCode)
assert.Equal(t, taskWithTagsResponse.Errors[1].RequestId, taskTagsRequestId)
Expand Down
21 changes: 19 additions & 2 deletions agent/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ import (
"strings"

commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/aws-sdk-go-v2/service/ecs/types"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/smithy-go"

"github.com/pkg/errors"
)
Expand Down Expand Up @@ -140,16 +142,31 @@ func Remove(slice []string, s int) []string {
// the passed in error code.
func IsAWSErrorCodeEqual(err error, code string) bool {
awsErr, ok := err.(awserr.Error)
return ok && awsErr.Code() == code
if ok {
return awsErr.Code() == code
}

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
return apiErr.ErrorCode() == code
}

return false
}

// GetRequestFailureStatusCode returns the status code from a
// RequestFailure error, or 0 if the error is not of that type
func GetRequestFailureStatusCode(err error) int {
var statusCode int
if reqErr, ok := err.(awserr.RequestFailure); ok {
statusCode = reqErr.StatusCode()
return reqErr.StatusCode()
}

var re *awshttp.ResponseError
if errors.As(err, &re) {
return re.HTTPStatusCode()
}

return statusCode
}

Expand Down
Loading

0 comments on commit 9eee623

Please sign in to comment.