From 6b0bfda8ac0c2eebfa0e89dd0d37c5fc58160c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Tue, 17 Jul 2018 12:58:57 +0200 Subject: [PATCH] Add `runner_unsupported` CI failure --- app/models/ci/build.rb | 18 ++++++++++----- app/models/commit_status.rb | 3 ++- app/presenters/commit_status_presenter.rb | 5 +++-- app/services/ci/register_job_service.rb | 26 ++++++++++++++++------ lib/api/runner.rb | 10 ++++++++- lib/gitlab/ci/status/build/failed.rb | 3 ++- spec/presenters/ci/build_presenter_spec.rb | 2 +- 7 files changed, 49 insertions(+), 18 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 35b20bc1e0b..5a6af97af62 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -174,10 +174,6 @@ module Ci end end - before_transition any => [:running] do |build| - build.validates_dependencies! unless Feature.enabled?('ci_disable_validates_dependencies') - end - after_transition pending: :running do |build| build.ensure_metadata.update_timeout_state end @@ -343,6 +339,10 @@ module Ci { trace_sections: true } end + def runner_required_features + %w(variables) + end + def merge_request return @merge_request if defined?(@merge_request) @@ -581,7 +581,9 @@ module Ci options[:dependencies]&.empty? end - def validates_dependencies! + def valid_build_dependencies? + return unless Feature.enabled?('ci_disable_validates_dependencies') + dependencies.each do |dependency| raise MissingDependenciesError unless dependency.valid_dependency? end @@ -594,6 +596,12 @@ module Ci true end + def supported_runner?(features) + runner_required_features.all? do |feature_name| + features[feature_name] + end + end + def hide_secrets(trace) return unless trace diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 97516079b66..638237498ae 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -46,7 +46,8 @@ class CommitStatus < ActiveRecord::Base api_failure: 2, stuck_or_timeout_failure: 3, runner_system_failure: 4, - missing_dependency_failure: 5 + missing_dependency_failure: 5, + runner_unsupported: 6, } ## diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index 3a9088cfcb8..3ac9c00602c 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -6,7 +6,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated api_failure: 'There has been an API failure, please try again', stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again', runner_system_failure: 'There has been a runner system failure, please try again', - missing_dependency_failure: 'There has been a missing dependency failure' + missing_dependency_failure: 'There has been a missing dependency failure', + # COMMENTED to check if tests gonna fail: runner_unsupported: 'Your runner is unsupported. Upgrade runner to use new features of your Pipeline', }.freeze presents :build @@ -20,6 +21,6 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated end def unrecoverable? - script_failure? || missing_dependency_failure? + script_failure? || missing_dependency_failure? || runner_unsupported? end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index f7ccec3a700..51af39e52a1 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -41,16 +41,10 @@ module Ci begin # In case when 2 runners try to assign the same build, second runner will be declined # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. - begin - build.runner_id = runner.id - build.runner_session_attributes = params[:session] if params[:session].present? - - build.run! + if assign_runner!(build, params) register_success(build) return Result.new(build, true) # rubocop:disable Cop/AvoidReturnFromBlocks - rescue Ci::Build::MissingDependenciesError - build.drop!(:missing_dependency_failure) end rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError # We are looping to find another build that is not conflicting @@ -72,6 +66,24 @@ module Ci private + def assign_runner!(build, params) + build.runner_id = runner.id + build.runner_session_attributes = params[:session] if params[:session].present? + + unless build.valid_build_dependencies? + build.drop!(:missing_dependency_failure) + return false + end + + unless build.supported_runner?(params.dig(:info, :features)) + build.drop!(:runner_unsupported) + return false + end + + build.run! + return true + end + def builds_for_shared_runner new_builds. # don't run projects which have not enabled shared runners and builds diff --git a/lib/api/runner.rb b/lib/api/runner.rb index c7595493e11..c9931c2d603 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -80,7 +80,15 @@ module API params do requires :token, type: String, desc: %q(Runner's authentication token) optional :last_update, type: String, desc: %q(Runner's queue last_update token) - optional :info, type: Hash, desc: %q(Runner's metadata) + optional :info, type: Hash, desc: %q(Runner's metadata) do + optional :name, type: String, desc: %q(Runner's name) + optional :version, type: String, desc: %q(Runner's version) + optional :revision, type: String, desc: %q(Runner's revision) + optional :platform, type: String, desc: %q(Runner's platform) + optional :architecture, type: String, desc: %q(Runner's architecture) + optional :executor, type: String, desc: %q(Runner's executor) + optional :features, type: Hash, desc: %q(Runner's features) + end optional :session, type: Hash, desc: %q(Runner's session data) do optional :url, type: String, desc: %q(Session's url) optional :certificate, type: String, desc: %q(Session's certificate) diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb index 155f4fc1343..e87f5e42aa8 100644 --- a/lib/gitlab/ci/status/build/failed.rb +++ b/lib/gitlab/ci/status/build/failed.rb @@ -9,7 +9,8 @@ module Gitlab 'api_failure' => 'API failure', 'stuck_or_timeout_failure' => 'stuck or timeout failure', 'runner_system_failure' => 'runner system failure', - 'missing_dependency_failure' => 'missing dependency failure' + 'missing_dependency_failure' => 'missing dependency failure', + # COMMENTED to check if CI fails: 'runner_unsupported' => 'unsuported runner', }.freeze def status_tooltip diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb index 6dfaa3b72f7..412063378eb 100644 --- a/spec/presenters/ci/build_presenter_spec.rb +++ b/spec/presenters/ci/build_presenter_spec.rb @@ -231,7 +231,7 @@ describe Ci::BuildPresenter do let(:build) { create(:ci_build, :failed, :script_failure) } context 'when is a script or missing dependency failure' do - let(:failure_reasons) { %w(script_failure missing_dependency_failure) } + let(:failure_reasons) { %w(script_failure missing_dependency_failure runner_unsupported) } it 'should return false' do failure_reasons.each do |failure_reason| -- GitLab