Skip to content

Commit c66ccf5

Browse files
authored
fix: panic handlings and argocd app delete stuck in partial stage (#5770)
* fix: panic handlings * fix: false positive matrics on gitOps failures * fix: for GetConfigForHelmApps err: pg no row
1 parent 5170040 commit c66ccf5

File tree

7 files changed

+54
-16
lines changed

7 files changed

+54
-16
lines changed

pkg/appStore/installedApp/repository/InstalledAppRepository.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ func (impl InstalledAppRepositoryImpl) GetDeploymentSuccessfulStatusCountForTele
714714
func (impl InstalledAppRepositoryImpl) GetGitOpsInstalledAppsWhereArgoAppDeletedIsTrue(installedAppId int, envId int) (InstalledApps, error) {
715715
var installedApps InstalledApps
716716
err := impl.dbConnection.Model(&installedApps).
717-
Column("installed_apps.*", "App.app_name", "Environment.namespace", "Environment.cluster_id", "Environment.environment_name").
717+
Column("installed_apps.*", "App.id", "App.app_name", "Environment.namespace", "Environment.cluster_id", "Environment.environment_name").
718718
Where("deployment_app_delete_request = ?", true).
719719
Where("installed_apps.active = ?", true).
720720
Where("installed_apps.id = ?", installedAppId).

pkg/appStore/installedApp/service/AppStoreDeploymentService.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ func (impl *AppStoreDeploymentServiceImpl) MarkGitOpsInstalledAppsDeletedIfArgoA
877877
apiError.InternalMessage = "error in fetching partially deleted argoCd apps from installed app repo"
878878
return apiError
879879
}
880-
deploymentConfig, err := impl.deploymentConfigService.GetConfigForHelmApps(installedAppId, envId)
880+
deploymentConfig, err := impl.deploymentConfigService.GetConfigForHelmApps(installedApp.App.Id, envId)
881881
if err != nil {
882882
impl.logger.Errorw("error in getting deployment config by appId and envId", "appId", installedAppId, "envId", envId, "err", err)
883883
apiError.HttpStatusCode = http.StatusInternalServerError

pkg/deployment/gitOps/git/GitServiceGithub.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ package git
1919
import (
2020
"context"
2121
"crypto/tls"
22+
"errors"
2223
"fmt"
24+
"github.com/devtron-labs/common-lib/utils/runTime"
2325
bean2 "github.com/devtron-labs/devtron/api/bean/gitOps"
2426
globalUtil "github.com/devtron-labs/devtron/util"
2527
"github.com/devtron-labs/devtron/util/retryFunc"
@@ -91,6 +93,15 @@ func (impl GitHubClient) DeleteRepository(config *bean2.GitOpsConfigDto) error {
9193
return nil
9294
}
9395

96+
func IsRepoNotFound(err error) bool {
97+
if err == nil {
98+
return false
99+
}
100+
var responseErr *github.ErrorResponse
101+
ok := errors.As(err, &responseErr)
102+
return ok && responseErr.Response.StatusCode == 404
103+
}
104+
94105
func (impl GitHubClient) CreateRepository(ctx context.Context, config *bean2.GitOpsConfigDto) (url string, isNew bool, detailedErrorGitOpsConfigActions DetailedErrorGitOpsConfigActions) {
95106
var err error
96107
start := time.Now()
@@ -100,15 +111,14 @@ func (impl GitHubClient) CreateRepository(ctx context.Context, config *bean2.Git
100111

101112
detailedErrorGitOpsConfigActions.StageErrorMap = make(map[string]error)
102113
repoExists := true
103-
url, err = impl.GetRepoUrl(config)
114+
url, err = impl.getRepoUrl(ctx, config, IsRepoNotFound)
104115
if err != nil {
105-
responseErr, ok := err.(*github.ErrorResponse)
106-
if !ok || responseErr.Response.StatusCode != 404 {
116+
if IsRepoNotFound(err) {
117+
repoExists = false
118+
} else {
107119
impl.logger.Errorw("error in creating github repo", "err", err)
108120
detailedErrorGitOpsConfigActions.StageErrorMap[GetRepoUrlStage] = err
109121
return "", false, detailedErrorGitOpsConfigActions
110-
} else {
111-
repoExists = false
112122
}
113123
}
114124
if repoExists {
@@ -251,12 +261,21 @@ func (impl GitHubClient) CommitValues(ctx context.Context, config *ChartConfig,
251261
}
252262

253263
func (impl GitHubClient) GetRepoUrl(config *bean2.GitOpsConfigDto) (repoUrl string, err error) {
264+
ctx := context.Background()
265+
return impl.getRepoUrl(ctx, config, globalUtil.AllPublishableError())
266+
}
267+
268+
func (impl GitHubClient) getRepoUrl(ctx context.Context, config *bean2.GitOpsConfigDto,
269+
isNonPublishableError globalUtil.EvalIsNonPublishableErr) (repoUrl string, err error) {
254270
start := time.Now()
255271
defer func() {
272+
if isNonPublishableError(err) {
273+
impl.logger.Debugw("found non publishable error. skipping metrics publish!", "caller method", runTime.GetCallerFunctionName(), "err", err)
274+
return
275+
}
256276
globalUtil.TriggerGitOpsMetrics("GetRepoUrl", "GitHubClient", start, err)
257277
}()
258278

259-
ctx := context.Background()
260279
repo, _, err := impl.client.Repositories.Get(ctx, impl.org, config.GitRepoName)
261280
if err != nil {
262281
impl.logger.Errorw("error in getting repo url by repo name", "org", impl.org, "gitRepoName", config.GitRepoName, "err", err)

pkg/pipeline/CdHandler.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,11 +475,15 @@ func (impl *CdHandlerImpl) getWorkflowLogs(pipelineId int, cdWorkflow *pipelineC
475475
if !cdWorkflow.BlobStorageEnabled {
476476
return nil, nil, errors.New("logs-not-stored-in-repository")
477477
} else if string(v1alpha1.NodeSucceeded) == cdWorkflow.Status || string(v1alpha1.NodeError) == cdWorkflow.Status || string(v1alpha1.NodeFailed) == cdWorkflow.Status || cdWorkflow.Status == executors.WorkflowCancel {
478-
impl.Logger.Debugw("pod is not live ", "err", err)
478+
impl.Logger.Debugw("pod is not live", "podName", cdWorkflow.PodName, "err", err)
479479
return impl.getLogsFromRepository(pipelineId, cdWorkflow, clusterConfig, runStageInEnv)
480480
}
481-
impl.Logger.Errorw("err on fetch workflow logs", "err", err)
482-
return nil, nil, err
481+
if err != nil {
482+
impl.Logger.Errorw("err on fetch workflow logs", "err", err)
483+
return nil, nil, err
484+
} else if logStream == nil {
485+
return nil, cleanUp, fmt.Errorf("no logs found for pod %s", cdWorkflow.PodName)
486+
}
483487
}
484488
logReader := bufio.NewReader(logStream)
485489
return logReader, cleanUp, err

pkg/pipeline/CiHandler.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -798,11 +798,15 @@ func (impl *CiHandlerImpl) getWorkflowLogs(pipelineId int, ciWorkflow *pipelineC
798798
if !ciWorkflow.BlobStorageEnabled {
799799
return nil, nil, &util.ApiError{Code: "200", HttpStatusCode: 400, UserMessage: "logs-not-stored-in-repository"}
800800
} else if string(v1alpha1.NodeSucceeded) == ciWorkflow.Status || string(v1alpha1.NodeError) == ciWorkflow.Status || string(v1alpha1.NodeFailed) == ciWorkflow.Status || ciWorkflow.Status == executors.WorkflowCancel {
801-
impl.Logger.Errorw("err", "err", err)
801+
impl.Logger.Debugw("pod is not live", "podName", ciWorkflow.PodName, "err", err)
802802
return impl.getLogsFromRepository(pipelineId, ciWorkflow, clusterConfig, isExt)
803803
}
804-
impl.Logger.Errorw("err", "err", err)
805-
return nil, nil, &util.ApiError{Code: "200", HttpStatusCode: 400, UserMessage: err.Error()}
804+
if err != nil {
805+
impl.Logger.Errorw("err on fetch workflow logs", "err", err)
806+
return nil, nil, &util.ApiError{Code: "200", HttpStatusCode: 400, UserMessage: err.Error()}
807+
} else if logStream == nil {
808+
return nil, cleanUp, fmt.Errorf("no logs found for pod %s", ciWorkflow.PodName)
809+
}
806810
}
807811
logReader := bufio.NewReader(logStream)
808812
return logReader, cleanUp, err

pkg/pipeline/CiLogService.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,12 @@ func (impl *CiLogServiceImpl) FetchRunningWorkflowLogs(ciLogRequest types.BuildL
6868
}
6969
req := impl.k8sUtil.GetLogsForAPod(kubeClient, ciLogRequest.Namespace, ciLogRequest.PodName, CiPipeline.Main, true)
7070
podLogs, err := req.Stream(context.Background())
71-
if podLogs == nil || err != nil {
72-
impl.logger.Errorw("error in opening stream", "name", ciLogRequest.PodName)
71+
if err != nil {
72+
impl.logger.Errorw("error in opening stream", "name", ciLogRequest.PodName, "err", err)
7373
return nil, nil, err
74+
} else if podLogs == nil {
75+
impl.logger.Warnw("no stream reader found", "name", ciLogRequest.PodName)
76+
return nil, func() error { return nil }, err
7477
}
7578
cleanUpFunc := func() error {
7679
impl.logger.Info("closing running pod log stream")

util/helper.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,14 @@ func TriggerGitOpsMetrics(operation string, method string, startTime time.Time,
278278
middleware.GitOpsDuration.WithLabelValues(operation, method, status).Observe(time.Since(startTime).Seconds())
279279
}
280280

281+
type EvalIsNonPublishableErr func(err error) bool
282+
283+
func AllPublishableError() EvalIsNonPublishableErr {
284+
return func(err error) bool {
285+
return false
286+
}
287+
}
288+
281289
func InterfaceToString(resp interface{}) string {
282290
var dat string
283291
b, err := json.Marshal(resp)

0 commit comments

Comments
 (0)