Skip to content

Make sure head pippeline always corresponds with an MR

Jarka Košanová requested to merge 37354-pipelines-update into master

What does this MR do?

It does not update the head_pipeline of merge requests directly from Ci::CreatePipelineService but enqueues the update instead.

It also overrides head_pipeline - in case last pipeline sha equals head_diff_sha it returns the pipeline. In case the shas don't equal it returns nil.

Are there points in the code the reviewer needs to double check?

When CI is enabled for a project and pipeline is nil an error Could not connect to the CI server. Please check your settings and try again. Not sure this error is correct. Maybe we should return Pipeline blocked. The pipeline for this merge request requires a manual action to proceed instead.

That could be done in mr_widget_store.js changing this.isPipelineBlocked = pipelineStatus ? pipelineStatus.group === 'manual' : false;

to

this.isPipelineBlocked = (pipelineStatus ? pipelineStatus.group === 'manual' : false) || (data.has_ci && !data.pipeline);

Why was this MR needed?

As described in the issue it can happen that the head pipeline of a merge request is set to the pipeline that is actually not the pipeline that should be associated to the merge request.

This can happen due to the following workflow:

  1. a user pushes a change
  2. GitPushService is called - from this service
  • UpdateMergeRequestsWorker job is enqueued
  • Ci::CreatePipelineService is executed
  1. inside Ci::CreatePipelineService
  • a new pipeline is created
  • head_pipeline_id of all merge requests that have the same source_project and source_branch as the created pipeline are updated to the id of the created pipeline
  1. head_pipeline can be updated before a merge request is refreshed from UpdateMergeRequestsWorker -> the merge request still references an old sha but associated pipeline already a new sha

The second possible way to get into this state is when a pipeline has to be started always manually. (the old pipeline is still as head pipeline when no one has started the new one yet)

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #37354 (closed)

Edited by Jarka Košanová

Merge request reports