Commit a7dab8b6 authored by Avielle Wolfe's avatar Avielle Wolfe Committed by Dylan Griffith

Remove runner <-> projects cross-joins where possible

parent 309f6cd0
......@@ -28,7 +28,7 @@ module Mutations
def authenticate_delete_runner!(runner)
return if current_user.can_admin_all_resources?
"Runner #{runner.to_global_id} associated with more than one project" if runner.projects.count > 1
"Runner #{runner.to_global_id} associated with more than one project" if runner.runner_projects.count > 1
end
def find_object(id)
......
......@@ -246,7 +246,7 @@ module Ci
begin
transaction do
self.projects << project
self.runner_projects << ::Ci::RunnerProject.new(project: project, runner: self)
self.save!
end
rescue ActiveRecord::RecordInvalid => e
......@@ -280,9 +280,7 @@ module Ci
end
def belongs_to_more_than_one_project?
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
self.projects.limit(2).count(:all) > 1
end
runner_projects.limit(2).count(:all) > 1
end
def assigned_to_group?
......@@ -432,12 +430,10 @@ module Ci
end
def no_projects
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
if projects.any?
if runner_projects.any?
errors.add(:runner, 'cannot have projects assigned')
end
end
end
def no_groups
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
......@@ -448,12 +444,10 @@ module Ci
end
def any_project
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
unless projects.any?
unless runner_projects.any?
errors.add(:runner, 'needs to be assigned to at least one project')
end
end
end
def exactly_one_group
::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
......
......@@ -72,7 +72,7 @@ module Clusters
if cluster.group_type?
attributes[:groups] = [group]
elsif cluster.project_type?
attributes[:projects] = [project]
attributes[:runner_projects] = [::Ci::RunnerProject.new(project: project)]
end
attributes
......
......@@ -9,7 +9,7 @@
.row
.col-md-6
%h4= _('Restrict projects for this runner')
- if @runner.projects.any?
- if @runner.runner_projects.any?
%table.table{ data: { testid: 'assigned-projects' } }
%thead
%tr
......
......@@ -39,7 +39,7 @@
- if runner.group_type?
= _('n/a')
- else
= runner.projects.count(:all)
= runner.runner_projects.count(:all)
.table-section.section-5
.table-mobile-header{ role: 'rowheader' }= _('Jobs')
......
......@@ -204,7 +204,7 @@ module API
not_found!('Runner') unless runner_project
runner = runner_project.runner
forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.projects.count == 1
forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.runner_projects.count == 1
destroy_conditionally!(runner_project)
end
......@@ -331,7 +331,7 @@ module API
def authenticate_delete_runner!(runner)
return if current_user.admin?
forbidden!("Runner associated with more than one project") if runner.projects.count > 1
forbidden!("Runner associated with more than one project") if runner.runner_projects.count > 1
forbidden!("No access granted") unless can?(current_user, :delete_runner, runner)
end
......
......@@ -2,7 +2,14 @@
FactoryBot.define do
factory :ci_runner_project, class: 'Ci::RunnerProject' do
runner factory: [:ci_runner, :project]
project
after(:build) do |runner_project, evaluator|
unless runner_project.runner.present?
runner_project.runner = build(
:ci_runner, :project, runner_projects: [runner_project]
)
end
end
end
end
......@@ -10,6 +10,16 @@ FactoryBot.define do
runner_type { :instance_type }
transient do
projects { [] }
end
after(:build) do |runner, evaluator|
evaluator.projects.each do |proj|
runner.runner_projects << build(:ci_runner_project, project: proj)
end
end
trait :online do
contacted_at { Time.now }
end
......@@ -30,7 +40,9 @@ FactoryBot.define do
runner_type { :project_type }
after(:build) do |runner, evaluator|
runner.projects << build(:project) if runner.projects.empty?
if runner.runner_projects.empty?
runner.runner_projects << build(:ci_runner_project)
end
end
end
......
......@@ -271,7 +271,7 @@ RSpec.describe Ci::Runner do
expect(subject).to be_truthy
expect(runner).to be_project_type
expect(runner.projects).to eq([project])
expect(runner.runner_projects.pluck(:project_id)).to match_array([project.id])
expect(runner.only_for?(project)).to be_truthy
end
end
......@@ -735,7 +735,7 @@ RSpec.describe Ci::Runner do
context 'with invalid runner' do
before do
runner.projects = []
runner.runner_projects.delete_all
end
it 'still updates redis cache and database' do
......
......@@ -96,8 +96,9 @@ RSpec.describe Clusters::Applications::Runner do
it 'creates a project runner' do
subject
runner_projects = Project.where(id: runner.runner_projects.pluck(:project_id))
expect(runner).to be_project_type
expect(runner.projects).to eq [project]
expect(runner_projects).to match_array [project]
end
end
......
......@@ -250,7 +250,7 @@ RSpec.describe 'Query.runner(id)' do
end
before do
project_runner2.projects.clear
project_runner2.runner_projects.clear
post_graphql(query, current_user: user)
end
......
......@@ -41,8 +41,8 @@
- "./ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb"
- "./ee/spec/services/ee/merge_requests/refresh_service_spec.rb"
- "./ee/spec/workers/scan_security_report_secrets_worker_spec.rb"
- "./spec/controllers/admin/runners_controller_spec.rb"
- "./spec/controllers/groups/settings/ci_cd_controller_spec.rb"
- "./spec/controllers/admin/runners_controller_spec.rb"
- "./spec/controllers/projects/merge_requests_controller_spec.rb"
- "./spec/controllers/projects/settings/ci_cd_controller_spec.rb"
- "./spec/features/admin/admin_runners_spec.rb"
......@@ -56,7 +56,6 @@
- "./spec/finders/ci/pipelines_for_merge_request_finder_spec.rb"
- "./spec/finders/ci/runners_finder_spec.rb"
- "./spec/frontend/fixtures/runner.rb"
- "./spec/graphql/mutations/ci/runner/delete_spec.rb"
- "./spec/graphql/resolvers/ci/group_runners_resolver_spec.rb"
- "./spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb"
- "./spec/lib/api/entities/package_spec.rb"
......@@ -71,13 +70,11 @@
- "./spec/models/ci/job_artifact_spec.rb"
- "./spec/models/ci/pipeline_spec.rb"
- "./spec/models/ci/runner_spec.rb"
- "./spec/models/clusters/applications/runner_spec.rb"
- "./spec/models/deployment_spec.rb"
- "./spec/models/environment_spec.rb"
- "./spec/models/merge_request_spec.rb"
- "./spec/models/project_spec.rb"
- "./spec/models/user_spec.rb"
- "./spec/presenters/ci/build_runner_presenter_spec.rb"
- "./spec/presenters/ci/pipeline_presenter_spec.rb"
- "./spec/presenters/packages/detail/package_presenter_spec.rb"
- "./spec/requests/api/ci/runner/runners_post_spec.rb"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment