Skip to content

Backend: Add missing foreign key constraint between `Ci::Build` and `Ci::Runner`

Summary

This is a follow-up issue originated from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30176#note_189771942

A foreign key with on_delete: :nullify dependency must be added between ci_builds and ci_runners table for the runner_id column.

Improvements

Benefits

  1. This way we don't need to set this nullify logic on a Rails after_destroy hook which is agains our guidelines.
  2. We currently don't nullify this column, which is error prone since it indicates that a build is associated to a runner which might already have been deleted.

Risks

We should check if we currently have runner_ids with already deleted runners.

  1. When creating the migration we should first guarantee that we have no ci_builds with runner_ids for runners that were already deleted. Otherwise the creation of the foreign key could fail. So I believe will need to run some kind of cleaner job to nullify this ids.
  2. This query to find all ci_builds with deleted runners might be heavy.
  3. We must guarantee that no runner is deleted while the migration to clean the runner_id and create the foreign_key finishes. Otherwise, we need to mitigate this possibility by doing a first deployment with the rails dependent: :nullify, to try to guarantee that if a runner gets deleted, it won't leave a old runner_id in the build's column.

Involved components

It's a database refactoring.

Optional: Intended side effects

Optional: Missing test coverage

We should create a migration spec

Edited by Mark Nuzzo