Skip to content

Database testing did not catch PDM with bug due to code changes following verification

Summary

A PDM containing a bug was merged into the default branch in !205554 (merged). (See Post-deploy migration SetAllowImmediateNamespac... (#572443 - closed) for more information about the bug.) During the MR, the database testing job ran and marked the migration as "valid". However, the migration file was later updated within the same MR, and the testing job never ran again, preventing us from catching the bug before the MR was merged.

The MR contained only one commit which was repeatedly updated. We can compare the two commits where the verification ran and the one that was merged and see that they were different:

  • Verification comment: !205554 (comment 2760861171)
  • Commit when the migrations ran: 1be01044
  • Commit that the MR was merged at: 4e8ec857
  • Migration file: db/post_migrate/20250910144106_set_allow_immediate_namespaces_deletion_to_false_on_saas.rb

We can see the diff between what was verified and what was eventually merged: (This change of the column from a normal column to a jsonb_accessor column seems to be the bug.)

$ diff <(g show 1be010447bfb56cda3d9c00976c4b8c9ccc72515:db/post_migrate/20250910144106_set_allow_immediate_namespaces_deletion_to_false_on_saas.rb) <(g show 4e8ec857ab9733920034c456795d19959ad42399:db/post_migrate/20250922144106_set_allow_immediate_namespaces_deletion_to_false_on_saas.rb)
7c7
<   class MigrationApplicationSettings < MigrationRecord
---
>   class MigrationApplicationSetting < MigrationRecord
8a9,11
>
>     jsonb_accessor :namespace_deletion_settings,
>       allow_immediate_namespaces_deletion: [:boolean, { default: true }]
12c15
<     return unless should_run?(MigrationApplicationSettings.last)
---
>     return unless should_run?
15c18
<     MigrationApplicationSettings.update_all(allow_immediate_namespaces_deletion: false)
---
>     MigrationApplicationSetting.update_all(allow_immediate_namespaces_deletion: false)
19c22
<     return unless should_run?(MigrationApplicationSettings.last)
---
>     return unless should_run?
22c25
<     MigrationApplicationSettings.update_all(allow_immediate_namespaces_deletion: true)
---
>     MigrationApplicationSetting.update_all(allow_immediate_namespaces_deletion: true)
27,28c30,31
<   def should_run?(application_setting)
<     Gitlab.com? || application_setting&.gitlab_dedicated_instance
---
>   def should_run?
>     Gitlab.com? || MigrationApplicationSetting.last&.gitlab_dedicated_instance

Exit criteria

  • Database testing should verify the migration that is about to be merged
  • Database testing should re-verify if the migration that was verified changes
Edited by 🤖 GitLab Bot 🤖