Merge branch 'fix-ci-requests-concurrency' into 'master'

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

changelogs/unreleased/fix-ci-requests-concurrency-for-newer-runners.yml

+---
+title: 'Fix CI requests concurrency for newer runners that prevents from picking pending builds (from 1.9.0-rc5)'
+merge_request: 8760

lib/ci/api/builds.rb

@@ -18,18 +18,20 @@ 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
+          if result.valid?
+            if result.build
               Gitlab::Metrics.add_event(:build_found,
-                                      project: build.project.path_with_namespace)
+                                        project: result.build.project.path_with_namespace)
 
-            present build, with: Entities::BuildDetails
+              present result.build, with: Entities::BuildDetails
             else
               Gitlab::Metrics.add_event(:build_not_found)
 
@@ -37,6 +39,11 @@ module Ci
 
               build_not_found!
             end
+          else
+            # We received build that is invalid due to concurrency conflict
+            Gitlab::Metrics.add_event(:build_invalid)
+            conflict!
+          end
         end
 
         # Update an existing build - Runners only

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',

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