Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add timeout for SDK waiter operations #253

Merged
merged 3 commits into from
Oct 24, 2024
Merged

Add timeout for SDK waiter operations #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edd9220
handler: Add timeout for SDK waiter operations
smallhive Oct 23, 2024
18c2dd3
handler: Return http code according to the NeoFS status
smallhive Oct 23, 2024
53dde94
api: Update API status codes
smallhive Oct 23, 2024
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
1 change: 1 addition & 0 deletions cmd/neofs-rest-gw/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@ func newNeofsAPI(ctx context.Context, logger *zap.Logger, v *viper.Viper) (*hand
}

apiPrm.DefaultTimestamp = v.GetBool(cfgPoolDefaultTimestamp)
apiPrm.WaiterOperationTimeout = time.Duration(uint64(ni.MsPerBlock())*4) * time.Millisecond

return handlers.NewAPI(&apiPrm)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/neofs-rest-gw/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ func restContainerEACLPut(ctx context.Context, t *testing.T, clientPool *pool.Po
query := make(url.Values)
query.Add(walletConnectQuery, strconv.FormatBool(useWalletConnect))

doSetEACLRequest(ctx, t, httpClient, cnrID, query, bearerToken, invalidBody, http.StatusBadRequest, nil)
doSetEACLRequest(ctx, t, httpClient, cnrID, query, bearerToken, invalidBody, http.StatusInternalServerError, nil)

resp := &apiserver.SuccessResponse{}
doSetEACLRequest(ctx, t, httpClient, cnrID, query, bearerToken, body, http.StatusOK, resp)
Expand Down Expand Up @@ -1614,7 +1614,7 @@ func restContainerPutInvalid(ctx context.Context, t *testing.T) {
prepareCommonHeaders(request.Header, bearerToken)

resp := &apiserver.ErrorResponse{}
doRequest(t, httpClient, request, http.StatusBadRequest, resp)
doRequest(t, httpClient, request, http.StatusInternalServerError, resp)
require.Equal(t, uint32(0), resp.Code)
require.Equal(t, apiserver.GW, resp.Type)
}
Expand Down Expand Up @@ -1650,7 +1650,7 @@ func restContainerPut(ctx context.Context, t *testing.T, clientPool *pool.Pool)
require.NoError(t, err)
prepareCommonHeaders(request.Header, bearerToken)

doRequest(t, httpClient, request, http.StatusBadRequest, nil)
doRequest(t, httpClient, request, http.StatusInternalServerError, nil)
roman-khimov marked this conversation as resolved.
Show resolved Hide resolved

// create container with name in local scope
containerPutInfo := &apiserver.ContainerPutInfo{
Expand Down
3 changes: 3 additions & 0 deletions handlers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
PrometheusService *metrics.Service
PprofService *metrics.Service
ServiceShutdownTimeout time.Duration
WaiterOperationTimeout time.Duration
}

type BearerToken struct {
Expand Down Expand Up @@ -83,6 +84,7 @@
gateMetric: prm.GateMetric,
serviceShutdownTimeout: prm.ServiceShutdownTimeout,
networkInfoGetter: cache.NewNetworkInfoCache(prm.Pool),
waiterOperationTimeout: prm.WaiterOperationTimeout,

Check warning on line 87 in handlers/api.go

View check run for this annotation

Codecov / codecov/patch

handlers/api.go#L87 

Added line #L87 was not covered by tests
}, nil
}

Expand Down Expand Up @@ -150,6 +152,7 @@
pprofService *metrics.Service
serviceShutdownTimeout time.Duration
networkInfoGetter networkInfoGetter
waiterOperationTimeout time.Duration
}

func (a *RestAPI) StartCallback() {
Expand Down
81 changes: 41 additions & 40 deletions handlers/apiserver/rest-server.gen.go
View file
Open in desktop

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

21 changes: 15 additions & 6 deletions handlers/containers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@
return ctx.JSON(http.StatusBadRequest, resp)
}

cnrID, err := createContainer(ctx.Request().Context(), a.pool, stoken, body, params, a.signer, a.networkInfoGetter)
wCtx, cancel := context.WithTimeout(ctx.Request().Context(), a.waiterOperationTimeout)
defer cancel()

cnrID, err := createContainer(wCtx, a.pool, stoken, body, params, a.signer, a.networkInfoGetter)

Check warning on line 70 in handlers/containers.go

View check run for this annotation

Codecov / codecov/patch

handlers/containers.go#L67-L70 

Added lines #L67 - L70 were not covered by tests
if err != nil {
resp := a.logAndGetErrorResponse("create container", err)
return ctx.JSON(http.StatusBadRequest, resp)
return ctx.JSON(getResponseCodeFromStatus(err), resp)

Check warning on line 73 in handlers/containers.go

View check run for this annotation

Codecov / codecov/patch

handlers/containers.go#L73 

Added line #L73 was not covered by tests
}

resp := apiserver.PutContainerOK{
Expand Down Expand Up @@ -136,9 +139,12 @@
return ctx.JSON(http.StatusBadRequest, resp)
}

if err = setContainerEACL(ctx.Request().Context(), a.pool, cnrID, stoken, body, a.signer); err != nil {
wCtx, cancel := context.WithTimeout(ctx.Request().Context(), a.waiterOperationTimeout)
defer cancel()

if err = setContainerEACL(wCtx, a.pool, cnrID, stoken, body, a.signer); err != nil {

Check warning on line 145 in handlers/containers.go

View check run for this annotation

Codecov / codecov/patch

handlers/containers.go#L142-L145 

Added lines #L142 - L145 were not covered by tests
resp := a.logAndGetErrorResponse("failed set container eacl", err)
return ctx.JSON(http.StatusBadRequest, resp)
return ctx.JSON(getResponseCodeFromStatus(err), resp)

Check warning on line 147 in handlers/containers.go

View check run for this annotation

Codecov / codecov/patch

handlers/containers.go#L147 

Added line #L147 was not covered by tests
}

ctx.Response().Header().Set(accessControlAllowOriginHeader, "*")
Expand Down Expand Up @@ -255,9 +261,12 @@
prm.WithinSession(stoken)

wait := waiter.NewContainerDeleteWaiter(a.pool, waiter.DefaultPollInterval)
if err = wait.ContainerDelete(ctx.Request().Context(), cnrID, a.signer, prm); err != nil {
wCtx, cancel := context.WithTimeout(ctx.Request().Context(), a.waiterOperationTimeout)
defer cancel()

if err = wait.ContainerDelete(wCtx, cnrID, a.signer, prm); err != nil {

Check warning on line 267 in handlers/containers.go

View check run for this annotation

Codecov / codecov/patch

handlers/containers.go#L264-L267 

Added lines #L264 - L267 were not covered by tests
resp := a.logAndGetErrorResponse("delete container", err, zap.String("container", containerID))
return ctx.JSON(http.StatusBadRequest, resp)
return ctx.JSON(getResponseCodeFromStatus(err), resp)

Check warning on line 269 in handlers/containers.go

View check run for this annotation

Codecov / codecov/patch

handlers/containers.go#L269 

Added line #L269 was not covered by tests
}

ctx.Response().Header().Set(accessControlAllowOriginHeader, "*")
Expand Down
53 changes: 53 additions & 0 deletions handlers/conv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package handlers

import (
"context"
"errors"
"net/http"

apistatus "github.com/nspcc-dev/neofs-sdk-go/client/status"
)

func getResponseCodeFromStatus(err error) int {
if err == nil {
return http.StatusOK
}

Check warning on line 14 in handlers/conv.go

View check run for this annotation

Codecov / codecov/patch

handlers/conv.go#L11-L14 

Added lines #L11 - L14 were not covered by tests

roman-khimov marked this conversation as resolved.
Show resolved Hide resolved
switch {
case errors.Is(err, context.Canceled):
return http.StatusGatewayTimeout
case errors.Is(err, context.DeadlineExceeded):
return http.StatusGatewayTimeout
case errors.Is(err, apistatus.ErrEACLNotFound):
return http.StatusNotFound
case errors.Is(err, apistatus.ErrContainerNotFound):
return http.StatusNotFound
case errors.Is(err, apistatus.ErrSessionTokenNotFound):
return http.StatusNotFound
case errors.Is(err, apistatus.ErrSessionTokenExpired):
return http.StatusForbidden
roman-khimov marked this conversation as resolved.
Show resolved Hide resolved
case errors.Is(err, apistatus.ErrObjectLocked):
return http.StatusConflict
case errors.Is(err, apistatus.ErrObjectAlreadyRemoved):
return http.StatusGone
case errors.Is(err, apistatus.ErrLockNonRegularObject):
return http.StatusForbidden
case errors.Is(err, apistatus.ErrObjectAccessDenied):
return http.StatusForbidden
case errors.Is(err, apistatus.ErrObjectNotFound):
return http.StatusNotFound
case errors.Is(err, apistatus.ErrObjectOutOfRange):
return http.StatusRequestedRangeNotSatisfiable
case errors.Is(err, apistatus.ErrServerInternal):
roman-khimov marked this conversation as resolved.
Show resolved Hide resolved
return http.StatusBadGateway
case errors.Is(err, apistatus.ErrWrongMagicNumber):
return http.StatusBadGateway
case errors.Is(err, apistatus.ErrSignatureVerification):
return http.StatusBadGateway
case errors.Is(err, apistatus.ErrNodeUnderMaintenance):
return http.StatusBadGateway

Check warning on line 48 in handlers/conv.go

View check run for this annotation

Codecov / codecov/patch

handlers/conv.go#L16-L48 

Added lines #L16 - L48 were not covered by tests

default:
return http.StatusInternalServerError

Check warning on line 51 in handlers/conv.go

View check run for this annotation

Codecov / codecov/patch

handlers/conv.go#L50-L51 

Added lines #L50 - L51 were not covered by tests
}
}
72 changes: 72 additions & 0 deletions spec/rest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,30 @@ paths:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"403":
description: Access denied
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"404":
description: Not found.
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"502":
description: Bad gateway.
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"504":
description: Gateway timeout
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
x-codegen-request-body-name: container
options:
operationId: optionsContainersPutList
Expand Down Expand Up @@ -558,6 +582,30 @@ paths:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"403":
description: Access denied
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"404":
description: Not found.
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"502":
description: Bad gateway.
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"504":
description: Gateway timeout
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
options:
operationId: optionsContainersGetDelete
parameters:
Expand Down Expand Up @@ -633,6 +681,30 @@ paths:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"403":
description: Access denied
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"404":
description: Not found.
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"502":
description: Bad gateway.
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
"504":
description: Gateway timeout
content:
'*/*':
schema:
$ref: '#/components/schemas/ErrorResponse'
x-codegen-request-body-name: eacl
options:
operationId: optionsContainersEACL
Expand Down
Loading