Skip to content
Snippets Groups Projects
Commit 36d6b6f8 authored by Kamil Trzcińśki's avatar Kamil Trzcińśki Committed by Dylan Griffith
Browse files

Refactor validations and make runner factory by default to be instance-wide runner

parent cb23e111
No related branches found
No related tags found
No related merge requests found
Showing
with 134 additions and 137 deletions
Loading
Loading
@@ -57,12 +57,15 @@ class Runner < ActiveRecord::Base
end
 
validate :tag_constraints
validate :no_projects, unless: :project_type?
validate :no_groups, unless: :group_type?
validate :only_one_group, if: :group_type?
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 :is_shared_is_valid
acts_as_taggable
 
after_destroy :cleanup_runner_queue
Loading
Loading
@@ -118,8 +121,8 @@ def assign_to(project, current_user = nil)
raise ArgumentError, 'Transitioning a group runner to a project runner is not supported'
end
 
self.save
project.runner_projects.create(runner_id: self.id)
self.projects << project
self.save!
end
 
def display_name
Loading
Loading
@@ -258,19 +261,31 @@ def assignable_for?(project_id)
 
def no_projects
if projects.any?
errors.add(:runner, 'cannot assign project to a non-project runner')
errors.add(:runner, 'cannot have projects assigned')
end
end
 
def no_groups
if groups.any?
errors.add(:runner, 'cannot assign group to a non-group runner')
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
 
def only_one_group
if groups.many?
errors.add(:runner, 'can only be assigned to one group')
def is_shared_is_valid
unless is_shared? == instance_type?
errors.add(:is_shared, 'is not equal to instance_type?')
end
end
 
Loading
Loading
Loading
Loading
@@ -3,7 +3,7 @@
describe Groups::RunnersController do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:runner) { create(:ci_runner, :group) }
let(:runner) { create(:ci_runner, :group, groups: [group]) }
 
let(:params) do
{
Loading
Loading
@@ -15,7 +15,6 @@
before do
sign_in(user)
group.add_master(user)
group.runners << runner
end
 
describe '#update' do
Loading
Loading
Loading
Loading
@@ -3,7 +3,7 @@
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
{
Loading
Loading
@@ -16,7 +16,6 @@
before do
sign_in(user)
project.add_master(user)
project.runners << runner
end
 
describe '#update' do
Loading
Loading
Loading
Loading
@@ -19,12 +19,12 @@
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)
Loading
Loading
FactoryBot.define do
factory :ci_runner_project, class: Ci::RunnerProject do
runner factory: :ci_runner
runner factory: [:ci_runner, :project]
project
end
end
Loading
Loading
@@ -3,37 +3,27 @@
sequence(:description) { |n| "My runner#{n}" }
 
platform "darwin"
is_shared false
active true
access_level :not_protected
runner_type :project_type
 
trait :online do
contacted_at Time.now
end
is_shared true
runner_type :instance_type
 
trait :shared do
trait :instance do
is_shared true
runner_type :instance_type
end
 
trait :specific do
is_shared false
end
trait :group do
is_shared false
runner_type :group_type
end
 
trait :project do
is_shared false
runner_type :project_type
end
 
trait :instance do
is_shared true
runner_type :instance_type
end
trait :inactive do
active false
end
Loading
Loading
Loading
Loading
@@ -76,7 +76,7 @@
 
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
 
Loading
Loading
@@ -90,7 +90,7 @@
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
 
Loading
Loading
@@ -149,8 +149,9 @@
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
 
Loading
Loading
@@ -158,9 +159,9 @@
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
 
Loading
Loading
@@ -168,9 +169,10 @@
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
 
Loading
Loading
@@ -179,8 +181,9 @@
end
 
describe 'disable/destroy' do
let(:runner) { create(:ci_runner, :project, projects: [@project1]) }
before do
@project1.runners << runner
visit admin_runner_path(runner)
end
 
Loading
Loading
Loading
Loading
@@ -29,11 +29,7 @@
end
 
context 'when a project_type runner is activated on the project' do
given(:specific_runner) { create(:ci_runner, :project) }
background do
project.runners << specific_runner
end
given(:specific_runner) { create(:ci_runner, :project, projects: [specific_runner]) }
 
scenario 'user sees the specific runner' do
visit project_runners_path(project)
Loading
Loading
@@ -126,11 +122,10 @@
 
context 'when a specific runner exists in another project' do
given(:another_project) { create(:project) }
given(:specific_runner) { create(:ci_runner, :project) }
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
Loading
Loading
Loading
Loading
@@ -2,7 +2,7 @@
 
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 }
 
Loading
Loading
Loading
Loading
@@ -149,10 +149,9 @@
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
 
Loading
Loading
@@ -1407,12 +1406,7 @@ def create_mr(build, pipeline, factory: :merge_request, created_at: Time.now)
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
Loading
Loading
Loading
Loading
@@ -1598,7 +1598,7 @@ def create_build(name, stage_idx)
 
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
Loading
Loading
Loading
Loading
@@ -21,7 +21,7 @@
end
end
 
context '#only_one_group' do
context '#exactly_one_group' do
let(:group) { create(:group) }
let(:runner) { create(:ci_runner, :group, groups: [group]) }
 
Loading
Loading
@@ -29,42 +29,43 @@
runner.groups << build(:group)
 
expect(runner).not_to be_valid
expect(runner.errors.full_messages).to include('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
 
context 'runner_type validations' do
let(:group) { create(:group) }
set(:group) { create(:group) }
set(:project) { create(:project) }
let(:group_runner) { create(:ci_runner, :group, groups: [group]) }
let(:project_runner) { create(:ci_runner, :project) }
let(:project_runner) { create(:ci_runner, :project, projects: [project]) }
let(:instance_runner) { create(:ci_runner, :instance) }
 
it 'disallows assigning group to project_type runner' do
project_runner.groups << build(:group)
 
expect(project_runner).not_to be_valid
expect(project_runner.errors.full_messages).to include('Runner cannot assign group to a non-group runner')
expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned')
end
 
it 'disallows assigning group to instance_type runner' do
instance_runner.groups << build(:group)
 
expect(instance_runner).not_to be_valid
expect(instance_runner.errors.full_messages).to include('Runner cannot assign group to a non-group runner')
expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned')
end
 
it 'disallows assigning project to group_type runner' do
group_runner.projects << build(:project)
 
expect(group_runner).not_to be_valid
expect(group_runner.errors.full_messages).to include('Runner cannot assign project to a non-project runner')
expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned')
end
 
it 'disallows assigning project to instance_type runner' do
instance_runner.projects << build(:project)
 
expect(instance_runner).not_to be_valid
expect(instance_runner.errors.full_messages).to include('Runner cannot assign project to a non-project runner')
expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned')
end
 
it 'should fail to save a group assigned to a project runner even if the runner is already saved' do
Loading
Loading
@@ -107,8 +108,8 @@
describe '.shared' do
let(:group) { create(:group) }
let(:project) { create(:project) }
let!(:group_runner) { create(:ci_runner, :group) }
let!(:project_runner) { create(:ci_runner, :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 only shared runners' do
Loading
Loading
@@ -120,11 +121,11 @@
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
Loading
Loading
@@ -175,31 +176,32 @@
 
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(runner).to be_specific
expect(runner).to be_project_type
expect(runner.projects).to eq([project])
Loading
Loading
@@ -208,7 +210,8 @@
end
 
context 'with group runner' do
let!(:runner) { FactoryBot.create(:ci_runner, :group) }
let(:group) { create(:group) }
let(:runner) { create(:ci_runner, :group, groups: [group]) }
 
it 'raises an error' do
expect { subject }
Loading
Loading
@@ -221,15 +224,15 @@
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? }
 
Loading
Loading
@@ -299,21 +302,20 @@ def stub_redis_runner_contacted_at(value)
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
Loading
Loading
@@ -367,18 +369,14 @@ def stub_redis_runner_contacted_at(value)
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
Loading
Loading
@@ -393,10 +391,8 @@ def stub_redis_runner_contacted_at(value)
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
Loading
Loading
@@ -404,10 +400,8 @@ def stub_redis_runner_contacted_at(value)
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
Loading
Loading
@@ -461,7 +455,7 @@ def stub_redis_runner_contacted_at(value)
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 }
 
Loading
Loading
@@ -618,15 +612,32 @@ def expect_redis_update
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(:runner) { create(:ci_runner, :project, projects: [project]) }
let(:project) { create(:project) }
let(:another_project) { create(:project) }
 
context 'with already assigned project' do
subject { described_class.assignable_for(project) }
context 'with shared runners' do
let(:runner) { create(:ci_runner, :instance) }
context 'does not give owned runner' do
subject { described_class.assignable_for(project) }
it { is_expected.to be_empty }
end
context 'does not give shared runner' do
subject { described_class.assignable_for(another_project) }
it { is_expected.to be_empty }
end
end
context 'with unlocked runner' do
context 'does not give owned runner' do
subject { described_class.assignable_for(project) }
it { is_expected.to be_empty }
end
 
it { is_expected.to be_empty }
end
Loading
Loading
@@ -643,19 +654,16 @@ def expect_redis_update
 
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
Loading
Loading
@@ -705,14 +713,14 @@ def expect_redis_update
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
Loading
Loading
@@ -740,7 +748,7 @@ def expect_redis_update
end
 
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_truthy }
Loading
Loading
Loading
Loading
@@ -1306,8 +1306,8 @@ def create_pipeline
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 }
Loading
Loading
@@ -1317,7 +1317,7 @@ def create_pipeline
end
 
it 'has a specific runner' do
project.runners << specific_runner
specific_runner
 
expect(project.any_runners?).to be_truthy
end
Loading
Loading
@@ -1329,13 +1329,13 @@ def create_pipeline
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
Loading
Loading
Loading
Loading
@@ -1907,8 +1907,7 @@
 
describe '#ci_owned_runners' do
let(:user) { create(:user) }
let(:runner_1) { create(:ci_runner) }
let(:runner_2) { create(:ci_runner) }
let(:runner) { create(:ci_runner, :project, projects: [project]) }
 
context 'without any projects nor groups' do
let!(:project) { create(:project, runners: [runner_1]) }
Loading
Loading
Loading
Loading
@@ -262,7 +262,7 @@
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")
Loading
Loading
@@ -271,7 +271,6 @@
before do
stub_artifacts_object_storage
job
project.runners << runner
end
 
describe 'POST /api/v4/jobs/request' do
Loading
Loading
@@ -381,7 +380,7 @@
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
Loading
Loading
Loading
Loading
@@ -11,7 +11,7 @@
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!(:shared_runner) { create(:ci_runner, :instance, description: 'Shared runner') }
let!(:unused_project_runner) { create(:ci_runner) }
 
let!(:project_runner) do
Loading
Loading
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
Loading
Loading
Loading
Loading
@@ -292,7 +292,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) }
Loading
Loading
Loading
Loading
@@ -6,13 +6,9 @@
let(:pipeline) { create(:ci_pipeline, project: project) }
 
context 'when updating specific runners' do
let(:runner) { create(:ci_runner, :project) }
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
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment