From df150bae0a40ed353d5a0cb740d7930a3bdd8bf8 Mon Sep 17 00:00:00 2001
From: Doug Stull <dstull@gitlab.com>
Date: Fri, 9 Feb 2024 21:48:26 +0000
Subject: [PATCH] Re-enqueue backfilling of the default organization owners
- bad sequencing of kicking this off before feature flag
was fully enabled
Changelog: other
---
.../backfill_default_organization_owners.yml | 4 +-
...fill_default_organization_owners_again.yml | 9 +++
...ue_backfill_default_organization_owners.rb | 19 +-----
...ue_backfill_default_organization_owners.rb | 33 ++++++++++
db/schema_migrations/20240126210641 | 1 +
.../backfill_default_organization_owners.rb | 24 +------
...kfill_default_organization_owners_again.rb | 36 +++++++++++
..._default_organization_owners_again_spec.rb | 64 +++++++++++++++++++
...ckfill_default_organization_owners_spec.rb | 49 ++------------
...kfill_default_organization_owners_spec.rb} | 2 +-
10 files changed, 153 insertions(+), 88 deletions(-)
create mode 100644 db/docs/batched_background_migrations/backfill_default_organization_owners_again.yml
create mode 100644 db/post_migrate/20240126210641_requeue_backfill_default_organization_owners.rb
create mode 100644 db/schema_migrations/20240126210641
create mode 100644 lib/gitlab/background_migration/backfill_default_organization_owners_again.rb
create mode 100644 spec/lib/gitlab/background_migration/backfill_default_organization_owners_again_spec.rb
rename spec/migrations/{20240108182342_queue_backfill_default_organization_owners_spec.rb => 20240126210641_requeue_backfill_default_organization_owners_spec.rb} (89%)
diff --git a/db/docs/batched_background_migrations/backfill_default_organization_owners.yml b/db/docs/batched_background_migrations/backfill_default_organization_owners.yml
index ccd7f226a4853..47fa60e6786fd 100644
--- a/db/docs/batched_background_migrations/backfill_default_organization_owners.yml
+++ b/db/docs/batched_background_migrations/backfill_default_organization_owners.yml
@@ -1,9 +1,9 @@
---
migration_job_name: BackfillDefaultOrganizationOwners
-description: Populates organization_users with instance admins to the default organization.
+description: No-op in favor of re-enqueueing with BackfillDefaultOrganizationOwnersAgain
feature_category: cell
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141297
milestone: '16.8'
queued_migration_version: 20240108182342
finalize_after: '2024-02-15'
-finalized_by: # version of the migration that ensured this bbm
+finalized_by: 20240126210641
diff --git a/db/docs/batched_background_migrations/backfill_default_organization_owners_again.yml b/db/docs/batched_background_migrations/backfill_default_organization_owners_again.yml
new file mode 100644
index 0000000000000..31200571f40f6
--- /dev/null
+++ b/db/docs/batched_background_migrations/backfill_default_organization_owners_again.yml
@@ -0,0 +1,9 @@
+---
+migration_job_name: BackfillDefaultOrganizationOwnersAgain
+description: Populates organization_users with instance admins to the default organization.
+feature_category: cell
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142979
+milestone: '16.9'
+queued_migration_version: 20240126210641
+finalize_after: '2024-03-15'
+finalized_by: # version of the migration that ensured this bbm
diff --git a/db/post_migrate/20240108182342_queue_backfill_default_organization_owners.rb b/db/post_migrate/20240108182342_queue_backfill_default_organization_owners.rb
index a29fbba5cbcd0..c7956b01a1b95 100644
--- a/db/post_migrate/20240108182342_queue_backfill_default_organization_owners.rb
+++ b/db/post_migrate/20240108182342_queue_backfill_default_organization_owners.rb
@@ -2,28 +2,13 @@
class QueueBackfillDefaultOrganizationOwners < Gitlab::Database::Migration[2.2]
milestone '16.9'
-
- MIGRATION = 'BackfillDefaultOrganizationOwners'
- DELAY_INTERVAL = 2.minutes
- BATCH_SIZE = 3_000
- SUB_BATCH_SIZE = 250
- MAX_BATCH_SIZE = 10_000
-
restrict_gitlab_migration gitlab_schema: :gitlab_main
def up
- queue_batched_background_migration(
- MIGRATION,
- :users,
- :id,
- job_interval: DELAY_INTERVAL,
- batch_size: BATCH_SIZE,
- sub_batch_size: SUB_BATCH_SIZE,
- max_batch_size: MAX_BATCH_SIZE
- )
+ # no-op
end
def down
- delete_batched_background_migration(MIGRATION, :users, :id, [])
+ # no-op
end
end
diff --git a/db/post_migrate/20240126210641_requeue_backfill_default_organization_owners.rb b/db/post_migrate/20240126210641_requeue_backfill_default_organization_owners.rb
new file mode 100644
index 0000000000000..a4f8f086e22cf
--- /dev/null
+++ b/db/post_migrate/20240126210641_requeue_backfill_default_organization_owners.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+class RequeueBackfillDefaultOrganizationOwners < Gitlab::Database::Migration[2.2]
+ milestone '16.9'
+
+ OLD_MIGRATION = 'BackfillDefaultOrganizationOwners'
+ MIGRATION = 'BackfillDefaultOrganizationOwnersAgain'
+ DELAY_INTERVAL = 2.minutes
+ BATCH_SIZE = 3_000
+ SUB_BATCH_SIZE = 250
+ MAX_BATCH_SIZE = 10_000
+
+ restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+ def up
+ # remove so we can re-enqueue
+ delete_batched_background_migration(OLD_MIGRATION, :users, :id, [])
+
+ queue_batched_background_migration(
+ MIGRATION,
+ :users,
+ :id,
+ job_interval: DELAY_INTERVAL,
+ batch_size: BATCH_SIZE,
+ sub_batch_size: SUB_BATCH_SIZE,
+ max_batch_size: MAX_BATCH_SIZE
+ )
+ end
+
+ def down
+ delete_batched_background_migration(MIGRATION, :users, :id, [])
+ end
+end
diff --git a/db/schema_migrations/20240126210641 b/db/schema_migrations/20240126210641
new file mode 100644
index 0000000000000..dc03ed7da9a56
--- /dev/null
+++ b/db/schema_migrations/20240126210641
@@ -0,0 +1 @@
+ec234d88c305f23c5d6b9ee9463fc2f17a02b9f5df3d648a9d0abc0a405d442b
\ No newline at end of file
diff --git a/lib/gitlab/background_migration/backfill_default_organization_owners.rb b/lib/gitlab/background_migration/backfill_default_organization_owners.rb
index 123d951a519b0..6022918845801 100644
--- a/lib/gitlab/background_migration/backfill_default_organization_owners.rb
+++ b/lib/gitlab/background_migration/backfill_default_organization_owners.rb
@@ -4,32 +4,10 @@ module Gitlab
module BackgroundMigration
class BackfillDefaultOrganizationOwners < BatchedMigrationJob
operation_name :backfill_default_organization_owners # This is used as the key on collecting metrics
- scope_to ->(relation) { relation.where(admin: true) }
feature_category :cell
- module Organizations
- class OrganizationUser < ApplicationRecord
- self.table_name = 'organization_users'
- self.inheritance_column = :_type_disabled
- end
- end
-
def perform
- each_sub_batch do |sub_batch|
- organization_users_attributes = sub_batch.map do |user|
- {
- user_id: user.id,
- organization_id: ::Organizations::Organization::DEFAULT_ORGANIZATION_ID,
- access_level: Gitlab::Access::OWNER
- }
- end
-
- Organizations::OrganizationUser.upsert_all(
- organization_users_attributes,
- returning: false,
- unique_by: [:organization_id, :user_id]
- )
- end
+ # no-op, replaced by BackfillDefaultOrganizationOwnersAgain
end
end
end
diff --git a/lib/gitlab/background_migration/backfill_default_organization_owners_again.rb b/lib/gitlab/background_migration/backfill_default_organization_owners_again.rb
new file mode 100644
index 0000000000000..9b4cb57ca6e72
--- /dev/null
+++ b/lib/gitlab/background_migration/backfill_default_organization_owners_again.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module BackgroundMigration
+ class BackfillDefaultOrganizationOwnersAgain < BatchedMigrationJob
+ operation_name :backfill_default_organization_owners_again # This is used as the key on collecting metrics
+ scope_to ->(relation) { relation.where(admin: true) }
+ feature_category :cell
+
+ module Organizations
+ class OrganizationUser < ApplicationRecord
+ self.table_name = 'organization_users'
+ self.inheritance_column = :_type_disabled
+ end
+ end
+
+ def perform
+ each_sub_batch do |sub_batch|
+ organization_users_attributes = sub_batch.map do |user|
+ {
+ user_id: user.id,
+ organization_id: ::Organizations::Organization::DEFAULT_ORGANIZATION_ID,
+ access_level: Gitlab::Access::OWNER
+ }
+ end
+
+ Organizations::OrganizationUser.upsert_all(
+ organization_users_attributes,
+ returning: false,
+ unique_by: [:organization_id, :user_id]
+ )
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/background_migration/backfill_default_organization_owners_again_spec.rb b/spec/lib/gitlab/background_migration/backfill_default_organization_owners_again_spec.rb
new file mode 100644
index 0000000000000..91fa57e78fe66
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/backfill_default_organization_owners_again_spec.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillDefaultOrganizationOwnersAgain, schema: 20240126210641, feature_category: :cell do
+ let(:organization_users) { table(:organization_users) }
+ let(:users) { table(:users) }
+
+ let!(:first_admin) { users.create!(name: 'first', email: 'first_admin@example.com', projects_limit: 1, admin: true) }
+ let!(:last_admin) { users.create!(name: 'last', email: 'last_admin@example.com', projects_limit: 1, admin: true) }
+ let!(:user) { users.create!(name: 'user', email: 'user@example.com', projects_limit: 1) }
+
+ subject(:migration) do
+ described_class.new(
+ start_id: first_admin.id,
+ end_id: user.id,
+ batch_table: :users,
+ batch_column: :id,
+ sub_batch_size: 100,
+ pause_ms: 0,
+ connection: ApplicationRecord.connection
+ )
+ end
+
+ describe '#perform' do
+ context 'with no entries for admin user in organization_users' do
+ it 'adds admins correctly with the default organization to organization_users' do
+ expect(organization_users.count).to eq(0)
+
+ expect { migration.perform }.to change { organization_users.count }.by(2)
+
+ expect(organization_user_as_owner_exists?(first_admin.id)).to be(true)
+ expect(organization_user_as_owner_exists?(last_admin.id)).to be(true)
+ end
+ end
+
+ context 'when admin already exists in organization_users as a default user' do
+ before do
+ organization_users.create!(
+ organization_id: Organizations::Organization::DEFAULT_ORGANIZATION_ID,
+ user_id: first_admin.id,
+ access_level: Gitlab::Access::GUEST
+ )
+ end
+
+ it 'updates the organization_users entry to owner' do
+ expect(organization_users.count).to eq(1)
+
+ expect { migration.perform }.to change { organization_users.count }.by(1)
+
+ expect(organization_user_as_owner_exists?(first_admin.id)).to be(true)
+ expect(organization_user_as_owner_exists?(last_admin.id)).to be(true)
+ end
+ end
+ end
+
+ def organization_user_as_owner_exists?(user_id)
+ organization_users.exists?(
+ organization_id: Organizations::Organization::DEFAULT_ORGANIZATION_ID,
+ user_id: user_id,
+ access_level: Gitlab::Access::OWNER
+ )
+ end
+end
diff --git a/spec/lib/gitlab/background_migration/backfill_default_organization_owners_spec.rb b/spec/lib/gitlab/background_migration/backfill_default_organization_owners_spec.rb
index f0763b9f4f615..c484efca2a67e 100644
--- a/spec/lib/gitlab/background_migration/backfill_default_organization_owners_spec.rb
+++ b/spec/lib/gitlab/background_migration/backfill_default_organization_owners_spec.rb
@@ -3,17 +3,10 @@
require 'spec_helper'
RSpec.describe Gitlab::BackgroundMigration::BackfillDefaultOrganizationOwners, schema: 20240108182342, feature_category: :cell do
- let(:organization_users) { table(:organization_users) }
- let(:users) { table(:users) }
-
- let!(:first_admin) { users.create!(name: 'first', email: 'first_admin@example.com', projects_limit: 1, admin: true) }
- let!(:last_admin) { users.create!(name: 'last', email: 'last_admin@example.com', projects_limit: 1, admin: true) }
- let!(:user) { users.create!(name: 'user', email: 'user@example.com', projects_limit: 1) }
-
subject(:migration) do
described_class.new(
- start_id: first_admin.id,
- end_id: user.id,
+ start_id: 1,
+ end_id: 2,
batch_table: :users,
batch_column: :id,
sub_batch_size: 100,
@@ -23,42 +16,8 @@
end
describe '#perform' do
- context 'with no entries for admin user in organization_users' do
- it 'adds admins correctly with the default organization to organization_users' do
- expect(organization_users.count).to eq(0)
-
- expect { migration.perform }.to change { organization_users.count }.by(2)
-
- expect(organization_user_as_owner_exists?(first_admin.id)).to be(true)
- expect(organization_user_as_owner_exists?(last_admin.id)).to be(true)
- end
- end
-
- context 'when admin already exists in organization_users as a default user' do
- before do
- organization_users.create!(
- organization_id: Organizations::Organization::DEFAULT_ORGANIZATION_ID,
- user_id: first_admin.id,
- access_level: Gitlab::Access::GUEST
- )
- end
-
- it 'updates the organization_users entry to owner' do
- expect(organization_users.count).to eq(1)
-
- expect { migration.perform }.to change { organization_users.count }.by(1)
-
- expect(organization_user_as_owner_exists?(first_admin.id)).to be(true)
- expect(organization_user_as_owner_exists?(last_admin.id)).to be(true)
- end
+ it 'runs without error for no-op' do
+ migration.perform
end
end
-
- def organization_user_as_owner_exists?(user_id)
- organization_users.exists?(
- organization_id: Organizations::Organization::DEFAULT_ORGANIZATION_ID,
- user_id: user_id,
- access_level: Gitlab::Access::OWNER
- )
- end
end
diff --git a/spec/migrations/20240108182342_queue_backfill_default_organization_owners_spec.rb b/spec/migrations/20240126210641_requeue_backfill_default_organization_owners_spec.rb
similarity index 89%
rename from spec/migrations/20240108182342_queue_backfill_default_organization_owners_spec.rb
rename to spec/migrations/20240126210641_requeue_backfill_default_organization_owners_spec.rb
index aee7183bdb9bd..7d5887b4f14c6 100644
--- a/spec/migrations/20240108182342_queue_backfill_default_organization_owners_spec.rb
+++ b/spec/migrations/20240126210641_requeue_backfill_default_organization_owners_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
require_migration!
-RSpec.describe QueueBackfillDefaultOrganizationOwners, feature_category: :cell do
+RSpec.describe RequeueBackfillDefaultOrganizationOwners, feature_category: :cell do
let!(:batched_migration) { described_class::MIGRATION }
it 'schedules a new batched migration' do
--
GitLab