From 3f8c05ef73cca0e874829344e3c8f14c3273c642 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 2 May 2024 12:26:00 +0530 Subject: [PATCH 1/6] handle context cancelled and deadline exceeded in fetch resource tree api --- .../app/appList/AppListingRestHandler.go | 20 ++++++++++++------- internal/constants/InternalErrorCode.go | 10 ++++++++++ internal/util/ErrorUtil.go | 13 ++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/api/restHandler/app/appList/AppListingRestHandler.go b/api/restHandler/app/appList/AppListingRestHandler.go index a0330f4104..f7c5dce031 100644 --- a/api/restHandler/app/appList/AppListingRestHandler.go +++ b/api/restHandler/app/appList/AppListingRestHandler.go @@ -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" @@ -645,26 +646,27 @@ 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) + apiError.UserMessage = constants.UnableToFetchResourceTreeErrMsg + common.WriteJsonResp(w, apiError, nil, http.StatusInternalServerError) } func (handler AppListingRestHandlerImpl) FetchAppStageStatus(w http.ResponseWriter, r *http.Request) { @@ -1110,6 +1112,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) diff --git a/internal/constants/InternalErrorCode.go b/internal/constants/InternalErrorCode.go index 14019a47bd..5117be6be2 100644 --- a/internal/constants/InternalErrorCode.go +++ b/internal/constants/InternalErrorCode.go @@ -94,5 +94,15 @@ 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" +) diff --git a/internal/util/ErrorUtil.go b/internal/util/ErrorUtil.go index 6e98e2baf0..31aa1689e7 100644 --- a/internal/util/ErrorUtil.go +++ b/internal/util/ErrorUtil.go @@ -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 { @@ -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 +} From 1b06c1e12b5f7f14c71f12d1b44f06a485426c68 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 2 May 2024 15:32:08 +0530 Subject: [PATCH 2/6] handle context cancelled and deadline exceeded error for resource tree fetch api for acd deployment --- api/restHandler/app/appList/AppListingRestHandler.go | 11 ++++++++++- client/argocdServer/bean/bean.go | 2 +- internal/constants/InternalErrorCode.go | 1 + internal/errors/bean.go | 8 ++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/api/restHandler/app/appList/AppListingRestHandler.go b/api/restHandler/app/appList/AppListingRestHandler.go index f7c5dce031..e7ba12dd29 100644 --- a/api/restHandler/app/appList/AppListingRestHandler.go +++ b/api/restHandler/app/appList/AppListingRestHandler.go @@ -1009,9 +1009,18 @@ 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) + httpStatusCode := http.StatusInternalServerError + internalMsg := fmt.Sprintf("%s, err:- %s", constants.UnableToFetchResourceTreeForAcdErrMsg, err.Error()) + clientCode, _ := util.GetClientDetailedError(err) + if clientCode.IsDeadlineExceededCode() { + httpStatusCode = http.StatusRequestTimeout + } else if clientCode.IsCanceledCode() { + httpStatusCode = constants.HttpClientSideTimeout + } 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 diff --git a/client/argocdServer/bean/bean.go b/client/argocdServer/bean/bean.go index 5e5bb13f8d..5440335b2b 100644 --- a/client/argocdServer/bean/bean.go +++ b/client/argocdServer/bean/bean.go @@ -36,7 +36,7 @@ const ( Progressing = "Progressing" Suspended = "Suspended" TimeoutFast = 10 * time.Second - TimeoutSlow = 30 * time.Second + TimeoutSlow = (1 * time.Second) / 10 TimeoutLazy = 60 * time.Second HIBERNATING = "HIBERNATING" SUCCEEDED = "Succeeded" diff --git a/internal/constants/InternalErrorCode.go b/internal/constants/InternalErrorCode.go index 5117be6be2..29b09d1fbf 100644 --- a/internal/constants/InternalErrorCode.go +++ b/internal/constants/InternalErrorCode.go @@ -105,4 +105,5 @@ 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" ) diff --git a/internal/errors/bean.go b/internal/errors/bean.go index 27b2f46b06..92d291e24b 100644 --- a/internal/errors/bean.go +++ b/internal/errors/bean.go @@ -17,3 +17,11 @@ 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 +} From 07be4ec6e8c1d1915ce7ec013387c27ded80cc50 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 2 May 2024 16:28:59 +0530 Subject: [PATCH 3/6] handle context cancelled and deadline exceeded error sync argo app with normal refresh --- .../app/appList/AppListingRestHandler.go | 7 +----- .../argocdServer/ArgoClientWrapperService.go | 6 +++++ internal/constants/InternalErrorCode.go | 1 + internal/errors/bean.go | 23 ++++++++++++++++++- pkg/k8s/K8sCommonService.go | 2 +- 5 files changed, 31 insertions(+), 8 deletions(-) diff --git a/api/restHandler/app/appList/AppListingRestHandler.go b/api/restHandler/app/appList/AppListingRestHandler.go index e7ba12dd29..c06b3599db 100644 --- a/api/restHandler/app/appList/AppListingRestHandler.go +++ b/api/restHandler/app/appList/AppListingRestHandler.go @@ -1009,14 +1009,9 @@ 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) - httpStatusCode := http.StatusInternalServerError internalMsg := fmt.Sprintf("%s, err:- %s", constants.UnableToFetchResourceTreeForAcdErrMsg, err.Error()) clientCode, _ := util.GetClientDetailedError(err) - if clientCode.IsDeadlineExceededCode() { - httpStatusCode = http.StatusRequestTimeout - } else if clientCode.IsCanceledCode() { - httpStatusCode = constants.HttpClientSideTimeout - } + httpStatusCode := clientCode.GetHttpStatusCodeForGivenGrpcCode() err = &util.ApiError{ HttpStatusCode: httpStatusCode, Code: constants.AppDetailResourceTreeNotFound, diff --git a/client/argocdServer/ArgoClientWrapperService.go b/client/argocdServer/ArgoClientWrapperService.go index 78274d321b..15be5a4641 100644 --- a/client/argocdServer/ArgoClientWrapperService.go +++ b/client/argocdServer/ArgoClientWrapperService.go @@ -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" ) @@ -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 } diff --git a/internal/constants/InternalErrorCode.go b/internal/constants/InternalErrorCode.go index 29b09d1fbf..dcaf5c68a6 100644 --- a/internal/constants/InternalErrorCode.go +++ b/internal/constants/InternalErrorCode.go @@ -106,4 +106,5 @@ const ( 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" ) diff --git a/internal/errors/bean.go b/internal/errors/bean.go index 92d291e24b..55d3726b54 100644 --- a/internal/errors/bean.go +++ b/internal/errors/bean.go @@ -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 @@ -25,3 +29,20 @@ func (r *ClientStatusCode) IsDeadlineExceededCode() bool { 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 + } +} diff --git a/pkg/k8s/K8sCommonService.go b/pkg/k8s/K8sCommonService.go index ddd3acbc0f..1659894084 100644 --- a/pkg/k8s/K8sCommonService.go +++ b/pkg/k8s/K8sCommonService.go @@ -50,7 +50,7 @@ type K8sCommonServiceImpl struct { } type K8sApplicationServiceConfig struct { BatchSize int `env:"BATCH_SIZE" envDefault:"5"` - TimeOutInSeconds int `env:"TIMEOUT_IN_SECONDS" envDefault:"5"` + TimeOutInSeconds int `env:"TIMEOUT_IN_SECONDS" envDefault:"0.1"` } func NewK8sCommonServiceImpl(Logger *zap.SugaredLogger, k8sUtils *k8s.K8sServiceImpl, From 21899673d2b7c7c198a5f4e9d05dd9c6c5ef6bba Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Fri, 3 May 2024 13:38:20 +0530 Subject: [PATCH 4/6] revert TIMEOUT_IN_SECONDS --- pkg/k8s/K8sCommonService.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/k8s/K8sCommonService.go b/pkg/k8s/K8sCommonService.go index 1659894084..ddd3acbc0f 100644 --- a/pkg/k8s/K8sCommonService.go +++ b/pkg/k8s/K8sCommonService.go @@ -50,7 +50,7 @@ type K8sCommonServiceImpl struct { } type K8sApplicationServiceConfig struct { BatchSize int `env:"BATCH_SIZE" envDefault:"5"` - TimeOutInSeconds int `env:"TIMEOUT_IN_SECONDS" envDefault:"0.1"` + TimeOutInSeconds int `env:"TIMEOUT_IN_SECONDS" envDefault:"5"` } func NewK8sCommonServiceImpl(Logger *zap.SugaredLogger, k8sUtils *k8s.K8sServiceImpl, From 5f05de85bb9f8d6333743d4a5552d60b2268470e Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Fri, 3 May 2024 15:24:01 +0530 Subject: [PATCH 5/6] revert bean TimeoutSlow param --- client/argocdServer/bean/bean.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/argocdServer/bean/bean.go b/client/argocdServer/bean/bean.go index 5440335b2b..5e5bb13f8d 100644 --- a/client/argocdServer/bean/bean.go +++ b/client/argocdServer/bean/bean.go @@ -36,7 +36,7 @@ const ( Progressing = "Progressing" Suspended = "Suspended" TimeoutFast = 10 * time.Second - TimeoutSlow = (1 * time.Second) / 10 + TimeoutSlow = 30 * time.Second TimeoutLazy = 60 * time.Second HIBERNATING = "HIBERNATING" SUCCEEDED = "Succeeded" From 52742d72dd800afe5093f6563376f8e03d7356e3 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Fri, 3 May 2024 15:28:31 +0530 Subject: [PATCH 6/6] fix --- api/restHandler/app/appList/AppListingRestHandler.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/api/restHandler/app/appList/AppListingRestHandler.go b/api/restHandler/app/appList/AppListingRestHandler.go index c06b3599db..22e908c3b2 100644 --- a/api/restHandler/app/appList/AppListingRestHandler.go +++ b/api/restHandler/app/appList/AppListingRestHandler.go @@ -665,7 +665,14 @@ func (handler AppListingRestHandlerImpl) handleResourceTreeErrAndDeletePipelineI } } // not returned yet therefore no specific error to be handled, send error in internal message - apiError.UserMessage = constants.UnableToFetchResourceTreeErrMsg + 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) }