diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index bab10606faf8690e9002e0afb52b87351872738a..c6fd345acee240054ff8acef06c3ed344bbefd47 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -224,6 +224,7 @@ class Runner < Ci::ApplicationRecord
scope :with_creator, -> { preload(:creator) }
validate :tag_constraints
+ validates :sharding_key_id, presence: true, on: :create, unless: :instance_type?
validates :name, length: { maximum: 256 }, if: :name_changed?
validates :description, length: { maximum: 1024 }, if: :description_changed?
validates :access_level, presence: true
@@ -231,6 +232,7 @@ 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_groups, unless: :group_type?
validate :any_project, if: :project_type?
validate :exactly_one_group, if: :group_type?
@@ -354,6 +356,7 @@ def assign_to(project, current_user = nil)
begin
transaction do
+ self.sharding_key_id = project.id if self.runner_projects.empty?
self.runner_projects << ::Ci::RunnerProject.new(project: project, runner: self)
self.save!
end
@@ -571,6 +574,12 @@ def tag_constraints
end
end
+ def no_sharding_key_id
+ if sharding_key_id
+ errors.add(:runner, 'cannot have sharding_key_id assigned')
+ end
+ end
+
def no_projects
if runner_projects.any?
errors.add(:runner, 'cannot have projects assigned')
diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb
index da9c555e8fb0ce12a981a8b812547bf535dc25c1..b717dab1b3700c75a18e4cb61b5903fea69d577a 100644
--- a/app/services/ci/runners/register_runner_service.rb
+++ b/app/services/ci/runners/register_runner_service.rb
@@ -44,10 +44,10 @@ def attrs_from_token
{ runner_type: :instance_type }
elsif runner_registrar_valid?('project') && project = ::Project.find_by_runners_token(registration_token)
# Create a project runner
- { runner_type: :project_type, projects: [project] }
+ { runner_type: :project_type, projects: [project], sharding_key_id: project.id }
elsif runner_registrar_valid?('group') && group = ::Group.find_by_runners_token(registration_token)
# Create a group runner
- { runner_type: :group_type, groups: [group] }
+ { runner_type: :group_type, groups: [group], sharding_key_id: group.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 2eae5069046b74b957789f1b9b0e085e33e36846..1c32d3f3d357a3d8214cfb52d4af16adf0c93f57 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
@@ -12,8 +12,11 @@ def initialize(user:, params:)
end
def normalize_params
- params[:runner_type] = 'group_type'
- params[:groups] = [scope]
+ params.merge!({
+ runner_type: 'group_type',
+ sharding_key_id: scope&.id,
+ groups: [scope]
+ })
end
def validate_params
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 487da9965133b214ed98e6c4b798901fd7865f5e..e8649bd8b6545451012d5e3c995beae0ebad47fa 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
@@ -12,8 +12,11 @@ def initialize(user:, params:)
end
def normalize_params
- params[:runner_type] = 'project_type'
- params[:projects] = [scope]
+ params.merge!({
+ runner_type: 'project_type',
+ sharding_key_id: scope&.id,
+ projects: [scope]
+ })
end
def validate_params
diff --git a/db/docs/batched_background_migrations/backfill_sharding_key_id_on_ci_runners.yml b/db/docs/batched_background_migrations/backfill_sharding_key_id_on_ci_runners.yml
new file mode 100644
index 0000000000000000000000000000000000000000..640aba92d7b570f1c750088973bf618e27bdb736
--- /dev/null
+++ b/db/docs/batched_background_migrations/backfill_sharding_key_id_on_ci_runners.yml
@@ -0,0 +1,10 @@
+---
+migration_job_name: BackfillShardingKeyIdOnCiRunners
+description: >-
+ Backfills the `sharding_key_id` column from `ci_runner_namespaces` and `ci_runner_projects`.
+ The column will serve as the sharding key in the future partitioned (by `runner_type`) table.
+feature_category: runner
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/166916
+milestone: '17.5'
+queued_migration_version: 20240923132401
+finalized_by: # version of the migration that finalized this BBM
diff --git a/db/migrate/20240923130542_add_sharding_key_id_to_ci_runners.rb b/db/migrate/20240923130542_add_sharding_key_id_to_ci_runners.rb
new file mode 100644
index 0000000000000000000000000000000000000000..45c1c6b478e3d82a7e81f0b953d677502fa66220
--- /dev/null
+++ b/db/migrate/20240923130542_add_sharding_key_id_to_ci_runners.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class AddShardingKeyIdToCiRunners < Gitlab::Database::Migration[2.2]
+ milestone '17.5'
+ enable_lock_retries!
+
+ def up
+ add_column :ci_runners, :sharding_key_id, :bigint, null: true, if_not_exists: true
+ end
+
+ def down
+ remove_column :ci_runners, :sharding_key_id, if_exists: true
+ end
+end
diff --git a/db/post_migrate/20240923132401_queue_backfill_sharding_key_id_on_ci_runners.rb b/db/post_migrate/20240923132401_queue_backfill_sharding_key_id_on_ci_runners.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b48fb9a1e8d1d0541582195fd37d6ff6085350fe
--- /dev/null
+++ b/db/post_migrate/20240923132401_queue_backfill_sharding_key_id_on_ci_runners.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class QueueBackfillShardingKeyIdOnCiRunners < Gitlab::Database::Migration[2.2]
+ milestone '17.5'
+
+ restrict_gitlab_migration gitlab_schema: :gitlab_ci
+
+ MIGRATION = 'BackfillShardingKeyIdOnCiRunners'
+ DELAY_INTERVAL = 2.minutes
+ BATCH_SIZE = 1000
+ SUB_BATCH_SIZE = 100
+
+ def up
+ queue_batched_background_migration(
+ MIGRATION,
+ :ci_runners,
+ :id,
+ job_interval: DELAY_INTERVAL,
+ batch_size: BATCH_SIZE,
+ sub_batch_size: SUB_BATCH_SIZE
+ )
+ end
+
+ def down
+ delete_batched_background_migration(MIGRATION, :ci_runners, :id, [])
+ end
+end
diff --git a/db/schema_migrations/20240923130542 b/db/schema_migrations/20240923130542
new file mode 100644
index 0000000000000000000000000000000000000000..a915bcb6482725f56b551e90da7601fffb1012b4
--- /dev/null
+++ b/db/schema_migrations/20240923130542
@@ -0,0 +1 @@
+6aea04fdb5dc8a8753d73cfd4c20ecdf4747875460f9081cf9c0f8b9013d2c7c
\ No newline at end of file
diff --git a/db/schema_migrations/20240923132401 b/db/schema_migrations/20240923132401
new file mode 100644
index 0000000000000000000000000000000000000000..cc6cf707b42790de0b2f81e4472277030cc42bce
--- /dev/null
+++ b/db/schema_migrations/20240923132401
@@ -0,0 +1 @@
+9a9d6e31fdbf283db7c22fbd0f7e4dafb6e745dab183330591c4e3e3bb901acd
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 73d5f122a004953213ab00226a46b671218415e2..5bb9afa1fe264210d902f3271416f9932ed6929f 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -8888,6 +8888,7 @@ CREATE TABLE ci_runners (
creator_id bigint,
creation_state smallint DEFAULT 0 NOT NULL,
allowed_plan_ids bigint[] DEFAULT '{}'::bigint[] NOT NULL,
+ sharding_key_id bigint,
CONSTRAINT check_46c685e76f CHECK ((char_length((description)::text) <= 1024)),
CONSTRAINT check_91230910ec CHECK ((char_length((name)::text) <= 256)),
CONSTRAINT check_ce275cee06 CHECK ((char_length(maintainer_note) <= 1024))
diff --git a/lib/gitlab/background_migration/backfill_sharding_key_id_on_ci_runners.rb b/lib/gitlab/background_migration/backfill_sharding_key_id_on_ci_runners.rb
new file mode 100644
index 0000000000000000000000000000000000000000..1e6552d895eb9550605c4d655dcd7e5296abb004
--- /dev/null
+++ b/lib/gitlab/background_migration/backfill_sharding_key_id_on_ci_runners.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module BackgroundMigration
+ class BackfillShardingKeyIdOnCiRunners < BatchedMigrationJob
+ operation_name :backfill_sharding_key_on_ci_runners
+ scope_to ->(relation) { relation.where(runner_type: [2, 3]) }
+
+ feature_category :runner
+
+ UPDATE_GROUP_RUNNERS_SQL = <<~SQL
+ UPDATE ci_runners
+ SET sharding_key_id = ci_runner_namespaces.namespace_id
+ FROM ci_runner_namespaces
+ WHERE ci_runners.id = ci_runner_namespaces.runner_id
+ AND ci_runners.id IN (?);
+ SQL
+
+ UPDATE_PROJECT_RUNNERS_SQL = <<~SQL
+ UPDATE ci_runners
+ SET sharding_key_id = (
+ SELECT ci_runner_projects.project_id
+ FROM ci_runner_projects
+ WHERE ci_runner_projects.runner_id = ci_runners.id
+ ORDER BY ci_runner_projects.id ASC
+ LIMIT 1
+ )
+ FROM ci_runner_projects
+ WHERE ci_runners.id = ci_runner_projects.runner_id
+ AND ci_runners.id IN (?);
+ SQL
+
+ class CiRunner < ::Ci::ApplicationRecord
+ self.table_name = 'ci_runners'
+ end
+
+ def perform
+ each_sub_batch do |sub_batch|
+ sub_batch = sub_batch.where(sharding_key_id: nil).limit(sub_batch_size).select(:id)
+
+ connection.exec_update(CiRunner.sanitize_sql([UPDATE_GROUP_RUNNERS_SQL, sub_batch.where(runner_type: 2)]))
+ connection.exec_update(CiRunner.sanitize_sql([UPDATE_PROJECT_RUNNERS_SQL, sub_batch.where(runner_type: 3)]))
+ end
+ end
+ end
+ end
+end
diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb
index a35647a60c0a89eb7cdeea4420f83cac16eb3af5..03d652803403898f48cf52098ef04c15f7b9cc09 100644
--- a/spec/db/schema_spec.rb
+++ b/spec/db/schema_spec.rb
@@ -91,6 +91,7 @@
ci_unit_test_failures: %w[project_id],
p_ci_pipelines: %w[partition_id auto_canceled_by_partition_id auto_canceled_by_id],
p_ci_runner_machine_builds: %w[project_id],
+ ci_runners: %w[sharding_key_id], # This value is meant to populate the partitioned table, no other usage
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_namespaces.rb b/spec/factories/ci/runner_namespaces.rb
index 3ae30e68ef166c20463e5a8969ac6c8e90fd40e3..3e37adeda506642698958da2d5aa9b67b1ee5313 100644
--- a/spec/factories/ci/runner_namespaces.rb
+++ b/spec/factories/ci/runner_namespaces.rb
@@ -6,8 +6,10 @@
after(:build) do |runner_namespace, evaluator|
if runner_namespace.runner.nil?
- runner_namespace.runner = build(:ci_runner, :group, runner_namespaces: [runner_namespace])
runner_namespace.namespace = evaluator.group
+ runner_namespace.runner =
+ build(:ci_runner, :group, runner_namespaces: [runner_namespace],
+ sharding_key_id: runner_namespace.namespace_id)
end
end
end
diff --git a/spec/factories/ci/runner_projects.rb b/spec/factories/ci/runner_projects.rb
index 35184a416b158140028ff0fcbbd38b438e40e9ae..f1e04f92fc1e2e4f4d7a0730833a088e2ff5f36c 100644
--- a/spec/factories/ci/runner_projects.rb
+++ b/spec/factories/ci/runner_projects.rb
@@ -6,8 +6,9 @@
after(:build) do |runner_project, evaluator|
if runner_project.runner.nil?
- runner_project.runner = build(:ci_runner, :project, runner_projects: [runner_project])
runner_project.project = evaluator.project
+ runner_project.runner =
+ build(:ci_runner, :project, runner_projects: [runner_project], sharding_key_id: evaluator.project.id)
end
end
end
diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb
index 5389ead95a7964ea96cd75c4967879424128a1c3..3241f55a065d6abc45734b074ddf3750b13c5dca 100644
--- a/spec/factories/ci/runners.rb
+++ b/spec/factories/ci/runners.rb
@@ -20,10 +20,12 @@
end
after(:build) do |runner, evaluator|
+ 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.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
@@ -107,6 +109,15 @@
transient do
without_projects { true }
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
+ end
+
after(:create) do |runner, evaluator|
runner.runner_projects.delete_all
end
diff --git a/spec/lib/gitlab/background_migration/backfill_sharding_key_id_on_ci_runners_spec.rb b/spec/lib/gitlab/background_migration/backfill_sharding_key_id_on_ci_runners_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..e4b44d7bf175a11d53826ce6afbbadf070ecf4c9
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/backfill_sharding_key_id_on_ci_runners_spec.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillShardingKeyIdOnCiRunners, schema: 20240923132401,
+ migration: :gitlab_ci, feature_category: :runner do
+ let(:runners) { table(:ci_runners) }
+ let(:runner_projects) { table(:ci_runner_projects) }
+ let(:runner_namespaces) { table(:ci_runner_namespaces) }
+ let(:connection) { Ci::ApplicationRecord.connection }
+
+ let(:args) do
+ min, max = runners.pick('MIN(id)', 'MAX(id)')
+
+ {
+ start_id: min,
+ end_id: max,
+ batch_table: 'ci_runners',
+ batch_column: 'id',
+ sub_batch_size: 100,
+ pause_ms: 0,
+ connection: connection
+ }
+ end
+
+ let(:group_id) { 100 }
+ let(:other_group_id) { 101 }
+ let(:project_id) { 1000 }
+ let(:other_project_id) { 1001 }
+ let!(:orphaned_project_runner) { create_project_runner(project_ids: []) }
+
+ subject(:perform_migration) { described_class.new(**args).perform }
+
+ before do
+ runners.create!(runner_type: 1)
+
+ create_project_runner(project_ids: project_id)
+ create_project_runner(project_ids: other_project_id)
+
+ create_group_runner(group_id: group_id)
+ create_group_runner(group_id: other_group_id)
+ end
+
+ it 'backfills sharding_key_id', :aggregate_failures do
+ expect { perform_migration }.not_to change { orphaned_project_runner.sharding_key_id }
+
+ runners.where(runner_type: 1).find_each do |runner|
+ expect(runner.sharding_key_id).to be_nil
+ end
+
+ runner_namespaces.find_each do |runner_namespace|
+ expect(runners.find(runner_namespace.runner_id).sharding_key_id).to eq(runner_namespace.namespace_id)
+ end
+
+ runner_projects.find_each do |runner_project|
+ expect(runners.find(runner_project.runner_id).sharding_key_id).to eq(runner_project.project_id)
+ end
+ end
+
+ context 'when a project runner is shared with other projects' do
+ let(:runner_project_ids) { [other_project_id, project_id] }
+
+ it 'backfills sharding_key_id with the id of the owner project' do
+ shared_project_runner = create_project_runner(project_ids: runner_project_ids)
+
+ expect do
+ perform_migration
+ end.to change { shared_project_runner.reload.sharding_key_id }.from(nil).to(other_project_id)
+ end
+
+ context 'when the project has a different owner project' do
+ let(:runner_project_ids) { [project_id, other_project_id] }
+
+ it 'backfills sharding_key_id with the id of the owner project' do
+ shared_project_runner = create_project_runner(project_ids: runner_project_ids)
+
+ expect do
+ perform_migration
+ end.to change { shared_project_runner.reload.sharding_key_id }.from(nil).to(project_id)
+ end
+ end
+ end
+
+ def create_group_runner(group_id:)
+ runners.create!(runner_type: 2, sharding_key_id: nil).tap do |runner|
+ runner_namespaces.create!(runner_id: runner.id, namespace_id: group_id)
+ end
+ end
+
+ def create_project_runner(project_ids:)
+ project_ids = Array.wrap(project_ids)
+
+ runners.create!(runner_type: 3, sharding_key_id: nil).tap do |runner|
+ project_ids.each do |project_id|
+ runner_projects.create!(runner_id: runner.id, project_id: project_id)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/database/query_analyzers/prevent_cross_database_modification_spec.rb b/spec/lib/gitlab/database/query_analyzers/prevent_cross_database_modification_spec.rb
index 80c098cea101d00f3d217bdbf42557dcfe42499c..729061e3a0476ec70f865cd82ebb745775717876 100644
--- a/spec/lib/gitlab/database/query_analyzers/prevent_cross_database_modification_spec.rb
+++ b/spec/lib/gitlab/database/query_analyzers/prevent_cross_database_modification_spec.rb
@@ -181,7 +181,7 @@ def run_queries
sql_log_contains: [/UPDATE "projects"/, /SELECT "ci_pipelines"\.\* FROM "ci_pipelines" .*FOR UPDATE/]
context 'when the modification is inside a factory save! call' do
- let(:runner) { create(:ci_runner, :project, projects: [build(:project)]) }
+ let(:runner) { create(:ci_runner, :project, projects: [create(:project)]) }
it 'does not raise an error' do
runner
diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_metric_spec.rb
index 33605783671a322ebc41b9ea17b648ffccd11d73..c1313961202bbf9830d7a68d0c3232796b036c4c 100644
--- a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_metric_spec.rb
+++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_metric_spec.rb
@@ -7,10 +7,7 @@
let(:expected_value) { 1 }
before do
- create(:ci_runner,
- :group,
- groups: [group]
- )
+ create(:ci_runner, :group, groups: [group])
end
it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', data_source: 'database' }
diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_online_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_online_metric_spec.rb
index 24d6ea6f1e9c6165fe5338edb1d2188d1ddeb21d..37d31e325817734cc0ee405bb2e31fc8f97de3a2 100644
--- a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_online_metric_spec.rb
+++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_group_type_active_online_metric_spec.rb
@@ -3,15 +3,11 @@
require 'spec_helper'
RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountCiRunnersGroupTypeActiveOnlineMetric, feature_category: :runner do
- let(:group) { create(:group) }
+ let_it_be(:group) { create(:group) }
let(:expected_value) { 1 }
before do
- create(:ci_runner,
- :group,
- groups: [group],
- contacted_at: 1.second.ago
- )
+ create(:ci_runner, :group, groups: [group], contacted_at: 1.second.ago)
end
it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', data_source: 'database' }
diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_metric_spec.rb
index eeb699c1377ac4c0a4eccdbdd0521ee1ccceba91..811dcfccc0e20ec62aefd360c83577a10a8fbc4b 100644
--- a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_metric_spec.rb
+++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_metric_spec.rb
@@ -3,14 +3,12 @@
require 'spec_helper'
RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountCiRunnersProjectTypeActiveMetric, feature_category: :runner do
- let(:project) { build(:project) }
+ let_it_be(:project) { create(:project) }
+
let(:expected_value) { 1 }
before do
- create(:ci_runner,
- :project,
- projects: [project]
- )
+ create(:ci_runner, :project, projects: [project])
end
it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', data_source: 'database' }
diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_online_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_online_metric_spec.rb
index c3ed752ae0487d048bb98ec53d1bc19c9ae5c624..ecc5b46582e600bf0b76db473b4838a1dbbe6652 100644
--- a/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_online_metric_spec.rb
+++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_ci_runners_project_type_active_online_metric_spec.rb
@@ -3,15 +3,12 @@
require 'spec_helper'
RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountCiRunnersProjectTypeActiveOnlineMetric, feature_category: :runner do
- let(:project) { build(:project) }
+ let_it_be(:project) { create(:project) }
+
let(:expected_value) { 1 }
before do
- create(:ci_runner,
- :project,
- projects: [project],
- contacted_at: 1.second.ago
- )
+ create(:ci_runner, :project, projects: [project], contacted_at: 1.second.ago)
end
it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', data_source: 'database' }
diff --git a/spec/migrations/20240531202753_queue_backfill_or_drop_ci_pipeline_on_project_id_spec.rb b/spec/migrations/20240531202753_queue_backfill_or_drop_ci_pipeline_on_project_id_spec.rb
index e4fe7b1f9726340edb0e6d4cb6d6ba247438ec88..ad02215335ccb62abd3590fa4a58128985712adc 100644
--- a/spec/migrations/20240531202753_queue_backfill_or_drop_ci_pipeline_on_project_id_spec.rb
+++ b/spec/migrations/20240531202753_queue_backfill_or_drop_ci_pipeline_on_project_id_spec.rb
@@ -9,7 +9,7 @@
it 'schedules a new batched migration' do
reversible_migration do |migration|
migration.before -> {
- expect(batched_migration).not_to have_scheduled_batched_migration
+ expect(batched_migration).not_to have_scheduled_batched_migration(gitlab_schema: :gitlab_ci)
}
migration.after -> {
diff --git a/spec/migrations/20240821075507_queue_backfill_ci_job_variables_project_id_spec.rb b/spec/migrations/20240821075507_queue_backfill_ci_job_variables_project_id_spec.rb
index 3bf06909168de0c93821f180aa8ebdb158fb4568..d081e42b8af71e662322b883d0742aa82628d039 100644
--- a/spec/migrations/20240821075507_queue_backfill_ci_job_variables_project_id_spec.rb
+++ b/spec/migrations/20240821075507_queue_backfill_ci_job_variables_project_id_spec.rb
@@ -9,7 +9,7 @@
it 'schedules a new batched migration' do
reversible_migration do |migration|
migration.before -> {
- expect(batched_migration).not_to have_scheduled_batched_migration
+ expect(batched_migration).not_to have_scheduled_batched_migration(gitlab_schema: :gitlab_ci)
}
migration.after -> {
diff --git a/spec/migrations/20240923132401_queue_backfill_sharding_key_id_on_ci_runners_spec.rb b/spec/migrations/20240923132401_queue_backfill_sharding_key_id_on_ci_runners_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..bd35d1569c8a1d5aa1cc0093301a556ecbf63005
--- /dev/null
+++ b/spec/migrations/20240923132401_queue_backfill_sharding_key_id_on_ci_runners_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueBackfillShardingKeyIdOnCiRunners, migration: :gitlab_ci, feature_category: :runner do
+ let!(:batched_migration) { described_class::MIGRATION }
+
+ it 'schedules a new batched migration' do
+ reversible_migration do |migration|
+ migration.before -> {
+ expect(batched_migration).not_to have_scheduled_batched_migration(gitlab_schema: :gitlab_ci)
+ }
+
+ migration.after -> {
+ expect(batched_migration).to have_scheduled_batched_migration(
+ gitlab_schema: :gitlab_ci,
+ table_name: :ci_runners,
+ column_name: :id,
+ interval: described_class::DELAY_INTERVAL,
+ batch_size: described_class::BATCH_SIZE,
+ sub_batch_size: described_class::SUB_BATCH_SIZE
+ )
+ }
+ end
+ end
+end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 3ebbbcfea32585251c5f1657422ebff441fa7f71..07ebc84da758e69f0ebea758f4b28d94b49eca0a 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -101,29 +101,89 @@
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).on(:create) }
+
+ context 'when runner is instance type' do
+ let(:runner) { build(:ci_runner, :instance_type) }
+
+ 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
+ end
context 'when runner is not allowed to pick untagged jobs' do
context 'when runner does not have tags' do
let(:runner) { build(:ci_runner, tag_list: [], run_untagged: false) }
- it 'is not valid' do
- expect(runner).to be_invalid
- end
+ it { expect(runner).to be_invalid }
end
context 'when runner has too many tags' do
let(:runner) { build(:ci_runner, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" }, run_untagged: false) }
- it 'is not valid' do
- expect(runner).to be_invalid
- end
+ it { expect(runner).to be_invalid }
end
context 'when runner has tags' do
let(:runner) { build(:ci_runner, tag_list: ['tag'], run_untagged: false) }
- it 'is valid' do
- expect(runner).to be_valid
+ it { expect(runner).to be_valid }
+ end
+ end
+
+ context 'shading_key_id validations' do
+ 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
+ end
+
+ it { expect(runner).to be_valid }
+ end
+ end
+
+ context 'with group runner' do
+ let(:runner) { build(:ci_runner, :group, groups: [group]) }
+
+ 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).not_to be_valid
+ expect(runner.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).not_to be_valid
+ expect(runner.errors[:sharding_key_id]).to contain_exactly("can't be blank")
+ end
end
end
end
@@ -490,6 +550,10 @@
expect(runner).to be_project_type
expect(runner.runner_projects.pluck(:project_id)).to contain_exactly(project.id, other_project.id)
end
+
+ it 'does not change sharding_key_id' do
+ expect { assign_to }.not_to change { runner.sharding_key_id }.from(other_project.id)
+ 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 efab371482b0789a6d8cc6d4fae91c5e18a2231c..1845ca78f786cb434fe0a8385f186ddf56d574a0 100644
--- a/spec/services/ci/runners/create_runner_service_spec.rb
+++ b/spec/services/ci/runners/create_runner_service_spec.rb
@@ -220,6 +220,10 @@
it_behaves_like 'it can create a runner'
+ it 'populates sharding_key_id correctly' do
+ expect(runner.sharding_key_id).to eq(group.id)
+ end
+
context 'with missing scope param' do
let(:params) { { runner_type: 'group_type' } }
@@ -261,6 +265,10 @@
it_behaves_like 'it can create a runner'
+ it 'populates sharding_key_id correctly' do
+ expect(runner.sharding_key_id).to eq(project.id)
+ end
+
context 'with missing scope param' do
let(:params) { { runner_type: 'project_type' } }