diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index fd1c457a7ff142cdfffeaeb525ad750454a3d358..5321f9ddcc1e458a721ac41bfad489336024e4e5 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -20,6 +20,8 @@ class Runner < Ci::ApplicationRecord self.primary_key = :id + ignore_column :sharding_key_id, remove_with: '18.5', remove_after: '2025-09-22' + add_authentication_token_field :token, encrypted: :optional, expires_at: :compute_token_expiration, @@ -28,8 +30,8 @@ class Runner < Ci::ApplicationRecord if: ->(token_owner_record) { token_owner_record.owner }, payload: { o: ->(token_owner_record) { token_owner_record.owner.try(:organization_id) }, - g: ->(token_owner_record) { token_owner_record.group_type? ? token_owner_record.sharding_key_id : nil }, - p: ->(token_owner_record) { token_owner_record.project_type? ? token_owner_record.sharding_key_id : nil }, + g: ->(token_owner_record) { token_owner_record.runner_namespaces.first&.namespace_id }, + p: ->(token_owner_record) { token_owner_record.runner_projects.first&.project_id }, u: ->(token_owner_record) { token_owner_record.creator_id }, t: ->(token_owner_record) { token_owner_record.partition_id } } @@ -151,7 +153,6 @@ class Runner < Ci::ApplicationRecord scope :created_by_admins, -> { with_creator_id(User.admins.ids) } scope :with_creator_id, ->(value) { where(creator_id: value) } - scope :with_sharding_key, ->(value) { where(sharding_key_id: value) } scope :belonging_to_group_or_project_descendants, ->(group_id) { group_ids = Ci::NamespaceMirror.by_group_and_descendants(group_id).select(:namespace_id) @@ -242,7 +243,6 @@ class Runner < Ci::ApplicationRecord scope :with_api_entity_associations, -> { preload(:creator) } validate :tag_constraints - validates :sharding_key_id, presence: true, unless: :instance_type? validates :organization_id, presence: true, on: [:create, :update], unless: :instance_type? validates :name, length: { maximum: 256 }, if: :name_changed? validates :description, length: { maximum: 1024 }, if: :description_changed? @@ -251,7 +251,6 @@ class Runner < Ci::ApplicationRecord validates :registration_type, presence: true validate :no_projects, unless: :project_type? - validate :no_sharding_key_id, if: :instance_type? validate :no_organization_id, if: :instance_type? validate :no_groups, unless: :group_type? validate :any_project, if: :project_type? @@ -382,11 +381,6 @@ def assign_to(project, current_user = nil) begin transaction do - if projects.id_in(sharding_key_id).empty? && !update_project_id - self.errors.add(:assign_to, 'Runner is orphaned and no fallback owner exists') - next false - end - self.runner_projects << ::Ci::RunnerProject.new(project: project, runner: self) self.save! end @@ -550,7 +544,6 @@ def ensure_manager(system_xid) # rubocop: disable Performance/ActiveRecordSubtransactionMethods -- This is used only in API endpoints outside of transactions RunnerManager.safe_find_or_create_by!(runner_id: id, system_xid: system_xid.to_s) do |m| m.runner_type = runner_type - m.sharding_key_id = sharding_key_id m.organization_id = organization_id end # rubocop: enable Performance/ActiveRecordSubtransactionMethods @@ -592,20 +585,6 @@ def fallback_owner_project Project.order(projects_added_to_runner_asc).find_by_id(project_ids) end - # Ensure we have a valid sharding_key_id. Logic is similar to the one used in - # Ci::Runners::UpdateProjectRunnersOwnerService when a project is deleted - def update_project_id - project_id = fallback_owner_project&.id - - return if project_id.nil? - - self.clear_memoization(:owner) - self.sharding_key_id = project_id - - runner_managers.where(runner_type: :project_type).each_batch { |batch| batch.update_all(sharding_key_id: project_id) } - taggings.where(runner_type: :project_type).update_all(sharding_key_id: project_id) - end - def compute_token_expiration_instance return unless expiration_interval = Gitlab::CurrentSettings.runner_token_expiration_interval @@ -653,10 +632,6 @@ def tag_constraints end end - def no_sharding_key_id - errors.add(:runner, 'cannot have sharding_key_id assigned') if sharding_key_id - end - def no_organization_id errors.add(:runner, 'cannot have organization_id assigned') if organization_id end diff --git a/app/models/ci/runner_manager.rb b/app/models/ci/runner_manager.rb index e82f24127231dc806638437f3cf74ad0c1861e0f..7f78d50df886d2e5b4fe67d93eeb2d87a921990b 100644 --- a/app/models/ci/runner_manager.rb +++ b/app/models/ci/runner_manager.rb @@ -12,6 +12,8 @@ class RunnerManager < Ci::ApplicationRecord self.table_name = 'ci_runner_machines' self.primary_key = :id + ignore_column :sharding_key_id, remove_with: '18.5', remove_after: '2025-09-22' + AVAILABLE_STATUSES = %w[online offline never_contacted stale].freeze AVAILABLE_STATUSES_INCL_DEPRECATED = AVAILABLE_STATUSES @@ -55,7 +57,6 @@ class RunnerManager < Ci::ApplicationRecord validates :runner, presence: true validates :runner_type, presence: true, on: :create validates :system_xid, presence: true, length: { maximum: 64 } - validates :sharding_key_id, presence: true, on: :create, unless: :instance_type? validates :organization_id, presence: true, on: [:create, :update], unless: :instance_type? validates :version, length: { maximum: 2048 } validates :revision, length: { maximum: 255 } @@ -65,7 +66,6 @@ class RunnerManager < Ci::ApplicationRecord validates :config, json_schema: { filename: 'ci_runner_config' } validates :runtime_features, json_schema: { filename: 'ci_runner_runtime_features' } - validate :no_sharding_key_id, if: :instance_type? validate :no_organization_id, if: :instance_type? cached_attr_reader :version, :revision, :platform, :architecture, :ip_address, :contacted_at, @@ -193,10 +193,6 @@ def schedule_runner_version_update(new_version) Ci::Runners::ProcessRunnerVersionUpdateWorker.perform_async(new_version) end - def no_sharding_key_id - errors.add(:runner_manager, 'cannot have sharding_key_id assigned') if sharding_key_id - end - def no_organization_id errors.add(:runner_manager, 'cannot have organization_id assigned') if organization_id end diff --git a/app/models/ci/runner_tagging.rb b/app/models/ci/runner_tagging.rb index e7c98b15861b4bf8fe9d27bd1de6cfaa825a7e82..13d14a0721065b28d4fb3636af72d4f15a6f1573 100644 --- a/app/models/ci/runner_tagging.rb +++ b/app/models/ci/runner_tagging.rb @@ -7,6 +7,8 @@ class RunnerTagging < Ci::ApplicationRecord self.table_name = :ci_runner_taggings self.primary_key = :id + ignore_column :sharding_key_id, remove_with: '18.5', remove_after: '2025-09-22' + query_constraints :runner_id, :runner_type before_validation :set_runner_type, on: :create, if: -> { runner_type.nil? && runner } @@ -19,10 +21,8 @@ class RunnerTagging < Ci::ApplicationRecord belongs_to :tag, class_name: 'Ci::Tag', optional: false validates :runner_type, presence: true - validates :sharding_key_id, presence: true, unless: :instance_type? validates :organization_id, presence: true, on: [:create, :update], unless: :instance_type? - validate :no_sharding_key_id, if: :instance_type? validate :no_organization_id, if: :instance_type? scope :scoped_runners, -> do @@ -37,12 +37,6 @@ def set_runner_type self.runner_type = runner.runner_type end - def no_sharding_key_id - return if sharding_key_id.nil? - - errors.add(:sharding_key_id, 'instance_type runners must not have a sharding_key_id') - end - def no_organization_id return if organization_id.nil? diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb index 02de174aacf567b2b749093040c42f29082bef77..6ec9e6562ffb990853035b671f958201ffae5199 100644 --- a/app/services/ci/runners/register_runner_service.rb +++ b/app/services/ci/runners/register_runner_service.rb @@ -47,10 +47,10 @@ def attrs_from_token { runner_type: :instance_type, organization_id: nil } elsif runner_registrar_valid?('project') && project = ::Project.find_by_runners_token(registration_token) # Create a project runner - { runner_type: :project_type, projects: [project], sharding_key_id: project.id, organization_id: project.organization_id } + { runner_type: :project_type, projects: [project], organization_id: project.organization_id } elsif runner_registrar_valid?('group') && group = ::Group.find_by_runners_token(registration_token) # Create a group runner - { runner_type: :group_type, groups: [group], sharding_key_id: group.id, organization_id: group.organization_id } + { runner_type: :group_type, groups: [group], organization_id: group.organization_id } elsif registration_token.present? && !Gitlab::CurrentSettings.allow_runner_registration_token {} # Will result in a :runner_registration_disallowed response end diff --git a/app/services/ci/runners/runner_creation_strategies/group_runner_strategy.rb b/app/services/ci/runners/runner_creation_strategies/group_runner_strategy.rb index 384db2609bdb572843a90a56981a0dd1e7061a35..68ef90746052706ee08d464d5cb89a794dd9598b 100644 --- a/app/services/ci/runners/runner_creation_strategies/group_runner_strategy.rb +++ b/app/services/ci/runners/runner_creation_strategies/group_runner_strategy.rb @@ -14,7 +14,6 @@ def initialize(user:, params:) def normalize_params params.merge!({ runner_type: 'group_type', - sharding_key_id: scope&.id, organization_id: scope&.organization_id, groups: [scope] }) diff --git a/app/services/ci/runners/runner_creation_strategies/project_runner_strategy.rb b/app/services/ci/runners/runner_creation_strategies/project_runner_strategy.rb index 0c1d54ba9291f029a63d977fd7030427c353020a..a7d13964de82bc6fc25e1de7d89cd62eebb0eec2 100644 --- a/app/services/ci/runners/runner_creation_strategies/project_runner_strategy.rb +++ b/app/services/ci/runners/runner_creation_strategies/project_runner_strategy.rb @@ -14,7 +14,6 @@ def initialize(user:, params:) def normalize_params params.merge!({ runner_type: 'project_type', - sharding_key_id: scope&.id, organization_id: scope&.organization_id, projects: [scope] }) diff --git a/app/services/ci/runners/update_project_runners_owner_service.rb b/app/services/ci/runners/update_project_runners_owner_service.rb deleted file mode 100644 index 508f1eb657adb0f5fa3d29dfe62646d682d7447b..0000000000000000000000000000000000000000 --- a/app/services/ci/runners/update_project_runners_owner_service.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -module Ci - module Runners - # Service used to recompute runner owners after a project is deleted. - class UpdateProjectRunnersOwnerService - BATCH_SIZE = 1000 - - # @param [Int] project_id: the ID of the deleted project - # @param [Int] namespace_id: the ID of the parent namespace of the deleted project - def initialize(project_id, namespace_id) - @project_id = project_id - @organization_id = Namespace.find(namespace_id).organization_id - end - - def execute - # Since the project was deleted in the 'main' database, let's ensure that the respective - # ci_runner_projects join records are also gone (would be handled by LFK otherwise, - # but it is a helpful precondition for the service's logic) - Ci::RunnerProject.belonging_to_project(project_id).each_batch do |batch| - batch.delete_all - end - - # rubocop: disable CodeReuse/ActiveRecord -- this query is too specific to generalize on the models - runner_projects = - Ci::RunnerProject.where(Ci::RunnerProject.arel_table[:runner_id].eq(Ci::Runner.arel_table[:id])) - orphaned_runners = Ci::Runner.project_type.with_sharding_key(project_id) - - orphaned_runners.select(:id).each_batch(of: BATCH_SIZE) do |batch| - runners_missing_owner_project = Ci::Runner.id_in(batch.limit(BATCH_SIZE).pluck_primary_key) - runners_with_fallback_owner = runners_missing_owner_project.where_exists(runner_projects.limit(1)) - - Ci::Runner.transaction do - runner_ids = runners_with_fallback_owner.limit(BATCH_SIZE).pluck_primary_key - - runners_with_fallback_owner.update_all(runner_id_update_query(Ci::Runner.arel_table[:id])) - Ci::RunnerManager.project_type.for_runner(runner_ids) - .update_all(runner_id_update_query(Ci::RunnerManager.arel_table[:runner_id])) - Ci::RunnerTagging.project_type.for_runner(runner_ids) - .update_all(runner_id_update_query(Ci::RunnerTagging.arel_table[:runner_id])) - - # Delete any orphaned runners that are still pointing to the project - # (they are the ones which no longer have any matching ci_runner_projects records) - # We can afford to sidestep Ci::Runner hooks since we know by definition that - # there are no Ci::RunnerProject models pointing to these runners (it's the reason they are being deleted) - runners_missing_owner_project.project_type.with_sharding_key(project_id).delete_all - end - end - # rubocop: enable CodeReuse/ActiveRecord - - ServiceResponse.success - end - - private - - attr_reader :project_id - - def runner_id_update_query(runner_id_column) - # rubocop: disable CodeReuse/ActiveRecord -- this query is too specific to generalize on the models - runner_projects = Ci::RunnerProject.where(Ci::RunnerProject.arel_table[:runner_id].eq(runner_id_column)) - - <<~SQL - sharding_key_id = (#{runner_projects.order(id: :asc).limit(1).select(:project_id).to_sql}), - organization_id = #{@organization_id} - SQL - # rubocop: enable CodeReuse/ActiveRecord - end - end - end -end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 49df3ac41dd1fdedced3f111b69d6f876685a13c..b8cb3ef076011c168425da993c71522187b2c6c1 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -3487,16 +3487,6 @@ :idempotent: true :tags: [] :queue_namespace: -- :name: ci_runners_update_project_runners_owner - :worker_name: Ci::Runners::UpdateProjectRunnersOwnerWorker - :feature_category: :runner - :has_external_dependencies: false - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: true - :tags: [] - :queue_namespace: - :name: ci_safe_disable_pipeline_variables :worker_name: Ci::SafeDisablePipelineVariablesWorker :feature_category: :ci_variables diff --git a/app/workers/ci/runners/update_project_runners_owner_worker.rb b/app/workers/ci/runners/update_project_runners_owner_worker.rb deleted file mode 100644 index 8399d77255b1dc000a7ee42ba908c391f23fbe84..0000000000000000000000000000000000000000 --- a/app/workers/ci/runners/update_project_runners_owner_worker.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -module Ci - module Runners - class UpdateProjectRunnersOwnerWorker - include Gitlab::EventStore::Subscriber - - data_consistency :sticky - - idempotent! - - feature_category :runner - - def handle_event(event) - ::Ci::Runners::UpdateProjectRunnersOwnerService.new(event.data[:project_id], event.data[:namespace_id]).execute - end - end - end -end diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 740fb667780f529d6b72c531f56d616ceb3cc9b5..47ab217cdb3a5148082ae69c36971ea7b7a69f5f 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -175,13 +175,8 @@ ci_runner_taggings: - table: tags column: tag_id on_delete: async_delete -ci_runner_taggings_group_type: - - table: namespaces - column: sharding_key_id - on_delete: async_delete -ci_runner_taggings_project_type: - - table: projects - column: sharding_key_id + - table: organizations + column: organization_id on_delete: async_delete ci_runners: - table: users diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 2f579e3f979c4980b7b99e445384d5a25d2ac5fa..11662fdacc5da9ec1ea47e6496653a263c02e468 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -235,8 +235,6 @@ - 1 - - ci_runners_process_runner_version_update - 1 -- - ci_runners_update_project_runners_owner - - 1 - - ci_safe_disable_pipeline_variables - 1 - - ci_slsa_publish_statement diff --git a/db/migrate/20250714180055_remove_sharding_key_check_constraint_from_ci_runners.rb b/db/migrate/20250714180055_remove_sharding_key_check_constraint_from_ci_runners.rb new file mode 100644 index 0000000000000000000000000000000000000000..bf08c4548f236493dd6246f0b6215350e69e3b12 --- /dev/null +++ b/db/migrate/20250714180055_remove_sharding_key_check_constraint_from_ci_runners.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class RemoveShardingKeyCheckConstraintFromCiRunners < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.3' + + CONSTRAINT_NAME = 'check_sharding_key_id_nullness' + + def up + remove_check_constraint(:instance_type_ci_runners, CONSTRAINT_NAME) + remove_check_constraint(:group_type_ci_runners, CONSTRAINT_NAME) + remove_check_constraint(:project_type_ci_runners, CONSTRAINT_NAME) + end + + def down + add_check_constraint(:instance_type_ci_runners, 'sharding_key_id IS NULL', CONSTRAINT_NAME) + add_check_constraint(:group_type_ci_runners, 'sharding_key_id IS NOT NULL', CONSTRAINT_NAME) + add_check_constraint(:project_type_ci_runners, 'sharding_key_id IS NOT NULL', CONSTRAINT_NAME) + end +end diff --git a/db/migrate/20250714180107_remove_sharding_key_check_constraint_from_ci_runner_machines.rb b/db/migrate/20250714180107_remove_sharding_key_check_constraint_from_ci_runner_machines.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b5adc590f5ab3fac14d4c16804e97fe5510221d --- /dev/null +++ b/db/migrate/20250714180107_remove_sharding_key_check_constraint_from_ci_runner_machines.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class RemoveShardingKeyCheckConstraintFromCiRunnerMachines < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.3' + + CONSTRAINT_NAME = 'check_sharding_key_id_nullness' + + def up + remove_check_constraint(:instance_type_ci_runner_machines, CONSTRAINT_NAME) + remove_check_constraint(:group_type_ci_runner_machines, CONSTRAINT_NAME) + remove_check_constraint(:project_type_ci_runner_machines, CONSTRAINT_NAME) + end + + def down + add_check_constraint(:instance_type_ci_runner_machines, 'sharding_key_id IS NULL', CONSTRAINT_NAME) + add_check_constraint(:group_type_ci_runner_machines, 'sharding_key_id IS NOT NULL', CONSTRAINT_NAME) + add_check_constraint(:project_type_ci_runner_machines, 'sharding_key_id IS NOT NULL', CONSTRAINT_NAME) + end +end diff --git a/db/migrate/20250714180114_remove_sharding_key_check_constraint_from_ci_runner_taggings.rb b/db/migrate/20250714180114_remove_sharding_key_check_constraint_from_ci_runner_taggings.rb new file mode 100644 index 0000000000000000000000000000000000000000..09dbd26a1fbefb5365ec043b89287d3e0ab9dec2 --- /dev/null +++ b/db/migrate/20250714180114_remove_sharding_key_check_constraint_from_ci_runner_taggings.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class RemoveShardingKeyCheckConstraintFromCiRunnerTaggings < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.3' + + CONSTRAINT_NAME = 'check_sharding_key_id_nullness' + + def up + remove_check_constraint(:ci_runner_taggings_instance_type, CONSTRAINT_NAME) + remove_check_constraint(:ci_runner_taggings_group_type, CONSTRAINT_NAME) + remove_check_constraint(:ci_runner_taggings_project_type, CONSTRAINT_NAME) + end + + def down + add_check_constraint(:ci_runner_taggings_instance_type, 'sharding_key_id IS NULL', CONSTRAINT_NAME) + add_check_constraint(:ci_runner_taggings_group_type, 'sharding_key_id IS NOT NULL', CONSTRAINT_NAME) + add_check_constraint(:ci_runner_taggings_project_type, 'sharding_key_id IS NOT NULL', CONSTRAINT_NAME) + end +end diff --git a/db/migrate/20250714180214_cleanup_records_with_null_sharding_key_id_values_from_ci_runners.rb b/db/migrate/20250714180214_cleanup_records_with_null_sharding_key_id_values_from_ci_runners.rb new file mode 100644 index 0000000000000000000000000000000000000000..e3643676a336e8c94ece73167e8ed0f982e81473 --- /dev/null +++ b/db/migrate/20250714180214_cleanup_records_with_null_sharding_key_id_values_from_ci_runners.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CleanupRecordsWithNullShardingKeyIdValuesFromCiRunners < Gitlab::Database::Migration[2.3] + milestone '18.3' + + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_ci + + BATCH_SIZE = 1000 + + class CiRunner < MigrationRecord + include EachBatch + + self.table_name = 'ci_runners' + self.primary_key = :id + end + + def up + # no-op - this migration is required to allow a rollback of `RemoveShardingKeyCheckConstraintFromCiRunners` + end + + def down + CiRunner.each_batch(of: BATCH_SIZE) do |relation| + relation.where.not(runner_type: 1).where(sharding_key_id: nil).delete_all + end + end +end diff --git a/db/migrate/20250714180215_cleanup_records_with_null_sharding_key_id_values_from_ci_runner_machines.rb b/db/migrate/20250714180215_cleanup_records_with_null_sharding_key_id_values_from_ci_runner_machines.rb new file mode 100644 index 0000000000000000000000000000000000000000..dc7d327b27250148f1376203e884b4183c7d0be5 --- /dev/null +++ b/db/migrate/20250714180215_cleanup_records_with_null_sharding_key_id_values_from_ci_runner_machines.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CleanupRecordsWithNullShardingKeyIdValuesFromCiRunnerMachines < Gitlab::Database::Migration[2.3] + milestone '18.3' + + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_ci + + BATCH_SIZE = 1000 + + class CiRunnerMachine < MigrationRecord + include EachBatch + + self.table_name = 'ci_runner_machines' + self.primary_key = :id + end + + def up + # no-op - this migration is required to allow a rollback of `RemoveShardingKeyCheckConstraintFromCiRunnerMachines` + end + + def down + CiRunnerMachine.each_batch(of: BATCH_SIZE) do |relation| + relation.where.not(runner_type: 1).where(sharding_key_id: nil).delete_all + end + end +end diff --git a/db/migrate/20250714180216_cleanup_records_with_null_sharding_key_id_values_from_ci_runner_taggings.rb b/db/migrate/20250714180216_cleanup_records_with_null_sharding_key_id_values_from_ci_runner_taggings.rb new file mode 100644 index 0000000000000000000000000000000000000000..54a734477b20d2f0ab362d94e1ad8f0048ca4beb --- /dev/null +++ b/db/migrate/20250714180216_cleanup_records_with_null_sharding_key_id_values_from_ci_runner_taggings.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CleanupRecordsWithNullShardingKeyIdValuesFromCiRunnerTaggings < Gitlab::Database::Migration[2.3] + milestone '18.3' + + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_ci + + BATCH_SIZE = 1000 + + class CiRunnerTagging < MigrationRecord + include EachBatch + + self.table_name = 'ci_runner_taggings' + self.primary_key = :id + end + + def up + # no-op - this migration is required to allow a rollback of `RemoveShardingKeyCheckConstraintFromCiRunnerTaggings` + end + + def down + CiRunnerTagging.each_batch(of: BATCH_SIZE) do |relation| + relation.where.not(runner_type: 1).where(sharding_key_id: nil).delete_all + end + end +end diff --git a/db/schema_migrations/20250714180055 b/db/schema_migrations/20250714180055 new file mode 100644 index 0000000000000000000000000000000000000000..a310c6573be4f69e39463cbb8134d2d7d76b966e --- /dev/null +++ b/db/schema_migrations/20250714180055 @@ -0,0 +1 @@ +3b2da24581e854c47b085df4bf8bef15fdba74453fd271e64d36ed285baf5d24 \ No newline at end of file diff --git a/db/schema_migrations/20250714180107 b/db/schema_migrations/20250714180107 new file mode 100644 index 0000000000000000000000000000000000000000..1cc3615c15056a226607f1b189b4b4220d7a07a3 --- /dev/null +++ b/db/schema_migrations/20250714180107 @@ -0,0 +1 @@ +7a16f30def5e8674924f5f33e61a81df910cc90f034012d361da2de0bd735569 \ No newline at end of file diff --git a/db/schema_migrations/20250714180114 b/db/schema_migrations/20250714180114 new file mode 100644 index 0000000000000000000000000000000000000000..e1d5aea0a72f6b877c6bde2a0cb0a3a9c4c05336 --- /dev/null +++ b/db/schema_migrations/20250714180114 @@ -0,0 +1 @@ +9ffbf92243027d215556609e129e77ef61e974d6a36838d1b36837c5941beadf \ No newline at end of file diff --git a/db/schema_migrations/20250714180214 b/db/schema_migrations/20250714180214 new file mode 100644 index 0000000000000000000000000000000000000000..95d3fdb2aae7a2e9de9cbf33c064d3992ac2950e --- /dev/null +++ b/db/schema_migrations/20250714180214 @@ -0,0 +1 @@ +8d62b8a7cc62acb17543ed5a412a069fb085e35191f470dd297eec257088c15f \ No newline at end of file diff --git a/db/schema_migrations/20250714180215 b/db/schema_migrations/20250714180215 new file mode 100644 index 0000000000000000000000000000000000000000..dc14f8706f7499f92b90f6018ab5aa1d5ca7daaa --- /dev/null +++ b/db/schema_migrations/20250714180215 @@ -0,0 +1 @@ +8f25892403ae3ac8fb4c40bb95ee13f6098b99239ca2fd8ea4cba8e9d2aab9e7 \ No newline at end of file diff --git a/db/schema_migrations/20250714180216 b/db/schema_migrations/20250714180216 new file mode 100644 index 0000000000000000000000000000000000000000..34b9d5b7e2490081e39d493222575b22e88482e8 --- /dev/null +++ b/db/schema_migrations/20250714180216 @@ -0,0 +1 @@ +9367f3b6d9c0d8b21e3cd49a321a37ab8de5709b8f83fcdeb7cf1047a5c8a680 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8b651fd1910ef77600189734b980b3da9b403a16..9148620115090de0972f6a277ea6bec4648f697e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12311,8 +12311,7 @@ CREATE TABLE ci_runner_taggings_group_type ( runner_id bigint NOT NULL, sharding_key_id bigint, runner_type smallint NOT NULL, - organization_id bigint, - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NOT NULL)) + organization_id bigint ); CREATE SEQUENCE ci_runner_taggings_id_seq @@ -12330,8 +12329,7 @@ CREATE TABLE ci_runner_taggings_instance_type ( runner_id bigint NOT NULL, sharding_key_id bigint, runner_type smallint NOT NULL, - organization_id bigint, - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NULL)) + organization_id bigint ); CREATE TABLE ci_runner_taggings_project_type ( @@ -12340,8 +12338,7 @@ CREATE TABLE ci_runner_taggings_project_type ( runner_id bigint NOT NULL, sharding_key_id bigint, runner_type smallint NOT NULL, - organization_id bigint, - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NOT NULL)) + organization_id bigint ); CREATE TABLE ci_runner_versions ( @@ -15737,8 +15734,7 @@ CREATE TABLE group_type_ci_runner_machines ( CONSTRAINT check_7dc4eee8a5 CHECK ((char_length(version) <= 2048)), CONSTRAINT check_b1e456641b CHECK ((char_length(ip_address) <= 1024)), CONSTRAINT check_c788f4b18a CHECK ((char_length(platform) <= 255)), - CONSTRAINT check_f3d25ab844 CHECK ((char_length(architecture) <= 255)), - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NOT NULL)) + CONSTRAINT check_f3d25ab844 CHECK ((char_length(architecture) <= 255)) ); CREATE TABLE group_type_ci_runners ( @@ -15771,8 +15767,7 @@ CREATE TABLE group_type_ci_runners ( CONSTRAINT check_1f8618ab23 CHECK ((char_length(name) <= 256)), CONSTRAINT check_24b281f5bf CHECK ((char_length(maintainer_note) <= 1024)), CONSTRAINT check_5db8ae9d30 CHECK ((char_length(description) <= 1024)), - CONSTRAINT check_af25130d5a CHECK ((char_length(token) <= 128)), - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NOT NULL)) + CONSTRAINT check_af25130d5a CHECK ((char_length(token) <= 128)) ); CREATE TABLE group_wiki_repositories ( @@ -16412,8 +16407,7 @@ CREATE TABLE instance_type_ci_runner_machines ( CONSTRAINT check_7dc4eee8a5 CHECK ((char_length(version) <= 2048)), CONSTRAINT check_b1e456641b CHECK ((char_length(ip_address) <= 1024)), CONSTRAINT check_c788f4b18a CHECK ((char_length(platform) <= 255)), - CONSTRAINT check_f3d25ab844 CHECK ((char_length(architecture) <= 255)), - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NULL)) + CONSTRAINT check_f3d25ab844 CHECK ((char_length(architecture) <= 255)) ); CREATE TABLE instance_type_ci_runners ( @@ -16446,8 +16440,7 @@ CREATE TABLE instance_type_ci_runners ( CONSTRAINT check_1f8618ab23 CHECK ((char_length(name) <= 256)), CONSTRAINT check_24b281f5bf CHECK ((char_length(maintainer_note) <= 1024)), CONSTRAINT check_5db8ae9d30 CHECK ((char_length(description) <= 1024)), - CONSTRAINT check_af25130d5a CHECK ((char_length(token) <= 128)), - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NULL)) + CONSTRAINT check_af25130d5a CHECK ((char_length(token) <= 128)) ); CREATE TABLE integrations ( @@ -22162,8 +22155,7 @@ CREATE TABLE project_type_ci_runner_machines ( CONSTRAINT check_7dc4eee8a5 CHECK ((char_length(version) <= 2048)), CONSTRAINT check_b1e456641b CHECK ((char_length(ip_address) <= 1024)), CONSTRAINT check_c788f4b18a CHECK ((char_length(platform) <= 255)), - CONSTRAINT check_f3d25ab844 CHECK ((char_length(architecture) <= 255)), - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NOT NULL)) + CONSTRAINT check_f3d25ab844 CHECK ((char_length(architecture) <= 255)) ); CREATE TABLE project_type_ci_runners ( @@ -22196,8 +22188,7 @@ CREATE TABLE project_type_ci_runners ( CONSTRAINT check_1f8618ab23 CHECK ((char_length(name) <= 256)), CONSTRAINT check_24b281f5bf CHECK ((char_length(maintainer_note) <= 1024)), CONSTRAINT check_5db8ae9d30 CHECK ((char_length(description) <= 1024)), - CONSTRAINT check_af25130d5a CHECK ((char_length(token) <= 128)), - CONSTRAINT check_sharding_key_id_nullness CHECK ((sharding_key_id IS NOT NULL)) + CONSTRAINT check_af25130d5a CHECK ((char_length(token) <= 128)) ); CREATE TABLE project_uploads ( diff --git a/lib/gitlab/ci/tags/bulk_insert/runner_taggings_configuration.rb b/lib/gitlab/ci/tags/bulk_insert/runner_taggings_configuration.rb index 2612bdfca64c3ae4f020cdd1bfdb170ec21bc65e..9b776c6dda465d716b6fdf44d85141771656896d 100644 --- a/lib/gitlab/ci/tags/bulk_insert/runner_taggings_configuration.rb +++ b/lib/gitlab/ci/tags/bulk_insert/runner_taggings_configuration.rb @@ -31,7 +31,6 @@ def attributes_map(runner) { runner_id: runner.id, runner_type: runner.runner_type, - sharding_key_id: runner.sharding_key_id, organization_id: runner.organization_id } end diff --git a/lib/gitlab/event_store.rb b/lib/gitlab/event_store.rb index f20ab0c44627b478ff57ce27a280df714f9d2e25..83286c288a09ffbd504271885eb51d6392ca7f78 100644 --- a/lib/gitlab/event_store.rb +++ b/lib/gitlab/event_store.rb @@ -42,7 +42,6 @@ def configure!(store) store.subscribe ::MergeRequests::UpdateHeadPipelineWorker, to: ::Ci::PipelineCreatedEvent store.subscribe ::Namespaces::UpdateRootStatisticsWorker, to: ::Projects::ProjectDeletedEvent - store.subscribe ::Ci::Runners::UpdateProjectRunnersOwnerWorker, to: ::Projects::ProjectDeletedEvent store.subscribe ::MergeRequests::ProcessAutoMergeFromEventWorker, to: ::MergeRequests::AutoMerge::TitleDescriptionUpdateEvent diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index d197e922f6d1d335ba976dbc9c2ca80398362c9a..5762dc66f1d4431d9dc4dba0f24a63869aff0c5a 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -85,18 +85,18 @@ ci_resources: %w[project_id], p_ci_pipelines: %w[partition_id auto_canceled_by_partition_id auto_canceled_by_id trigger_id], p_ci_runner_machine_builds: %w[project_id], - ci_runner_taggings: %w[runner_id organization_id sharding_key_id], # The organization_id/sharding_key_id values are meant to populate the partitioned table, no other usage. The runner_id FK exists at the partition level - ci_runner_taggings_instance_type: %w[tag_id organization_id sharding_key_id], # organization_id/sharding_key_id are always NULL in this partition, tag_id is handled on ci_runner_taggings - ci_runner_taggings_group_type: %w[tag_id organization_id], # tag_id is handled on ci_runner_taggings. These records are indirectly deleted by the FK to group_type_ci_runners - ci_runner_taggings_project_type: %w[tag_id organization_id], # tag_id is handled on ci_runner_taggings. These records are indirectly deleted by the FK to project_type_ci_runners - ci_runners: %w[sharding_key_id], # This value is meant to populate the partitioned table, no other usage - instance_type_ci_runners: %w[creator_id organization_id sharding_key_id], # No need for LFKs on partition, already handled on ci_runners routing table. - group_type_ci_runners: %w[creator_id organization_id sharding_key_id], # No need for LFKs on partition, already handled on ci_runners routing table. - project_type_ci_runners: %w[creator_id organization_id sharding_key_id], # No need for LFKs on partition, already handled on ci_runners routing table. - ci_runner_machines: %w[runner_id organization_id sharding_key_id], # The runner_id and sharding_key_id fields are only used in the partitions, and have the appropriate FKs. The runner_id field will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/503749. - instance_type_ci_runner_machines: %w[organization_id sharding_key_id], # This field is always NULL in this partition. - group_type_ci_runner_machines: %w[organization_id sharding_key_id], # No need for LFK, rows will be deleted by the FK to ci_runners. - project_type_ci_runner_machines: %w[organization_id sharding_key_id], # No need for LFK, rows will be deleted by the FK to ci_runners. + ci_runner_taggings: %w[runner_id organization_id sharding_key_id], # The organization_id value is meant to populate the partitioned table, no other usage. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. The runner_id FK exists at the partition level + ci_runner_taggings_instance_type: %w[tag_id organization_id sharding_key_id], # organization_id is always NULL in this partition, tag_id is handled on ci_runner_taggings. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650 + ci_runner_taggings_group_type: %w[tag_id organization_id sharding_key_id], # tag_id is handled on ci_runner_taggings. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. These records are indirectly deleted by the FK to group_type_ci_runners + ci_runner_taggings_project_type: %w[tag_id organization_id sharding_key_id], # tag_id is handled on ci_runner_taggings. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. These records are indirectly deleted by the FK to project_type_ci_runners + ci_runners: %w[sharding_key_id], # The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + instance_type_ci_runners: %w[creator_id organization_id sharding_key_id], # No need for LFKs on partition, already handled on ci_runners routing table. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + group_type_ci_runners: %w[creator_id organization_id sharding_key_id], # No need for LFKs on partition, already handled on ci_runners routing table. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + project_type_ci_runners: %w[creator_id organization_id sharding_key_id], # No need for LFKs on partition, already handled on ci_runners routing table. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + ci_runner_machines: %w[runner_id organization_id sharding_key_id], # The organization_id field is only used in the partitions, and have the appropriate FKs. The runner_id field will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/503749. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + instance_type_ci_runner_machines: %w[organization_id sharding_key_id], # This field is always NULL in this partition. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + group_type_ci_runner_machines: %w[organization_id sharding_key_id], # No need for LFK, rows will be deleted by the FK to ci_runners. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. + project_type_ci_runner_machines: %w[organization_id sharding_key_id], # No need for LFK, rows will be deleted by the FK to ci_runners. The sharding_key_id will be dropped in https://gitlab.com/gitlab-org/gitlab/-/issues/547650. ci_runner_projects: %w[runner_id], ci_sources_pipelines: %w[partition_id source_partition_id source_job_id], ci_sources_projects: %w[partition_id], diff --git a/spec/factories/ci/runner_machines.rb b/spec/factories/ci/runner_machines.rb index 82dfa6f83c40926565246efaaa6bb56d20819b87..2ac737a327ab657d6c38795c425407d818af65b5 100644 --- a/spec/factories/ci/runner_machines.rb +++ b/spec/factories/ci/runner_machines.rb @@ -9,7 +9,6 @@ after(:build) do |runner_manager, evaluator| runner_manager.runner_type ||= evaluator.runner.runner_type - runner_manager.sharding_key_id ||= evaluator.runner.sharding_key_id runner_manager.organization_id ||= evaluator.runner.organization_id end diff --git a/spec/factories/ci/runner_namespaces.rb b/spec/factories/ci/runner_namespaces.rb index 9b5dd1abef25ae895045989cf5cf70096d4f9609..5a8f5789295d77a0e934d44d2baa46ddf8220265 100644 --- a/spec/factories/ci/runner_namespaces.rb +++ b/spec/factories/ci/runner_namespaces.rb @@ -9,7 +9,7 @@ runner_namespace.namespace = evaluator.group runner_namespace.runner = build(:ci_runner, :group, runner_namespaces: [runner_namespace], - sharding_key_id: runner_namespace.namespace_id, organization_id: runner_namespace.namespace.organization_id) + organization_id: runner_namespace.namespace.organization_id) end end end diff --git a/spec/factories/ci/runner_tagging.rb b/spec/factories/ci/runner_tagging.rb index eb8fe047243738e1b748f3b6ebc9a1d66cd4acac..67cbf1b344197c7c08539f7b1f7b8b9feb5e592c 100644 --- a/spec/factories/ci/runner_tagging.rb +++ b/spec/factories/ci/runner_tagging.rb @@ -5,7 +5,6 @@ runner factory: :ci_runner tag factory: :ci_tag - sharding_key_id { runner.sharding_key_id } organization_id { runner.organization_id } end end diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb index 35ccb1b2f6ed2fc0ddc69e908b9f89d437d4c0f2..941b65e8d7c51facd8e79d5b1b86da6fcc69309e 100644 --- a/spec/factories/ci/runners.rb +++ b/spec/factories/ci/runners.rb @@ -21,13 +21,11 @@ after(:build) do |runner, evaluator| runner.organization_id ||= evaluator.projects.first&.organization_id if runner.project_type? - runner.sharding_key_id ||= evaluator.projects.first&.id if runner.project_type? evaluator.projects.each do |proj| runner.runner_projects << build(:ci_runner_project, runner: runner, project: proj) end runner.organization_id ||= evaluator.groups.first&.organization_id if runner.group_type? - runner.sharding_key_id ||= evaluator.groups.first&.id if runner.group_type? evaluator.groups.each do |group| runner.runner_namespaces << build(:ci_runner_namespace, runner: runner, namespace: group) end @@ -121,12 +119,6 @@ end after(:build) do |runner, evaluator| - next if runner.sharding_key_id - - # Point to a "no longer existing" project ID, as a project runner must always have been created - # with a sharding key id - runner.sharding_key_id = (2**63) - 1 - runner.organization_id = ::Organizations::Organization::DEFAULT_ORGANIZATION_ID end diff --git a/spec/lib/api/ci/helpers/runner_spec.rb b/spec/lib/api/ci/helpers/runner_spec.rb index 70719a21f67dbf3bcbca188914859676198bb8ee..1d2936752448149a9b94994518ae191209ee010a 100644 --- a/spec/lib/api/ci/helpers/runner_spec.rb +++ b/spec/lib/api/ci/helpers/runner_spec.rb @@ -121,7 +121,6 @@ expect(current_runner_manager.contacted_at).to be_nil expect(current_runner_manager.runner).to eq(runner) expect(current_runner_manager.runner_type).to eq(runner.runner_type) - expect(current_runner_manager.sharding_key_id).to eq(runner.sharding_key_id) # Verify that a second call doesn't raise an error expect { helper.current_runner_manager }.not_to raise_error @@ -137,7 +136,6 @@ expect(current_runner_manager.system_xid).to eq(::API::Ci::Helpers::Runner::LEGACY_SYSTEM_XID) expect(current_runner_manager.runner).to eq(runner) expect(current_runner_manager.runner_type).to eq(runner.runner_type) - expect(current_runner_manager.sharding_key_id).to eq(runner.sharding_key_id) end end end diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index 2bbef7c6ab65660dbb55cef28769dbd0874d1153..8d4c69b4552034d8915ebd5224a9502e7fcb9380 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -35,30 +35,30 @@ # The following tables are work in progress as part of # https://gitlab.com/gitlab-org/gitlab/-/issues/398199 - # TODO: Remove these excepttions once the issue is closed. + # TODO: Remove these exceptions once the issue is closed. let(:uploads_and_partitions) do - [ - "achievement_uploads.namespace_id", - "ai_vectorizable_file_uploads.project_id", - "alert_management_alert_metric_image_uploads.project_id", - "bulk_import_export_upload_uploads.project_id", "bulk_import_export_upload_uploads.namespace_id", - "dependency_list_export_part_uploads.organization_id", - "dependency_list_export_uploads.organization_id", "dependency_list_export_uploads.namespace_id", - "dependency_list_export_uploads.project_id", - "design_management_action_uploads.namespace_id", - "import_export_upload_uploads.project_id", "import_export_upload_uploads.namespace_id", - "issuable_metric_image_uploads.namespace_id", - "namespace_uploads.namespace_id", - "note_uploads.namespace_id", - "organization_detail_uploads.organization_id", - "project_import_export_relation_export_upload_uploads.project_id", - "project_topic_uploads.organization_id", - "project_uploads.project_id", - "snippet_uploads.organization_id", - "vulnerability_export_part_uploads.organization_id", - "vulnerability_export_uploads.organization_id", - "vulnerability_archive_export_uploads.project_id", - "vulnerability_remediation_uploads.project_id" + %w[ + achievement_uploads.namespace_id + ai_vectorizable_file_uploads.project_id + alert_management_alert_metric_image_uploads.project_id + bulk_import_export_upload_uploads.project_id bulk_import_export_upload_uploads.namespace_id + dependency_list_export_part_uploads.organization_id + dependency_list_export_uploads.organization_id dependency_list_export_uploads.namespace_id + dependency_list_export_uploads.project_id + design_management_action_uploads.namespace_id + import_export_upload_uploads.project_id import_export_upload_uploads.namespace_id + issuable_metric_image_uploads.namespace_id + namespace_uploads.namespace_id + note_uploads.namespace_id + organization_detail_uploads.organization_id + project_import_export_relation_export_upload_uploads.project_id + project_topic_uploads.organization_id + project_uploads.project_id + snippet_uploads.organization_id + vulnerability_export_part_uploads.organization_id + vulnerability_export_uploads.organization_id + vulnerability_archive_export_uploads.project_id + vulnerability_remediation_uploads.project_id ] end diff --git a/spec/models/ci/runner_manager_spec.rb b/spec/models/ci/runner_manager_spec.rb index 12b9ebd92080a25e30ab3eb7aef1ad68f31ee3fe..700260eae305e4e46dfc7ef9511d6a07d40ca052 100644 --- a/spec/models/ci/runner_manager_spec.rb +++ b/spec/models/ci/runner_manager_spec.rb @@ -22,7 +22,6 @@ it { is_expected.to validate_presence_of(:system_xid) } it { is_expected.to validate_length_of(:system_xid).is_at_most(64) } it { is_expected.to validate_presence_of(:runner_type).on(:create) } - it { is_expected.to validate_presence_of(:sharding_key_id).on(:create) } it { is_expected.to validate_presence_of(:organization_id).on([:create, :update]) } it { is_expected.to validate_length_of(:version).is_at_most(2048) } it { is_expected.to validate_length_of(:revision).is_at_most(255) } @@ -35,18 +34,6 @@ it { expect(runner_manager).to be_valid } - context 'when sharding_key_id is present' do - let(:runner_manager) do - build(:ci_runner_machine, runner: build(:ci_runner, sharding_key_id: non_existing_record_id)) - end - - it 'is invalid' do - expect(runner_manager).to be_invalid - expect(runner_manager.errors.full_messages).to contain_exactly( - 'Runner manager cannot have sharding_key_id assigned') - end - end - context 'when organization_id is present' do let(:runner_manager) do build(:ci_runner_machine, runner: build(:ci_runner, organization_id: non_existing_record_id)) @@ -76,60 +63,6 @@ end end - describe 'shading_key_id validations' do - let(:runner_manager) { build(:ci_runner_machine, runner: runner) } - - context 'with instance runner' do - let(:runner) { build(:ci_runner, :instance) } - - it { expect(runner).to be_valid } - - context 'when sharding_key_id is not present' do - before do - runner.sharding_key_id = nil - runner_manager.sharding_key_id = nil - end - - it { expect(runner_manager).to be_valid } - end - end - - context 'with group runner' do - let(:runner) { build(:ci_runner, :group, groups: [group]) } - - it { expect(runner_manager).to be_valid } - - context 'when sharding_key_id is not present' do - before do - runner.sharding_key_id = nil - runner_manager.sharding_key_id = nil - end - - it 'adds error to model', :aggregate_failures do - expect(runner_manager).not_to be_valid - expect(runner_manager.errors[:sharding_key_id]).to contain_exactly("can't be blank") - end - end - end - - context 'with project runner' do - let(:runner) { build(:ci_runner, :project, projects: [project]) } - - it { expect(runner).to be_valid } - - context 'when sharding_key_id is not present' do - before do - runner.sharding_key_id = nil - end - - it 'adds error to model', :aggregate_failures do - expect(runner_manager).not_to be_valid - expect(runner_manager.errors[:sharding_key_id]).to contain_exactly("can't be blank") - end - end - end - end - context 'when runner has runtime features' do it 'is valid' do runner_manager = build(:ci_runner_machine, runtime_features: { cancelable: true }) diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 3eb01b9cc43cd8bc93e2e07ebf6718770dd347c0..45aff5a90542c29b2502eab3f525677656eba29e 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -127,7 +127,6 @@ it { is_expected.to validate_presence_of(:access_level) } it { is_expected.to validate_presence_of(:runner_type) } it { is_expected.to validate_presence_of(:registration_type) } - it { is_expected.to validate_presence_of(:sharding_key_id) } it { is_expected.to validate_presence_of(:organization_id).on([:create, :update]) } context 'when runner is instance type' do @@ -135,15 +134,6 @@ it { expect(runner).to be_valid } - context 'when sharding_key_id is present' do - let(:runner) { build(:ci_runner, :instance_type, sharding_key_id: non_existing_record_id) } - - it 'is invalid' do - expect(runner).to be_invalid - expect(runner.errors.full_messages).to contain_exactly('Runner cannot have sharding_key_id assigned') - end - end - context 'when organization_id is present' do let(:runner) { build(:ci_runner, :instance_type, organization_id: non_existing_record_id) } @@ -1614,7 +1604,7 @@ def expect_db_update shared_examples 'a group runner encrypted token' do |prefix| let(:runner_type) { :group_type } - let(:attrs) { { groups: [group], sharding_key_id: group.id } } + let(:attrs) { { groups: [group], organization_id: group.organization_id } } it_behaves_like 'an encrypted routable token for resource', prefix do let(:resource) { group } @@ -1623,7 +1613,7 @@ def expect_db_update shared_examples 'a project runner encrypted token' do |prefix| let(:runner_type) { :project_type } - let(:attrs) { { projects: [project], sharding_key_id: project.id } } + let(:attrs) { { projects: [project], organization_id: project.organization_id } } it_behaves_like 'an encrypted routable token for resource', prefix do let(:resource) { project } @@ -2573,44 +2563,5 @@ def expect_db_update it { is_expected.to contain_exactly(instance_runner, group_runner, project_runner) } end end - - describe '.with_sharding_key' do - subject(:scope) { described_class.with_runner_type(runner_type).with_sharding_key(sharding_key_id) } - - let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } - let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project, other_project]) } - - context 'with group_type' do - let(:runner_type) { 'group_type' } - - context 'when sharding_key_id exists' do - let(:sharding_key_id) { group.id } - - it { is_expected.to contain_exactly(group_runner) } - end - - context 'when sharding_key_id does not exist' do - let(:sharding_key_id) { non_existing_record_id } - - it { is_expected.to eq [] } - end - end - - context 'with project_type' do - let(:runner_type) { 'project_type' } - - context 'when sharding_key_id exists' do - let(:sharding_key_id) { project.id } - - it { is_expected.to contain_exactly(project_runner) } - end - - context 'when sharding_key_id does not exist' do - let(:sharding_key_id) { non_existing_record_id } - - it { is_expected.to eq [] } - end - end - end end end diff --git a/spec/models/ci/runner_tagging_spec.rb b/spec/models/ci/runner_tagging_spec.rb index e21331b476d9da47a529bf104be8af76653a541a..bc6d99db220c0e81a9b22884ae3f0bd606b86a8e 100644 --- a/spec/models/ci/runner_tagging_spec.rb +++ b/spec/models/ci/runner_tagging_spec.rb @@ -10,41 +10,8 @@ describe 'validations' do it { is_expected.to validate_presence_of(:runner_type) } - it { is_expected.to validate_presence_of(:sharding_key_id) } it { is_expected.to validate_presence_of(:organization_id).on([:create, :update]) } - describe 'sharding_key_id' do - subject(:runner_tagging) { runner.taggings.first } - - context 'when runner_type is instance_type' do - let(:runner) { create(:ci_runner, :instance, tag_list: ['postgres']) } - - it { is_expected.to be_valid } - - context 'and sharding_key_id is not nil' do - before do - runner_tagging.sharding_key_id = group.id - end - - it { is_expected.to be_invalid } - end - end - - context 'when runner_type is group_type' do - let(:runner) { create(:ci_runner, :group, groups: [group], tag_list: ['postgres']) } - - it { is_expected.to be_valid } - - context 'and sharding_key_id is nil' do - before do - runner_tagging.sharding_key_id = nil - end - - it { is_expected.to be_invalid } - end - end - end - describe 'organization_id' do subject(:runner_tagging) { runner.taggings.first } @@ -107,30 +74,35 @@ end end - context 'with loose foreign key on namespaces.id' do - it_behaves_like 'cleanup by a loose foreign key' do - let(:model_table_name) { 'ci_runner_taggings_group_type' } - let(:lfk_column) { :sharding_key_id } - let_it_be(:parent) { create(:group) } - let_it_be(:runner) { create(:ci_runner, :group, groups: [parent]) } - let_it_be(:tag) { create(:ci_tag, name: 'ruby') } - let_it_be(:model) do - create(:ci_runner_tagging, runner: runner, runner_type: runner.runner_type, sharding_key_id: parent.id, - tag_id: tag.id) + context 'with loose foreign key on organizations.id' do + context 'with group runner type' do + it_behaves_like 'cleanup by a loose foreign key' do + let(:model_table_name) { 'ci_runner_taggings' } + let(:lfk_column) { :organization_id } + let_it_be(:parent) { create(:organization) } + let_it_be(:group) { create(:group, organization: parent) } + let_it_be(:runner) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:tag) { create(:ci_tag, name: 'ruby') } + let_it_be(:model) do + create(:ci_runner_tagging, runner: runner, runner_type: runner.runner_type, + organization_id: parent.id, tag_id: tag.id) + end end end - end - context 'with loose foreign key on projects.id' do - it_behaves_like 'cleanup by a loose foreign key' do - let(:model_table_name) { 'ci_runner_taggings_project_type' } - let(:lfk_column) { :sharding_key_id } - let_it_be(:parent) { create(:project, group: group) } - let_it_be(:runner) { create(:ci_runner, :project, projects: [parent]) } - let_it_be(:tag) { create(:ci_tag, name: 'ruby') } - let_it_be(:model) do - create(:ci_runner_tagging, runner: runner, runner_type: runner.runner_type, sharding_key_id: parent.id, - tag_id: tag.id) + context 'with project runner type' do + it_behaves_like 'cleanup by a loose foreign key' do + let(:model_table_name) { 'ci_runner_taggings' } + let(:lfk_column) { :organization_id } + let_it_be(:parent) { create(:organization) } + let_it_be(:group) { create(:group, organization: parent) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:tag) { create(:ci_tag, name: 'ruby') } + let_it_be(:model) do + create(:ci_runner_tagging, runner: runner, runner_type: runner.runner_type, + organization_id: parent.id, tag_id: tag.id) + end end end end diff --git a/spec/services/ci/runners/create_runner_service_spec.rb b/spec/services/ci/runners/create_runner_service_spec.rb index 73400c919049ba42db6277614e4983f3eea1a29e..da817f089cd29142b3de8091f77b39473535af5c 100644 --- a/spec/services/ci/runners/create_runner_service_spec.rb +++ b/spec/services/ci/runners/create_runner_service_spec.rb @@ -283,10 +283,6 @@ ) end - it 'populates sharding_key_id correctly' do - expect(runner.sharding_key_id).to eq(group.id) - end - it 'does not track runner creation with maintenance note' do expect { execute }.not_to trigger_internal_events('set_runner_maintenance_note') end @@ -336,10 +332,6 @@ it_behaves_like 'it can create a runner' it_behaves_like 'runner creation transaction behavior' - it 'populates sharding_key_id correctly' do - expect(runner.sharding_key_id).to eq(project.id) - end - context 'when maintenance note is specified' do let(:params) { { runner_type: 'project_type', scope: project, maintenance_note: 'a note' } } diff --git a/spec/services/ci/runners/update_project_runners_owner_service_spec.rb b/spec/services/ci/runners/update_project_runners_owner_service_spec.rb deleted file mode 100644 index c0ef5e1a716376b85963f59136e1c959299ce6d6..0000000000000000000000000000000000000000 --- a/spec/services/ci/runners/update_project_runners_owner_service_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::Ci::Runners::UpdateProjectRunnersOwnerService, '#execute', feature_category: :runner do - let_it_be(:owner_group) { create(:group) } - let_it_be(:owner_project) { create(:project, group: owner_group) } - let_it_be(:new_projects) { create_list(:project, 2) } - - let_it_be(:owned_runner1) do - create(:ci_runner, :project, projects: [owner_project, new_projects.first], tag_list: %w[tag1]) - end - - let_it_be(:owned_runner2) { create(:ci_runner, :project, projects: [owner_project], tag_list: %w[tag1 tag2]) } - let_it_be(:owned_runner3) do - create(:ci_runner, :project, projects: [owner_project, *new_projects.reverse], tag_list: %w[tag2]) - end - - let_it_be(:owned_runner4) do - create(:ci_runner, :project, projects: [owner_project, *new_projects], tag_list: %w[tag1 tag2]) - end - - let_it_be(:other_runner) { create(:ci_runner, :project, projects: new_projects, tag_list: %w[tag1 tag2]) } - let_it_be(:orphaned_runner) { create(:ci_runner, :project, :without_projects, tag_list: %w[tag1 tag2]) } - - let_it_be(:owned_runner1_manager) { create(:ci_runner_machine, runner: owned_runner1) } - let_it_be(:owned_runner2_manager) { create(:ci_runner_machine, runner: owned_runner2) } - let_it_be(:owned_runner3_manager) { create(:ci_runner_machine, runner: owned_runner3) } - let_it_be(:owned_runner4_manager) { create(:ci_runner_machine, runner: owned_runner4) } - - let(:service) { described_class.new(owner_project.id, owner_project.namespace_id) } - - subject(:execute) { service.execute } - - before_all do - owner_project.destroy! - end - - before do - stub_const("#{described_class}::BATCH_SIZE", 2) - end - - it 'updates sharding_key_id on affected runners', :aggregate_failures do - expect { execute } - # owned_runner1's owner project was deleted - .to change { owned_runner1.reload.sharding_key_id }.from(owner_project.id).to(new_projects.first.id) - .and change { owned_runner1_manager.reload.sharding_key_id }.from(owner_project.id).to(new_projects.first.id) - .and change { tagging_sharding_key_id_for_runner(owned_runner1) }.from(owner_project.id).to(new_projects.first.id) - # owned_runner2's owner project was deleted and there were no other associated projects to fall back to - .and change { Ci::Runner.find_by_id(owned_runner2) }.to(nil) # delete, since no other project to adopt it - .and change { Ci::RunnerManager.find_by_id(owned_runner2_manager) }.to(nil) - .and change { Ci::RunnerTagging.find_by_runner_id(owned_runner2) }.to(nil) - # owned_runner3's owner project was deleted - .and change { owned_runner3.reload.sharding_key_id }.from(owner_project.id).to(new_projects.last.id) - .and change { owned_runner3_manager.reload.sharding_key_id }.from(owner_project.id).to(new_projects.last.id) - .and change { tagging_sharding_key_id_for_runner(owned_runner3) }.from(owner_project.id).to(new_projects.last.id) - # owned_runner4's owner project was deleted - .and change { owned_runner4.reload.sharding_key_id }.from(owner_project.id).to(new_projects.first.id) - .and change { owned_runner4_manager.reload.sharding_key_id }.from(owner_project.id).to(new_projects.first.id) - .and change { tagging_sharding_key_id_for_runner(owned_runner4) }.from(owner_project.id).to(new_projects.first.id) - # runners whose owner project was not affected - .and not_change { other_runner.reload.sharding_key_id }.from(new_projects.first.id) - .and not_change { orphaned_runner.reload.sharding_key_id } - .and not_change { tagging_sharding_key_id_for_runner(orphaned_runner) } - - expect(execute).to be_success - end - - private - - def tagging_sharding_key_id_for_runner(runner) - runner.taggings.pluck(:sharding_key_id).uniq.sole - end -end diff --git a/spec/workers/ci/runners/update_project_runners_owner_worker_spec.rb b/spec/workers/ci/runners/update_project_runners_owner_worker_spec.rb deleted file mode 100644 index ab5cbdbc6f02797fce6c2711ab6d972856d6f848..0000000000000000000000000000000000000000 --- a/spec/workers/ci/runners/update_project_runners_owner_worker_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Ci::Runners::UpdateProjectRunnersOwnerWorker, '#handle_event', feature_category: :runner do - let_it_be(:project) { create(:project).tap(&:destroy) } - - let(:project_deleted_event) { Projects::ProjectDeletedEvent.new(data: data) } - let(:data) do - { project_id: project.id, namespace_id: project.namespace_id, root_namespace_id: project.root_namespace.id } - end - - it_behaves_like 'subscribes to event' do - let(:event) { project_deleted_event } - end - - it 'calls Ci::Runners::UpdateProjectRunnersOwnerService' do - expect_next_instance_of( - Ci::Runners::UpdateProjectRunnersOwnerService, project.id, project.namespace_id - ) do |service| - expect(service).to receive(:execute) - end - - described_class.new.handle_event(project_deleted_event) - end -end