diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-01-25 21:14:56 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2017-01-25 21:14:56 +0300 |
commit | 637894de8868b05499a275e1685621473addef58 (patch) | |
tree | 017f01e30effce8f80cfae61be5296e318f55cdb | |
parent | 6cfe60df2275dace9804f4bc37b9746a9eadc6fd (diff) | |
parent | 31be74c74b72a1cd33e1630f9462ab5c43f2e319 (diff) |
Merge branch 'fix-ci-requests-concurrency' into 'master'
Fix CI requests concurrency
See merge request !8760
-rw-r--r-- | app/services/ci/register_build_service.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/fix-ci-requests-concurrency-for-newer-runners.yml | 3 | ||||
-rw-r--r-- | lib/ci/api/builds.rb | 25 | ||||
-rw-r--r-- | spec/requests/ci/api/builds_spec.rb | 14 | ||||
-rw-r--r-- | spec/services/ci/register_build_service_spec.rb | 2 |
5 files changed, 38 insertions, 12 deletions
diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb index cd548b3c8d5..6f03bf2be13 100644 --- a/app/services/ci/register_build_service.rb +++ b/app/services/ci/register_build_service.rb @@ -6,6 +6,8 @@ module Ci attr_reader :runner + Result = Struct.new(:build, :valid?) + def initialize(runner) @runner = runner end @@ -29,10 +31,10 @@ module Ci build.run! end - build + Result.new(build, true) rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError - nil + Result.new(build, false) end private diff --git a/changelogs/unreleased/fix-ci-requests-concurrency-for-newer-runners.yml b/changelogs/unreleased/fix-ci-requests-concurrency-for-newer-runners.yml new file mode 100644 index 00000000000..075d74b9cb8 --- /dev/null +++ b/changelogs/unreleased/fix-ci-requests-concurrency-for-newer-runners.yml @@ -0,0 +1,3 @@ +--- +title: 'Fix CI requests concurrency for newer runners that prevents from picking pending builds (from 1.9.0-rc5)' +merge_request: 8760 diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index a9da8ea7eeb..8b939663ffd 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -18,24 +18,31 @@ module Ci if current_runner.is_runner_queue_value_latest?(params[:last_update]) header 'X-GitLab-Last-Update', params[:last_update] + Gitlab::Metrics.add_event(:build_not_found_cached) return build_not_found! end new_update = current_runner.ensure_runner_queue_value - build = Ci::RegisterBuildService.new(current_runner).execute + result = Ci::RegisterBuildService.new(current_runner).execute - if build - Gitlab::Metrics.add_event(:build_found, - project: build.project.path_with_namespace) + if result.valid? + if result.build + Gitlab::Metrics.add_event(:build_found, + project: result.build.project.path_with_namespace) - present build, with: Entities::BuildDetails - else - Gitlab::Metrics.add_event(:build_not_found) + present result.build, with: Entities::BuildDetails + else + Gitlab::Metrics.add_event(:build_not_found) - header 'X-GitLab-Last-Update', new_update + header 'X-GitLab-Last-Update', new_update - build_not_found! + build_not_found! + end + else + # We received build that is invalid due to concurrency conflict + Gitlab::Metrics.add_event(:build_invalid) + conflict! end end diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 270c23e3f19..8dbe5f0b025 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -91,6 +91,20 @@ describe Ci::API::Builds do expect { register_builds }.to change { runner.reload.contacted_at } end + context 'when concurrently updating build' do + before do + expect_any_instance_of(Ci::Build).to receive(:run!). + and_raise(ActiveRecord::StaleObjectError.new(nil, nil)) + end + + it 'returns a conflict' do + register_builds info: { platform: :darwin } + + expect(response).to have_http_status(409) + expect(response.headers).not_to have_key('X-GitLab-Last-Update') + end + end + context 'registry credentials' do let(:registry_credentials) do { 'type' => 'registry', diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb index 27d7853bbdd..d9f774a1095 100644 --- a/spec/services/ci/register_build_service_spec.rb +++ b/spec/services/ci/register_build_service_spec.rb @@ -171,7 +171,7 @@ module Ci end def execute(runner) - described_class.new(runner).execute + described_class.new(runner).execute.build end end end |