Read about our upcoming Code of Conduct on this issue

Commit 4ac13e05 authored by Kamil Trzciński's avatar Kamil Trzciński
Browse files

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

parent d3474f8c0965
......@@ -74,7 +74,7 @@ def define_runners_variables
.ordered
.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)
......
......@@ -122,7 +122,7 @@ def render_pipeline_status(pipeline, tooltip_placement: 'left')
def no_runners_for_project?(project)
project.runners.blank? &&
Ci::Runner.shared.blank?
Ci::Runner.instance_type.blank?
end
def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body')
......
......@@ -2,6 +2,7 @@ module Ci
class Runner < ActiveRecord::Base
extend Gitlab::Ci::Model
include Gitlab::SQL::Pattern
include IgnorableColumn
include RedisCacheable
include ChronicDurationAttribute
......@@ -11,6 +12,8 @@ class Runner < ActiveRecord::Base
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
ignore_column :is_shared
has_many :builds
has_many :runner_projects, inverse_of: :runner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :runner_projects
......@@ -21,13 +24,16 @@ class Runner < ActiveRecord::Base
before_validation :set_default_values
scope :specific, -> { where(is_shared: false) }
scope :shared, -> { where(is_shared: true) }
scope :active, -> { where(active: true) }
scope :paused, -> { where(active: false) }
scope :online, -> { where('contacted_at > ?', contact_time_deadline) }
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) {
joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
}
......@@ -39,9 +45,9 @@ class Runner < ActiveRecord::Base
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(
[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
)
from("(#{union.to_sql}) ci_runners")
......@@ -63,7 +69,6 @@ class Runner < ActiveRecord::Base
validate :no_groups, unless: :group_type?
validate :any_project, if: :project_type?
validate :exactly_one_group, if: :group_type?
validate :validate_is_shared
acts_as_taggable
......@@ -113,8 +118,7 @@ def set_default_values
end
def assign_to(project, current_user = nil)
if shared?
self.is_shared = false if shared?
if instance_type?
self.runner_type = :project_type
elsif group_type?
raise ArgumentError, 'Transitioning a group runner to a project runner is not supported'
......@@ -137,10 +141,6 @@ def display_name
description
end
def shared?
is_shared
end
def online?
contacted_at && contacted_at > self.class.contact_time_deadline
end
......@@ -159,10 +159,6 @@ def belongs_to_one_project?
runner_projects.count == 1
end
def specific?
!shared?
end
def assigned_to_group?
runner_namespaces.any?
end
......@@ -260,7 +256,7 @@ def tag_constraints
end
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
def no_projects
......@@ -287,12 +283,6 @@ def exactly_one_group
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)
(run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty?
end
......
......@@ -1422,7 +1422,7 @@ def shared_runners_available?
end
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
def group_runners
......
......@@ -1032,7 +1032,7 @@ def ci_owned_runners
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
......
......@@ -4,7 +4,7 @@ class RunnerEntity < Grape::Entity
expose :id, :description
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)
end
......
......@@ -15,7 +15,7 @@ def initialize(runner)
def execute
builds =
if runner.shared?
if runner.instance_type?
builds_for_shared_runner
elsif runner.group_type?
builds_for_group_runner
......@@ -99,7 +99,7 @@ def builds_for_group_runner
end
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')
end
......@@ -115,7 +115,7 @@ def register_failure
end
def register_success(job)
labels = { shared_runner: runner.shared?,
labels = { shared_runner: runner.instance_type?,
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?
......@@ -123,10 +123,10 @@ def register_success(job)
end
def jobs_running_for_project(job)
return '+Inf' unless runner.shared?
return '+Inf' unless runner.instance_type?
# 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
running_jobs_count < JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET ? running_jobs_count : "#{JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET}+"
end
......
%tr{ id: dom_id(runner) }
%td
- if runner.shared?
- if runner.instance_type?
%span.badge.badge-success shared
- elsif runner.group_type?
%span.badge.badge-success group
......@@ -21,7 +21,7 @@
%td
= runner.ip_address
%td
- if runner.shared? || runner.group_type?
- if runner.instance_type? || runner.group_type?
n/a
- else
= runner.projects.count(:all)
......
......@@ -2,7 +2,7 @@
%h3.project-title
Runner ##{@runner.id}
.float-right
- if @runner.shared?
- if @runner.instance_type?
%span.runner-state.runner-state-shared
Shared
- else
......@@ -13,7 +13,7 @@
- breadcrumb_title "##{@runner.id}"
- @no_container = true
- if @runner.shared?
- if @runner.instance_type?
.bs-callout.bs-callout-success
%h4 This Runner will process jobs from ALL UNASSIGNED projects
%p
......
......@@ -26,7 +26,7 @@
- else
- 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'
- 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|
= f.hidden_field :runner_id, value: runner.id
= f.submit _('Enable for this project'), class: 'btn btn-sm'
......
......@@ -3,7 +3,7 @@
%h3.page-title
Runner ##{@runner.id}
.float-right
- if @runner.shared?
- if @runner.instance_type?
%span.runner-state.runner-state-shared
Shared
- elsif @runner.group_type?
......
---
title: Remove the use of `is_shared` of `Ci::Runner`
merge_request:
author:
type: other
......@@ -1010,7 +1010,7 @@ class Runner < Grape::Entity
expose :description
expose :ip_address
expose :active
expose :is_shared
expose :instance_type?, as: :is_shared
expose :name
expose :online?, as: :online
expose :status
......@@ -1024,7 +1024,7 @@ class RunnerDetails < Runner
expose :access_level
expose :version, :revision, :platform, :architecture
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|
if options[:current_user].admin?
runner.projects
......
......@@ -24,13 +24,13 @@ class Runner < Grape::API
attributes =
if runner_registration_token_valid?
# 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])
# 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])
# 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
forbidden!
end
......
......@@ -119,7 +119,7 @@ class Runners < Grape::API
use :pagination
end
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
end
......@@ -180,7 +180,7 @@ def get_runner(id)
end
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)
end
......
......@@ -55,7 +55,7 @@ def runner_hook_attrs(runner)
id: runner.id,
description: runner.description,
active: runner.active?,
is_shared: runner.is_shared?
is_shared: runner.instance_type?
}
end
end
......
......@@ -6,7 +6,6 @@
active true
access_level :not_protected
is_shared true
runner_type :instance_type
trait :online do
......@@ -14,12 +13,10 @@
end
trait :instance do
is_shared true
runner_type :instance_type
end
trait :group do
is_shared false
runner_type :group_type
after(:build) do |runner, evaluator|
......@@ -28,7 +25,6 @@
end
trait :project do
is_shared false
runner_type :project_type
after(:build) do |runner, evaluator|
......
......@@ -105,7 +105,7 @@
end
end
describe '.shared' do
describe '.instance_type' do
let(:group) { create(:group) }
let(:project) { create(:project) }
let!(:group_runner) { create(:ci_runner, :group, groups: [group]) }
......@@ -113,7 +113,7 @@
let!(:shared_runner) { create(:ci_runner, :instance) }
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
......@@ -155,7 +155,7 @@
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
# group specific
group = create(:group)
......@@ -168,7 +168,7 @@
# globally shared
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
)
end
......@@ -202,7 +202,6 @@
it 'transitions shared runner to project runner and assigns project' do
expect(subject).to be_truthy
expect(runner).to be_specific
expect(runner).to be_project_type
expect(runner.projects).to eq([project])
expect(runner.only_for?(project)).to be_truthy
......
......@@ -136,7 +136,7 @@
delete api("/runners/#{unused_project_runner.id}", admin)
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
......@@ -300,7 +300,7 @@ def update_runner(id, user, args)
delete api("/runners/#{shared_runner.id}", admin)
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
it_behaves_like '412 response' do
......@@ -314,7 +314,7 @@ def update_runner(id, user, args)
delete api("/runners/#{project_runner.id}", admin)
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
......@@ -349,7 +349,7 @@ def update_runner(id, user, args)
delete api("/runners/#{project_runner.id}", user)
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
it_behaves_like '412 response' do
......@@ -584,12 +584,12 @@ def update_runner(id, user, args)
end
end
it 'enables a shared runner' do
it 'enables a instance-wide runner' do
expect do
post api("/projects/#{project.id}/runners", admin), runner_id: shared_runner.id
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)
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