Merge remote-tracking branch 'upstream/master' into feature/runner-lock-on-project

* upstream/master: (337 commits)
  Update CHANGELOG for !4659
  Center the header logo for all Devise emails
  Add previews for all customized Devise emails
  Customize the Devise `unlock_instructions` email
  Customize the Devise `reset_password_instructions` email
  Customize the Devise `password_change` emails
  Use gitlab-git 10.2.0
  Use Git cached counters on project show page
  Fix indentation scss-lint errors
  Added title attribute to enties in tree view Closes #18353
  Banzai::Filter::ExternalLinkFilter use XPath
  Reduce queries in IssueReferenceFilter
  Use gitlab_git 10.1.4
  Fixed ordering in Project.find_with_namespace
  Fix images in emails
  Banzai::Filter::UploadLinkFilter use XPath
  Turn Group#owners into a has_many association
  Make project_id nullable
  CHANGELOG [ci skip]
  CHANGELOG [ci skip]
  ...

3 files changed, 57 insertions, 43 deletions

diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb
index 64bcdac5c65..2dcb052d274 100644
--- a/app/services/ci/create_builds_service.rb
+++ b/app/services/ci/create_builds_service.rb
@@ -2,10 +2,11 @@ module Ci
   class CreateBuildsService
     def initialize(pipeline)
       @pipeline = pipeline
+      @config = pipeline.config_processor
     end
 
     def execute(stage, user, status, trigger_request = nil)
-      builds_attrs = config_processor.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request)
+      builds_attrs = @config.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request)
 
       # check when to create next build
       builds_attrs = builds_attrs.select do |build_attrs|
@@ -19,33 +20,37 @@ module Ci
         end
       end
 
+      # don't create the same build twice
+      builds_attrs.reject! do |build_attrs|
+        @pipeline.builds.find_by(ref: @pipeline.ref,
+                                 tag: @pipeline.tag,
+                                 trigger_request: trigger_request,
+                                 name: build_attrs[:name])
+      end
+
       builds_attrs.map do |build_attrs|
-        # don't create the same build twice
-        unless @pipeline.builds.find_by(ref: @pipeline.ref, tag: @pipeline.tag,
-                                        trigger_request: trigger_request, name: build_attrs[:name])
-          build_attrs.slice!(:name,
-                             :commands,
-                             :tag_list,
-                             :options,
-                             :allow_failure,
-                             :stage,
-                             :stage_idx)
+        build_attrs.slice!(:name,
+                           :commands,
+                           :tag_list,
+                           :options,
+                           :allow_failure,
+                           :stage,
+                           :stage_idx,
+                           :environment)
 
-          build_attrs.merge!(ref: @pipeline.ref,
-                             tag: @pipeline.tag,
-                             trigger_request: trigger_request,
-                             user: user,
-                             project: @pipeline.project)
+        build_attrs.merge!(pipeline: @pipeline,
+                           ref: @pipeline.ref,
+                           tag: @pipeline.tag,
+                           trigger_request: trigger_request,
+                           user: user,
+                           project: @pipeline.project)
 
-          @pipeline.builds.create!(build_attrs)
-        end
+        ##
+        # We do not persist new builds here.
+        # Those will be persisted when @pipeline is saved.
+        #
+        @pipeline.builds.new(build_attrs)
       end
     end
-
-    private
-
-    def config_processor
-      @config_processor ||= @pipeline.config_processor
-    end
   end
 end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index a7751b8effc..b1ee6874190 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -8,7 +8,9 @@ module Ci
         return pipeline
       end
 
-      unless commit
+      if commit
+        pipeline.sha = commit.id
+      else
         pipeline.errors.add(:base, 'Commit not found')
         return pipeline
       end
@@ -18,22 +20,18 @@ module Ci
         return pipeline
       end
 
-      begin
-        Ci::Pipeline.transaction do
-          pipeline.sha = commit.id
+      unless pipeline.config_processor
+        pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file')
+        return pipeline
+      end
 
-          unless pipeline.config_processor
-            pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file')
-            raise ActiveRecord::Rollback
-          end
+      pipeline.save!
 
-          pipeline.save!
-          pipeline.create_builds(current_user)
-        end
-      rescue
-        pipeline.errors.add(:base, 'The pipeline could not be created. Please try again.')
+      unless pipeline.create_builds(current_user)
+        pipeline.errors.add(:base, 'No builds for this pipeline.')
       end
 
+      pipeline.save
       pipeline
     end
 
diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb
index 511505bc9a9..9a187f5d694 100644
--- a/app/services/ci/register_build_service.rb
+++ b/app/services/ci/register_build_service.rb
@@ -7,15 +7,19 @@ module Ci
 
       builds =
         if current_runner.shared?
-          # don't run projects which have not enables shared runners
-          builds.joins(:project).where(projects: { builds_enabled: true, shared_runners_enabled: true })
+          builds.
+            # don't run projects which have not enabled shared runners
+            joins(:project).where(projects: { builds_enabled: true, shared_runners_enabled: true }).
+
+            # this returns builds that are ordered by number of running builds
+            # we prefer projects that don't use shared runners at all
+            joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.gl_project_id=project_builds.gl_project_id").
+            order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC')
         else
-          # do run projects which are only assigned to this runner
-          builds.where(project: current_runner.projects.where(builds_enabled: true))
+          # do run projects which are only assigned to this runner (FIFO)
+          builds.where(project: current_runner.projects.where(builds_enabled: true)).order('created_at ASC')
         end
 
-      builds = builds.order('created_at ASC')
-
       build = builds.find do |build|
         current_runner.can_pick?(build)
       end
@@ -35,5 +39,12 @@ module Ci
     rescue StateMachines::InvalidTransition
       nil
     end
+
+    private
+
+    def running_builds_for_shared_runners
+      Ci::Build.running.where(runner: Ci::Runner.shared).
+        group(:gl_project_id).select(:gl_project_id, 'count(*) AS running_builds')
+    end
   end
 end