Skip to content

Make status transition resilient for downstream pipeline creation

Fabio Pitino requested to merge fix-pipeline-creation-race-conditions into master

What does this MR do?

Related to #207234 (closed) (post-regression issue)

Related to #198354 (closed) (original issue)

Related to #202239 (closed) (related issue)

In this MR we are reintroducing the changes that were originally reverted but this time changing the strategy a little bit. The previous changes had caused a regression on production where race conditions in pipeline status update caused state machine errors to be added to the created downstream pipeline. Because of that we dropped the bridge thinking that they were "user errors" in the downstream pipeline, rather than "internal errors".

The concurrent bridge status update also made the bridge job to be a stale object, so when trying to drop the bridge it was raising ActiveRecord::StaleObjectError. This caused the CreateCrossProjectPipelineWorker to retry automatically, creating duplicate downstream pipelines.

This MR uses a different approach:

  • Make sure that CreateCrossProjectPipelineWorker doesn't create a duplicate pipeline when the sidekiq worker is retried.
  • Make sure that we don't raise an error in state_machine transition, that could disturb entire pipeline/builds status transition.
  • Make sure that we have a centralized place to update the bridge status.

Feature flag

I've decided to make this change behind a feature flag: ci_drop_bridge_on_downstream_errors

Feature flag is going to be removed in #207948 (closed)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Fabio Pitino

Merge request reports