Skip to content

Commit

Permalink
Merge pull request #253 from nspcc-dev/246-container-creation-can-han…
Browse files Browse the repository at this point in the history
…g-indefinitely-in-case-of-underlying-errors

Add timeout for SDK waiter operations
  • Loading branch information
roman-khimov authored Oct 24, 2024
2 parents e1b12eb + 53dde94 commit d6fe7f8
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 49 deletions.
1 change: 1 addition & 0 deletions cmd/neofs-rest-gw/config.go
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
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)

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

type BearerToken struct {
Expand Down Expand Up @@ -83,6 +84,7 @@ func NewAPI(prm *PrmAPI) (*RestAPI, error) {
gateMetric: prm.GateMetric,
serviceShutdownTimeout: prm.ServiceShutdownTimeout,
networkInfoGetter: cache.NewNetworkInfoCache(prm.Pool),
waiterOperationTimeout: prm.WaiterOperationTimeout,
}, nil
}

Expand Down Expand Up @@ -150,6 +152,7 @@ type RestAPI struct {
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

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
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ func (a *RestAPI) PutContainer(ctx echo.Context, params apiserver.PutContainerPa
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)
if err != nil {
resp := a.logAndGetErrorResponse("create container", err)
return ctx.JSON(http.StatusBadRequest, resp)
return ctx.JSON(getResponseCodeFromStatus(err), resp)
}

resp := apiserver.PutContainerOK{
Expand Down Expand Up @@ -136,9 +139,12 @@ func (a *RestAPI) PutContainerEACL(ctx echo.Context, containerID apiserver.Conta
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 {
resp := a.logAndGetErrorResponse("failed set container eacl", err)
return ctx.JSON(http.StatusBadRequest, resp)
return ctx.JSON(getResponseCodeFromStatus(err), resp)
}

ctx.Response().Header().Set(accessControlAllowOriginHeader, "*")
Expand Down Expand Up @@ -255,9 +261,12 @@ func (a *RestAPI) DeleteContainer(ctx echo.Context, containerID apiserver.Contai
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 {
resp := a.logAndGetErrorResponse("delete container", err, zap.String("container", containerID))
return ctx.JSON(http.StatusBadRequest, resp)
return ctx.JSON(getResponseCodeFromStatus(err), resp)
}

ctx.Response().Header().Set(accessControlAllowOriginHeader, "*")
Expand Down
53 changes: 53 additions & 0 deletions handlers/conv.go
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
}

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
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):
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

default:
return http.StatusInternalServerError
}
}
72 changes: 72 additions & 0 deletions spec/rest.yaml
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

0 comments on commit d6fe7f8

Please sign in to comment.