Merge master into issue_12758issue_12758

6 files changed, 101 insertions, 61 deletions

diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb
index 3a74ae094e8..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,34 +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,
-                             :environment)
+        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 4ff268a6f06..f0ed09a629a 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|
         build.can_be_served?(current_runner)
       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
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index 418f5cf8091..f947e8f452e 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -1,15 +1,11 @@
 class CreateCommitBuildsService
   def execute(project, user, params)
-    return false unless project.builds_enabled?
+    return unless project.builds_enabled?
 
     before_sha = params[:checkout_sha] || params[:before]
     sha = params[:checkout_sha] || params[:after]
     origin_ref = params[:ref]
 
-    unless origin_ref && sha.present?
-      return false
-    end
-
     ref = Gitlab::Git.ref_name(origin_ref)
     tag = Gitlab::Git.tag_ref?(origin_ref)
 
@@ -18,23 +14,50 @@ class CreateCommitBuildsService
       return false
     end
 
-    pipeline = Ci::Pipeline.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
+    @pipeline = Ci::Pipeline.new(project: project, sha: sha, ref: ref, before_sha: before_sha, tag: tag)
 
-    # Skip creating pipeline when no gitlab-ci.yml is found
-    unless pipeline.ci_yaml_file
+    ##
+    # Skip creating pipeline if no gitlab-ci.yml is found
+    #
+    unless @pipeline.ci_yaml_file
       return false
     end
 
-    # Create a new pipeline
-    pipeline.save!
-
+    ##
     # Skip creating builds for commits that have [ci skip]
-    unless pipeline.skip_ci?
-      # Create builds for commit
-      pipeline.create_builds(user)
+    # but save pipeline object
+    #
+    if @pipeline.skip_ci?
+      return save_pipeline!
+    end
+
+    ##
+    # Skip creating builds when CI config is invalid
+    # but save pipeline object
+    #
+    unless @pipeline.config_processor
+      return save_pipeline!
     end
 
-    pipeline.touch
-    pipeline
+    ##
+    # Skip creating pipeline object if there are no builds for it.
+    #
+    unless @pipeline.create_builds(user)
+      @pipeline.errors.add(:base, 'No builds created')
+      return false
+    end
+
+    save_pipeline!
+  end
+
+  private
+
+  ##
+  # Create a new pipeline and touch object to calculate status
+  #
+  def save_pipeline!
+    @pipeline.save!
+    @pipeline.touch
+    @pipeline
   end
 end
diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index 8f5c3393dfc..d7a0c25a044 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -3,7 +3,7 @@ class GitHooksService
 
   def execute(user, repo_path, oldrev, newrev, ref)
     @repo_path  = repo_path
-    @user       = Gitlab::ShellEnv.gl_id(user)
+    @user       = Gitlab::GlId.gl_id(user)
     @oldrev     = oldrev
     @newrev     = newrev
     @ref        = ref
diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index eb73948006e..23b6668e0d1 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -11,5 +11,9 @@ module Projects
     def merge_requests
       @project.merge_requests.opened.select([:iid, :title])
     end
+
+    def labels
+      @project.labels.select([:title, :color])
+    end
   end
 end