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
- This way we don't need to set this nullify logic on a Rails
after_destroyhook which is agains our guidelines. - 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.
- When creating the migration we should first guarantee that we have no
ci_buildswithrunner_idsfor 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. - This query to find all
ci_buildswith deleted runners might be heavy. - We must guarantee that no runner is deleted while the migration to clean the
runner_idand create the foreign_key finishes. Otherwise, we need to mitigate this possibility by doing a first deployment with the railsdependent: :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