Skip to content

Commit fd18069

Browse files
prakash100198prakash100198
committed
code review incorporate 1
1 parent 58ccedf commit fd18069

File tree

10 files changed

+142
-186
lines changed

10 files changed

+142
-186
lines changed

‎pkg/build/trigger/HandlerService.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ func (impl *HandlerServiceImpl) fetchVariableSnapshotForCiRetrigger(trigger *typ
671671
return prePostAndRefPluginResponse.VariableSnapshot, nil
672672
}
673673

674-
func (impl *HandlerServiceImpl) triggerCiPipeline(trigger *types.CiTriggerRequest) (int, error) {
674+
func (impl *HandlerServiceImpl) prepareCiWfRequest(trigger *types.CiTriggerRequest) (map[string]string, *pipelineConfig.CiWorkflow, *types.WorkflowRequest, error) {
675675
var variableSnapshot map[string]string
676676
var savedCiWf *pipelineConfig.CiWorkflow
677677
var workflowRequest *types.WorkflowRequest
@@ -680,7 +680,7 @@ func (impl *HandlerServiceImpl) triggerCiPipeline(trigger *types.CiTriggerReques
680680
variableSnapshot, err = impl.fetchVariableSnapshotForCiRetrigger(trigger)
681681
if err != nil {
682682
impl.Logger.Errorw("error in fetchVariableSnapshotForCiRetrigger", "triggerRequest", trigger, "err", err)
683-
return 0, err
683+
return nil, nil, nil, err
684684
}
685685
savedCiWf, workflowRequest = trigger.RetriggerCiWorkflow, trigger.RetriggerWorkflowRequest
686686
if trigger.RetriggerCiWorkflow != nil {
@@ -691,10 +691,19 @@ func (impl *HandlerServiceImpl) triggerCiPipeline(trigger *types.CiTriggerReques
691691
variableSnapshot, savedCiWf, workflowRequest, err = impl.StartCiWorkflowAndPrepareWfRequest(trigger)
692692
if err != nil {
693693
impl.Logger.Errorw("error in starting ci workflow and preparing wf request", "triggerRequest", trigger, "err", err)
694-
return 0, err
694+
return nil, nil, nil, err
695695
}
696696
workflowRequest.CiPipelineType = trigger.PipelineType
697697
}
698+
return variableSnapshot, savedCiWf, workflowRequest, nil
699+
}
700+
701+
func (impl *HandlerServiceImpl) triggerCiPipeline(trigger *types.CiTriggerRequest) (int, error) {
702+
variableSnapshot, savedCiWf, workflowRequest, err := impl.prepareCiWfRequest(trigger)
703+
if err != nil {
704+
impl.Logger.Errorw("error in preparing wf request", "triggerRequest", trigger, "err", err)
705+
return 0, err
706+
}
698707

699708
err = impl.executeCiPipeline(workflowRequest)
700709
if err != nil {
@@ -744,11 +753,13 @@ func (impl *HandlerServiceImpl) StartCiWorkflowAndPrepareWfRequest(trigger *type
744753
impl.Logger.Debugw("ci pipeline manual trigger", "request", trigger)
745754
ciMaterials, err := impl.GetCiMaterials(trigger.PipelineId, trigger.CiMaterials)
746755
if err != nil {
756+
impl.Logger.Errorw("error in getting ci materials", "pipelineId", trigger.PipelineId, "ciMaterials", trigger.CiMaterials, "err", err)
747757
return nil, nil, nil, err
748758
}
749759

750760
ciPipelineScripts, err := impl.ciPipelineRepository.FindCiScriptsByCiPipelineId(trigger.PipelineId)
751761
if err != nil && !util.IsErrNoRows(err) {
762+
impl.Logger.Errorw("error in getting ci script by pipeline id", "pipelineId", trigger.PipelineId, "err", err)
752763
return nil, nil, nil, err
753764
}
754765

@@ -764,12 +775,14 @@ func (impl *HandlerServiceImpl) StartCiWorkflowAndPrepareWfRequest(trigger *type
764775
ciWorkflowConfigNamespace := impl.config.GetDefaultNamespace()
765776
envModal, isJob, err := impl.getEnvironmentForJob(pipeline, trigger)
766777
if err != nil {
778+
impl.Logger.Errorw("error in getting environment for job", "pipelineId", trigger.PipelineId, "err", err)
767779
return nil, nil, nil, err
768780
}
769781
if isJob && envModal != nil {
770782

771783
err = impl.checkArgoSetupRequirement(envModal)
772784
if err != nil {
785+
impl.Logger.Errorw("error in checking argo setup requirement", "envModal", envModal, "err", err)
773786
return nil, nil, nil, err
774787
}
775788

@@ -870,6 +883,7 @@ func (impl *HandlerServiceImpl) StartCiWorkflowAndPrepareWfRequest(trigger *type
870883
err = impl.updateCiWorkflow(workflowRequest, savedCiWf)
871884
appLabels, err := impl.appCrudOperationService.GetLabelsByAppId(pipeline.AppId)
872885
if err != nil {
886+
impl.Logger.Errorw("error in getting labels by appId", "appId", pipeline.AppId, "err", err)
873887
return nil, nil, nil, err
874888
}
875889
workflowRequest.AppLabels = appLabels

‎pkg/deployment/trigger/devtronApps/postStageHandlerCode.go

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ func (impl *HandlerServiceImpl) TriggerPostStage(request bean.CdTriggerRequest)
3535
triggeredAt := time.Now()
3636
triggeredBy := request.TriggeredBy
3737
pipeline := request.Pipeline
38-
artifact := request.Artifact
3938
cdWf := request.CdWf
4039
ctx := context.Background() //before there was only one context. To check why here we are not using ctx from request.TriggerContext
4140
env, namespace, err := impl.getEnvAndNsIfRunStageInEnv(ctx, request)
@@ -104,23 +103,9 @@ func (impl *HandlerServiceImpl) TriggerPostStage(request bean.CdTriggerRequest)
104103
impl.logger.Errorw("error, checkVulnerabilityStatusAndFailWfIfNeeded", "err", err, "runner", runner)
105104
return nil, err
106105
}
107-
108-
var cdStageWorkflowRequest *types.WorkflowRequest
109-
if request.IsRetrigger {
110-
// Retrieve workflow request from snapshot
111-
workflowRequest, err := impl.workflowTriggerAuditService.GetWorkflowRequestFromSnapshotForRetrigger(runner.Id, types.POST_CD_WORKFLOW_TYPE)
112-
if err != nil {
113-
impl.logger.Errorw("error retrieving workflow request from snapshot for post cd workflow type", "cdWorkflowId", runner.CdWorkflow.Id, "err", err)
114-
return nil, err
115-
}
116-
cdStageWorkflowRequest = workflowRequest
117-
// Update dynamic fields in the workflow request for retrigger
118-
impl.updateWorkflowRequestForCdRetrigger(cdStageWorkflowRequest, runner)
119-
} else {
120-
cdStageWorkflowRequest, err = impl.getPrePostCdStageWorkflowRequest(ctx, runner, cdWf, pipeline, env, artifact, types.POST, envDevploymentConfig, triggeredBy)
121-
if err != nil {
122-
return impl.buildWfRequestErrorHandler(runner, err, triggeredBy)
123-
}
106+
cdStageWorkflowRequest, err := impl.preparePrePostCdWorkflowRequest(ctx, runner, cdWf, request, env, types.POST, envDevploymentConfig)
107+
if err != nil {
108+
return impl.buildWfRequestErrorHandler(runner, err, triggeredBy)
124109
}
125110
_, jobHelmPackagePath, err := impl.workflowService.SubmitWorkflow(cdStageWorkflowRequest)
126111
if err != nil {

‎pkg/deployment/trigger/devtronApps/preStageHandlerCode.go

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,34 @@ import (
6161
"time"
6262
)
6363

64+
func (impl *HandlerServiceImpl) preparePrePostCdWorkflowRequest(ctx context.Context, runner *pipelineConfig.CdWorkflowRunner, cdWf *pipelineConfig.CdWorkflow,
65+
request bean.CdTriggerRequest, env *repository4.Environment, stageType string, envDeploymentConfig *bean5.DeploymentConfig) (*types.WorkflowRequest, error) {
66+
var cdStageWorkflowRequest *types.WorkflowRequest
67+
var err error
68+
if request.IsRetrigger {
69+
// retrieve workflow request from snapshot
70+
workflowType := types.PRE_CD_WORKFLOW_TYPE
71+
if stageType == types.POST {
72+
workflowType = types.POST_CD_WORKFLOW_TYPE
73+
}
74+
workflowRequest, err := impl.workflowTriggerAuditService.GetWorkflowRequestFromSnapshotForRetrigger(runner.Id, workflowType)
75+
if err != nil {
76+
impl.logger.Errorw("error retrieving workflow request from snapshot for pre/post cd stage type", "workflowType", workflowType, "cdWorkflowId", runner.CdWorkflow.Id, "err", err)
77+
return nil, err
78+
}
79+
cdStageWorkflowRequest = workflowRequest
80+
// Update dynamic fields in the workflow request for retrigger
81+
impl.updateWorkflowRequestForCdRetrigger(cdStageWorkflowRequest, runner)
82+
} else {
83+
cdStageWorkflowRequest, err = impl.getPrePostCdStageWorkflowRequest(ctx, runner, cdWf, request.Pipeline, env, request.Artifact, stageType, envDeploymentConfig, request.TriggeredBy)
84+
if err != nil {
85+
impl.logger.Errorw("error retrieving workflow request from snapshot for pre/post cd stage type", "stageType", stageType, "cdWorkflowId", runner.CdWorkflow.Id, "err", err)
86+
return nil, err
87+
}
88+
}
89+
return cdStageWorkflowRequest, nil
90+
}
91+
6492
func (impl *HandlerServiceImpl) TriggerPreStage(request bean.CdTriggerRequest) (*bean6.ManifestPushTemplate, error) {
6593
request.WorkflowType = apiBean.CD_WORKFLOW_TYPE_PRE
6694
// setting triggeredAt variable to have consistent data for various audit log places in db for deployment time
@@ -123,23 +151,9 @@ func (impl *HandlerServiceImpl) TriggerPreStage(request bean.CdTriggerRequest) (
123151
impl.logger.Errorw("error, checkVulnerabilityStatusAndFailWfIfNeeded", "err", err, "runner", runner)
124152
return nil, err
125153
}
126-
127-
var cdStageWorkflowRequest *types.WorkflowRequest
128-
if request.IsRetrigger {
129-
// Retrieve workflow request from snapshot
130-
workflowRequest, err := impl.workflowTriggerAuditService.GetWorkflowRequestFromSnapshotForRetrigger(runner.Id, types.PRE_CD_WORKFLOW_TYPE)
131-
if err != nil {
132-
impl.logger.Errorw("error retrieving workflow request from snapshot for pre cd stage type", "cdWorkflowId", runner.CdWorkflow.Id, "err", err)
133-
return nil, err
134-
}
135-
cdStageWorkflowRequest = workflowRequest
136-
// Update dynamic fields in the workflow request for retrigger
137-
impl.updateWorkflowRequestForCdRetrigger(cdStageWorkflowRequest, runner)
138-
} else {
139-
cdStageWorkflowRequest, err = impl.getPrePostCdStageWorkflowRequest(ctx, runner, cdWf, pipeline, env, artifact, types.PRE, envDeploymentConfig, triggeredBy)
140-
if err != nil {
141-
return impl.buildWfRequestErrorHandler(runner, err, triggeredBy)
142-
}
154+
cdStageWorkflowRequest, err := impl.preparePrePostCdWorkflowRequest(ctx, runner, cdWf, request, env, types.PRE, envDeploymentConfig)
155+
if err != nil {
156+
return impl.buildWfRequestErrorHandler(runner, err, triggeredBy)
143157
}
144158
_, span := otel.Tracer("orchestrator").Start(ctx, "cdWorkflowService.SubmitWorkflow")
145159
_, jobHelmPackagePath, err := impl.workflowService.SubmitWorkflow(cdStageWorkflowRequest)

‎pkg/executor/WorkflowService.go

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (impl *WorkflowServiceImpl) createWorkflowTemplateAndAuditTrigger(workflowR
167167
}
168168
err = impl.auditTrigger(workflowRequest)
169169
if err != nil {
170-
impl.Logger.Errorw("error occurred while auditing trigger", "err", err)
170+
impl.Logger.Errorw("error occurred while auditing trigger", "workflowRunnerId", workflowRequest.WorkflowRunnerId, "ciWorkflowId", workflowRequest.WorkflowId, "err", err)
171171
return bean3.WorkflowTemplate{}, err
172172
}
173173
workflowTemplate := workflowRequest.GetWorkflowTemplate(workflowJson, impl.ciCdConfig)
@@ -263,9 +263,9 @@ func (impl *WorkflowServiceImpl) createWorkflowTemplateAndAuditTrigger(workflowR
263263

264264
if workflowRequest.IsCiTypeWorkflowRequest() && workflowRequest.IsCiRetriggerType() {
265265
// here we need to update the workflow template with cpu request and limit, memory limit and request and Build timeout (in oss this is applicable on all ci builds i.e. applied globally)
266-
err = impl.updateWorkflowTemplateWithInfraConfigFromHistory(workflowRequest, &workflowTemplate)
266+
err = impl.populateReTriggerWorkflowTemplateWithInfraConfig(workflowRequest, &workflowTemplate)
267267
if err != nil {
268-
impl.Logger.Errorw("error occurred while updating workflow template with infra config from history", "err", err)
268+
impl.Logger.Errorw("error occurred while updating workflow template with infra config from history", "ciWorkflowId", workflowRequest.WorkflowId, "err", err)
269269
return bean3.WorkflowTemplate{}, err
270270
}
271271

@@ -524,9 +524,9 @@ func (impl *WorkflowServiceImpl) getWfClient(environment *repository2.Environmen
524524
return wfClient, nil
525525
}
526526

527-
// updateWorkflowTemplateWithInfraConfigFromHistory updates the workflow template with CPU, memory limits/requests and timeout
527+
// populateReTriggerWorkflowTemplateWithInfraConfig updates the workflow template with CPU, memory limits/requests and timeout
528528
// from the infra_config_trigger_history table based on previous workflow ID.
529-
func (impl *WorkflowServiceImpl) updateWorkflowTemplateWithInfraConfigFromHistory(workflowRequest *types.WorkflowRequest, workflowTemplate *bean3.WorkflowTemplate) error {
529+
func (impl *WorkflowServiceImpl) populateReTriggerWorkflowTemplateWithInfraConfig(workflowRequest *types.WorkflowRequest, workflowTemplate *bean3.WorkflowTemplate) error {
530530
// Skip if no previous workflow ID is available or if this is not a CI/Job workflow
531531
if workflowRequest.ReferenceCiWorkflowId == 0 {
532532
impl.Logger.Debugw("skipping infra config history update", "referenceWorkflowId", workflowRequest.ReferenceCiWorkflowId, "workflowType", workflowRequest.Type)
@@ -536,7 +536,7 @@ func (impl *WorkflowServiceImpl) updateWorkflowTemplateWithInfraConfigFromHistor
536536
// Get infra config from history based on previous workflow ID
537537
historicalInfraConfig, err := impl.infraConfigAuditService.GetInfraConfigByWorkflowId(workflowRequest.ReferenceCiWorkflowId, bean.CI_WORKFLOW_TYPE.String())
538538
if err != nil {
539-
impl.Logger.Warnw("could not retrieve infra config from history, using current config", "referenceWorkflowId", workflowRequest.ReferenceCiWorkflowId, "err", err)
539+
impl.Logger.Errorw("could not retrieve infra config from history, using current config", "referenceWorkflowId", workflowRequest.ReferenceCiWorkflowId, "err", err)
540540
return nil // Don't fail the workflow, just use current config
541541
}
542542

@@ -545,17 +545,21 @@ func (impl *WorkflowServiceImpl) updateWorkflowTemplateWithInfraConfigFromHistor
545545
return nil
546546
}
547547

548-
impl.Logger.Infow("applying historical infra config to workflow template", "referenceWorkflowId", workflowRequest.ReferenceCiWorkflowId, "historicalConfig", historicalInfraConfig)
548+
impl.Logger.Infow("applying historical infra config to workflow template", "referenceWorkflowId", workflowRequest.ReferenceCiWorkflowId)
549549

550550
// apply historical infra configurations to a workflow template
551-
impl.applyInfraConfigToWorkflowTemplate(workflowRequest, workflowTemplate, historicalInfraConfig)
551+
err = impl.applyInfraConfigToWorkflowTemplate(workflowRequest, workflowTemplate, historicalInfraConfig)
552+
if err != nil {
553+
impl.Logger.Errorw("error in applying infra config to workflow template", "referenceWorkflowId", workflowRequest.ReferenceCiWorkflowId, "err", err)
554+
return err
555+
}
552556

553557
return nil
554558
}
555559

556560
// applyInfraConfigToWorkflowTemplate applies the historical infra configuration to the workflow template.
557561
// This function handles the core OSS functionality and can be extended in enterprise for additional fields.
558-
func (impl *WorkflowServiceImpl) applyInfraConfigToWorkflowTemplate(workflowRequest *types.WorkflowRequest, workflowTemplate *bean3.WorkflowTemplate, infraConfig *v1.InfraConfig) {
562+
func (impl *WorkflowServiceImpl) applyInfraConfigToWorkflowTemplate(workflowRequest *types.WorkflowRequest, workflowTemplate *bean3.WorkflowTemplate, infraConfig *v1.InfraConfig) error {
559563
// Apply timeout configuration
560564
if infraConfig.GetCiDefaultTimeout() > 0 {
561565
timeout := infraConfig.GetCiTimeoutInt()
@@ -581,15 +585,17 @@ func (impl *WorkflowServiceImpl) applyInfraConfigToWorkflowTemplate(workflowRequ
581585
container.Resources.Limits[v12.ResourceCPU] = cpuLimit
582586
impl.Logger.Debugw("applied historical CPU limit to workflow template", "cpuLimit", infraConfig.GetCiLimitCpu(), "workflowId", workflowRequest.WorkflowId)
583587
} else {
584-
impl.Logger.Warnw("failed to parse CPU limit from historical config", "cpuLimit", infraConfig.GetCiLimitCpu(), "err", err)
588+
impl.Logger.Errorw("failed to parse CPU limit from historical config", "cpuLimit", infraConfig.GetCiLimitCpu(), "err", err)
589+
return err
585590
}
586591
}
587592
if infraConfig.GetCiReqCpu() != "" {
588593
if cpuRequest, err := resource.ParseQuantity(infraConfig.GetCiReqCpu()); err == nil {
589594
container.Resources.Requests[v12.ResourceCPU] = cpuRequest
590595
impl.Logger.Debugw("applied historical CPU request to workflow template", "cpuRequest", infraConfig.GetCiReqCpu(), "workflowId", workflowRequest.WorkflowId)
591596
} else {
592-
impl.Logger.Warnw("failed to parse CPU request from historical config", "cpuRequest", infraConfig.GetCiReqCpu(), "err", err)
597+
impl.Logger.Errorw("failed to parse CPU request from historical config", "cpuRequest", infraConfig.GetCiReqCpu(), "err", err)
598+
return err
593599
}
594600
}
595601

@@ -599,23 +605,26 @@ func (impl *WorkflowServiceImpl) applyInfraConfigToWorkflowTemplate(workflowRequ
599605
container.Resources.Limits[v12.ResourceMemory] = memoryLimit
600606
impl.Logger.Debugw("applied historical memory limit to workflow template", "memoryLimit", infraConfig.GetCiLimitMem(), "workflowId", workflowRequest.WorkflowId)
601607
} else {
602-
impl.Logger.Warnw("failed to parse memory limit from historical config", "memoryLimit", infraConfig.GetCiLimitMem(), "err", err)
608+
impl.Logger.Errorw("failed to parse memory limit from historical config", "memoryLimit", infraConfig.GetCiLimitMem(), "err", err)
609+
return err
603610
}
604611
}
605612
if infraConfig.GetCiReqMem() != "" {
606613
if memoryRequest, err := resource.ParseQuantity(infraConfig.GetCiReqMem()); err == nil {
607614
container.Resources.Requests[v12.ResourceMemory] = memoryRequest
608615
impl.Logger.Debugw("applied historical memory request to workflow template", "memoryRequest", infraConfig.GetCiReqMem(), "workflowId", workflowRequest.WorkflowId)
609616
} else {
610-
impl.Logger.Warnw("failed to parse memory request from historical config", "memoryRequest", infraConfig.GetCiReqMem(), "err", err)
617+
impl.Logger.Errorw("failed to parse memory request from historical config", "memoryRequest", infraConfig.GetCiReqMem(), "err", err)
618+
return err
611619
}
612620
}
613621
}
614622

615-
impl.applyEnterpriseInfraConfigToWorkflowTemplate(workflowRequest, workflowTemplate, infraConfig)
623+
return impl.applyEnterpriseInfraConfigToWorkflowTemplate(workflowRequest, workflowTemplate, infraConfig)
616624
}
617625

618626
// applyEnterpriseInfraConfigToWorkflowTemplate is a placeholder for enterprise-specific infra config application.
619-
func (impl *WorkflowServiceImpl) applyEnterpriseInfraConfigToWorkflowTemplate(workflowRequest *types.WorkflowRequest, workflowTemplate *bean3.WorkflowTemplate, infraConfig *v1.InfraConfig) {
627+
func (impl *WorkflowServiceImpl) applyEnterpriseInfraConfigToWorkflowTemplate(workflowRequest *types.WorkflowRequest, workflowTemplate *bean3.WorkflowTemplate, infraConfig *v1.InfraConfig) error {
620628
impl.Logger.Debugw("enterprise infra config application (no-op in OSS)", "workflowId", workflowRequest.WorkflowId)
629+
return nil
621630
}

‎pkg/executor/adapter/auditAdapter/adapter.go

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎pkg/infraConfig/repository/audit/infraConfigAuditRepository.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,5 @@ func (impl *InfraConfigAuditRepositoryImpl) GetInfraConfigHistoryByWorkflowId(wo
9191
Where("workflow_id = ?", workflowId).
9292
Where("workflow_type = ?", workflowType).
9393
Select()
94-
if err != nil {
95-
return nil, err
96-
}
97-
return infraConfigHistories, nil
94+
return infraConfigHistories, err
9895
}

‎pkg/infraConfig/service/audit/infraConfigAudit.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,15 @@ func (impl *InfraConfigAuditServiceImpl) GetInfraConfigByWorkflowId(workflowId i
8181
impl.logger.Errorw("failed to get infra config history by workflow id", "error", err, "workflowId", workflowId, "workflowType", workflowType)
8282
return nil, err
8383
}
84+
infraConfig, err := impl.fetchInfraConfigFromHistory(infraConfigHistories)
85+
if err != nil {
86+
impl.logger.Errorw("failed to fetch infra config from history", "infraConfigHistories", infraConfigHistories, "err", err)
87+
return nil, err
88+
}
89+
return infraConfig, nil
90+
}
8491

85-
// Create InfraConfig from history records
92+
func (impl *InfraConfigAuditServiceImpl) fetchInfraConfigFromHistory(infraConfigHistories []*audit.InfraConfigTriggerHistory) (*infraBean.InfraConfig, error) {
8693
infraConfig := &infraBean.InfraConfig{}
8794
for _, history := range infraConfigHistories {
8895
switch history.Key {
@@ -99,7 +106,8 @@ func (impl *InfraConfigAuditServiceImpl) GetInfraConfigByWorkflowId(workflowId i
99106
if timeout, parseErr := strconv.ParseFloat(history.ValueString, 64); parseErr == nil {
100107
infraConfig.CiDefaultTimeout = timeout
101108
} else {
102-
impl.logger.Warnw("failed to parse timeout value", "valueString", history.ValueString, "parseErr", parseErr)
109+
impl.logger.Errorw("failed to parse timeout value", "valueString", history.ValueString, "parseErr", parseErr)
110+
return nil, parseErr
103111
}
104112
}
105113
}

0 commit comments

Comments
 (0)