Merge branch 'cleanup-bigint-cibuildneeds-for-self-managed' into 'master'

Cleanup conversion of bigint ci build needs for self-managed

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127386



Merged-by: Diogo Frazão <dfrazao@gitlab.com>
Approved-by: Diogo Frazão <dfrazao@gitlab.com>
Co-authored-by: Laura Montemayor <lmontemayor@gitlab.com>

db/post_migrate/20230724123547_cleanup_conversion_big_int_ci_build_needs_self_managed.rb

+# frozen_string_literal: true
+
+class CleanupConversionBigIntCiBuildNeedsSelfManaged < Gitlab::Database::Migration[2.1]
+  include Gitlab::Database::MigrationHelpers::ConvertToBigint
+
+  enable_lock_retries!
+
+  TABLE = :ci_build_needs
+
+  def up
+    return if should_skip?
+    return unless column_exists?(TABLE, :id_convert_to_bigint)
+
+    # rubocop:disable Migration/WithLockRetriesDisallowedMethod
+    with_lock_retries do
+      cleanup_conversion_of_integer_to_bigint(TABLE, :id)
+    end
+    # rubocop:enable Migration/WithLockRetriesDisallowedMethod
+  end
+
+  def down
+    return if should_skip?
+    return if column_exists?(TABLE, :id_convert_to_bigint)
+
+    restore_conversion_of_integer_to_bigint(TABLE, :id)
+  end
+
+  def should_skip?
+    com_or_dev_or_test_but_not_jh?
+  end
+end

db/schema_migrations/20230724123547

+658cb25d5add4ad4e26d7baef6759f8512fa0244dd347b0522ad75ac496c9965
\ No newline at end of file

spec/migrations/cleanup_conversion_big_int_ci_build_needs_self_managed_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe CleanupConversionBigIntCiBuildNeedsSelfManaged, feature_category: :database do
+  after do
+    connection = described_class.new.connection
+    connection.execute('ALTER TABLE ci_build_needs DROP COLUMN IF EXISTS id_convert_to_bigint')
+  end
+
+  describe '#up' do
+    context 'when it is GitLab.com, dev, or test but not JiHu' do
+      before do
+        # As we call `schema_migrate_down!` before each example, and for this migration
+        # `#down` is same as `#up`, we need to ensure we start from the expected state.
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs DROP COLUMN IF EXISTS id_convert_to_bigint')
+      end
+
+      it 'does nothing' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        disable_migrations_output do
+          reversible_migration do |migration|
+            migration.before -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+
+            migration.after -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+          end
+        end
+      end
+    end
+
+    context 'when there is a self-managed instance with the temporary column already dropped' do
+      before do
+        # As we call `schema_migrate_down!` before each example, and for this migration
+        # `#down` is same as `#up`, we need to ensure we start from the expected state.
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id TYPE bigint')
+        connection.execute('ALTER TABLE ci_build_needs DROP COLUMN IF EXISTS id_convert_to_bigint')
+      end
+
+      it 'does nothing' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        migrate!
+
+        expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+        expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+      end
+    end
+
+    context 'when there is a self-managed instance with the temporary columns' do
+      before do
+        # As we call `schema_migrate_down!` before each example, and for this migration
+        # `#down` is same as `#up`, we need to ensure we start from the expected state.
+        connection = described_class.new.connection
+        connection.execute('ALTER TABLE ci_build_needs ALTER COLUMN id TYPE bigint')
+        connection.execute('ALTER TABLE ci_build_needs ADD COLUMN IF NOT EXISTS id_convert_to_bigint integer')
+      end
+
+      it 'drops the temporary column' do
+        # rubocop: disable RSpec/AnyInstanceOf
+        allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false)
+        # rubocop: enable RSpec/AnyInstanceOf
+
+        ci_build_needs = table(:ci_build_needs)
+
+        disable_migrations_output do
+          reversible_migration do |migration|
+            migration.before -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find do |c|
+                       c.name == 'id_convert_to_bigint'
+                     end.sql_type).to eq('integer')
+            }
+
+            migration.after -> {
+              ci_build_needs.reset_column_information
+
+              expect(ci_build_needs.columns.find { |c| c.name == 'id' }.sql_type).to eq('bigint')
+              expect(ci_build_needs.columns.find { |c| c.name == 'id_convert_to_bigint' }).to be nil
+            }
+          end
+        end
+      end
+    end
+  end
+end