Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2018-06-01 15:43:43 +0300
committerKamil Trzciński <ayufan@ayufan.eu>2018-06-01 15:43:43 +0300
commitafb2d667cae42b77e090b4b240848c4d4ddcaf2d (patch)
tree6cb66332f5ec2889bbc4e3dbd870a6b65fbe8aa1
parent709e8b263863c5a92959700b67462c2ebe4f1831 (diff)
parent5c6c184f70719c464690455abc02e777b3ba4b7b (diff)
Merge branch '46010-add-more-validations-for-runners-and-runner-type' into 'master'
Improve validations for Ci::Runner#runner_type See merge request gitlab-org/gitlab-ce!18901
-rw-r--r--app/controllers/admin/runner_projects_controller.rb4
-rw-r--r--app/controllers/projects/runner_projects_controller.rb3
-rw-r--r--app/models/ci/runner.rb52
-rw-r--r--app/models/ci/runner_namespace.rb6
-rw-r--r--app/models/ci/runner_project.rb4
-rw-r--r--app/models/clusters/applications/runner.rb5
-rw-r--r--app/models/namespace.rb2
-rw-r--r--app/models/project.rb2
-rw-r--r--app/services/ci/register_job_service.rb5
-rw-r--r--lib/api/runner.rb16
-rw-r--r--lib/api/runners.rb6
-rw-r--r--spec/controllers/groups/runners_controller_spec.rb3
-rw-r--r--spec/controllers/projects/runners_controller_spec.rb3
-rw-r--r--spec/controllers/projects/settings/ci_cd_controller_spec.rb8
-rw-r--r--spec/factories/ci/runner_projects.rb2
-rw-r--r--spec/factories/ci/runners.rb31
-rw-r--r--spec/features/admin/admin_runners_spec.rb19
-rw-r--r--spec/features/runners_spec.rb27
-rw-r--r--spec/finders/runner_jobs_finder_spec.rb2
-rw-r--r--spec/models/ci/build_spec.rb10
-rw-r--r--spec/models/ci/pipeline_spec.rb2
-rw-r--r--spec/models/ci/runner_spec.rb199
-rw-r--r--spec/models/project_spec.rb14
-rw-r--r--spec/models/user_spec.rb40
-rw-r--r--spec/requests/api/runner_spec.rb18
-rw-r--r--spec/requests/api/runners_spec.rb81
-rw-r--r--spec/serializers/runner_entity_spec.rb4
-rw-r--r--spec/services/ci/register_job_service_spec.rb42
-rw-r--r--spec/services/ci/update_build_queue_service_spec.rb17
29 files changed, 327 insertions, 300 deletions
diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb
index 7ed2de71028..7aba77d8129 100644
--- a/app/controllers/admin/runner_projects_controller.rb
+++ b/app/controllers/admin/runner_projects_controller.rb
@@ -4,9 +4,7 @@ class Admin::RunnerProjectsController < Admin::ApplicationController
def create
@runner = Ci::Runner.find(params[:runner_project][:runner_id])
- runner_project = @runner.assign_to(@project, current_user)
-
- if runner_project.persisted?
+ if @runner.assign_to(@project, current_user)
redirect_to admin_runner_path(@runner)
else
redirect_to admin_runner_path(@runner), alert: 'Failed adding runner to project'
diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb
index 0ec2490655f..a080724634b 100644
--- a/app/controllers/projects/runner_projects_controller.rb
+++ b/app/controllers/projects/runner_projects_controller.rb
@@ -9,9 +9,8 @@ class Projects::RunnerProjectsController < Projects::ApplicationController
return head(403) unless can?(current_user, :assign_runner, @runner)
path = project_runners_path(project)
- runner_project = @runner.assign_to(project, current_user)
- if runner_project.persisted?
+ if @runner.assign_to(project, current_user)
redirect_to path
else
redirect_to path, alert: 'Failed adding runner to project'
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 530eacf4be0..57edd6a4956 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -12,9 +12,9 @@ module Ci
FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze
has_many :builds
- has_many :runner_projects, 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 :runner_namespaces
+ has_many :runner_namespaces, inverse_of: :runner
has_many :groups, through: :runner_namespaces
has_one :last_build, ->() { order('id DESC') }, class_name: 'Ci::Build'
@@ -56,10 +56,15 @@ module Ci
end
validate :tag_constraints
- validate :either_projects_or_group
validates :access_level, presence: true
validates :runner_type, presence: true
+ validate :no_projects, unless: :project_type?
+ 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
after_destroy :cleanup_runner_queue
@@ -115,8 +120,15 @@ module Ci
raise ArgumentError, 'Transitioning a group runner to a project runner is not supported'
end
- self.save
- project.runner_projects.create(runner_id: self.id)
+ begin
+ transaction do
+ self.projects << project
+ self.save!
+ end
+ rescue ActiveRecord::RecordInvalid => e
+ self.errors.add(:assign_to, e.message)
+ false
+ end
end
def display_name
@@ -253,13 +265,33 @@ module Ci
self.class.owned_or_shared(project_id).where(id: self.id).any?
end
- def either_projects_or_group
- if groups.many?
- errors.add(:runner, 'can only be assigned to one group')
+ def no_projects
+ if projects.any?
+ errors.add(:runner, 'cannot have projects assigned')
+ end
+ end
+
+ def no_groups
+ if groups.any?
+ errors.add(:runner, 'cannot have groups assigned')
+ end
+ end
+
+ def any_project
+ unless projects.any?
+ errors.add(:runner, 'needs to be assigned to at least one project')
+ end
+ end
+
+ def exactly_one_group
+ unless groups.one?
+ errors.add(:runner, 'needs to be assigned to exactly one group')
end
+ end
- if assigned_to_group? && assigned_to_project?
- errors.add(:runner, 'can only be assigned either to projects or to a group')
+ def validate_is_shared
+ unless is_shared? == instance_type?
+ errors.add(:is_shared, 'is not equal to instance_type?')
end
end
diff --git a/app/models/ci/runner_namespace.rb b/app/models/ci/runner_namespace.rb
index 3269f86e8ca..29508fdd326 100644
--- a/app/models/ci/runner_namespace.rb
+++ b/app/models/ci/runner_namespace.rb
@@ -2,8 +2,10 @@ module Ci
class RunnerNamespace < ActiveRecord::Base
extend Gitlab::Ci::Model
- belongs_to :runner
- belongs_to :namespace, class_name: '::Namespace'
+ belongs_to :runner, inverse_of: :runner_namespaces, validate: true
+ belongs_to :namespace, inverse_of: :runner_namespaces, class_name: '::Namespace'
belongs_to :group, class_name: '::Group', foreign_key: :namespace_id
+
+ validates :runner_id, uniqueness: { scope: :namespace_id }
end
end
diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb
index 505d178ba8e..52437047300 100644
--- a/app/models/ci/runner_project.rb
+++ b/app/models/ci/runner_project.rb
@@ -2,8 +2,8 @@ module Ci
class RunnerProject < ActiveRecord::Base
extend Gitlab::Ci::Model
- belongs_to :runner
- belongs_to :project
+ belongs_to :runner, inverse_of: :runner_projects
+ belongs_to :project, inverse_of: :runner_projects
validates :runner_id, uniqueness: { scope: :project_id }
end
diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb
index b881b4eaf36..e6f795f3e0b 100644
--- a/app/models/clusters/applications/runner.rb
+++ b/app/models/clusters/applications/runner.rb
@@ -43,7 +43,7 @@ module Clusters
def create_and_assign_runner
transaction do
- project.runners.create!(runner_create_params).tap do |runner|
+ Ci::Runner.create!(runner_create_params).tap do |runner|
update!(runner_id: runner.id)
end
end
@@ -53,7 +53,8 @@ module Clusters
{
name: 'kubernetes-cluster',
runner_type: :project_type,
- tag_list: %w(kubernetes cluster)
+ tag_list: %w(kubernetes cluster),
+ projects: [project]
}
end
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 3dad4277713..52fe529c016 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -21,7 +21,7 @@ class Namespace < ActiveRecord::Base
has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :project_statistics
- has_many :runner_namespaces, class_name: 'Ci::RunnerNamespace'
+ has_many :runner_namespaces, inverse_of: :namespace, class_name: 'Ci::RunnerNamespace'
has_many :runners, through: :runner_namespaces, source: :runner, class_name: 'Ci::Runner'
# This should _not_ be `inverse_of: :namespace`, because that would also set
diff --git a/app/models/project.rb b/app/models/project.rb
index af9fca62dc3..32298fc7f5c 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -236,7 +236,7 @@ class Project < ActiveRecord::Base
has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName'
has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks
- has_many :runner_projects, class_name: 'Ci::RunnerProject'
+ has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project
has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
has_many :variables, class_name: 'Ci::Variable'
has_many :triggers, class_name: 'Ci::Trigger'
diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb
index 4291631913a..317d1defbba 100644
--- a/app/services/ci/register_job_service.rb
+++ b/app/services/ci/register_job_service.rb
@@ -89,7 +89,10 @@ module Ci
end
def builds_for_group_runner
- hierarchy_groups = Gitlab::GroupHierarchy.new(runner.groups).base_and_descendants
+ # Workaround for weird Rails bug, that makes `runner.groups.to_sql` to return `runner_id = NULL`
+ groups = Group.joins(:runner_namespaces).merge(runner.runner_namespaces)
+
+ hierarchy_groups = Gitlab::GroupHierarchy.new(groups).base_and_descendants
projects = Project.where(namespace_id: hierarchy_groups)
.with_group_runners_enabled
.with_builds_enabled
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 5b7ae89440c..e9886c76870 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -21,24 +21,26 @@ module API
attributes = attributes_for_keys([:description, :active, :locked, :run_untagged, :tag_list, :maximum_timeout])
.merge(get_runner_details_from_request)
- runner =
+ attributes =
if runner_registration_token_valid?
# Create shared runner. Requires admin access
- Ci::Runner.create(attributes.merge(is_shared: true, runner_type: :instance_type))
+ attributes.merge(is_shared: true, runner_type: :instance_type)
elsif project = Project.find_by(runners_token: params[:token])
# Create a specific runner for the project
- project.runners.create(attributes.merge(runner_type: :project_type))
+ attributes.merge(is_shared: false, runner_type: :project_type, projects: [project])
elsif group = Group.find_by(runners_token: params[:token])
# Create a specific runner for the group
- group.runners.create(attributes.merge(runner_type: :group_type))
+ attributes.merge(is_shared: false, runner_type: :group_type, groups: [group])
+ else
+ forbidden!
end
- break forbidden! unless runner
+ runner = Ci::Runner.create(attributes)
- if runner.id
+ if runner.persisted?
present runner, with: Entities::RunnerRegistrationDetails
else
- not_found!
+ render_validation_error!(runner)
end
end
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index 5cb96d467c0..2b78075ddbf 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -133,12 +133,10 @@ module API
runner = get_runner(params[:runner_id])
authenticate_enable_runner!(runner)
- runner_project = runner.assign_to(user_project)
-
- if runner_project.persisted?
+ if runner.assign_to(user_project)
present runner, with: Entities::Runner
else
- conflict!("Runner was already enabled for this project")
+ render_validation_error!(runner)
end
end
diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb
index 6d31b0ce959..5770d15557c 100644
--- a/spec/controllers/groups/runners_controller_spec.rb
+++ b/spec/controllers/groups/runners_controller_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
describe Groups::RunnersController do
let(:user) { create(:user) }
let(:group) { create(:group) }
- let(:runner) { create(:ci_runner) }
+ let(:runner) { create(:ci_runner, :group, groups: [group]) }
let(:params) do
{
@@ -15,7 +15,6 @@ describe Groups::RunnersController do
before do
sign_in(user)
group.add_master(user)
- group.runners << runner
end
describe '#update' do
diff --git a/spec/controllers/projects/runners_controller_spec.rb b/spec/controllers/projects/runners_controller_spec.rb
index 89a13f3c976..2082dd2cff0 100644
--- a/spec/controllers/projects/runners_controller_spec.rb
+++ b/spec/controllers/projects/runners_controller_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
describe Projects::RunnersController do
let(:user) { create(:user) }
let(:project) { create(:project) }
- let(:runner) { create(:ci_runner) }
+ let(:runner) { create(:ci_runner, :project, projects: [project]) }
let(:params) do
{
@@ -16,7 +16,6 @@ describe Projects::RunnersController do
before do
sign_in(user)
project.add_master(user)
- project.runners << runner
end
describe '#update' do
diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
index f1810763d2d..d53fe9bf734 100644
--- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
@@ -19,12 +19,12 @@ describe Projects::Settings::CiCdController do
end
context 'with group runners' do
- let(:group_runner) { create(:ci_runner, runner_type: :group_type) }
let(:parent_group) { create(:group) }
- let(:group) { create(:group, runners: [group_runner], parent: parent_group) }
+ let(:group) { create(:group, parent: parent_group) }
+ let(:group_runner) { create(:ci_runner, :group, groups: [group]) }
let(:other_project) { create(:project, group: group) }
- let!(:project_runner) { create(:ci_runner, projects: [other_project], runner_type: :project_type) }
- let!(:shared_runner) { create(:ci_runner, :shared) }
+ let!(:project_runner) { create(:ci_runner, :project, projects: [other_project]) }
+ let!(:shared_runner) { create(:ci_runner, :instance) }
it 'sets assignable project runners only' do
group.add_master(user)
diff --git a/spec/factories/ci/runner_projects.rb b/spec/factories/ci/runner_projects.rb
index f605e90ceed..ec15972c423 100644
--- a/spec/factories/ci/runner_projects.rb
+++ b/spec/factories/ci/runner_projects.rb
@@ -1,6 +1,6 @@
FactoryBot.define do
factory :ci_runner_project, class: Ci::RunnerProject do
- runner factory: :ci_runner
+ runner factory: [:ci_runner, :project]
project
end
end
diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb
index cdc170b9ccb..6fb621b5e51 100644
--- a/spec/factories/ci/runners.rb
+++ b/spec/factories/ci/runners.rb
@@ -3,22 +3,45 @@ FactoryBot.define do
sequence(:description) { |n| "My runner#{n}" }
platform "darwin"
- is_shared false
active true
access_level :not_protected
- runner_type :project_type
+
+ is_shared true
+ runner_type :instance_type
trait :online do
contacted_at Time.now
end
- trait :shared do
+ trait :instance do
is_shared true
runner_type :instance_type
end
- trait :specific do
+ trait :group do
+ is_shared false
+ runner_type :group_type
+
+ after(:build) do |runner, evaluator|
+ runner.groups << build(:group) if runner.groups.empty?
+ end
+ end
+
+ trait :project do
is_shared false
+ runner_type :project_type
+
+ after(:build) do |runner, evaluator|
+ runner.projects << build(:project) if runner.projects.empty?
+ end
+ end
+
+ trait :without_projects do
+ # we use that to create invalid runner:
+ # the one without projects
+ after(:create) do |runner, evaluator|
+ runner.runner_projects.delete_all
+ end
end
trait :inactive do
diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb
index c33014cbb31..be8754a5315 100644
--- a/spec/features/admin/admin_runners_spec.rb
+++ b/spec/features/admin/admin_runners_spec.rb
@@ -62,7 +62,7 @@ describe "Admin Runners" do
context 'group runner' do
let(:group) { create(:group) }
- let!(:runner) { create(:ci_runner, groups: [group], runner_type: :group_type) }
+ let!(:runner) { create(:ci_runner, :group, groups: [group]) }
it 'shows the label and does not show the project count' do
visit admin_runners_path
@@ -76,7 +76,7 @@ describe "Admin Runners" do
context 'shared runner' do
it 'shows the label and does not show the project count' do
- runner = create :ci_runner, :shared
+ runner = create :ci_runner, :instance
visit admin_runners_path
@@ -90,7 +90,7 @@ describe "Admin Runners" do
context 'specific runner' do
it 'shows the label and the project count' do
project = create :project
- runner = create :ci_runner, projects: [project]
+ runner = create :ci_runner, :project, projects: [project]
visit admin_runners_path
@@ -149,8 +149,9 @@ describe "Admin Runners" do
end
context 'with specific runner' do
+ let(:runner) { create(:ci_runner, :project, projects: [@project1]) }
+
before do
- @project1.runners << runner
visit admin_runner_path(runner)
end
@@ -158,9 +159,9 @@ describe "Admin Runners" do
end
context 'with locked runner' do
+ let(:runner) { create(:ci_runner, :project, projects: [@project1], locked: true) }
+
before do
- runner.update(locked: true)
- @project1.runners << runner
visit admin_runner_path(runner)
end
@@ -168,9 +169,10 @@ describe "Admin Runners" do
end
context 'with shared runner' do
+ let(:runner) { create(:ci_runner, :instance) }
+
before do
@project1.destroy
- runner.update(is_shared: true)
visit admin_runner_path(runner)
end
@@ -179,8 +181,9 @@ describe "Admin Runners" do
end
describe 'disable/destroy' do
+ let(:runner) { create(:ci_runner, :project, projects: [@project1]) }
+
before do
- @project1.runners << runner
visit admin_runner_path(runner)
end
diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb
index e0cd963fe39..9ce7d538004 100644
--- a/spec/features/runners_spec.rb
+++ b/spec/features/runners_spec.rb
@@ -28,12 +28,8 @@ feature 'Runners' do
project.add_master(user)
end
- context 'when a specific runner is activated on the project' do
- given(:specific_runner) { create(:ci_runner, :specific) }
-
- background do
- project.runners << specific_runner
- end
+ context 'when a project_type runner is activated on the project' do
+ given!(:specific_runner) { create(:ci_runner, :project, projects: [project]) }
scenario 'user sees the specific runner' do
visit project_runners_path(project)
@@ -114,7 +110,7 @@ feature 'Runners' do
end
context 'when a shared runner is activated on the project' do
- given!(:shared_runner) { create(:ci_runner, :shared) }
+ given!(:shared_runner) { create(:ci_runner, :instance) }
scenario 'user sees CI/CD setting page' do
visit project_runners_path(project)
@@ -126,11 +122,10 @@ feature 'Runners' do
context 'when a specific runner exists in another project' do
given(:another_project) { create(:project) }
- given(:specific_runner) { create(:ci_runner, :specific) }
+ given!(:specific_runner) { create(:ci_runner, :project, projects: [another_project]) }
background do
another_project.add_master(user)
- another_project.runners << specific_runner
end
scenario 'user enables and disables a specific runner' do
@@ -220,8 +215,8 @@ feature 'Runners' do
end
context 'project with a group but no group runner' do
- given(:group) { create :group }
- given(:project) { create :project, group: group }
+ given(:group) { create(:group) }
+ given(:project) { create(:project, group: group) }
scenario 'group runners are not available' do
visit project_runners_path(project)
@@ -234,9 +229,9 @@ feature 'Runners' do
end
context 'project with a group and a group runner' do
- given(:group) { create :group }
- given(:project) { create :project, group: group }
- given!(:ci_runner) { create :ci_runner, groups: [group], description: 'group-runner' }
+ given(:group) { create(:group) }
+ given(:project) { create(:project, group: group) }
+ given!(:ci_runner) { create(:ci_runner, :group, groups: [group], description: 'group-runner') }
scenario 'group runners are available' do
visit project_runners_path(project)
@@ -263,7 +258,7 @@ feature 'Runners' do
end
context 'group runners in group settings' do
- given(:group) { create :group }
+ given(:group) { create(:group) }
background do
group.add_master(user)
end
@@ -277,7 +272,7 @@ feature 'Runners' do
end
context 'group with a runner' do
- let!(:runner) { create :ci_runner, groups: [group], description: 'group-runner' }
+ let!(:runner) { create(:ci_runner, :group, groups: [group], description: 'group-runner') }
scenario 'the runner is visible' do
visit group_settings_ci_cd_path(group)
diff --git a/spec/finders/runner_jobs_finder_spec.rb b/spec/finders/runner_jobs_finder_spec.rb
index 4275b1a7ff1..97304170c4e 100644
--- a/spec/finders/runner_jobs_finder_spec.rb
+++ b/spec/finders/runner_jobs_finder_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
describe RunnerJobsFinder do
let(:project) { create(:project) }
- let(:runner) { create(:ci_runner, :shared) }
+ let(:runner) { create(:ci_runner, :instance) }
subject { described_class.new(runner, params).execute }
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 77179028ede..b5eac913639 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -148,10 +148,9 @@ describe Ci::Build do
end
context 'when there are runners' do
- let(:runner) { create(:ci_runner) }
+ let(:runner) { create(:ci_runner, :project, projects: [build.project]) }
before do
- build.project.runners << runner
runner.update_attributes(contacted_at: 1.second.ago)
end
@@ -1388,12 +1387,7 @@ describe Ci::Build do
it { is_expected.to be_truthy }
context "and there are specific runner" do
- let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) }
-
- before do
- build.project.runners << runner
- runner.save
- end
+ let!(:runner) { create(:ci_runner, :project, projects: [build.project], contacted_at: 1.second.ago) }
it { is_expected.to be_falsey }
end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index e4f4c62bd22..f5295bec65b 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1589,7 +1589,7 @@ describe Ci::Pipeline, :mailer do
context 'when pipeline is not stuck' do
before do
- create(:ci_runner, :shared, :online)
+ create(:ci_runner, :instance, :online)
end
it 'is not stuck' do
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 0fbc934f669..0f072aa1719 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -21,60 +21,58 @@ describe Ci::Runner do
end
end
- context 'either_projects_or_group' do
+ context '#exactly_one_group' do
let(:group) { create(:group) }
+ let(:runner) { create(:ci_runner, :group, groups: [group]) }
- it 'disallows assigning to a group if already assigned to a group' do
- runner = create(:ci_runner, groups: [group])
-
+ it 'disallows assigning group if already assigned to a group' do
runner.groups << build(:group)
expect(runner).not_to be_valid
- expect(runner.errors.full_messages).to eq ['Runner can only be assigned to one group']
+ expect(runner.errors.full_messages).to include('Runner needs to be assigned to exactly one group')
end
+ end
- it 'disallows assigning to a group if already assigned to a project' do
- project = create(:project)
- runner = create(:ci_runner, projects: [project])
+ context 'runner_type validations' do
+ set(:group) { create(:group) }
+ set(:project) { create(:project) }
+ let(:group_runner) { create(:ci_runner, :group, groups: [group]) }
+ let(:project_runner) { create(:ci_runner, :project, projects: [project]) }
+ let(:instance_runner) { create(:ci_runner, :instance) }
- runner.groups << build(:group)
+ it 'disallows assigning group to project_type runner' do
+ project_runner.groups << build(:group)
- expect(runner).not_to be_valid
- expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group']
+ expect(project_runner).not_to be_valid
+ expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned')
end
- it 'disallows assigning to a project if already assigned to a group' do
- runner = create(:ci_runner, groups: [group])
-
- runner.projects << build(:project)
+ it 'disallows assigning group to instance_type runner' do
+ instance_runner.groups << build(:group)
- expect(runner).not_to be_valid
- expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group']
+ expect(instance_runner).not_to be_valid
+ expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned')
end
- it 'allows assigning to a group if not assigned to a group nor a project' do
- runner = create(:ci_runner)
-
- runner.groups << build(:group)
+ it 'disallows assigning project to group_type runner' do
+ group_runner.projects << build(:project)
- expect(runner).to be_valid
+ expect(group_runner).not_to be_valid
+ expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned')
end
- it 'allows assigning to a project if not assigned to a group nor a project' do
- runner = create(:ci_runner)
-
- runner.projects << build(:project)
+ it 'disallows assigning project to instance_type runner' do
+ instance_runner.projects << build(:project)
- expect(runner).to be_valid
+ expect(instance_runner).not_to be_valid
+ expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned')
end
- it 'allows assigning to a project if already assigned to a project' do
- project = create(:project)
- runner = create(:ci_runner, projects: [project])
-
- runner.projects << build(:project)
+ it 'should fail to save a group assigned to a project runner even if the runner is already saved' do
+ group_runner
- expect(runner).to be_valid
+ expect { create(:group, runners: [project_runner]) }
+ .to raise_error(ActiveRecord::RecordInvalid)
end
end
end
@@ -110,17 +108,12 @@ describe Ci::Runner do
describe '.shared' do
let(:group) { create(:group) }
let(:project) { create(:project) }
+ let!(:group_runner) { create(:ci_runner, :group, groups: [group]) }
+ let!(:project_runner) { create(:ci_runner, :project, projects: [project]) }
+ let!(:shared_runner) { create(:ci_runner, :instance) }
- it 'returns the shared group runner' do
- runner = create(:ci_runner, :shared, groups: [group])
-
- expect(described_class.shared).to eq [runner]
- end
-
- it 'returns the shared project runner' do
- runner = create(:ci_runner, :shared, projects: [project])
-
- expect(described_class.shared).to eq [runner]
+ it 'returns only shared runners' do
+ expect(described_class.shared).to contain_exactly(shared_runner)
end
end
@@ -128,11 +121,11 @@ describe Ci::Runner do
it 'returns the specific project runner' do
# own
specific_project = create(:project)
- specific_runner = create(:ci_runner, :specific, projects: [specific_project])
+ specific_runner = create(:ci_runner, :project, projects: [specific_project])
# other
other_project = create(:project)
- create(:ci_runner, :specific, projects: [other_project])
+ create(:ci_runner, :project, projects: [other_project])
expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner]
end
@@ -141,17 +134,17 @@ describe Ci::Runner do
describe '.belonging_to_parent_group_of_project' do
let(:project) { create(:project, group: group) }
let(:group) { create(:group) }
- let(:runner) { create(:ci_runner, :specific, groups: [group]) }
+ let(:runner) { create(:ci_runner, :group, groups: [group]) }
let!(:unrelated_group) { create(:group) }
let!(:unrelated_project) { create(:project, group: unrelated_group) }
- let!(:unrelated_runner) { create(:ci_runner, :specific, groups: [unrelated_group]) }
+ let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_group]) }
it 'returns the specific group runner' do
expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner)
end
context 'with a parent group with a runner', :nested_groups do
- let(:runner) { create(:ci_runner, :specific, groups: [parent_group]) }
+ let(:runner) { create(:ci_runner, :group, groups: [parent_group]) }
let(:project) { create(:project, group: group) }
let(:group) { create(:group, parent: parent_group) }
let(:parent_group) { create(:group) }
@@ -167,13 +160,13 @@ describe Ci::Runner do
# group specific
group = create(:group)
project = create(:project, group: group)
- group_runner = create(:ci_runner, :specific, groups: [group])
+ group_runner = create(:ci_runner, :group, groups: [group])
# project specific
- project_runner = create(:ci_runner, :specific, projects: [project])
+ project_runner = create(:ci_runner, :project, projects: [project])
# globally shared
- shared_runner = create(:ci_runner, :shared)
+ shared_runner = create(:ci_runner, :instance)
expect(described_class.owned_or_shared(project.id)).to contain_exactly(
group_runner, project_runner, shared_runner
@@ -183,31 +176,32 @@ describe Ci::Runner do
describe '#display_name' do
it 'returns the description if it has a value' do
- runner = FactoryBot.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
+ runner = build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448'
end
it 'returns the token if it does not have a description' do
- runner = FactoryBot.create(:ci_runner)
+ runner = create(:ci_runner)
expect(runner.display_name).to eq runner.description
end
it 'returns the token if the description is an empty string' do
- runner = FactoryBot.build(:ci_runner, description: '', token: 'token')
+ runner = build(:ci_runner, description: '', token: 'token')
expect(runner.display_name).to eq runner.token
end
end
describe '#assign_to' do
- let!(:project) { FactoryBot.create(:project) }
+ let(:project) { create(:project) }
subject { runner.assign_to(project) }
context 'with shared_runner' do
- let!(:runner) { FactoryBot.create(:ci_runner, :shared) }
+ let(:runner) { create(:ci_runner, :instance) }
it 'transitions shared runner to project runner and assigns project' do
- subject
+ expect(subject).to be_truthy
+
expect(runner).to be_specific
expect(runner).to be_project_type
expect(runner.projects).to eq([project])
@@ -216,7 +210,8 @@ describe Ci::Runner do
end
context 'with group runner' do
- let!(:runner) { FactoryBot.create(:ci_runner, runner_type: :group_type) }
+ let(:group) { create(:group) }
+ let(:runner) { create(:ci_runner, :group, groups: [group]) }
it 'raises an error' do
expect { subject }
@@ -229,15 +224,15 @@ describe Ci::Runner do
subject { described_class.online }
before do
- @runner1 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.year.ago)
- @runner2 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago)
+ @runner1 = create(:ci_runner, :instance, contacted_at: 1.year.ago)
+ @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago)
end
it { is_expected.to eq([@runner2])}
end
describe '#online?' do
- let(:runner) { FactoryBot.create(:ci_runner, :shared) }
+ let(:runner) { create(:ci_runner, :instance) }
subject { runner.online? }
@@ -307,21 +302,20 @@ describe Ci::Runner do
end
describe '#can_pick?' do
- let(:pipeline) { create(:ci_pipeline) }
+ set(:pipeline) { create(:ci_pipeline) }
let(:build) { create(:ci_build, pipeline: pipeline) }
- let(:runner) { create(:ci_runner, tag_list: tag_list, run_untagged: run_untagged) }
+ let(:runner_project) { build.project }
+ let(:runner) { create(:ci_runner, :project, projects: [runner_project], tag_list: tag_list, run_untagged: run_untagged) }
let(:tag_list) { [] }
let(:run_untagged) { true }
subject { runner.can_pick?(build) }
- before do
- build.project.runners << runner
- end
-
context 'a different runner' do
+ let(:other_project) { create(:project) }
+ let(:other_runner) { create(:ci_runner, :project, projects: [other_project], tag_list: tag_list, run_untagged: run_untagged) }
+
it 'cannot handle builds' do
- other_runner = create(:ci_runner)
expect(other_runner.can_pick?(build)).to be_falsey
end
end
@@ -375,18 +369,14 @@ describe Ci::Runner do
end
context 'when runner is shared' do
- let(:runner) { create(:ci_runner, :shared) }
-
- before do
- build.project.runners = []
- end
+ let(:runner) { create(:ci_runner, :instance) }
it 'can handle builds' do
expect(runner.can_pick?(build)).to be_truthy
end
context 'when runner is locked' do
- let(:runner) { create(:ci_runner, :shared, locked: true) }
+ let(:runner) { create(:ci_runner, :instance, locked: true) }
it 'can handle builds' do
expect(runner.can_pick?(build)).to be_truthy
@@ -401,10 +391,8 @@ describe Ci::Runner do
end
end
- context 'when runner is not assigned to a project' do
- before do
- build.project.runners = []
- end
+ context 'when runner is assigned to another project' do
+ let(:runner_project) { create(:project) }
it 'cannot handle builds' do
expect(runner.can_pick?(build)).to be_falsey
@@ -412,10 +400,8 @@ describe Ci::Runner do
end
context 'when runner is assigned to a group' do
- before do
- build.project.runners = []
- runner.groups << create(:group, projects: [build.project])
- end
+ let(:group) { create(:group, projects: [build.project]) }
+ let(:runner) { create(:ci_runner, :group, tag_list: tag_list, run_untagged: run_untagged, groups: [group]) }
it 'can handle builds' do
expect(runner.can_pick?(build)).to be_truthy
@@ -469,7 +455,7 @@ describe Ci::Runner do
end
describe '#status' do
- let(:runner) { FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) }
+ let(:runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) }
subject { runner.status }
@@ -626,12 +612,13 @@ describe Ci::Runner do
end
describe '.assignable_for' do
- let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) }
- let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) }
- let!(:group_runner) { create(:ci_runner, runner_type: :group_type) }
- let!(:instance_runner) { create(:ci_runner, :shared) }
let(:project) { create(:project) }
+ let(:group) { create(:group) }
let(:another_project) { create(:project) }
+ let!(:unlocked_project_runner) { create(:ci_runner, :project, projects: [project]) }
+ let!(:locked_project_runner) { create(:ci_runner, :project, locked: true, projects: [project]) }
+ let!(:group_runner) { create(:ci_runner, :group, groups: [group]) }
+ let!(:instance_runner) { create(:ci_runner, :instance) }
context 'with already assigned project' do
subject { described_class.assignable_for(project) }
@@ -651,19 +638,16 @@ describe Ci::Runner do
describe "belongs_to_one_project?" do
it "returns false if there are two projects runner assigned to" do
- runner = FactoryBot.create(:ci_runner)
- project = FactoryBot.create(:project)
- project1 = FactoryBot.create(:project)
- project.runners << runner
- project1.runners << runner
+ project1 = create(:project)
+ project2 = create(:project)
+ runner = create(:ci_runner, :project, projects: [project1, project2])
expect(runner.belongs_to_one_project?).to be_falsey
end
it "returns true" do
- runner = FactoryBot.create(:ci_runner)
- project = FactoryBot.create(:project)
- project.runners << runner
+ project = create(:project)
+ runner = create(:ci_runner, :project, projects: [project])
expect(runner.belongs_to_one_project?).to be_truthy
end
@@ -713,21 +697,21 @@ describe Ci::Runner do
subject { runner.assigned_to_group? }
context 'when project runner' do
- let(:runner) { create(:ci_runner, description: 'Project runner', projects: [project]) }
+ let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) }
let(:project) { create(:project) }
it { is_expected.to be_falsey }
end
context 'when shared runner' do
- let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') }
+ let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') }
it { is_expected.to be_falsey }
end
context 'when group runner' do
let(:group) { create(:group) }
- let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) }
+ let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) }
it { is_expected.to be_truthy }
end
@@ -737,18 +721,18 @@ describe Ci::Runner do
subject { runner.assigned_to_project? }
context 'when group runner' do
- let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) }
+ let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) }
let(:group) { create(:group) }
it { is_expected.to be_falsey }
end
context 'when shared runner' do
- let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') }
+ let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') }
it { is_expected.to be_falsey }
end
context 'when project runner' do
- let(:runner) { create(:ci_runner, description: 'Group runner', projects: [project]) }
+ let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) }
let(:project) { create(:project) }
it { is_expected.to be_truthy }
@@ -780,4 +764,17 @@ describe Ci::Runner do
end
end
end
+
+ describe 'project runner without projects is destroyable' do
+ subject { create(:ci_runner, :project, :without_projects) }
+
+ it 'does not have projects' do
+ expect(subject.runner_projects).to be_empty
+ end
+
+ it 'can be destroyed' do
+ subject
+ expect { subject.destroy }.to change { described_class.count }.by(-1)
+ end
+ end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index af2240f4f89..9a76452a808 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1177,8 +1177,8 @@ describe Project do
describe '#any_runners?' do
context 'shared runners' do
let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) }
- let(:specific_runner) { create(:ci_runner) }
- let(:shared_runner) { create(:ci_runner, :shared) }
+ let(:specific_runner) { create(:ci_runner, :project, projects: [project]) }
+ let(:shared_runner) { create(:ci_runner, :instance) }
context 'for shared runners disabled' do
let(:shared_runners_enabled) { false }
@@ -1188,7 +1188,7 @@ describe Project do
end
it 'has a specific runner' do
- project.runners << specific_runner
+ specific_runner
expect(project.any_runners?).to be_truthy
end
@@ -1200,13 +1200,13 @@ describe Project do
end
it 'checks the presence of specific runner' do
- project.runners << specific_runner
+ specific_runner
expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy
end
it 'returns false if match cannot be found' do
- project.runners << specific_runner
+ specific_runner
expect(project.any_runners? { false }).to be_falsey
end
@@ -1238,7 +1238,7 @@ describe Project do
context 'group runners' do
let(:project) { create(:project, group_runners_enabled: group_runners_enabled) }
let(:group) { create(:group, projects: [project]) }
- let(:group_runner) { create(:ci_runner, groups: [group]) }
+ let(:group_runner) { create(:ci_runner, :group, groups: [group]) }
context 'for group runners disabled' do
let(:group_runners_enabled) { false }
@@ -1279,7 +1279,7 @@ describe Project do
end
describe '#shared_runners' do
- let!(:runner) { create(:ci_runner, :shared) }
+ let!(:runner) { create(:ci_runner, :instance) }
subject { project.shared_runners }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 16b409844fa..09dfeae6377 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1858,13 +1858,10 @@ describe User do
describe '#ci_owned_runners' do
let(:user) { create(:user) }
- let(:runner_1) { create(:ci_runner) }
- let(:runner_2) { create(:ci_runner) }
+ let!(:project) { create(:project) }
+ let(:runner) { create(:ci_runner, :project, projects: [project]) }
context 'without any projects nor groups' do
- let!(:project) { create(:project, runners: [runner_1]) }
- let!(:group) { create(:group) }
-
it 'does not load' do
expect(user.ci_owned_runners).to be_empty
end
@@ -1872,38 +1869,40 @@ describe User do
context 'with personal projects runners' do
let(:namespace) { create(:namespace, owner: user) }
- let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
+ let!(:project) { create(:project, namespace: namespace) }
it 'loads' do
- expect(user.ci_owned_runners).to contain_exactly(runner_1)
+ expect(user.ci_owned_runners).to contain_exactly(runner)
end
end
context 'with personal group runner' do
- let!(:project) { create(:project, runners: [runner_1]) }
+ let!(:project) { create(:project) }
+ let(:group_runner) { create(:ci_runner, :group, groups: [group]) }
let!(:group) do
- create(:group, runners: [runner_2]).tap do |group|
+ create(:group).tap do |group|
group.add_owner(user)
end
end
it 'loads' do
- expect(user.ci_owned_runners).to contain_exactly(runner_2)
+ expect(user.ci_owned_runners).to contain_exactly(group_runner)
end
end
context 'with personal project and group runner' do
let(:namespace) { create(:namespace, owner: user) }
- let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
+ let!(:project) { create(:project, namespace: namespace) }
+ let!(:group_runner) { create(:ci_runner, :group, groups: [group]) }
let!(:group) do
- create(:group, runners: [runner_2]).tap do |group|
+ create(:group).tap do |group|
group.add_owner(user)
end
end
it 'loads' do
- expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2)
+ expect(user.ci_owned_runners).to contain_exactly(runner, group_runner)
end
end
@@ -1914,7 +1913,7 @@ describe User do
end
it 'loads' do
- expect(user.ci_owned_runners).to contain_exactly(runner_1)
+ expect(user.ci_owned_runners).to contain_exactly(runner)
end
end
@@ -1931,7 +1930,7 @@ describe User do
context 'with groups projects runners' do
let(:group) { create(:group) }
- let!(:project) { create(:project, group: group, runners: [runner_1]) }
+ let!(:project) { create(:project, group: group) }
def add_user(access)
group.add_user(user, access)
@@ -1941,11 +1940,8 @@ describe User do
end
context 'with groups runners' do
- let!(:group) do
- create(:group, runners: [runner_1]).tap do |group|
- group.add_owner(user)
- end
- end
+ let!(:runner) { create(:ci_runner, :group, groups: [group]) }
+ let!(:group) { create(:group) }
def add_user(access)
group.add_user(user, access)
@@ -1955,7 +1951,7 @@ describe User do
end
context 'with other projects runners' do
- let!(:project) { create(:project, runners: [runner_1]) }
+ let!(:project) { create(:project) }
def add_user(access)
project.add_role(user, access)
@@ -1968,7 +1964,7 @@ describe User do
let(:group) { create(:group) }
let(:another_user) { create(:user) }
let(:subgroup) { create(:group, parent: group) }
- let!(:project) { create(:project, group: subgroup, runners: [runner_1]) }
+ let!(:project) { create(:project, group: subgroup) }
def add_user(access)
group.add_user(user, access)
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 6aadf839dbd..319ac389083 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -111,11 +111,13 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end
context 'when tags are not provided' do
- it 'returns 404 error' do
+ it 'returns 400 error' do
post api('/runners'), token: registration_token,
run_untagged: false
- expect(response).to have_gitlab_http_status 404
+ expect(response).to have_gitlab_http_status 400
+ expect(json_response['message']).to include(
+ 'tags_list' => ['can not be empty when runner is not allowed to pick untagged jobs'])
end
end
end
@@ -262,16 +264,12 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
describe '/api/v4/jobs' do
let(:project) { create(:project, shared_runners_enabled: false) }
let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
- let(:runner) { create(:ci_runner) }
+ let(:runner) { create(:ci_runner, :project, projects: [project]) }
let(:job) do
create(:ci_build, :artifacts, :extended_options,
pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate")
end
- before do
- project.runners << runner
- end
-
describe 'POST /api/v4/jobs/request' do
let!(:last_update) {}
let!(:new_update) { }
@@ -379,7 +377,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end
context 'when shared runner requests job for project without shared_runners_enabled' do
- let(:runner) { create(:ci_runner, :shared) }
+ let(:runner) { create(:ci_runner, :instance) }
it_behaves_like 'no jobs available'
end
@@ -724,7 +722,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end
context 'when runner specifies lower timeout' do
- let(:runner) { create(:ci_runner, maximum_timeout: 1000) }
+ let(:runner) { create(:ci_runner, :project, maximum_timeout: 1000, projects: [project]) }
it 'contains info about timeout overridden by runner' do
request_job
@@ -735,7 +733,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end
context 'when runner specifies bigger timeout' do
- let(:runner) { create(:ci_runner, maximum_timeout: 2000) }
+ let(:runner) { create(:ci_runner, :project, maximum_timeout: 2000, projects: [project]) }
it 'contains info about timeout not overridden by runner' do
request_job
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index c7587c877fc..0c7937feed6 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -11,23 +11,10 @@ describe API::Runners do
let(:group) { create(:group).tap { |group| group.add_owner(user) } }
let(:group2) { create(:group).tap { |group| group.add_owner(user) } }
- let!(:shared_runner) { create(:ci_runner, :shared, description: 'Shared runner') }
- let!(:unused_project_runner) { create(:ci_runner) }
-
- let!(:project_runner) do
- create(:ci_runner, description: 'Project runner').tap do |runner|
- create(:ci_runner_project, runner: runner, project: project)
- end
- end
-
- let!(:two_projects_runner) do
- create(:ci_runner, description: 'Two projects runner').tap do |runner|
- create(:ci_runner_project, runner: runner, project: project)
- create(:ci_runner_project, runner: runner, project: project2)
- end
- end
-
- let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group], runner_type: :group_type) }
+ let!(:shared_runner) { create(:ci_runner, :instance, description: 'Shared runner') }
+ let!(:project_runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) }
+ let!(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) }
+ let!(:group_runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) }
before do
# Set project access for users
@@ -141,6 +128,18 @@ describe API::Runners do
end
context 'when runner is not shared' do
+ context 'when unused runner is present' do
+ let!(:unused_project_runner) { create(:ci_runner, :project, :without_projects) }
+
+ it 'deletes unused runner' do
+ expect do
+ 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
+ end
+
it "returns runner's details" do
get api("/runners/#{project_runner.id}", admin)
@@ -310,14 +309,6 @@ describe API::Runners do
end
context 'when runner is not shared' do
- it 'deletes unused runner' do
- expect do
- 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
-
it 'deletes used project runner' do
expect do
delete api("/runners/#{project_runner.id}", admin)
@@ -543,11 +534,7 @@ describe API::Runners do
describe 'POST /projects/:id/runners' do
context 'authorized user' do
- let(:project_runner2) do
- create(:ci_runner).tap do |runner|
- create(:ci_runner_project, runner: runner, project: project2)
- end
- end
+ let(:project_runner2) { create(:ci_runner, :project, projects: [project2]) }
it 'enables specific runner' do
expect do
@@ -560,7 +547,7 @@ describe API::Runners do
expect do
post api("/projects/#{project.id}/runners", user), runner_id: project_runner.id
end.to change { project.runners.count }.by(0)
- expect(response).to have_gitlab_http_status(409)
+ expect(response).to have_gitlab_http_status(400)
end
it 'does not enable locked runner' do
@@ -586,11 +573,15 @@ describe API::Runners do
end
context 'user is admin' do
- it 'enables any specific runner' do
- expect do
- post api("/projects/#{project.id}/runners", admin), runner_id: unused_project_runner.id
- end.to change { project.runners.count }.by(+1)
- expect(response).to have_gitlab_http_status(201)
+ context 'when project runner is used' do
+ let!(:new_project_runner) { create(:ci_runner, :project) }
+
+ it 'enables any specific runner' do
+ expect do
+ post api("/projects/#{project.id}/runners", admin), runner_id: new_project_runner.id
+ end.to change { project.runners.count }.by(+1)
+ expect(response).to have_gitlab_http_status(201)
+ end
end
it 'enables a shared runner' do
@@ -603,14 +594,6 @@ describe API::Runners do
end
end
- context 'user is not admin' do
- it 'does not enable runner without access to' do
- post api("/projects/#{project.id}/runners", user), runner_id: unused_project_runner.id
-
- expect(response).to have_gitlab_http_status(403)
- end
- end
-
it 'raises an error when no runner_id param is provided' do
post api("/projects/#{project.id}/runners", admin)
@@ -618,6 +601,16 @@ describe API::Runners do
end
end
+ context 'user is not admin' do
+ let!(:new_project_runner) { create(:ci_runner, :project) }
+
+ it 'does not enable runner without access to' do
+ post api("/projects/#{project.id}/runners", user), runner_id: new_project_runner.id
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+
context 'authorized user without permissions' do
it 'does not enable runner' do
post api("/projects/#{project.id}/runners", user2)
diff --git a/spec/serializers/runner_entity_spec.rb b/spec/serializers/runner_entity_spec.rb
index 439ba2cbca2..ba99d568eba 100644
--- a/spec/serializers/runner_entity_spec.rb
+++ b/spec/serializers/runner_entity_spec.rb
@@ -1,10 +1,10 @@
require 'spec_helper'
describe RunnerEntity do
- let(:runner) { create(:ci_runner, :specific) }
+ let(:project) { create(:project) }
+ let(:runner) { create(:ci_runner, :project, projects: [project]) }
let(:entity) { described_class.new(runner, request: request, current_user: user) }
let(:request) { double('request') }
- let(:project) { create(:project) }
let(:user) { create(:admin) }
before do
diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb
index 8063bc7e1ac..3816bd0deb5 100644
--- a/spec/services/ci/register_job_service_spec.rb
+++ b/spec/services/ci/register_job_service_spec.rb
@@ -5,15 +5,11 @@ module Ci
set(:group) { create(:group) }
set(:project) { create(:project, group: group, shared_runners_enabled: false, group_runners_enabled: false) }
set(:pipeline) { create(:ci_pipeline, project: project) }
- let!(:shared_runner) { create(:ci_runner, is_shared: true) }
- let!(:specific_runner) { create(:ci_runner, is_shared: false) }
- let!(:group_runner) { create(:ci_runner, groups: [group], runner_type: :group_type) }
+ let!(:shared_runner) { create(:ci_runner, :instance) }
+ let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) }
+ let!(:group_runner) { create(:ci_runner, :group, groups: [group]) }
let!(:pending_job) { create(:ci_build, pipeline: pipeline) }
- before do
- specific_runner.assign_to(project)
- end
-
describe '#execute' do
context 'runner follow tag list' do
it "picks build with the same tag" do
@@ -181,24 +177,24 @@ module Ci
end
context 'for multiple builds' do
- let!(:project2) { create :project, group_runners_enabled: true, group: group }
- let!(:pipeline2) { create :ci_pipeline, project: project2 }
- let!(:project3) { create :project, group_runners_enabled: true, group: group }
- let!(:pipeline3) { create :ci_pipeline, project: project3 }
+ let!(:project2) { create(:project, group_runners_enabled: true, group: group) }
+ let!(:pipeline2) { create(:ci_pipeline, project: project2) }
+ let!(:project3) { create(:project, group_runners_enabled: true, group: group) }
+ let!(:pipeline3) { create(:ci_pipeline, project: project3) }
let!(:build1_project1) { pending_job }
- let!(:build2_project1) { create :ci_build, pipeline: pipeline }
- let!(:build3_project1) { create :ci_build, pipeline: pipeline }
- let!(:build1_project2) { create :ci_build, pipeline: pipeline2 }
- let!(:build2_project2) { create :ci_build, pipeline: pipeline2 }
- let!(:build1_project3) { create :ci_build, pipeline: pipeline3 }
+ let!(:build2_project1) { create(:ci_build, pipeline: pipeline) }
+ let!(:build3_project1) { create(:ci_build, pipeline: pipeline) }
+ let!(:build1_project2) { create(:ci_build, pipeline: pipeline2) }
+ let!(:build2_project2) { create(:ci_build, pipeline: pipeline2) }
+ let!(:build1_project3) { create(:ci_build, pipeline: pipeline3) }
# these shouldn't influence the scheduling
- let!(:unrelated_group) { create :group }
- let!(:unrelated_project) { create :project, group_runners_enabled: true, group: unrelated_group }
- let!(:unrelated_pipeline) { create :ci_pipeline, project: unrelated_project }
- let!(:build1_unrelated_project) { create :ci_build, pipeline: unrelated_pipeline }
- let!(:unrelated_group_runner) { create :ci_runner, groups: [unrelated_group] }
+ let!(:unrelated_group) { create(:group) }
+ let!(:unrelated_project) { create(:project, group_runners_enabled: true, group: unrelated_group) }
+ let!(:unrelated_pipeline) { create(:ci_pipeline, project: unrelated_project) }
+ let!(:build1_unrelated_project) { create(:ci_build, pipeline: unrelated_pipeline) }
+ let!(:unrelated_group_runner) { create(:ci_runner, :group, groups: [unrelated_group]) }
it 'does not consider builds from other group runners' do
expect(described_class.new(group_runner).send(:builds_for_group_runner).count).to eq 6
@@ -292,7 +288,7 @@ module Ci
end
context 'when access_level of runner is not_protected' do
- let!(:specific_runner) { create(:ci_runner, :specific) }
+ let!(:specific_runner) { create(:ci_runner, :project, projects: [project]) }
context 'when a job is protected' do
let!(:pending_job) { create(:ci_build, :protected, pipeline: pipeline) }
@@ -324,7 +320,7 @@ module Ci
end
context 'when access_level of runner is ref_protected' do
- let!(:specific_runner) { create(:ci_runner, :ref_protected, :specific) }
+ let!(:specific_runner) { create(:ci_runner, :project, :ref_protected, projects: [project]) }
context 'when a job is protected' do
let!(:pending_job) { create(:ci_build, :protected, pipeline: pipeline) }
diff --git a/spec/services/ci/update_build_queue_service_spec.rb b/spec/services/ci/update_build_queue_service_spec.rb
index 74a23ed2a3f..ca0c6be5da6 100644
--- a/spec/services/ci/update_build_queue_service_spec.rb
+++ b/spec/services/ci/update_build_queue_service_spec.rb
@@ -6,19 +6,18 @@ describe Ci::UpdateBuildQueueService do
let(:pipeline) { create(:ci_pipeline, project: project) }
context 'when updating specific runners' do
- let(:runner) { create(:ci_runner) }
+ let(:runner) { create(:ci_runner, :project, projects: [project]) }
context 'when there is a runner that can pick build' do
- before do
- build.project.runners << runner
- end
-
it 'ticks runner queue value' do
expect { subject.execute(build) }.to change { runner.ensure_runner_queue_value }
end
end
context 'when there is no runner that can pick build' do
+ let(:another_project) { create(:project) }
+ let(:runner) { create(:ci_runner, :project, projects: [another_project]) }
+
it 'does not tick runner queue value' do
expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value }
end
@@ -26,7 +25,7 @@ describe Ci::UpdateBuildQueueService do
end
context 'when updating shared runners' do
- let(:runner) { create(:ci_runner, :shared) }
+ let(:runner) { create(:ci_runner, :instance) }
context 'when there is no runner that can pick build' do
it 'ticks runner queue value' do
@@ -56,9 +55,9 @@ describe Ci::UpdateBuildQueueService do
end
context 'when updating group runners' do
- let(:group) { create :group }
- let(:project) { create :project, group: group }
- let(:runner) { create :ci_runner, groups: [group] }
+ let(:group) { create(:group) }
+ let(:project) { create(:project, group: group) }
+ let(:runner) { create(:ci_runner, :group, groups: [group]) }
context 'when there is a runner that can pick build' do
it 'ticks runner queue value' do