diff --git a/api/restHandler/GitOpsConfigRestHandler.go b/api/restHandler/GitOpsConfigRestHandler.go index 4b04c8c103..9309c1fc0d 100644 --- a/api/restHandler/GitOpsConfigRestHandler.go +++ b/api/restHandler/GitOpsConfigRestHandler.go @@ -204,7 +204,7 @@ func (impl GitOpsConfigRestHandlerImpl) GitOpsConfigured(w http.ResponseWriter, if gitopsConf.Active { gitopsConfigured = true allowCustomRepository = gitopsConf.AllowCustomRepository - + break } } } diff --git a/client/argocdServer/ArgoClientWrapperService.go b/client/argocdServer/ArgoClientWrapperService.go index 80edfb339f..d12281c10d 100644 --- a/client/argocdServer/ArgoClientWrapperService.go +++ b/client/argocdServer/ArgoClientWrapperService.go @@ -3,6 +3,7 @@ package argocdServer import ( "context" "encoding/json" + "fmt" application2 "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" repository2 "github.com/argoproj/argo-cd/v2/pkg/apiclient/repository" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" @@ -13,6 +14,7 @@ import ( "github.com/devtron-labs/devtron/client/argocdServer/repository" "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" "strings" @@ -43,8 +45,8 @@ type ArgoClientWrapperService interface { // UpdateArgoCDSyncModeIfNeeded - if ARGO_AUTO_SYNC_ENABLED=true and app is in manual sync mode or vice versa update app UpdateArgoCDSyncModeIfNeeded(ctx context.Context, argoApplication *v1alpha1.Application) (err error) - // RegisterGitOpsRepoInArgo - register a repository in argo-cd with retry mechanism - RegisterGitOpsRepoInArgo(ctx context.Context, gitOpsRepoUrl string, userId int32) error + // RegisterGitOpsRepoInArgoWithRetry - register a repository in argo-cd with retry mechanism + RegisterGitOpsRepoInArgoWithRetry(ctx context.Context, gitOpsRepoUrl string, userId int32) error // GetArgoAppByName fetches an argoCd app by its name GetArgoAppByName(ctx context.Context, appName string) (*v1alpha1.Application, error) @@ -156,63 +158,17 @@ func (impl *ArgoClientWrapperServiceImpl) CreateRequestForArgoCDSyncModeUpdateRe }}} } -func (impl *ArgoClientWrapperServiceImpl) RegisterGitOpsRepoInArgo(ctx context.Context, gitOpsRepoUrl string, userId int32) error { - - retryCount := 0 - // label to register git repository in ArgoCd - // ArgoCd requires approx 80 to 120 sec after the last commit to allow create-repository action - // hence this operation needed to be perform with retry -CreateArgoRepositoryWithRetry: - repo := &v1alpha1.Repository{ - Repo: gitOpsRepoUrl, +func (impl *ArgoClientWrapperServiceImpl) RegisterGitOpsRepoInArgoWithRetry(ctx context.Context, gitOpsRepoUrl string, userId int32) error { + callback := func() error { + return impl.createRepoInArgoCd(ctx, gitOpsRepoUrl) } - repo, err := impl.repositoryService.Create(ctx, &repository2.RepoCreateRequest{Repo: repo, Upsert: true}) - if err != nil { - impl.logger.Errorw("error in creating argo Repository", "err", err) - retryCount, err = impl.handleArgoRepoCreationError(retryCount, gitOpsRepoUrl, userId, err) - if err != nil { - impl.logger.Errorw("error in RegisterGitOpsRepoInArgo with retry operation", "err", err) - return err - } - impl.logger.Errorw("retrying RegisterGitOpsRepoInArgo operation", "retry count", retryCount) - time.Sleep(10 * time.Second) - goto CreateArgoRepositoryWithRetry - } - impl.logger.Infow("gitOps repo registered in argo", "name", gitOpsRepoUrl) - return err -} - -func (impl *ArgoClientWrapperServiceImpl) handleArgoRepoCreationError(retryCount int, repoUrl string, userId int32, argoCdErr error) (int, error) { - // retry limit exhausted - if retryCount >= bean.RegisterRepoMaxRetryCount { - return 0, argoCdErr + argoCdErr := retryFunc.Retry(callback, impl.isRetryableArgoRepoCreationError, bean.RegisterRepoMaxRetryCount, 10*time.Second, impl.logger) + if argoCdErr != nil { + impl.logger.Errorw("error in registering GitOps repository", "repoName", gitOpsRepoUrl, "err", argoCdErr) + return impl.handleArgoRepoCreationError(argoCdErr, ctx, gitOpsRepoUrl, userId) } - // This error occurs inconsistently; ArgoCD requires 80-120s after last commit for create repository operation - argoRepoSyncDelayErrMessage := "Unable to resolve 'HEAD' to a commit SHA" - isSyncDelayError := strings.Contains(argoCdErr.Error(), argoRepoSyncDelayErrMessage) - - // ArgoCD can't register empty repo and throws these error message in such cases - emptyRepoErrorMessages := []string{"failed to get index: 404 Not Found", "remote repository is empty"} - isEmptyRepoError := false - for _, errMsg := range emptyRepoErrorMessages { - if strings.Contains(argoCdErr.Error(), errMsg) { - isEmptyRepoError = true - } - } - // unknown error handling - if !isSyncDelayError && !isEmptyRepoError { - return 0, argoCdErr - } - if isEmptyRepoError { - // - found empty repository, create some file in repository - gitOpsRepoName := impl.gitOpsConfigReadService.GetGitOpsRepoNameFromUrl(repoUrl) - err := impl.gitOperationService.CreateReadmeInGitRepo(gitOpsRepoName, userId) - if err != nil { - impl.logger.Errorw("error in creating file in git repo", "err", err) - return 0, err - } - } - return retryCount + 1, nil + impl.logger.Infow("gitOps repo registered in argo", "repoName", gitOpsRepoUrl) + return nil } func (impl *ArgoClientWrapperServiceImpl) GetArgoAppByName(ctx context.Context, appName string) (*v1alpha1.Application, error) { @@ -246,9 +202,52 @@ func (impl *ArgoClientWrapperServiceImpl) GetGitOpsRepoName(ctx context.Context, impl.logger.Errorw("no argo app exists", "acdAppName", appName, "err", err) return gitOpsRepoName, err } - if acdApplication != nil { + // safety checks nil pointers + if acdApplication != nil && acdApplication.Spec.Source != nil { gitOpsRepoUrl := acdApplication.Spec.Source.RepoURL gitOpsRepoName = impl.gitOpsConfigReadService.GetGitOpsRepoNameFromUrl(gitOpsRepoUrl) + return gitOpsRepoName, nil + } + return gitOpsRepoName, fmt.Errorf("unable to get any ArgoCd application '%s'", appName) +} + +// createRepoInArgoCd is the wrapper function to Create Repository in ArgoCd +func (impl *ArgoClientWrapperServiceImpl) createRepoInArgoCd(ctx context.Context, gitOpsRepoUrl string) error { + repo := &v1alpha1.Repository{ + Repo: gitOpsRepoUrl, + } + repo, err := impl.repositoryService.Create(ctx, &repository2.RepoCreateRequest{Repo: repo, Upsert: true}) + if err != nil { + impl.logger.Errorw("error in creating argo Repository", "err", err) + return err + } + return nil +} + +// isRetryableArgoRepoCreationError returns whether to retry or not, based on the error returned from callback func +// In createRepoInArgoCd, we will retry only if the error matches to bean.ArgoRepoSyncDelayErr +func (impl *ArgoClientWrapperServiceImpl) isRetryableArgoRepoCreationError(argoCdErr error) bool { + return strings.Contains(argoCdErr.Error(), bean.ArgoRepoSyncDelayErr) +} + +// handleArgoRepoCreationError - manages the error thrown while performing createRepoInArgoCd +func (impl *ArgoClientWrapperServiceImpl) handleArgoRepoCreationError(argoCdErr error, ctx context.Context, gitOpsRepoUrl string, userId int32) error { + emptyRepoErrorMessages := bean.EmptyRepoErrorList + isEmptyRepoError := false + for _, errMsg := range emptyRepoErrorMessages { + if strings.Contains(argoCdErr.Error(), errMsg) { + isEmptyRepoError = true + } + } + if isEmptyRepoError { + // - found empty repository, create some file in repository + gitOpsRepoName := impl.gitOpsConfigReadService.GetGitOpsRepoNameFromUrl(gitOpsRepoUrl) + err := impl.gitOperationService.CreateReadmeInGitRepo(gitOpsRepoName, userId) + if err != nil { + impl.logger.Errorw("error in creating file in git repo", "err", err) + return err + } } - return gitOpsRepoName, nil + // try to register with after creating readme file + return impl.createRepoInArgoCd(ctx, gitOpsRepoUrl) } diff --git a/client/argocdServer/bean/bean.go b/client/argocdServer/bean/bean.go index 0814999465..5e5bb13f8d 100644 --- a/client/argocdServer/bean/bean.go +++ b/client/argocdServer/bean/bean.go @@ -21,8 +21,15 @@ type ArgoCdAppPatchReqDto struct { PatchType string } +// RegisterRepoMaxRetryCount is the maximum retries to be performed to register a repository in ArgoCd const RegisterRepoMaxRetryCount = 3 +// EmptyRepoErrorList - ArgoCD can't register empty repo and throws these error message in such cases +var EmptyRepoErrorList = []string{"failed to get index: 404 Not Found", "remote repository is empty"} + +// ArgoRepoSyncDelayErr - This error occurs inconsistently; ArgoCD requires 80-120s after last commit for create repository operation +const ArgoRepoSyncDelayErr = "Unable to resolve 'HEAD' to a commit SHA" + const ( Degraded = "Degraded" Healthy = "Healthy" diff --git a/pkg/app/ManifestPushService.go b/pkg/app/ManifestPushService.go index 0ca5182bb1..db4af44cde 100644 --- a/pkg/app/ManifestPushService.go +++ b/pkg/app/ManifestPushService.go @@ -72,7 +72,7 @@ func (impl *GitOpsManifestPushServiceImpl) migrateRepoForGitOperation(manifestPu return "", fmt.Errorf("No repository configured for Gitops! Error while creating git repository: '%s'", gitOpsRepoName) } chartGitAttr.ChartLocation = manifestPushTemplate.ChartLocation - err = impl.argoClientWrapperService.RegisterGitOpsRepoInArgo(ctx, chartGitAttr.RepoUrl, manifestPushTemplate.UserId) + err = impl.argoClientWrapperService.RegisterGitOpsRepoInArgoWithRetry(ctx, chartGitAttr.RepoUrl, manifestPushTemplate.UserId) if err != nil { impl.logger.Errorw("error in registering app in acd", "err", err) return "", fmt.Errorf("Error in registering repository '%s' in ArgoCd", gitOpsRepoName) diff --git a/pkg/appStore/installedApp/repository/InstalledAppRepository.go b/pkg/appStore/installedApp/repository/InstalledAppRepository.go index 96a6a0910b..6d737b7724 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppRepository.go +++ b/pkg/appStore/installedApp/repository/InstalledAppRepository.go @@ -131,6 +131,7 @@ type InstalledAppRepository interface { GetInstalledAppByAppName(appName string) (*InstalledApps, error) GetInstalledAppByAppIdAndDeploymentType(appId int, deploymentAppType string) (InstalledApps, error) GetInstalledAppByInstalledAppVersionId(installedAppVersionId int) (InstalledApps, error) + // GetInstalledAppByGitOpsAppName will return all the active installed_apps with matching `app_name-environment_name` GetInstalledAppByGitOpsAppName(acdAppName string) (*InstalledApps, error) GetInstalledAppByGitRepoUrl(repoName, repoUrl string) (*InstalledApps, error) @@ -755,6 +756,7 @@ func (impl InstalledAppRepositoryImpl) GetInstalledAppByGitOpsAppName(acdAppName model := &InstalledApps{} err := impl.dbConnection.Model(model). Column("installed_apps.*", "App", "Environment"). + // TODO add deployment_app_name filed in installed_apps table Where("CONCAT(app.app_name, ?, environment.environment_name) = ?", "-", acdAppName). Where("installed_apps.active = true"). Where("environment.active = true"). diff --git a/pkg/appStore/installedApp/service/FullMode/deployment/FullModeDeploymentService.go b/pkg/appStore/installedApp/service/FullMode/deployment/FullModeDeploymentService.go index 4f16da3443..88bcf7ebe3 100644 --- a/pkg/appStore/installedApp/service/FullMode/deployment/FullModeDeploymentService.go +++ b/pkg/appStore/installedApp/service/FullMode/deployment/FullModeDeploymentService.go @@ -135,7 +135,7 @@ func (impl *FullModeDeploymentServiceImpl) InstallApp(installAppVersionRequest * ctx, cancel := context.WithTimeout(ctx, 1*time.Minute) defer cancel() //STEP 4: registerInArgo - err := impl.argoClientWrapperService.RegisterGitOpsRepoInArgo(ctx, chartGitAttr.RepoUrl, installAppVersionRequest.UserId) + err := impl.argoClientWrapperService.RegisterGitOpsRepoInArgoWithRetry(ctx, chartGitAttr.RepoUrl, installAppVersionRequest.UserId) if err != nil { impl.Logger.Errorw("error in argo registry", "err", err) return nil, err diff --git a/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppArgoCdService.go b/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppArgoCdService.go index 7fbbf2bf6d..26e3e9e31a 100644 --- a/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppArgoCdService.go +++ b/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppArgoCdService.go @@ -177,7 +177,7 @@ func (impl *FullModeDeploymentServiceImpl) patchAcdApp(ctx context.Context, inst ctx, cancel := context.WithTimeout(ctx, 1*time.Minute) defer cancel() //registerInArgo - err := impl.argoClientWrapperService.RegisterGitOpsRepoInArgo(ctx, chartGitAttr.RepoUrl, installAppVersionRequest.UserId) + err := impl.argoClientWrapperService.RegisterGitOpsRepoInArgoWithRetry(ctx, chartGitAttr.RepoUrl, installAppVersionRequest.UserId) if err != nil { impl.Logger.Errorw("error in argo registry", "err", err) return err diff --git a/pkg/chart/ChartService.go b/pkg/chart/ChartService.go index fbe68f9b86..40c00f438d 100644 --- a/pkg/chart/ChartService.go +++ b/pkg/chart/ChartService.go @@ -64,7 +64,7 @@ type ChartService interface { ChartRefAutocompleteForAppOrEnv(appId int, envId int) (*chartRefBean.ChartRefAutocompleteResponse, error) - ConfigureGitOpsRepoUrl(appId int, repoUrl, chartLocation string, userId int32) error + ConfigureGitOpsRepoUrl(appId int, repoUrl, chartLocation string, isCustomRepo bool, userId int32) error OverrideGitOpsRepoUrl(appId int, repoUrl string, userId int32) error IsGitOpsRepoConfiguredForDevtronApps(appId int) (bool, error) @@ -170,18 +170,35 @@ func (impl *ChartServiceImpl) Create(templateRequest TemplateRequest, ctx contex impl.logger.Debug("updating all other charts which are not latest but may be set previous true, setting previous=false") //step 2 - noLatestCharts, err := impl.chartRepository.FindNoLatestChartForAppByAppId(templateRequest.AppId) + tx, err := impl.chartRepository.StartTx() + if err != nil { + impl.logger.Errorw("error in starting transaction to update charts", "error", err) + return nil, err + } + defer impl.chartRepository.RollbackTx(tx) + + noLatestCharts, dbErr := impl.chartRepository.FindNoLatestChartForAppByAppId(templateRequest.AppId) + if dbErr != nil && !util.IsErrNoRows(dbErr) { + impl.logger.Errorw("error in getting non-latest charts", "appId", templateRequest.AppId, "err", err) + return nil, err + } + var updatedCharts []*chartRepoRepository.Chart for _, noLatestChart := range noLatestCharts { if noLatestChart.Id != templateRequest.Id { - noLatestChart.Latest = false // these are already false by d way noLatestChart.Previous = false - err = impl.chartRepository.Update(noLatestChart) - if err != nil { - return nil, err - } + updatedCharts = append(updatedCharts, noLatestChart) } } + err = impl.chartRepository.UpdateAllInTx(tx, updatedCharts) + if err != nil { + return nil, err + } + err = impl.chartRepository.CommitTx(tx) + if err != nil { + impl.logger.Errorw("error in committing transaction to update charts", "error", err) + return nil, err + } impl.logger.Debug("now going to update latest entry in db to false and previous flag = true") // now finally update latest entry in db to false and previous true @@ -414,7 +431,7 @@ func (impl *ChartServiceImpl) chartAdaptor(chart *chartRepoRepository.Chart, isA if !apiGitOpsBean.IsGitOpsRepoNotConfigured(chart.GitRepoUrl) { gitRepoUrl = chart.GitRepoUrl } - return &TemplateRequest{ + templateRequest := &TemplateRequest{ RefChartTemplate: chart.ReferenceTemplate, Id: chart.Id, AppId: chart.AppId, @@ -428,7 +445,11 @@ func (impl *ChartServiceImpl) chartAdaptor(chart *chartRepoRepository.Chart, isA CurrentViewEditor: chart.CurrentViewEditor, GitRepoUrl: gitRepoUrl, IsCustomGitRepository: chart.IsCustomGitRepository, - }, nil + } + if chart.Latest { + templateRequest.LatestChartVersion = chart.ChartVersion + } + return templateRequest, nil } func (impl *ChartServiceImpl) getChartMetaData(templateRequest TemplateRequest) (*chart.Metadata, error) { @@ -494,8 +515,7 @@ func (impl *ChartServiceImpl) IsGitOpsRepoConfiguredForDevtronApps(appId int) (b gitOpsConfigStatus, err := impl.gitOpsConfigReadService.IsGitOpsConfigured() if util.IsErrNoRows(err) { return false, nil - } - if err != nil { + } else if err != nil { impl.logger.Errorw("error in fetching latest chart for app by appId") return false, err } @@ -507,10 +527,7 @@ func (impl *ChartServiceImpl) IsGitOpsRepoConfiguredForDevtronApps(appId int) (b impl.logger.Errorw("error in fetching latest chart for app by appId") return false, err } - if apiGitOpsBean.IsGitOpsRepoNotConfigured(latestChartConfiguredInApp.GitRepoUrl) { - return false, nil - } - return true, nil + return !apiGitOpsBean.IsGitOpsRepoNotConfigured(latestChartConfiguredInApp.GitRepoUrl), nil } func (impl *ChartServiceImpl) FindLatestChartForAppByAppId(appId int) (chartTemplate *TemplateRequest, err error) { @@ -574,22 +591,40 @@ func (impl *ChartServiceImpl) UpdateAppOverride(ctx context.Context, templateReq impl.logger.Debug("updating all other charts which are not latest but may be set previous true, setting previous=false") //step 3 + tx, err := impl.chartRepository.StartTx() + if err != nil { + impl.logger.Errorw("error in starting transaction to update charts", "error", err) + return nil, err + } + defer impl.chartRepository.RollbackTx(tx) + _, span = otel.Tracer("orchestrator").Start(ctx, "chartRepository.FindNoLatestChartForAppByAppId") - noLatestCharts, err := impl.chartRepository.FindNoLatestChartForAppByAppId(templateRequest.AppId) + noLatestCharts, dbErr := impl.chartRepository.FindNoLatestChartForAppByAppId(templateRequest.AppId) span.End() + if dbErr != nil && !util.IsErrNoRows(dbErr) { + impl.logger.Errorw("error in getting non-latest charts", "appId", templateRequest.AppId, "err", err) + return nil, err + } + var updatedCharts []*chartRepoRepository.Chart for _, noLatestChart := range noLatestCharts { if noLatestChart.Id != templateRequest.Id { - noLatestChart.Latest = false // these are already false by d way noLatestChart.Previous = false - _, span = otel.Tracer("orchestrator").Start(ctx, "chartRepository.Update") - err = impl.chartRepository.Update(noLatestChart) - span.End() - if err != nil { - return nil, err - } + updatedCharts = append(updatedCharts, noLatestChart) } } + _, span = otel.Tracer("orchestrator").Start(ctx, "chartRepository.Update") + err = impl.chartRepository.UpdateAllInTx(tx, updatedCharts) + span.End() + if err != nil { + return nil, err + } + + err = impl.chartRepository.CommitTx(tx) + if err != nil { + impl.logger.Errorw("error in committing transaction to update charts", "error", err) + return nil, err + } impl.logger.Debug("now going to update latest entry in db to false and previous flag = true") // now finally update latest entry in db to false and previous true @@ -856,22 +891,36 @@ func (impl *ChartServiceImpl) CheckIfChartRefUserUploadedByAppId(id int) (bool, return chartData.UserUploaded, err } -func (impl *ChartServiceImpl) ConfigureGitOpsRepoUrl(appId int, repoUrl, chartLocation string, userId int32) error { +func (impl *ChartServiceImpl) ConfigureGitOpsRepoUrl(appId int, repoUrl, chartLocation string, isCustomRepo bool, userId int32) error { charts, err := impl.chartRepository.FindActiveChartsByAppId(appId) - if err != nil && util.IsErrNoRows(err) { + if err != nil && !util.IsErrNoRows(err) { + return err + } + tx, err := impl.chartRepository.StartTx() + if err != nil { + impl.logger.Errorw("error in starting transaction to update charts", "error", err) return err } + defer impl.chartRepository.RollbackTx(tx) + var updatedCharts []*chartRepoRepository.Chart for _, ch := range charts { if apiGitOpsBean.IsGitOpsRepoNotConfigured(ch.GitRepoUrl) { ch.GitRepoUrl = repoUrl + ch.IsCustomGitRepository = isCustomRepo ch.ChartLocation = chartLocation ch.UpdateAuditLog(userId) - err = impl.chartRepository.Update(ch) - if err != nil { - return err - } + updatedCharts = append(updatedCharts, ch) } } + err = impl.chartRepository.UpdateAllInTx(tx, updatedCharts) + if err != nil { + return err + } + err = impl.chartRepository.CommitTx(tx) + if err != nil { + impl.logger.Errorw("error in committing transaction to update charts", "error", err) + return err + } return nil } @@ -880,14 +929,29 @@ func (impl *ChartServiceImpl) OverrideGitOpsRepoUrl(appId int, repoUrl string, u if err != nil && util.IsErrNoRows(err) { return err } + tx, err := impl.chartRepository.StartTx() + if err != nil { + impl.logger.Errorw("error in starting transaction to update charts", "error", err) + return err + } + defer impl.chartRepository.RollbackTx(tx) + var updatedCharts []*chartRepoRepository.Chart for _, ch := range charts { - ch.GitRepoUrl = repoUrl - ch.UpdateAuditLog(userId) - err = impl.chartRepository.Update(ch) - if err != nil { - return err + if !ch.IsCustomGitRepository { + ch.GitRepoUrl = repoUrl + ch.UpdateAuditLog(userId) + updatedCharts = append(updatedCharts, ch) } } + err = impl.chartRepository.UpdateAllInTx(tx, updatedCharts) + if err != nil { + return err + } + err = impl.chartRepository.CommitTx(tx) + if err != nil { + impl.logger.Errorw("error in committing transaction to update charts", "error", err) + return err + } return nil } @@ -896,8 +960,7 @@ func (impl *ChartServiceImpl) IsGitOpsRepoAlreadyRegistered(gitOpsRepoUrl string if err != nil && !util.IsErrNoRows(err) { impl.logger.Errorw("error in fetching chartModel", "repoUrl", gitOpsRepoUrl, "err", err) return true, err - } - if util.IsErrNoRows(err) { + } else if util.IsErrNoRows(err) { return false, nil } impl.logger.Errorw("repository is already in use for devtron app", "repoUrl", gitOpsRepoUrl, "appId", chartModel.AppId) diff --git a/pkg/chart/bean.go b/pkg/chart/bean.go index 64a8e27d0a..970fb4f28e 100644 --- a/pkg/chart/bean.go +++ b/pkg/chart/bean.go @@ -30,6 +30,7 @@ type TemplateRequest struct { GitRepoUrl string `json:"-"` IsCustomGitRepository bool `json:"-"` UserId int32 `json:"-"` + LatestChartVersion string `json:"-"` } type ChartUpgradeRequest struct { diff --git a/pkg/chart/gitOpsConfig/DevtronAppGitOpsConfigService.go b/pkg/chart/gitOpsConfig/DevtronAppGitOpsConfigService.go index fde8026372..7614f36928 100644 --- a/pkg/chart/gitOpsConfig/DevtronAppGitOpsConfigService.go +++ b/pkg/chart/gitOpsConfig/DevtronAppGitOpsConfigService.go @@ -19,22 +19,21 @@ package gitOpsConfig import ( "context" - "fmt" apiGitOpsBean "github.com/devtron-labs/devtron/api/bean/gitOps" "github.com/devtron-labs/devtron/client/argocdServer" + chartService "github.com/devtron-labs/devtron/pkg/chart" "github.com/devtron-labs/devtron/pkg/chart/gitOpsConfig/bean" commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean" "github.com/devtron-labs/devtron/pkg/deployment/gitOps/config" "github.com/devtron-labs/devtron/pkg/deployment/gitOps/validation" bean3 "github.com/devtron-labs/devtron/pkg/deployment/gitOps/validation/bean" "net/http" + "path/filepath" //"github.com/devtron-labs/devtron/pkg/pipeline" - chartRepoRepository "github.com/devtron-labs/devtron/pkg/chartRepo/repository" - "time" - "github.com/devtron-labs/devtron/internal/util" + chartRepoRepository "github.com/devtron-labs/devtron/pkg/chartRepo/repository" "go.uber.org/zap" ) @@ -46,6 +45,7 @@ type DevtronAppGitOpConfigService interface { type DevtronAppGitOpConfigServiceImpl struct { logger *zap.SugaredLogger chartRepository chartRepoRepository.ChartRepository + chartService chartService.ChartService gitOpsConfigReadService config.GitOpsConfigReadService gitOpsValidationService validation.GitOpsValidationService argoClientWrapperService argocdServer.ArgoClientWrapperService @@ -53,12 +53,14 @@ type DevtronAppGitOpConfigServiceImpl struct { func NewDevtronAppGitOpConfigServiceImpl(logger *zap.SugaredLogger, chartRepository chartRepoRepository.ChartRepository, + chartService chartService.ChartService, gitOpsConfigReadService config.GitOpsConfigReadService, gitOpsValidationService validation.GitOpsValidationService, argoClientWrapperService argocdServer.ArgoClientWrapperService) *DevtronAppGitOpConfigServiceImpl { return &DevtronAppGitOpConfigServiceImpl{ logger: logger, chartRepository: chartRepository, + chartService: chartService, gitOpsConfigReadService: gitOpsConfigReadService, gitOpsValidationService: gitOpsValidationService, argoClientWrapperService: argoClientWrapperService, @@ -72,27 +74,44 @@ func (impl *DevtronAppGitOpConfigServiceImpl) SaveAppLevelGitOpsConfiguration(ap return err } if !gitOpsConfigurationStatus.IsGitOpsConfigured { - return fmt.Errorf("Gitops integration is not installed/configured. Please install/configure gitops.") + apiErr := &util.ApiError{ + HttpStatusCode: http.StatusPreconditionFailed, + UserMessage: "GitOps integration is not installed/configured. Please install/configure GitOps.", + InternalMessage: "GitOps integration is not installed/configured. Please install/configure GitOps.", + } + return apiErr } if !gitOpsConfigurationStatus.AllowCustomRepository { apiErr := &util.ApiError{ HttpStatusCode: http.StatusConflict, - UserMessage: "Invalid request! Please configure Gitops with 'Allow changing git repository for application'.", + UserMessage: "Invalid request! Please configure GitOps with 'Allow changing git repository for application'.", InternalMessage: "Invalid request! Custom repository is not valid, as the global configuration is updated", } return apiErr } if impl.isGitRepoUrlPresent(appGitOpsRequest.AppId) { - return fmt.Errorf("Invalid request! GitOps repository is already configured.") + apiErr := &util.ApiError{ + HttpStatusCode: http.StatusBadRequest, + UserMessage: "Invalid request! GitOps repository is already configured.", + InternalMessage: "Invalid request! GitOps repository is already configured.", + } + return apiErr } - charts, err := impl.chartRepository.FindActiveChartsByAppId(appGitOpsRequest.AppId) - if err != nil { - impl.logger.Errorw("Error in fetching charts for app", "err", err, "appId", appGitOpsRequest.AppId) + appDeploymentTemplate, err := impl.chartService.FindLatestChartForAppByAppId(appGitOpsRequest.AppId) + if util.IsErrNoRows(err) { + impl.logger.Errorw("no base charts configured for app", "appId", appGitOpsRequest.AppId, "err", err) + apiErr := &util.ApiError{ + HttpStatusCode: http.StatusPreconditionFailed, + UserMessage: "Invalid request! Base deployment chart is not configured for the app", + InternalMessage: "Invalid request! Base deployment chart is not configured for the app", + } + return apiErr + } else if err != nil { + impl.logger.Errorw("error in fetching latest chart for app by appId", "appId", appGitOpsRequest.AppId, "err", err) return err } - validateCustomGitRepoURLRequest := bean3.ValidateCustomGitRepoURLRequest{ GitRepoURL: appGitOpsRequest.GitOpsRepoURL, UserId: appGitOpsRequest.UserId, @@ -111,24 +130,19 @@ func (impl *DevtronAppGitOpConfigServiceImpl) SaveAppLevelGitOpsConfiguration(ap // ValidateCustomGitRepoURL returns sanitized repo url after validation appGitOpsRequest.GitOpsRepoURL = repoUrl chartGitAttr := &commonBean.ChartGitAttribute{ - RepoUrl: repoUrl, + RepoUrl: repoUrl, + ChartLocation: filepath.Join(appDeploymentTemplate.RefChartTemplate, appDeploymentTemplate.LatestChartVersion), } - err = impl.argoClientWrapperService.RegisterGitOpsRepoInArgo(ctx, chartGitAttr.RepoUrl, appGitOpsRequest.UserId) + err = impl.argoClientWrapperService.RegisterGitOpsRepoInArgoWithRetry(ctx, chartGitAttr.RepoUrl, appGitOpsRequest.UserId) if err != nil { impl.logger.Errorw("error while register git repo in argo", "err", err) return err } - - for _, chart := range charts { - chart.GitRepoUrl = repoUrl - chart.UpdatedOn = time.Now() - chart.UpdatedBy = appGitOpsRequest.UserId - chart.IsCustomGitRepository = gitOpsConfigurationStatus.AllowCustomRepository && appGitOpsRequest.GitOpsRepoURL != apiGitOpsBean.GIT_REPO_DEFAULT - err = impl.chartRepository.Update(chart) - if err != nil { - impl.logger.Errorw("error in updating git repo url in charts while saving git repo url", "err", err, "appGitOpsRequest", appGitOpsRequest) - return err - } + isCustomGitOpsRepo := gitOpsConfigurationStatus.AllowCustomRepository && appGitOpsRequest.GitOpsRepoURL != apiGitOpsBean.GIT_REPO_DEFAULT + err = impl.chartService.ConfigureGitOpsRepoUrl(appGitOpsRequest.AppId, chartGitAttr.RepoUrl, chartGitAttr.ChartLocation, isCustomGitOpsRepo, appGitOpsRequest.UserId) + if err != nil { + impl.logger.Errorw("error in updating git repo url in charts", "err", err) + return err } return nil } @@ -138,29 +152,40 @@ func (impl *DevtronAppGitOpConfigServiceImpl) GetAppLevelGitOpsConfiguration(app if err != nil { impl.logger.Errorw("error in fetching active gitOps config", "err", err) return nil, err - } - if !gitOpsConfigurationStatus.IsGitOpsConfigured { - return nil, fmt.Errorf("Gitops integration is not installed/configured. Please install/configure gitops.") - } - if !gitOpsConfigurationStatus.AllowCustomRepository { + } else if !gitOpsConfigurationStatus.IsGitOpsConfigured { + apiErr := &util.ApiError{ + HttpStatusCode: http.StatusPreconditionFailed, + UserMessage: "GitOps integration is not installed/configured. Please install/configure GitOps.", + InternalMessage: "GitOps integration is not installed/configured. Please install/configure GitOps.", + } + return nil, apiErr + } else if !gitOpsConfigurationStatus.AllowCustomRepository { apiErr := &util.ApiError{ HttpStatusCode: http.StatusConflict, - UserMessage: "Invalid request! Please configure Gitops with 'Allow changing git repository for application'.", + UserMessage: "Invalid request! Please configure GitOps with 'Allow changing git repository for application'.", InternalMessage: "Invalid request! Custom repository is not valid, as the global configuration is updated", } return nil, apiErr } - chart, err := impl.chartRepository.FindLatestChartForAppByAppId(appId) - if err != nil { - impl.logger.Errorw("error in fetching latest chart for app by appId", "err", err, "appId", appId) + appDeploymentTemplate, err := impl.chartService.FindLatestChartForAppByAppId(appId) + if util.IsErrNoRows(err) { + impl.logger.Errorw("no base charts configured for app", "appId", appId, "err", err) + apiErr := &util.ApiError{ + HttpStatusCode: http.StatusPreconditionFailed, + UserMessage: "Invalid request! Base deployment chart is not configured for the app", + InternalMessage: "Invalid request! Base deployment chart is not configured for the app", + } + return nil, apiErr + } else if err != nil { + impl.logger.Errorw("error in fetching latest chart for app by appId", "appId", appId, "err", err) return nil, err } appGitOpsConfigResponse := &bean.AppGitOpsConfigResponse{ IsEditable: true, } - isGitOpsRepoConfigured := !apiGitOpsBean.IsGitOpsRepoNotConfigured(chart.GitRepoUrl) + isGitOpsRepoConfigured := !apiGitOpsBean.IsGitOpsRepoNotConfigured(appDeploymentTemplate.GitRepoUrl) if isGitOpsRepoConfigured { - appGitOpsConfigResponse.GitRepoURL = chart.GitRepoUrl + appGitOpsConfigResponse.GitRepoURL = appDeploymentTemplate.GitRepoUrl appGitOpsConfigResponse.IsEditable = false return appGitOpsConfigResponse, nil } @@ -173,8 +198,5 @@ func (impl *DevtronAppGitOpConfigServiceImpl) isGitRepoUrlPresent(appId int) boo impl.logger.Errorw("error fetching git repo url from the latest chart") return false } - if apiGitOpsBean.IsGitOpsRepoNotConfigured(fetchedChart.GitRepoUrl) { - return false - } - return true + return !apiGitOpsBean.IsGitOpsRepoNotConfigured(fetchedChart.GitRepoUrl) } diff --git a/pkg/chartRepo/repository/ChartsRepository.go b/pkg/chartRepo/repository/ChartsRepository.go index bf5d30ec15..c742b943c7 100644 --- a/pkg/chartRepo/repository/ChartsRepository.go +++ b/pkg/chartRepo/repository/ChartsRepository.go @@ -45,6 +45,7 @@ type ChartRepository interface { FindLatestByAppId(appId int) (chart *Chart, err error) FindById(id int) (chart *Chart, err error) Update(chart *Chart) error + UpdateAllInTx(tx *pg.Tx, charts []*Chart) error FindActiveChartsByAppId(appId int) (charts []*Chart, err error) FindLatestChartForAppByAppId(appId int) (chart *Chart, err error) @@ -54,14 +55,19 @@ type ChartRepository interface { FindPreviousChartByAppId(appId int) (chart *Chart, err error) FindNumberOfAppsWithDeploymentTemplate(appIds []int) (int, error) FindChartByGitRepoUrl(gitRepoUrl string) (*Chart, error) + sql.TransactionWrapper } func NewChartRepository(dbConnection *pg.DB) *ChartRepositoryImpl { - return &ChartRepositoryImpl{dbConnection: dbConnection} + return &ChartRepositoryImpl{ + dbConnection: dbConnection, + TransactionUtilImpl: sql.NewTransactionUtilImpl(dbConnection), + } } type ChartRepositoryImpl struct { dbConnection *pg.DB + *sql.TransactionUtilImpl } func (repositoryImpl ChartRepositoryImpl) FindOne(chartRepo, chartName, chartVersion string) (*Chart, error) { @@ -188,6 +194,16 @@ func (repositoryImpl ChartRepositoryImpl) Update(chart *Chart) error { return err } +func (repositoryImpl ChartRepositoryImpl) UpdateAllInTx(tx *pg.Tx, charts []*Chart) error { + for _, chart := range charts { + _, err := tx.Model(chart).WherePK().UpdateNotNull() + if err != nil { + return err + } + } + return nil +} + func (repositoryImpl ChartRepositoryImpl) FindById(id int) (chart *Chart, err error) { chart = &Chart{} err = repositoryImpl.dbConnection.Model(chart). diff --git a/pkg/deployment/gitOps/git/GitOperationService.go b/pkg/deployment/gitOps/git/GitOperationService.go index bc5444faef..20d26fbac6 100644 --- a/pkg/deployment/gitOps/git/GitOperationService.go +++ b/pkg/deployment/gitOps/git/GitOperationService.go @@ -407,6 +407,9 @@ func (impl *GitOperationServiceImpl) UpdateGitHostUrlByProvider(request *apiBean } func buildGithubOrgUrl(host, orgId string) (orgUrl string, err error) { + if !strings.HasPrefix(host, HTTP_URL_PROTOCOL) && !strings.HasPrefix(host, HTTPS_URL_PROTOCOL) { + return orgUrl, fmt.Errorf("invalid host url '%s'", host) + } hostUrl, err := url.Parse(host) if err != nil { return "", err diff --git a/pkg/deployment/gitOps/git/GitOpsHelper.go b/pkg/deployment/gitOps/git/GitOpsHelper.go index 10f4b445c6..e929eb2ff5 100644 --- a/pkg/deployment/gitOps/git/GitOpsHelper.go +++ b/pkg/deployment/gitOps/git/GitOpsHelper.go @@ -20,7 +20,7 @@ package git import ( "context" "fmt" - bean2 "github.com/devtron-labs/devtron/api/bean/gitOps" + apiGitOpsBean "github.com/devtron-labs/devtron/api/bean/gitOps" git "github.com/devtron-labs/devtron/pkg/deployment/gitOps/git/commandManager" "github.com/devtron-labs/devtron/util" "os" @@ -161,7 +161,19 @@ func (impl *GitOpsHelper) getBranch(ctx git.GitContext, rootDir string) (string, return branch, nil } -func SanitiseCustomGitRepoURL(activeGitOpsConfig bean2.GitOpsConfigDto, gitRepoURL string) (sanitisedGitRepoURL string) { +/* +SanitiseCustomGitRepoURL +- It will sanitise the user given repository url based on GitOps provider + +Case BITBUCKET_PROVIDER: + - The clone URL format https://@bitbucket.org//.git + - Here the can differ from user to user. SanitiseCustomGitRepoURL will return the repo url in format : https://bitbucket.org//.git + +Case AZURE_DEVOPS_PROVIDER: + - The clone URL format https://@dev.azure.com///_git/ + - Here the can differ from user to user. SanitiseCustomGitRepoURL will return the repo url in format : https://dev.azure.com///_git/ +*/ +func SanitiseCustomGitRepoURL(activeGitOpsConfig apiGitOpsBean.GitOpsConfigDto, gitRepoURL string) (sanitisedGitRepoURL string) { sanitisedGitRepoURL = gitRepoURL if activeGitOpsConfig.Provider == BITBUCKET_PROVIDER && strings.Contains(gitRepoURL, fmt.Sprintf("://%s@%s", activeGitOpsConfig.Username, "bitbucket.org/")) { sanitisedGitRepoURL = strings.ReplaceAll(gitRepoURL, fmt.Sprintf("://%s@%s", activeGitOpsConfig.Username, "bitbucket.org/"), "://bitbucket.org/") diff --git a/pkg/deployment/gitOps/git/GitServiceBitbucket.go b/pkg/deployment/gitOps/git/GitServiceBitbucket.go index 92c90850ba..a25845b4fa 100644 --- a/pkg/deployment/gitOps/git/GitServiceBitbucket.go +++ b/pkg/deployment/gitOps/git/GitServiceBitbucket.go @@ -13,6 +13,8 @@ import ( ) const ( + HTTP_URL_PROTOCOL = "http://" + HTTPS_URL_PROTOCOL = "https://" BITBUCKET_CLONE_BASE_URL = "https://bitbucket.org/" BITBUCKET_GITOPS_DIR = "bitbucketGitOps" BITBUCKET_REPO_NOT_FOUND_ERROR = "404 Not Found" diff --git a/pkg/pipeline/AppDeploymentTypeChangeManager.go b/pkg/pipeline/AppDeploymentTypeChangeManager.go index 35f95e96db..2f4e6e0a54 100644 --- a/pkg/pipeline/AppDeploymentTypeChangeManager.go +++ b/pkg/pipeline/AppDeploymentTypeChangeManager.go @@ -462,7 +462,7 @@ func (impl *AppDeploymentTypeChangeManagerImpl) DeleteDeploymentApps(ctx context impl.logger.Errorw("error in registering acd app", "err", AcdRegisterErr) } if AcdRegisterErr == nil { - RepoURLUpdateErr = impl.chartService.ConfigureGitOpsRepoUrl(pipeline.AppId, chartGitAttr.RepoUrl, chartGitAttr.ChartLocation, userId) + RepoURLUpdateErr = impl.chartService.ConfigureGitOpsRepoUrl(pipeline.AppId, chartGitAttr.RepoUrl, chartGitAttr.ChartLocation, false, userId) if RepoURLUpdateErr != nil { impl.logger.Errorw("error in updating git repo url in charts", "err", RepoURLUpdateErr) } diff --git a/pkg/pipeline/DeploymentPipelineConfigService.go b/pkg/pipeline/DeploymentPipelineConfigService.go index 2c17812925..6579224ff6 100644 --- a/pkg/pipeline/DeploymentPipelineConfigService.go +++ b/pkg/pipeline/DeploymentPipelineConfigService.go @@ -421,7 +421,7 @@ func (impl *CdPipelineConfigServiceImpl) CreateCdPipelines(pipelineCreateRequest return nil, err } // below function will update gitRepoUrl for charts if user has not already provided gitOps repoURL - err = impl.chartService.ConfigureGitOpsRepoUrl(pipelineCreateRequest.AppId, chartGitAttr.RepoUrl, chartGitAttr.ChartLocation, pipelineCreateRequest.UserId) + err = impl.chartService.ConfigureGitOpsRepoUrl(pipelineCreateRequest.AppId, chartGitAttr.RepoUrl, chartGitAttr.ChartLocation, false, pipelineCreateRequest.UserId) if err != nil { impl.logger.Errorw("error in updating git repo url in charts", "err", err) return nil, err @@ -1641,7 +1641,7 @@ func (impl *CdPipelineConfigServiceImpl) ValidateCDPipelineRequest(pipelineCreat } func (impl *CdPipelineConfigServiceImpl) RegisterInACD(ctx context.Context, chartGitAttr *commonBean.ChartGitAttribute, userId int32) error { - err := impl.argoClientWrapperService.RegisterGitOpsRepoInArgo(ctx, chartGitAttr.RepoUrl, userId) + err := impl.argoClientWrapperService.RegisterGitOpsRepoInArgoWithRetry(ctx, chartGitAttr.RepoUrl, userId) if err != nil { impl.logger.Errorw("error while register git repo in argo", "err", err) return err @@ -1973,27 +1973,6 @@ func (impl *CdPipelineConfigServiceImpl) handleDigestPolicyOperations(tx *pg.Tx, return resourceQualifierId, nil } -// TODO Asutosh: -func (impl *CdPipelineConfigServiceImpl) updateGitRepoUrlInCharts(appId int, chartGitAttribute *commonBean.ChartGitAttribute, userId int32) error { - charts, err := impl.chartRepository.FindActiveChartsByAppId(appId) - if err != nil && pg.ErrNoRows != err { - return err - } - for _, ch := range charts { - if len(ch.GitRepoUrl) == 0 { - ch.GitRepoUrl = chartGitAttribute.RepoUrl - ch.ChartLocation = chartGitAttribute.ChartLocation - ch.UpdatedOn = time.Now() - ch.UpdatedBy = userId - err = impl.chartRepository.Update(ch) - if err != nil { - return err - } - } - } - return nil -} - func (impl *CdPipelineConfigServiceImpl) DeleteCdPipelinePartial(pipeline *pipelineConfig.Pipeline, ctx context.Context, deleteAction int, userId int32) (*bean.AppDeleteResponseDTO, error) { cascadeDelete := true forceDelete := false diff --git a/util/retryFunc/RetryFunction.go b/util/retryFunc/RetryFunction.go new file mode 100644 index 0000000000..76dfeeab1f --- /dev/null +++ b/util/retryFunc/RetryFunction.go @@ -0,0 +1,24 @@ +package retryFunc + +import ( + "fmt" + "go.uber.org/zap" + "time" +) + +// Retry performs a function with retries, delay, and a max number of attempts. +func Retry(fn func() error, shouldRetry func(err error) bool, maxRetries int, delay time.Duration, logger *zap.SugaredLogger) error { + var err error + for i := 0; i < maxRetries; i++ { + err = fn() + if err == nil { + return nil + } + if !shouldRetry(err) { + return err + } + logger.Infow(fmt.Sprintf("Attempt %d failed, retrying in %v", i+1, delay), "err", err) + time.Sleep(delay) + } + return fmt.Errorf("after %d attempts, last error: %s", maxRetries, err) +} diff --git a/wire_gen.go b/wire_gen.go index bb7da8e491..003b7e9a5e 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -577,7 +577,7 @@ func InitializeApp() (*App, error) { return nil, err } gitOpsValidationServiceImpl := validation.NewGitOpsValidationServiceImpl(sugaredLogger, gitFactory, gitOperationServiceImpl, gitOpsConfigReadServiceImpl, chartTemplateServiceImpl, chartServiceImpl, installedAppDBExtendedServiceImpl) - devtronAppGitOpConfigServiceImpl := gitOpsConfig.NewDevtronAppGitOpConfigServiceImpl(sugaredLogger, chartRepositoryImpl, gitOpsConfigReadServiceImpl, gitOpsValidationServiceImpl, argoClientWrapperServiceImpl) + devtronAppGitOpConfigServiceImpl := gitOpsConfig.NewDevtronAppGitOpConfigServiceImpl(sugaredLogger, chartRepositoryImpl, chartServiceImpl, gitOpsConfigReadServiceImpl, gitOpsValidationServiceImpl, argoClientWrapperServiceImpl) cdHandlerImpl := pipeline.NewCdHandlerImpl(sugaredLogger, userServiceImpl, cdWorkflowRepositoryImpl, ciLogServiceImpl, ciArtifactRepositoryImpl, ciPipelineMaterialRepositoryImpl, pipelineRepositoryImpl, environmentRepositoryImpl, ciWorkflowRepositoryImpl, enforcerUtilImpl, resourceGroupServiceImpl, imageTaggingServiceImpl, k8sServiceImpl, workflowServiceImpl, clusterServiceImplExtended, blobStorageConfigServiceImpl, customTagServiceImpl) appWorkflowServiceImpl := appWorkflow2.NewAppWorkflowServiceImpl(sugaredLogger, appWorkflowRepositoryImpl, ciCdPipelineOrchestratorImpl, ciPipelineRepositoryImpl, pipelineRepositoryImpl, enforcerUtilImpl, resourceGroupServiceImpl, appRepositoryImpl, userAuthServiceImpl, chartServiceImpl) appListingViewBuilderImpl := app2.NewAppListingViewBuilderImpl(sugaredLogger)