Make sure head pippeline always corresponds with an MR
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:
- a user pushes a change
-
GitPushService
is called - from this service
-
UpdateMergeRequestsWorker
job is enqueued -
Ci::CreatePipelineService
is executed
- 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
-
head_pipeline
can be updated before a merge request is refreshed fromUpdateMergeRequestsWorker
-> 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?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added -
Tests added for this feature/bug - Review
-
Has been reviewed by UX -
Has been reviewed by Frontend -
Has been reviewed by Backend -
Has been reviewed by Database
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Squashed related commits together
What are the relevant issue numbers?
Closes #37354 (closed)