diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/ci/admin/runners_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/projects/runners_controller.rb | 2 | ||||
| -rw-r--r-- | app/helpers/runners_helper.rb | 26 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 12 | ||||
| -rw-r--r-- | app/models/ci/project.rb | 6 | ||||
| -rw-r--r-- | app/models/ci/runner.rb | 17 | ||||
| -rw-r--r-- | app/models/commit_status.rb | 4 | ||||
| -rw-r--r-- | app/services/ci/register_build_service.rb | 2 | ||||
| -rw-r--r-- | app/views/projects/builds/show.html.haml | 23 | ||||
| -rw-r--r-- | app/views/projects/commit_statuses/_commit_status.html.haml | 3 | ||||
| -rw-r--r-- | spec/helpers/runners_helper_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/build_spec.rb | 101 | ||||
| -rw-r--r-- | spec/models/ci/project_spec.rb | 13 | ||||
| -rw-r--r-- | spec/models/ci/runner_spec.rb | 65 |
15 files changed, 257 insertions, 24 deletions
diff --git a/CHANGELOG b/CHANGELOG index 0a38485b7b7..07ff3d6de42 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -28,6 +28,7 @@ v 8.1.0 (unreleased) - Move CI triggers page to project settings area - Move CI project settings page to CE project settings area - Fix bug when removed file was not appearing in merge request diff + - Show warning when build cannot be served by any of the available CI runners - Note the original location of a moved project when notifying users of the move - Improve error message when merging fails - Add support of multibyte characters in LDAP UID (Roman Petrov) diff --git a/app/controllers/ci/admin/runners_controller.rb b/app/controllers/ci/admin/runners_controller.rb index 9a68add9083..110954a612d 100644 --- a/app/controllers/ci/admin/runners_controller.rb +++ b/app/controllers/ci/admin/runners_controller.rb @@ -6,7 +6,7 @@ module Ci @runners = Ci::Runner.order('id DESC') @runners = @runners.search(params[:search]) if params[:search].present? @runners = @runners.page(params[:page]).per(30) - @active_runners_cnt = Ci::Runner.where("contacted_at > ?", 1.minutes.ago).count + @active_runners_cnt = Ci::Runner.online.count end def show @@ -66,7 +66,7 @@ module Ci end def runner_params - params.require(:runner).permit(:token, :description, :tag_list, :contacted_at, :active) + params.require(:runner).permit(:token, :description, :tag_list, :active) end end end diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index 6cb6e3ef6d4..deb07a21416 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -60,6 +60,6 @@ class Projects::RunnersController < Projects::ApplicationController end def runner_params - params.require(:runner).permit(:description, :tag_list, :contacted_at, :active) + params.require(:runner).permit(:description, :tag_list, :active) end end diff --git a/app/helpers/runners_helper.rb b/app/helpers/runners_helper.rb index 5d7d06c8490..5afebab88e1 100644 --- a/app/helpers/runners_helper.rb +++ b/app/helpers/runners_helper.rb @@ -1,20 +1,16 @@ module RunnersHelper def runner_status_icon(runner) - unless runner.contacted_at - return content_tag :i, nil, - class: "fa fa-warning-sign", - title: "New runner. Has not connected yet" - end - - status = - if runner.active? - runner.contacted_at > 3.hour.ago ? :online : :offline - else - :paused - end + status = runner.status + case status + when :not_connected + content_tag :i, nil, + class: "fa fa-warning", + title: "New runner. Has not connected yet" - content_tag :i, nil, - class: "fa fa-circle runner-status-#{status}", - title: "Runner is #{status}, last contact was #{time_ago_in_words(runner.contacted_at)} ago" + when :online, :offline, :paused + content_tag :i, nil, + class: "fa fa-circle runner-status-#{status}", + title: "Runner is #{status}, last contact was #{time_ago_in_words(runner.contacted_at)} ago" + end end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 23aed6d9952..5f8d44148ca 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -231,6 +231,18 @@ module Ci end end + def can_be_served?(runner) + (tag_list - runner.tag_list).empty? + end + + def any_runners_online? + project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) } + end + + def show_warning? + pending? && !any_runners_online? + end + private def yaml_variables diff --git a/app/models/ci/project.rb b/app/models/ci/project.rb index 88ba933a434..ef28353a30c 100644 --- a/app/models/ci/project.rb +++ b/app/models/ci/project.rb @@ -115,12 +115,12 @@ module Ci web_url end - def any_runners? - if runners.active.any? + def any_runners?(&block) + if runners.active.any?(&block) return true end - shared_runners_enabled && Ci::Runner.shared.active.any? + shared_runners_enabled && Ci::Runner.shared.active.any?(&block) end def set_default_values diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 6838ccfaaab..02a3e9db1fa 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -20,6 +20,8 @@ module Ci class Runner < ActiveRecord::Base extend Ci::Model + + LAST_CONTACT_TIME = 5.minutes.ago has_many :builds, class_name: 'Ci::Build' has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject' @@ -33,6 +35,7 @@ module Ci scope :shared, ->() { where(is_shared: true) } scope :active, ->() { where(active: true) } scope :paused, ->() { where(active: false) } + scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) } acts_as_taggable @@ -65,6 +68,20 @@ module Ci is_shared end + def online? + contacted_at && contacted_at > LAST_CONTACT_TIME + end + + def status + if contacted_at.nil? + :not_connected + elsif active? + online? ? :online : :offline + else + :paused + end + end + def belongs_to_one_project? runner_projects.count == 1 end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index b4d91b1b0c3..92905c618eb 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -88,4 +88,8 @@ class CommitStatus < ActiveRecord::Base def retry_url nil end + + def show_warning? + false + end end diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb index 71b61bbe389..7beb098659c 100644 --- a/app/services/ci/register_build_service.rb +++ b/app/services/ci/register_build_service.rb @@ -17,7 +17,7 @@ module Ci builds = builds.order('created_at ASC') build = builds.find do |build| - (build.tag_list - current_runner.tag_list).empty? + build.can_be_served?(current_runner) end diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 9c3ae622b72..91c1b16c9f6 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -25,7 +25,7 @@ %a Build ##{@build.id} · - %i.fa.fa-warning-sign + %i.fa.fa-warning This build was retried. .gray-content-block.second-block @@ -39,6 +39,27 @@ .pull-right = @build.updated_at.stamp('19:00 Aug 27') + - if @build.show_warning? + - unless @build.any_runners_online? + .bs-callout.bs-callout-warning + %p + - if no_runners_for_project?(@build.project) + This build is stuck, because the project doesn't have runners assigned. + - elsif @build.tags.any? + This build is stuck. + %br + This build is stuck, because you don't have any active runners online with these tags assigned to the project: + - @build.tags.each do |tag| + %span.label.label-primary + = tag + - else + This build is stuck, because you don't have any active runners online that can run this build. + + %br + Go to + = link_to namespace_project_runners_path(@build.gl_project.namespace, @build.gl_project) do + Runners page + .row.prepend-top-default .col-md-9 .clearfix diff --git a/app/views/projects/commit_statuses/_commit_status.html.haml b/app/views/projects/commit_statuses/_commit_status.html.haml index 7314f8e79d3..637154f56aa 100644 --- a/app/views/projects/commit_statuses/_commit_status.html.haml +++ b/app/views/projects/commit_statuses/_commit_status.html.haml @@ -9,6 +9,9 @@ - else %strong Build ##{commit_status.id} + - if commit_status.show_warning? + %i.fa.fa-warning.text-warning + %td = commit_status.ref diff --git a/spec/helpers/runners_helper_spec.rb b/spec/helpers/runners_helper_spec.rb index b3d635a1932..35f91b7decf 100644 --- a/spec/helpers/runners_helper_spec.rb +++ b/spec/helpers/runners_helper_spec.rb @@ -12,7 +12,7 @@ describe RunnersHelper do end it "returns online text" do - runner = FactoryGirl.build(:ci_runner, contacted_at: 1.hour.ago, active: true) + runner = FactoryGirl.build(:ci_runner, contacted_at: 1.second.ago, active: true) expect(runner_status_icon(runner)).to include("Runner is online") end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 7f999581ec7..7f5abb83ac2 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -299,4 +299,105 @@ describe Ci::Build do is_expected.to eq(['rec1', pusher_email]) end end + + describe :can_be_served? do + let(:runner) { FactoryGirl.create :ci_specific_runner } + + before { build.project.runners << runner } + + context 'runner without tags' do + it 'can handle builds without tags' do + expect(build.can_be_served?(runner)).to be_truthy + end + + it 'cannot handle build with tags' do + build.tag_list = ['aa'] + expect(build.can_be_served?(runner)).to be_falsey + end + end + + context 'runner with tags' do + before { runner.tag_list = ['bb', 'cc'] } + + it 'can handle builds without tags' do + expect(build.can_be_served?(runner)).to be_truthy + end + + it 'can handle build with matching tags' do + build.tag_list = ['bb'] + expect(build.can_be_served?(runner)).to be_truthy + end + + it 'cannot handle build with not matching tags' do + build.tag_list = ['aa'] + expect(build.can_be_served?(runner)).to be_falsey + end + end + end + + describe :any_runners_online? do + subject { build.any_runners_online? } + + context 'when no runners' do + it { is_expected.to be_falsey } + end + + context 'if there are runner' do + let(:runner) { FactoryGirl.create :ci_specific_runner } + + before do + build.project.runners << runner + runner.update_attributes(contacted_at: 1.second.ago) + end + + it { is_expected.to be_truthy } + + it 'that is inactive' do + runner.update_attributes(active: false) + is_expected.to be_falsey + end + + it 'that is not online' do + runner.update_attributes(contacted_at: nil) + is_expected.to be_falsey + end + + it 'that cannot handle build' do + expect_any_instance_of(Ci::Build).to receive(:can_be_served?).and_return(false) + is_expected.to be_falsey + end + + end + end + + describe :show_warning? do + subject { build.show_warning? } + + %w(pending).each do |state| + context "if commit_status.status is #{state}" do + before { build.status = state } + + it { is_expected.to be_truthy } + + context "and there are specific runner" do + let(:runner) { FactoryGirl.create :ci_specific_runner, contacted_at: 1.second.ago } + + before do + build.project.runners << runner + runner.save + end + + it { is_expected.to be_falsey } + end + end + end + + %w(success failed canceled running).each do |state| + context "if commit_status.status is #{state}" do + before { build.status = state } + + it { is_expected.to be_falsey } + end + end + end end diff --git a/spec/models/ci/project_spec.rb b/spec/models/ci/project_spec.rb index dec4720a711..c1605d68859 100644 --- a/spec/models/ci/project_spec.rb +++ b/spec/models/ci/project_spec.rb @@ -259,5 +259,18 @@ describe Ci::Project do FactoryGirl.create(:ci_shared_runner) expect(project.any_runners?).to be_falsey end + + it "checks the presence of specific runner" do + project = FactoryGirl.create(:ci_project) + specific_runner = FactoryGirl.create(:ci_specific_runner) + project.runners << specific_runner + expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + end + + it "checks the presence of shared runner" do + project = FactoryGirl.create(:ci_project, shared_runners_enabled: true) + shared_runner = FactoryGirl.create(:ci_shared_runner) + expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + end end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 757593a7ab8..536a737a33d 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -48,6 +48,71 @@ describe Ci::Runner do it { expect(shared_runner.only_for?(project)).to be_truthy } end + describe :online do + subject { Ci::Runner.online } + + before do + @runner1 = FactoryGirl.create(:ci_shared_runner, contacted_at: 1.year.ago) + @runner2 = FactoryGirl.create(:ci_shared_runner, contacted_at: 1.second.ago) + end + + it { is_expected.to eq([@runner2])} + end + + describe :online? do + let(:runner) { FactoryGirl.create(:ci_shared_runner) } + + subject { runner.online? } + + context 'never contacted' do + before { runner.contacted_at = nil } + + it { is_expected.to be_falsey } + end + + context 'contacted long time ago time' do + before { runner.contacted_at = 1.year.ago } + + it { is_expected.to be_falsey } + end + + context 'contacted 1s ago' do + before { runner.contacted_at = 1.second.ago } + + it { is_expected.to be_truthy } + end + end + + describe :status do + let(:runner) { FactoryGirl.create(:ci_shared_runner, contacted_at: 1.second.ago) } + + subject { runner.status } + + context 'never connected' do + before { runner.contacted_at = nil } + + it { is_expected.to eq(:not_connected) } + end + + context 'contacted 1s ago' do + before { runner.contacted_at = 1.second.ago } + + it { is_expected.to eq(:online) } + end + + context 'contacted long time ago' do + before { runner.contacted_at = 1.year.ago } + + it { is_expected.to eq(:offline) } + end + + context 'inactive' do + before { runner.active = false } + + it { is_expected.to eq(:paused) } + end + end + describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do runner = FactoryGirl.create(:ci_specific_runner) |