Skip to content

Migrating awserr ResponseError to smithy ResponseError in GetRequestFailureStatusCode #4516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ func (agent *ecsAgent) waitUntilInstanceInService(pollWaitDuration time.Duration
if err != nil {
// Do not exit if error is due to throttling or temporary server errors
// These are likely transient, as at this point IMDS has been successfully queried for state
switch utils.GetRequestFailureStatusCode(err) {
switch utils.GetResponseErrorStatusCode(err) {
case 429, 500, 502, 503, 504:
seelog.Warnf("Encountered error while waiting for warmed instance to go in service: %v", err)
default:
Expand All @@ -569,7 +569,7 @@ func (agent *ecsAgent) pollUntilTargetLifecyclePresent(pollWaitDuration time.Dur
for i := 0; i < pollMaxTimes; i++ {
targetState, err = agent.getTargetLifecycle(maxRetries)
if targetState != "" ||
(err != nil && utils.GetRequestFailureStatusCode(err) != 404) {
(err != nil && utils.GetResponseErrorStatusCode(err) != 404) {
break
}
time.Sleep(pollWaitDuration)
Expand Down
28 changes: 25 additions & 3 deletions agent/app/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"net/http"
"sort"
"testing"
"time"
Expand Down Expand Up @@ -58,6 +59,10 @@ import (
awsv2 "github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"

"github.com/aws/smithy-go"
smithyhttp "github.com/aws/smithy-go/transport/http"

"github.com/docker/docker/api/types"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand All @@ -73,11 +78,14 @@ const (
instanceID = "i-123"
warmedState = "Warmed:Running"
testTargetLifecycleMaxRetryCount = 1
serviceId = "ec2imds"
getMetadataOperationId = "GetMetadata"
)

var notFoundErr = awserr.NewRequestFailure(awserr.Error(awserr.New("NotFound", "", errors.New(""))), 404, "")
var badReqErr = awserr.NewRequestFailure(awserr.Error(awserr.New("BadRequest", "", errors.New(""))), 400, "")
var serverErr = awserr.NewRequestFailure(awserr.Error(awserr.New("InternalServerError", "", errors.New(""))), 500, "")
var notFoundErr = getResponseError(404)
var badReqErr = getResponseError(400)
var serverErr = getResponseError(500)

var apiVersions = []dockerclient.DockerVersion{
dockerclient.MinDockerAPIVersion,
}
Expand Down Expand Up @@ -122,6 +130,20 @@ func setup(t *testing.T) (*gomock.Controller,
mock_serviceconnect.NewMockManager(ctrl)
}

func getResponseError(statusCode int) *smithy.OperationError {
return &smithy.OperationError{
ServiceID: serviceId,
OperationName: getMetadataOperationId,
Err: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: statusCode,
},
},
},
}
}

func TestDoStartMinimumSupportedDockerVersionTerminal(t *testing.T) {
ctrl, credentialsManager, state, imageManager, client,
dockerClient, stateManagerFactory, saveableOptionFactory, execCmdMgr, _ := setup(t)
Expand Down
2 changes: 1 addition & 1 deletion agent/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
github.com/aws/aws-sdk-go-v2/credentials v1.17.42
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.18
github.com/aws/aws-sdk-go-v2/service/ecr v1.41.1
github.com/aws/smithy-go v1.22.2
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 @@ -53,7 +54,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.2 // 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
18 changes: 13 additions & 5 deletions agent/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import (
"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"
smithyhttp "github.com/aws/smithy-go/transport/http"

"github.com/pkg/errors"
)
Expand Down Expand Up @@ -143,12 +145,18 @@ func IsAWSErrorCodeEqual(err error, code string) bool {
return ok && awsErr.Code() == code
}

// GetRequestFailureStatusCode returns the status code from a
// RequestFailure error, or 0 if the error is not of that type
func GetRequestFailureStatusCode(err error) int {
// GetResponseErrorStatusCode returns the status code from a
// ResponseError error of a server client error from AWS SDK Go V2,
// or 0 if the error is not of that type
// https://docs.aws.amazon.com/sdk-for-go/v2/developer-guide/handle-errors.html
func GetResponseErrorStatusCode(err error) int {
var statusCode int
if reqErr, ok := err.(awserr.RequestFailure); ok {
statusCode = reqErr.StatusCode()
var oe *smithy.OperationError
if errors.As(err, &oe) {
var responseErr *smithyhttp.ResponseError
if errors.As(oe.Err, &responseErr) {
statusCode = responseErr.HTTPStatusCode()
}
}
return statusCode
}
Expand Down
30 changes: 24 additions & 6 deletions agent/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package utils
import (
"errors"
"fmt"
"net/http"
"os"
"sort"
"testing"
Expand All @@ -27,10 +28,17 @@ import (
"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/aws/smithy-go"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const (
serviceId = "ec2imds"
getMetadataOperationId = "GetMetadata"
)

func TestDefaultIfBlank(t *testing.T) {
const defaultValue = "a boring default"
const specifiedValue = "new value"
Expand Down Expand Up @@ -129,27 +137,37 @@ func TestIsAWSErrorCodeEqual(t *testing.T) {
}
}

func TestGetRequestFailureStatusCode(t *testing.T) {
func TestGetResponseErrorStatusCode(t *testing.T) {
testcases := []struct {
name string
err error
res int
}{
{
name: "TestGetRequestFailureStatusCodeSuccess",
err: awserr.NewRequestFailure(awserr.Error(awserr.New("BadRequest", "", errors.New(""))), 400, ""),
res: 400,
name: "TestGetResponseErrorStatusCodeSuccess",
err: &smithy.OperationError{
ServiceID: serviceId,
OperationName: getMetadataOperationId,
Err: &smithyhttp.ResponseError{
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: 400,
},
},
},
},
res: 400,
},
{
name: "TestGetRequestFailureStatusCodeWrongErrType",
name: "TestGetResponseErrorStatusCodeCodeWrongErrType",
err: errors.New("err"),
res: 0,
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.res, GetRequestFailureStatusCode(tc.err))
assert.Equal(t, tc.res, GetResponseErrorStatusCode(tc.err))
})
}
}
Expand Down
Loading