Commit ca93faf1 authored by Kamil Trzciński's avatar Kamil Trzciński

Remove the use of `is_shared` of `Ci::Runner`

parent 7da7af3a
...@@ -74,7 +74,7 @@ module Projects ...@@ -74,7 +74,7 @@ module Projects
.ordered .ordered
.page(params[:page]).per(20) .page(params[:page]).per(20)
@shared_runners = ::Ci::Runner.shared.active @shared_runners = ::Ci::Runner.instance_type.active
@shared_runners_count = @shared_runners.count(:all) @shared_runners_count = @shared_runners.count(:all)
......
...@@ -122,7 +122,7 @@ module CiStatusHelper ...@@ -122,7 +122,7 @@ module CiStatusHelper
def no_runners_for_project?(project) def no_runners_for_project?(project)
project.runners.blank? && project.runners.blank? &&
Ci::Runner.shared.blank? Ci::Runner.instance_type.blank?
end end
def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body') def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body')
......
...@@ -2,6 +2,7 @@ module Ci ...@@ -2,6 +2,7 @@ module Ci
class Runner < ActiveRecord::Base class Runner < ActiveRecord::Base
extend Gitlab::Ci::Model extend Gitlab::Ci::Model
include Gitlab::SQL::Pattern include Gitlab::SQL::Pattern
include IgnorableColumn
include RedisCacheable include RedisCacheable
include ChronicDurationAttribute include ChronicDurationAttribute
...@@ -11,6 +12,8 @@ module Ci ...@@ -11,6 +12,8 @@ module Ci
AVAILABLE_SCOPES = %w[specific shared active paused online].freeze AVAILABLE_SCOPES = %w[specific shared active paused online].freeze
FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze
ignore_column :is_shared
has_many :builds has_many :builds
has_many :runner_projects, inverse_of: :runner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :runner_projects, inverse_of: :runner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :runner_projects has_many :projects, through: :runner_projects
...@@ -21,13 +24,16 @@ module Ci ...@@ -21,13 +24,16 @@ module Ci
before_validation :set_default_values before_validation :set_default_values
scope :specific, -> { where(is_shared: false) }
scope :shared, -> { where(is_shared: true) }
scope :active, -> { where(active: true) } scope :active, -> { where(active: true) }
scope :paused, -> { where(active: false) } scope :paused, -> { where(active: false) }
scope :online, -> { where('contacted_at > ?', contact_time_deadline) } scope :online, -> { where('contacted_at > ?', contact_time_deadline) }
scope :ordered, -> { order(id: :desc) } scope :ordered, -> { order(id: :desc) }
# BACKWARD COMPATIBILITY: There are needed to maintain compatibility with `AVAILABLE_SCOPES` used by `lib/api/runners.rb`
scope :shared, -> { instance_type }
# this should get replaced with `project_type.or(group_type)` once using Rails5
scope :specific, -> { where(runner_type: [runner_types[:project_type], runner_types[:group_type]]) }
scope :belonging_to_project, -> (project_id) { scope :belonging_to_project, -> (project_id) {
joins(:runner_projects).where(ci_runner_projects: { project_id: project_id }) joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
} }
...@@ -39,9 +45,9 @@ module Ci ...@@ -39,9 +45,9 @@ module Ci
joins(:groups).where(namespaces: { id: hierarchy_groups }) joins(:groups).where(namespaces: { id: hierarchy_groups })
} }
scope :owned_or_shared, -> (project_id) do scope :owned_or_instance_wide, -> (project_id) do
union = Gitlab::SQL::Union.new( union = Gitlab::SQL::Union.new(
[belonging_to_project(project_id), belonging_to_parent_group_of_project(project_id), shared], [belonging_to_project(project_id), belonging_to_parent_group_of_project(project_id), instance_type],
remove_duplicates: false remove_duplicates: false
) )
from("(#{union.to_sql}) ci_runners") from("(#{union.to_sql}) ci_runners")
...@@ -63,7 +69,6 @@ module Ci ...@@ -63,7 +69,6 @@ module Ci
validate :no_groups, unless: :group_type? validate :no_groups, unless: :group_type?
validate :any_project, if: :project_type? validate :any_project, if: :project_type?
validate :exactly_one_group, if: :group_type? validate :exactly_one_group, if: :group_type?
validate :validate_is_shared
acts_as_taggable acts_as_taggable
...@@ -113,8 +118,7 @@ module Ci ...@@ -113,8 +118,7 @@ module Ci
end end
def assign_to(project, current_user = nil) def assign_to(project, current_user = nil)
if shared? if instance_type?
self.is_shared = false if shared?
self.runner_type = :project_type self.runner_type = :project_type
elsif group_type? elsif group_type?
raise ArgumentError, 'Transitioning a group runner to a project runner is not supported' raise ArgumentError, 'Transitioning a group runner to a project runner is not supported'
...@@ -137,10 +141,6 @@ module Ci ...@@ -137,10 +141,6 @@ module Ci
description description
end end
def shared?
is_shared
end
def online? def online?
contacted_at && contacted_at > self.class.contact_time_deadline contacted_at && contacted_at > self.class.contact_time_deadline
end end
...@@ -159,10 +159,6 @@ module Ci ...@@ -159,10 +159,6 @@ module Ci
runner_projects.count == 1 runner_projects.count == 1
end end
def specific?
!shared?
end
def assigned_to_group? def assigned_to_group?
runner_namespaces.any? runner_namespaces.any?
end end
...@@ -260,7 +256,7 @@ module Ci ...@@ -260,7 +256,7 @@ module Ci
end end
def assignable_for?(project_id) def assignable_for?(project_id)
self.class.owned_or_shared(project_id).where(id: self.id).any? self.class.owned_or_instance_wide(project_id).where(id: self.id).any?
end end
def no_projects def no_projects
...@@ -287,12 +283,6 @@ module Ci ...@@ -287,12 +283,6 @@ module Ci
end end
end end
def validate_is_shared
unless is_shared? == instance_type?
errors.add(:is_shared, 'is not equal to instance_type?')
end
end
def accepting_tags?(build) def accepting_tags?(build)
(run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty? (run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty?
end end
......
...@@ -1422,7 +1422,7 @@ class Project < ActiveRecord::Base ...@@ -1422,7 +1422,7 @@ class Project < ActiveRecord::Base
end end
def shared_runners def shared_runners
@shared_runners ||= shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none @shared_runners ||= shared_runners_available? ? Ci::Runner.instance_type : Ci::Runner.none
end end
def group_runners def group_runners
......
...@@ -1032,7 +1032,7 @@ class User < ActiveRecord::Base ...@@ -1032,7 +1032,7 @@ class User < ActiveRecord::Base
union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids]) union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids])
Ci::Runner.specific.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection Ci::Runner.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
end end
end end
......
...@@ -4,7 +4,7 @@ class RunnerEntity < Grape::Entity ...@@ -4,7 +4,7 @@ class RunnerEntity < Grape::Entity
expose :id, :description expose :id, :description
expose :edit_path, expose :edit_path,
if: -> (*) { can?(request.current_user, :admin_build, project) && runner.specific? } do |runner| if: -> (*) { can?(request.current_user, :admin_build, project) && runner.project_type? } do |runner|
edit_project_runner_path(project, runner) edit_project_runner_path(project, runner)
end end
......
...@@ -15,7 +15,7 @@ module Ci ...@@ -15,7 +15,7 @@ module Ci
def execute def execute
builds = builds =
if runner.shared? if runner.instance_type?
builds_for_shared_runner builds_for_shared_runner
elsif runner.group_type? elsif runner.group_type?
builds_for_group_runner builds_for_group_runner
...@@ -99,7 +99,7 @@ module Ci ...@@ -99,7 +99,7 @@ module Ci
end end
def running_builds_for_shared_runners def running_builds_for_shared_runners
Ci::Build.running.where(runner: Ci::Runner.shared) Ci::Build.running.where(runner: Ci::Runner.instance_type)
.group(:project_id).select(:project_id, 'count(*) AS running_builds') .group(:project_id).select(:project_id, 'count(*) AS running_builds')
end end
...@@ -115,7 +115,7 @@ module Ci ...@@ -115,7 +115,7 @@ module Ci
end end
def register_success(job) def register_success(job)
labels = { shared_runner: runner.shared?, labels = { shared_runner: runner.instance_type?,
jobs_running_for_project: jobs_running_for_project(job) } jobs_running_for_project: jobs_running_for_project(job) }
job_queue_duration_seconds.observe(labels, Time.now - job.queued_at) unless job.queued_at.nil? job_queue_duration_seconds.observe(labels, Time.now - job.queued_at) unless job.queued_at.nil?
...@@ -123,10 +123,10 @@ module Ci ...@@ -123,10 +123,10 @@ module Ci
end end
def jobs_running_for_project(job) def jobs_running_for_project(job)
return '+Inf' unless runner.shared? return '+Inf' unless runner.instance_type?
# excluding currently started job # excluding currently started job
running_jobs_count = job.project.builds.running.where(runner: Ci::Runner.shared) running_jobs_count = job.project.builds.running.where(runner: Ci::Runner.instance_type)
.limit(JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET + 1).count - 1 .limit(JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET + 1).count - 1
running_jobs_count < JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET ? running_jobs_count : "#{JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET}+" running_jobs_count < JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET ? running_jobs_count : "#{JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET}+"
end end
......
%tr{ id: dom_id(runner) } %tr{ id: dom_id(runner) }
%td %td
- if runner.shared? - if runner.instance_type?
%span.badge.badge-success shared %span.badge.badge-success shared
- elsif runner.group_type? - elsif runner.group_type?
%span.badge.badge-success group %span.badge.badge-success group
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
%td %td
= runner.ip_address = runner.ip_address
%td %td
- if runner.shared? || runner.group_type? - if runner.instance_type? || runner.group_type?
n/a n/a
- else - else
= runner.projects.count(:all) = runner.projects.count(:all)
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
%h3.project-title %h3.project-title
Runner ##{@runner.id} Runner ##{@runner.id}
.float-right .float-right
- if @runner.shared? - if @runner.instance_type?
%span.runner-state.runner-state-shared %span.runner-state.runner-state-shared
Shared Shared
- else - else
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
- breadcrumb_title "##{@runner.id}" - breadcrumb_title "##{@runner.id}"
- @no_container = true - @no_container = true
- if @runner.shared? - if @runner.instance_type?
.bs-callout.bs-callout-success .bs-callout.bs-callout-success
%h4 This Runner will process jobs from ALL UNASSIGNED projects %h4 This Runner will process jobs from ALL UNASSIGNED projects
%p %p
......
...@@ -26,7 +26,7 @@ ...@@ -26,7 +26,7 @@
- else - else
- runner_project = @project.runner_projects.find_by(runner_id: runner) - runner_project = @project.runner_projects.find_by(runner_id: runner)
= link_to _('Disable for this project'), project_runner_project_path(@project, runner_project), data: { confirm: _("Are you sure?") }, method: :delete, class: 'btn btn-danger btn-sm' = link_to _('Disable for this project'), project_runner_project_path(@project, runner_project), data: { confirm: _("Are you sure?") }, method: :delete, class: 'btn btn-danger btn-sm'
- elsif !(runner.is_shared? || runner.group_type?) # We can simplify this to `runner.project_type?` when migrating #runner_type is complete - elsif runner.project_type?
= form_for [@project.namespace.becomes(Namespace), @project, @project.runner_projects.new] do |f| = form_for [@project.namespace.becomes(Namespace), @project, @project.runner_projects.new] do |f|
= f.hidden_field :runner_id, value: runner.id = f.hidden_field :runner_id, value: runner.id
= f.submit _('Enable for this project'), class: 'btn btn-sm' = f.submit _('Enable for this project'), class: 'btn btn-sm'
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
%h3.page-title %h3.page-title
Runner ##{@runner.id} Runner ##{@runner.id}
.float-right .float-right
- if @runner.shared? - if @runner.instance_type?
%span.runner-state.runner-state-shared %span.runner-state.runner-state-shared
Shared Shared
- elsif @runner.group_type? - elsif @runner.group_type?
......
---
title: Remove the use of `is_shared` of `Ci::Runner`
merge_request:
author:
type: other
...@@ -1010,7 +1010,7 @@ module API ...@@ -1010,7 +1010,7 @@ module API
expose :description expose :description
expose :ip_address expose :ip_address
expose :active expose :active
expose :is_shared expose :instance_type?, as: :is_shared
expose :name expose :name
expose :online?, as: :online expose :online?, as: :online
expose :status expose :status
...@@ -1024,7 +1024,7 @@ module API ...@@ -1024,7 +1024,7 @@ module API
expose :access_level expose :access_level
expose :version, :revision, :platform, :architecture expose :version, :revision, :platform, :architecture
expose :contacted_at expose :contacted_at
expose :token, if: lambda { |runner, options| options[:current_user].admin? || !runner.is_shared? } expose :token, if: lambda { |runner, options| options[:current_user].admin? || !runner.instance_type? }
expose :projects, with: Entities::BasicProjectDetails do |runner, options| expose :projects, with: Entities::BasicProjectDetails do |runner, options|
if options[:current_user].admin? if options[:current_user].admin?
runner.projects runner.projects
......
...@@ -24,13 +24,13 @@ module API ...@@ -24,13 +24,13 @@ module API
attributes = attributes =
if runner_registration_token_valid? if runner_registration_token_valid?
# Create shared runner. Requires admin access # Create shared runner. Requires admin access
attributes.merge(is_shared: true, runner_type: :instance_type) attributes.merge(runner_type: :instance_type)
elsif project = Project.find_by(runners_token: params[:token]) elsif project = Project.find_by(runners_token: params[:token])
# Create a specific runner for the project # Create a specific runner for the project
attributes.merge(is_shared: false, runner_type: :project_type, projects: [project]) attributes.merge(runner_type: :project_type, projects: [project])
elsif group = Group.find_by(runners_token: params[:token]) elsif group = Group.find_by(runners_token: params[:token])
# Create a specific runner for the group # Create a specific runner for the group
attributes.merge(is_shared: false, runner_type: :group_type, groups: [group]) attributes.merge(runner_type: :group_type, groups: [group])
else else
forbidden! forbidden!
end end
......
...@@ -119,7 +119,7 @@ module API ...@@ -119,7 +119,7 @@ module API
use :pagination use :pagination
end end
get ':id/runners' do get ':id/runners' do
runners = filter_runners(Ci::Runner.owned_or_shared(user_project.id), params[:scope]) runners = filter_runners(Ci::Runner.owned_or_instance_wide(user_project.id), params[:scope])
present paginate(runners), with: Entities::Runner present paginate(runners), with: Entities::Runner
end end
...@@ -180,7 +180,7 @@ module API ...@@ -180,7 +180,7 @@ module API
end end
def authenticate_show_runner!(runner) def authenticate_show_runner!(runner)
return if runner.is_shared || current_user.admin? return if runner.instance_type? || current_user.admin?
forbidden!("No access granted") unless can?(current_user, :read_runner, runner) forbidden!("No access granted") unless can?(current_user, :read_runner, runner)
end end
......
...@@ -55,7 +55,7 @@ module Gitlab ...@@ -55,7 +55,7 @@ module Gitlab
id: runner.id, id: runner.id,
description: runner.description, description: runner.description,
active: runner.active?, active: runner.active?,
is_shared: runner.is_shared? is_shared: runner.instance_type?
} }
end end
end end
......
...@@ -6,7 +6,6 @@ FactoryBot.define do ...@@ -6,7 +6,6 @@ FactoryBot.define do
active true active true
access_level :not_protected access_level :not_protected
is_shared true
runner_type :instance_type runner_type :instance_type
trait :online do trait :online do
...@@ -14,12 +13,10 @@ FactoryBot.define do ...@@ -14,12 +13,10 @@ FactoryBot.define do
end end
trait :instance do trait :instance do
is_shared true
runner_type :instance_type runner_type :instance_type
end end
trait :group do trait :group do
is_shared false
runner_type :group_type runner_type :group_type
after(:build) do |runner, evaluator| after(:build) do |runner, evaluator|
...@@ -28,7 +25,6 @@ FactoryBot.define do ...@@ -28,7 +25,6 @@ FactoryBot.define do
end end
trait :project do trait :project do
is_shared false
runner_type :project_type runner_type :project_type
after(:build) do |runner, evaluator| after(:build) do |runner, evaluator|
......
...@@ -105,7 +105,7 @@ describe Ci::Runner do ...@@ -105,7 +105,7 @@ describe Ci::Runner do
end end
end end
describe '.shared' do describe '.instance_type' do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:project) } let(:project) { create(:project) }
let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group_runner) { create(:ci_runner, :group, groups: [group]) }
...@@ -113,7 +113,7 @@ describe Ci::Runner do ...@@ -113,7 +113,7 @@ describe Ci::Runner do
let!(:shared_runner) { create(:ci_runner, :instance) } let!(:shared_runner) { create(:ci_runner, :instance) }
it 'returns only shared runners' do it 'returns only shared runners' do
expect(described_class.shared).to contain_exactly(shared_runner) expect(described_class.instance_type).to contain_exactly(shared_runner)
end end
end end
...@@ -155,7 +155,7 @@ describe Ci::Runner do ...@@ -155,7 +155,7 @@ describe Ci::Runner do
end end
end end
describe '.owned_or_shared' do describe '.owned_or_instance_wide' do
it 'returns a globally shared, a project specific and a group specific runner' do it 'returns a globally shared, a project specific and a group specific runner' do
# group specific # group specific
group = create(:group) group = create(:group)
...@@ -168,7 +168,7 @@ describe Ci::Runner do ...@@ -168,7 +168,7 @@ describe Ci::Runner do
# globally shared # globally shared
shared_runner = create(:ci_runner, :instance) shared_runner = create(:ci_runner, :instance)
expect(described_class.owned_or_shared(project.id)).to contain_exactly( expect(described_class.owned_or_instance_wide(project.id)).to contain_exactly(
group_runner, project_runner, shared_runner group_runner, project_runner, shared_runner
) )
end end
...@@ -202,7 +202,6 @@ describe Ci::Runner do ...@@ -202,7 +202,6 @@ describe Ci::Runner do
it 'transitions shared runner to project runner and assigns project' do it 'transitions shared runner to project runner and assigns project' do
expect(subject).to be_truthy expect(subject).to be_truthy
expect(runner).to be_specific
expect(runner).to be_project_type expect(runner).to be_project_type
expect(runner.projects).to eq([project]) expect(runner.projects).to eq([project])
expect(runner.only_for?(project)).to be_truthy expect(runner.only_for?(project)).to be_truthy
......
...@@ -136,7 +136,7 @@ describe API::Runners do ...@@ -136,7 +136,7 @@ describe API::Runners do
delete api("/runners/#{unused_project_runner.id}", admin) delete api("/runners/#{unused_project_runner.id}", admin)
expect(response).to have_gitlab_http_status(204) expect(response).to have_gitlab_http_status(204)
end.to change { Ci::Runner.specific.count }.by(-1) end.to change { Ci::Runner.project_type.count }.by(-1)
end end
end end
...@@ -300,7 +300,7 @@ describe API::Runners do ...@@ -300,7 +300,7 @@ describe API::Runners do
delete api("/runners/#{shared_runner.id}", admin) delete api("/runners/#{shared_runner.id}", admin)
expect(response).to have_gitlab_http_status(204) expect(response).to have_gitlab_http_status(204)
end.to change { Ci::Runner.shared.count }.by(-1) end.to change { Ci::Runner.instance_type.count }.by(-1)
end end
it_behaves_like '412 response' do it_behaves_like '412 response' do
...@@ -314,7 +314,7 @@ describe API::Runners do ...@@ -314,7 +314,7 @@ describe API::Runners do
delete api("/runners/#{project_runner.id}", admin) delete api("/runners/#{project_runner.id}", admin)
expect(response).to have_http_status(204) expect(response).to have_http_status(204)
end.to change { Ci::Runner.specific.count }.by(-1) end.to change { Ci::Runner.project_type.count }.by(-1)
end end
end end
...@@ -349,7 +349,7 @@ describe API::Runners do ...@@ -349,7 +349,7 @@ describe API::Runners do
delete api("/runners/#{project_runner.id}", user) delete api("/runners/#{project_runner.id}", user)
expect(response).to have_http_status(204) expect(response).to have_http_status(204)
end.to change { Ci::Runner.specific.count }.by(-1) end.to change { Ci::Runner.project_type.count }.by(-1)
end end
it_behaves_like '412 response' do it_behaves_like '412 response' do
...@@ -584,12 +584,12 @@ describe API::Runners do ...@@ -584,12 +584,12 @@ describe API::Runners do
end end
end end
it 'enables a shared runner' do it 'enables a instance-wide runner' do
expect do expect do
post api("/projects/#{project.id}/runners", admin), runner_id: shared_runner.id post api("/projects/#{project.id}/runners", admin), runner_id: shared_runner.id
end.to change { project.runners.count }.by(1) end.to change { project.runners.count }.by(1)
expect(shared_runner.reload).not_to be_shared expect(shared_runner.reload).not_to be_instance_type
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
end end
end end
......
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