Skip to content

fix: handle 5xx in fetch resource tree api and cd-trigger #5050

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 8 commits into from
May 3, 2024
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
33 changes: 25 additions & 8 deletions api/restHandler/app/appList/AppListingRestHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package appList
import (
"context"
"encoding/json"
"errors"
"fmt"
"github.com/devtron-labs/devtron/api/helm-app/gRPC"
client "github.com/devtron-labs/devtron/api/helm-app/service"
Expand Down Expand Up @@ -645,26 +646,34 @@ func (handler AppListingRestHandlerImpl) FetchResourceTree(w http.ResponseWriter

func (handler AppListingRestHandlerImpl) handleResourceTreeErrAndDeletePipelineIfNeeded(w http.ResponseWriter, err error,
appId int, envId int, acdToken string, cdPipeline *pipelineConfig.Pipeline) {
var apiError *util.ApiError
ok := errors.As(err, &apiError)
if cdPipeline.DeploymentAppType == util.PIPELINE_DEPLOYMENT_TYPE_ACD {
apiError, ok := err.(*util.ApiError)
if ok && apiError != nil {
if apiError.Code == constants.AppDetailResourceTreeNotFound && cdPipeline.DeploymentAppDeleteRequest == true && cdPipeline.DeploymentAppCreated == true {
acdAppFound, appDeleteErr := handler.pipeline.MarkGitOpsDevtronAppsDeletedWhereArgoAppIsDeleted(appId, envId, acdToken, cdPipeline)
if appDeleteErr != nil {
common.WriteJsonResp(w, fmt.Errorf("error in deleting devtron pipeline for deleted argocd app"), nil, http.StatusInternalServerError)
apiError.UserMessage = constants.ErrorDeletingPipelineForDeletedArgoAppMsg
common.WriteJsonResp(w, apiError, nil, http.StatusInternalServerError)
return
} else if appDeleteErr == nil && !acdAppFound {
common.WriteJsonResp(w, fmt.Errorf("argocd app deleted"), nil, http.StatusNotFound)
apiError.UserMessage = constants.ArgoAppDeletedErrMsg
common.WriteJsonResp(w, apiError, nil, http.StatusNotFound)
return
}
}
}
}
// not returned yet therefore no specific error to be handled, send error in internal message
common.WriteJsonResp(w, &util.ApiError{
InternalMessage: err.Error(),
UserMessage: "unable to fetch resource tree",
}, nil, http.StatusInternalServerError)
if ok && apiError != nil {
apiError.UserMessage = constants.UnableToFetchResourceTreeErrMsg
} else {
apiError = &util.ApiError{
InternalMessage: err.Error(),
UserMessage: constants.UnableToFetchResourceTreeErrMsg,
}
}
common.WriteJsonResp(w, apiError, nil, http.StatusInternalServerError)
}

func (handler AppListingRestHandlerImpl) FetchAppStageStatus(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -1007,9 +1016,13 @@ func (handler AppListingRestHandlerImpl) fetchResourceTree(w http.ResponseWriter
handler.logger.Debugw("FetchAppDetailsV2, time elapsed in fetching application for environment ", "elapsed", elapsed, "appId", appId, "envId", envId)
if err != nil {
handler.logger.Errorw("service err, FetchAppDetailsV2, resource tree", "err", err, "app", appId, "env", envId)
internalMsg := fmt.Sprintf("%s, err:- %s", constants.UnableToFetchResourceTreeForAcdErrMsg, err.Error())
clientCode, _ := util.GetClientDetailedError(err)
httpStatusCode := clientCode.GetHttpStatusCodeForGivenGrpcCode()
err = &util.ApiError{
HttpStatusCode: httpStatusCode,
Code: constants.AppDetailResourceTreeNotFound,
InternalMessage: "app detail fetched, failed to get resource tree from acd",
InternalMessage: internalMsg,
UserMessage: "Error fetching detail, if you have recently created this deployment pipeline please try after sometime.",
}
return resourceTree, err
Expand Down Expand Up @@ -1110,6 +1123,10 @@ func (handler AppListingRestHandlerImpl) fetchResourceTree(w http.ResponseWriter
resp, err := handler.k8sCommonService.GetManifestsByBatch(r.Context(), validRequest)
if err != nil {
handler.logger.Errorw("error in getting manifest by batch", "err", err, "clusterId", clusterIdString)
httpStatus, ok := util.IsErrorContextCancelledOrDeadlineExceeded(err)
if ok {
return nil, &util.ApiError{HttpStatusCode: httpStatus, Code: strconv.Itoa(httpStatus), InternalMessage: err.Error()}
}
return nil, err
}
newResourceTree := handler.k8sCommonService.PortNumberExtraction(resp, resourceTree)
Expand Down
6 changes: 6 additions & 0 deletions client/argocdServer/ArgoClientWrapperService.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import (
"github.com/devtron-labs/devtron/client/argocdServer/application"
"github.com/devtron-labs/devtron/client/argocdServer/bean"
"github.com/devtron-labs/devtron/client/argocdServer/repository"
"github.com/devtron-labs/devtron/internal/constants"
"github.com/devtron-labs/devtron/internal/util"
"github.com/devtron-labs/devtron/pkg/deployment/gitOps/config"
"github.com/devtron-labs/devtron/pkg/deployment/gitOps/git"
"github.com/devtron-labs/devtron/util/retryFunc"
"go.uber.org/zap"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"strconv"
"strings"
"time"
)
Expand Down Expand Up @@ -101,6 +103,10 @@ func (impl *ArgoClientWrapperServiceImpl) GetArgoAppWithNormalRefresh(context co
impl.logger.Debugw("trying to normal refresh application through get ", "argoAppName", argoAppName)
_, err := impl.acdClient.Get(context, &application2.ApplicationQuery{Name: &argoAppName, Refresh: &refreshType})
if err != nil {
internalMsg := fmt.Sprintf("%s, err:- %s", constants.CannotGetAppWithRefreshErrMsg, err.Error())
clientCode, _ := util.GetClientDetailedError(err)
httpStatusCode := clientCode.GetHttpStatusCodeForGivenGrpcCode()
err = &util.ApiError{HttpStatusCode: httpStatusCode, Code: strconv.Itoa(httpStatusCode), InternalMessage: internalMsg, UserMessage: err.Error()}
impl.logger.Errorw("cannot get application with refresh", "app", argoAppName)
return err
}
Expand Down
12 changes: 12 additions & 0 deletions internal/constants/InternalErrorCode.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,17 @@ const (
VulnerabilityFound string = "10002"
)

const (
HttpClientSideTimeout = 499
)

var AppAlreadyExists = &ErrorCode{"4001", "application %s already exists"}
var AppDoesNotExist = &ErrorCode{"4004", "application %s does not exist"}

const (
ErrorDeletingPipelineForDeletedArgoAppMsg = "error in deleting devtron pipeline for deleted argocd app"
ArgoAppDeletedErrMsg = "argocd app deleted"
UnableToFetchResourceTreeErrMsg = "unable to fetch resource tree"
UnableToFetchResourceTreeForAcdErrMsg = "app detail fetched, failed to get resource tree from acd"
CannotGetAppWithRefreshErrMsg = "cannot get application with refresh"
)
31 changes: 30 additions & 1 deletion internal/errors/bean.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package errors

import "google.golang.org/grpc/codes"
import (
"github.com/devtron-labs/devtron/internal/constants"
"google.golang.org/grpc/codes"
"net/http"
)

type ClientStatusCode struct {
Code codes.Code
Expand All @@ -17,3 +21,28 @@ func (r *ClientStatusCode) IsNotFoundCode() bool {
func (r *ClientStatusCode) IsFailedPreconditionCode() bool {
return r.Code == codes.FailedPrecondition
}

func (r *ClientStatusCode) IsDeadlineExceededCode() bool {
return r.Code == codes.DeadlineExceeded
}

func (r *ClientStatusCode) IsCanceledCode() bool {
return r.Code == codes.Canceled
}

func (r *ClientStatusCode) GetHttpStatusCodeForGivenGrpcCode() int {
switch r.Code {
case codes.InvalidArgument:
return http.StatusConflict
case codes.NotFound:
return http.StatusNotFound
case codes.FailedPrecondition:
return http.StatusPreconditionFailed
case codes.DeadlineExceeded:
return http.StatusRequestTimeout
case codes.Canceled:
return constants.HttpClientSideTimeout
default:
return http.StatusInternalServerError
}
}
13 changes: 13 additions & 0 deletions internal/util/ErrorUtil.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@
package util

import (
"context"
errors2 "errors"
"fmt"
"github.com/devtron-labs/devtron/internal/constants"
"github.com/devtron-labs/devtron/internal/errors"
"github.com/go-pg/pg"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"net/http"
)

type ApiError struct {
Expand Down Expand Up @@ -66,3 +70,12 @@ func GetClientDetailedError(err error) (*errors.ClientStatusCode, string) {
}
return grpcCode, err.Error()
}

func IsErrorContextCancelledOrDeadlineExceeded(err error) (int, bool) {
if errors2.Is(err, context.Canceled) {
return constants.HttpClientSideTimeout, true
} else if errors2.Is(err, context.DeadlineExceeded) {
return http.StatusRequestTimeout, true
}
return 0, false
}
Loading