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

fix(edge): parse the response body with failed request [EE-7244] #661

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

fix(edge): parse the response body with failed request [EE-7244] #661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
04af5ac
fix(edge): parse response body with failed request
oscarzhou-portainer Jun 13, 2024
ef4569b
fix(edge): add parsed response body error log
oscarzhou-portainer Jun 14, 2024
6ac038f
fix(edge): update error log for async polling
oscarzhou-portainer Jun 17, 2024
385914a
fix(edge): update error package
oscarzhou-portainer Jun 17, 2024
a9c97e8
chore: update portainer package
oscarzhou-portainer Jun 17, 2024
46c81c1
chore: fix lint error
oscarzhou-portainer Jun 17, 2024
9426755
fix(edge/poll): update logging format
oscarzhou-portainer Jul 8, 2024
55b9929
fix(edge): improve the log struct
oscarzhou-portainer Jul 10, 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
53 changes: 30 additions & 23 deletions edge/client/error_response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,45 @@ import (
"encoding/json"
"net/http"

httperror "github.com/portainer/portainer/pkg/libhttp/error"
"github.com/rs/zerolog/log"
)

type errorData struct {
Details string
Message string
type NonOkResponseError struct {
msg string
}

func parseError(resp *http.Response) *errorData {
errorData := &errorData{}
func newNonOkResponseError(msg string) *NonOkResponseError {
return &NonOkResponseError{msg: msg}
}

err := json.NewDecoder(resp.Body).Decode(&errorData)
if err != nil {
log.Debug().CallerSkipFrame(1).
Err(err).
Int("status_code", resp.StatusCode).
Msg("failed to decode error response")
func (e *NonOkResponseError) Error() string {
return e.msg
}

return nil
}
type ForbiddenResponseError struct {
msg string
}

return errorData
func newForbiddenResponseError(msg string) *ForbiddenResponseError {
return &ForbiddenResponseError{msg: msg}
}

func logError(resp *http.Response, errorData *errorData) {
if errorData == nil {
return
}
func (e *ForbiddenResponseError) Error() string {
return e.msg
}

log.Debug().CallerSkipFrame(1).
Str("error_response_message", errorData.Message).
Str("error_response_details", errorData.Details).
Int("status_code", resp.StatusCode).
Msg("poll request failure")
func decodeNonOkayResponse(resp *http.Response, ctxMsg string) *httperror.HandlerError {
var respErr httperror.HandlerError
if err := json.NewDecoder(resp.Body).Decode(&respErr); err != nil {
log.
Error().
Err(err).
CallerSkipFrame(1).
Str("context", ctxMsg).
Int("response_code", resp.StatusCode).
Msg("PollClient failed to decode server response")
return nil
}
return &respErr
}
18 changes: 11 additions & 7 deletions edge/client/portainer_edge_async_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,14 +415,18 @@ func (client *PortainerAsyncClient) executeAsyncRequest(payload AsyncRequest, po
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
errorData := parseError(resp)
logError(resp, errorData)

if errorData != nil {
return nil, errors.New(errorData.Message + ": " + errorData.Details)
ctxMsg := "AsyncEdgeAgentExecuteAsyncRequest"
errMsg := "AsyncEdgeAgent failed to execute async request"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Str("response message", err.Message).
Str("response_message", err.Message).

We need to have a consistent naming for variables, I suggest that we use snake case for this. I will update the guidelines with that.

Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Msg(errMsg)
}

return nil, errors.New("short poll request failed")
return nil, newNonOkResponseError(errMsg)
}

var asyncResponse AsyncResponse
Expand Down
124 changes: 87 additions & 37 deletions edge/client/portainer_edge_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,6 @@ type logFilePayload struct {
FileContent string
}

type NonOkResponseError struct {
msg string
}

func newNonOkResponseError(msg string) *NonOkResponseError {
return &NonOkResponseError{msg: msg}
}

Comment on lines -54 to -57
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot remove this, we rely on that error type in other parts of the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type was not removed, only the constructor function was removed, because I found that it is not used.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is technically true but the constructor function is not used anymore because you removed it in this PR. 🤣

You can't do that, we need to return that error because we depend on it in other places, please see:

agent/edge/poll.go

Lines 217 to 225 in 676d62a

environmentStatus, err := service.portainerClient.GetEnvironmentStatus()
if err != nil {
var nonOkError *client.NonOkResponseError
if errors.As(err, &nonOkError) {
service.edgeManager.SetEndpointID(0)
}
return err
}

func (e *NonOkResponseError) Error() string {
return e.msg
}

// NewPortainerEdgeClient returns a pointer to a new PortainerEdgeClient instance
func NewPortainerEdgeClient(serverAddress string, setEIDFn setEndpointIDFn, getEIDFn getEndpointIDFn, edgeID string, agentPlatform agent.ContainerPlatform, metaFields agent.EdgeMetaFields, httpClient *edgeHTTPClient) *PortainerEdgeClient {
c := &PortainerEdgeClient{
Expand Down Expand Up @@ -121,9 +109,18 @@ func (client *PortainerEdgeClient) GetEnvironmentID() (portainer.EndpointID, err
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
log.Debug().Int("response_code", resp.StatusCode).Msg("global key request failure")

return 0, errors.New("global key request failed")
ctxMsg := "EdgeAgentGetEnvironmentID"
errMsg := "EdgeAgent failed to request global key"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Msg(errMsg)
}
return 0, newNonOkResponseError(errMsg)
}

var responseData globalKeyResponse
Expand Down Expand Up @@ -166,14 +163,18 @@ func (client *PortainerEdgeClient) GetEnvironmentStatus(flags ...string) (*PollS
}

if resp.StatusCode != http.StatusOK {
errorData := parseError(resp)
logError(resp, errorData)

if errorData != nil {
return nil, newNonOkResponseError(errorData.Message + ": " + errorData.Details)
ctxMsg := "EdgeAgentGetEnvironmentStatus"
errMsg := "EdgeAgent failed to request edge environment status"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Msg(errMsg)
}

return nil, newNonOkResponseError("short poll request failed")
return nil, newNonOkResponseError(errMsg)
}

var responseData PollStatusResponse
Expand Down Expand Up @@ -208,9 +209,18 @@ func (client *PortainerEdgeClient) GetEdgeStackConfig(edgeStackID int, version *
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
log.Error().Int("response_code", resp.StatusCode).Msg("GetEdgeStackConfig operation failed")

return nil, errors.New("GetEdgeStackConfig operation failed")
ctxMsg := "EdgeAgentGetEdgeStackConfig"
errMsg := "EdgeAgent failed to request edge stack config"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Msg(errMsg)
}
return nil, newNonOkResponseError(errMsg)
}

var data edge.StackPayload
Expand Down Expand Up @@ -265,9 +275,18 @@ func (client *PortainerEdgeClient) SetEdgeStackStatus(
resp.Body.Close()

if resp.StatusCode != http.StatusOK {
log.Error().Int("response_code", resp.StatusCode).Msg("SetEdgeStackStatus operation failed")

return errors.New("SetEdgeStackStatus operation failed")
ctxMsg := "EdgeAgentSetEdgeStackStatus"
errMsg := "EdgeAgent failed to set edge stack status"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Msg(errMsg)
}
return newNonOkResponseError(errMsg)
}

return nil
Expand Down Expand Up @@ -302,9 +321,18 @@ func (client *PortainerEdgeClient) SetEdgeJobStatus(edgeJobStatus agent.EdgeJobS
resp.Body.Close()

if resp.StatusCode != http.StatusOK {
log.Error().Int("response_code", resp.StatusCode).Msg("SetEdgeJobStatus operation failed")

return errors.New("SetEdgeJobStatus operation failed")
ctxMsg := "EdgeAgentSetEdgeJobStatus"
errMsg := "EdgeAgent failed to set edge job status"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Msg(errMsg)
}
return newNonOkResponseError(errMsg)
}

return nil
Expand All @@ -326,13 +354,24 @@ func (client *PortainerEdgeClient) GetEdgeConfig(id EdgeConfigID) (*EdgeConfig,
}

if resp.StatusCode != http.StatusOK {
log.Error().Int("response_code", resp.StatusCode).Msg("GetEdgeConfig operation failed")
ctxMsg := "EdgeAgentGetEdgeConfig"
errMsg := "EdgeAgent failed to get edge config info"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Int("edge config", int(id)).
Msg(errMsg)
}

if resp.StatusCode == http.StatusForbidden {
return nil, errors.New("GetEdgeConfig operation forbidden")
return nil, newForbiddenResponseError(errMsg)
}

return nil, errors.New("GetEdgeConfig operation failed")
return nil, newNonOkResponseError(errMsg)
}

var data EdgeConfig
Expand Down Expand Up @@ -362,9 +401,20 @@ func (client *PortainerEdgeClient) SetEdgeConfigState(id EdgeConfigID, state Edg
resp.Body.Close()

if resp.StatusCode != http.StatusOK {
log.Error().Int("edge_config_id", int(id)).Stringer("state", state).Int("response_code", resp.StatusCode).Msg("SetEdgeConfigState operation failed")

return errors.New("SetEdgeConfigState operation failed")
ctxMsg := "EdgeAgentSetEdgeConfigState"
errMsg := "EdgeAgent failed to set state to edge config"
if err := decodeNonOkayResponse(resp, ctxMsg); err != nil {
log.
Error().Err(err.Err).
Str("context", ctxMsg).
Str("response message", err.Message).
Int("status code", err.StatusCode).
Int("endpoint id", int(client.getEndpointIDFn())).
Int("edge config id", int(id)).
Int("edge state", int(state)).
Msg(errMsg)
}
return newNonOkResponseError(errMsg)
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions edge/poll.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"math/rand"
"strconv"
"strings"
"time"

"github.com/portainer/agent"
Expand Down Expand Up @@ -345,7 +344,8 @@ func (service *PollService) processEdgeConfig(fn func(*client.EdgeConfig) error,
if err != nil {
log.Error().Err(err).Msg("an error occurred while retrieving an edge configuration")

if strings.Contains(err.Error(), "forbidden") {
var forbiddenError *client.ForbiddenResponseError
if errors.As(err, &forbiddenError) {
if err := service.portainerClient.SetEdgeConfigState(edgeConfigID, client.EdgeConfigFailureState); err != nil {
log.Error().Err(err).Msg("an error occurred while updating the edge configuration state")
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require (
github.com/mitchellh/mapstructure v1.5.0
github.com/opencontainers/image-spec v1.1.0-rc5
github.com/pkg/errors v0.9.1
github.com/portainer/portainer v0.6.1-0.20240430014408-a9ead542b3aa
github.com/portainer/portainer v0.6.1-0.20240616212454-be9d3285e178
github.com/rs/zerolog v1.29.0
github.com/stretchr/testify v1.9.0
github.com/wI2L/jsondiff v0.2.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/portainer/portainer v0.6.1-0.20240430014408-a9ead542b3aa h1:gfKGsoMclwChTqxX4eCfIFo7b4ncHUdHdY7yQwi7HO8=
github.com/portainer/portainer v0.6.1-0.20240430014408-a9ead542b3aa/go.mod h1:AeF9ey0EZ44IK7+kuwCFwcmfY12PfCwmJs57r9STj6s=
github.com/portainer/portainer v0.6.1-0.20240616212454-be9d3285e178 h1:U/Wg0rjTySv2Sa2u8TVPXNyLE+9Ypx/fGRJSiqGrNac=
github.com/portainer/portainer v0.6.1-0.20240616212454-be9d3285e178/go.mod h1:AeF9ey0EZ44IK7+kuwCFwcmfY12PfCwmJs57r9STj6s=
github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI=
github.com/prometheus/client_golang v0.9.2/go.mod h1:OsXs2jCmiKlQ1lTBmv21f2mNfw4xf/QclQDMrYNZzcM=
github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
Expand Down
Loading