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 ccd7f226a4853139c9461b25196b9af366372dc0..47fa60e6786fdf6ce0c15fb30fa13d11684df920 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 0000000000000000000000000000000000000000..31200571f40f604ed77a169c54064df6e70a403c --- /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 a29fbba5cbcd03e338e0ab688edbefb1ba859b85..c7956b01a1b953b9344199ef30450a8d6c06056e 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 0000000000000000000000000000000000000000..a4f8f086e22cfeba26b42ef170eda969cca1a444 --- /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 0000000000000000000000000000000000000000..dc03ed7da9a5627b88e6abe39cbc836286998a1c --- /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 123d951a519b0178c0d8f716229d450ed3e61747..602291884580191c6af95981db67aec3f55f9083 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 0000000000000000000000000000000000000000..9b4cb57ca6e7296b891ada7a1bb4bd1af0e5a6a3 --- /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 0000000000000000000000000000000000000000..91fa57e78fe6616677430792d32dc0c125547f5c --- /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 f0763b9f4f6155d8f6a81ad05d1f1bf740e7fec2..c484efca2a67ec0d059348ae2d143e5601be654a 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 aee7183bdb9bd73168463863090dcbb054640382..7d5887b4f14c6f6a37b4724b328c895f00857d3f 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