diff options
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/settings/ci_cd_controller_spec.rb | 11 | ||||
-rw-r--r-- | spec/factories/project_settings.rb | 5 | ||||
-rw-r--r-- | spec/factories/projects.rb | 9 | ||||
-rw-r--r-- | spec/features/admin/admin_runners_spec.rb | 41 | ||||
-rw-r--r-- | spec/features/runners_spec.rb | 80 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 2 | ||||
-rw-r--r-- | spec/models/ci/runner_spec.rb | 307 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 120 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 64 | ||||
-rw-r--r-- | spec/requests/api/runner_spec.rb | 15 | ||||
-rw-r--r-- | spec/requests/api/runners_spec.rb | 218 | ||||
-rw-r--r-- | spec/services/ci/register_job_service_spec.rb | 120 | ||||
-rw-r--r-- | spec/services/ci/update_build_queue_service_spec.rb | 62 |
13 files changed, 929 insertions, 125 deletions
diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index 7dae9b85d78..1cf395b0328 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -17,6 +17,17 @@ describe Projects::Settings::CiCdController do expect(response).to have_gitlab_http_status(200) expect(response).to render_template(:show) end + + it 'sets assignable project runners' do + group = create(:group, runners: [create(:ci_runner)], parent: create(:group)) + group.add_master(user) + project_runner = create(:ci_runner, projects: [create(:project, group: group)]) + create(:ci_runner, :shared) + + get :show, namespace_id: project.namespace, project_id: project + + expect(assigns(:assignable_runners)).to eq [project_runner] + end end describe '#reset_cache' do diff --git a/spec/factories/project_settings.rb b/spec/factories/project_settings.rb new file mode 100644 index 00000000000..90240ee6de0 --- /dev/null +++ b/spec/factories/project_settings.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :project_settings do + project + end +end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 1904615778c..3d2810bfdba 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -15,14 +15,18 @@ FactoryBot.define do namespace creator { group ? create(:user) : namespace&.owner } - # Nest Project Feature attributes transient do + # Nest Project Feature attributes wiki_access_level ProjectFeature::ENABLED builds_access_level ProjectFeature::ENABLED snippets_access_level ProjectFeature::ENABLED issues_access_level ProjectFeature::ENABLED merge_requests_access_level ProjectFeature::ENABLED repository_access_level ProjectFeature::ENABLED + + # we can't assign the delegated `#settings` attributes directly, as the + # `#settings` relation needs to be created first + group_runners_enabled nil end after(:create) do |project, evaluator| @@ -47,6 +51,9 @@ FactoryBot.define do end project.group&.refresh_members_authorized_projects + + # assign the delegated `#settings` attributes after create + project.reload.group_runners_enabled = evaluator.group_runners_enabled unless evaluator.group_runners_enabled.nil? end trait :public do diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 8de2e3d199b..b0aa2e8b588 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -59,6 +59,47 @@ describe "Admin Runners" do expect(page).to have_text 'No runners found' end end + + context 'group runner' do + it 'shows the label and does not show the project count' do + group = create :group + runner = create :ci_runner, groups: [group] + + visit admin_runners_path + + within "#runner_#{runner.id}" do + expect(page).to have_selector '.label', text: 'group' + expect(page).to have_text 'n/a' + end + end + end + + context 'shared runner' do + it 'shows the label and does not show the project count' do + runner = create :ci_runner, :shared + + visit admin_runners_path + + within "#runner_#{runner.id}" do + expect(page).to have_selector '.label', text: 'shared' + expect(page).to have_text 'n/a' + end + end + end + + context 'specific runner' do + it 'shows the label and the project count' do + project = create :project + runner = create :ci_runner, projects: [project] + + visit admin_runners_path + + within "#runner_#{runner.id}" do + expect(page).to have_selector '.label', text: 'specific' + expect(page).to have_text '1' + end + end + end end describe "Runner show page" do diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index df65c2d2f83..b396e103345 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -181,4 +181,84 @@ feature 'Runners' do expect(page.find('.shared-runners-description')).to have_content('Disable shared Runners') end end + + context 'group runners' do + background do + project.add_master(user) + end + + given(:group) { create :group } + + context 'as project and group master' do + background do + group.add_master(user) + end + + context 'project with a group but no group runner' do + given(:project) { create :project, group: group } + + scenario 'group runners are not available' do + visit runners_path(project) + + expect(page).to have_content 'This group does not provide any group Runners yet.' + + expect(page).to have_content 'Setup a group Runner manually' + expect(page).not_to have_content 'Ask your group master to setup a group Runner.' + end + end + end + + context 'as project master' do + context 'project without a group' do + given(:project) { create :project } + + scenario 'group runners are not available' do + visit runners_path(project) + + expect(page).to have_content 'This project does not belong to a group and can therefore not make use of group Runners.' + end + end + + context 'project with a group but no group runner' do + given(:group) { create :group } + given(:project) { create :project, group: group } + + scenario 'group runners are not available' do + visit runners_path(project) + + expect(page).to have_content 'This group does not provide any group Runners yet.' + + expect(page).not_to have_content 'Setup a group Runner manually' + expect(page).to have_content 'Ask your group master to setup a group Runner.' + end + 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' } + + scenario 'group runners are available' do + visit runners_path(project) + + expect(page).to have_content 'Available group Runners : 1' + expect(page).to have_content 'group-runner' + end + + scenario 'group runners may be disabled for a project' do + visit runners_path(project) + + click_on 'Disable group Runners' + + expect(page).to have_content 'Enable group Runners' + expect(project.reload.group_runners_enabled).to be false + + click_on 'Enable group Runners' + + expect(page).to have_content 'Disable group Runners' + expect(project.reload.group_runners_enabled).to be true + end + end + end + end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index e7f20f81fe0..dd5640dd9de 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -258,7 +258,6 @@ project: - builds - runner_projects - runners -- active_runners - variables - triggers - pipeline_schedules @@ -286,6 +285,7 @@ project: - internal_ids - project_deploy_tokens - deploy_tokens +- settings - ci_cd_settings award_emoji: - awardable diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ab170e6351c..23e18efea70 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -19,6 +19,63 @@ describe Ci::Runner do end end end + + context 'either_projects_or_group' do + it 'disallows assigning to a group if already assigned to a group' do + group = create(:group) + runner = create(:ci_runner, groups: [group]) + + 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'] + end + + it 'disallows assigning to a group if already assigned to a project' do + project = create(:project) + runner = create(:ci_runner, projects: [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'] + end + + it 'disallows assigning to a project if already assigned to a group' do + group = create(:group) + runner = create(:ci_runner, groups: [group]) + + runner.projects << build(:project) + + 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'] + 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) + + expect(runner).to be_valid + 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) + + expect(runner).to be_valid + 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) + + expect(runner).to be_valid + end + end end describe '#access_level' do @@ -49,6 +106,95 @@ describe Ci::Runner do end end + describe '.shared' do + it 'returns the shared group runner' do + group = create :group + runner = create :ci_runner, :shared, groups: [group] + + expect(described_class.shared).to eq [runner] + end + + it 'returns the shared project runner' do + project = create :project + runner = create :ci_runner, :shared, projects: [project] + + expect(described_class.shared).to eq [runner] + end + end + + describe '.belonging_to_project' do + it 'returns the specific project runner' do + # own + specific_project = create :project + specific_runner = create :ci_runner, :specific, projects: [specific_project] + + # other + other_project = create :project + create :ci_runner, :specific, projects: [other_project] + + expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner] + end + end + + describe '.belonging_to_any_project' do + it 'returns the specific project runner' do + # project + project_project = create :project + project_runner = create :ci_runner, :specific, projects: [project_project] + + # group + group = create :group + create :project, group: group + create :ci_runner, :specific, groups: [group] + + expect(described_class.belonging_to_any_project).to eq [project_runner] + end + end + + describe '.belonging_to_group' do + it 'returns the specific group runner' do + # own + specific_group = create :group + specific_project = create :project, group: specific_group + specific_runner = create :ci_runner, :specific, groups: [specific_group] + + # other + other_group = create :group + create :project, group: other_group + create :ci_runner, :specific, groups: [other_group] + + expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner] + end + + it 'returns the group runner from a parent group' do + parent_group = create :group + group = create :group, parent: parent_group + project = create :project, group: group + runner = create :ci_runner, :specific, groups: [parent_group] + + expect(described_class.belonging_to_group(project.id)).to eq [runner] + end + end + + describe '.owned_or_shared' do + it 'returns a globally shared, a project specific and a group specific runner' do + # group specific + group = create :group + project = create :project, group: group + group_runner = create :ci_runner, :specific, groups: [group] + + # project specific + project_runner = create :ci_runner, :specific, projects: [project] + + # globally shared + shared_runner = create :ci_runner, :shared + + expect(described_class.owned_or_shared(project.id)).to match_array [ + group_runner, project_runner, shared_runner + ] + end + end + 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') @@ -171,6 +317,13 @@ describe Ci::Runner do build.project.runners << runner end + context 'a different runner' do + it 'cannot handle builds' do + other_runner = create :ci_runner + expect(other_runner.can_pick?(build)).to be_falsey + end + end + context 'when runner does not have tags' do it 'can handle builds without tags' do expect(runner.can_pick?(build)).to be_truthy @@ -212,7 +365,7 @@ describe Ci::Runner do context 'when runner cannot pick untagged jobs' do before do - runner.run_untagged = false + runner.update_attributes!(run_untagged: false) end it 'cannot handle builds without tags' do @@ -225,7 +378,7 @@ describe Ci::Runner do context 'when runner is shared' do before do - runner.is_shared = true + runner.update_attributes!(is_shared: true) build.project.runners = [] end @@ -235,7 +388,7 @@ describe Ci::Runner do context 'when runner is locked' do before do - runner.locked = true + runner.update_attributes!(locked: true) end it 'can handle builds' do @@ -260,6 +413,17 @@ describe Ci::Runner do expect(runner.can_pick?(build)).to be_falsey end end + + context 'when runner is assigned to a group' do + before do + build.project.runners = [] + runner.groups << create(:group, projects: [build.project]) + end + + it 'can handle builds' do + expect(runner.can_pick?(build)).to be_truthy + end + end end context 'when access_level of runner is not_protected' do @@ -583,4 +747,141 @@ describe Ci::Runner do expect(described_class.search(runner.description.upcase)).to eq([runner]) end end + + describe 'group?' do + it 'returns false when the runner is a project runner' do + project = create :project + runner = create :ci_runner, description: 'Project runner', projects: [project] + + expect(runner.group?).to be false + end + + it 'returns false when the runner is a shared runner' do + runner = create :ci_runner, :shared, description: 'Shared runner' + + expect(runner.group?).to be false + end + + it 'returns true when the runner is assigned to a group' do + group = create :group + runner = create :ci_runner, description: 'Group runner', groups: [group] + + expect(runner.group?).to be true + end + end + + describe 'project?' do + it 'returns false when the runner is a group prunner' do + group = create :group + runner = create :ci_runner, description: 'Group runner', groups: [group] + + expect(runner.project?).to be false + end + + it 'returns false when the runner is a shared runner' do + runner = create :ci_runner, :shared, description: 'Shared runner' + + expect(runner.project?).to be false + end + + it 'returns true when the runner is assigned to a project' do + project = create :project + runner = create :ci_runner, description: 'Group runner', projects: [project] + + expect(runner.project?).to be true + end + end + + describe '#accessible_projects' do + let!(:shared_runner) { create(:ci_runner, :shared) } + let!(:shared_project) { create(:project, shared_runners_enabled: true) } + + let!(:project_runner) { create(:ci_runner) } + let!(:project_project) { create(:project, runners: [project_runner], shared_runners_enabled: false) } + + let!(:group_runner) { create(:ci_runner) } + + let!(:parent_group) { create(:group) } + let!(:parent_group_project) do + create(:project, group: parent_group, shared_runners_enabled: false) + end + + let!(:group) { create :group, runners: [group_runner], parent: parent_group } + let!(:group_project) do + create(:project, group: group, shared_runners_enabled: false) + end + + let!(:nested_group_project) do + nested_group = create :group, parent: group + create(:project, group: nested_group, shared_runners_enabled: false) + end + + it 'returns the project with a shared runner' do + expect(shared_runner.reload.accessible_projects).to eq [shared_project] + end + + it 'returns the project with a project runner' do + expect(project_runner.reload.accessible_projects).to eq [project_project] + end + + it 'returns the projects with a group and nested group runner' do + expect(group_runner.reload.accessible_projects).to eq [group_project, nested_group_project] + end + + context 'deleted' do + before do + shared_project.update_attributes!(pending_delete: true) + project_project.update_attributes!(pending_delete: true) + group_project.update_attributes!(pending_delete: true) + nested_group_project.update_attributes!(pending_delete: true) + end + + it 'returns no projects' do + expect(shared_runner.reload.accessible_projects).to be_empty + expect(project_runner.reload.accessible_projects).to be_empty + expect(group_runner.reload.accessible_projects).to be_empty + end + end + + context 'builds disabled' do + before do + shared_project.update_attributes!(builds_enabled: false) + project_project.update_attributes!(builds_enabled: false) + group_project.update_attributes!(builds_enabled: false) + nested_group_project.update_attributes!(builds_enabled: false) + end + + it 'returns no projects' do + expect(shared_runner.reload.accessible_projects).to be_empty + expect(project_runner.reload.accessible_projects).to be_empty + expect(group_runner.reload.accessible_projects).to be_empty + end + end + end + + describe '#invalidate_build_cache!' do + context 'runner can pick the build' do + it 'calls #tick_runner_queue' do + ci_build = build :ci_build + runner = build :ci_runner + allow(runner).to receive(:can_pick?).with(ci_build).and_return(true) + + expect(runner).to receive(:tick_runner_queue) + + runner.invalidate_build_cache!(ci_build) + end + end + + context 'runner cannot pick the build' do + it 'does not call #tick_runner_queue' do + ci_build = build :ci_build + runner = build :ci_runner + allow(runner).to receive(:can_pick?).with(ci_build).and_return(false) + + expect(runner).not_to receive(:tick_runner_queue) + + runner.invalidate_build_cache!(ci_build) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a9587b1005e..265941acbe7 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -63,7 +63,6 @@ describe Project do it { is_expected.to have_many(:build_trace_section_names)} it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } - it { is_expected.to have_many(:active_runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } it { is_expected.to have_many(:pages_domains) } @@ -124,6 +123,26 @@ describe Project do expect(subject.boards.size).to eq 1 end end + + describe '#settings' do + it 'creates lazily a settings record when the project does not have one associated' do + project = create :project + expect(ProjectSettings.count).to eq 0 + + expect(project.settings).to be_a ProjectSettings + + expect(ProjectSettings.count).to eq 1 + end + + it 'returns the associated record when the project has one associated' do + project = create :project, settings: create(:project_settings) + expect(ProjectSettings.count).to eq 1 + + expect(project.settings).to be_a ProjectSettings + + expect(ProjectSettings.count).to eq 1 + end + end end describe 'modules' do @@ -1140,44 +1159,79 @@ describe Project do end describe '#any_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) } + 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 } - context 'for shared runners disabled' do - let(:shared_runners_enabled) { false } + context 'for shared runners disabled' do + let(:shared_runners_enabled) { false } - it 'has no runners available' do - expect(project.any_runners?).to be_falsey - end + it 'has no runners available' do + expect(project.any_runners?).to be_falsey + end - it 'has a specific runner' do - project.runners << specific_runner - expect(project.any_runners?).to be_truthy - end + it 'has a specific runner' do + project.runners << specific_runner + expect(project.any_runners?).to be_truthy + end - it 'has a shared runner, but they are prohibited to use' do - shared_runner - expect(project.any_runners?).to be_falsey + it 'has a shared runner, but they are prohibited to use' do + shared_runner + expect(project.any_runners?).to be_falsey + end + + it 'checks the presence of specific runner' do + project.runners << specific_runner + expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + end end - it 'checks the presence of specific runner' do - project.runners << specific_runner - expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + context 'for shared runners enabled' do + let(:shared_runners_enabled) { true } + + it 'has a shared runner' do + shared_runner + expect(project.any_runners?).to be_truthy + end + + it 'checks the presence of shared runner' do + shared_runner + expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + end end end - context 'for shared runners enabled' do - let(:shared_runners_enabled) { true } + 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] } - it 'has a shared runner' do - shared_runner - expect(project.any_runners?).to be_truthy + context 'for group runners disabled' do + let(:group_runners_enabled) { false } + + it 'has no runners available' do + expect(project.any_runners?).to be_falsey + end + + it 'has a group runner, but they are prohibited to use' do + group_runner + expect(project.any_runners?).to be_falsey + end end - it 'checks the presence of shared runner' do - shared_runner - expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + context 'for group runners enabled' do + let(:group_runners_enabled) { true } + + it 'has a group runner' do + group_runner + expect(project.any_runners?).to be_truthy + end + + it 'checks the presence of group runner' do + group_runner + expect(project.any_runners? { |runner| runner == group_runner }).to be_truthy + end end end end @@ -3541,6 +3595,18 @@ describe Project do end end + describe '#toggle_settings!' do + it 'toggles the value on #settings' do + project = create :project, group_runners_enabled: false + + expect(project.group_runners_enabled).to be false + + project.toggle_settings!(:group_runners_enabled) + + expect(project.group_runners_enabled).to be true + end + end + describe '#gitlab_deploy_token' do let(:project) { create(:project) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3f2eb58f009..79447a65e94 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1787,14 +1787,12 @@ describe User do describe '#ci_authorized_runners' do let(:user) { create(:user) } - let(:runner) { create(:ci_runner) } + let(:runner_1) { create(:ci_runner) } + let(:runner_2) { create(:ci_runner) } - before do - project.runners << runner - end - - context 'without any projects' do - let(:project) { create(: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_authorized_runners).to be_empty @@ -1803,10 +1801,38 @@ describe User do context 'with personal projects runners' do let(:namespace) { create(:namespace, owner: user) } - let(:project) { create(:project, namespace: namespace) } + let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner) + expect(user.ci_authorized_runners).to contain_exactly(runner_1) + end + end + + context 'with personal group runner' do + let!(:project) { create(:project, runners: [runner_1]) } + let!(:group) do + create(:group, runners: [runner_2]).tap do |group| + group.add_owner(user) + end + end + + it 'loads' do + expect(user.ci_authorized_runners).to contain_exactly(runner_2) + 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!(:group) do + create(:group, runners: [runner_2]).tap do |group| + group.add_owner(user) + end + end + + it 'loads' do + expect(user.ci_authorized_runners).to contain_exactly(runner_1, runner_2) end end @@ -1817,7 +1843,7 @@ describe User do end it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner) + expect(user.ci_authorized_runners).to contain_exactly(runner_1) end end @@ -1834,7 +1860,21 @@ describe User do context 'with groups projects runners' do let(:group) { create(:group) } - let(:project) { create(:project, group: group) } + let!(:project) { create(:project, group: group, runners: [runner_1]) } + + def add_user(access) + group.add_user(user, access) + end + + it_behaves_like :member + end + + context 'with groups runners' do + let!(:group) do + create(:group, runners: [runner_1]).tap do |group| + group.add_owner(user) + end + end def add_user(access) group.add_user(user, access) @@ -1844,7 +1884,7 @@ describe User do end context 'with other projects runners' do - let(:project) { create(:project) } + let!(:project) { create(:project, runners: [runner_1]) } def add_user(access) project.add_role(user, access) diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 17c7a511857..5ea110b4d82 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -45,7 +45,7 @@ describe API::Runner do context 'when project token is used' do let(:project) { create(:project) } - it 'creates runner' do + it 'creates project runner' do post api('/runners'), token: project.runners_token expect(response).to have_gitlab_http_status 201 @@ -54,6 +54,19 @@ describe API::Runner do expect(Ci::Runner.first.token).not_to eq(project.runners_token) end end + + context 'when group token is used' do + let(:group) { create(:group) } + + it 'creates a group runner' do + post api('/runners'), token: group.runners_token + + expect(response).to have_http_status 201 + expect(group.runners.size).to eq(1) + expect(Ci::Runner.first.token).not_to eq(registration_token) + expect(Ci::Runner.first.token).not_to eq(group.runners_token) + end + end end context 'when runner description is provided' do diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index d30f0cf36e2..89e21ba9914 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -8,22 +8,27 @@ describe API::Runners do let(:project) { create(:project, creator_id: user.id) } let(:project2) { create(:project, creator_id: user.id) } - let!(:shared_runner) { create(:ci_runner, :shared) } - let!(:unused_specific_runner) { create(:ci_runner) } + let(:group) { create(:group).tap { |group| group.add_owner(user) } } + let(:group2) { create(:group).tap { |group| group.add_owner(user) } } - let!(:specific_runner) do - create(:ci_runner).tap do |runner| + 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).tap do |runner| + 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]) } + before do # Set project access for users create(:project_member, :master, user: user, project: project) @@ -37,9 +42,13 @@ describe API::Runners do get api('/runners', user) shared = json_response.any? { |r| r['is_shared'] } + descriptions = json_response.map { |runner| runner['description'] } expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array + expect(descriptions).to contain_exactly( + 'Project runner', 'Group runner', 'Two projects runner' + ) expect(shared).to be_falsey end @@ -129,10 +138,26 @@ describe API::Runners do context 'when runner is not shared' do it "returns runner's details" do - get api("/runners/#{specific_runner.id}", admin) + get api("/runners/#{project_runner.id}", admin) expect(response).to have_gitlab_http_status(200) - expect(json_response['description']).to eq(specific_runner.description) + expect(json_response['description']).to eq(project_runner.description) + end + + it "returns the project's details for a project runner" do + get api("/runners/#{project_runner.id}", admin) + + expect(json_response['projects'].first['id']).to eq(project.id) + end + + it "returns the group's details for a group runner" do + get api("/runners/#{group_runner.id}", admin) + + expect(json_response['groups'].first).to eq( + 'id' => group.id, + 'web_url' => group.web_url, + 'name' => group.name + ) end end @@ -146,10 +171,10 @@ describe API::Runners do context "runner project's administrative user" do context 'when runner is not shared' do it "returns runner's details" do - get api("/runners/#{specific_runner.id}", user) + get api("/runners/#{project_runner.id}", user) expect(response).to have_gitlab_http_status(200) - expect(json_response['description']).to eq(specific_runner.description) + expect(json_response['description']).to eq(project_runner.description) end end @@ -163,17 +188,40 @@ describe API::Runners do end end + context "runner group's administrative user" do + context 'when runner is not shared' do + it "returns runner's details" do + get api("/runners/#{group_runner.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['id']).to eq(group_runner.id) + end + end + end + context 'other authorized user' do - it "does not return runner's details" do - get api("/runners/#{specific_runner.id}", user2) + it "does not return project runner's details" do + get api("/runners/#{project_runner.id}", user2) + + expect(response).to have_http_status(403) + end + + it "does not return group runner's details" do + get api("/runners/#{group_runner.id}", user2) expect(response).to have_gitlab_http_status(403) end end context 'unauthorized user' do - it "does not return runner's details" do - get api("/runners/#{specific_runner.id}") + it "does not return project runner's details" do + get api("/runners/#{project_runner.id}") + + expect(response).to have_http_status(401) + end + + it "does not return group runner's details" do + get api("/runners/#{group_runner.id}") expect(response).to have_gitlab_http_status(401) end @@ -212,16 +260,16 @@ describe API::Runners do context 'when runner is not shared' do it 'updates runner' do - description = specific_runner.description - runner_queue_value = specific_runner.ensure_runner_queue_value + description = project_runner.description + runner_queue_value = project_runner.ensure_runner_queue_value - update_runner(specific_runner.id, admin, description: 'test') - specific_runner.reload + update_runner(project_runner.id, admin, description: 'test') + project_runner.reload expect(response).to have_gitlab_http_status(200) - expect(specific_runner.description).to eq('test') - expect(specific_runner.description).not_to eq(description) - expect(specific_runner.ensure_runner_queue_value) + expect(project_runner.description).to eq('test') + expect(project_runner.description).not_to eq(description) + expect(project_runner.ensure_runner_queue_value) .not_to eq(runner_queue_value) end end @@ -247,27 +295,49 @@ describe API::Runners do end context 'when runner is not shared' do - it 'does not update runner without access to it' do - put api("/runners/#{specific_runner.id}", user2), description: 'test' + it 'does not update project runner without access to it' do + put api("/runners/#{project_runner.id}", user2), description: 'test' + + expect(response).to have_http_status(403) + end + + it 'does not update group runner without access to it' do + put api("/runners/#{group_runner.id}", user2), description: 'test' expect(response).to have_gitlab_http_status(403) end - it 'updates runner with access to it' do - description = specific_runner.description - put api("/runners/#{specific_runner.id}", admin), description: 'test' - specific_runner.reload + it 'updates project runner with access to it' do + description = project_runner.description + put api("/runners/#{project_runner.id}", admin), description: 'test' + project_runner.reload expect(response).to have_gitlab_http_status(200) - expect(specific_runner.description).to eq('test') - expect(specific_runner.description).not_to eq(description) + expect(project_runner.description).to eq('test') + expect(project_runner.description).not_to eq(description) + end + + it 'updates group runner with access to it' do + description = group_runner.description + put api("/runners/#{group_runner.id}", admin), description: 'test' + group_runner.reload + + expect(response).to have_gitlab_http_status(200) + expect(group_runner.description).to eq('test') + expect(group_runner.description).not_to eq(description) end end end context 'unauthorized user' do - it 'does not delete runner' do - put api("/runners/#{specific_runner.id}") + it 'does not delete project runner' do + put api("/runners/#{project_runner.id}") + + expect(response).to have_http_status(401) + end + + it 'does not delete group runner' do + put api("/runners/#{group_runner.id}") expect(response).to have_gitlab_http_status(401) end @@ -293,15 +363,23 @@ describe API::Runners do context 'when runner is not shared' do it 'deletes unused runner' do expect do - delete api("/runners/#{unused_specific_runner.id}", admin) + 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 runner' do + it 'deletes used project runner' do + expect do + delete api("/runners/#{project_runner.id}", admin) + + expect(response).to have_http_status(204) + end.to change { Ci::Runner.specific.count }.by(-1) + end + + it 'deletes used group runner' do expect do - delete api("/runners/#{specific_runner.id}", admin) + delete api("/runners/#{group_runner.id}", admin) expect(response).to have_gitlab_http_status(204) end.to change { Ci::Runner.specific.count }.by(-1) @@ -325,32 +403,46 @@ describe API::Runners do context 'when runner is not shared' do it 'does not delete runner without access to it' do - delete api("/runners/#{specific_runner.id}", user2) + delete api("/runners/#{project_runner.id}", user2) expect(response).to have_gitlab_http_status(403) end - it 'does not delete runner with more than one associated project' do + it 'does not delete project runner with more than one associated project' do delete api("/runners/#{two_projects_runner.id}", user) expect(response).to have_gitlab_http_status(403) end - it 'deletes runner for one owned project' do + it 'deletes project runner for one owned project' do expect do - delete api("/runners/#{specific_runner.id}", user) + delete api("/runners/#{project_runner.id}", user) + + expect(response).to have_http_status(204) + end.to change { Ci::Runner.specific.count }.by(-1) + end + + it 'deletes group runner for one owned group' do + expect do + delete api("/runners/#{group_runner.id}", user) expect(response).to have_gitlab_http_status(204) end.to change { Ci::Runner.specific.count }.by(-1) end it_behaves_like '412 response' do - let(:request) { api("/runners/#{specific_runner.id}", user) } + let(:request) { api("/runners/#{project_runner.id}", user) } end end end context 'unauthorized user' do - it 'does not delete runner' do - delete api("/runners/#{specific_runner.id}") + it 'does not delete project runner' do + delete api("/runners/#{project_runner.id}") + + expect(response).to have_http_status(401) + end + + it 'does not delete group runner' do + delete api("/runners/#{group_runner.id}") expect(response).to have_gitlab_http_status(401) end @@ -361,8 +453,8 @@ describe API::Runners do set(:job_1) { create(:ci_build) } let!(:job_2) { create(:ci_build, :running, runner: shared_runner, project: project) } let!(:job_3) { create(:ci_build, :failed, runner: shared_runner, project: project) } - let!(:job_4) { create(:ci_build, :running, runner: specific_runner, project: project) } - let!(:job_5) { create(:ci_build, :failed, runner: specific_runner, project: project) } + let!(:job_4) { create(:ci_build, :running, runner: project_runner, project: project) } + let!(:job_5) { create(:ci_build, :failed, runner: project_runner, project: project) } context 'admin user' do context 'when runner exists' do @@ -380,7 +472,7 @@ describe API::Runners do context 'when runner is specific' do it 'return jobs' do - get api("/runners/#{specific_runner.id}/jobs", admin) + get api("/runners/#{project_runner.id}/jobs", admin) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers @@ -392,7 +484,7 @@ describe API::Runners do context 'when valid status is provided' do it 'return filtered jobs' do - get api("/runners/#{specific_runner.id}/jobs?status=failed", admin) + get api("/runners/#{project_runner.id}/jobs?status=failed", admin) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers @@ -405,7 +497,7 @@ describe API::Runners do context 'when invalid status is provided' do it 'return 400' do - get api("/runners/#{specific_runner.id}/jobs?status=non-existing", admin) + get api("/runners/#{project_runner.id}/jobs?status=non-existing", admin) expect(response).to have_gitlab_http_status(400) end @@ -433,7 +525,7 @@ describe API::Runners do context 'when runner is specific' do it 'return jobs' do - get api("/runners/#{specific_runner.id}/jobs", user) + get api("/runners/#{project_runner.id}/jobs", user) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers @@ -445,7 +537,7 @@ describe API::Runners do context 'when valid status is provided' do it 'return filtered jobs' do - get api("/runners/#{specific_runner.id}/jobs?status=failed", user) + get api("/runners/#{project_runner.id}/jobs?status=failed", user) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers @@ -458,7 +550,7 @@ describe API::Runners do context 'when invalid status is provided' do it 'return 400' do - get api("/runners/#{specific_runner.id}/jobs?status=non-existing", user) + get api("/runners/#{project_runner.id}/jobs?status=non-existing", user) expect(response).to have_gitlab_http_status(400) end @@ -476,7 +568,7 @@ describe API::Runners do context 'other authorized user' do it 'does not return jobs' do - get api("/runners/#{specific_runner.id}/jobs", user2) + get api("/runners/#{project_runner.id}/jobs", user2) expect(response).to have_gitlab_http_status(403) end @@ -484,7 +576,7 @@ describe API::Runners do context 'unauthorized user' do it 'does not return jobs' do - get api("/runners/#{specific_runner.id}/jobs") + get api("/runners/#{project_runner.id}/jobs") expect(response).to have_gitlab_http_status(401) end @@ -523,7 +615,7 @@ describe API::Runners do describe 'POST /projects/:id/runners' do context 'authorized user' do - let(:specific_runner2) do + let(:project_runner2) do create(:ci_runner).tap do |runner| create(:ci_runner_project, runner: runner, project: project2) end @@ -531,23 +623,23 @@ describe API::Runners do it 'enables specific runner' do expect do - post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id + post api("/projects/#{project.id}/runners", user), runner_id: project_runner2.id end.to change { project.runners.count }.by(+1) expect(response).to have_gitlab_http_status(201) end it 'avoids changes when enabling already enabled runner' do expect do - post api("/projects/#{project.id}/runners", user), runner_id: specific_runner.id + 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) end it 'does not enable locked runner' do - specific_runner2.update(locked: true) + project_runner2.update(locked: true) expect do - post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id + post api("/projects/#{project.id}/runners", user), runner_id: project_runner2.id end.to change { project.runners.count }.by(0) expect(response).to have_gitlab_http_status(403) @@ -559,10 +651,16 @@ describe API::Runners do expect(response).to have_gitlab_http_status(403) end + it 'does not enable group runner' do + post api("/projects/#{project.id}/runners", user), runner_id: group_runner.id + + expect(response).to have_http_status(403) + end + context 'user is admin' do it 'enables any specific runner' do expect do - post api("/projects/#{project.id}/runners", admin), runner_id: unused_specific_runner.id + 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) end @@ -570,7 +668,7 @@ describe API::Runners do 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_specific_runner.id + post api("/projects/#{project.id}/runners", user), runner_id: unused_project_runner.id expect(response).to have_gitlab_http_status(403) end @@ -619,7 +717,7 @@ describe API::Runners do context 'when runner have one associated projects' do it "does not disable project's runner" do expect do - delete api("/projects/#{project.id}/runners/#{specific_runner.id}", user) + delete api("/projects/#{project.id}/runners/#{project_runner.id}", user) end.to change { project.runners.count }.by(0) expect(response).to have_gitlab_http_status(403) end @@ -634,7 +732,7 @@ describe API::Runners do context 'authorized user without permissions' do it "does not disable project's runner" do - delete api("/projects/#{project.id}/runners/#{specific_runner.id}", user2) + delete api("/projects/#{project.id}/runners/#{project_runner.id}", user2) expect(response).to have_gitlab_http_status(403) end @@ -642,7 +740,7 @@ describe API::Runners do context 'unauthorized user' do it "does not disable project's runner" do - delete api("/projects/#{project.id}/runners/#{specific_runner.id}") + delete api("/projects/#{project.id}/runners/#{project_runner.id}") expect(response).to have_gitlab_http_status(401) end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 8a537e83d5f..0c343425392 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' module Ci describe RegisterJobService do - let!(:project) { FactoryBot.create :project, shared_runners_enabled: false } - let!(:pipeline) { FactoryBot.create :ci_pipeline, project: project } - let!(:pending_job) { FactoryBot.create :ci_build, pipeline: pipeline } - let!(:shared_runner) { FactoryBot.create(:ci_runner, is_shared: true) } - let!(:specific_runner) { FactoryBot.create(:ci_runner, is_shared: false) } + let!(:project) { create :project, shared_runners_enabled: false } + let!(:group) { create :group } + let!(:pipeline) { create :ci_pipeline, project: project } + let!(:pending_job) { create :ci_build, pipeline: pipeline } + 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] } before do specific_runner.assign_to(project) @@ -167,6 +169,108 @@ module Ci end end + context 'allow group runners' do + before do + project.update!(group_runners_enabled: true, group: group) + 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!(: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 } + + # 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] } + + it 'does not consider builds from other group runners' do + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 6 + execute(group_runner) + + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 5 + execute(group_runner) + + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 4 + execute(group_runner) + + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 3 + execute(group_runner) + + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 2 + execute(group_runner) + + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 1 + execute(group_runner) + + expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 0 + expect(execute(group_runner)).to be_nil + end + + it 'prefers projects without builds first' do + # it gets for one build from each of the projects + expect(execute(group_runner)).to eq(build1_project1) + expect(execute(group_runner)).to eq(build1_project2) + expect(execute(group_runner)).to eq(build1_project3) + + # then it gets a second build from each of the projects + expect(execute(group_runner)).to eq(build2_project1) + expect(execute(group_runner)).to eq(build2_project2) + + # in the end the third build + expect(execute(group_runner)).to eq(build3_project1) + + expect(execute(group_runner)).to be_nil + end + + it 'equalises number of running builds' do + # after finishing the first build for project 1, get a second build from the same project + expect(execute(group_runner)).to eq(build1_project1) + build1_project1.reload.success + expect(execute(group_runner)).to eq(build2_project1) + + expect(execute(group_runner)).to eq(build1_project2) + build1_project2.reload.success + expect(execute(group_runner)).to eq(build2_project2) + expect(execute(group_runner)).to eq(build1_project3) + expect(execute(group_runner)).to eq(build3_project1) + + expect(execute(group_runner)).to be_nil + end + end + + context 'group runner' do + let(:build) { execute(group_runner) } + + it { expect(build).to be_kind_of(Build) } + it { expect(build).to be_valid } + it { expect(build).to be_running } + it { expect(build.runner).to eq(group_runner) } + end + end + + context 'disallow group runners' do + before do + project.update(group_runners_enabled: false) + end + + context 'group runner' do + let(:build) { execute(group_runner) } + + it { expect(build).to be_nil } + end + end + context 'when first build is stalled' do before do pending_job.update(lock_version: 0) @@ -178,7 +282,7 @@ module Ci let!(:other_build) { create :ci_build, pipeline: pipeline } before do - allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) + allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner) .and_return(Ci::Build.where(id: [pending_job, other_build])) end @@ -190,7 +294,7 @@ module Ci context 'when single build is in queue' do before do - allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) + allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner) .and_return(Ci::Build.where(id: pending_job)) end @@ -201,7 +305,7 @@ module Ci context 'when there is no build in queue' do before do - allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) + allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner) .and_return(Ci::Build.none) end diff --git a/spec/services/ci/update_build_queue_service_spec.rb b/spec/services/ci/update_build_queue_service_spec.rb index 0da0e57dbcd..74a23ed2a3f 100644 --- a/spec/services/ci/update_build_queue_service_spec.rb +++ b/spec/services/ci/update_build_queue_service_spec.rb @@ -8,21 +8,19 @@ describe Ci::UpdateBuildQueueService do context 'when updating specific runners' do let(:runner) { create(:ci_runner) } - context 'when there are runner that can pick build' do + 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 } + expect { subject.execute(build) }.to change { runner.ensure_runner_queue_value } end end - context 'when there are no runners that can pick build' do + context 'when there is no runner that can pick build' do it 'does not tick runner queue value' do - expect { subject.execute(build) } - .not_to change { runner.ensure_runner_queue_value } + expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value } end end end @@ -30,21 +28,61 @@ describe Ci::UpdateBuildQueueService do context 'when updating shared runners' do let(:runner) { create(:ci_runner, :shared) } - context 'when there are runner that can pick build' do + context 'when there is no runner that can pick build' do it 'ticks runner queue value' do - expect { subject.execute(build) } - .to change { runner.ensure_runner_queue_value } + expect { subject.execute(build) }.to change { runner.ensure_runner_queue_value } end end - context 'when there are no runners that can pick build' do + context 'when there is no runner that can pick build due to tag mismatch' do before do build.tag_list = [:docker] end it 'does not tick runner queue value' do - expect { subject.execute(build) } - .not_to change { runner.ensure_runner_queue_value } + expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value } + end + end + + context 'when there is no runner that can pick build due to being disabled on project' do + before do + build.project.shared_runners_enabled = false + end + + it 'does not tick runner queue value' do + expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value } + end + end + end + + context 'when updating group runners' do + let(:group) { create :group } + let(:project) { create :project, group: group } + let(:runner) { create :ci_runner, groups: [group] } + + context 'when there is a runner that can pick build' do + 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 due to tag mismatch' do + before do + build.tag_list = [:docker] + end + + it 'does not tick runner queue value' do + expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value } + end + end + + context 'when there is no runner that can pick build due to being disabled on project' do + before do + build.project.group_runners_enabled = false + end + + it 'does not tick runner queue value' do + expect { subject.execute(build) }.not_to change { runner.ensure_runner_queue_value } end end end |